diff --git a/mod_ci/controllers.py b/mod_ci/controllers.py index a31765bb3..aaff5f4a4 100755 --- a/mod_ci/controllers.py +++ b/mod_ci/controllers.py @@ -2043,7 +2043,7 @@ def start_ci(): g.log.debug('server ping successful') return json.dumps({'msg': 'Hi!'}) - x_hub_signature = request.headers.get('X-Hub-Signature') + x_hub_signature = request.headers.get('X-Hub-Signature-256') if not is_valid_signature(x_hub_signature, request.data, g.github['ci_key']): g.log.warning(f'CI signature failed: {x_hub_signature}') diff --git a/tests/base.py b/tests/base.py index 3bbc9bb33..17bb139d4 100644 --- a/tests/base.py +++ b/tests/base.py @@ -240,9 +240,8 @@ def generate_signature(data, private_key): """ import hashlib import hmac - algorithm = hashlib.__dict__.get('sha1') encoded_key = bytes(private_key, 'latin-1') - mac = hmac.new(encoded_key, msg=data, digestmod=algorithm) + mac = hmac.new(encoded_key, msg=data, digestmod=hashlib.sha256) return mac.hexdigest() @@ -252,12 +251,12 @@ def generate_git_api_header(event, sig): :param event: Name of the event type that triggered the delivery. :param sig: The HMAC hex digest of the response body. The HMAC hex digest is generated - using the sha1 hash function and the secret as the HMAC key. + using the sha256 hash function and the secret as the HMAC key. """ return Headers([ ('X-GitHub-Event', event), ('X-GitHub-Delivery', "72d3162e-cc78-11e3-81ab-4c9367dc0958"), - ('X-Hub-Signature', f"sha1={sig}"), + ('X-Hub-Signature-256', f"sha256={sig}"), ('User-Agent', "GitHub-Hookshot/044aadd"), ('Content-Type', "application/json"), ('Content-Length', 6615) diff --git a/tests/test_utility.py b/tests/test_utility.py index db9d8fe80..568603083 100644 --- a/tests/test_utility.py +++ b/tests/test_utility.py @@ -48,3 +48,28 @@ def example_function(*args, **kwargs): mock_is_github_ip.assert_called_once() self.assertEqual(mock_abort.call_count, 7) self.assertEqual(response, "Test Success") + + def test_is_valid_signature_accepts_sha256(self): + """Test that a matching X-Hub-Signature-256 value is accepted.""" + import hashlib + import hmac + + from utility import is_valid_signature + + data = b'{"zen": "Keep it logically awesome."}' + digest = hmac.new(b'secret', msg=data, digestmod=hashlib.sha256).hexdigest() + self.assertTrue(is_valid_signature(f'sha256={digest}', data, 'secret')) + self.assertFalse(is_valid_signature(f'sha256={digest}', data, 'other-secret')) + + def test_is_valid_signature_rejects_malformed_headers(self): + """Test that malformed or unexpected signature headers are rejected instead of raising.""" + import hmac + + from utility import is_valid_signature + + data = b'{}' + md5_digest = hmac.new(b'secret', msg=data, digestmod='md5').hexdigest() + sha1_digest = hmac.new(b'secret', msg=data, digestmod='sha1').hexdigest() + for header in (None, '', 'sha256', 'new=abc', 'unknown=abc', 'sha256=\u00e9', + f'md5={md5_digest}', f'sha1={sha1_digest}'): + self.assertFalse(is_valid_signature(header, data, 'secret'), header) diff --git a/utility.py b/utility.py index ecc8df7e5..c4b842d8e 100644 --- a/utility.py +++ b/utility.py @@ -72,7 +72,7 @@ def decorated_function(*args, **kwargs): g.log.warning(f"Unauthorized attempt by IP {request_ip}") abort(abort_code) - for header in ['X-GitHub-Event', 'X-GitHub-Delivery', 'X-Hub-Signature', 'User-Agent']: + for header in ['X-GitHub-Event', 'X-GitHub-Delivery', 'X-Hub-Signature-256', 'User-Agent']: if header not in request.headers: g.log.critical(f"{header} not in headers!") abort(abort_code) @@ -153,16 +153,26 @@ def is_valid_signature(x_hub_signature, data, private_key): """ Re-check if the GitHub hook request got valid signature. - :param x_hub_signature: Signature to check + :param x_hub_signature: Value of the ``X-Hub-Signature-256`` header, + e.g. ``sha256=`` :type x_hub_signature: str :param data: Signature's data :type data: bytearray :param private_key: Signature's token :type private_key: str + :return: False if the signature is missing, malformed, not SHA-256, + or does not match. + :rtype: bool """ - hash_algorithm, github_signature = x_hub_signature.split('=', 1) - algorithm = hashlib.__dict__.get(hash_algorithm) + if not x_hub_signature: + return False + + hash_algorithm, separator, github_signature = x_hub_signature.partition('=') + if not separator or hash_algorithm != 'sha256': + return False + encoded_key = bytes(private_key, 'latin-1') - mac = hmac.new(encoded_key, msg=data, digestmod=algorithm) + mac = hmac.new(encoded_key, msg=data, digestmod=hashlib.sha256) - return hmac.compare_digest(mac.hexdigest(), github_signature) + # Compare bytes: compare_digest raises TypeError on non-ASCII str input. + return hmac.compare_digest(mac.hexdigest().encode(), github_signature.encode('utf-8', 'replace'))