Merge "Deprecate redundant constant_time_compare function"

oslo_utils/secretutils.py

@@ -24,29 +24,10 @@ import hmac
 import debtcollector.removals
 
 
-def _constant_time_compare(first, second):
-    """Return True if both string or binary inputs are equal, otherwise False.
-
-    This function should take a constant amount of time regardless of
-    how many characters in the strings match. This function uses an
-    approach designed to prevent timing analysis by avoiding
-    content-based short circuiting behaviour, making it appropriate
-    for cryptography.
-    """
-    first = str(first)
-    second = str(second)
-    if len(first) != len(second):
-        return False
-    result = 0
-    for x, y in zip(first, second):
-        result |= ord(x) ^ ord(y)
-    return result == 0
-
-
-try:
-    constant_time_compare = hmac.compare_digest
-except AttributeError:
-    constant_time_compare = _constant_time_compare
+@debtcollector.removals.remove(message='Use hmac.compare_digest instead',
+                               category=PendingDeprecationWarning)
+def constant_time_compare(*args, **kwargs):
+    return hmac.compare_digest(*args, **kwargs)
 
 
 @debtcollector.removals.remove(message='Use hashlib.md5 instead',

oslo_utils/tests/test_secretutils.py

@@ -31,37 +31,6 @@ class SecretUtilsTest(testscenarios.TestWithScenarios,
         ('unicode', {'converter': lambda text: text}),
     ]
 
-    def test_constant_time_compare(self):
-        # make sure it works as a compare, the "constant time" aspect
-        # isn't appropriate to test in unittests
-
-        # Make sure the unittests are applied to our function instead of
-        # the built-in function, otherwise that is in vain.
-        ctc = secretutils._constant_time_compare
-
-        self.assertTrue(ctc(self.converter('abcd'),
-                            self.converter('abcd')))
-        self.assertTrue(ctc(self.converter(''),
-                            self.converter('')))
-        self.assertTrue(ctc('abcd', 'abcd'))
-        self.assertFalse(ctc(self.converter('abcd'),
-                             self.converter('efgh')))
-        self.assertFalse(ctc(self.converter('abc'),
-                             self.converter('abcd')))
-        self.assertFalse(ctc(self.converter('abc'),
-                             self.converter('abc\x00')))
-        self.assertFalse(ctc(self.converter(''),
-                             self.converter('abc')))
-        self.assertTrue(ctc(self.converter('abcd1234'),
-                            self.converter('abcd1234')))
-        self.assertFalse(ctc(self.converter('abcd1234'),
-                             self.converter('ABCD234')))
-        self.assertFalse(ctc(self.converter('abcd1234'),
-                             self.converter('a')))
-        self.assertFalse(ctc(self.converter('abcd1234'),
-                             self.converter('1234abcd')))
-        self.assertFalse(ctc('abcd1234', '1234abcd'))
-
     _test_data = "Openstack forever".encode('utf-8')
     _md5_digest = hashlib.md5(_test_data).digest()
 

releasenotes/notes/deprecate-constant_time_compare-53669f464c9811c1.yaml

@@ -0,0 +1,6 @@
+---
+deprecations:
+  - |
+    The ``oslo_utils.secretutils.constant_time_compare`` function has been
+    deprecated. Use the ``compare_digest`` function from the built-in ``hmac``
+    module.