Issue #14532: Add a secure_compare() helper to the hmac module, to mitigate

timing attacks. Patch by Jon Oberheide.

5 files changed, 94 insertions, 1 deletions

diff --git a/Doc/library/hmac.rst b/Doc/library/hmac.rst
index eff2724..e8f6488 100644
--- a/Doc/library/hmac.rst
+++ b/Doc/library/hmac.rst
@@ -38,6 +38,13 @@ An HMAC object has the following methods:
    given to the constructor.  It may contain non-ASCII bytes, including NUL
    bytes.
 
+   .. warning::
+
+      When comparing the output of :meth:`digest` to an externally-supplied
+      digest during a verification routine, it is recommended to use the
+      :func:`hmac.secure_compare` function instead of the ``==`` operator
+      to avoid potential timing attacks.
+
 
 .. method:: HMAC.hexdigest()
 
@@ -45,6 +52,13 @@ An HMAC object has the following methods:
    length containing only hexadecimal digits.  This may be used to exchange the
    value safely in email or other non-binary environments.
 
+   .. warning::
+
+      When comparing the output of :meth:`hexdigest` to an externally-supplied
+      digest during a verification routine, it is recommended to use the
+      :func:`hmac.secure_compare` function instead of the ``==`` operator
+      to avoid potential timing attacks.
+
 
 .. method:: HMAC.copy()
 
@@ -52,6 +66,24 @@ An HMAC object has the following methods:
    compute the digests of strings that share a common initial substring.
 
 
+This module also provides the following helper function:
+
+.. function:: secure_compare(a, b)
+
+   Returns the equivalent of ``a == b``, but using a time-independent
+   comparison method. Comparing the full lengths of the inputs *a* and *b*,
+   instead of short-circuiting the comparison upon the first unequal byte,
+   prevents leaking information about the inputs being compared and mitigates
+   potential timing attacks. The inputs must be either :class:`str` or
+   :class:`bytes` instances.
+
+   .. note::
+
+      While the :func:`hmac.secure_compare` function prevents leaking the
+      contents of the inputs via a timing attack, it does leak the length
+      of the inputs. However, this generally is not a security risk.
+
+
 .. seealso::
 
    Module :mod:`hashlib`
diff --git a/Lib/hmac.py b/Lib/hmac.py
index 956fc65..13ffdbe 100644
--- a/Lib/hmac.py
+++ b/Lib/hmac.py
@@ -13,6 +13,27 @@ 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")
+
+    if len(a) != len(b):
+        return False
+
+    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)
+
+    return result == 0
+
+
 class HMAC:
     """RFC 2104 HMAC class.  Also complies with RFC 4231.
 
diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py
index 4de0620..042bc5d 100644
--- a/Lib/test/test_hmac.py
+++ b/Lib/test/test_hmac.py
@@ -302,12 +302,48 @@ class CopyTestCase(unittest.TestCase):
         self.assertEqual(h1.hexdigest(), h2.hexdigest(),
             "Hexdigest of copy doesn't match original hexdigest.")
 
+class SecureCompareTestCase(unittest.TestCase):
+
+    def test_compare(self):
+        # Testing input type exception handling
+        a, b = 100, 200
+        self.assertRaises(TypeError, hmac.secure_compare, a, b)
+        a, b = 100, "foobar"
+        self.assertRaises(TypeError, hmac.secure_compare, a, b)
+        a, b = "foobar", b"foobar"
+        self.assertRaises(TypeError, hmac.secure_compare, a, b)
+
+        # Testing str/bytes of different lengths
+        a, b = "foobar", "foo"
+        self.assertFalse(hmac.secure_compare(a, b))
+        a, b = b"foobar", b"foo"
+        self.assertFalse(hmac.secure_compare(a, b))
+        a, b = b"\xde\xad\xbe\xef", b"\xde\xad"
+        self.assertFalse(hmac.secure_compare(a, b))
+
+        # Testing str/bytes of same lengths, different values
+        a, b = "foobar", "foobaz"
+        self.assertFalse(hmac.secure_compare(a, b))
+        a, b = b"foobar", b"foobaz"
+        self.assertFalse(hmac.secure_compare(a, b))
+        a, b = b"\xde\xad\xbe\xef", b"\xab\xad\x1d\xea"
+        self.assertFalse(hmac.secure_compare(a, b))
+
+        # Testing str/bytes of same lengths, same values
+        a, b = "foobar", "foobar"
+        self.assertTrue(hmac.secure_compare(a, b))
+        a, b = b"foobar", b"foobar"
+        self.assertTrue(hmac.secure_compare(a, b))
+        a, b = b"\xde\xad\xbe\xef", b"\xde\xad\xbe\xef"
+        self.assertTrue(hmac.secure_compare(a, b))
+
 def test_main():
     support.run_unittest(
         TestVectorsTestCase,
         ConstructorTestCase,
         SanityTestCase,
-        CopyTestCase
+        CopyTestCase,
+        SecureCompareTestCase
     )
 
 if __name__ == "__main__":
diff --git a/Misc/ACKS b/Misc/ACKS
index c7dc2f4..2be4bf7 100644
--- a/Misc/ACKS
+++ b/Misc/ACKS
@@ -746,6 +746,7 @@ Nigel O'Brian
 John O'Connor
 Kevin O'Connor
 Tim O'Malley
+Jon Oberheide
 Pascal Oberndoerfer
 Jeffrey Ollie
 Adam Olsen
diff --git a/Misc/NEWS b/Misc/NEWS
index 9adce9f..031738d 100644
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -23,6 +23,9 @@ Core and Builtins
 Library
 -------
 
+- Issue #14532: Add a secure_compare() helper to the hmac module, to mitigate
+  timing attacks. Patch by Jon Oberheide. 
+
 - Add importlib.util.resolve_name().
 
 - Issue #14366: Support lzma compression in zip files.