From 69f5cac2d7b3fa1df68e123c59adc6bebbdb8373 Mon Sep 17 00:00:00 2001 From: Xavier Morel Date: Wed, 30 Aug 2023 11:43:13 +0200 Subject: [PATCH] [FIX] runbot_merge: support non-ascii secrets & sha256 signatures Per the Github webhook documentation: 1. sha1 signatures are deprecated, github recommends sha256 (though that's unlikely to be a concern anyway), and dummy-central supports both so it should be no issue. > If possible, we recommend that you use the x-hub-signature-256 > header for improved security. 2. Non-ascii secrets are supported and should be utf8-encoded to compute signatures... that's not actually documented as github docs only mention payload encoding but it seems to make sense anyway. Also improve the warning message by replacing the signature (which is useless) by the delivery id (which could allow introspecing the hook or something). --- runbot_merge/controllers/__init__.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/runbot_merge/controllers/__init__.py b/runbot_merge/controllers/__init__.py index 3e693824..5d42abe5 100644 --- a/runbot_merge/controllers/__init__.py +++ b/runbot_merge/controllers/__init__.py @@ -98,10 +98,10 @@ class MergebotController(Controller): ('name', '=', repo), ]).project_id.secret if secret: - signature = 'sha1=' + hmac.new(secret.encode('ascii'), req.get_data(), hashlib.sha1).hexdigest() - if not hmac.compare_digest(signature, req.headers.get('X-Hub-Signature', '')): - _logger.warning("Ignored hook with incorrect signature %s", - req.headers.get('X-Hub-Signature')) + signature = 'sha256=' + hmac.new(secret.encode(), req.get_data(), hashlib.sha256).hexdigest() + if not hmac.compare_digest(signature, req.headers.get('X-Hub-Signature-256', '')): + _logger.warning("Ignored hook %s with incorrect signature", + req.headers.get('X-Github-Delivery')) return werkzeug.exceptions.Forbidden() sentry_sdk.set_context('webhook', request.jsonrequest)