Issue #15061: Don't oversell the capabilities of the new non-shortcircuiting comparison function in hmac

1 files changed, 14 insertions, 14 deletions

diff --git a/Lib/hmac.py b/Lib/hmac.py
index 13ffdbe..e47965b 100644
--- a/Lib/hmac.py
+++ b/Lib/hmac.py
@@ -13,24 +13,24 @@ trans_36 = bytes((x ^ 0x36) for x in range(256))
 digest_size = None
 
 
-def secure_compare(a, b):
-    """Returns the equivalent of 'a == b', but using a time-independent
-    comparison method to prevent timing attacks."""
-    if not ((isinstance(a, str) and isinstance(b, str)) or
-            (isinstance(a, bytes) and isinstance(b, bytes))):
-        raise TypeError("inputs must be strings or bytes")
-
+def compare_digest(a, b):
+    """Returns the equivalent of 'a == b', but avoids content based short
+    circuiting to reduce the vulnerability to timing attacks."""
+    # Consistent timing matters more here than data type flexibility
+    if not (isinstance(a, bytes) and isinstance(b, bytes)):
+        raise TypeError("inputs must be bytes instances")
+
+    # We assume the length of the expected digest is public knowledge,
+    # thus this early return isn't leaking anything an attacker wouldn't
+    # already know
     if len(a) != len(b):
         return False
 
+    # We assume that integers in the bytes range are all cached,
+    # thus timing shouldn't vary much due to integer object creation
     result = 0
-    if isinstance(a, bytes):
-        for x, y in zip(a, b):
-            result |= x ^ y
-    else:
-        for x, y in zip(a, b):
-            result |= ord(x) ^ ord(y)
-
+    for x, y in zip(a, b):
+        result |= x ^ y
     return result == 0