gh-136547: fix hashlib_helper for blocking and requesting digests (#136762)

- Fix `hashlib_helper.block_algorithm` where the dummy functions were incorrectly defined.
- Rename `hashlib_helper.HashAPI` to `hashlib_helper.HashInfo` and add more helper methods.
- Simplify `hashlib_helper.requires_*()` functions.
- Rewrite some private helpers in `hashlib_helper`.
- Remove `find_{builtin,openssl}_hashdigest_constructor()` as they are no more needed and were
  not meant to be public in the first place.
- Fix some tests in `test_hashlib` when FIPS mode is on.

Author: picnixz

Lib/hashlib.py

@@ -128,12 +128,22 @@ def __get_openssl_constructor(name):
         # Prefer our builtin blake2 implementation.
         return __get_builtin_constructor(name)
     try:
-        # MD5, SHA1, and SHA2 are in all supported OpenSSL versions
-        # SHA3/shake are available in OpenSSL 1.1.1+
+        # Fetch the OpenSSL hash function if it exists,
+        # independently of the context security policy.
         f = getattr(_hashlib, 'openssl_' + name)
-        # Allow the C module to raise ValueError.  The function will be
-        # defined but the hash not actually available.  Don't fall back to
-        # builtin if the current security policy blocks a digest, bpo#40695.
+        # Check if the context security policy blocks the digest or not
+        # by allowing the C module to raise a ValueError. The function
+        # will be defined but the hash will not be available at runtime.
+        #
+        # We use "usedforsecurity=False" to prevent falling back to the
+        # built-in function in case the security policy does not allow it.
+        #
+        # Note that this only affects the explicit named constructors,
+        # and not the algorithms exposed through hashlib.new() which
+        # can still be resolved to a built-in function even if the
+        # current security policy does not allow it.
+        #
+        # See https://github.com/python/cpython/issues/84872.
         f(usedforsecurity=False)
         # Use the C function directly (very fast)
         return f

Lib/test/test_hashlib.py

@@ -466,13 +466,17 @@ def check(self, name, data, hexdigest, shake=False, **kwargs):
 
     def check_file_digest(self, name, data, hexdigest):
         hexdigest = hexdigest.lower()
-        try:
-            hashlib.new(name)
-        except ValueError:
-            # skip, algorithm is blocked by security policy.
-            return
-        digests = [name]
-        digests.extend(self.constructors_to_test[name])
+        digests = []
+        for digest in [name, *self.constructors_to_test[name]]:
+            try:
+                if callable(digest):
+                    digest(b"")
+                else:
+                    hashlib.new(digest)
+            except ValueError:
+                # skip, algorithm is blocked by security policy.
+                continue
+            digests.append(digest)
 
         with open(os_helper.TESTFN, "wb") as f:
             f.write(data)

Lib/test/test_support.py

@@ -2,6 +2,7 @@
 import errno
 import importlib
 import itertools
+import inspect
 import io
 import logging
 import os
@@ -798,6 +799,7 @@ def test_linked_to_musl(self):
     # SuppressCrashReport
 
 
+@hashlib_helper.requires_builtin_hashes()
 class TestHashlibSupport(unittest.TestCase):
 
     @classmethod
@@ -806,11 +808,20 @@ def setUpClass(cls):
         cls.hashlib = import_helper.import_module("hashlib")
         cls.hmac = import_helper.import_module("hmac")
 
-        # We required the extension modules to be present since blocking
-        # HACL* implementations while allowing OpenSSL ones would still
-        # result in failures.
+        # All C extension modules must be present since blocking
+        # the built-in implementation while allowing OpenSSL or vice-versa
+        # may result in failures depending on the exposed built-in hashes.
         cls._hashlib = import_helper.import_module("_hashlib")
         cls._hmac = import_helper.import_module("_hmac")
+        cls._md5 = import_helper.import_module("_md5")
+
+    def skip_if_fips_mode(self):
+        if self._hashlib.get_fips_mode():
+            self.skipTest("disabled in FIPS mode")
+
+    def skip_if_not_fips_mode(self):
+        if not self._hashlib.get_fips_mode():
+            self.skipTest("requires FIPS mode")
 
     def check_context(self, disabled=True):
         if disabled:
@@ -831,25 +842,19 @@ def try_import_attribute(self, fullname, default=None):
         except TypeError:
             return default
 
-    def validate_modules(self):
-        if hasattr(hashlib_helper, 'hashlib'):
-            self.assertIs(hashlib_helper.hashlib, self.hashlib)
-        if hasattr(hashlib_helper, 'hmac'):
-            self.assertIs(hashlib_helper.hmac, self.hmac)
-
-    def fetch_hash_function(self, name, typ):
-        entry = hashlib_helper._EXPLICIT_CONSTRUCTORS[name]
-        match typ:
+    def fetch_hash_function(self, name, implementation):
+        info = hashlib_helper.get_hash_info(name)
+        match implementation:
             case "hashlib":
-                assert entry.hashlib is not None, entry
-                return getattr(self.hashlib, entry.hashlib)
+                assert info.hashlib is not None, info
+                return getattr(self.hashlib, info.hashlib)
             case "openssl":
                 try:
-                    return getattr(self._hashlib, entry.openssl, None)
+                    return getattr(self._hashlib, info.openssl, None)
                 except TypeError:
                     return None
-            case "builtin":
-                return self.try_import_attribute(entry.fullname(typ))
+        fullname = info.fullname(implementation)
+        return self.try_import_attribute(fullname)
 
     def fetch_hmac_function(self, name):
         fullname = hashlib_helper._EXPLICIT_HMAC_CONSTRUCTORS[name]
@@ -914,16 +919,12 @@ def check_builtin_hmac(self, name, *, disabled=True):
     )
     def test_disable_hash(self, name, allow_openssl, allow_builtin):
         # In FIPS mode, the function may be available but would still need
-        # to raise a ValueError. For simplicity, we don't test the helper
-        # when we're in FIPS mode.
-        if self._hashlib.get_fips_mode():
-            self.skipTest("hash functions may still be blocked in FIPS mode")
+        # to raise a ValueError, so we will test the helper separately.
+        self.skip_if_fips_mode()
         flags = dict(allow_openssl=allow_openssl, allow_builtin=allow_builtin)
-        is_simple_disabled = not allow_builtin and not allow_openssl
+        is_fully_disabled = not allow_builtin and not allow_openssl
 
         with hashlib_helper.block_algorithm(name, **flags):
-            self.validate_modules()
-
             # OpenSSL's blake2s and blake2b are unknown names
             # when only the OpenSSL interface is available.
             if allow_openssl and not allow_builtin:
@@ -932,25 +933,104 @@ def test_disable_hash(self, name, allow_openssl, allow_builtin):
             else:
                 name_for_hashlib_new = name
 
-            with self.check_context(is_simple_disabled):
+            with self.check_context(is_fully_disabled):
                 _ = self.hashlib.new(name_for_hashlib_new)
-            with self.check_context(is_simple_disabled):
-                _ = getattr(self.hashlib, name)(b"")
+
+            # Since _hashlib is present, explicit blake2b/blake2s constructors
+            # use the built-in implementation, while others (since we are not
+            # in FIPS mode and since _hashlib exists) use the OpenSSL function.
+            with self.check_context(is_fully_disabled):
+                _ = getattr(self.hashlib, name)()
 
             self.check_openssl_hash(name, disabled=not allow_openssl)
             self.check_builtin_hash(name, disabled=not allow_builtin)
 
             if name not in hashlib_helper.NON_HMAC_DIGEST_NAMES:
-                with self.check_context(is_simple_disabled):
+                with self.check_context(is_fully_disabled):
                     _ = self.hmac.new(b"", b"", name)
-                with self.check_context(is_simple_disabled):
+                with self.check_context(is_fully_disabled):
                     _ = self.hmac.HMAC(b"", b"", name)
-                with self.check_context(is_simple_disabled):
+                with self.check_context(is_fully_disabled):
                     _ = self.hmac.digest(b"", b"", name)
 
                 self.check_openssl_hmac(name, disabled=not allow_openssl)
                 self.check_builtin_hmac(name, disabled=not allow_builtin)
 
+    @hashlib_helper.block_algorithm("md5")
+    def test_disable_hash_md5_in_fips_mode(self):
+        self.skip_if_not_fips_mode()
+
+        self.assertRaises(ValueError, self.hashlib.new, "md5")
+        self.assertRaises(ValueError, self._hashlib.new, "md5")
+        self.assertRaises(ValueError, self.hashlib.md5)
+        self.assertRaises(ValueError, self._hashlib.openssl_md5)
+
+        kwargs = dict(usedforsecurity=True)
+        self.assertRaises(ValueError, self.hashlib.new, "md5", **kwargs)
+        self.assertRaises(ValueError, self._hashlib.new, "md5", **kwargs)
+        self.assertRaises(ValueError, self.hashlib.md5, **kwargs)
+        self.assertRaises(ValueError, self._hashlib.openssl_md5, **kwargs)
+
+    @hashlib_helper.block_algorithm("md5", allow_openssl=True)
+    def test_disable_hash_md5_in_fips_mode_allow_openssl(self):
+        self.skip_if_not_fips_mode()
+        # Allow the OpenSSL interface to be used but not the HACL* one.
+        # hashlib.new("md5") is dispatched to hashlib.openssl_md5()
+        self.assertRaises(ValueError, self.hashlib.new, "md5")
+        # dispatched to hashlib.openssl_md5() in FIPS mode
+        h2 = self.hashlib.new("md5", usedforsecurity=False)
+        self.assertIsInstance(h2, self._hashlib.HASH)
+
+        # block_algorithm() does not mock hashlib.md5 and _hashlib.openssl_md5
+        self.assertNotHasAttr(self.hashlib.md5, "__wrapped__")
+        self.assertNotHasAttr(self._hashlib.openssl_md5, "__wrapped__")
+
+        hashlib_md5 = inspect.unwrap(self.hashlib.md5)
+        self.assertIs(hashlib_md5, self._hashlib.openssl_md5)
+        self.assertRaises(ValueError, self.hashlib.md5)
+        # allow MD5 to be used in FIPS mode if usedforsecurity=False
+        h3 = self.hashlib.md5(usedforsecurity=False)
+        self.assertIsInstance(h3, self._hashlib.HASH)
+
+    @hashlib_helper.block_algorithm("md5", allow_builtin=True)
+    def test_disable_hash_md5_in_fips_mode_allow_builtin(self):
+        self.skip_if_not_fips_mode()
+        # Allow the HACL* interface to be used but not the OpenSSL one.
+        h1 = self.hashlib.new("md5")  # dispatched to _md5.md5()
+        self.assertNotIsInstance(h1, self._hashlib.HASH)
+        h2 = self.hashlib.new("md5", usedforsecurity=False)
+        self.assertIsInstance(h2, type(h1))
+
+        # block_algorithm() mocks hashlib.md5 and _hashlib.openssl_md5
+        self.assertHasAttr(self.hashlib.md5, "__wrapped__")
+        self.assertHasAttr(self._hashlib.openssl_md5, "__wrapped__")
+
+        hashlib_md5 = inspect.unwrap(self.hashlib.md5)
+        openssl_md5 = inspect.unwrap(self._hashlib.openssl_md5)
+        self.assertIs(hashlib_md5, openssl_md5)
+        self.assertRaises(ValueError, self.hashlib.md5)
+        self.assertRaises(ValueError, self.hashlib.md5,
+                          usedforsecurity=False)
+
+    @hashlib_helper.block_algorithm("md5",
+                                    allow_openssl=True,
+                                    allow_builtin=True)
+    def test_disable_hash_md5_in_fips_mode_allow_all(self):
+        self.skip_if_not_fips_mode()
+        # hashlib.new() isn't blocked as it falls back to _md5.md5
+        self.assertIsInstance(self.hashlib.new("md5"), self._md5.MD5Type)
+        self.assertRaises(ValueError, self._hashlib.new, "md5")
+        h = self._hashlib.new("md5", usedforsecurity=False)
+        self.assertIsInstance(h, self._hashlib.HASH)
+
+        self.assertNotHasAttr(self.hashlib.md5, "__wrapped__")
+        self.assertNotHasAttr(self._hashlib.openssl_md5, "__wrapped__")
+
+        self.assertIs(self.hashlib.md5, self._hashlib.openssl_md5)
+        self.assertRaises(ValueError, self.hashlib.md5)
+        h = self.hashlib.md5(usedforsecurity=False)
+        self.assertIsInstance(h, self._hashlib.HASH)
+
 
 if __name__ == '__main__':
     unittest.main()