From 5d15daaab62c95a249c19feac4074728cba9e292 Mon Sep 17 00:00:00 2001 From: Prashanth Pai Date: Tue, 7 Jun 2016 14:20:47 +0530 Subject: [PATCH] Don't include salt in HMAC computation Currently, the input to HMAC function is the entire stored credential in the format '$` but it should rather be only the hashed key/password. With this change, validate_creds() method is invoked and only the hash of the password is used in HMAC computation. Change-Id: I1a9bbcac6f49c23f3256572f148e55249a59f7ed Signed-off-by: Prashanth Pai --- swauth/middleware.py | 10 +++++++++- test/unit/test_middleware.py | 24 ++++++++++++++++++++++++ 2 files changed, 33 insertions(+), 1 deletion(-) diff --git a/swauth/middleware.py b/swauth/middleware.py index 0bf6c6d..3fd5316 100644 --- a/swauth/middleware.py +++ b/swauth/middleware.py @@ -344,8 +344,16 @@ class Swauth(object): env['PATH_INFO'] = path.replace("%s:%s" % (account, user), account_id, 1) detail = json.loads(resp.body) + if detail: + creds = detail.get('auth') + try: + auth_encoder, creds_dict = \ + swauth.authtypes.validate_creds(creds) + except ValueError as e: + self.logger.error('%s' % e.args[0]) + return None - password = detail['auth'].split(':')[-1] + password = creds_dict['hash'] msg = base64.urlsafe_b64decode(unquote(token)) # https://bugs.python.org/issue5285 diff --git a/test/unit/test_middleware.py b/test/unit/test_middleware.py index af8218c..1ed3314 100644 --- a/test/unit/test_middleware.py +++ b/test/unit/test_middleware.py @@ -14,6 +14,7 @@ # limitations under the License. from contextlib import contextmanager +import hashlib import json import mock from time import time @@ -4062,6 +4063,29 @@ class TestAuth(unittest.TestCase): 'ozNCArMDAwMAovY29udGFpbmVyMw==' self.assertEqual(self.test_auth.get_groups(env, token), None) + def test_s3_only_hash_passed_to_hmac(self): + key = 'dadada' + salt = 'zuck' + key_hash = hashlib.sha1('%s%s' % (salt, key)).hexdigest() + auth_stored = "sha1:%s$%s" % (salt, key_hash) + self.test_auth.app = FakeApp(iter([ + ('200 Ok', {}, + json.dumps({"auth": auth_stored, + "groups": [{'name': "act:usr"}, {'name': "act"}, + {'name': ".admin"}]})), + ('204 Ok', {'X-Container-Meta-Account-Id': 'AUTH_act'}, '')])) + env = \ + {'HTTP_AUTHORIZATION': 'AWS act:user:whatever', + 'PATH_INFO': '/v1/AUTH_act/c1'} + token = 'UFVUCgoKRnJpLCAyNiBGZWIgMjAxNiAwNjo0NT'\ + 'ozNCArMDAwMAovY29udGFpbmVyMw==' + mock_hmac_new = mock.MagicMock() + with mock.patch('hmac.new', mock_hmac_new): + self.test_auth.get_groups(env, token) + self.assertTrue(mock_hmac_new.called) + # Assert that string passed to hmac.new is only the hash + self.assertEqual(mock_hmac_new.call_args[0][0], key_hash) + if __name__ == '__main__': unittest.main()