Skip to content

Commit a08b7c3

Browse files
miss-islingtontiran
miss-islington
and
tiran
authored
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 <christian@python.org> Automerge-Triggered-By: @tiran (cherry picked from commit 909b571) Co-authored-by: Christian Heimes <christian@python.org>
1 parent 983b17c commit a08b7c3

File tree

6 files changed

+90
-34
lines changed

6 files changed

+90
-34
lines changed

‎Lib/hashlib.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -154,7 +154,7 @@ def __hash_new(name, data=b'', **kwargs):
154154
# salt, personal, tree hashing or SSE.
155155
return __get_builtin_constructor(name)(data, **kwargs)
156156
try:
157-
return _hashlib.new(name, data)
157+
return _hashlib.new(name, data, **kwargs)
158158
except ValueError:
159159
# If the _hashlib module (OpenSSL) doesn't support the named
160160
# hash, try using our builtin implementations.

‎Lib/test/test_hashlib.py

Lines changed: 82 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import itertools
1414
import os
1515
import sys
16+
import sysconfig
1617
import threading
1718
import unittest
1819
import warnings
@@ -26,11 +27,20 @@
2627
c_hashlib = import_fresh_module('hashlib', fresh=['_hashlib'])
2728
py_hashlib = import_fresh_module('hashlib', blocked=['_hashlib'])
2829

30+
builtin_hashes = sysconfig.get_config_var("PY_BUILTIN_HASHLIB_HASHES")
31+
if builtin_hashes is None:
32+
builtin_hashes = {'md5', 'sha1', 'sha256', 'sha512', 'sha3', 'blake2'}
33+
else:
34+
builtin_hashes = {
35+
m.strip() for m in builtin_hashes.strip('"').lower().split(",")
36+
}
37+
2938
try:
30-
from _hashlib import HASH, HASHXOF
39+
from _hashlib import HASH, HASHXOF, openssl_md_meth_names
3140
except ImportError:
3241
HASH = None
3342
HASHXOF = None
43+
openssl_md_meth_names = frozenset()
3444

3545
try:
3646
import _blake2
@@ -175,10 +185,17 @@ def hash_constructors(self):
175185
constructors = self.constructors_to_test.values()
176186
return itertools.chain.from_iterable(constructors)
177187

188+
@property
189+
def is_fips_mode(self):
190+
if hasattr(self._hashlib, "get_fips_mode"):
191+
return self._hashlib.get_fips_mode()
192+
else:
193+
return None
194+
178195
def test_hash_array(self):
179196
a = array.array("b", range(10))
180197
for cons in self.hash_constructors:
181-
c = cons(a)
198+
c = cons(a, usedforsecurity=False)
182199
if c.name in self.shakes:
183200
c.hexdigest(16)
184201
else:
@@ -193,14 +210,26 @@ def test_algorithms_available(self):
193210
self.assertTrue(set(hashlib.algorithms_guaranteed).
194211
issubset(hashlib.algorithms_available))
195212

196-
def test_usedforsecurity(self):
213+
def test_usedforsecurity_true(self):
214+
hashlib.new("sha256", usedforsecurity=True)
215+
if self.is_fips_mode:
216+
self.skipTest("skip in FIPS mode")
197217
for cons in self.hash_constructors:
198218
cons(usedforsecurity=True)
199-
cons(usedforsecurity=False)
200219
cons(b'', usedforsecurity=True)
201-
cons(b'', usedforsecurity=False)
202-
hashlib.new("sha256", usedforsecurity=True)
220+
hashlib.new("md5", usedforsecurity=True)
221+
hashlib.md5(usedforsecurity=True)
222+
if self._hashlib is not None:
223+
self._hashlib.new("md5", usedforsecurity=True)
224+
self._hashlib.openssl_md5(usedforsecurity=True)
225+
226+
def test_usedforsecurity_false(self):
203227
hashlib.new("sha256", usedforsecurity=False)
228+
for cons in self.hash_constructors:
229+
cons(usedforsecurity=False)
230+
cons(b'', usedforsecurity=False)
231+
hashlib.new("md5", usedforsecurity=False)
232+
hashlib.md5(usedforsecurity=False)
204233
if self._hashlib is not None:
205234
self._hashlib.new("md5", usedforsecurity=False)
206235
self._hashlib.openssl_md5(usedforsecurity=False)
@@ -240,7 +269,7 @@ def test_get_builtin_constructor(self):
240269

241270
def test_hexdigest(self):
242271
for cons in self.hash_constructors:
243-
h = cons()
272+
h = cons(usedforsecurity=False)
244273
if h.name in self.shakes:
245274
self.assertIsInstance(h.digest(16), bytes)
246275
self.assertEqual(hexstr(h.digest(16)), h.hexdigest(16))
@@ -252,7 +281,7 @@ def test_digest_length_overflow(self):
252281
# See issue #34922
253282
large_sizes = (2**29, 2**32-10, 2**32+10, 2**61, 2**64-10, 2**64+10)
254283
for cons in self.hash_constructors:
255-
h = cons()
284+
h = cons(usedforsecurity=False)
256285
if h.name not in self.shakes:
257286
continue
258287
if HASH is not None and isinstance(h, HASH):
@@ -266,13 +295,16 @@ def test_digest_length_overflow(self):
266295

267296
def test_name_attribute(self):
268297
for cons in self.hash_constructors:
269-
h = cons()
298+
h = cons(usedforsecurity=False)
270299
self.assertIsInstance(h.name, str)
271300
if h.name in self.supported_hash_names:
272301
self.assertIn(h.name, self.supported_hash_names)
273302
else:
274303
self.assertNotIn(h.name, self.supported_hash_names)
275-
self.assertEqual(h.name, hashlib.new(h.name).name)
304+
self.assertEqual(
305+
h.name,
306+
hashlib.new(h.name, usedforsecurity=False).name
307+
)
276308

277309
def test_large_update(self):
278310
aas = b'a' * 128
@@ -281,7 +313,7 @@ def test_large_update(self):
281313
dees = b'd' * 2048 # HASHLIB_GIL_MINSIZE
282314

283315
for cons in self.hash_constructors:
284-
m1 = cons()
316+
m1 = cons(usedforsecurity=False)
285317
m1.update(aas)
286318
m1.update(bees)
287319
m1.update(cees)
@@ -291,15 +323,15 @@ def test_large_update(self):
291323
else:
292324
args = ()
293325

294-
m2 = cons()
326+
m2 = cons(usedforsecurity=False)
295327
m2.update(aas + bees + cees + dees)
296328
self.assertEqual(m1.digest(*args), m2.digest(*args))
297329

298-
m3 = cons(aas + bees + cees + dees)
330+
m3 = cons(aas + bees + cees + dees, usedforsecurity=False)
299331
self.assertEqual(m1.digest(*args), m3.digest(*args))
300332

301333
# verify copy() doesn't touch original
302-
m4 = cons(aas + bees + cees)
334+
m4 = cons(aas + bees + cees, usedforsecurity=False)
303335
m4_digest = m4.digest(*args)
304336
m4_copy = m4.copy()
305337
m4_copy.update(dees)
@@ -359,7 +391,7 @@ def check_blocksize_name(self, name, block_size=0, digest_size=0,
359391
digest_length=None):
360392
constructors = self.constructors_to_test[name]
361393
for hash_object_constructor in constructors:
362-
m = hash_object_constructor()
394+
m = hash_object_constructor(usedforsecurity=False)
363395
self.assertEqual(m.block_size, block_size)
364396
self.assertEqual(m.digest_size, digest_size)
365397
if digest_length:
@@ -418,15 +450,24 @@ def test_blocksize_name_blake2(self):
418450
self.check_blocksize_name('blake2s', 64, 32)
419451

420452
def test_case_md5_0(self):
421-
self.check('md5', b'', 'd41d8cd98f00b204e9800998ecf8427e')
453+
self.check(
454+
'md5', b'', 'd41d8cd98f00b204e9800998ecf8427e',
455+
usedforsecurity=False
456+
)
422457

423458
def test_case_md5_1(self):
424-
self.check('md5', b'abc', '900150983cd24fb0d6963f7d28e17f72')
459+
self.check(
460+
'md5', b'abc', '900150983cd24fb0d6963f7d28e17f72',
461+
usedforsecurity=False
462+
)
425463

426464
def test_case_md5_2(self):
427-
self.check('md5',
428-
b'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789',
429-
'd174ab98d277d9f5a5611c2c9f419d9f')
465+
self.check(
466+
'md5',
467+
b'ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789',
468+
'd174ab98d277d9f5a5611c2c9f419d9f',
469+
usedforsecurity=False
470+
)
430471

431472
@unittest.skipIf(sys.maxsize < _4G + 5, 'test cannot run on 32-bit systems')
432473
@bigmemtest(size=_4G + 5, memuse=1, dry_run=False)
@@ -806,22 +847,28 @@ def test_gil(self):
806847
gil_minsize = 2048
807848

808849
for cons in self.hash_constructors:
809-
m = cons()
850+
m = cons(usedforsecurity=False)
810851
m.update(b'1')
811852
m.update(b'#' * gil_minsize)
812853
m.update(b'1')
813854

814-
m = cons(b'x' * gil_minsize)
855+
m = cons(b'x' * gil_minsize, usedforsecurity=False)
815856
m.update(b'1')
816857

817-
m = hashlib.md5()
858+
m = hashlib.sha256()
818859
m.update(b'1')
819860
m.update(b'#' * gil_minsize)
820861
m.update(b'1')
821-
self.assertEqual(m.hexdigest(), 'cb1e1a2cbc80be75e19935d621fb9b21')
862+
self.assertEqual(
863+
m.hexdigest(),
864+
'1cfceca95989f51f658e3f3ffe7f1cd43726c9e088c13ee10b46f57cef135b94'
865+
)
822866

823-
m = hashlib.md5(b'x' * gil_minsize)
824-
self.assertEqual(m.hexdigest(), 'cfb767f225d58469c5de3632a8803958')
867+
m = hashlib.sha256(b'1' + b'#' * gil_minsize + b'1')
868+
self.assertEqual(
869+
m.hexdigest(),
870+
'1cfceca95989f51f658e3f3ffe7f1cd43726c9e088c13ee10b46f57cef135b94'
871+
)
825872

826873
@support.reap_threads
827874
def test_threaded_hashing(self):
@@ -859,10 +906,10 @@ def hash_in_chunks(chunk_size):
859906

860907
self.assertEqual(expected_hash, hasher.hexdigest())
861908

862-
@unittest.skipUnless(hasattr(c_hashlib, 'get_fips_mode'),
863-
'need _hashlib.get_fips_mode')
864909
def test_get_fips_mode(self):
865-
self.assertIsInstance(c_hashlib.get_fips_mode(), int)
910+
fips_mode = self.is_fips_mode
911+
if fips_mode is not None:
912+
self.assertIsInstance(fips_mode, int)
866913

867914
@unittest.skipUnless(HASH is not None, 'need _hashlib')
868915
def test_internal_types(self):
@@ -934,8 +981,10 @@ class KDFTests(unittest.TestCase):
934981
(bytes.fromhex('9d9e9c4cd21fe4be24d5b8244c759665'), None),],
935982
}
936983

937-
def _test_pbkdf2_hmac(self, pbkdf2):
984+
def _test_pbkdf2_hmac(self, pbkdf2, supported):
938985
for digest_name, results in self.pbkdf2_results.items():
986+
if digest_name not in supported:
987+
continue
939988
for i, vector in enumerate(self.pbkdf2_test_vectors):
940989
password, salt, rounds, dklen = vector
941990
expected, overwrite_dklen = results[i]
@@ -946,6 +995,7 @@ def _test_pbkdf2_hmac(self, pbkdf2):
946995
(digest_name, password, salt, rounds, dklen))
947996
out = pbkdf2(digest_name, memoryview(password),
948997
memoryview(salt), rounds, dklen)
998+
self.assertEqual(out, expected)
949999
out = pbkdf2(digest_name, bytearray(password),
9501000
bytearray(salt), rounds, dklen)
9511001
self.assertEqual(out, expected)
@@ -967,12 +1017,12 @@ def _test_pbkdf2_hmac(self, pbkdf2):
9671017
self.assertEqual(out, self.pbkdf2_results['sha1'][0][0])
9681018

9691019
def test_pbkdf2_hmac_py(self):
970-
self._test_pbkdf2_hmac(py_hashlib.pbkdf2_hmac)
1020+
self._test_pbkdf2_hmac(py_hashlib.pbkdf2_hmac, builtin_hashes)
9711021

9721022
@unittest.skipUnless(hasattr(c_hashlib, 'pbkdf2_hmac'),
9731023
' test requires OpenSSL > 1.0')
9741024
def test_pbkdf2_hmac_c(self):
975-
self._test_pbkdf2_hmac(c_hashlib.pbkdf2_hmac)
1025+
self._test_pbkdf2_hmac(c_hashlib.pbkdf2_hmac, openssl_md_meth_names)
9761026

9771027

9781028
@unittest.skipUnless(hasattr(c_hashlib, 'scrypt'),

‎Lib/test/test_smtplib.py

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1067,6 +1067,7 @@ def testAUTH_CRAM_MD5(self):
10671067
self.assertEqual(resp, (235, b'Authentication Succeeded'))
10681068
smtp.close()
10691069

1070+
@hashlib_helper.requires_hashdigest('md5')
10701071
def testAUTH_multiple(self):
10711072
# Test that multiple authentication methods are tried.
10721073
self.serv.add_feature("AUTH BOGUS PLAIN LOGIN CRAM-MD5")

‎Lib/test/test_tools/test_md5sum.py

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,12 +3,14 @@
33
import os
44
import unittest
55
from test import support
6+
from test.support import hashlib_helper
67
from test.support.script_helper import assert_python_ok, assert_python_failure
78

89
from test.test_tools import scriptsdir, skip_if_missing
910

1011
skip_if_missing()
1112

13+
@hashlib_helper.requires_hashdigest('md5')
1214
class MD5SumTests(unittest.TestCase):
1315
@classmethod
1416
def setUpClass(cls):

‎Lib/test/test_urllib2_localnet.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -316,14 +316,14 @@ def test_basic_auth_httperror(self):
316316
self.assertRaises(urllib.error.HTTPError, urllib.request.urlopen, self.server_url)
317317

318318

319+
@hashlib_helper.requires_hashdigest("md5")
319320
class ProxyAuthTests(unittest.TestCase):
320321
URL = "http://localhost"
321322

322323
USER = "tester"
323324
PASSWD = "test123"
324325
REALM = "TestRealm"
325326

326-
@hashlib_helper.requires_hashdigest("md5")
327327
def setUp(self):
328328
super(ProxyAuthTests, self).setUp()
329329
# Ignore proxy bypass settings in the environment.

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

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
func:`hashlib.new` passed ``usedforsecurity`` to OpenSSL EVP constructor
2+
``_hashlib.new()``. test_hashlib and test_smtplib handle strict security
3+
policy better.

0 commit comments

Comments
 (0)