Fix changing of auth_type in existing deployments

This changes does two things:
* Adds Sha512 as a supported auth_type.
* Fixes breakage when auth_type is changed in existing deployments.

If an existing gswauth deployment changes `auth_type` in conf file to a
different one (for example: sha1 to sha512), all attempts to authorize
existing/old users will fail because of change in encoder type.

With this change, the credentials match is done using an encoder with
which the password was initially encoded. This allows gswauth deployments
to change auth_type and old users will still be able to authorize.

A note on auth_type_salt:
There's still a distinction between how salt is managed in gswauth and
swauth: swauth will use a random salt if a salt is not set in conf file
where as gswauth will default to 'gswauthsalt' if a salt is not set in
conf file. This distinction is to ensure backward compatibility.

This change is derived from following upstream changes in swauth repo:
e14a7b3df86969d478090b314d9660b6d835afa7
https://review.openstack.org/#/c/285195/
https://review.openstack.org/#/c/285292/

Change-Id: I9a43adc4964d8e9f9f1faf73063a6dc1cd8ff354
Signed-off-by: Prashanth Pai <ppai@redhat.com>
Reviewed-on: http://review.gluster.org/13654
Reviewed-by: Thiago da Silva <thiago@redhat.com>
Tested-by: Thiago da Silva <thiago@redhat.com>

3 files changed, 136 insertions, 8 deletions

diff --git a/gluster/swift/common/middleware/gswauth/swauth/authtypes.py b/gluster/swift/common/middleware/gswauth/swauth/authtypes.py
index 90aad72..fbf532d 100644
--- a/gluster/swift/common/middleware/gswauth/swauth/authtypes.py
+++ b/gluster/swift/common/middleware/gswauth/swauth/authtypes.py
@@ -35,6 +35,7 @@ value or to a default value.
 """
 
 import hashlib
+import os
 
 
 #: Maximum length any valid token should ever be.
@@ -80,6 +81,20 @@ class Sha1(object):
     must be capitalized. encode and match methods must be provided and are
     the only ones that will be used by swauth.
     """
+
+    def encode_w_salt(self, salt, key):
+        """
+        Encodes a user key with salt into a particular format. The result of
+        this method will be used internally.
+
+        :param salt: Salt for hashing
+        :param key: User's secret key
+        :returns: A string representing user credentials
+        """
+        enc_key = '%s%s' % (salt, key)
+        enc_val = hashlib.sha1(enc_key).hexdigest()
+        return "sha1:%s$%s" % (salt, enc_val)
+
     def encode(self, key):
         """
         Encodes a user key into a particular format. The result of this method
@@ -88,9 +103,8 @@ class Sha1(object):
         :param key: User's secret key
         :returns: A string representing user credentials
         """
-        enc_key = '%s%s' % (self.salt, key)
-        enc_val = hashlib.sha1(enc_key).hexdigest()
-        return "sha1:%s$%s" % (self.salt, enc_val)
+        salt = self.salt or os.urandom(32).encode('base64').rstrip()
+        return self.encode_w_salt(salt, key)
 
     def match(self, key, creds):
         """
@@ -100,4 +114,59 @@ class Sha1(object):
         :param creds: User's stored credentials
         :returns: True if the supplied key is valid, False otherwise
         """
-        return self.encode(key) == creds
+
+        type, rest = creds.split(':')
+        salt, enc = rest.split('$')
+
+        return self.encode_w_salt(salt, key) == creds
+
+
+class Sha512(object):
+    """
+    Provides a particular auth type for encoding format for encoding and
+    matching user keys.
+
+    This class must be all lowercase except for the first character, which
+    must be capitalized. encode and match methods must be provided and are
+    the only ones that will be used by swauth.
+    """
+
+    def encode_w_salt(self, salt, key):
+        """
+        Encodes a user key with salt into a particular format. The result of
+        this method will be used internal.
+
+        :param salt: Salt for hashing
+        :param key: User's secret key
+        :returns: A string representing user credentials
+        """
+        enc_key = '%s%s' % (salt, key)
+        enc_val = hashlib.sha512(enc_key).hexdigest()
+        return "sha512:%s$%s" % (salt, enc_val)
+
+    def encode(self, key):
+        """
+        Encodes a user key into a particular format. The result of this method
+        will be used by swauth for storing user credentials.
+
+        If salt is not manually set in conf file, a random salt will be
+        generated and used.
+
+        :param key: User's secret key
+        :returns: A string representing user credentials
+        """
+        salt = self.salt or os.urandom(32).encode('base64').rstrip()
+        return self.encode_w_salt(salt, key)
+
+    def match(self, key, creds):
+        """Checks whether the user-provided key matches the user's credentials
+
+        :param key: User-supplied key
+        :param creds: User's stored credentials
+        :returns: True if the supplied key is valid, False otherwise
+        """
+
+        type, rest = creds.split(':')
+        salt, enc = rest.split('$')
+
+        return self.encode_w_salt(salt, key) == creds
diff --git a/gluster/swift/common/middleware/gswauth/swauth/middleware.py b/gluster/swift/common/middleware/gswauth/swauth/middleware.py
index e181ece..745c6f1 100644
--- a/gluster/swift/common/middleware/gswauth/swauth/middleware.py
+++ b/gluster/swift/common/middleware/gswauth/swauth/middleware.py
@@ -1475,14 +1475,21 @@ class Swauth(object):
     def credentials_match(self, user_detail, key):
         """
         Returns True if the key is valid for the user_detail.
-        It will use self.auth_encoder to check for a key match.
+        It will use auth_encoder type the password was encoded with,
+        to check for a key match.
 
         :param user_detail: The dict for the user.
         :param key: The key to validate for the user.
         :returns: True if the key is valid for the user, False if not.
         """
-        return user_detail and self.auth_encoder().match(
-            key, user_detail.get('auth'))
+        if user_detail:
+            creds = user_detail.get('auth')
+            auth_type = creds.split(':')[0]
+            auth_encoder = getattr(authtypes, auth_type.title(), None)
+            if auth_encoder is None:
+                self.logger.error('Invalid auth_type %s' % auth_type)
+                return False
+        return user_detail and auth_encoder().match(key, creds)
 
     def is_user_changing_own_key(self, req, user):
         """
diff --git a/test/unit/common/middleware/gswauth/swauth/test_middleware.py b/test/unit/common/middleware/gswauth/swauth/test_middleware.py
index 67abbe4..2d30082 100644
--- a/test/unit/common/middleware/gswauth/swauth/test_middleware.py
+++ b/test/unit/common/middleware/gswauth/swauth/test_middleware.py
@@ -18,8 +18,8 @@ try:
 except ImportError:
     import json
 import unittest
-from nose import SkipTest
 from contextlib import contextmanager
+import mock
 from time import time
 
 from swift.common.swob import Request, Response
@@ -126,6 +126,33 @@ class TestAuth(unittest.TestCase):
                 'token_life': str(DEFAULT_TOKEN_LIFE),
                 'max_token_life': str(MAX_TOKEN_LIFE)})(FakeApp())
 
+    def test_salt(self):
+        for auth_type in ('sha1', 'sha512'):
+            # Salt not manually set (gswauthsalt should be default)
+            test_auth = \
+                auth.filter_factory({
+                    'super_admin_key': 'supertest',
+                    'token_life': str(DEFAULT_TOKEN_LIFE),
+                    'max_token_life': str(MAX_TOKEN_LIFE),
+                    'auth_type': auth_type})(FakeApp())
+            self.assertEqual(test_auth.auth_encoder.salt, "gswauthsalt")
+            h_key = test_auth.auth_encoder().encode("key")
+            prefix = auth_type + ":" + "gswauthsalt" + '$'
+            self.assertTrue(h_key.startswith(prefix))
+
+            # Salt manually set
+            test_auth = \
+                auth.filter_factory({
+                    'super_admin_key': 'supertest',
+                    'token_life': str(DEFAULT_TOKEN_LIFE),
+                    'max_token_life': str(MAX_TOKEN_LIFE),
+                    'auth_type': auth_type,
+                    'auth_type_salt': "mysalt"})(FakeApp())
+            self.assertEqual(test_auth.auth_encoder.salt, "mysalt")
+            h_key = test_auth.auth_encoder().encode("key")
+            prefix = auth_type + ":" + "mysalt" + '$'
+            self.assertTrue(h_key.startswith(prefix))
+
     def test_super_admin_key_not_required(self):
         auth.filter_factory({})(FakeApp())
 
@@ -237,6 +264,31 @@ class TestAuth(unittest.TestCase):
             ath.dsc_url2,
             'http://host2/path2')
 
+    def test_credentials_match_auth_encoder_type(self):
+        plaintext_auth = {'auth': 'plaintext:key'}
+        sha1_key = ("sha1:T0YFdhqN4uDRWiYLxWa7H2T8AewG4fEYQyJFRLsgcfk=$46c58"
+                    "07eb8a32e8f404fea9eaaeb60b7e1207ff1")
+        sha1_auth = {'auth': sha1_key}
+        sha512_key = ("sha512:aSm0jEeqIp46T5YLZy1r8+cXs/Xzs1S4VUwVauhBs44=$ef"
+                      "7332ec1288bf69c75682eb8d459d5a84baa7e43f45949c242a9af9"
+                      "7130ef16ac361fe1aa33a789e218122b83c54ef1923fc015080741"
+                      "ca21f6187329f6cb7a")
+        sha512_auth = {'auth': sha512_key}
+
+        # test all possible config settings work with all possible auth types
+        for auth_type in ('plaintext', 'sha1', 'sha512'):
+            test_auth = auth.filter_factory({'super_admin_key': 'superkey',
+                                            'auth_type': auth_type})(FakeApp())
+            for detail in (plaintext_auth, sha1_auth, sha512_auth):
+                self.assertTrue(test_auth.credentials_match(detail, 'key'))
+            # test invalid auth type stored
+            invalid_detail = {'auth': 'Junk:key'}
+            test_auth.logger = mock.Mock()
+            self.assertFalse(test_auth.credentials_match(invalid_detail,
+                                                         'key'))
+            # make sure error is logged
+            test_auth.logger.called_once_with('Invalid auth_type Junk')
+
     def test_top_level_denied(self):
         resp = Request.blank(
             '/').get_response(self.test_auth)