From c7f768020298bf2fabb349930eb51e842d5f56a1 Mon Sep 17 00:00:00 2001 From: Christian Heimes Date: Mon, 25 May 2020 12:42:54 +0200 Subject: [PATCH] bpo-17258: Stronger HMAC in multiprocessing Signed-off-by: Christian Heimes --- Lib/multiprocessing/connection.py | 80 +++++++++++++++++-- Lib/test/_test_multiprocessing.py | 32 +++++++- .../2020-05-25-12-42-36.bpo-17258.lf2554.rst | 1 + 3 files changed, 103 insertions(+), 10 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2020-05-25-12-42-36.bpo-17258.lf2554.rst diff --git a/Lib/multiprocessing/connection.py b/Lib/multiprocessing/connection.py index 510e4b5aba4..1bc41f97ad6 100644 --- a/Lib/multiprocessing/connection.py +++ b/Lib/multiprocessing/connection.py @@ -11,6 +11,7 @@ __all__ = [ 'Client', 'Listener', 'Pipe', 'wait' ] import io import os +import re import sys import socket import struct @@ -734,30 +735,93 @@ CHALLENGE = b'#CHALLENGE#' WELCOME = b'#WELCOME#' FAILURE = b'#FAILURE#' -def deliver_challenge(connection, authkey): +_mac_algo_re = re.compile( + rb'^{(?P(md5|sha256|sha384|sha3_256|sha3_384))}' + rb'(?P.*)$' +) + +def _create_response(authkey, message): + """Create a MAC based on authkey and message + + The MAC algorithm defaults to HMAC-MD5, unless MD5 is not available or + the message has a '{digestmod}' prefix. For legacy HMAC-MD5, the response + is the raw MAC, otherwise the response is prefixed with '{digestmod}', + e.g. b'{sha256}abcdefg...' + + Note: The MAC protects the entire message including the digestmod prefix. + """ import hmac + # message: {digest}payload, the MAC protects header and payload + mo = _mac_algo_re.match(message) + if mo is not None: + digestmod = mo.group('digestmod').decode('ascii') + else: + # old-style MD5 with fallback + digestmod = None + + if digestmod is None: + try: + return hmac.new(authkey, message, 'md5').digest() + except ValueError: + # MD5 is not available, fall back to SHA2-256 + digestmod = 'sha256' + prefix = b'{%s}' % digestmod.encode('ascii') + return prefix + hmac.new(authkey, message, digestmod).digest() + + +def _verify_challenge(authkey, message, response): + """Verify MAC challenge + + If our message did not include a digestmod prefix, the client is allowed + to select a stronger digestmod (HMAC-MD5 legacy to HMAC-SHA2-256). + + In case our message is prefixed, a client cannot downgrade to a weaker + algorithm, because the MAC is calculated over the entire message + including the '{digestmod}' prefix. + """ + import hmac + mo = _mac_algo_re.match(response) + if mo is not None: + # get digestmod from response. + digestmod = mo.group('digestmod').decode('ascii') + mac = mo.group('payload') + else: + digestmod = 'md5' + mac = response + try: + expected = hmac.new(authkey, message, digestmod).digest() + except ValueError: + raise AuthenticationError(f'unsupported digest {digestmod}') + if not hmac.compare_digest(expected, mac): + raise AuthenticationError('digest received was wrong') + return True + + +def deliver_challenge(connection, authkey, digestmod=None): if not isinstance(authkey, bytes): raise ValueError( "Authkey must be bytes, not {0!s}".format(type(authkey))) message = os.urandom(MESSAGE_LENGTH) + if digestmod is not None: + message = b'{%s}%s' % (digestmod.encode('ascii'), message) connection.send_bytes(CHALLENGE + message) - digest = hmac.new(authkey, message, 'md5').digest() response = connection.recv_bytes(256) # reject large message - if response == digest: - connection.send_bytes(WELCOME) - else: + try: + _verify_challenge(authkey, message, response) + except AuthenticationError: connection.send_bytes(FAILURE) - raise AuthenticationError('digest received was wrong') + raise + else: + connection.send_bytes(WELCOME) def answer_challenge(connection, authkey): - import hmac if not isinstance(authkey, bytes): raise ValueError( "Authkey must be bytes, not {0!s}".format(type(authkey))) message = connection.recv_bytes(256) # reject large message assert message[:len(CHALLENGE)] == CHALLENGE, 'message = %r' % message message = message[len(CHALLENGE):] - digest = hmac.new(authkey, message, 'md5').digest() + digest = _create_response(authkey, message) connection.send_bytes(digest) response = connection.recv_bytes(256) # reject large message if response != WELCOME: diff --git a/Lib/test/_test_multiprocessing.py b/Lib/test/_test_multiprocessing.py index fd3b4303f03..d5d7d5a0931 100644 --- a/Lib/test/_test_multiprocessing.py +++ b/Lib/test/_test_multiprocessing.py @@ -46,6 +46,7 @@ import multiprocessing.heap import multiprocessing.managers import multiprocessing.pool import multiprocessing.queues +from multiprocessing.connection import wait, AuthenticationError from multiprocessing import util @@ -118,8 +119,6 @@ HAVE_GETVALUE = not getattr(_multiprocessing, WIN32 = (sys.platform == "win32") -from multiprocessing.connection import wait - def wait_for_handle(handle, timeout): if timeout is not None and timeout < 0.0: timeout = None @@ -4494,6 +4493,35 @@ class OtherTest(unittest.TestCase): multiprocessing.connection.answer_challenge, _FakeConnection(), b'abc') + +@hashlib_helper.requires_hashdigest('md5') +@hashlib_helper.requires_hashdigest('sha256') +class ChallengeResponseTest(unittest.TestCase): + authkey = b'supadupasecretkey' + + def create_response(self, message): + return multiprocessing.connection._create_response( + self.authkey, message + ) + + def verify_challenge(self, message, response): + return multiprocessing.connection._verify_challenge( + self.authkey, message, response + ) + + def test_challengeresponse(self): + for algo in [None, "md5", "sha256"]: + msg = b'mymessage' + if algo is not None: + prefix = b'{%s}' % algo.encode("ascii") + else: + prefix = b'' + msg = prefix + msg + response = self.create_response(msg) + if not response.startswith(prefix): + self.fail(response) + self.verify_challenge(msg, response) + # # Test Manager.start()/Pool.__init__() initializer feature - see issue 5585 # diff --git a/Misc/NEWS.d/next/Library/2020-05-25-12-42-36.bpo-17258.lf2554.rst b/Misc/NEWS.d/next/Library/2020-05-25-12-42-36.bpo-17258.lf2554.rst new file mode 100644 index 00000000000..b06e4a3bd88 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2020-05-25-12-42-36.bpo-17258.lf2554.rst @@ -0,0 +1 @@ +:mod:`multiprocessing` supports stronger HMAC algorithms