Improve auth_str_equal().

This patch is to improve auth_str_equal() a bit.  The whole point of
this function is to do a string comparison in constant time to help
protect against timing attacks.  The original implementation had a bit
of a silly property in that it would exit early if the strings were not
of the same length.  This would theoretically still allow someone to
discover the proper length of a password.

This patch moves the length verification to the end.  It also makes it
so the main loop time to run is a function of the provided password
length instead of the length of the shorter of the two strings.

Change-Id: I6dbe076818b7e3e8a313544ebd5c5734b5a025e5

3 files changed, 16 insertions, 11 deletions

diff --git a/keystone/common/utils.py b/keystone/common/utils.py
index 254070c1..b7d4b9c0 100644
--- a/keystone/common/utils.py
+++ b/keystone/common/utils.py
@@ -243,21 +243,25 @@ def unixtime(dt_obj):
     return time.mktime(dt_obj.utctimetuple())
 
 
-def auth_str_equal(s1, s2):
+def auth_str_equal(provided, known):
     """Constant-time string comparison.
 
-    :params s1: the first string
-    :params s2: the second string
+    :params provided: the first string
+    :params known: 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.
+    against timing attacks.  When using the function for this purpose, always
+    provide the user-provided password as the first argument.  The time this
+    function will take is always a factor of the length of this string.
     """
-    if len(s1) != len(s2):
-        return False
     result = 0
-    for (a, b) in zip(s1, s2):
-        result |= ord(a) ^ ord(b)
-    return result == 0
+    p_len = len(provided)
+    k_len = len(known)
+    for i in xrange(p_len):
+        a = ord(provided[i]) if i < p_len else 0
+        b = ord(known[i]) if i < k_len else 0
+        result |= a ^ b
+    return (p_len == k_len) & (result == 0)
diff --git a/keystone/contrib/ec2/core.py b/keystone/contrib/ec2/core.py
index 24e1aab4..60d1329f 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 utils.auth_str_equal(signature, credentials['signature']):
+        if utils.auth_str_equal(credentials['signature'], 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 not utils.auth_str_equal(signature, credentials.signature):
+            if not utils.auth_str_equal(credentials.signature, signature):
                 # TODO(termie): proper exception
                 msg = 'Invalid signature'
                 raise webob.exc.HTTPUnauthorized(explanation=msg)
diff --git a/tests/test_utils.py b/tests/test_utils.py
index 1f4123b8..b551b815 100644
--- a/tests/test_utils.py
+++ b/tests/test_utils.py
@@ -65,4 +65,5 @@ class UtilsTestCase(test.TestCase):
     def test_auth_str_equal(self):
         self.assertTrue(utils.auth_str_equal('abc123', 'abc123'))
         self.assertFalse(utils.auth_str_equal('a', 'aaaaa'))
+        self.assertFalse(utils.auth_str_equal('aaaaa', 'a'))
         self.assertFalse(utils.auth_str_equal('ABC123', 'abc123'))