bpo-9216: hashlib usedforsecurity fixes (GH-20258)

func:`hashlib.new` passed ``usedforsecurity`` to OpenSSL EVP constructor
``_hashlib.new()``. test_hashlib and test_smtplib handle strict security
policy better.

Signed-off-by: Christian Heimes <[email protected]>

Automerge-Triggered-By: @tiran

Author: tiran

Lib/hashlib.py

@@ -154,7 +154,7 @@ def __hash_new(name, data=b'', **kwargs):
         # salt, personal, tree hashing or SSE.
         return __get_builtin_constructor(name)(data, **kwargs)
     try:
-        return _hashlib.new(name, data)
+        return _hashlib.new(name, data, **kwargs)
     except ValueError:
         # If the _hashlib module (OpenSSL) doesn't support the named
         # hash, try using our builtin implementations.

Lib/test/test_hashlib.py

@@ -13,6 +13,7 @@
 import itertools
 import os
 import sys
+import sysconfig
 import threading
 import unittest
 import warnings
@@ -26,11 +27,20 @@
 c_hashlib = import_fresh_module('hashlib', fresh=['_hashlib'])
 py_hashlib = import_fresh_module('hashlib', blocked=['_hashlib'])
 
+builtin_hashes = sysconfig.get_config_var("PY_BUILTIN_HASHLIB_HASHES")
+if builtin_hashes is None:
+    builtin_hashes = {'md5', 'sha1', 'sha256', 'sha512', 'sha3', 'blake2'}
+else:
+    builtin_hashes = {
+        m.strip() for m in builtin_hashes.strip('"').lower().split(",")
+    }
+
 try:
-    from _hashlib import HASH, HASHXOF
+    from _hashlib import HASH, HASHXOF, openssl_md_meth_names
 except ImportError:
     HASH = None
     HASHXOF = None
+    openssl_md_meth_names = frozenset()
 
 try:
     import _blake2
@@ -175,10 +185,17 @@ def hash_constructors(self):
         constructors = self.constructors_to_test.values()
         return itertools.chain.from_iterable(constructors)
 
+    @property
+    def is_fips_mode(self):
+        if hasattr(self._hashlib, "get_fips_mode"):
+            return self._hashlib.get_fips_mode()
+        else:
+            return None
+
     def test_hash_array(self):
         a = array.array("b", range(10))
         for cons in self.hash_constructors:
-            c = cons(a)
+            c = cons(a, usedforsecurity=False)
             if c.name in self.shakes:
                 c.hexdigest(16)
             else:
@@ -193,14 +210,26 @@ def test_algorithms_available(self):
         self.assertTrue(set(hashlib.algorithms_guaranteed).
                             issubset(hashlib.algorithms_available))
 
-    def test_usedforsecurity(self):
+    def test_usedforsecurity_true(self):
+        hashlib.new("sha256", usedforsecurity=True)
+        if self.is_fips_mode:
+            self.skipTest("skip in FIPS mode")
         for cons in self.hash_constructors:
             cons(usedforsecurity=True)
-            cons(usedforsecurity=False)
             cons(b'', usedforsecurity=True)
-            cons(b'', usedforsecurity=False)
-        hashlib.new("sha256", usedforsecurity=True)
+        hashlib.new("md5", usedforsecurity=True)
+        hashlib.md5(usedforsecurity=True)
+        if self._hashlib is not None:
+            self._hashlib.new("md5", usedforsecurity=True)
+            self._hashlib.openssl_md5(usedforsecurity=True)
+
+    def test_usedforsecurity_false(self):
         hashlib.new("sha256", usedforsecurity=False)
+        for cons in self.hash_constructors:
+            cons(usedforsecurity=False)
+            cons(b'', usedforsecurity=False)
+        hashlib.new("md5", usedforsecurity=False)
+        hashlib.md5(usedforsecurity=False)
         if self._hashlib is not None:
             self._hashlib.new("md5", usedforsecurity=False)
             self._hashlib.openssl_md5(usedforsecurity=False)
@@ -240,7 +269,7 @@ def test_get_builtin_constructor(self):
 
     def test_hexdigest(self):
         for cons in self.hash_constructors:
-            h = cons()
+            h = cons(usedforsecurity=False)
             if h.name in self.shakes:
                 self.assertIsInstance(h.digest(16), bytes)
                 self.assertEqual(hexstr(h.digest(16)), h.hexdigest(16))
@@ -252,7 +281,7 @@ def test_digest_length_overflow(self):
         # See issue #34922
         large_sizes = (2**29, 2**32-10, 2**32+10, 2**61, 2**64-10, 2**64+10)
         for cons in self.hash_constructors:
-            h = cons()
+            h = cons(usedforsecurity=False)
             if h.name not in self.shakes:
                 continue
             if HASH is not None and isinstance(h, HASH):
@@ -266,13 +295,16 @@ def test_digest_length_overflow(self):
 
     def test_name_attribute(self):
         for cons in self.hash_constructors:
-            h = cons()
+            h = cons(usedforsecurity=False)
             self.assertIsInstance(h.name, str)
             if h.name in self.supported_hash_names:
                 self.assertIn(h.name, self.supported_hash_names)
             else:
                 self.assertNotIn(h.name, self.supported_hash_names)
-            self.assertEqual(h.name, hashlib.new(h.name).name)
+            self.assertEqual(
+                h.name,
+                hashlib.new(h.name, usedforsecurity=False).name
+            )
 
     def test_large_update(self):
         aas = b'a' * 128
@@ -281,7 +313,7 @@ def test_large_update(self):
         dees = b'd' * 2048 #  HASHLIB_GIL_MINSIZE
 
         for cons in self.hash_constructors:
-            m1 = cons()
+            m1 = cons(usedforsecurity=False)
             m1.update(aas)
             m1.update(bees)
             m1.update(cees)
@@ -291,15 +323,15 @@ def test_large_update(self):
             else:
                 args = ()
 
-            m2 = cons()
+            m2 = cons(usedforsecurity=False)
             m2.update(aas + bees + cees + dees)
             self.assertEqual(m1.digest(*args), m2.digest(*args))
 
-            m3 = cons(aas + bees + cees + dees)
+            m3 = cons(aas + bees + cees + dees, usedforsecurity=False)
             self.assertEqual(m1.digest(*args), m3.digest(*args))
 
             # verify copy() doesn't touch original
-            m4 = cons(aas + bees + cees)
+            m4 = cons(aas + bees + cees, usedforsecurity=False)
             m4_digest = m4.digest(*args)
             m4_copy = m4.copy()
             m4_copy.update(dees)
@@ -359,7 +391,7 @@ def check_blocksize_name(self, name, block_size=0, digest_size=0,
                              digest_length=None):
         constructors = self.constructors_to_test[name]
         for hash_object_constructor in constructors:
-            m = hash_object_constructor()
+            m = hash_object_constructor(usedforsecurity=False)
             self.assertEqual(m.block_size, block_size)
             self.assertEqual(m.digest_size, digest_size)
             if digest_length:
@@ -418,15 +450,24 @@ def test_blocksize_name_blake2(self):
         self.check_blocksize_name('blake2s', 64, 32)
 
     def test_case_md5_0(self):
-        self.check('md5', b'', 'd41d8cd98f00b204e9800998ecf8427e')
+        self.check(
+            'md5', b'', 'd41d8cd98f00b204e9800998ecf8427e',
+            usedforsecurity=False
+        )
 
     def test_case_md5_1(self):
-        self.check('md5', b'abc', '900150983cd24fb0d6963f7d28e17f72')
+        self.check(
+            'md5', b'abc', '900150983cd24fb0d6963f7d28e17f72',
+            usedforsecurity=False
+        )
 
     def test_case_md5_2(self):
-        self.check('md5',
-                   b'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789',
-                   'd174ab98d277d9f5a5611c2c9f419d9f')
+        self.check(
+            'md5',
+            b'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789',
+            'd174ab98d277d9f5a5611c2c9f419d9f',
+            usedforsecurity=False
+        )
 
     @unittest.skipIf(sys.maxsize < _4G + 5, 'test cannot run on 32-bit systems')
     @bigmemtest(size=_4G + 5, memuse=1, dry_run=False)
@@ -806,22 +847,28 @@ def test_gil(self):
         gil_minsize = 2048
 
         for cons in self.hash_constructors:
-            m = cons()
+            m = cons(usedforsecurity=False)
             m.update(b'1')
             m.update(b'#' * gil_minsize)
             m.update(b'1')
 
-            m = cons(b'x' * gil_minsize)
+            m = cons(b'x' * gil_minsize, usedforsecurity=False)
             m.update(b'1')
 
-        m = hashlib.md5()
+        m = hashlib.sha256()
         m.update(b'1')
         m.update(b'#' * gil_minsize)
         m.update(b'1')
-        self.assertEqual(m.hexdigest(), 'cb1e1a2cbc80be75e19935d621fb9b21')
+        self.assertEqual(
+            m.hexdigest(),
+            '1cfceca95989f51f658e3f3ffe7f1cd43726c9e088c13ee10b46f57cef135b94'
+        )
 
-        m = hashlib.md5(b'x' * gil_minsize)
-        self.assertEqual(m.hexdigest(), 'cfb767f225d58469c5de3632a8803958')
+        m = hashlib.sha256(b'1' + b'#' * gil_minsize + b'1')
+        self.assertEqual(
+            m.hexdigest(),
+            '1cfceca95989f51f658e3f3ffe7f1cd43726c9e088c13ee10b46f57cef135b94'
+        )
 
     @support.reap_threads
     def test_threaded_hashing(self):
@@ -859,10 +906,10 @@ def hash_in_chunks(chunk_size):
 
         self.assertEqual(expected_hash, hasher.hexdigest())
 
-    @unittest.skipUnless(hasattr(c_hashlib, 'get_fips_mode'),
-                         'need _hashlib.get_fips_mode')
     def test_get_fips_mode(self):
-        self.assertIsInstance(c_hashlib.get_fips_mode(), int)
+        fips_mode = self.is_fips_mode
+        if fips_mode is not None:
+            self.assertIsInstance(fips_mode, int)
 
     @unittest.skipUnless(HASH is not None, 'need _hashlib')
     def test_internal_types(self):
@@ -934,8 +981,10 @@ class KDFTests(unittest.TestCase):
             (bytes.fromhex('9d9e9c4cd21fe4be24d5b8244c759665'), None),],
     }
 
-    def _test_pbkdf2_hmac(self, pbkdf2):
+    def _test_pbkdf2_hmac(self, pbkdf2, supported):
         for digest_name, results in self.pbkdf2_results.items():
+            if digest_name not in supported:
+                continue
             for i, vector in enumerate(self.pbkdf2_test_vectors):
                 password, salt, rounds, dklen = vector
                 expected, overwrite_dklen = results[i]
@@ -946,6 +995,7 @@ def _test_pbkdf2_hmac(self, pbkdf2):
                                  (digest_name, password, salt, rounds, dklen))
                 out = pbkdf2(digest_name, memoryview(password),
                              memoryview(salt), rounds, dklen)
+                self.assertEqual(out, expected)
                 out = pbkdf2(digest_name, bytearray(password),
                              bytearray(salt), rounds, dklen)
                 self.assertEqual(out, expected)
@@ -967,12 +1017,12 @@ def _test_pbkdf2_hmac(self, pbkdf2):
         self.assertEqual(out, self.pbkdf2_results['sha1'][0][0])
 
     def test_pbkdf2_hmac_py(self):
-        self._test_pbkdf2_hmac(py_hashlib.pbkdf2_hmac)
+        self._test_pbkdf2_hmac(py_hashlib.pbkdf2_hmac, builtin_hashes)
 
     @unittest.skipUnless(hasattr(c_hashlib, 'pbkdf2_hmac'),
                      '   test requires OpenSSL > 1.0')
     def test_pbkdf2_hmac_c(self):
-        self._test_pbkdf2_hmac(c_hashlib.pbkdf2_hmac)
+        self._test_pbkdf2_hmac(c_hashlib.pbkdf2_hmac, openssl_md_meth_names)
 
 
     @unittest.skipUnless(hasattr(c_hashlib, 'scrypt'),

Lib/test/test_smtplib.py

@@ -1067,6 +1067,7 @@ def testAUTH_CRAM_MD5(self):
         self.assertEqual(resp, (235, b'Authentication Succeeded'))
         smtp.close()
 
+    @hashlib_helper.requires_hashdigest('md5')
     def testAUTH_multiple(self):
         # Test that multiple authentication methods are tried.
         self.serv.add_feature("AUTH BOGUS PLAIN LOGIN CRAM-MD5")

Lib/test/test_tools/test_md5sum.py

@@ -3,12 +3,14 @@
 import os
 import unittest
 from test import support
+from test.support import hashlib_helper
 from test.support.script_helper import assert_python_ok, assert_python_failure
 
 from test.test_tools import scriptsdir, skip_if_missing
 
 skip_if_missing()
 
+@hashlib_helper.requires_hashdigest('md5')
 class MD5SumTests(unittest.TestCase):
     @classmethod
     def setUpClass(cls):

Lib/test/test_urllib2_localnet.py

@@ -316,14 +316,14 @@ def test_basic_auth_httperror(self):
         self.assertRaises(urllib.error.HTTPError, urllib.request.urlopen, self.server_url)
 
 
+@hashlib_helper.requires_hashdigest("md5")
 class ProxyAuthTests(unittest.TestCase):
     URL = "http://localhost"
 
     USER = "tester"
     PASSWD = "test123"
     REALM = "TestRealm"
 
-    @hashlib_helper.requires_hashdigest("md5")
     def setUp(self):
         super(ProxyAuthTests, self).setUp()
         # Ignore proxy bypass settings in the environment.

Misc/NEWS.d/next/Library/2020-05-20-12-53-20.bpo-9216.ps7Yf1.rst

@@ -0,0 +1,3 @@
+func:`hashlib.new` passed ``usedforsecurity`` to OpenSSL EVP constructor
+``_hashlib.new()``. test_hashlib and test_smtplib handle strict security
+policy better.