Skip to content

gh-136912: fix handling of OverflowError in hmac.digest #136917

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 12 additions & 0 deletions Lib/hmac.py
Original file line number Diff line number Diff line change
Expand Up @@ -241,13 +241,25 @@ def digest(key, msg, digest):
if _hashopenssl and isinstance(digest, (str, _functype)):
try:
return _hashopenssl.hmac_digest(key, msg, digest)
except OverflowError:
# 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, _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)
Expand Down
59 changes: 50 additions & 9 deletions Lib/test/test_hmac.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1499,6 +1503,43 @@ def test_with_fallback(self):
finally:
cache.pop('foo')

@hashlib_helper.requires_openssl_hashdigest("md5")
@bigmemtest(size=_4G + 5, memuse=2, dry_run=False)
def test_hmac_digest_overflow_error_openssl_only(self, size):
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 + 5, memuse=2, dry_run=False)
def test_hmac_digest_overflow_error_builtin_only(self, size):
hmac = import_fresh_module("hmac", blocked=["_hashlib"])
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."""

bigkey = b'K' * size
bigmsg = b'M' * size

with patch.object(hmac, "_compute_digest_fallback") as slow:
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 + 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"])

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.
Expand All @@ -1511,7 +1552,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

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
One-shot :func:`hmac.digest` now properly handles large keys and messages.
Patch by Bénédikt Tran.
Loading