Skip to content

gh-136787: raise consistent ValueError for bad hashes #136802

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

Merged
merged 6 commits into from
Jul 20, 2025
Merged
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: 11 additions & 1 deletion Lib/hashlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,11 @@
}

def __get_builtin_constructor(name):
if not isinstance(name, str):
# Since this function is only used by new(), we use the same
# exception as _hashlib.new() when 'name' is of incorrect type.
err = f"new() argument 'name' must be str, not {type(name).__name__}"
raise TypeError(err)
cache = __builtin_constructor_cache
constructor = cache.get(name)
if constructor is not None:
Expand Down Expand Up @@ -120,10 +125,13 @@ def __get_builtin_constructor(name):
if constructor is not None:
return constructor

raise ValueError('unsupported hash type ' + name)
# Keep the message in sync with hashlib.h::HASHLIB_UNSUPPORTED_ALGORITHM.
raise ValueError(f'unsupported hash algorithm {name}')


def __get_openssl_constructor(name):
# This function is only used until the module has been initialized.
assert isinstance(name, str), "invalid call to __get_openssl_constructor()"
if name in __block_openssl_constructor:
# Prefer our builtin blake2 implementation.
return __get_builtin_constructor(name)
Expand Down Expand Up @@ -154,6 +162,8 @@ def __hash_new(name, *args, **kwargs):
optionally initialized with data (which must be a bytes-like object).
"""
if name in __block_openssl_constructor:
# __block_openssl_constructor is expected to contain strings only
assert isinstance(name, str), f"unexpected name: {name}"
# Prefer our builtin blake2 implementation.
return __get_builtin_constructor(name)(*args, **kwargs)
try:
Expand Down
14 changes: 14 additions & 0 deletions Lib/hmac.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,16 @@
digest_size = None


def _is_shake_constructor(digest_like):
if isinstance(digest_like, str):
name = digest_like
else:
h = digest_like() if callable(digest_like) else digest_like.new()
if not isinstance(name := getattr(h, "name", None), str):
return False
return name.startswith(("shake", "SHAKE"))


def _get_digest_constructor(digest_like):
if callable(digest_like):
return digest_like
Expand Down Expand Up @@ -109,6 +119,8 @@ def _init_old(self, key, msg, digestmod):
import warnings

digest_cons = _get_digest_constructor(digestmod)
if _is_shake_constructor(digest_cons):
raise ValueError(f"unsupported hash algorithm {digestmod}")

self._hmac = None
self._outer = digest_cons()
Expand Down Expand Up @@ -243,6 +255,8 @@ def digest(key, msg, digest):

def _compute_digest_fallback(key, msg, digest):
digest_cons = _get_digest_constructor(digest)
if _is_shake_constructor(digest_cons):
raise ValueError(f"unsupported hash algorithm {digest}")
inner = digest_cons()
outer = digest_cons()
blocksize = getattr(inner, 'block_size', 64)
Expand Down
8 changes: 6 additions & 2 deletions Lib/test/test_hashlib.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,7 +343,9 @@ def test_clinic_signature_errors(self):

def test_unknown_hash(self):
self.assertRaises(ValueError, hashlib.new, 'spam spam spam spam spam')
self.assertRaises(TypeError, hashlib.new, 1)
# ensure that the exception message remains consistent
err = re.escape("new() argument 'name' must be str, not int")
self.assertRaisesRegex(TypeError, err, hashlib.new, 1)

def test_new_upper_to_lower(self):
self.assertEqual(hashlib.new("SHA256").name, "sha256")
Expand All @@ -370,7 +372,9 @@ def test_get_builtin_constructor(self):
sys.modules['_md5'] = _md5
else:
del sys.modules['_md5']
self.assertRaises(TypeError, get_builtin_constructor, 3)
# ensure that the exception message remains consistent
err = re.escape("new() argument 'name' must be str, not int")
self.assertRaises(TypeError, err, get_builtin_constructor, 3)
constructor = get_builtin_constructor('md5')
self.assertIs(constructor, _md5.md5)
self.assertEqual(sorted(builtin_constructor_cache), ['MD5', 'md5'])
Expand Down
2 changes: 1 addition & 1 deletion Lib/test/test_hmac.py
Original file line number Diff line number Diff line change
Expand Up @@ -960,7 +960,7 @@ def raiser():
with self.assertRaisesRegex(RuntimeError, "custom exception"):
func(b'key', b'msg', raiser)

with self.assertRaisesRegex(ValueError, 'hash type'):
with self.assertRaisesRegex(ValueError, 'unsupported hash algorithm'):
func(b'key', b'msg', 'unknown')

with self.assertRaisesRegex(AttributeError, 'new'):
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
:mod:`hashlib`: improve exception messages when a hash algorithm is not
recognized, blocked by the current security policy or incompatible with
the desired operation (for instance, using HMAC with SHAKE).
Patch by Bénédikt Tran.
Loading
Loading