Github - Require webhook_token
It's quite unsafe to run without webhook_token, and quite easy for us to
run our tests with a terribly predictable one. This will ensure that
nobody accidentally runs a Zuul vulnerable to MITM proxy attacks.
Per the link right under the doc we just changed, we also use
hmac.compare_digest to prevent timing analysis by malicious attackers
which would help them discover the secret.
Change-Id: Ie8aa83b81b8e4ef1bb755a664bf416a8663930fa
diff --git a/doc/source/admin/drivers/github.rst b/doc/source/admin/drivers/github.rst
index 5075f80..3c46be9 100644
--- a/doc/source/admin/drivers/github.rst
+++ b/doc/source/admin/drivers/github.rst
@@ -48,9 +48,8 @@
<https://help.github.com/articles/creating-an-access-token-for-command-line-use/>`_.
**webhook_token**
- Optional: Token for validating the webhook event payloads.
- If not specified, payloads are not validated. In the GitHub App Configuration
- page, this is called "Webhook secret".
+ Required token for validating the webhook event payloads. In the
+ GitHub App Configuration page, this is called "Webhook secret".
See `Securing your webhooks
<https://developer.github.com/webhooks/securing/>`_.
diff --git a/tests/base.py b/tests/base.py
index c08a49e..568e15f 100755
--- a/tests/base.py
+++ b/tests/base.py
@@ -918,7 +918,9 @@
port = self.webapp.server.socket.getsockname()[1]
name, data = event
payload = json.dumps(data).encode('utf8')
- headers = {'X-Github-Event': name}
+ secret = self.connection_config['webhook_token']
+ signature = githubconnection._sign_request(payload, secret)
+ headers = {'X-Github-Event': name, 'X-Hub-Signature': signature}
req = urllib.request.Request(
'http://localhost:%s/connection/%s/payload'
% (port, self.connection_name),
diff --git a/tests/fixtures/zuul-connections-gerrit-and-github.conf b/tests/fixtures/zuul-connections-gerrit-and-github.conf
index 64757d8..04f2cc2 100644
--- a/tests/fixtures/zuul-connections-gerrit-and-github.conf
+++ b/tests/fixtures/zuul-connections-gerrit-and-github.conf
@@ -21,6 +21,7 @@
[connection github]
driver=github
+webhook_token=00000000000000000000000000000000000000000
[connection outgoing_smtp]
driver=smtp
diff --git a/tests/fixtures/zuul-github-driver.conf b/tests/fixtures/zuul-github-driver.conf
index 3d61ab6..732c30a 100644
--- a/tests/fixtures/zuul-github-driver.conf
+++ b/tests/fixtures/zuul-github-driver.conf
@@ -15,6 +15,7 @@
[connection github]
driver=github
+webhook_token=0000000000000000000000000000000000000000
[connection github_ssh]
driver=github
diff --git a/tests/fixtures/zuul-push-reqs.conf b/tests/fixtures/zuul-push-reqs.conf
index 4faac13..cb699e0 100644
--- a/tests/fixtures/zuul-push-reqs.conf
+++ b/tests/fixtures/zuul-push-reqs.conf
@@ -15,6 +15,7 @@
[connection github]
driver=github
+webhook_token=00000000000000000000000000000000000000000
[connection gerrit]
driver=gerrit
diff --git a/zuul/driver/github/githubconnection.py b/zuul/driver/github/githubconnection.py
index c3cafba..ba063fb 100644
--- a/zuul/driver/github/githubconnection.py
+++ b/zuul/driver/github/githubconnection.py
@@ -40,6 +40,12 @@
PREVIEW_JSON_ACCEPT = 'application/vnd.github.machine-man-preview+json'
+def _sign_request(body, secret):
+ signature = 'sha1=' + hmac.new(
+ secret.encode('utf-8'), body, hashlib.sha1).hexdigest()
+ return signature
+
+
class UTC(datetime.tzinfo):
"""UTC"""
@@ -263,7 +269,7 @@
def _validate_signature(self, request):
secret = self.connection.connection_config.get('webhook_token', None)
if secret is None:
- return True
+ raise RuntimeError("webhook_token is required")
body = request.body
try:
@@ -272,13 +278,12 @@
raise webob.exc.HTTPUnauthorized(
'Please specify a X-Hub-Signature header with secret.')
- payload_signature = 'sha1=' + hmac.new(secret.encode('utf-8'),
- body,
- hashlib.sha1).hexdigest()
+ payload_signature = _sign_request(body, secret)
self.log.debug("Payload Signature: {0}".format(str(payload_signature)))
self.log.debug("Request Signature: {0}".format(str(request_signature)))
- if str(payload_signature) != str(request_signature):
+ if not hmac.compare_digest(
+ str(payload_signature), str(request_signature)):
raise webob.exc.HTTPUnauthorized(
'Request signature does not match calculated payload '
'signature. Check that secret is correct.')