[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).
This commit is contained in:
Xavier Morel 2023-08-30 11:43:13 +02:00
parent 302fd42cae
commit 69f5cac2d7

View File

@ -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)