Webhook now verifies the new hmac signature instead of just comparing the secret as plain text.

This commit is contained in:
Marvin Pohl 2021-03-09 21:54:06 +01:00
parent 708fad884e
commit 0fd2394bcd
2 changed files with 35 additions and 5 deletions

View File

@ -8,7 +8,9 @@ from buildbot.test.util.misc import TestReactorMixin
from twisted.internet import defer from twisted.internet import defer
from twisted.trial import unittest from twisted.trial import unittest
from buildbot_gitea.webhook import GiteaHandler, _HEADER_EVENT_TYPE from buildbot_gitea.webhook import GiteaHandler, _HEADER_EVENT_TYPE, _HEADER_SIGNATURE
giteaJsonPushPayload_Signature = 'b5feb0994ad24c209188d36a30cecfea86666aa9c65a419b068f73f91152e7bc'
giteaJsonPushPayload = rb""" giteaJsonPushPayload = rb"""
{ {
@ -195,6 +197,8 @@ giteaInvalidSecretPush = rb"""
} }
""" """
giteaJsonPullRequestPayload_Signature = '8685905c03fa521dd1eacfb84405195dbca2a08206c3a978a3656399f5dbe01a'
giteaJsonPullRequestPayload = rb""" giteaJsonPullRequestPayload = rb"""
{ {
"secret": "test", "secret": "test",
@ -370,6 +374,9 @@ giteaJsonPullRequestPayload = rb"""
} }
""" """
giteaJsonPullRequestPayloadNotMergeable_Signature = '5552a0cbcbb3fe6286681bc7846754929be0d3f27ccc32914e5fd3ce01f34632'
giteaJsonPullRequestPayloadNotMergeable = rb""" giteaJsonPullRequestPayloadNotMergeable = rb"""
{ {
"secret": "test", "secret": "test",
@ -545,6 +552,7 @@ giteaJsonPullRequestPayloadNotMergeable = rb"""
} }
""" """
giteaJsonPullRequestPayloadMerged_Signature = '4d3b1045aea9aa5cce4f7270d549c11d212c55036d9c547d0c9327891d56bf97'
giteaJsonPullRequestPayloadMerged = rb""" giteaJsonPullRequestPayloadMerged = rb"""
{ {
"secret": "test", "secret": "test",
@ -801,6 +809,7 @@ class TestChangeHookGiteaPush(unittest.TestCase, TestReactorMixin):
self.request.uri = b'/change_hook/gitea' self.request.uri = b'/change_hook/gitea'
self.request.method = b'POST' self.request.method = b'POST'
self.request.received_headers[_HEADER_EVENT_TYPE] = b"push" self.request.received_headers[_HEADER_EVENT_TYPE] = b"push"
self.request.received_headers[_HEADER_SIGNATURE] = giteaJsonPushPayload_Signature
res = yield self.request.test_render(self.changeHook) res = yield self.request.test_render(self.changeHook)
self.checkChangesFromPush(res) self.checkChangesFromPush(res)
@ -810,6 +819,7 @@ class TestChangeHookGiteaPush(unittest.TestCase, TestReactorMixin):
self.request.uri = b'/change_hook/gitea' self.request.uri = b'/change_hook/gitea'
self.request.method = b'POST' self.request.method = b'POST'
self.request.received_headers[_HEADER_EVENT_TYPE] = b"pull_request" self.request.received_headers[_HEADER_EVENT_TYPE] = b"pull_request"
self.request.received_headers[_HEADER_SIGNATURE] = giteaJsonPullRequestPayload_Signature
res = yield self.request.test_render(self.changeHook) res = yield self.request.test_render(self.changeHook)
self.checkChangesFromPullRequest(res) self.checkChangesFromPullRequest(res)
@ -819,6 +829,7 @@ class TestChangeHookGiteaPush(unittest.TestCase, TestReactorMixin):
self.request.uri = b'/change_hook/gitea' self.request.uri = b'/change_hook/gitea'
self.request.method = b'POST' self.request.method = b'POST'
self.request.received_headers[_HEADER_EVENT_TYPE] = b"pull_request" self.request.received_headers[_HEADER_EVENT_TYPE] = b"pull_request"
self.request.received_headers[_HEADER_SIGNATURE] = giteaJsonPullRequestPayloadNotMergeable_Signature
yield self.request.test_render(self.changeHook) yield self.request.test_render(self.changeHook)
self.assertEqual(len(self.changeHook.master.data.updates.changesAdded), 0) self.assertEqual(len(self.changeHook.master.data.updates.changesAdded), 0)
@ -828,6 +839,7 @@ class TestChangeHookGiteaPush(unittest.TestCase, TestReactorMixin):
self.request.uri = b'/change_hook/gitea' self.request.uri = b'/change_hook/gitea'
self.request.method = b'POST' self.request.method = b'POST'
self.request.received_headers[_HEADER_EVENT_TYPE] = b"pull_request" self.request.received_headers[_HEADER_EVENT_TYPE] = b"pull_request"
self.request.received_headers[_HEADER_SIGNATURE] = giteaJsonPullRequestPayloadMerged_Signature
yield self.request.test_render(self.changeHook) yield self.request.test_render(self.changeHook)
self.assertEqual(len(self.changeHook.master.data.updates.changesAdded), 0) self.assertEqual(len(self.changeHook.master.data.updates.changesAdded), 0)
@ -864,6 +876,7 @@ class TestChangeHookGiteaPushOnlySingle(unittest.TestCase, TestReactorMixin):
self.request.uri = b'/change_hook/gitea' self.request.uri = b'/change_hook/gitea'
self.request.method = b'POST' self.request.method = b'POST'
self.request.received_headers[_HEADER_EVENT_TYPE] = b"push" self.request.received_headers[_HEADER_EVENT_TYPE] = b"push"
self.request.received_headers[_HEADER_SIGNATURE] = giteaJsonPushPayload_Signature
res = yield self.request.test_render(self.changeHook) res = yield self.request.test_render(self.changeHook)
self.checkChangesFromPush(res) self.checkChangesFromPush(res)
@ -881,6 +894,7 @@ class TestChangeHookGiteaSecretPhrase(unittest.TestCase, TestReactorMixin):
self.request.uri = b'/change_hook/gitea' self.request.uri = b'/change_hook/gitea'
self.request.method = b'POST' self.request.method = b'POST'
self.request.received_headers[_HEADER_EVENT_TYPE] = b"push" self.request.received_headers[_HEADER_EVENT_TYPE] = b"push"
self.request.received_headers[_HEADER_SIGNATURE] = giteaJsonPushPayload_Signature
yield self.request.test_render(self.changeHook) yield self.request.test_render(self.changeHook)
self.assertEqual(len(self.changeHook.master.data.updates.changesAdded), 2) self.assertEqual(len(self.changeHook.master.data.updates.changesAdded), 2)
@ -890,6 +904,7 @@ class TestChangeHookGiteaSecretPhrase(unittest.TestCase, TestReactorMixin):
self.request.uri = b'/change_hook/gitea' self.request.uri = b'/change_hook/gitea'
self.request.method = b'POST' self.request.method = b'POST'
self.request.received_headers[_HEADER_EVENT_TYPE] = b"push" self.request.received_headers[_HEADER_EVENT_TYPE] = b"push"
self.request.received_headers[_HEADER_SIGNATURE] = giteaJsonPushPayload_Signature
yield self.request.test_render(self.changeHook) yield self.request.test_render(self.changeHook)
self.assertEqual(len(self.changeHook.master.data.updates.changesAdded), 0) self.assertEqual(len(self.changeHook.master.data.updates.changesAdded), 0)
@ -922,6 +937,7 @@ class TestChangeHookGiteaClass(unittest.TestCase, TestReactorMixin):
self.request.uri = b'/change_hook/gitea' self.request.uri = b'/change_hook/gitea'
self.request.method = b'POST' self.request.method = b'POST'
self.request.received_headers[_HEADER_EVENT_TYPE] = b'push' self.request.received_headers[_HEADER_EVENT_TYPE] = b'push'
self.request.received_headers[_HEADER_SIGNATURE] = giteaJsonPushPayload_Signature
yield self.request.test_render(self.changeHook) yield self.request.test_render(self.changeHook)
self.checkChanges() self.checkChanges()
@ -932,6 +948,7 @@ class TestChangeHookGiteaClass(unittest.TestCase, TestReactorMixin):
self.request.uri = b'/change_hook/gitea' self.request.uri = b'/change_hook/gitea'
self.request.method = b'POST' self.request.method = b'POST'
self.request.received_headers[_HEADER_EVENT_TYPE] = b'release' self.request.received_headers[_HEADER_EVENT_TYPE] = b'release'
self.request.received_headers[_HEADER_SIGNATURE] = giteaJsonPushPayload_Signature
yield self.request.test_render(self.changeHook) yield self.request.test_render(self.changeHook)
self.checkChanges() self.checkChanges()

View File

@ -1,5 +1,7 @@
import json import json
import re import re
import hmac
import hashlib
from buildbot.util import bytes2unicode from buildbot.util import bytes2unicode
from buildbot.www.hooks.base import BaseHookHandler from buildbot.www.hooks.base import BaseHookHandler
@ -7,6 +9,7 @@ from twisted.python import log
from dateutil.parser import parse as dateparse from dateutil.parser import parse as dateparse
_HEADER_EVENT_TYPE = 'X-Gitea-Event' _HEADER_EVENT_TYPE = 'X-Gitea-Event'
_HEADER_SIGNATURE = 'HTTP_X_GITEA_SIGNATURE'
class GiteaHandler(BaseHookHandler): class GiteaHandler(BaseHookHandler):
@ -117,11 +120,21 @@ class GiteaHandler(BaseHookHandler):
secret = self.options.get('secret') secret = self.options.get('secret')
try: try:
content = request.content.read() content = request.content.read()
payload = json.loads(bytes2unicode(content)) content_text = bytes2unicode(content)
payload = json.loads(content_text)
except Exception as exception: except Exception as exception:
raise ValueError('Error loading JSON: ' + str(exception)) raise ValueError('Error loading JSON: ' + str(exception))
if secret is not None and secret != payload['secret']:
raise ValueError('Invalid secret') if secret is not None:
signature = hmac.new(
secret.encode("UTF-8"),
content_text.strip().encode("UTF-8"),
digestmod=hashlib.sha256)
header_signature = bytes2unicode(
request.getHeader(_HEADER_SIGNATURE))
if signature.hexdigest() != header_signature:
raise ValueError('Invalid secret')
event_type = bytes2unicode(request.getHeader(_HEADER_EVENT_TYPE)) event_type = bytes2unicode(request.getHeader(_HEADER_EVENT_TYPE))
log.msg("Received event '{}' from gitea".format(event_type)) log.msg("Received event '{}' from gitea".format(event_type))