gh-136787: raise consistent ValueError for bad hashes

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:

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)

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'])

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'):

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.0 and later as the default reason
+ * usually lack information and function locations are no longer 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,26 @@ get_hashlib_utf8name_by_evp_md(const EVP_MD *md)
     return get_hashlib_utf8name_by_nid(EVP_MD_nid(md));
 }
 
+/*
+ * Return 1 if the property query clause [1] must be "-fips" and 0 otherwise.
+ *
+ * [1] https://docs.openssl.org/master/man7/property
+ */
+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 +655,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 +666,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 +678,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 +687,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 +723,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 +1768,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 +1789,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,
@@ -1713,7 +1800,14 @@ _hashlib_hmac_singleshot_impl(PyObject *module, Py_buffer *key,
     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 +1858,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 +1883,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;
     }