Support validating webhook signature (#12)

codenotify · Oct 7, 2022 · 0acda12 · 0acda12
1 parent 2b44dc6
commit 0acda12
Show file tree

Hide file tree

Showing 6 changed files with 90 additions and 6 deletions.
diff --git a/README.md b/README.md
@@ -38,7 +38,7 @@ $ mkdir -p custom/conf
 $ touch custom/conf/app.ini
 ```
 
-Please refer to [Local development > Step 2: Create a test GitHub App](#step-2-create-a-test-github-app) for creating a GitHub App, setting up a reverse proxy and filling out necessary configuration options.
+Please refer to [Local development > Step 2: Create a test GitHub App](#step-2-create-a-test-github-app) for creating a GitHub App, setting up a reverse proxy and filling out necessary configuration options. View [`conf/app.ini`](conf/app.ini) for all available configuration options.
 
 > **Note**
 > The [Caddy web server](https://caddyserver.com/) is recommended for production use with automatic HTTPS.

diff --git a/conf/app.ini b/conf/app.ini
@@ -15,6 +15,8 @@ CLIENT_ID =
 CLIENT_SECRET =
 ; The "Private key" of the GitHub App.
 PRIVATE_KEY =
+; The "Webhook secret" of the GitHub App.
+WEBHOOK_SECRET =
 
 ; Configuration of the Codenotify.
 [codenotify]

diff --git a/github.go b/github.go
@@ -7,7 +7,11 @@ package main
 import (
 	"bytes"
 	"context"
+	"crypto/hmac"
 	"crypto/rand"
+	"crypto/sha256"
+	"crypto/subtle"
+	"encoding/hex"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -26,6 +30,22 @@ import (
 	"github.com/codenotify/codenotify.run/internal/conf"
 )
 
+// validateGitHubWebhookSignature256 returns true if the signature matches the
+// HMAC hex digested SHA256 hash of the body using the given key.
+func validateGitHubWebhookSignature256(signature, key string, body []byte) (bool, error) {
+	signature = strings.TrimPrefix(signature, "sha256=")
+	m := hmac.New(sha256.New, []byte(key))
+	if _, err := m.Write(body); err != nil {
+		return false, err
+	}
+	got := hex.EncodeToString(m.Sum(nil))
+
+	// NOTE: Use constant time string comparison helps mitigate certain timing
+	// attacks against regular equality operators, see
+	// https://docs.github.com/en/developers/webhooks-and-events/webhooks/securing-your-webhooks#validating-payloads-from-github
+	return subtle.ConstantTimeCompare([]byte(signature), []byte(got)) == 1, nil
+}
+
 func newGitHubClient(ctx context.Context, appID, installationID int64, privateKey string) (*github.Client, string, error) {
 	tr, err := ghinstallation.NewAppsTransport(http.DefaultTransport, appID, []byte(privateKey))
 	if err != nil {

diff --git a/github_test.go b/github_test.go
@@ -0,0 +1,46 @@
+// Copyright 2022 Unknwon. All rights reserved.
+// Use of this source code is governed by a MIT-style
+// license that can be found in the LICENSE file.
+
+package main
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestValidateGitHubWebhookSignature256(t *testing.T) {
+	//nolint:misspell
+	const payload = `{"ref":"refs/heads/main","before":"07da1e122bdbb81da8499b6d82c6b6302581a5a7","after":"5a96148ac5ef11a53b838b8cc0d9c929420657f3","repository":{"id":470746482,"node_id":"R_kgDOHA8Fcg","name":"bytebase-test","full_name":"unknwon/bytebase-test","private":true,"owner":{"name":"unknwon","email":"jc@unknwon.io","login":"unknwon","id":2946214,"node_id":"MDQ6VXNlcjI5NDYyMTQ=","avatar_url":"https://avatars.githubusercontent.com/u/2946214?v=4","gravatar_id":"","url":"https://api.github.com/users/unknwon","html_url":"https://github.com/unknwon","followers_url":"https://api.github.com/users/unknwon/followers","following_url":"https://api.github.com/users/unknwon/following{/other_user}","gists_url":"https://api.github.com/users/unknwon/gists{/gist_id}","starred_url":"https://api.github.com/users/unknwon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/unknwon/subscriptions","organizations_url":"https://api.github.com/users/unknwon/orgs","repos_url":"https://api.github.com/users/unknwon/repos","events_url":"https://api.github.com/users/unknwon/events{/privacy}","received_events_url":"https://api.github.com/users/unknwon/received_events","type":"User","site_admin":false},"html_url":"https://github.com/unknwon/bytebase-test","description":null,"fork":false,"url":"https://github.com/unknwon/bytebase-test","forks_url":"https://api.github.com/repos/unknwon/bytebase-test/forks","keys_url":"https://api.github.com/repos/unknwon/bytebase-test/keys{/key_id}","collaborators_url":"https://api.github.com/repos/unknwon/bytebase-test/collaborators{/collaborator}","teams_url":"https://api.github.com/repos/unknwon/bytebase-test/teams","hooks_url":"https://api.github.com/repos/unknwon/bytebase-test/hooks","issue_events_url":"https://api.github.com/repos/unknwon/bytebase-test/issues/events{/number}","events_url":"https://api.github.com/repos/unknwon/bytebase-test/events","assignees_url":"https://api.github.com/repos/unknwon/bytebase-test/assignees{/user}","branches_url":"https://api.github.com/repos/unknwon/bytebase-test/branches{/branch}","tags_url":"https://api.github.com/repos/unknwon/bytebase-test/tags","blobs_url":"https://api.github.com/repos/unknwon/bytebase-test/git/blobs{/sha}","git_tags_url":"https://api.github.com/repos/unknwon/bytebase-test/git/tags{/sha}","git_refs_url":"https://api.github.com/repos/unknwon/bytebase-test/git/refs{/sha}","trees_url":"https://api.github.com/repos/unknwon/bytebase-test/git/trees{/sha}","statuses_url":"https://api.github.com/repos/unknwon/bytebase-test/statuses/{sha}","languages_url":"https://api.github.com/repos/unknwon/bytebase-test/languages","stargazers_url":"https://api.github.com/repos/unknwon/bytebase-test/stargazers","contributors_url":"https://api.github.com/repos/unknwon/bytebase-test/contributors","subscribers_url":"https://api.github.com/repos/unknwon/bytebase-test/subscribers","subscription_url":"https://api.github.com/repos/unknwon/bytebase-test/subscription","commits_url":"https://api.github.com/repos/unknwon/bytebase-test/commits{/sha}","git_commits_url":"https://api.github.com/repos/unknwon/bytebase-test/git/commits{/sha}","comments_url":"https://api.github.com/repos/unknwon/bytebase-test/comments{/number}","issue_comment_url":"https://api.github.com/repos/unknwon/bytebase-test/issues/comments{/number}","contents_url":"https://api.github.com/repos/unknwon/bytebase-test/contents/{+path}","compare_url":"https://api.github.com/repos/unknwon/bytebase-test/compare/{base}...{head}","merges_url":"https://api.github.com/repos/unknwon/bytebase-test/merges","archive_url":"https://api.github.com/repos/unknwon/bytebase-test/{archive_format}{/ref}","downloads_url":"https://api.github.com/repos/unknwon/bytebase-test/downloads","issues_url":"https://api.github.com/repos/unknwon/bytebase-test/issues{/number}","pulls_url":"https://api.github.com/repos/unknwon/bytebase-test/pulls{/number}","milestones_url":"https://api.github.com/repos/unknwon/bytebase-test/milestones{/number}","notifications_url":"https://api.github.com/repos/unknwon/bytebase-test/notifications{?since,all,participating}","labels_url":"https://api.github.com/repos/unknwon/bytebase-test/labels{/name}","releases_url":"https://api.github.com/repos/unknwon/bytebase-test/releases{/id}","deployments_url":"https://api.github.com/repos/unknwon/bytebase-test/deployments","created_at":1647463607,"updated_at":"2022-03-16T20:46:47Z","pushed_at":1658671596,"git_url":"git://github.com/unknwon/bytebase-test.git","ssh_url":"git@github.com:unknwon/bytebase-test.git","clone_url":"https://github.com/unknwon/bytebase-test.git","svn_url":"https://github.com/unknwon/bytebase-test","homepage":null,"size":20,"stargazers_count":0,"watchers_count":0,"language":null,"has_issues":true,"has_projects":true,"has_downloads":true,"has_wiki":true,"has_pages":false,"forks_count":0,"mirror_url":null,"archived":false,"disabled":false,"open_issues_count":0,"license":{"key":"apache-2.0","name":"Apache License 2.0","spdx_id":"Apache-2.0","url":"https://api.github.com/licenses/apache-2.0","node_id":"MDc6TGljZW5zZTI="},"allow_forking":true,"is_template":false,"web_commit_signoff_required":false,"topics":[],"visibility":"private","forks":0,"open_issues":0,"watchers":0,"default_branch":"main","stargazers":0,"master_branch":"main"},"pusher":{"name":"unknwon","email":"jc@unknwon.io"},"sender":{"login":"unknwon","id":2946214,"node_id":"MDQ6VXNlcjI5NDYyMTQ=","avatar_url":"https://avatars.githubusercontent.com/u/2946214?v=4","gravatar_id":"","url":"https://api.github.com/users/unknwon","html_url":"https://github.com/unknwon","followers_url":"https://api.github.com/users/unknwon/followers","following_url":"https://api.github.com/users/unknwon/following{/other_user}","gists_url":"https://api.github.com/users/unknwon/gists{/gist_id}","starred_url":"https://api.github.com/users/unknwon/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/unknwon/subscriptions","organizations_url":"https://api.github.com/users/unknwon/orgs","repos_url":"https://api.github.com/users/unknwon/repos","events_url":"https://api.github.com/users/unknwon/events{/privacy}","received_events_url":"https://api.github.com/users/unknwon/received_events","type":"User","site_admin":false},"created":false,"deleted":false,"forced":false,"base_ref":null,"compare":"https://github.com/unknwon/bytebase-test/compare/07da1e122bdb...5a96148ac5ef","commits":[{"id":"5a96148ac5ef11a53b838b8cc0d9c929420657f3","tree_id":"8a842b23b62886d2ee12c152eda741cf39b1ceef","distinct":true,"message":"Create testdb_dev__202101131000__baseline__create_tablefoo_for_bar.sql","timestamp":"2022-07-24T22:06:36+08:00","url":"https://github.com/unknwon/bytebase-test/commit/5a96148ac5ef11a53b838b8cc0d9c929420657f3","author":{"name":"Joe Chen","email":"jc@unknwon.io","username":"unknwon"},"committer":{"name":"GitHub","email":"noreply@github.com","username":"web-flow"},"added":["Dev/testdb_dev__202101131000__baseline__create_tablefoo_for_bar.sql"],"removed":[],"modified":[]}],"head_commit":{"id":"5a96148ac5ef11a53b838b8cc0d9c929420657f3","tree_id":"8a842b23b62886d2ee12c152eda741cf39b1ceef","distinct":true,"message":"Create testdb_dev__202101131000__baseline__create_tablefoo_for_bar.sql","timestamp":"2022-07-24T22:06:36+08:00","url":"https://github.com/unknwon/bytebase-test/commit/5a96148ac5ef11a53b838b8cc0d9c929420657f3","author":{"name":"Joe Chen","email":"jc@unknwon.io","username":"unknwon"},"committer":{"name":"GitHub","email":"noreply@github.com","username":"web-flow"},"added":["Dev/testdb_dev__202101131000__baseline__create_tablefoo_for_bar.sql"],"removed":[],"modified":[]}}`
+
+	t.Run("wrong key", func(t *testing.T) {
+		got, err := validateGitHubWebhookSignature256(
+			"sha256=6bf313c917fd04a3c6c85270bab6c2a6ae40b7ab37767107bf80ad5c6a0a0deb",
+			"abadkey",
+			[]byte(payload),
+		)
+		assert.False(t, got)
+		assert.NoError(t, err)
+	})
+
+	t.Run("wrong signature", func(t *testing.T) {
+		got, err := validateGitHubWebhookSignature256(
+			"sha256=8335bc69262e94b20753316d844e155ae4d7826a6c89f12e98083ed0dce8d057",
+			"bZovosSKsJ8QKCG9",
+			[]byte(payload),
+		)
+		assert.False(t, got)
+		assert.NoError(t, err)
+	})
+
+	t.Run("success", func(t *testing.T) {
+		got, err := validateGitHubWebhookSignature256(
+			"sha256=6bf313c917fd04a3c6c85270bab6c2a6ae40b7ab37767107bf80ad5c6a0a0deb",
+			"bZovosSKsJ8QKCG9",
+			[]byte(payload),
+		)
+		assert.True(t, got)
+		assert.NoError(t, err)
+	})
+}
diff --git a/internal/conf/conf.go b/internal/conf/conf.go
@@ -30,10 +30,11 @@ type Config struct {
 	}
 	// GitHubApp contains the GitHub App configuration.
 	GitHubApp struct {
-		AppID        int64  `ini:"APP_ID"`
-		ClientID     string `ini:"CLIENT_ID"`
-		ClientSecret string
-		PrivateKey   string
+		AppID         int64  `ini:"APP_ID"`
+		ClientID      string `ini:"CLIENT_ID"`
+		ClientSecret  string
+		PrivateKey    string
+		WebhookSecret string
 	}
 	// Codenotify contains the Codenotify configuration.
 	Codenotify struct {

diff --git a/main.go b/main.go
@@ -8,6 +8,7 @@ import (
 	"context"
 	"encoding/json"
 	"fmt"
+	"io"
 	"net/http"
 	"os"
 
@@ -55,8 +56,22 @@ func main() {
 			return http.StatusOK, fmt.Sprintf("Event %q has been received but nothing to do", event)
 		}
 
+		body, err := io.ReadAll(r.Body)
+		if err != nil {
+			return http.StatusInternalServerError, fmt.Sprintf("Failed to read request body: %v", err)
+		}
+
+		if config.GitHubApp.WebhookSecret != "" {
+			ok, err := validateGitHubWebhookSignature256(r.Header.Get("X-Hub-Signature-256"), config.GitHubApp.WebhookSecret, body)
+			if err != nil {
+				return http.StatusInternalServerError, fmt.Sprintf("Failed to validate signature: %v", err)
+			} else if !ok {
+				return http.StatusBadRequest, `Mismatched payload signature for "X-Hub-Signature-256"`
+			}
+		}
+
 		var payload github.PullRequestEvent
-		err = json.NewDecoder(r.Body).Decode(&payload)
+		err = json.Unmarshal(body, &payload)
 		if err != nil {
 			return http.StatusBadRequest, fmt.Sprintf("Failed to decode payload: %v", err)
 		}