From ed593b94234b222cb53d73e49283ae027d3ada1c Mon Sep 17 00:00:00 2001 From: Russell Bryant Date: Tue, 28 Feb 2012 11:42:19 -0500 Subject: [PATCH] Use constant time string comparisons for auth. Fix bug 942644. Use constant time string comparisons when doing authentication to help guard against timing attacks. --- AUTHORS | 1 + keystone/common/utils.py | 20 ++++++++++++++++++++ keystone/contrib/ec2/core.py | 4 ++-- keystone/contrib/s3/core.py | 3 ++- tests/test_utils.py | 5 +++++ 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/AUTHORS b/AUTHORS index 890583d..1a6bdb6 100644 --- a/AUTHORS +++ b/AUTHORS @@ -76,6 +76,7 @@ Ramana Juvvadi Robin Norwood root root +Russell Bryant saikrishna1511@gmail.com Sai Krishna Salvatore Orlando diff --git a/keystone/common/utils.py b/keystone/common/utils.py index 2621386..427db46 100644 --- a/keystone/common/utils.py +++ b/keystone/common/utils.py @@ -241,3 +241,23 @@ def unixtime(dt_obj): """ return time.mktime(dt_obj.utctimetuple()) + + +def strcmp_const_time(s1, s2): + """Constant-time string comparison. + + :params s1: the first string + :params s2: the second string + + :return: True if the strings are equal. + + This function takes two strings and compares them. It is intended to be + used when doing a comparison for authentication purposes to help guard + against timing attacks. + """ + if len(s1) != len(s2): + return False + result = 0 + for (a, b) in zip(s1, s2): + result |= ord(a) ^ ord(b) + return result == 0 diff --git a/keystone/contrib/ec2/core.py b/keystone/contrib/ec2/core.py index 874afbe..a189ef8 100644 --- a/keystone/contrib/ec2/core.py +++ b/keystone/contrib/ec2/core.py @@ -105,7 +105,7 @@ class Ec2Controller(wsgi.Application): def check_signature(self, creds_ref, credentials): signer = utils.Ec2Signer(creds_ref['secret']) signature = signer.generate(credentials) - if signature == credentials['signature']: + if utils.strcmp_const_time(signature, credentials['signature']): return # NOTE(vish): Some libraries don't use the port when signing # requests, so try again without port. @@ -113,7 +113,7 @@ class Ec2Controller(wsgi.Application): hostname, _port = credentials['host'].split(':') credentials['host'] = hostname signature = signer.generate(credentials) - if signature != credentials.signature: + if not utils.strcmp_const_time(signature, credentials.signature): # TODO(termie): proper exception msg = 'Invalid signature' raise webob.exc.HTTPUnauthorized(explanation=msg) diff --git a/keystone/contrib/s3/core.py b/keystone/contrib/s3/core.py index ff68cc6..cbeeeb1 100644 --- a/keystone/contrib/s3/core.py +++ b/keystone/contrib/s3/core.py @@ -25,6 +25,7 @@ import hmac from hashlib import sha1 from keystone import config +from keystone.common import utils from keystone.common import wsgi from keystone.contrib import ec2 @@ -47,5 +48,5 @@ class S3Controller(ec2.Ec2Controller): key = str(creds_ref['secret']) signed = base64.encodestring(hmac.new(key, msg, sha1).digest()).strip() - if credentials['signature'] != signed: + if not utils.strcmp_const_time(credentials['signature'], signed): raise Exception('Not Authorized') diff --git a/tests/test_utils.py b/tests/test_utils.py index 6a8249f..91a7523 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -61,3 +61,8 @@ class UtilsTestCase(test.TestCase): output = utils.isotime(dt) expected = '1987-10-13T01:02:03Z' self.assertEqual(output, expected) + + def test_strcmp_const_time(self): + self.assertTrue(utils.strcmp_const_time('abc123', 'abc123')) + self.assertFalse(utils.strcmp_const_time('a', 'aaaaa')) + self.assertFalse(utils.strcmp_const_time('ABC123', 'abc123')) -- 1.7.7.6