From da465aca560ee838a50decadf86950d3dc954b11 Mon Sep 17 00:00:00 2001
From: Russell Bryant <rbryant@redhat.com>
Date: Tue, 28 Feb 2012 10:55:38 -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.

Change-Id: Iaaefb13f7618b06834630d9ccb97aff056b4bf4c
---
 nova/api/openstack/auth.py |    2 +-
 nova/auth/manager.py       |    6 +++---
 nova/tests/test_utils.py   |    5 +++++
 nova/utils.py              |   20 ++++++++++++++++++++
 4 files changed, 29 insertions(+), 4 deletions(-)

diff --git a/nova/api/openstack/auth.py b/nova/api/openstack/auth.py
index 544b101..18aaf83 100644
--- a/nova/api/openstack/auth.py
+++ b/nova/api/openstack/auth.py
@@ -242,7 +242,7 @@ class AuthMiddleware(base_wsgi.Middleware):
             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 = {}
diff --git a/nova/auth/manager.py b/nova/auth/manager.py
index e2516bc..23d9cee 100644
--- a/nova/auth/manager.py
+++ b/nova/auth/manager.py
@@ -328,7 +328,7 @@ class AuthManager(object):
             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)
@@ -340,7 +340,7 @@ class AuthManager(object):
             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 != '':
@@ -349,7 +349,7 @@ class AuthManager(object):
                                                        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,
diff --git a/nova/tests/test_utils.py b/nova/tests/test_utils.py
index 843b48d..94b8719 100644
--- a/nova/tests/test_utils.py
+++ b/nova/tests/test_utils.py
@@ -398,6 +398,11 @@ class GenericUtilsTestCase(test.TestCase):
         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):
diff --git a/nova/utils.py b/nova/utils.py
index ef49321..a9bea5e 100644
--- a/nova/utils.py
+++ b/nova/utils.py
@@ -1557,3 +1557,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
-- 
1.7.7.6