Merge pull request #461 from moorereason/iss289-hmac-cleanup

Transition payload hash option names to hmac
adnanh · Sep 28, 2020 · 0f4bbfa · 0f4bbfa
2 parents 6797bf7 + 6bbf14f
commit 0f4bbfa
Show file tree

Hide file tree

Showing 11 changed files with 47 additions and 28 deletions.
diff --git a/docs/Hook-Examples.md b/docs/Hook-Examples.md
@@ -46,7 +46,7 @@ This page is still work in progress. Feel free to contribute!
         {
           "match":
           {
-            "type": "payload-hash-sha1",
+            "type": "payload-hmac-sha1",
             "secret": "mysecret",
             "parameter":
             {
@@ -166,7 +166,7 @@ Values in the request body can be accessed in the command or to the match rule b
         {
           "match":
           {
-            "type": "payload-hash-sha256",
+            "type": "payload-hmac-sha256",
             "secret": "mysecret",
             "parameter":
             {

diff --git a/docs/Hook-Rules.md b/docs/Hook-Rules.md
@@ -9,9 +9,9 @@
 * [Match](#match)
   * [Match value](#match-value)
   * [Match regex](#match-regex)
-  * [Match payload-hash-sha1](#match-payload-hash-sha1)
-  * [Match payload-hash-sha256](#match-payload-hash-sha256)
-  * [Match payload-hash-sha512](#match-payload-hash-sha512)
+  * [Match payload-hmac-sha1](#match-payload-hmac-sha1)
+  * [Match payload-hmac-sha256](#match-payload-hmac-sha256)
+  * [Match payload-hmac-sha512](#match-payload-hmac-sha512)
   * [Match Whitelisted IP range](#match-whitelisted-ip-range)
   * [Match scalr-signature](#match-scalr-signature)
 
@@ -110,7 +110,7 @@
                 "source": "header",
                 "name": "X-Hub-Signature"
             },
-            "type": "payload-hash-sha1",
+            "type": "payload-hmac-sha1",
             "secret": "mysecret"
         }
     },
@@ -150,9 +150,7 @@
 
 *Please note:* Due to technical reasons, _number_ and _boolean_ values in the _match rule_ must be wrapped around with a pair of quotes.
 
-There are three different match rules:
-
-### 1. Match value
+### Match value
 ```json
 {
   "match":
@@ -168,7 +166,7 @@ There are three different match rules:
 }
 ```
 
-### 2. Match regex
+### Match regex
 For the regex syntax, check out <http://golang.org/pkg/regexp/syntax/>
 ```json
 {
@@ -185,12 +183,13 @@ For the regex syntax, check out <http://golang.org/pkg/regexp/syntax/>
 }
 ```
 
-### 3. Match payload-hash-sha1
+### Match payload-hmac-sha1
+Validate the HMAC of the payload using the SHA1 hash and the given *secret*.
 ```json
 {
   "match":
   {
-    "type": "payload-hash-sha1",
+    "type": "payload-hmac-sha1",
     "secret": "yoursecret",
     "parameter":
     {
@@ -208,12 +207,13 @@ will be tried unless a match is found. For example:
 X-Hub-Signature: sha1=the-first-signature,sha1=the-second-signature
 ```
 
-### 4. Match payload-hash-sha256
+### Match payload-hmac-sha256
+Validate the HMAC of the payload using the SHA256 hash and the given *secret*.
 ```json
 {
   "match":
   {
-    "type": "payload-hash-sha256",
+    "type": "payload-hmac-sha256",
     "secret": "yoursecret",
     "parameter":
     {
@@ -231,12 +231,13 @@ will be tried unless a match is found. For example:
 X-Hub-Signature: sha256=the-first-signature,sha256=the-second-signature
 ```
 
-### 5. Match payload-hash-sha512
+### Match payload-hmac-sha512
+Validate the HMAC of the payload using the SHA512 hash and the given *secret*.
 ```json
 {
   "match":
   {
-    "type": "payload-hash-sha512",
+    "type": "payload-hmac-sha512",
     "secret": "yoursecret",
     "parameter":
     {
@@ -254,7 +255,7 @@ will be tried unless a match is found. For example:
 X-Hub-Signature: sha512=the-first-signature,sha512=the-second-signature
 ```
 
-### 6. Match Whitelisted IP range
+### Match Whitelisted IP range
 
 The IP can be IPv4- or IPv6-formatted, using [CIDR notation](https://en.wikipedia.org/wiki/Classless_Inter-Domain_Routing#CIDR_blocks).  To match a single IP address only, use `/32`.
 
@@ -268,7 +269,7 @@ The IP can be IPv4- or IPv6-formatted, using [CIDR notation](https://en.wikipedi
 }
 ```
 
-### 7. Match scalr-signature
+### Match scalr-signature
 
 The trigger rule checks the scalr signature and also checks that the request was signed less than 5 minutes before it was received. 
 A unqiue signing key is generated for each webhook endpoint URL you register in Scalr.

diff --git a/docs/Templates.md b/docs/Templates.md
@@ -6,7 +6,7 @@ In additional to the [built-in Go template functions and features][tt], `webhook
 
 ## Example Usage
 
-In the example `hooks.json` file below, the `payload-hash-sha1` matching rule looks up the secret hash from the environment using the `getenv` template function.
+In the example `hooks.json` file below, the `payload-hmac-sha1` matching rule looks up the HMAC secret from the environment using the `getenv` template function.
 Additionally, the result is piped through the built-in Go template function `js` to ensure that the result is a well-formed Javascript/JSON string.
 
 ```
@@ -44,7 +44,7 @@ Additionally, the result is piped through the built-in Go template function `js`
         {
           "match":
           {
-            "type": "payload-hash-sha1",
+            "type": "payload-hmac-sha1",
             "secret": "{{ getenv "XXXTEST_SECRET" | js }}",
             "parameter":
             {

diff --git a/hooks.json.example b/hooks.json.example
@@ -33,7 +33,7 @@
         {
           "match":
           {
-            "type": "payload-hash-sha1",
+            "type": "payload-hmac-sha1",
             "secret": "mysecret",
             "parameter":
             {

diff --git a/hooks.json.tmpl.example b/hooks.json.tmpl.example
@@ -33,7 +33,7 @@
         {
           "match":
           {
-            "type": "payload-hash-sha1",
+            "type": "payload-hmac-sha1",
             "secret": "{{ getenv "XXXTEST_SECRET" | js }}",
             "parameter":
             {

diff --git a/hooks.yaml.example b/hooks.yaml.example
@@ -15,7 +15,7 @@
   trigger-rule:
     and:
     - match:
-        type: payload-hash-sha1
+        type: payload-hmac-sha1
         secret: mysecret
         parameter:
           source: header

diff --git a/hooks.yaml.tmpl.example b/hooks.yaml.tmpl.example
@@ -15,7 +15,7 @@
   trigger-rule:
     and:
     - match:
-        type: payload-hash-sha1
+        type: payload-hmac-sha1
         secret: "{{ getenv "XXXTEST_SECRET" | js }}"
         parameter:
           source: header

diff --git a/internal/hook/hook.go b/internal/hook/hook.go
@@ -883,6 +883,9 @@ type MatchRule struct {
 const (
 	MatchValue      string = "value"
 	MatchRegex      string = "regex"
+	MatchHMACSHA1   string = "payload-hmac-sha1"
+	MatchHMACSHA256 string = "payload-hmac-sha256"
+	MatchHMACSHA512 string = "payload-hmac-sha512"
 	MatchHashSHA1   string = "payload-hash-sha1"
 	MatchHashSHA256 string = "payload-hash-sha256"
 	MatchHashSHA512 string = "payload-hash-sha512"
@@ -907,12 +910,21 @@ func (r MatchRule) Evaluate(req *Request) (bool, error) {
 		case MatchRegex:
 			return regexp.MatchString(r.Regex, arg)
 		case MatchHashSHA1:
+			log.Print(`warn: use of deprecated option payload-hash-sha1; use payload-hmac-sha1 instead`)
+			fallthrough
+		case MatchHMACSHA1:
 			_, err := CheckPayloadSignature(req.Body, r.Secret, arg)
 			return err == nil, err
 		case MatchHashSHA256:
+			log.Print(`warn: use of deprecated option payload-hash-sha256: use payload-hmac-sha256 instead`)
+			fallthrough
+		case MatchHMACSHA256:
 			_, err := CheckPayloadSignature256(req.Body, r.Secret, arg)
 			return err == nil, err
 		case MatchHashSHA512:
+			log.Print(`warn: use of deprecated option payload-hash-sha512: use payload-hmac-sha512 instead`)
+			fallthrough
+		case MatchHMACSHA512:
 			_, err := CheckPayloadSignature512(req.Body, r.Secret, arg)
 			return err == nil, err
 		}

diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go
@@ -487,16 +487,22 @@ var matchRuleTests = []struct {
 }{
 	{"value", "", "", "z", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", true, false},
 	{"regex", "^z", "", "z", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", true, false},
+	{"payload-hmac-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "b17e04cbb22afa8ffbff8796fc1894ed27badd9e"}, nil, nil, []byte(`{"a": "z"}`), "", true, false},
 	{"payload-hash-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "b17e04cbb22afa8ffbff8796fc1894ed27badd9e"}, nil, nil, []byte(`{"a": "z"}`), "", true, false},
+	{"payload-hmac-sha256", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89"}, nil, nil, []byte(`{"a": "z"}`), "", true, false},
 	{"payload-hash-sha256", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89"}, nil, nil, []byte(`{"a": "z"}`), "", true, false},
 	// failures
 	{"value", "", "", "X", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, false},
 	{"regex", "^X", "", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, false},
 	{"value", "", "2", "X", "", Argument{"header", "a", "", false}, map[string]interface{}{"Y": "z"}, nil, nil, []byte{}, "", false, true}, // reference invalid header
 	// errors
 	{"regex", "*", "", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, true},                   // invalid regex
+	{"payload-hmac-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true},   // invalid hmac
 	{"payload-hash-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true},   // invalid hmac
+	{"payload-hmac-sha256", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac
 	{"payload-hash-sha256", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac
+	{"payload-hmac-sha512", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac
+	{"payload-hash-sha512", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": ""}, nil, nil, []byte{}, "", false, true}, // invalid hmac
 	// IP whitelisting, valid cases
 	{"ip-whitelist", "", "", "", "192.168.0.1/24", Argument{}, nil, nil, nil, []byte{}, "192.168.0.2:9000", true, false}, // valid IPv4, with range
 	{"ip-whitelist", "", "", "", "192.168.0.1/24", Argument{}, nil, nil, nil, []byte{}, "192.168.0.2:9000", true, false}, // valid IPv4, with range

diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl
@@ -31,7 +31,7 @@
         {
           "match":
           {
-            "type": "payload-hash-sha1",
+            "type": "payload-hmac-sha1",
             "secret": "mysecret",
             "parameter":
             {
@@ -288,7 +288,7 @@
         {
           "match":
           {
-            "type": "payload-hash-sha1",
+            "type": "payload-hmac-sha1",
             "secret": "mysecret",
             "parameter":
             {

diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl
@@ -8,7 +8,7 @@
           source: header
           name: X-Hub-Signature
         secret: mysecret
-        type: payload-hash-sha1
+        type: payload-hmac-sha1
     - match:
         parameter:
           source: payload
@@ -174,4 +174,4 @@
           source: header
           name: X-Hub-Signature
         secret: mysecret
-        type: payload-hash-sha1
+        type: payload-hmac-sha1