style: more informative comments for webhook verification responses

Co-Authored-By: dgw <dgw@technobabbl.es>

secret_stress-test.py

@@ -0,0 +1,145 @@
+#!/usr/bin/env python
+
+from __future__ import print_function
+
+import sys
+
+import hashlib
+import hmac
+from multiprocessing import Pool
+import random
+import requests
+
+
+# Helper constants
+class YAY_COLORS:
+    GREEN = '\033[92m'
+    RED = '\033[91m'
+    RESET = '\033[0m'
+
+
+VALID_REQUEST = '.'
+INVALID_REQUEST = '*'
+
+CORRECT_RESPONSE = True
+INCORRECT_RESPONSE = False
+
+# Test constants
+TEST_URL = "http://localhost:3333/webhook"  # Use this for local testing
+# TEST_URL = "http://bot.humorbaby.net:3333/webhook"  # Use this for more realistic testing; replace humorbaby.net as needed.
+
+REAL_WEBHOOK_SECRET = "asdf"  # Use to make valid request
+FAKE_WEBHOOK_SECRET = "asdff"  # Use to make invalid request
+
+PAYLOAD = """{"event": "testing", "repository": {"full_name": "testing/123"}}"""
+"""Fake payload won't trigger any actual bot messages, but will still be recieved and processed as normal."""
+
+# Get correct signature
+hash_ = hmac.new(REAL_WEBHOOK_SECRET, msg=PAYLOAD, digestmod=hashlib.sha1)
+CORRECT_SIGNATURE = 'sha1=' + hash_.hexdigest()
+
+# Make incorrect signature; yes, a random string could work, but let's be consistent, ey?
+hash_ = hmac.new(FAKE_WEBHOOK_SECRET, msg=PAYLOAD, digestmod=hashlib.sha1)
+INCORRECT_SIGNATURE = 'sha1=' + hash_.hexdigest()
+
+
+def send_valid_request():
+    headers_ = {
+        'X-Hub-Signature': CORRECT_SIGNATURE,
+        'content-type': 'application/json',
+    }
+
+    r = requests.post(TEST_URL, data=PAYLOAD, headers=headers_)
+    return (
+        VALID_REQUEST,  # Request type
+        True if r.status_code == 200 else False,  # Success status
+        (YAY_COLORS.GREEN if r.status_code == 200 else YAY_COLORS.RED) + VALID_REQUEST + YAY_COLORS.RESET,
+    )
+
+
+def send_invalid_request():
+    headers_ = {
+        'X-Hub-Signature': INCORRECT_SIGNATURE,
+        'content-type': 'application/json',
+    }
+
+    r = requests.post(TEST_URL, data=PAYLOAD, headers=headers_)
+    return (
+        INVALID_REQUEST,  # Request type
+        False if r.status_code == 200 else True,  # Success status
+        (YAY_COLORS.RED if r.status_code == 200 else YAY_COLORS.GREEN) + INVALID_REQUEST + YAY_COLORS.RESET
+    )
+
+
+def send_random_request(random_seed):
+    random.seed(random_seed)
+    request_ = random.choice([send_valid_request, send_invalid_request])
+    return request_()
+
+
+class Progress:
+    def __init__(self):
+        self.valid_requests = {
+            CORRECT_RESPONSE: 0,
+            INCORRECT_RESPONSE: 0,
+        }
+        self.invalid_requests = self.valid_requests.copy()
+
+    def callback_(self, result):
+        (request_type, status, msg) = result
+        {
+            VALID_REQUEST: self.valid_requests,
+            INVALID_REQUEST: self.invalid_requests,
+        }[request_type][status] += 1
+
+        print(msg, end='')
+        sys.stdout.flush()
+
+
+# Main
+NUM_PROCS = 32
+NUM_REQUESTS = 1000
+
+RANDOM_SEED = 12345  # For reproducability of # vaild/invalid reqeusts
+random.seed(RANDOM_SEED)
+
+pool = Pool(NUM_PROCS)
+progress = Progress()
+
+results_ = []
+for i in range(NUM_REQUESTS):
+    proc_random_seed = random.randint(1, 1e10 - 1)
+    results_.append(
+        pool.apply_async(
+            send_random_request,
+            (proc_random_seed, ),
+            callback=progress.callback_
+        )
+    )
+
+try:
+    [x.get() for x in results_]
+    pool.close()
+except Exception:
+    pool.terminate()
+finally:
+    pool.join()
+
+summary = """
+Valid Requests    --> {} sent
+{:>5d} correct response(s)
+{:>5d} incorrect response(s)
+
+Invalid requests  --> {} sent
+{:>5d} correct response(s)
+{:>5d} incorrect response(s)
+""".format(
+    sum(progress.valid_requests.values()),
+    progress.valid_requests[CORRECT_RESPONSE],
+    progress.valid_requests[INCORRECT_RESPONSE],
+    sum(progress.invalid_requests.values()),
+    progress.invalid_requests[CORRECT_RESPONSE],
+    progress.invalid_requests[INCORRECT_RESPONSE],
+).strip()
+
+print('\n', summary, sep='')

sopel_modules/github/webhook.py

@@ -135,15 +135,21 @@ def verify_request():
         return bottle.abort(400, msg)  # 400 Bad Request; missing required header
 
     digest_name, payload_signature = request_headers.get('X-Hub-Signature').split('=')
-    # Currently, GitHub only uses 'SHA1'
+    # Currently, GitHub only uses 'sha1'; log a warning if a different digest is
+    # specified by the server. GitHub may have started using a new digest and
+    # this should be confirmed.
     if digest_name != 'sha1':
         LOGGER.warning('Unexpected signature digest: {}'.format(digest_name))
         debug_log_request(request_headers, request_body)
 
     try:
         digest_mod = getattr(hashlib, digest_name)
     except AttributeError:
-        # Specified digest is not available. Did GitHub start using new digests?
+        # The previous digest check does not require a 'sha1' digest, but simply
+        # warns when an unexpected digest is specified. The function will
+        # attempt to find the digest specified in the signature, but if it is
+        # not currently supported by Python's `hashlib`, an error will be logged
+        # and returned.
         msg = 'Unsupported signature digest: {}'.format(digest_name)
         LOGGER.error(msg)
         debug_log_request(request_headers, request_body)
@@ -156,7 +162,7 @@ def verify_request():
         msg = 'Request signature mismatch.'
         LOGGER.error(msg)
         debug_log_request(request_headers, request_body)
-        return abort_request(401, msg)  # 401 Unauthorized;...? invalid "authentication token"
+        return abort_request(401, msg)  # 403 Forbidden; server understood the request but refuses to authorize it
 
 
 @bottle.get("/webhook")