Skip to content

Commit

Permalink
Use constant time string comparisons for auth.
Browse files Browse the repository at this point in the history
Fix bug 942644.

Use constant time string comparisons when doing authentication to help
guard against timing attacks.

Change-Id: Iaaefb13f7618b06834630d9ccb97aff056b4bf4c
  • Loading branch information
russellb committed Feb 28, 2012
1 parent f9d23c6 commit 1ea9986
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 4 deletions.
2 changes: 1 addition & 1 deletion nova/api/openstack/auth.py
Expand Up @@ -242,7 +242,7 @@ def _authorize_user(self, username, key, req):
LOG.warn(_("User not found with provided API key."))
user = None

if user and user.name == username:
if user and utils.strcmp_const_time(user.name, username):
token_hash = hashlib.sha1('%s%s%f' % (username, key,
time.time())).hexdigest()
token_dict = {}
Expand Down
6 changes: 3 additions & 3 deletions nova/auth/manager.py
Expand Up @@ -328,7 +328,7 @@ def authenticate(self, access, signature, params, verb='GET',
LOG.debug(_('user.secret: %s'), user.secret)
LOG.debug(_('expected_signature: %s'), expected_signature)
LOG.debug(_('signature: %s'), signature)
if signature != expected_signature:
if not utils.strcmp_const_time(signature, expected_signature):
LOG.audit(_("Invalid signature for user %s"), user.name)
raise exception.InvalidSignature(signature=signature,
user=user)
Expand All @@ -340,7 +340,7 @@ def authenticate(self, access, signature, params, verb='GET',
LOG.debug(_('user.secret: %s'), user.secret)
LOG.debug(_('expected_signature: %s'), expected_signature)
LOG.debug(_('signature: %s'), signature)
if signature != expected_signature:
if not utils.strcmp_const_time(signature, expected_signature):
(addr_str, port_str) = utils.parse_server_string(server_string)
# If the given server_string contains port num, try without it.
if port_str != '':
Expand All @@ -349,7 +349,7 @@ def authenticate(self, access, signature, params, verb='GET',
addr_str, path)
LOG.debug(_('host_only_signature: %s'),
host_only_signature)
if signature == host_only_signature:
if utils.strcmp_const_time(signature, host_only_signature):
return (user, project)
LOG.audit(_("Invalid signature for user %s"), user.name)
raise exception.InvalidSignature(signature=signature,
Expand Down
5 changes: 5 additions & 0 deletions nova/tests/test_utils.py
Expand Up @@ -398,6 +398,11 @@ def fake_execute(*args, **kwargs):
self.assertRaises(exception.FileNotFound,
utils.read_file_as_root, 'bad')

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'))


class IsUUIDLikeTestCase(test.TestCase):
def assertUUIDLike(self, val, expected):
Expand Down
20 changes: 20 additions & 0 deletions nova/utils.py
Expand Up @@ -1560,3 +1560,23 @@ def tempdir(**kwargs):
shutil.rmtree(tmpdir)
except OSError, e:
LOG.debug(_('Could not remove tmpdir: %s'), str(e))


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

0 comments on commit 1ea9986

Please sign in to comment.