Repository: google/go-github
Pull request #1783 (2021-01-26 15:51:33)

Add support for the X-Hub-Signature-256 header
This relates to #592 and adds support for the `X-Hub-Signature-256` header. I've chosen to set the SHA256 header as the default and falling back to the SHA1 header when the SHA256 header is empty. The GitHub documentation [recommends](https://docs.github.com/en/developers/webhooks-and-events/securing-your-webhooks#validating-payloads-from-github) using the SHA256 header so I don't expect the fallback to be needed in a live environment. Still, this fallback prevents introducing breaking changes when used in tests.

codecov[bot]: # [Codecov](https://codecov.io/gh/google/go-github/pull/1783?src=pr&el=h1) Report
> Merging [#1783](https://codecov.io/gh/google/go-github/pull/1783?src=pr&el=desc) (ab644e5) into [master](https://codecov.io/gh/google/go-github/commit/1094addc7307f45f46942e01e1804cc6121f2bac?el=desc) (1094add) will **increase** coverage by `0.00%`.
> The diff coverage is `100.00%`.

[![Impacted file tree graph](https://codecov.io/gh/google/go-github/pull/1783/graphs/tree.svg?width=650&height=150&src=pr&token=EgYjGjZlI0)](https://codecov.io/gh/google/go-github/pull/1783?src=pr&el=tree)

```diff
@@           Coverage Diff           @@
##           master    #1783   +/-   ##
=======================================
  Coverage   97.53%   97.54%           
=======================================
  Files          98       98           
  Lines        6378     6383    +5     
=======================================
+ Hits         6221     6226    +5     
  Misses         85       85           
  Partials       72       72           
```


| [Impacted Files](https://codecov.io/gh/google/go-github/pull/1783?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [github/messages.go](https://codecov.io/gh/google/go-github/pull/1783/diff?src=pr&el=tree#diff-Z2l0aHViL21lc3NhZ2VzLmdv) | `97.01% <100.00%> (+0.24%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/google/go-github/pull/1783?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/google/go-github/pull/1783?src=pr&el=footer). Last update [1094add...ab644e5](https://codecov.io/gh/google/go-github/pull/1783?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

willnorris: (and this looks great by the way... thanks for adding this!)
willnorris: /cc @gauntface and @taquitos as FYI

if I remember correctly, there is some code in Google's CLA checker that constructs a fake webhook payload to manually trigger rescanning a CLA.  If go-github were to ever drop support for these SHA-1 signatures (which there's probably no need to do, but it's not inconceivable), you would need to make sure to move over to SHA-256 for signing those fake webhook payloads.

Patch:
diff --git a/github/messages.go b/github/messages.go
index 81b4651d..428684d6 100644
--- a/github/messages.go
+++ b/github/messages.go
@@ -30,8 +30,10 @@ const (
 	// sha256Prefix and sha512Prefix are provided for future compatibility.
 	sha256Prefix = "sha256"
 	sha512Prefix = "sha512"
-	// signatureHeader is the GitHub header key used to pass the HMAC hexdigest.
-	signatureHeader = "X-Hub-Signature"
+	// sha1SignatureHeader is the GitHub header key used to pass the HMAC-SHA1 hexdigest.
+	sha1SignatureHeader = "X-Hub-Signature"
+	// sha256SignatureHeader is the GitHub header key used to pass the HMAC-SHA256 hexdigest.
+	sha256SignatureHeader = "X-Hub-Signature-256"
 	// eventTypeHeader is the GitHub header key used to pass the event type.
 	eventTypeHeader = "X-Github-Event"
 	// deliveryIDHeader is the GitHub header key used to pass the unique ID for the webhook event.
@@ -190,7 +192,10 @@ func ValidatePayload(r *http.Request, secretToken []byte) (payload []byte, err e
 	// Only validate the signature if a secret token exists. This is intended for
 	// local development only and all webhooks should ideally set up a secret token.
 	if len(secretToken) > 0 {
-		sig := r.Header.Get(signatureHeader)
+		sig := r.Header.Get(sha256SignatureHeader)
+		if sig == "" {
+			sig = r.Header.Get(sha1SignatureHeader)
+		}
 		if err := ValidateSignature(sig, body, secretToken); err != nil {
 			return nil, err
 		}


Test patch:
diff --git a/github/messages_test.go b/github/messages_test.go
index 78497285..8b31b9d0 100644
--- a/github/messages_test.go
+++ b/github/messages_test.go
@@ -29,12 +29,13 @@ func TestValidatePayload(t *testing.T) {
 	const defaultSignature = "sha1=126f2c800419c60137ce748d7672e77b65cf16d6"
 	secretKey := []byte("0123456789abcdef")
 	tests := []struct {
-		signature   string
-		eventID     string
-		event       string
-		wantEventID string
-		wantEvent   string
-		wantPayload string
+		signature       string
+		signatureHeader string
+		eventID         string
+		event           string
+		wantEventID     string
+		wantEvent       string
+		wantPayload     string
 	}{
 		// The following tests generate expected errors:
 		{},                         // Missing signature
@@ -62,6 +63,13 @@ func TestValidatePayload(t *testing.T) {
 			wantEvent:   "ping",
 			wantPayload: defaultBody,
 		},
+		{
+			signature:       "sha256=b1f8020f5b4cd42042f807dd939015c4a418bc1ff7f604dd55b0a19b5d953d9b",
+			signatureHeader: sha256SignatureHeader,
+			event:           "ping",
+			wantEvent:       "ping",
+			wantPayload:     defaultBody,
+		},
 		{
 			signature:   "sha512=8456767023c1195682e182a23b3f5d19150ecea598fde8cb85918f7281b16079471b1329f92b912c4d8bd7455cb159777db8f29608b20c7c87323ba65ae62e1f",
 			event:       "ping",
@@ -77,7 +85,11 @@ func TestValidatePayload(t *testing.T) {
 			t.Fatalf("NewRequest: %v", err)
 		}
 		if test.signature != "" {
-			req.Header.Set(signatureHeader, test.signature)
+			if test.signatureHeader != "" {
+				req.Header.Set(test.signatureHeader, test.signature)
+			} else {
+				req.Header.Set(sha1SignatureHeader, test.signature)
+			}
 		}
 		req.Header.Set("Content-Type", "application/json")
 
@@ -107,7 +119,7 @@ func TestValidatePayload_FormGet(t *testing.T) {
 	}
 	req.PostForm = form
 	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-	req.Header.Set(signatureHeader, signature)
+	req.Header.Set(sha1SignatureHeader, signature)
 
 	got, err := ValidatePayload(req, secretKey)
 	if err != nil {
@@ -118,7 +130,7 @@ func TestValidatePayload_FormGet(t *testing.T) {
 	}
 
 	// check that if payload is invalid we get error
-	req.Header.Set(signatureHeader, "invalid signature")
+	req.Header.Set(sha1SignatureHeader, "invalid signature")
 	if _, err = ValidatePayload(req, []byte{0}); err == nil {
 		t.Error("ValidatePayload = nil, want err")
 	}
@@ -137,7 +149,7 @@ func TestValidatePayload_FormPost(t *testing.T) {
 		t.Fatalf("NewRequest: %v", err)
 	}
 	req.Header.Set("Content-Type", "application/x-www-form-urlencoded")
-	req.Header.Set(signatureHeader, signature)
+	req.Header.Set(sha1SignatureHeader, signature)
 
 	got, err := ValidatePayload(req, secretKey)
 	if err != nil {
@@ -148,7 +160,7 @@ func TestValidatePayload_FormPost(t *testing.T) {
 	}
 
 	// check that if payload is invalid we get error
-	req.Header.Set(signatureHeader, "invalid signature")
+	req.Header.Set(sha1SignatureHeader, "invalid signature")
 	if _, err = ValidatePayload(req, []byte{0}); err == nil {
 		t.Error("ValidatePayload = nil, want err")
 	}