From 0acda12e6c59feba24036da40a051f3a47e23e5b Mon Sep 17 00:00:00 2001 From: Joe Chen Date: Fri, 7 Oct 2022 10:48:38 +0800 Subject: [PATCH] Support validating webhook signature (#12) --- README.md | 2 +- conf/app.ini | 2 ++ github.go | 20 +++++++++++++++++++ github_test.go | 46 +++++++++++++++++++++++++++++++++++++++++++ internal/conf/conf.go | 9 +++++---- main.go | 17 +++++++++++++++- 6 files changed, 90 insertions(+), 6 deletions(-) create mode 100644 github_test.go diff --git a/README.md b/README.md index a19382d..4dfda94 100644 --- 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 index f04ea07..0934d14 100644 --- 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 index d5f49b1..d995817 100644 --- 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 new file mode 100644 index 0000000..133077b --- /dev/null +++ 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 index 9788f62..40451d9 100644 --- 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 index 1f89a6f..609565f 100644 --- 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) }