From 1082bd595dd0da9a913171309748bc9ee1c1c724 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 22 Jul 2025 09:24:11 +0200 Subject: [PATCH 1/5] fix how large messages and keys are rejected by HMAC --- Lib/hmac.py | 12 +++- Lib/test/test_hmac.py | 68 ++++++++++++++++--- ...-07-21-11-56-47.gh-issue-136912.zWosAL.rst | 2 + 3 files changed, 72 insertions(+), 10 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst diff --git a/Lib/hmac.py b/Lib/hmac.py index e50d355fc60871..4416989513cdee 100644 --- a/Lib/hmac.py +++ b/Lib/hmac.py @@ -241,13 +241,23 @@ def digest(key, msg, digest): if _hashopenssl and isinstance(digest, (str, _functype)): try: return _hashopenssl.hmac_digest(key, msg, digest) + except OverflowError: + try: + return _hashopenssl.hmac_new(key, msg, digest).digest() + except _hashopenssl.UnsupportedDigestmodError: + pass except _hashopenssl.UnsupportedDigestmodError: pass if _hmac and isinstance(digest, str): try: return _hmac.compute_digest(key, msg, digest) - except (OverflowError, _hmac.UnknownHashError): + except OverflowError: + try: + return _hmac.new(key, msg, digest).digest() + except _hmac.UnknownHashError: + pass + except _hmac.UnknownHashError: pass return _compute_digest_fallback(key, msg, digest) diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py index 02ded86678343f..b63b5cb357a608 100644 --- a/Lib/test/test_hmac.py +++ b/Lib/test/test_hmac.py @@ -21,20 +21,21 @@ import hmac import hashlib import random -import test.support.hashlib_helper as hashlib_helper import types import unittest -import unittest.mock as mock import warnings from _operator import _compare_digest as operator_compare_digest +from test.support import _4G, bigmemtest from test.support import check_disallow_instantiation +from test.support import hashlib_helper, import_helper from test.support.hashlib_helper import ( BuiltinHashFunctionsTrait, HashFunctionsTrait, NamedHashFunctionsTrait, OpenSSLHashFunctionsTrait, ) -from test.support.import_helper import import_fresh_module, import_module +from test.support.import_helper import import_fresh_module +from unittest.mock import patch try: import _hashlib @@ -727,7 +728,7 @@ def setUpClass(cls): super().setUpClass() for meth in ['_init_openssl_hmac', '_init_builtin_hmac']: fn = getattr(cls.hmac.HMAC, meth) - cm = mock.patch.object(cls.hmac.HMAC, meth, autospec=True, wraps=fn) + cm = patch.object(cls.hmac.HMAC, meth, autospec=True, wraps=fn) cls.enterClassContext(cm) @classmethod @@ -949,7 +950,11 @@ class PyConstructorTestCase(ThroughObjectMixin, PyConstructorBaseMixin, class PyModuleConstructorTestCase(ThroughModuleAPIMixin, PyConstructorBaseMixin, unittest.TestCase): - """Test the hmac.new() and hmac.digest() functions.""" + """Test the hmac.new() and hmac.digest() functions. + + Note that "self.hmac" is imported by blocking "_hashlib" and "_hmac". + For testing functions in "hmac", extend PyMiscellaneousTests instead. + """ def test_hmac_digest_digestmod_parameter(self): func = self.hmac_digest @@ -1445,9 +1450,8 @@ def test_hmac_constructor_uses_builtin(self): hmac = import_fresh_module("hmac", blocked=["_hashlib"]) def watch_method(cls, name): - return mock.patch.object( - cls, name, autospec=True, wraps=getattr(cls, name) - ) + wraps = getattr(cls, name) + return patch.object(cls, name, autospec=True, wraps=wraps) with ( watch_method(hmac.HMAC, '_init_openssl_hmac') as f, @@ -1499,6 +1503,52 @@ def test_with_fallback(self): finally: cache.pop('foo') + @hashlib_helper.requires_openssl_hashdigest("md5") + @bigmemtest(size=_4G, memuse=2, dry_run=False) + def test_hmac_digest_overflow_error_openssl_only(self, size): + self.do_test_hmac_digest_overflow_error_fast(size, openssl=True) + + @hashlib_helper.requires_builtin_hashdigest("_md5", "md5") + @bigmemtest(size=_4G , memuse=2, dry_run=False) + def test_hmac_digest_overflow_error_builtin_only(self, size): + self.do_test_hmac_digest_overflow_error_fast(size, openssl=False) + + def do_test_hmac_digest_overflow_error_fast(self, size, *, openssl): + """Check that C hmac.digest() works for large inputs.""" + + if openssl: + hmac = import_fresh_module("hmac", blocked=["_hashlib"]) + c_module_name, c_method_name = "_hmac", "new" + else: + hmac = import_fresh_module("hmac", blocked=["_hmac"]) + c_module_name, c_method_name = "_hashlib", "hmac_new" + + cext = import_helper.import_module(c_module_name) + cnew = getattr(cext, c_method_name) + + bigkey = b'K' * size + bigmsg = b'M' * size + + with patch.object(hmac, "_compute_digest_fallback") as slow: + with patch.object(cext, c_method_name, wraps=cnew) as new: + self.assertIsInstance(hmac.digest(bigkey, b'm', "md5"), bytes) + new.assert_called_once() + with patch.object(cext, c_method_name, wraps=cnew) as new: + self.assertIsInstance(hmac.digest(b'k', bigmsg, "md5"), bytes) + new.assert_called_once() + slow.assert_not_called() + + @hashlib_helper.requires_hashdigest("md5", openssl=True) + @bigmemtest(size=_4G, memuse=2, dry_run=False) + def test_hmac_digest_no_overflow_error_in_fallback(self, size): + hmac = import_fresh_module("hmac", blocked=["_hashlib", "_hmac"]) + + for key, msg in [(b'K' * size, b'm'), (b'k', b'M' * size)]: + with self.subTest(keysize=len(key), msgsize=len(msg)): + with patch.object(hmac, "_compute_digest_fallback") as slow: + self.assertIsInstance(hmac.digest(key, msg, "md5"), bytes) + slow.assert_called_once() + class BuiltinMiscellaneousTests(BuiltinModuleMixin, unittest.TestCase): """HMAC-BLAKE2 is not standardized as BLAKE2 is a keyed hash function. @@ -1511,7 +1561,7 @@ class BuiltinMiscellaneousTests(BuiltinModuleMixin, unittest.TestCase): @classmethod def setUpClass(cls): super().setUpClass() - cls.blake2 = import_module("_blake2") + cls.blake2 = import_helper.import_module("_blake2") cls.blake2b = cls.blake2.blake2b cls.blake2s = cls.blake2.blake2s diff --git a/Misc/NEWS.d/next/Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst b/Misc/NEWS.d/next/Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst new file mode 100644 index 00000000000000..bb3d0d3338a2e5 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst @@ -0,0 +1,2 @@ +One-shot :func:`hmac.digest` now properly handles large keys and messages +by delegating to :func:`hmac.new` when possible. Patch by Bénédikt Tran. From 4d412bd10383f41841a6ba7d30c1a1f974bdfa42 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Tue, 22 Jul 2025 16:50:10 +0200 Subject: [PATCH 2/5] always fallback to the pure Python implementation --- Lib/hmac.py | 22 ++++++----- Lib/test/test_hmac.py | 39 +++++++------------ ...-07-21-11-56-47.gh-issue-136912.zWosAL.rst | 4 +- 3 files changed, 29 insertions(+), 36 deletions(-) diff --git a/Lib/hmac.py b/Lib/hmac.py index 4416989513cdee..9d3fae8b1b1597 100644 --- a/Lib/hmac.py +++ b/Lib/hmac.py @@ -242,22 +242,24 @@ def digest(key, msg, digest): try: return _hashopenssl.hmac_digest(key, msg, digest) except OverflowError: - try: - return _hashopenssl.hmac_new(key, msg, digest).digest() - except _hashopenssl.UnsupportedDigestmodError: - pass + # OpenSSL's HMAC limits the size of the key to INT_MAX. + # Instead of falling back to HACL* implementation which + # may still not be supported due to a too large key, we + # directly switch to the pure Python fallback instead + # even if we could have used streaming HMAC for small keys + # but large messages. + return _compute_digest_fallback(key, msg, digest) except _hashopenssl.UnsupportedDigestmodError: pass if _hmac and isinstance(digest, str): try: return _hmac.compute_digest(key, msg, digest) - except OverflowError: - try: - return _hmac.new(key, msg, digest).digest() - except _hmac.UnknownHashError: - pass - except _hmac.UnknownHashError: + except (OverflowError, _hmac.UnknownHashError): + # HACL* HMAC limits the size of the key to UINT32_MAX + # so we fallback to the pure Python implementation even + # if streaming HMAC may have been used for small keys + # and large messages. pass return _compute_digest_fallback(key, msg, digest) diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py index b63b5cb357a608..79d3f6faf70415 100644 --- a/Lib/test/test_hmac.py +++ b/Lib/test/test_hmac.py @@ -1504,42 +1504,33 @@ def test_with_fallback(self): cache.pop('foo') @hashlib_helper.requires_openssl_hashdigest("md5") - @bigmemtest(size=_4G, memuse=2, dry_run=False) + @bigmemtest(size=_4G + 5, memuse=2, dry_run=False) def test_hmac_digest_overflow_error_openssl_only(self, size): - self.do_test_hmac_digest_overflow_error_fast(size, openssl=True) + hmac = import_fresh_module("hmac", blocked=["_hmac"]) + self.do_test_hmac_digest_overflow_error_switch_to_slow(hmac, size) @hashlib_helper.requires_builtin_hashdigest("_md5", "md5") - @bigmemtest(size=_4G , memuse=2, dry_run=False) + @bigmemtest(size=_4G + 5, memuse=2, dry_run=False) def test_hmac_digest_overflow_error_builtin_only(self, size): - self.do_test_hmac_digest_overflow_error_fast(size, openssl=False) - - def do_test_hmac_digest_overflow_error_fast(self, size, *, openssl): - """Check that C hmac.digest() works for large inputs.""" - - if openssl: - hmac = import_fresh_module("hmac", blocked=["_hashlib"]) - c_module_name, c_method_name = "_hmac", "new" - else: - hmac = import_fresh_module("hmac", blocked=["_hmac"]) - c_module_name, c_method_name = "_hashlib", "hmac_new" + hmac = import_fresh_module("hmac", blocked=["_hashlib"]) + self.do_test_hmac_digest_overflow_error_switch_to_slow(hmac, size) - cext = import_helper.import_module(c_module_name) - cnew = getattr(cext, c_method_name) + def do_test_hmac_digest_overflow_error_switch_to_slow(self, hmac, size): + """Check that hmac.digest() falls back to pure Python.""" bigkey = b'K' * size bigmsg = b'M' * size with patch.object(hmac, "_compute_digest_fallback") as slow: - with patch.object(cext, c_method_name, wraps=cnew) as new: - self.assertIsInstance(hmac.digest(bigkey, b'm', "md5"), bytes) - new.assert_called_once() - with patch.object(cext, c_method_name, wraps=cnew) as new: - self.assertIsInstance(hmac.digest(b'k', bigmsg, "md5"), bytes) - new.assert_called_once() - slow.assert_not_called() + self.assertIsInstance(hmac.digest(bigkey, b'm', "md5"), bytes) + slow.assert_called_once() + + with patch.object(hmac, "_compute_digest_fallback") as slow: + self.assertIsInstance(hmac.digest(b'k', bigmsg, "md5"), bytes) + slow.assert_called_once() @hashlib_helper.requires_hashdigest("md5", openssl=True) - @bigmemtest(size=_4G, memuse=2, dry_run=False) + @bigmemtest(size=_4G + 5, memuse=2, dry_run=False) def test_hmac_digest_no_overflow_error_in_fallback(self, size): hmac = import_fresh_module("hmac", blocked=["_hashlib", "_hmac"]) diff --git a/Misc/NEWS.d/next/Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst b/Misc/NEWS.d/next/Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst index bb3d0d3338a2e5..c8855cfa1a6585 100644 --- a/Misc/NEWS.d/next/Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst +++ b/Misc/NEWS.d/next/Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst @@ -1,2 +1,2 @@ -One-shot :func:`hmac.digest` now properly handles large keys and messages -by delegating to :func:`hmac.new` when possible. Patch by Bénédikt Tran. +One-shot :func:`hmac.digest` now properly handles large keys and messages. +Patch by Bénédikt Tran. From ac6b9836ba2ecdf81b8cc32f01a7bd9f263694ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Fri, 25 Jul 2025 14:44:14 +0200 Subject: [PATCH 3/5] fix tests --- Lib/test/test_hmac.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py index 79d3f6faf70415..f5b86215fbab97 100644 --- a/Lib/test/test_hmac.py +++ b/Lib/test/test_hmac.py @@ -1522,11 +1522,11 @@ def do_test_hmac_digest_overflow_error_switch_to_slow(self, hmac, size): bigmsg = b'M' * size with patch.object(hmac, "_compute_digest_fallback") as slow: - self.assertIsInstance(hmac.digest(bigkey, b'm', "md5"), bytes) + hmac.digest(bigkey, b'm', "md5") slow.assert_called_once() with patch.object(hmac, "_compute_digest_fallback") as slow: - self.assertIsInstance(hmac.digest(b'k', bigmsg, "md5"), bytes) + hmac.digest(b'k', bigmsg, "md5") slow.assert_called_once() @hashlib_helper.requires_hashdigest("md5", openssl=True) @@ -1537,7 +1537,7 @@ def test_hmac_digest_no_overflow_error_in_fallback(self, size): for key, msg in [(b'K' * size, b'm'), (b'k', b'M' * size)]: with self.subTest(keysize=len(key), msgsize=len(msg)): with patch.object(hmac, "_compute_digest_fallback") as slow: - self.assertIsInstance(hmac.digest(key, msg, "md5"), bytes) + hmac.digest(key, msg, "md5") slow.assert_called_once() From e1b14f432252ca4477e2bf8b9c098f1de296e197 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 26 Jul 2025 09:48:45 +0200 Subject: [PATCH 4/5] update comment --- Lib/test/test_hmac.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py index f5b86215fbab97..5c29369d10b143 100644 --- a/Lib/test/test_hmac.py +++ b/Lib/test/test_hmac.py @@ -1516,18 +1516,23 @@ def test_hmac_digest_overflow_error_builtin_only(self, size): self.do_test_hmac_digest_overflow_error_switch_to_slow(hmac, size) def do_test_hmac_digest_overflow_error_switch_to_slow(self, hmac, size): - """Check that hmac.digest() falls back to pure Python.""" + """Check that hmac.digest() falls back to pure Python. + + The *hmac* argument implements the HMAC module interface. + The *size* argument is a large key size or message size that would + trigger an OverflowError in the C implementation(s) of hmac.digest(). + """ bigkey = b'K' * size bigmsg = b'M' * size with patch.object(hmac, "_compute_digest_fallback") as slow: hmac.digest(bigkey, b'm', "md5") - slow.assert_called_once() + slow.assert_called_once() with patch.object(hmac, "_compute_digest_fallback") as slow: hmac.digest(b'k', bigmsg, "md5") - slow.assert_called_once() + slow.assert_called_once() @hashlib_helper.requires_hashdigest("md5", openssl=True) @bigmemtest(size=_4G + 5, memuse=2, dry_run=False) From 46b2d9763a98c1e362fe18fa00e90a2657785513 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sat, 26 Jul 2025 09:55:53 +0200 Subject: [PATCH 5/5] update NEWS --- .../Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst b/Misc/NEWS.d/next/Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst index c8855cfa1a6585..6c5f31145f76d1 100644 --- a/Misc/NEWS.d/next/Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst +++ b/Misc/NEWS.d/next/Library/2025-07-21-11-56-47.gh-issue-136912.zWosAL.rst @@ -1,2 +1,3 @@ -One-shot :func:`hmac.digest` now properly handles large keys and messages. +:func:`hmac.digest` now properly handles large keys and messages +by falling back to the pure Python implementation when necessary. Patch by Bénédikt Tran.