From c64a1a61e6fc542cada40eb069a239317e1af36e Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Wed, 25 Sep 2019 16:30:20 +0200 Subject: [PATCH] bpo-38270: Check for hash digest algorithms and avoid MD5 (GH-16382) Make it easier to run and test Python on systems with restrict crypto policies: * add requires_hashdigest to test.support to check if a hash digest algorithm is available and working * avoid MD5 in test_hmac * replace MD5 with SHA256 in test_tarfile * mark network tests that require MD5 for MD5-based digest auth or CRAM-MD5 https://bugs.python.org/issue38270 --- Lib/test/support/__init__.py | 24 ++++++++ Lib/test/test_hmac.py | 60 +++++++++++++------ Lib/test/test_imaplib.py | 6 +- Lib/test/test_poplib.py | 2 + Lib/test/test_smtplib.py | 11 +++- Lib/test/test_tarfile.py | 56 +++++++++-------- Lib/test/test_urllib2_localnet.py | 1 + .../2019-09-25-12-18-31.bpo-38270._x-9uH.rst | 4 ++ 8 files changed, 119 insertions(+), 45 deletions(-) create mode 100644 Misc/NEWS.d/next/Tests/2019-09-25-12-18-31.bpo-38270._x-9uH.rst diff --git a/Lib/test/support/__init__.py b/Lib/test/support/__init__.py index 46d646fe5b7..e401090c214 100644 --- a/Lib/test/support/__init__.py +++ b/Lib/test/support/__init__.py @@ -12,6 +12,7 @@ import fnmatch import functools import gc import glob +import hashlib import importlib import importlib.util import locale @@ -648,6 +649,29 @@ def requires_mac_ver(*min_version): return decorator +def requires_hashdigest(digestname): + """Decorator raising SkipTest if a hashing algorithm is not available + + The hashing algorithm could be missing or blocked by a strict crypto + policy. + + ValueError: [digital envelope routines: EVP_DigestInit_ex] disabled for FIPS + ValueError: unsupported hash type md4 + """ + def decorator(func): + @functools.wraps(func) + def wrapper(*args, **kwargs): + try: + hashlib.new(digestname) + except ValueError: + raise unittest.SkipTest( + f"hash digest '{digestname}' is not available." + ) + return func(*args, **kwargs) + return wrapper + return decorator + + HOST = "localhost" HOSTv4 = "127.0.0.1" HOSTv6 = "::1" diff --git a/Lib/test/test_hmac.py b/Lib/test/test_hmac.py index f2eb6d716d5..2c09de8b26f 100644 --- a/Lib/test/test_hmac.py +++ b/Lib/test/test_hmac.py @@ -6,6 +6,8 @@ import unittest import unittest.mock import warnings +from test.support import requires_hashdigest + def ignore_warning(func): @functools.wraps(func) @@ -19,6 +21,7 @@ def ignore_warning(func): class TestVectorsTestCase(unittest.TestCase): + @requires_hashdigest('md5') def test_md5_vectors(self): # Test the HMAC module against test vectors from the RFC. @@ -76,6 +79,7 @@ class TestVectorsTestCase(unittest.TestCase): b"and Larger Than One Block-Size Data"), "6f630fad67cda0ee1fb1f562db3aa53e") + @requires_hashdigest('sha1') def test_sha_vectors(self): def shatest(key, data, digest): h = hmac.HMAC(key, data, digestmod=hashlib.sha1) @@ -268,23 +272,28 @@ class TestVectorsTestCase(unittest.TestCase): '134676fb6de0446065c97440fa8c6a58', }) + @requires_hashdigest('sha224') def test_sha224_rfc4231(self): self._rfc4231_test_cases(hashlib.sha224, 'sha224', 28, 64) + @requires_hashdigest('sha256') def test_sha256_rfc4231(self): self._rfc4231_test_cases(hashlib.sha256, 'sha256', 32, 64) + @requires_hashdigest('sha384') def test_sha384_rfc4231(self): self._rfc4231_test_cases(hashlib.sha384, 'sha384', 48, 128) + @requires_hashdigest('sha512') def test_sha512_rfc4231(self): self._rfc4231_test_cases(hashlib.sha512, 'sha512', 64, 128) + @requires_hashdigest('sha256') def test_legacy_block_size_warnings(self): class MockCrazyHash(object): """Ain't no block_size attribute here.""" def __init__(self, *args): - self._x = hashlib.sha1(*args) + self._x = hashlib.sha256(*args) self.digest_size = self._x.digest_size def update(self, v): self._x.update(v) @@ -308,65 +317,78 @@ class TestVectorsTestCase(unittest.TestCase): data = b"Hi There" hmac.HMAC(key, data, digestmod=None) + class ConstructorTestCase(unittest.TestCase): + expected = ( + "6c845b47f52b3b47f6590c502db7825aad757bf4fadc8fa972f7cd2e76a5bdeb" + ) + + @requires_hashdigest('sha256') def test_normal(self): # Standard constructor call. - failed = 0 try: - h = hmac.HMAC(b"key", digestmod='md5') + hmac.HMAC(b"key", digestmod='sha256') except Exception: self.fail("Standard constructor call raised exception.") + @requires_hashdigest('sha256') def test_with_str_key(self): # Pass a key of type str, which is an error, because it expects a key # of type bytes with self.assertRaises(TypeError): - h = hmac.HMAC("key", digestmod='md5') + h = hmac.HMAC("key", digestmod='sha256') + @requires_hashdigest('sha256') def test_dot_new_with_str_key(self): # Pass a key of type str, which is an error, because it expects a key # of type bytes with self.assertRaises(TypeError): - h = hmac.new("key", digestmod='md5') + h = hmac.new("key", digestmod='sha256') + @requires_hashdigest('sha256') def test_withtext(self): # Constructor call with text. try: - h = hmac.HMAC(b"key", b"hash this!", digestmod='md5') + h = hmac.HMAC(b"key", b"hash this!", digestmod='sha256') except Exception: self.fail("Constructor call with text argument raised exception.") - self.assertEqual(h.hexdigest(), '34325b639da4cfd95735b381e28cb864') + self.assertEqual(h.hexdigest(), self.expected) + @requires_hashdigest('sha256') def test_with_bytearray(self): try: h = hmac.HMAC(bytearray(b"key"), bytearray(b"hash this!"), - digestmod="md5") + digestmod="sha256") except Exception: self.fail("Constructor call with bytearray arguments raised exception.") - self.assertEqual(h.hexdigest(), '34325b639da4cfd95735b381e28cb864') + self.assertEqual(h.hexdigest(), self.expected) + @requires_hashdigest('sha256') def test_with_memoryview_msg(self): try: - h = hmac.HMAC(b"key", memoryview(b"hash this!"), digestmod="md5") + h = hmac.HMAC(b"key", memoryview(b"hash this!"), digestmod="sha256") except Exception: self.fail("Constructor call with memoryview msg raised exception.") - self.assertEqual(h.hexdigest(), '34325b639da4cfd95735b381e28cb864') + self.assertEqual(h.hexdigest(), self.expected) + @requires_hashdigest('sha256') def test_withmodule(self): # Constructor call with text and digest module. try: - h = hmac.HMAC(b"key", b"", hashlib.sha1) + h = hmac.HMAC(b"key", b"", hashlib.sha256) except Exception: - self.fail("Constructor call with hashlib.sha1 raised exception.") + self.fail("Constructor call with hashlib.sha256 raised exception.") + class SanityTestCase(unittest.TestCase): + @requires_hashdigest('sha256') def test_exercise_all_methods(self): # Exercising all methods once. # This must not raise any exceptions try: - h = hmac.HMAC(b"my secret key", digestmod="md5") + h = hmac.HMAC(b"my secret key", digestmod="sha256") h.update(b"compute the hash of this text!") dig = h.digest() dig = h.hexdigest() @@ -374,11 +396,13 @@ class SanityTestCase(unittest.TestCase): except Exception: self.fail("Exception raised during normal usage of HMAC class.") + class CopyTestCase(unittest.TestCase): + @requires_hashdigest('sha256') def test_attributes(self): # Testing if attributes are of same type. - h1 = hmac.HMAC(b"key", digestmod="md5") + h1 = hmac.HMAC(b"key", digestmod="sha256") h2 = h1.copy() self.assertTrue(h1.digest_cons == h2.digest_cons, "digest constructors don't match.") @@ -387,9 +411,10 @@ class CopyTestCase(unittest.TestCase): self.assertEqual(type(h1.outer), type(h2.outer), "Types of outer don't match.") + @requires_hashdigest('sha256') def test_realcopy(self): # Testing if the copy method created a real copy. - h1 = hmac.HMAC(b"key", digestmod="md5") + h1 = hmac.HMAC(b"key", digestmod="sha256") h2 = h1.copy() # Using id() in case somebody has overridden __eq__/__ne__. self.assertTrue(id(h1) != id(h2), "No real copy of the HMAC instance.") @@ -398,9 +423,10 @@ class CopyTestCase(unittest.TestCase): self.assertTrue(id(h1.outer) != id(h2.outer), "No real copy of the attribute 'outer'.") + @requires_hashdigest('sha256') def test_equality(self): # Testing if the copy has the same digests. - h1 = hmac.HMAC(b"key", digestmod="md5") + h1 = hmac.HMAC(b"key", digestmod="sha256") h1.update(b"some random text") h2 = h1.copy() self.assertEqual(h1.digest(), h2.digest(), diff --git a/Lib/test/test_imaplib.py b/Lib/test/test_imaplib.py index 8ab532af3f0..03cffbe39c6 100644 --- a/Lib/test/test_imaplib.py +++ b/Lib/test/test_imaplib.py @@ -10,7 +10,8 @@ import threading import socket from test.support import (reap_threads, verbose, transient_internet, - run_with_tz, run_with_locale, cpython_only) + run_with_tz, run_with_locale, cpython_only, + requires_hashdigest) import unittest from unittest import mock from datetime import datetime, timezone, timedelta @@ -370,6 +371,7 @@ class NewIMAPTestsMixin(): self.assertEqual(code, 'OK') self.assertEqual(server.response, b'ZmFrZQ==\r\n') # b64 encoded 'fake' + @requires_hashdigest('md5') def test_login_cram_md5_bytes(self): class AuthHandler(SimpleIMAPHandler): capabilities = 'LOGINDISABLED AUTH=CRAM-MD5' @@ -387,6 +389,7 @@ class NewIMAPTestsMixin(): ret, _ = client.login_cram_md5("tim", b"tanstaaftanstaaf") self.assertEqual(ret, "OK") + @requires_hashdigest('md5') def test_login_cram_md5_plain_text(self): class AuthHandler(SimpleIMAPHandler): capabilities = 'LOGINDISABLED AUTH=CRAM-MD5' @@ -797,6 +800,7 @@ class ThreadedNetworkedTests(unittest.TestCase): b'ZmFrZQ==\r\n') # b64 encoded 'fake' @reap_threads + @requires_hashdigest('md5') def test_login_cram_md5(self): class AuthHandler(SimpleIMAPHandler): diff --git a/Lib/test/test_poplib.py b/Lib/test/test_poplib.py index 20d4eeac12d..7b1d854d1c0 100644 --- a/Lib/test/test_poplib.py +++ b/Lib/test/test_poplib.py @@ -309,9 +309,11 @@ class TestPOP3Class(TestCase): def test_rpop(self): self.assertOK(self.client.rpop('foo')) + @test_support.requires_hashdigest('md5') def test_apop_normal(self): self.assertOK(self.client.apop('foo', 'dummypassword')) + @test_support.requires_hashdigest('md5') def test_apop_REDOS(self): # Replace welcome with very long evil welcome. # NB The upper bound on welcome length is currently 2048. diff --git a/Lib/test/test_smtplib.py b/Lib/test/test_smtplib.py index f1332e9ef78..d0c9862edea 100644 --- a/Lib/test/test_smtplib.py +++ b/Lib/test/test_smtplib.py @@ -4,6 +4,7 @@ import email.mime.text from email.message import EmailMessage from email.base64mime import body_encode as encode_base64 import email.utils +import hashlib import hmac import socket import smtpd @@ -21,6 +22,7 @@ import unittest from test import support, mock_socket from test.support import HOST from test.support import threading_setup, threading_cleanup, join_thread +from test.support import requires_hashdigest from unittest.mock import Mock @@ -1009,6 +1011,7 @@ class SMTPSimTests(unittest.TestCase): self.assertEqual(resp, (235, b'Authentication Succeeded')) smtp.close() + @requires_hashdigest('md5') def testAUTH_CRAM_MD5(self): self.serv.add_feature("AUTH CRAM-MD5") smtp = smtplib.SMTP(HOST, self.port, local_hostname='localhost', timeout=15) @@ -1025,7 +1028,13 @@ class SMTPSimTests(unittest.TestCase): smtp.close() def test_auth_function(self): - supported = {'CRAM-MD5', 'PLAIN', 'LOGIN'} + supported = {'PLAIN', 'LOGIN'} + try: + hashlib.md5() + except ValueError: + pass + else: + supported.add('CRAM-MD5') for mechanism in supported: self.serv.add_feature("AUTH {}".format(mechanism)) for mechanism in supported: diff --git a/Lib/test/test_tarfile.py b/Lib/test/test_tarfile.py index 7e32cbccd6c..15324a4e488 100644 --- a/Lib/test/test_tarfile.py +++ b/Lib/test/test_tarfile.py @@ -1,7 +1,7 @@ import sys import os import io -from hashlib import md5 +from hashlib import sha256 from contextlib import contextmanager from random import Random import pathlib @@ -11,7 +11,7 @@ import unittest.mock import tarfile from test import support -from test.support import script_helper +from test.support import script_helper, requires_hashdigest # Check for our compression modules. try: @@ -27,8 +27,8 @@ try: except ImportError: lzma = None -def md5sum(data): - return md5(data).hexdigest() +def sha256sum(data): + return sha256(data).hexdigest() TEMPDIR = os.path.abspath(support.TESTFN) + "-tardir" tarextdir = TEMPDIR + '-extract-test' @@ -39,8 +39,12 @@ xzname = os.path.join(TEMPDIR, "testtar.tar.xz") tmpname = os.path.join(TEMPDIR, "tmp.tar") dotlessname = os.path.join(TEMPDIR, "testtar") -md5_regtype = "65f477c818ad9e15f7feab0c6d37742f" -md5_sparse = "a54fbc4ca4f4399a90e1b27164012fc6" +sha256_regtype = ( + "e09e4bc8b3c9d9177e77256353b36c159f5f040531bbd4b024a8f9b9196c71ce" +) +sha256_sparse = ( + "4f05a776071146756345ceee937b33fc5644f5a96b9780d1c7d6a32cdf164d7b" +) class TarTest: @@ -95,7 +99,7 @@ class UstarReadTest(ReadTest, unittest.TestCase): data = fobj.read() self.assertEqual(len(data), tarinfo.size, "regular file extraction failed") - self.assertEqual(md5sum(data), md5_regtype, + self.assertEqual(sha256sum(data), sha256_regtype, "regular file extraction failed") def test_fileobj_readlines(self): @@ -178,7 +182,7 @@ class UstarReadTest(ReadTest, unittest.TestCase): with self.tar.extractfile("ustar/regtype") as fobj: fobj = io.TextIOWrapper(fobj) data = fobj.read().encode("iso8859-1") - self.assertEqual(md5sum(data), md5_regtype) + self.assertEqual(sha256sum(data), sha256_regtype) try: fobj.seek(100) except AttributeError: @@ -543,13 +547,13 @@ class MiscReadTestBase(CommonReadTest): self.addCleanup(support.unlink, os.path.join(TEMPDIR, "ustar/lnktype")) with open(os.path.join(TEMPDIR, "ustar/lnktype"), "rb") as f: data = f.read() - self.assertEqual(md5sum(data), md5_regtype) + self.assertEqual(sha256sum(data), sha256_regtype) tar.extract("ustar/symtype", TEMPDIR) self.addCleanup(support.unlink, os.path.join(TEMPDIR, "ustar/symtype")) with open(os.path.join(TEMPDIR, "ustar/symtype"), "rb") as f: data = f.read() - self.assertEqual(md5sum(data), md5_regtype) + self.assertEqual(sha256sum(data), sha256_regtype) def test_extractall(self): # Test if extractall() correctly restores directory permissions @@ -684,7 +688,7 @@ class StreamReadTest(CommonReadTest, unittest.TestCase): data = fobj.read() self.assertEqual(len(data), tarinfo.size, "regular file extraction failed") - self.assertEqual(md5sum(data), md5_regtype, + self.assertEqual(sha256sum(data), sha256_regtype, "regular file extraction failed") def test_provoke_stream_error(self): @@ -796,8 +800,8 @@ class MemberReadTest(ReadTest, unittest.TestCase): def _test_member(self, tarinfo, chksum=None, **kwargs): if chksum is not None: with self.tar.extractfile(tarinfo) as f: - self.assertEqual(md5sum(f.read()), chksum, - "wrong md5sum for %s" % tarinfo.name) + self.assertEqual(sha256sum(f.read()), chksum, + "wrong sha256sum for %s" % tarinfo.name) kwargs["mtime"] = 0o7606136617 kwargs["uid"] = 1000 @@ -812,11 +816,11 @@ class MemberReadTest(ReadTest, unittest.TestCase): def test_find_regtype(self): tarinfo = self.tar.getmember("ustar/regtype") - self._test_member(tarinfo, size=7011, chksum=md5_regtype) + self._test_member(tarinfo, size=7011, chksum=sha256_regtype) def test_find_conttype(self): tarinfo = self.tar.getmember("ustar/conttype") - self._test_member(tarinfo, size=7011, chksum=md5_regtype) + self._test_member(tarinfo, size=7011, chksum=sha256_regtype) def test_find_dirtype(self): tarinfo = self.tar.getmember("ustar/dirtype") @@ -848,28 +852,28 @@ class MemberReadTest(ReadTest, unittest.TestCase): def test_find_sparse(self): tarinfo = self.tar.getmember("ustar/sparse") - self._test_member(tarinfo, size=86016, chksum=md5_sparse) + self._test_member(tarinfo, size=86016, chksum=sha256_sparse) def test_find_gnusparse(self): tarinfo = self.tar.getmember("gnu/sparse") - self._test_member(tarinfo, size=86016, chksum=md5_sparse) + self._test_member(tarinfo, size=86016, chksum=sha256_sparse) def test_find_gnusparse_00(self): tarinfo = self.tar.getmember("gnu/sparse-0.0") - self._test_member(tarinfo, size=86016, chksum=md5_sparse) + self._test_member(tarinfo, size=86016, chksum=sha256_sparse) def test_find_gnusparse_01(self): tarinfo = self.tar.getmember("gnu/sparse-0.1") - self._test_member(tarinfo, size=86016, chksum=md5_sparse) + self._test_member(tarinfo, size=86016, chksum=sha256_sparse) def test_find_gnusparse_10(self): tarinfo = self.tar.getmember("gnu/sparse-1.0") - self._test_member(tarinfo, size=86016, chksum=md5_sparse) + self._test_member(tarinfo, size=86016, chksum=sha256_sparse) def test_find_umlauts(self): tarinfo = self.tar.getmember("ustar/umlauts-" "\xc4\xd6\xdc\xe4\xf6\xfc\xdf") - self._test_member(tarinfo, size=7011, chksum=md5_regtype) + self._test_member(tarinfo, size=7011, chksum=sha256_regtype) def test_find_ustar_longname(self): name = "ustar/" + "12345/" * 39 + "1234567/longname" @@ -877,7 +881,7 @@ class MemberReadTest(ReadTest, unittest.TestCase): def test_find_regtype_oldv7(self): tarinfo = self.tar.getmember("misc/regtype-old-v7") - self._test_member(tarinfo, size=7011, chksum=md5_regtype) + self._test_member(tarinfo, size=7011, chksum=sha256_regtype) def test_find_pax_umlauts(self): self.tar.close() @@ -885,7 +889,7 @@ class MemberReadTest(ReadTest, unittest.TestCase): encoding="iso8859-1") tarinfo = self.tar.getmember("pax/umlauts-" "\xc4\xd6\xdc\xe4\xf6\xfc\xdf") - self._test_member(tarinfo, size=7011, chksum=md5_regtype) + self._test_member(tarinfo, size=7011, chksum=sha256_regtype) class LongnameTest: @@ -947,8 +951,8 @@ class GNUReadTest(LongnameTest, ReadTest, unittest.TestCase): filename = os.path.join(TEMPDIR, name) with open(filename, "rb") as fobj: data = fobj.read() - self.assertEqual(md5sum(data), md5_sparse, - "wrong md5sum for %s" % name) + self.assertEqual(sha256sum(data), sha256_sparse, + "wrong sha256sum for %s" % name) if self._fs_supports_holes(): s = os.stat(filename) @@ -2443,7 +2447,7 @@ class LinkEmulationTest(ReadTest, unittest.TestCase): self.tar.extract(name, TEMPDIR) with open(os.path.join(TEMPDIR, name), "rb") as f: data = f.read() - self.assertEqual(md5sum(data), md5_regtype) + self.assertEqual(sha256sum(data), sha256_regtype) # See issues #1578269, #8879, and #17689 for some history on these skips @unittest.skipIf(hasattr(os.path, "islink"), diff --git a/Lib/test/test_urllib2_localnet.py b/Lib/test/test_urllib2_localnet.py index 6b9b130fb6e..8cfb214c9af 100644 --- a/Lib/test/test_urllib2_localnet.py +++ b/Lib/test/test_urllib2_localnet.py @@ -322,6 +322,7 @@ class ProxyAuthTests(unittest.TestCase): PASSWD = "test123" REALM = "TestRealm" + @support.requires_hashdigest("md5") def setUp(self): super(ProxyAuthTests, self).setUp() # Ignore proxy bypass settings in the environment. diff --git a/Misc/NEWS.d/next/Tests/2019-09-25-12-18-31.bpo-38270._x-9uH.rst b/Misc/NEWS.d/next/Tests/2019-09-25-12-18-31.bpo-38270._x-9uH.rst new file mode 100644 index 00000000000..efb1b98b36d --- /dev/null +++ b/Misc/NEWS.d/next/Tests/2019-09-25-12-18-31.bpo-38270._x-9uH.rst @@ -0,0 +1,4 @@ +test.support now has a helper function to check for availibility of a +hash digest function. Several tests are refactored avoid MD5 and use +SHA256 instead. Other tests are marked to use MD5 and skipped when MD5 is +disabled.