[FIX] Reject malformed or unexpected webhook signatures instead of raising

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}')

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)

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)

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=<hex digest>``
     :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'))