From e158c10ccb80963ad8009beedfc1fda4a74628d0 Mon Sep 17 00:00:00 2001
From: changxun <zhoucx@dtdream.com>
Date: Wed, 23 May 2018 17:13:47 +0800
Subject: [PATCH] Fix exception with secretutils

1. There are some problems about the test method.
 problem 1:
  Unit tests may not cover our function, it depends on the python version
  that performed the test.
 problem 2:
  when using function 'constant_time_compare(first, second)', 'first' and
  'second' params are usually HMAC digest values, it is not appropriate to
  use utf-8 encoded values as mock data.

2. The previous commit `f1d332a` lead into a bug, but due to the problem 1
   and the problem 2, we did not find out the error.

Change-Id: I1c29bfe69f8eda60f3c5caaf3e5447dd5b69b108
Closes-Bug: #1772851
---
 oslo_utils/secretutils.py            | 38 +++++++++++++---------------
 oslo_utils/tests/test_secretutils.py | 11 ++++++--
 2 files changed, 27 insertions(+), 22 deletions(-)

diff --git a/oslo_utils/secretutils.py b/oslo_utils/secretutils.py
index 09b1b739..963c6341 100644
--- a/oslo_utils/secretutils.py
+++ b/oslo_utils/secretutils.py
@@ -20,28 +20,26 @@ Secret utilities.
 
 import hmac
 
-import six
 
+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:
-    def constant_time_compare(first, second):
-        """Returns True if both string 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.
-        """
-        if isinstance(first, six.string_types):
-            first = first.encode('utf-8')
-        if isinstance(second, six.string_types):
-            second = second.encode('utf-8')
-        if len(first) != len(second):
-            return False
-        result = 0
-        for x, y in zip(first, second):
-            result |= ord(x) ^ ord(y)
-        return result == 0
+    constant_time_compare = _constant_time_compare
diff --git a/oslo_utils/tests/test_secretutils.py b/oslo_utils/tests/test_secretutils.py
index 1c4b366b..faea3dd3 100644
--- a/oslo_utils/tests/test_secretutils.py
+++ b/oslo_utils/tests/test_secretutils.py
@@ -12,6 +12,8 @@
 #    License for the specific language governing permissions and limitations
 #    under the License.
 
+import hmac
+
 from oslotest import base as test_base
 import testscenarios
 
@@ -21,15 +23,20 @@ from oslo_utils import secretutils
 class SecretUtilsTest(testscenarios.TestWithScenarios,
                       test_base.BaseTestCase):
 
+    _gen_digest = lambda text: hmac.new(b'foo', text.encode('utf-8')).digest()
     scenarios = [
-        ('binary', {'converter': lambda text: text.encode('utf-8')}),
+        ('binary', {'converter': _gen_digest}),
         ('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
-        ctc = secretutils.constant_time_compare
+
+        # 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(u'abcd'),
                             self.converter(u'abcd')))
         self.assertTrue(ctc(self.converter(u''),