[Python-checkins] cpython: Issue #15061: Don't oversell the capabilities of the new non-shortcircuiting
nick.coghlan
python-checkins at python.org
Fri Jun 15 13:14:47 CEST 2012
http://hg.python.org/cpython/rev/f36af3766a20
changeset: 77436:f36af3766a20
user: Nick Coghlan <ncoghlan at gmail.com>
date: Fri Jun 15 21:14:08 2012 +1000
summary:
Issue #15061: Don't oversell the capabilities of the new non-shortcircuiting comparison function in hmac
files:
Doc/library/hmac.rst | 41 +++++++++++++++++-----------
Lib/hmac.py | 26 +++++++++---------
Lib/test/test_hmac.py | 44 ++++++++++++++++--------------
Misc/NEWS | 5 +++
4 files changed, 66 insertions(+), 50 deletions(-)
diff --git a/Doc/library/hmac.rst b/Doc/library/hmac.rst
--- a/Doc/library/hmac.rst
+++ b/Doc/library/hmac.rst
@@ -42,8 +42,8 @@
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.
+ :func:`compare_digest` function instead of the ``==`` operator
+ to reduce the vulnerability to timing attacks.
.. method:: HMAC.hexdigest()
@@ -54,10 +54,11 @@
.. 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.
+ The output of :meth:`hexdigest` should not be compared directly to an
+ externally-supplied digest during a verification routine. Instead, the
+ externally supplied digest should be converted to a :class:`bytes`
+ value and compared to the output of :meth:`digest` with
+ :func:`compare_digest`.
.. method:: HMAC.copy()
@@ -68,20 +69,28 @@
This module also provides the following helper function:
-.. function:: secure_compare(a, b)
+.. function:: compare_digest(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.
+ Returns the equivalent of ``a == b``, but avoids content based
+ short circuiting behaviour to reduce the vulnerability to timing
+ analysis. The inputs must be :class:`bytes` instances.
+
+ Using a short circuiting comparison (that is, one that terminates as soon
+ as it finds any difference between the values) to check digests for
+ correctness can be problematic, as it introduces a potential
+ vulnerability when an attacker can control both the message to be checked
+ *and* the purported signature value. By keeping the plaintext consistent
+ and supplying different signature values, an attacker may be able to use
+ timing variations to search the signature space for the expected value in
+ O(n) time rather than the desired O(2**n).
.. 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.
+ While this function reduces the likelihood of leaking the contents of
+ the expected digest via a timing attack, it still uses short circuiting
+ behaviour based on the *length* of the inputs. It is assumed that the
+ expected length of the digest is not a secret, as it is typically
+ published as part of a file format, network protocol or API definition.
.. versionadded:: 3.3
diff --git a/Lib/hmac.py b/Lib/hmac.py
--- a/Lib/hmac.py
+++ b/Lib/hmac.py
@@ -13,24 +13,24 @@
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
diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py
--- a/Lib/test/test_hmac.py
+++ b/Lib/test/test_hmac.py
@@ -302,40 +302,42 @@
self.assertEqual(h1.hexdigest(), h2.hexdigest(),
"Hexdigest of copy doesn't match original hexdigest.")
-class SecureCompareTestCase(unittest.TestCase):
+class CompareDigestTestCase(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)
+ self.assertRaises(TypeError, hmac.compare_digest, a, b)
+ a, b = 100, b"foobar"
+ self.assertRaises(TypeError, hmac.compare_digest, a, b)
+ a, b = b"foobar", 200
+ self.assertRaises(TypeError, hmac.compare_digest, a, b)
a, b = "foobar", b"foobar"
- self.assertRaises(TypeError, hmac.secure_compare, a, b)
+ self.assertRaises(TypeError, hmac.compare_digest, a, b)
+ a, b = b"foobar", "foobar"
+ self.assertRaises(TypeError, hmac.compare_digest, a, b)
+ a, b = "foobar", "foobar"
+ self.assertRaises(TypeError, hmac.compare_digest, a, b)
+ a, b = bytearray(b"foobar"), bytearray(b"foobar")
+ self.assertRaises(TypeError, hmac.compare_digest, a, b)
- # Testing str/bytes of different lengths
- a, b = "foobar", "foo"
- self.assertFalse(hmac.secure_compare(a, b))
+ # Testing bytes of different lengths
a, b = b"foobar", b"foo"
- self.assertFalse(hmac.secure_compare(a, b))
+ self.assertFalse(hmac.compare_digest(a, b))
a, b = b"\xde\xad\xbe\xef", b"\xde\xad"
- self.assertFalse(hmac.secure_compare(a, b))
+ self.assertFalse(hmac.compare_digest(a, b))
- # Testing str/bytes of same lengths, different values
- a, b = "foobar", "foobaz"
- self.assertFalse(hmac.secure_compare(a, b))
+ # Testing bytes of same lengths, different values
a, b = b"foobar", b"foobaz"
- self.assertFalse(hmac.secure_compare(a, b))
+ self.assertFalse(hmac.compare_digest(a, b))
a, b = b"\xde\xad\xbe\xef", b"\xab\xad\x1d\xea"
- self.assertFalse(hmac.secure_compare(a, b))
+ self.assertFalse(hmac.compare_digest(a, b))
- # Testing str/bytes of same lengths, same values
- a, b = "foobar", "foobar"
- self.assertTrue(hmac.secure_compare(a, b))
+ # Testing bytes of same lengths, same values
a, b = b"foobar", b"foobar"
- self.assertTrue(hmac.secure_compare(a, b))
+ self.assertTrue(hmac.compare_digest(a, b))
a, b = b"\xde\xad\xbe\xef", b"\xde\xad\xbe\xef"
- self.assertTrue(hmac.secure_compare(a, b))
+ self.assertTrue(hmac.compare_digest(a, b))
def test_main():
support.run_unittest(
@@ -343,7 +345,7 @@
ConstructorTestCase,
SanityTestCase,
CopyTestCase,
- SecureCompareTestCase
+ CompareDigestTestCase
)
if __name__ == "__main__":
diff --git a/Misc/NEWS b/Misc/NEWS
--- a/Misc/NEWS
+++ b/Misc/NEWS
@@ -21,6 +21,11 @@
Library
-------
+- Issue #15061: The inappropriately named hmac.secure_compare has been
+ renamed to hash.compare_digest, restricted to operating on bytes inputs
+ only and had its documentation updated to more acurrately reflect both its
+ intent and its limitations
+
- Issue #13841: Make child processes exit using sys.exit() on Windows.
- Issue #14936: curses_panel was converted to PEP 3121 and PEP 384 API.
--
Repository URL: http://hg.python.org/cpython
More information about the Python-checkins
mailing list