Skip to content

bpo-38270: Check for hash digest algorithms and avoid MD5 #16382

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 25, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 24 additions & 0 deletions Lib/test/support/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import functools
import gc
import glob
import hashlib
import importlib
import importlib.util
import locale
Expand Down Expand Up @@ -648,6 +649,29 @@ def wrapper(*args, **kw):
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"
Expand Down
60 changes: 43 additions & 17 deletions Lib/test/test_hmac.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import unittest.mock
import warnings

from test.support import requires_hashdigest


def ignore_warning(func):
@functools.wraps(func)
Expand All @@ -19,6 +21,7 @@ def wrapper(*args, **kwargs):

class TestVectorsTestCase(unittest.TestCase):

@requires_hashdigest('md5')
def test_md5_vectors(self):
# Test the HMAC module against test vectors from the RFC.

Expand Down Expand Up @@ -76,6 +79,7 @@ def md5test(key, data, digest):
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)
Expand Down Expand Up @@ -268,23 +272,28 @@ def hmactest(key, data, hexdigests):
'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)
Expand All @@ -308,77 +317,92 @@ def test_with_digestmod_no_default(self):
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()
h2 = h.copy()
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.")
Expand All @@ -387,9 +411,10 @@ def test_attributes(self):
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.")
Expand All @@ -398,9 +423,10 @@ def test_realcopy(self):
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(),
Expand Down
6 changes: 5 additions & 1 deletion Lib/test/test_imaplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,8 @@
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
Expand Down Expand Up @@ -370,6 +371,7 @@ def cmd_AUTHENTICATE(self, tag, args):
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'
Expand All @@ -387,6 +389,7 @@ def cmd_AUTHENTICATE(self, tag, args):
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'
Expand Down Expand Up @@ -797,6 +800,7 @@ def cmd_AUTHENTICATE(self, tag, args):
b'ZmFrZQ==\r\n') # b64 encoded 'fake'

@reap_threads
@requires_hashdigest('md5')
def test_login_cram_md5(self):

class AuthHandler(SimpleIMAPHandler):
Expand Down
2 changes: 2 additions & 0 deletions Lib/test/test_poplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,11 @@ def test_noop(self):
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.
Expand Down
11 changes: 10 additions & 1 deletion Lib/test/test_smtplib.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
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
Expand All @@ -21,6 +22,7 @@
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


Expand Down Expand Up @@ -1009,6 +1011,7 @@ def testAUTH_LOGIN(self):
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)
Expand All @@ -1025,7 +1028,13 @@ def testAUTH_multiple(self):
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:
Expand Down
Loading