From db6f4db364cf381a43fb7b782e3b2e9a8b77d6e7 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, 19 Jul 2025 14:05:22 +0200 Subject: [PATCH 1/6] raise consistent ValueError for bad hashes --- Lib/hashlib.py | 12 ++- Lib/hmac.py | 14 +++ Lib/test/test_hashlib.py | 8 +- Lib/test/test_hmac.py | 2 +- Modules/_hashopenssl.c | 192 ++++++++++++++++++++++++++++++--------- Modules/hashlib.h | 9 ++ Modules/hmacmodule.c | 2 +- 7 files changed, 190 insertions(+), 49 deletions(-) diff --git a/Lib/hashlib.py b/Lib/hashlib.py index 6c72fba03bf687..02470ba0fdd559 100644 --- a/Lib/hashlib.py +++ b/Lib/hashlib.py @@ -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: @@ -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) @@ -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: diff --git a/Lib/hmac.py b/Lib/hmac.py index 3683a4aa653a0a..e50d355fc60871 100644 --- a/Lib/hmac.py +++ b/Lib/hmac.py @@ -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 @@ -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() @@ -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) diff --git a/Lib/test/test_hashlib.py b/Lib/test/test_hashlib.py index 65e18639f82be5..7123641650263b 100644 --- a/Lib/test/test_hashlib.py +++ b/Lib/test/test_hashlib.py @@ -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") @@ -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']) diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py index ff6e1bce0ef801..02ded86678343f 100644 --- a/Lib/test/test_hmac.py +++ b/Lib/test/test_hmac.py @@ -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'): diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 6f04d9ec0c479f..acb3d25e67486e 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -74,6 +74,12 @@ #define PY_EVP_MD_CTX_md(CTX) EVP_MD_CTX_md(CTX) #endif +static inline int +PY_EVP_MD_xof(PY_EVP_MD *md) +{ + return md != NULL && ((EVP_MD_flags(md) & EVP_MD_FLAG_XOF) != 0); +} + /* hash alias map and fast lookup * * Map between Python's preferred names and OpenSSL internal names. Maintain @@ -319,6 +325,35 @@ py_wrapper_ERR_reason_error_string(unsigned long errcode) return reason ? reason : "no reason"; } +#ifdef Py_HAS_OPENSSL3_SUPPORT +/* + * Set an exception with additional information. + * + * This is only useful in OpenSSL 3 and later as the default reason + * usually lack information and function locations are no more encoded + * in the error code. + */ +static void +set_exception_with_ssl_errinfo(PyObject *exc_type, PyObject *exc_text, + const char *lib, const char *reason) +{ + assert(exc_type != NULL); + assert(exc_text != NULL); + if (lib && reason) { + PyErr_Format(exc_type, "[%s] %U (reason: %s)", lib, exc_text, reason); + } + else if (lib) { + PyErr_Format(exc_type, "[%s] %U", lib, exc_text); + } + else if (reason) { + PyErr_Format(exc_type, "%U (reason: %s)", exc_text, reason); + } + else { + PyErr_SetObject(exc_type, exc_text); + } +} +#endif + /* Set an exception of given type using the given OpenSSL error code. */ static void set_ssl_exception_from_errcode(PyObject *exc_type, unsigned long errcode) @@ -445,6 +480,68 @@ notify_smart_ssl_error_occurred_in(const char *funcname) raise_smart_ssl_error_f(PyExc_ValueError, "error in OpenSSL function %s()", funcname); } + +#ifdef Py_HAS_OPENSSL3_SUPPORT +static void +raise_unsupported_algorithm_impl(PyObject *exc_type, + const char *fallback_format, + const void *format_arg) +{ + // Since OpenSSL 3.0, if the algorithm is not supported or fetching fails, + // the reason lacks the algorithm name. + int errcode = ERR_peek_last_error(), reason_id; + switch (reason_id = ERR_GET_REASON(errcode)) { + case ERR_R_UNSUPPORTED: { + PyObject *text = PyUnicode_FromFormat(fallback_format, format_arg); + if (text != NULL) { + const char *lib = ERR_lib_error_string(errcode); + set_exception_with_ssl_errinfo(exc_type, text, lib, NULL); + Py_DECREF(text); + } + break; + } + case ERR_R_FETCH_FAILED: { + PyObject *text = PyUnicode_FromFormat(fallback_format, format_arg); + if (text != NULL) { + const char *lib = ERR_lib_error_string(errcode); + const char *reason = ERR_reason_error_string(errcode); + set_exception_with_ssl_errinfo(exc_type, text, lib, reason); + Py_DECREF(text); + } + break; + } + default: + raise_ssl_error_f(exc_type, fallback_format, format_arg); + break; + } + assert(PyErr_Occurred()); +} +#else +/* Before OpenSSL 3.0, error messages included enough information. */ +#define raise_unsupported_algorithm_impl raise_ssl_error_f +#endif + +static inline void +raise_unsupported_algorithm_error(_hashlibstate *state, PyObject *digestmod) +{ + raise_unsupported_algorithm_impl( + state->unsupported_digestmod_error, + HASHLIB_UNSUPPORTED_ALGORITHM, + digestmod + ); +} + +static inline void +raise_unsupported_str_algorithm_error(_hashlibstate *state, const char *name) +{ + raise_unsupported_algorithm_impl( + state->unsupported_digestmod_error, + HASHLIB_UNSUPPORTED_STR_ALGORITHM, + name + ); +} + +#undef raise_unsupported_algorithm_impl /* LCOV_EXCL_STOP */ /* @@ -522,6 +619,27 @@ get_hashlib_utf8name_by_evp_md(const EVP_MD *md) return get_hashlib_utf8name_by_nid(EVP_MD_nid(md)); } +/* + * Return 1 if the FIPS query property should be disabled. + * + * Note that returning 0 does not necesasrily mean that the + * fetched algorithm will be available. For instance, a FIPS + */ +static inline int +disable_fips_property(Py_hash_type py_ht) +{ + switch (py_ht) { + case Py_ht_evp: + case Py_ht_mac: + case Py_ht_pbkdf2: + return 0; + case Py_ht_evp_nosecurity: + return 1; + default: + Py_FatalError("unsupported hash type"); + } +} + /* * Get a new reference to an EVP_MD object described by name and purpose. * @@ -538,10 +656,7 @@ get_openssl_evp_md_by_utf8name(PyObject *module, const char *name, ); if (entry != NULL) { - switch (py_ht) { - case Py_ht_evp: - case Py_ht_mac: - case Py_ht_pbkdf2: + if (!disable_fips_property(py_ht)) { digest = FT_ATOMIC_LOAD_PTR_RELAXED(entry->evp); if (digest == NULL) { digest = PY_EVP_MD_fetch(entry->ossl_name, NULL); @@ -552,8 +667,8 @@ get_openssl_evp_md_by_utf8name(PyObject *module, const char *name, entry->evp = digest; #endif } - break; - case Py_ht_evp_nosecurity: + } + else { digest = FT_ATOMIC_LOAD_PTR_RELAXED(entry->evp_nosecurity); if (digest == NULL) { digest = PY_EVP_MD_fetch(entry->ossl_name, "-fips"); @@ -564,9 +679,6 @@ get_openssl_evp_md_by_utf8name(PyObject *module, const char *name, entry->evp_nosecurity = digest; #endif } - break; - default: - goto invalid_hash_type; } // if another thread same thing at same time make sure we got same ptr assert(other_digest == NULL || other_digest == digest); @@ -576,41 +688,15 @@ get_openssl_evp_md_by_utf8name(PyObject *module, const char *name, } else { // Fall back for looking up an unindexed OpenSSL specific name. - switch (py_ht) { - case Py_ht_evp: - case Py_ht_mac: - case Py_ht_pbkdf2: - digest = PY_EVP_MD_fetch(name, NULL); - break; - case Py_ht_evp_nosecurity: - digest = PY_EVP_MD_fetch(name, "-fips"); - break; - default: - goto invalid_hash_type; - } + const char *props = disable_fips_property(py_ht) ? "-fips" : NULL; + (void)props; // will only be used in OpenSSL 3.0 and later + digest = PY_EVP_MD_fetch(name, props); } if (digest == NULL) { - raise_ssl_error_f(state->unsupported_digestmod_error, - "unsupported digest name: %s", name); + raise_unsupported_str_algorithm_error(state, name); return NULL; } return digest; - -invalid_hash_type: - assert(digest == NULL); - PyErr_Format(PyExc_SystemError, "unsupported hash type %d", py_ht); - return NULL; -} - -/* - * Raise an exception indicating that 'digestmod' is not supported. - */ -static void -raise_unsupported_digestmod_error(PyObject *module, PyObject *digestmod) -{ - _hashlibstate *state = get_hashlib_state(module); - PyErr_Format(state->unsupported_digestmod_error, - "Unsupported digestmod %R", digestmod); } /* @@ -638,7 +724,8 @@ get_openssl_evp_md(PyObject *module, PyObject *digestmod, Py_hash_type py_ht) } if (name == NULL) { if (!PyErr_Occurred()) { - raise_unsupported_digestmod_error(module, digestmod); + _hashlibstate *state = get_hashlib_state(module); + raise_unsupported_algorithm_error(state, digestmod); } return NULL; } @@ -1682,7 +1769,7 @@ _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key, /*[clinic end generated code: output=82f19965d12706ac input=0a0790cc3db45c2e]*/ { unsigned char md[EVP_MAX_MD_SIZE] = {0}; - unsigned int md_len = 0; + unsigned int md_len = 0, is_xof; unsigned char *result; PY_EVP_MD *evp; @@ -1703,6 +1790,7 @@ _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key, } Py_BEGIN_ALLOW_THREADS + is_xof = PY_EVP_MD_xof(evp); result = HMAC( evp, (const void *)key->buf, (int)key->len, @@ -1710,10 +1798,18 @@ _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key, md, &md_len ); Py_END_ALLOW_THREADS + PY_EVP_MD_free(evp); if (result == NULL) { - notify_ssl_error_occurred_in(Py_STRINGIFY(HMAC)); + if (is_xof) { + _hashlibstate *state = get_hashlib_state(module); + /* use a better default error message if an XOF is used */ + raise_unsupported_algorithm_error(state, digest); + } + else { + notify_ssl_error_occurred_in(Py_STRINGIFY(HMAC)); + } return NULL; } return PyBytes_FromStringAndSize((const char*)md, md_len); @@ -1764,7 +1860,7 @@ _hashlib_hmac_new_impl(PyObject *module, Py_buffer *key, PyObject *msg_obj, PY_EVP_MD *digest; HMAC_CTX *ctx = NULL; HMACobject *self = NULL; - int r; + int is_xof, r; if (key->len > INT_MAX) { PyErr_SetString(PyExc_OverflowError, @@ -1789,10 +1885,18 @@ _hashlib_hmac_new_impl(PyObject *module, Py_buffer *key, PyObject *msg_obj, goto error; } + is_xof = PY_EVP_MD_xof(digest); r = HMAC_Init_ex(ctx, key->buf, (int)key->len, digest, NULL /* impl */); PY_EVP_MD_free(digest); if (r == 0) { - notify_ssl_error_occurred_in(Py_STRINGIFY(HMAC_Init_ex)); + if (is_xof) { + _hashlibstate *state = get_hashlib_state(module); + /* use a better default error message if an XOF is used */ + raise_unsupported_algorithm_error(state, digestmod); + } + else { + notify_ssl_error_occurred_in(Py_STRINGIFY(HMAC_Init_ex)); + } goto error; } diff --git a/Modules/hashlib.h b/Modules/hashlib.h index 9a7e72f34a7f9d..5de5922c345047 100644 --- a/Modules/hashlib.h +++ b/Modules/hashlib.h @@ -2,6 +2,15 @@ #include "pycore_lock.h" // PyMutex +/* + * Internal error messages used for reporting an unsupported hash algorithm. + * The algorithm can be given by its name, a callable or a PEP-247 module. + * The same message is raised by Lib/hashlib.py::__get_builtin_constructor() + * and _hmacmodule.c::find_hash_info(). + */ +#define HASHLIB_UNSUPPORTED_ALGORITHM "unsupported hash algorithm %S" +#define HASHLIB_UNSUPPORTED_STR_ALGORITHM "unsupported hash algorithm %s" + /* * Given a PyObject* obj, fill in the Py_buffer* viewp with the result * of PyObject_GetBuffer. Sets an exception and issues the erraction diff --git a/Modules/hmacmodule.c b/Modules/hmacmodule.c index 95e400231bb65c..b5405c99f1f8ce 100644 --- a/Modules/hmacmodule.c +++ b/Modules/hmacmodule.c @@ -656,7 +656,7 @@ find_hash_info(hmacmodule_state *state, PyObject *hash_info_ref) } if (rc == 0) { PyErr_Format(state->unknown_hash_error, - "unsupported hash type: %R", hash_info_ref); + HASHLIB_UNSUPPORTED_ALGORITHM, hash_info_ref); return NULL; } assert(info != NULL); From 19c74cb4df17516b3b40f8034352d0d362ac6c82 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, 19 Jul 2025 23:42:14 +0200 Subject: [PATCH 2/6] Apply suggestions from code review --- Modules/_hashopenssl.c | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index acb3d25e67486e..e0caa41a271dda 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -329,8 +329,8 @@ py_wrapper_ERR_reason_error_string(unsigned long errcode) /* * Set an exception with additional information. * - * This is only useful in OpenSSL 3 and later as the default reason - * usually lack information and function locations are no more encoded + * This is only useful in OpenSSL 3.0 and later as the default reason + * usually lack information and function locations are no longer encoded * in the error code. */ static void @@ -620,10 +620,9 @@ get_hashlib_utf8name_by_evp_md(const EVP_MD *md) } /* - * Return 1 if the FIPS query property should be disabled. + * Return 1 if the property query clause [1] must be "-fips" and 0 otherwise. * - * Note that returning 0 does not necesasrily mean that the - * fetched algorithm will be available. For instance, a FIPS + * [1] https://docs.openssl.org/master/man7/property */ static inline int disable_fips_property(Py_hash_type py_ht) @@ -1798,7 +1797,6 @@ _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key, md, &md_len ); Py_END_ALLOW_THREADS - PY_EVP_MD_free(evp); if (result == NULL) { From fd23f90475385eff898d740951d85fb8e07abc09 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, 19 Jul 2025 23:43:25 +0200 Subject: [PATCH 3/6] Update Modules/_hashopenssl.c --- Modules/_hashopenssl.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index e0caa41a271dda..5d17b9a7aec222 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -330,7 +330,7 @@ py_wrapper_ERR_reason_error_string(unsigned long errcode) * Set an exception with additional information. * * This is only useful in OpenSSL 3.0 and later as the default reason - * usually lack information and function locations are no longer encoded + * usually lacks information and function locations are no longer encoded * in the error code. */ static void From 6e8e8f28059f341a3ac0310634e7ba489c06e4fc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 20 Jul 2025 10:13:57 +0200 Subject: [PATCH 4/6] cosmetics --- Modules/_hashopenssl.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/Modules/_hashopenssl.c b/Modules/_hashopenssl.c index 5d17b9a7aec222..7086a3f6530841 100644 --- a/Modules/_hashopenssl.c +++ b/Modules/_hashopenssl.c @@ -74,6 +74,12 @@ #define PY_EVP_MD_CTX_md(CTX) EVP_MD_CTX_md(CTX) #endif +/* + * Return 1 if *md* is an extendable-output Function (XOF) and 0 otherwise. + * SHAKE128 and SHAKE256 are XOF functions but not BLAKE2B algorithms. + * + * This is a backport of the EVP_MD_xof() helper added in OpenSSL 3.4. + */ static inline int PY_EVP_MD_xof(PY_EVP_MD *md) { @@ -1768,9 +1774,10 @@ _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key, /*[clinic end generated code: output=82f19965d12706ac input=0a0790cc3db45c2e]*/ { unsigned char md[EVP_MAX_MD_SIZE] = {0}; - unsigned int md_len = 0, is_xof; + unsigned int md_len = 0; unsigned char *result; PY_EVP_MD *evp; + int is_xof; if (key->len > INT_MAX) { PyErr_SetString(PyExc_OverflowError, From cdec422d05f798709726c3f12a0473f78fb70101 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 20 Jul 2025 10:21:53 +0200 Subject: [PATCH 5/6] blurb --- .../Library/2025-07-20-10-21-49.gh-issue-136787._0Rbp_.rst | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 Misc/NEWS.d/next/Library/2025-07-20-10-21-49.gh-issue-136787._0Rbp_.rst diff --git a/Misc/NEWS.d/next/Library/2025-07-20-10-21-49.gh-issue-136787._0Rbp_.rst b/Misc/NEWS.d/next/Library/2025-07-20-10-21-49.gh-issue-136787._0Rbp_.rst new file mode 100644 index 00000000000000..815f9d0461ce8b --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-07-20-10-21-49.gh-issue-136787._0Rbp_.rst @@ -0,0 +1,4 @@ +:mod:`hashlib`: improve exception messages when a hash algorithm is not +recognized, blocked by the current security policy or unsupported for the +desired operation (for instance, using HMAC with SHAKE). Patch by Bénédikt +Tran. From cb4e222b40d7e3b94c9fe58ef2ce88405fc71d3a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?B=C3=A9n=C3=A9dikt=20Tran?= <10796600+picnixz@users.noreply.github.com> Date: Sun, 20 Jul 2025 10:24:36 +0200 Subject: [PATCH 6/6] wording --- .../Library/2025-07-20-10-21-49.gh-issue-136787._0Rbp_.rst | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/Misc/NEWS.d/next/Library/2025-07-20-10-21-49.gh-issue-136787._0Rbp_.rst b/Misc/NEWS.d/next/Library/2025-07-20-10-21-49.gh-issue-136787._0Rbp_.rst index 815f9d0461ce8b..c6a8088e5da81f 100644 --- a/Misc/NEWS.d/next/Library/2025-07-20-10-21-49.gh-issue-136787._0Rbp_.rst +++ b/Misc/NEWS.d/next/Library/2025-07-20-10-21-49.gh-issue-136787._0Rbp_.rst @@ -1,4 +1,4 @@ :mod:`hashlib`: improve exception messages when a hash algorithm is not -recognized, blocked by the current security policy or unsupported for the -desired operation (for instance, using HMAC with SHAKE). Patch by Bénédikt -Tran. +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.