From 8728ec4786155510c8a257fde417a80fe32fecc0 Mon Sep 17 00:00:00 2001 From: Fabrizio Destro <7031675+dexpota@users.noreply.github.com> Date: Sat, 19 Oct 2019 23:14:47 +0200 Subject: [PATCH 01/28] Add help target to Makefile --- Makefile | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index b341c1f..1a06482 100644 --- a/Makefile +++ b/Makefile @@ -1,12 +1,18 @@ OS = darwin freebsd linux openbsd ARCHS = 386 arm amd64 arm64 +.DEFAULT_GOAL := help + +.PHONY: help +help: + @grep -E '^[a-zA-Z_-]+:.*?## .*$$' $(MAKEFILE_LIST) | sort | awk 'BEGIN {FS = ":.*?## "}; {printf "\033[36m%-16s\033[0m %s\n", $$1, $$2}' + all: build release release-windows -build: deps +build: deps ## Build the project go build -release: clean deps +release: clean deps ## Generate releases for unix systems @for arch in $(ARCHS);\ do \ for os in $(OS);\ @@ -18,7 +24,7 @@ release: clean deps done \ done -release-windows: clean deps +release-windows: clean deps ## Generate release for windows @for arch in $(ARCHS);\ do \ echo "Building windows-$$arch"; \ @@ -27,12 +33,12 @@ release-windows: clean deps tar cz -C build -f build/webhook-windows-$$arch.tar.gz webhook-windows-$$arch; \ done -test: deps +test: deps ## Execute tests go test ./... -deps: +deps: ## Install dependencies using go get go get -d -v -t ./... -clean: +clean: ## Remove building artifacts rm -rf build rm -f webhook From a818e291131b3cdd34887688d653ac7a84487592 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 15 Nov 2019 15:38:27 -0700 Subject: [PATCH 02/28] feat: added multiple sig support --- hook/hook.go | 73 ++++++++++++++++++++++++++++++++++++++++------- hook/hook_test.go | 6 ++++ 2 files changed, 68 insertions(+), 11 deletions(-) diff --git a/hook/hook.go b/hook/hook.go index 72acb01..7418440 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -17,6 +17,7 @@ import ( "math" "net" "net/textproto" + "net/url" "os" "reflect" "regexp" @@ -48,13 +49,19 @@ const ( // SignatureError describes an invalid payload signature passed to Hook. type SignatureError struct { - Signature string + Signature string + Signatures []string } func (e *SignatureError) Error() string { if e == nil { return "" } + + if e.Signatures != nil { + return fmt.Sprintf("invalid payload signatures %s", e.Signatures) + } + return fmt.Sprintf("invalid payload signature %s", e.Signature) } @@ -94,13 +101,47 @@ func (e *ParseError) Error() string { return e.Err.Error() } +// ExtractCommaSeperatedValues will extract the values matching the key. +func ExtractCommaSeperatedValues(source, key string) []string { + parts := strings.Split(source, ",") + values := make([]string, 0) + for _, part := range parts { + m, err := url.ParseQuery(part) + if err != nil { + continue + } + + // Try to get the value. + value := m.Get(key) + if value != "" { + values = append(values, value) + } + } + + return values +} + +func ExtractSignatures(signature, key string) []string { + // If there are multiple possible matches, let the comma seperated extractor + // do it's work. + if strings.Contains(signature, ",") { + return ExtractCommaSeperatedValues(signature, key) + } + + // There were no commas, so just trim the prefix (if it even exists) and + // pass it back. + return []string{ + strings.TrimPrefix(signature, key+"="), + } +} + // CheckPayloadSignature calculates and verifies SHA1 signature of the given payload func CheckPayloadSignature(payload []byte, secret string, signature string) (string, error) { if secret == "" { return "", errors.New("signature validation secret can not be empty") } - signature = strings.TrimPrefix(signature, "sha1=") + signatures := ExtractSignatures(signature, "sha1") mac := hmac.New(sha1.New, []byte(secret)) _, err := mac.Write(payload) @@ -109,10 +150,15 @@ func CheckPayloadSignature(payload []byte, secret string, signature string) (str } expectedMAC := hex.EncodeToString(mac.Sum(nil)) - if !hmac.Equal([]byte(signature), []byte(expectedMAC)) { - return expectedMAC, &SignatureError{signature} + for _, signature := range signatures { + if hmac.Equal([]byte(signature), []byte(expectedMAC)) { + return expectedMAC, err + } + } + + return expectedMAC, &SignatureError{ + Signatures: signatures, } - return expectedMAC, err } // CheckPayloadSignature256 calculates and verifies SHA256 signature of the given payload @@ -121,7 +167,7 @@ func CheckPayloadSignature256(payload []byte, secret string, signature string) ( return "", errors.New("signature validation secret can not be empty") } - signature = strings.TrimPrefix(signature, "sha256=") + signatures := ExtractSignatures(signature, "sha256") mac := hmac.New(sha256.New, []byte(secret)) _, err := mac.Write(payload) @@ -151,10 +197,15 @@ func CheckPayloadSignature512(payload []byte, secret string, signature string) ( } expectedMAC := hex.EncodeToString(mac.Sum(nil)) - if !hmac.Equal([]byte(signature), []byte(expectedMAC)) { - return expectedMAC, &SignatureError{signature} + for _, signature := range signatures { + if hmac.Equal([]byte(signature), []byte(expectedMAC)) { + return expectedMAC, err + } + } + + return expectedMAC, &SignatureError{ + Signatures: signatures, } - return expectedMAC, err } func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey string, checkDate bool) (bool, error) { @@ -177,7 +228,7 @@ func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey expectedSignature := hex.EncodeToString(mac.Sum(nil)) if !hmac.Equal([]byte(providedSignature), []byte(expectedSignature)) { - return false, &SignatureError{providedSignature} + return false, &SignatureError{Signature: providedSignature} } if !checkDate { @@ -192,7 +243,7 @@ func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey delta := math.Abs(now.Sub(date).Seconds()) if delta > 300 { - return false, &SignatureError{"outdated"} + return false, &SignatureError{Signature: "outdated"} } return true, nil } diff --git a/hook/hook_test.go b/hook/hook_test.go index ce6d760..c7ddec0 100644 --- a/hook/hook_test.go +++ b/hook/hook_test.go @@ -48,8 +48,11 @@ var checkPayloadSignatureTests = []struct { }{ {[]byte(`{"a": "z"}`), "secret", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", true}, {[]byte(`{"a": "z"}`), "secret", "sha1=b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", true}, + {[]byte(`{"a": "z"}`), "secret", "sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e,sha1=b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", true}, // failures {[]byte(`{"a": "z"}`), "secret", "XXXe04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", false}, + {[]byte(`{"a": "z"}`), "secret", "sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", false}, + {[]byte(`{"a": "z"}`), "secret", "sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e,sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", false}, {[]byte(`{"a": "z"}`), "secreX", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "900225703e9342328db7307692736e2f7cc7b36e", false}, {[]byte(`{"a": "z"}`), "", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "", false}, } @@ -76,8 +79,11 @@ var checkPayloadSignature256Tests = []struct { }{ {[]byte(`{"a": "z"}`), "secret", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", true}, {[]byte(`{"a": "z"}`), "secret", "sha256=f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", true}, + {[]byte(`{"a": "z"}`), "secret", "sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89,sha256=f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", true}, // failures {[]byte(`{"a": "z"}`), "secret", "XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", false}, + {[]byte(`{"a": "z"}`), "secret", "sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", false}, + {[]byte(`{"a": "z"}`), "secret", "sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89,sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", false}, {[]byte(`{"a": "z"}`), "", "XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "", false}, } From 2088f61cbadbca2d50ce88ae8fa88e58e1e5bc40 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 15 Nov 2019 15:52:03 -0700 Subject: [PATCH 03/28] fix: simplify implementation --- hook/hook.go | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/hook/hook.go b/hook/hook.go index 7418440..1bc8969 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -17,7 +17,6 @@ import ( "math" "net" "net/textproto" - "net/url" "os" "reflect" "regexp" @@ -102,36 +101,29 @@ func (e *ParseError) Error() string { } // ExtractCommaSeperatedValues will extract the values matching the key. -func ExtractCommaSeperatedValues(source, key string) []string { +func ExtractCommaSeperatedValues(source, prefix string) []string { parts := strings.Split(source, ",") values := make([]string, 0) for _, part := range parts { - m, err := url.ParseQuery(part) - if err != nil { - continue - } - - // Try to get the value. - value := m.Get(key) - if value != "" { - values = append(values, value) + if strings.HasPrefix(part, prefix) { + values = append(values, strings.TrimPrefix(part, prefix)) } } return values } -func ExtractSignatures(signature, key string) []string { +func ExtractSignatures(signature, prefix string) []string { // If there are multiple possible matches, let the comma seperated extractor // do it's work. if strings.Contains(signature, ",") { - return ExtractCommaSeperatedValues(signature, key) + return ExtractCommaSeperatedValues(signature, prefix) } // There were no commas, so just trim the prefix (if it even exists) and // pass it back. return []string{ - strings.TrimPrefix(signature, key+"="), + strings.TrimPrefix(signature, prefix), } } @@ -141,7 +133,7 @@ func CheckPayloadSignature(payload []byte, secret string, signature string) (str return "", errors.New("signature validation secret can not be empty") } - signatures := ExtractSignatures(signature, "sha1") + signatures := ExtractSignatures(signature, "sha1=") mac := hmac.New(sha1.New, []byte(secret)) _, err := mac.Write(payload) @@ -167,7 +159,7 @@ func CheckPayloadSignature256(payload []byte, secret string, signature string) ( return "", errors.New("signature validation secret can not be empty") } - signatures := ExtractSignatures(signature, "sha256") + signatures := ExtractSignatures(signature, "sha256=") mac := hmac.New(sha256.New, []byte(secret)) _, err := mac.Write(payload) From c6e809a1a20077aae8f8fab8e03bea256fb2ec9e Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 18 Nov 2019 09:31:49 -0700 Subject: [PATCH 04/28] fix: spelling --- hook/hook.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/hook/hook.go b/hook/hook.go index 1bc8969..bc317ee 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -100,8 +100,8 @@ func (e *ParseError) Error() string { return e.Err.Error() } -// ExtractCommaSeperatedValues will extract the values matching the key. -func ExtractCommaSeperatedValues(source, prefix string) []string { +// ExtractCommaSeparatedValues will extract the values matching the key. +func ExtractCommaSeparatedValues(source, prefix string) []string { parts := strings.Split(source, ",") values := make([]string, 0) for _, part := range parts { @@ -117,7 +117,7 @@ func ExtractSignatures(signature, prefix string) []string { // If there are multiple possible matches, let the comma seperated extractor // do it's work. if strings.Contains(signature, ",") { - return ExtractCommaSeperatedValues(signature, prefix) + return ExtractCommaSeparatedValues(signature, prefix) } // There were no commas, so just trim the prefix (if it even exists) and From 3f5fee20c0ebb19791dac669f7f75072d831ca48 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 17 Dec 2019 10:18:08 -0700 Subject: [PATCH 05/28] fix: updated based on review - added support for sha512 - added notes to docs --- docs/Hook-Rules.md | 21 +++++++++++ hook/hook.go | 86 ++++++++++++++++++++-------------------------- 2 files changed, 59 insertions(+), 48 deletions(-) diff --git a/docs/Hook-Rules.md b/docs/Hook-Rules.md index 68b83d3..e1f92f6 100644 --- a/docs/Hook-Rules.md +++ b/docs/Hook-Rules.md @@ -186,6 +186,13 @@ For the regex syntax, check out } ``` +Note that if multiple signatures were passed via a comma separated string, each +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 ```json { @@ -202,6 +209,13 @@ For the regex syntax, check out } ``` +Note that if multiple signatures were passed via a comma separated string, each +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 ```json { @@ -218,6 +232,13 @@ For the regex syntax, check out } ``` +Note that if multiple signatures were passed via a comma separated string, each +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 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`. diff --git a/hook/hook.go b/hook/hook.go index bc317ee..43ac8c5 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -12,6 +12,7 @@ import ( "encoding/json" "errors" "fmt" + "hash" "io/ioutil" "log" "math" @@ -113,17 +114,40 @@ func ExtractCommaSeparatedValues(source, prefix string) []string { return values } -func ExtractSignatures(signature, prefix string) []string { +// ExtractSignatures will extract all the signatures from the source. +func ExtractSignatures(source, prefix string) []string { // If there are multiple possible matches, let the comma seperated extractor // do it's work. - if strings.Contains(signature, ",") { - return ExtractCommaSeparatedValues(signature, prefix) + if strings.Contains(source, ",") { + return ExtractCommaSeparatedValues(source, prefix) } // There were no commas, so just trim the prefix (if it even exists) and // pass it back. return []string{ - strings.TrimPrefix(signature, prefix), + strings.TrimPrefix(source, prefix), + } +} + +// ValidateMAC will verify that the expected mac for the given hash will match +// the one provided. +func ValidateMAC(payload []byte, mac hash.Hash, signatures []string) (string, error) { + // Write the payload to the provided hash. + _, err := mac.Write(payload) + if err != nil { + return "", err + } + + expectedMAC := hex.EncodeToString(mac.Sum(nil)) + + for _, signature := range signatures { + if hmac.Equal([]byte(signature), []byte(expectedMAC)) { + return expectedMAC, err + } + } + + return expectedMAC, &SignatureError{ + Signatures: signatures, } } @@ -133,24 +157,11 @@ func CheckPayloadSignature(payload []byte, secret string, signature string) (str return "", errors.New("signature validation secret can not be empty") } + // Extract the signatures. signatures := ExtractSignatures(signature, "sha1=") - mac := hmac.New(sha1.New, []byte(secret)) - _, err := mac.Write(payload) - if err != nil { - return "", err - } - expectedMAC := hex.EncodeToString(mac.Sum(nil)) - - for _, signature := range signatures { - if hmac.Equal([]byte(signature), []byte(expectedMAC)) { - return expectedMAC, err - } - } - - return expectedMAC, &SignatureError{ - Signatures: signatures, - } + // Validate the MAC. + return ValidateMAC(payload, hmac.New(sha1.New, []byte(secret)), signatures) } // CheckPayloadSignature256 calculates and verifies SHA256 signature of the given payload @@ -159,19 +170,11 @@ func CheckPayloadSignature256(payload []byte, secret string, signature string) ( return "", errors.New("signature validation secret can not be empty") } + // Extract the signatures. signatures := ExtractSignatures(signature, "sha256=") - mac := hmac.New(sha256.New, []byte(secret)) - _, err := mac.Write(payload) - if err != nil { - return "", err - } - expectedMAC := hex.EncodeToString(mac.Sum(nil)) - - if !hmac.Equal([]byte(signature), []byte(expectedMAC)) { - return expectedMAC, &SignatureError{signature} - } - return expectedMAC, err + // Validate the MAC. + return ValidateMAC(payload, hmac.New(sha256.New, []byte(secret)), signatures) } // CheckPayloadSignature512 calculates and verifies SHA512 signature of the given payload @@ -180,24 +183,11 @@ func CheckPayloadSignature512(payload []byte, secret string, signature string) ( return "", errors.New("signature validation secret can not be empty") } - signature = strings.TrimPrefix(signature, "sha512=") + // Extract the signatures. + signatures := ExtractSignatures(signature, "sha512=") - mac := hmac.New(sha512.New, []byte(secret)) - _, err := mac.Write(payload) - if err != nil { - return "", err - } - expectedMAC := hex.EncodeToString(mac.Sum(nil)) - - for _, signature := range signatures { - if hmac.Equal([]byte(signature), []byte(expectedMAC)) { - return expectedMAC, err - } - } - - return expectedMAC, &SignatureError{ - Signatures: signatures, - } + // Validate the MAC. + return ValidateMAC(payload, hmac.New(sha512.New, []byte(secret)), signatures) } func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey string, checkDate bool) (bool, error) { From 8c5b2e0c17470c7d9a72285f471030aa1ea007c1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adnan=20Hajdarevi=C4=87?= Date: Fri, 3 Jan 2020 23:38:49 +0100 Subject: [PATCH 06/28] Revert "Multiple Signature Support" --- docs/Hook-Rules.md | 21 --------- hook/hook.go | 111 ++++++++++++++++----------------------------- hook/hook_test.go | 6 --- 3 files changed, 39 insertions(+), 99 deletions(-) diff --git a/docs/Hook-Rules.md b/docs/Hook-Rules.md index e1f92f6..68b83d3 100644 --- a/docs/Hook-Rules.md +++ b/docs/Hook-Rules.md @@ -186,13 +186,6 @@ For the regex syntax, check out } ``` -Note that if multiple signatures were passed via a comma separated string, each -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 ```json { @@ -209,13 +202,6 @@ X-Hub-Signature: sha1=the-first-signature,sha1=the-second-signature } ``` -Note that if multiple signatures were passed via a comma separated string, each -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 ```json { @@ -232,13 +218,6 @@ X-Hub-Signature: sha256=the-first-signature,sha256=the-second-signature } ``` -Note that if multiple signatures were passed via a comma separated string, each -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 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`. diff --git a/hook/hook.go b/hook/hook.go index 43ac8c5..72acb01 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -12,7 +12,6 @@ import ( "encoding/json" "errors" "fmt" - "hash" "io/ioutil" "log" "math" @@ -49,19 +48,13 @@ const ( // SignatureError describes an invalid payload signature passed to Hook. type SignatureError struct { - Signature string - Signatures []string + Signature string } func (e *SignatureError) Error() string { if e == nil { return "" } - - if e.Signatures != nil { - return fmt.Sprintf("invalid payload signatures %s", e.Signatures) - } - return fmt.Sprintf("invalid payload signature %s", e.Signature) } @@ -101,67 +94,25 @@ func (e *ParseError) Error() string { return e.Err.Error() } -// ExtractCommaSeparatedValues will extract the values matching the key. -func ExtractCommaSeparatedValues(source, prefix string) []string { - parts := strings.Split(source, ",") - values := make([]string, 0) - for _, part := range parts { - if strings.HasPrefix(part, prefix) { - values = append(values, strings.TrimPrefix(part, prefix)) - } - } - - return values -} - -// ExtractSignatures will extract all the signatures from the source. -func ExtractSignatures(source, prefix string) []string { - // If there are multiple possible matches, let the comma seperated extractor - // do it's work. - if strings.Contains(source, ",") { - return ExtractCommaSeparatedValues(source, prefix) - } - - // There were no commas, so just trim the prefix (if it even exists) and - // pass it back. - return []string{ - strings.TrimPrefix(source, prefix), - } -} - -// ValidateMAC will verify that the expected mac for the given hash will match -// the one provided. -func ValidateMAC(payload []byte, mac hash.Hash, signatures []string) (string, error) { - // Write the payload to the provided hash. - _, err := mac.Write(payload) - if err != nil { - return "", err - } - - expectedMAC := hex.EncodeToString(mac.Sum(nil)) - - for _, signature := range signatures { - if hmac.Equal([]byte(signature), []byte(expectedMAC)) { - return expectedMAC, err - } - } - - return expectedMAC, &SignatureError{ - Signatures: signatures, - } -} - // CheckPayloadSignature calculates and verifies SHA1 signature of the given payload func CheckPayloadSignature(payload []byte, secret string, signature string) (string, error) { if secret == "" { return "", errors.New("signature validation secret can not be empty") } - // Extract the signatures. - signatures := ExtractSignatures(signature, "sha1=") + signature = strings.TrimPrefix(signature, "sha1=") - // Validate the MAC. - return ValidateMAC(payload, hmac.New(sha1.New, []byte(secret)), signatures) + mac := hmac.New(sha1.New, []byte(secret)) + _, err := mac.Write(payload) + if err != nil { + return "", err + } + expectedMAC := hex.EncodeToString(mac.Sum(nil)) + + if !hmac.Equal([]byte(signature), []byte(expectedMAC)) { + return expectedMAC, &SignatureError{signature} + } + return expectedMAC, err } // CheckPayloadSignature256 calculates and verifies SHA256 signature of the given payload @@ -170,11 +121,19 @@ func CheckPayloadSignature256(payload []byte, secret string, signature string) ( return "", errors.New("signature validation secret can not be empty") } - // Extract the signatures. - signatures := ExtractSignatures(signature, "sha256=") + signature = strings.TrimPrefix(signature, "sha256=") - // Validate the MAC. - return ValidateMAC(payload, hmac.New(sha256.New, []byte(secret)), signatures) + mac := hmac.New(sha256.New, []byte(secret)) + _, err := mac.Write(payload) + if err != nil { + return "", err + } + expectedMAC := hex.EncodeToString(mac.Sum(nil)) + + if !hmac.Equal([]byte(signature), []byte(expectedMAC)) { + return expectedMAC, &SignatureError{signature} + } + return expectedMAC, err } // CheckPayloadSignature512 calculates and verifies SHA512 signature of the given payload @@ -183,11 +142,19 @@ func CheckPayloadSignature512(payload []byte, secret string, signature string) ( return "", errors.New("signature validation secret can not be empty") } - // Extract the signatures. - signatures := ExtractSignatures(signature, "sha512=") + signature = strings.TrimPrefix(signature, "sha512=") - // Validate the MAC. - return ValidateMAC(payload, hmac.New(sha512.New, []byte(secret)), signatures) + mac := hmac.New(sha512.New, []byte(secret)) + _, err := mac.Write(payload) + if err != nil { + return "", err + } + expectedMAC := hex.EncodeToString(mac.Sum(nil)) + + if !hmac.Equal([]byte(signature), []byte(expectedMAC)) { + return expectedMAC, &SignatureError{signature} + } + return expectedMAC, err } func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey string, checkDate bool) (bool, error) { @@ -210,7 +177,7 @@ func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey expectedSignature := hex.EncodeToString(mac.Sum(nil)) if !hmac.Equal([]byte(providedSignature), []byte(expectedSignature)) { - return false, &SignatureError{Signature: providedSignature} + return false, &SignatureError{providedSignature} } if !checkDate { @@ -225,7 +192,7 @@ func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey delta := math.Abs(now.Sub(date).Seconds()) if delta > 300 { - return false, &SignatureError{Signature: "outdated"} + return false, &SignatureError{"outdated"} } return true, nil } diff --git a/hook/hook_test.go b/hook/hook_test.go index c7ddec0..ce6d760 100644 --- a/hook/hook_test.go +++ b/hook/hook_test.go @@ -48,11 +48,8 @@ var checkPayloadSignatureTests = []struct { }{ {[]byte(`{"a": "z"}`), "secret", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", true}, {[]byte(`{"a": "z"}`), "secret", "sha1=b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", true}, - {[]byte(`{"a": "z"}`), "secret", "sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e,sha1=b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", true}, // failures {[]byte(`{"a": "z"}`), "secret", "XXXe04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", false}, - {[]byte(`{"a": "z"}`), "secret", "sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", false}, - {[]byte(`{"a": "z"}`), "secret", "sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e,sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", false}, {[]byte(`{"a": "z"}`), "secreX", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "900225703e9342328db7307692736e2f7cc7b36e", false}, {[]byte(`{"a": "z"}`), "", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "", false}, } @@ -79,11 +76,8 @@ var checkPayloadSignature256Tests = []struct { }{ {[]byte(`{"a": "z"}`), "secret", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", true}, {[]byte(`{"a": "z"}`), "secret", "sha256=f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", true}, - {[]byte(`{"a": "z"}`), "secret", "sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89,sha256=f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", true}, // failures {[]byte(`{"a": "z"}`), "secret", "XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", false}, - {[]byte(`{"a": "z"}`), "secret", "sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", false}, - {[]byte(`{"a": "z"}`), "secret", "sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89,sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", false}, {[]byte(`{"a": "z"}`), "", "XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "", false}, } From 4897bea79f0cc3c42ed8aae595abd8f776b8e72c Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Fri, 24 Apr 2020 09:13:11 -0500 Subject: [PATCH 07/28] Use Go 1.14 cipher suites Now that Go 1.14 is out, we can remove cipher_suites.go and use the stdlib. --- cipher_suites.go | 102 ----------------------------------------------- tls.go | 4 +- 2 files changed, 2 insertions(+), 104 deletions(-) delete mode 100644 cipher_suites.go diff --git a/cipher_suites.go b/cipher_suites.go deleted file mode 100644 index 81db51f..0000000 --- a/cipher_suites.go +++ /dev/null @@ -1,102 +0,0 @@ -// Copyright 2010 The Go Authors. All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. - -// Copied from Go 1.14 tip src/crypto/tls/cipher_suites.go - -package main - -import ( - "crypto/tls" - "fmt" -) - -// CipherSuite is a TLS cipher suite. Note that most functions in this package -// accept and expose cipher suite IDs instead of this type. -type CipherSuite struct { - ID uint16 - Name string - - // Supported versions is the list of TLS protocol versions that can - // negotiate this cipher suite. - SupportedVersions []uint16 - - // Insecure is true if the cipher suite has known security issues - // due to its primitives, design, or implementation. - Insecure bool -} - -var ( - supportedUpToTLS12 = []uint16{tls.VersionTLS10, tls.VersionTLS11, tls.VersionTLS12} - supportedOnlyTLS12 = []uint16{tls.VersionTLS12} - supportedOnlyTLS13 = []uint16{tls.VersionTLS13} -) - -// CipherSuites returns a list of cipher suites currently implemented by this -// package, excluding those with security issues, which are returned by -// InsecureCipherSuites. -// -// The list is sorted by ID. Note that the default cipher suites selected by -// this package might depend on logic that can't be captured by a static list. -func CipherSuites() []*CipherSuite { - return []*CipherSuite{ - {tls.TLS_RSA_WITH_3DES_EDE_CBC_SHA, "TLS_RSA_WITH_3DES_EDE_CBC_SHA", supportedUpToTLS12, false}, - {tls.TLS_RSA_WITH_AES_128_CBC_SHA, "TLS_RSA_WITH_AES_128_CBC_SHA", supportedUpToTLS12, false}, - {tls.TLS_RSA_WITH_AES_256_CBC_SHA, "TLS_RSA_WITH_AES_256_CBC_SHA", supportedUpToTLS12, false}, - {tls.TLS_RSA_WITH_AES_128_GCM_SHA256, "TLS_RSA_WITH_AES_128_GCM_SHA256", supportedOnlyTLS12, false}, - {tls.TLS_RSA_WITH_AES_256_GCM_SHA384, "TLS_RSA_WITH_AES_256_GCM_SHA384", supportedOnlyTLS12, false}, - - {tls.TLS_AES_128_GCM_SHA256, "TLS_AES_128_GCM_SHA256", supportedOnlyTLS13, false}, - {tls.TLS_AES_256_GCM_SHA384, "TLS_AES_256_GCM_SHA384", supportedOnlyTLS13, false}, - {tls.TLS_CHACHA20_POLY1305_SHA256, "TLS_CHACHA20_POLY1305_SHA256", supportedOnlyTLS13, false}, - - {tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA, "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA", supportedUpToTLS12, false}, - {tls.TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA, "TLS_ECDHE_ECDSA_WITH_AES_256_CBC_SHA", supportedUpToTLS12, false}, - {tls.TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA, "TLS_ECDHE_RSA_WITH_3DES_EDE_CBC_SHA", supportedUpToTLS12, false}, - {tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA, "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA", supportedUpToTLS12, false}, - {tls.TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA, "TLS_ECDHE_RSA_WITH_AES_256_CBC_SHA", supportedUpToTLS12, false}, - {tls.TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256, "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256", supportedOnlyTLS12, false}, - {tls.TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384, "TLS_ECDHE_ECDSA_WITH_AES_256_GCM_SHA384", supportedOnlyTLS12, false}, - {tls.TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256, "TLS_ECDHE_RSA_WITH_AES_128_GCM_SHA256", supportedOnlyTLS12, false}, - {tls.TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384, "TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384", supportedOnlyTLS12, false}, - - // go1.14 - // {tls.TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256, "TLS_ECDHE_RSA_WITH_CHACHA20_POLY1305_SHA256", supportedOnlyTLS12, false}, - // {tls.TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256, "TLS_ECDHE_ECDSA_WITH_CHACHA20_POLY1305_SHA256", supportedOnlyTLS12, false}, - } -} - -// InsecureCipherSuites returns a list of cipher suites currently implemented by -// this package and which have security issues. -// -// Most applications should not use the cipher suites in this list, and should -// only use those returned by CipherSuites. -func InsecureCipherSuites() []*CipherSuite { - // RC4 suites are broken because RC4 is. - // CBC-SHA256 suites have no Lucky13 countermeasures. - return []*CipherSuite{ - {tls.TLS_RSA_WITH_RC4_128_SHA, "TLS_RSA_WITH_RC4_128_SHA", supportedUpToTLS12, true}, - {tls.TLS_RSA_WITH_AES_128_CBC_SHA256, "TLS_RSA_WITH_AES_128_CBC_SHA256", supportedOnlyTLS12, true}, - {tls.TLS_ECDHE_ECDSA_WITH_RC4_128_SHA, "TLS_ECDHE_ECDSA_WITH_RC4_128_SHA", supportedUpToTLS12, true}, - {tls.TLS_ECDHE_RSA_WITH_RC4_128_SHA, "TLS_ECDHE_RSA_WITH_RC4_128_SHA", supportedUpToTLS12, true}, - {tls.TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256, "TLS_ECDHE_ECDSA_WITH_AES_128_CBC_SHA256", supportedOnlyTLS12, true}, - {tls.TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256, "TLS_ECDHE_RSA_WITH_AES_128_CBC_SHA256", supportedOnlyTLS12, true}, - } -} - -// CipherSuiteName returns the standard name for the passed cipher suite ID -// (e.g. "TLS_ECDHE_ECDSA_WITH_AES_128_GCM_SHA256"), or a fallback representation -// of the ID value if the cipher suite is not implemented by this package. -func CipherSuiteName(id uint16) string { - for _, c := range CipherSuites() { - if c.ID == id { - return c.Name - } - } - for _, c := range InsecureCipherSuites() { - if c.ID == id { - return c.Name - } - } - return fmt.Sprintf("0x%04X", id) -} diff --git a/tls.go b/tls.go index 8e6cb73..526fd36 100644 --- a/tls.go +++ b/tls.go @@ -8,7 +8,7 @@ import ( ) func writeTLSSupportedCipherStrings(w io.Writer, min uint16) error { - for _, c := range CipherSuites() { + for _, c := range tls.CipherSuites() { var found bool for _, v := range c.SupportedVersions { @@ -50,7 +50,7 @@ func getTLSMinVersion(v string) uint16 { // getTLSCipherSuites converts a comma separated list of cipher suites into a // slice of TLS cipher suite IDs. func getTLSCipherSuites(v string) []uint16 { - supported := CipherSuites() + supported := tls.CipherSuites() if v == "" { suites := make([]uint16, len(supported)) From 4407c0190bb1c6e0e88d2c95e4083193746fbf0f Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Fri, 24 Apr 2020 15:32:33 -0500 Subject: [PATCH 08/28] Add request ID logging on missing command --- webhook.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webhook.go b/webhook.go index aebf1f6..874b883 100644 --- a/webhook.go +++ b/webhook.go @@ -569,12 +569,12 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in } if err != nil { - log.Printf("unable to locate command: '%s'", h.ExecuteCommand) + log.Printf("[%s] error locating command: '%s'", rid, h.ExecuteCommand) // check if parameters specified in execute-command by mistake if strings.IndexByte(h.ExecuteCommand, ' ') != -1 { s := strings.Fields(h.ExecuteCommand)[0] - log.Printf("use 'pass-arguments-to-command' to specify args for '%s'", s) + log.Printf("[%s] use 'pass-arguments-to-command' to specify args for '%s'", rid, s) } return "", err From 4f437e46421cf0f1cf9e26ac88fd87035bdd7dff Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Mon, 11 May 2020 20:21:37 -0500 Subject: [PATCH 09/28] Fix missing command test --- webhook_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/webhook_test.go b/webhook_test.go index 8b53981..ebf350d 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -171,7 +171,7 @@ func buildHookecho(t *testing.T) (binPath string, cleanupFn func()) { return binPath, func() { os.RemoveAll(tmp) } } -func genConfig(t *testing.T, bin string, hookTemplate string) (configPath string, cleanupFn func()) { +func genConfig(t *testing.T, bin, hookTemplate string) (configPath string, cleanupFn func()) { tmpl := template.Must(template.ParseFiles(hookTemplate)) tmp, err := ioutil.TempDir("", "webhook-config-") @@ -684,7 +684,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 // Check logs {"static params should pass", "static-params-ok", nil, "POST", nil, "application/json", `{}`, http.StatusOK, "arg: passed\n", `(?s)command output: arg: passed`}, - {"command with space logs warning", "warn-on-space", nil, "POST", nil, "application/json", `{}`, http.StatusInternalServerError, "Error occurred while executing the hook's command. Please check your logs for more details.", `(?s)unable to locate command.*use 'pass[-]arguments[-]to[-]command' to specify args`}, + {"command with space logs warning", "warn-on-space", nil, "POST", nil, "application/json", `{}`, http.StatusInternalServerError, "Error occurred while executing the hook's command. Please check your logs for more details.", `(?s)error locating command.*use 'pass[-]arguments[-]to[-]command' to specify args`}, {"unsupported content type error", "github", nil, "POST", map[string]string{"Content-Type": "nonexistent/format"}, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, `(?s)error parsing body payload due to unsupported content type header:`}, } From e6e324235dd142bfa312087d103b16b59e2d647b Mon Sep 17 00:00:00 2001 From: Adnan Hajdarevic Date: Tue, 12 May 2020 19:14:25 +0200 Subject: [PATCH 10/28] Bump version to v2.7.0 --- webhook.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/webhook.go b/webhook.go index 874b883..1af2ce5 100644 --- a/webhook.go +++ b/webhook.go @@ -28,7 +28,7 @@ import ( ) const ( - version = "2.6.11" + version = "2.7.0" ) var ( From 345bf3d4094de98df6bd56b91e04b4c0cfe6cc62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adnan=20Hajdarevi=C4=87?= Date: Thu, 14 May 2020 14:22:24 +0200 Subject: [PATCH 11/28] Update README.md --- README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 083c36f..9527262 100644 --- a/README.md +++ b/README.md @@ -28,9 +28,9 @@ If you don't have time to waste configuring, hosting, debugging and maintaining ### Building from source To get started, first make sure you've properly set up your [Go](http://golang.org/doc/install) 1.12 or newer environment and then run ```bash -$ go get github.com/adnanh/webhook +$ go build github.com/adnanh/webhook ``` -to get the latest version of the [webhook][w]. +to build the latest version of the [webhook][w]. ### Using package manager #### Snap store From 526c9a20ac8ae1dbe309db3a59b86b6476946398 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 21 May 2020 17:47:55 -0500 Subject: [PATCH 12/28] Fix request dumper The existing code had a bug in printing request params. Simplify the request logger by using httputil.DumpRequest. Also print the request before handing it downstream. Fixes #425 --- internal/middleware/dumper.go | 79 +++++++++++++---------------------- 1 file changed, 30 insertions(+), 49 deletions(-) diff --git a/internal/middleware/dumper.go b/internal/middleware/dumper.go index 582c163..ae0e110 100644 --- a/internal/middleware/dumper.go +++ b/internal/middleware/dumper.go @@ -8,13 +8,11 @@ import ( "bytes" "fmt" "io" - "io/ioutil" "net" "net/http" + "net/http/httputil" "sort" "strings" - - "github.com/gorilla/mux" ) // responseDupper tees the response to a buffer and a response writer. @@ -34,70 +32,53 @@ func Dumper(w io.Writer) func(http.Handler) http.Handler { // Request ID rid := r.Context().Value(RequestIDKey) - // Request URL - buf.WriteString(fmt.Sprintf("> [%s] %s %s", rid, r.Method, r.URL.String())) + // Dump request - // Request Headers - keys := make([]string, len(r.Header)) - i := 0 - for k := range r.Header { - keys[i] = k - i++ - } - sort.Strings(keys) - for _, k := range keys { - buf.WriteString(fmt.Sprintf("\n> [%s] %s: %s", rid, k, strings.Join(r.Header[k], ", "))) - } - - // Request parameters - params := mux.Vars(r) - keys = make([]string, len(params)) - i = 0 - for k := range params { - keys[i] = k - i++ - } - sort.Strings(keys) - for _, k := range keys { - buf.WriteString(fmt.Sprintf("\n> [%s] %s: %s", rid, k, strings.Join(r.Header[k], ", "))) - } - - // Request body - b, err := ioutil.ReadAll(r.Body) + bd, err := httputil.DumpRequest(r, true) if err != nil { - b = []byte("failed to read body: " + err.Error()) + buf.WriteString(fmt.Sprintf("[%s] Error dumping request for debugging: %s\n", rid, err)) } - if len(b) > 0 { - buf.WriteByte('\n') - lines := strings.Split(string(b), "\n") - for _, line := range lines { - buf.WriteString(fmt.Sprintf("> [%s] %s\n", rid, line)) - } + + sc := bufio.NewScanner(bytes.NewBuffer(bd)) + sc.Split(bufio.ScanLines) + for sc.Scan() { + buf.WriteString(fmt.Sprintf("> [%s] ", rid)) + buf.WriteString(sc.Text() + "\n") } - r.Body = ioutil.NopCloser(bytes.NewBuffer(b)) + + w.Write(buf.Bytes()) + buf.Reset() + + // Dump Response dupper := &responseDupper{ResponseWriter: rw, Buffer: &bytes.Buffer{}} h.ServeHTTP(dupper, r) - buf.WriteString(fmt.Sprintf("\n< [%s] %s", rid, http.StatusText(dupper.Status))) - keys = make([]string, len(dupper.Header())) - i = 0 + // Response Status + buf.WriteString(fmt.Sprintf("< [%s] %d %s\n", rid, dupper.Status, http.StatusText(dupper.Status))) + + // Response Headers + keys := make([]string, len(dupper.Header())) + i := 0 for k := range dupper.Header() { keys[i] = k i++ } sort.Strings(keys) for _, k := range keys { - buf.WriteString(fmt.Sprintf("\n< [%s] %s: %s", rid, k, strings.Join(dupper.Header()[k], ", "))) + buf.WriteString(fmt.Sprintf("< [%s] %s: %s\n", rid, k, strings.Join(dupper.Header()[k], ", "))) } + + // Response Body if dupper.Buffer.Len() > 0 { - buf.WriteByte('\n') - lines := strings.Split(dupper.Buffer.String(), "\n") - for _, line := range lines { - buf.WriteString(fmt.Sprintf("< [%s] %s\n", rid, line)) + buf.WriteString(fmt.Sprintf("< [%s]\n", rid)) + sc = bufio.NewScanner(dupper.Buffer) + sc.Split(bufio.ScanLines) + for sc.Scan() { + buf.WriteString(fmt.Sprintf("< [%s] ", rid)) + buf.WriteString(sc.Text() + "\n") } } - buf.WriteByte('\n') w.Write(buf.Bytes()) }) } From 41ac427a895a80b744e1ab698350277121fd0e1f Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 21 May 2020 18:02:52 -0500 Subject: [PATCH 13/28] Warn on failed validate of empty payload signature If signature validation fails on an empty payload, append a note to the end of the error message. Updates #423 --- internal/hook/hook.go | 36 +++++++++++++++++++++++------------- internal/hook/hook_test.go | 27 ++++++++++++++++++++------- test/hooks.json.tmpl | 24 ++++++++++++++++++++++++ test/hooks.yaml.tmpl | 13 +++++++++++++ webhook_test.go | 16 +++++++++++++++- 5 files changed, 95 insertions(+), 21 deletions(-) diff --git a/internal/hook/hook.go b/internal/hook/hook.go index a9b4774..dd5546c 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -73,6 +73,8 @@ func IsParameterNodeError(err error) bool { type SignatureError struct { Signature string Signatures []string + + emptyPayload bool } func (e *SignatureError) Error() string { @@ -80,11 +82,16 @@ func (e *SignatureError) Error() string { return "" } - if e.Signatures != nil { - return fmt.Sprintf("invalid payload signatures %s", e.Signatures) + var empty string + if e.emptyPayload { + empty = " on empty payload" } - return fmt.Sprintf("invalid payload signature %s", e.Signature) + if e.Signatures != nil { + return fmt.Sprintf("invalid payload signatures %s%s", e.Signatures, empty) + } + + return fmt.Sprintf("invalid payload signature %s%s", e.Signature, empty) } // ArgumentError describes an invalid argument passed to Hook. @@ -160,21 +167,24 @@ func ValidateMAC(payload []byte, mac hash.Hash, signatures []string) (string, er return "", err } - expectedMAC := hex.EncodeToString(mac.Sum(nil)) + actualMAC := hex.EncodeToString(mac.Sum(nil)) for _, signature := range signatures { - if hmac.Equal([]byte(signature), []byte(expectedMAC)) { - return expectedMAC, err + if hmac.Equal([]byte(signature), []byte(actualMAC)) { + return actualMAC, err } } - return expectedMAC, &SignatureError{ - Signatures: signatures, + e := &SignatureError{Signatures: signatures} + if len(payload) == 0 { + e.emptyPayload = true } + + return actualMAC, e } // CheckPayloadSignature calculates and verifies SHA1 signature of the given payload -func CheckPayloadSignature(payload []byte, secret string, signature string) (string, error) { +func CheckPayloadSignature(payload []byte, secret, signature string) (string, error) { if secret == "" { return "", errors.New("signature validation secret can not be empty") } @@ -187,7 +197,7 @@ func CheckPayloadSignature(payload []byte, secret string, signature string) (str } // CheckPayloadSignature256 calculates and verifies SHA256 signature of the given payload -func CheckPayloadSignature256(payload []byte, secret string, signature string) (string, error) { +func CheckPayloadSignature256(payload []byte, secret, signature string) (string, error) { if secret == "" { return "", errors.New("signature validation secret can not be empty") } @@ -200,7 +210,7 @@ func CheckPayloadSignature256(payload []byte, secret string, signature string) ( } // CheckPayloadSignature512 calculates and verifies SHA512 signature of the given payload -func CheckPayloadSignature512(payload []byte, secret string, signature string) (string, error) { +func CheckPayloadSignature512(payload []byte, secret, signature string) (string, error) { if secret == "" { return "", errors.New("signature validation secret can not be empty") } @@ -254,7 +264,7 @@ func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey // CheckIPWhitelist makes sure the provided remote address (of the form IP:port) falls within the provided IP range // (in CIDR form or a single IP address). -func CheckIPWhitelist(remoteAddr string, ipRange string) (bool, error) { +func CheckIPWhitelist(remoteAddr, ipRange string) (bool, error) { // Extract IP address from remote address. // IPv6 addresses will likely be surrounded by []. @@ -293,7 +303,7 @@ func CheckIPWhitelist(remoteAddr string, ipRange string) (bool, error) { // ReplaceParameter replaces parameter value with the passed value in the passed map // (please note you should pass pointer to the map, because we're modifying it) // based on the passed string -func ReplaceParameter(s string, params interface{}, value interface{}) bool { +func ReplaceParameter(s string, params, value interface{}) bool { if params == nil { return false } diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go index 489fde2..e348930 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -49,12 +49,14 @@ var checkPayloadSignatureTests = []struct { {[]byte(`{"a": "z"}`), "secret", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", true}, {[]byte(`{"a": "z"}`), "secret", "sha1=b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", true}, {[]byte(`{"a": "z"}`), "secret", "sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e,sha1=b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", true}, + {[]byte(``), "secret", "25af6174a0fcecc4d346680a72b7ce644b9a88e8", "25af6174a0fcecc4d346680a72b7ce644b9a88e8", true}, // failures {[]byte(`{"a": "z"}`), "secret", "XXXe04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", false}, {[]byte(`{"a": "z"}`), "secret", "sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", false}, {[]byte(`{"a": "z"}`), "secret", "sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e,sha1=XXXe04cbb22afa8ffbff8796fc1894ed27badd9e", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", false}, {[]byte(`{"a": "z"}`), "secreX", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "900225703e9342328db7307692736e2f7cc7b36e", false}, {[]byte(`{"a": "z"}`), "", "b17e04cbb22afa8ffbff8796fc1894ed27badd9e", "", false}, + {[]byte(``), "secret", "XXXf6174a0fcecc4d346680a72b7ce644b9a88e8", "25af6174a0fcecc4d346680a72b7ce644b9a88e8", false}, } func TestCheckPayloadSignature(t *testing.T) { @@ -80,11 +82,13 @@ var checkPayloadSignature256Tests = []struct { {[]byte(`{"a": "z"}`), "secret", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", true}, {[]byte(`{"a": "z"}`), "secret", "sha256=f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", true}, {[]byte(`{"a": "z"}`), "secret", "sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89,sha256=f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", true}, + {[]byte(``), "secret", "f9e66e179b6747ae54108f82f8ade8b3c25d76fd30afde6c395822c530196169", "f9e66e179b6747ae54108f82f8ade8b3c25d76fd30afde6c395822c530196169", true}, // failures {[]byte(`{"a": "z"}`), "secret", "XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", false}, {[]byte(`{"a": "z"}`), "secret", "sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", false}, {[]byte(`{"a": "z"}`), "secret", "sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89,sha256=XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "f417af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", false}, {[]byte(`{"a": "z"}`), "", "XXX7af3a21bd70379b5796d5f013915e7029f62c580fb0f500f59a35a6f04c89", "", false}, + {[]byte(``), "secret", "XXX66e179b6747ae54108f82f8ade8b3c25d76fd30afde6c395822c530196169", "f9e66e179b6747ae54108f82f8ade8b3c25d76fd30afde6c395822c530196169", false}, } func TestCheckPayloadSignature256(t *testing.T) { @@ -109,9 +113,11 @@ var checkPayloadSignature512Tests = []struct { }{ {[]byte(`{"a": "z"}`), "secret", "4ab17cc8ec668ead8bf498f87f8f32848c04d5ca3c9bcfcd3db9363f0deb44e580b329502a7fdff633d4d8fca301cc5c94a55a2fec458c675fb0ff2655898324", "4ab17cc8ec668ead8bf498f87f8f32848c04d5ca3c9bcfcd3db9363f0deb44e580b329502a7fdff633d4d8fca301cc5c94a55a2fec458c675fb0ff2655898324", true}, {[]byte(`{"a": "z"}`), "secret", "sha512=4ab17cc8ec668ead8bf498f87f8f32848c04d5ca3c9bcfcd3db9363f0deb44e580b329502a7fdff633d4d8fca301cc5c94a55a2fec458c675fb0ff2655898324", "4ab17cc8ec668ead8bf498f87f8f32848c04d5ca3c9bcfcd3db9363f0deb44e580b329502a7fdff633d4d8fca301cc5c94a55a2fec458c675fb0ff2655898324", true}, + {[]byte(``), "secret", "b0e9650c5faf9cd8ae02276671545424104589b3656731ec193b25d01b07561c27637c2d4d68389d6cf5007a8632c26ec89ba80a01c77a6cdd389ec28db43901", "b0e9650c5faf9cd8ae02276671545424104589b3656731ec193b25d01b07561c27637c2d4d68389d6cf5007a8632c26ec89ba80a01c77a6cdd389ec28db43901", true}, // failures {[]byte(`{"a": "z"}`), "secret", "74a0081f5b5988f4f3e8b8dd34dadc6291611f2e6260635a7e1535f8e95edb97ff520ba8b152e8ca5760ac42639854f3242e29efc81be73a8bf52d474d31ffea", "4ab17cc8ec668ead8bf498f87f8f32848c04d5ca3c9bcfcd3db9363f0deb44e580b329502a7fdff633d4d8fca301cc5c94a55a2fec458c675fb0ff2655898324", false}, {[]byte(`{"a": "z"}`), "", "74a0081f5b5988f4f3e8b8dd34dadc6291611f2e6260635a7e1535f8e95edb97ff520ba8b152e8ca5760ac42639854f3242e29efc81be73a8bf52d474d31ffea", "", false}, + {[]byte(``), "secret", "XXX9650c5faf9cd8ae02276671545424104589b3656731ec193b25d01b07561c27637c2d4d68389d6cf5007a8632c26ec89ba80a01c77a6cdd389ec28db43901", "b0e9650c5faf9cd8ae02276671545424104589b3656731ec193b25d01b07561c27637c2d4d68389d6cf5007a8632c26ec89ba80a01c77a6cdd389ec28db43901", false}, } func TestCheckPayloadSignature512(t *testing.T) { @@ -502,7 +508,8 @@ var andRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "z", "B": "y"}, nil, nil, []byte{}, + &map[string]interface{}{"A": "z", "B": "y"}, nil, nil, + []byte{}, true, false, }, { @@ -511,7 +518,8 @@ var andRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "z", "B": "Y"}, nil, nil, []byte{}, + &map[string]interface{}{"A": "z", "B": "Y"}, nil, nil, + []byte{}, false, false, }, // Complex test to cover Rules.Evaluate @@ -537,7 +545,8 @@ var andRuleTests = []struct { }, }, }, - &map[string]interface{}{"A": "z", "B": "y", "C": "x", "D": "w", "E": "X", "F": "X"}, nil, nil, []byte{}, + &map[string]interface{}{"A": "z", "B": "y", "C": "x", "D": "w", "E": "X", "F": "X"}, nil, nil, + []byte{}, true, false, }, {"empty rule", AndRule{{}}, nil, nil, nil, nil, false, false}, @@ -573,7 +582,8 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "z", "B": "X"}, nil, nil, []byte{}, + &map[string]interface{}{"A": "z", "B": "X"}, nil, nil, + []byte{}, true, false, }, { @@ -582,7 +592,8 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "X", "B": "y"}, nil, nil, []byte{}, + &map[string]interface{}{"A": "X", "B": "y"}, nil, nil, + []byte{}, true, false, }, { @@ -591,7 +602,8 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "Z", "B": "Y"}, nil, nil, []byte{}, + &map[string]interface{}{"A": "Z", "B": "Y"}, nil, nil, + []byte{}, false, false, }, // failures @@ -600,7 +612,8 @@ var orRuleTests = []struct { OrRule{ {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, }, - &map[string]interface{}{"Y": "Z"}, nil, nil, []byte{}, + &map[string]interface{}{"Y": "Z"}, nil, nil, + []byte{}, false, true, }, } diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index 0c7fc0c..71673ce 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -259,5 +259,29 @@ "name": "passed" } ], + }, + { + "id": "empty-payload-signature", + "execute-command": "{{ .Hookecho }}", + "command-working-directory": "/", + "include-command-output-in-response": true, + "trigger-rule": + { + "and": + [ + { + "match": + { + "type": "payload-hash-sha1", + "secret": "mysecret", + "parameter": + { + "source": "header", + "name": "X-Hub-Signature" + } + } + } + ] + } } ] diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index f1e1204..7c6e201 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -150,3 +150,16 @@ - id: warn-on-space execute-command: '{{ .Hookecho }} foo' include-command-output-in-response: true + +- id: empty-payload-signature + include-command-output-in-response: true + execute-command: '{{ .Hookecho }}' + command-working-directory: / + trigger-rule: + and: + - match: + parameter: + source: header + name: X-Hub-Signature + secret: mysecret + type: payload-hash-sha1 diff --git a/webhook_test.go b/webhook_test.go index ebf350d..6dda780 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -77,7 +77,7 @@ func TestWebhook(t *testing.T) { for _, tt := range hookHandlerTests { t.Run(tt.desc+"@"+hookTmpl, func(t *testing.T) { ip, port := serverAddress(t) - args := []string{fmt.Sprintf("-hooks=%s", configPath), fmt.Sprintf("-ip=%s", ip), fmt.Sprintf("-port=%s", port), "-verbose"} + args := []string{fmt.Sprintf("-hooks=%s", configPath), fmt.Sprintf("-ip=%s", ip), fmt.Sprintf("-port=%s", port), "-debug"} if len(tt.cliMethods) != 0 { args = append(args, "-http-methods="+strings.Join(tt.cliMethods, ",")) @@ -111,6 +111,7 @@ func TestWebhook(t *testing.T) { var res *http.Response req.Header.Add("Content-Type", tt.contentType) + req.ContentLength = int64(len(tt.body)) client := &http.Client{} res, err = client.Do(req) @@ -663,6 +664,19 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 ``, }, + { + "empty-payload-signature", // allow empty payload signature validation + "empty-payload-signature", + nil, + "POST", + map[string]string{"X-Hub-Signature": "33f9d709782f62b8b4a0178586c65ab098a39fe2"}, + "application/json", + ``, + http.StatusOK, + ``, + ``, + }, + // test with disallowed global HTTP method {"global disallowed method", "bitbucket", []string{"Post "}, "GET", nil, `{}`, "application/json", http.StatusMethodNotAllowed, ``, ``}, // test with disallowed HTTP method From cc98de88ce28775136e66fbdeba7a0af4fa33d14 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 28 May 2020 18:17:41 -0500 Subject: [PATCH 14/28] Fix godoc comment on LogEntry.Panic --- internal/middleware/logger.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/middleware/logger.go b/internal/middleware/logger.go index 50c6a44..5e74360 100644 --- a/internal/middleware/logger.go +++ b/internal/middleware/logger.go @@ -50,7 +50,7 @@ func (l *LogEntry) Write(status, bytes int, elapsed time.Duration) { log.Print(l.buf.String()) } -/// Panic prints the call stack for a panic. +// Panic prints the call stack for a panic. func (l *LogEntry) Panic(v interface{}, stack []byte) { e := l.NewLogEntry(l.req).(*LogEntry) fmt.Fprintf(e.buf, "panic: %#v", v) From 3d824b47b74547df7bfbe681c5e4250bec9405bf Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 28 May 2020 18:20:07 -0500 Subject: [PATCH 15/28] Rename var to avoid shadowing bytes package importShadow: shadow of imported package 'bytes' (gocritic) --- internal/middleware/logger.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/middleware/logger.go b/internal/middleware/logger.go index 5e74360..899dd1c 100644 --- a/internal/middleware/logger.go +++ b/internal/middleware/logger.go @@ -39,13 +39,13 @@ type LogEntry struct { } // Write constructs and writes the final log entry. -func (l *LogEntry) Write(status, bytes int, elapsed time.Duration) { +func (l *LogEntry) Write(status, totalBytes int, elapsed time.Duration) { rid := GetReqID(l.req.Context()) if rid != "" { fmt.Fprintf(l.buf, "[%s] ", rid) } - fmt.Fprintf(l.buf, "%03d | %s | %s | ", status, humanize.IBytes(uint64(bytes)), elapsed) + fmt.Fprintf(l.buf, "%03d | %s | %s | ", status, humanize.IBytes(uint64(totalBytes)), elapsed) l.buf.WriteString(l.req.Host + " | " + l.req.Method + " " + l.req.RequestURI) log.Print(l.buf.String()) } From c9199d62e46eabd03ce6c55a04746e0f8b0cb427 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 28 May 2020 18:13:28 -0500 Subject: [PATCH 16/28] Tighten file permissions on pidfile creation Fixes report from gosec: "G306: Expect WriteFile permissions to be 0600 or less." Also, use new octal number formatting. --- internal/pidfile/pidfile.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/pidfile/pidfile.go b/internal/pidfile/pidfile.go index c88c159..e602dcb 100644 --- a/internal/pidfile/pidfile.go +++ b/internal/pidfile/pidfile.go @@ -35,10 +35,10 @@ func New(path string) (*PIDFile, error) { return nil, err } // Note MkdirAll returns nil if a directory already exists - if err := MkdirAll(filepath.Dir(path), os.FileMode(0755)); err != nil { + if err := MkdirAll(filepath.Dir(path), os.FileMode(0o755)); err != nil { return nil, err } - if err := ioutil.WriteFile(path, []byte(fmt.Sprintf("%d", os.Getpid())), 0644); err != nil { + if err := ioutil.WriteFile(path, []byte(fmt.Sprintf("%d", os.Getpid())), 0o600); err != nil { return nil, err } From eefcd7f7d5be1407142f0e6713484d162588c3b3 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Mon, 27 Jul 2020 12:59:58 -0500 Subject: [PATCH 17/28] Require Go 1.14 When go.mod specifies go 1.14 or higher, the go tools now verify that vendor/modules.txt is consistent with go.mod. Fixed by running `go mod vendor`. --- .travis.yml | 2 +- go.mod | 2 +- vendor/modules.txt | 17 +++++++++++++++++ 3 files changed, 19 insertions(+), 2 deletions(-) diff --git a/.travis.yml b/.travis.yml index 86f29d5..54e4652 100644 --- a/.travis.yml +++ b/.travis.yml @@ -1,7 +1,7 @@ language: go go: - - 1.13.x + - 1.14.x - master os: diff --git a/go.mod b/go.mod index bc7efb7..48f350c 100644 --- a/go.mod +++ b/go.mod @@ -1,6 +1,6 @@ module github.com/adnanh/webhook -go 1.13 +go 1.14 require ( github.com/clbanning/mxj v1.8.4 diff --git a/vendor/modules.txt b/vendor/modules.txt index 06d8a6a..8c121cf 100644 --- a/vendor/modules.txt +++ b/vendor/modules.txt @@ -1,20 +1,37 @@ # github.com/clbanning/mxj v1.8.4 +## explicit github.com/clbanning/mxj # github.com/dustin/go-humanize v1.0.0 +## explicit github.com/dustin/go-humanize +# github.com/fsnotify/fsnotify v1.4.7 +## explicit # github.com/ghodss/yaml v1.0.0 +## explicit github.com/ghodss/yaml # github.com/go-chi/chi v4.0.2+incompatible +## explicit github.com/go-chi/chi github.com/go-chi/chi/middleware # github.com/gofrs/uuid v3.2.0+incompatible +## explicit github.com/gofrs/uuid # github.com/gorilla/mux v1.7.3 +## explicit github.com/gorilla/mux +# github.com/kr/pretty v0.1.0 +## explicit +# golang.org/x/net v0.0.0-20191209160850-c0dbc17a3553 +## explicit # golang.org/x/sys v0.0.0-20191228213918-04cbcbbfeed8 +## explicit golang.org/x/sys/unix golang.org/x/sys/windows +# gopkg.in/check.v1 v1.0.0-20190902080502-41f04d3bba15 +## explicit # gopkg.in/fsnotify.v1 v1.4.2 +## explicit gopkg.in/fsnotify.v1 # gopkg.in/yaml.v2 v2.0.0-20170812160011-eb3733d160e7 +## explicit gopkg.in/yaml.v2 From fb9b22a118d68675ac061e100ac89b633df40005 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Gabe=20Ga=C5=82=C4=85zka?= <38294951+flawedworld@users.noreply.github.com> Date: Wed, 22 Jul 2020 16:37:22 +0100 Subject: [PATCH 18/28] Change minimum golang version to 1.14 in README --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 9527262..1706b79 100644 --- a/README.md +++ b/README.md @@ -26,7 +26,7 @@ If you don't have time to waste configuring, hosting, debugging and maintaining # Getting started ## Installation ### Building from source -To get started, first make sure you've properly set up your [Go](http://golang.org/doc/install) 1.12 or newer environment and then run +To get started, first make sure you've properly set up your [Go](http://golang.org/doc/install) 1.14 or newer environment and then run ```bash $ go build github.com/adnanh/webhook ``` From 0e90ccb441288b3068d0f77cdd9bef37824ffaab Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Wed, 29 Jul 2020 09:49:02 -0500 Subject: [PATCH 19/28] Add support for top-level JSON array in payload Detect if leading character in JSON payload is an array bracket. If found, decode payload into an interface{} and then save the results into payload["root"]. References to payload values would need to reference the leading, "virtual" root node (i.e. "root.0.name"). Fixes #215 --- docs/Hook-Examples.md | 51 +++++++++++++++++++++++++++++++++++++++++++ test/hooks.json.tmpl | 16 ++++++++++++++ test/hooks.yaml.tmpl | 12 ++++++++++ webhook.go | 31 +++++++++++++++++++++----- webhook_test.go | 22 +++++++++++++++++++ 5 files changed, 127 insertions(+), 5 deletions(-) diff --git a/docs/Hook-Examples.md b/docs/Hook-Examples.md index 23896a7..cb4145e 100644 --- a/docs/Hook-Examples.md +++ b/docs/Hook-Examples.md @@ -425,6 +425,57 @@ Travis sends webhooks as `payload=`, so the payload needs to be par ] ``` +## JSON Array Payload + +If the JSON payload is an array instead of an object, `webhook` will process the payload and place it into a "root" object. +Therefore, references to payload values must begin with `root.`. + +For example, given the following payload (taken from the Sendgrid Event Webhook documentation): +```json +[ + { + "email": "example@test.com", + "timestamp": 1513299569, + "smtp-id": "<14c5d75ce93.dfd.64b469@ismtpd-555>", + "event": "processed", + "category": "cat facts", + "sg_event_id": "sg_event_id", + "sg_message_id": "sg_message_id" + }, + { + "email": "example@test.com", + "timestamp": 1513299569, + "smtp-id": "<14c5d75ce93.dfd.64b469@ismtpd-555>", + "event": "deferred", + "category": "cat facts", + "sg_event_id": "sg_event_id", + "sg_message_id": "sg_message_id", + "response": "400 try again later", + "attempt": "5" + } +] +``` + +A reference to the second item in the array would look like this: +```json +[ + { + "id": "sendgrid", + "execute-command": "{{ .Hookecho }}", + "trigger-rule": { + "match": { + "type": "value", + "parameter": { + "source": "payload", + "name": "root.1.event" + }, + "value": "deferred" + } + } + } +] +``` + ## XML Payload Given the following payload: diff --git a/test/hooks.json.tmpl b/test/hooks.json.tmpl index 71673ce..7fe7c8c 100644 --- a/test/hooks.json.tmpl +++ b/test/hooks.json.tmpl @@ -168,6 +168,22 @@ ], } }, + { + "id": "sendgrid", + "execute-command": "{{ .Hookecho }}", + "command-working-directory": "/", + "response-message": "success", + "trigger-rule": { + "match": { + "type": "value", + "parameter": { + "source": "payload", + "name": "root.0.event" + }, + "value": "processed" + } + } + }, { "id": "plex", "execute-command": "{{ .Hookecho }}", diff --git a/test/hooks.yaml.tmpl b/test/hooks.yaml.tmpl index 7c6e201..ace4277 100644 --- a/test/hooks.yaml.tmpl +++ b/test/hooks.yaml.tmpl @@ -97,6 +97,18 @@ name: "app.messages.message.#text" value: "Hello!!" +- id: sendgrid + execute-command: '{{ .Hookecho }}' + command-working-directory: / + response-message: success + trigger-rule: + match: + type: value + parameter: + source: payload + name: root.0.event + value: processed + - id: plex trigger-rule: match: diff --git a/webhook.go b/webhook.go index 1af2ce5..6a258c4 100644 --- a/webhook.go +++ b/webhook.go @@ -16,6 +16,7 @@ import ( "path/filepath" "strings" "time" + "unicode" "github.com/adnanh/webhook/internal/hook" "github.com/adnanh/webhook/internal/middleware" @@ -141,7 +142,7 @@ func main() { } if *logPath != "" { - file, err := os.OpenFile(*logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0666) + file, err := os.OpenFile(*logPath, os.O_CREATE|os.O_WRONLY|os.O_APPEND, 0o666) if err != nil { logQueue = append(logQueue, fmt.Sprintf("error opening log file %q: %v", *logPath, err)) // we'll bail out below @@ -385,9 +386,29 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { decoder := json.NewDecoder(bytes.NewReader(body)) decoder.UseNumber() - err := decoder.Decode(&payload) - if err != nil { - log.Printf("[%s] error parsing JSON payload %+v\n", rid, err) + var firstChar byte + for i := 0; i < len(body); i++ { + if unicode.IsSpace(rune(body[i])) { + continue + } + firstChar = body[i] + break + } + + if firstChar == byte('[') { + var arrayPayload interface{} + err := decoder.Decode(&arrayPayload) + if err != nil { + log.Printf("[%s] error parsing JSON array payload %+v\n", rid, err) + } + + payload = make(map[string]interface{}, 1) + payload["root"] = arrayPayload + } else { + err := decoder.Decode(&payload) + if err != nil { + log.Printf("[%s] error parsing JSON payload %+v\n", rid, err) + } } case strings.Contains(contentType, "x-www-form-urlencoded"): @@ -648,7 +669,7 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in return string(out), err } -func writeHttpResponseCode(w http.ResponseWriter, rid string, hookId string, responseCode int) { +func writeHttpResponseCode(w http.ResponseWriter, rid, hookId string, responseCode int) { // Check if the given return code is supported by the http package // by testing if there is a StatusText for this code. if len(http.StatusText(responseCode)) > 0 { diff --git a/webhook_test.go b/webhook_test.go index 6dda780..4671c24 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -547,6 +547,28 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 `success`, ``, }, + { + "payload-json-array", + "sendgrid", + nil, + "POST", + nil, + "application/json", + `[ + { + "email": "example@test.com", + "timestamp": 1513299569, + "smtp-id": "<14c5d75ce93.dfd.64b469@ismtpd-555>", + "event": "processed", + "category": "cat facts", + "sg_event_id": "sg_event_id", + "sg_message_id": "sg_message_id" + } +]`, + http.StatusOK, + `success`, + ``, + }, { "multipart", "plex", From 534e99bf131e9a331761eceaeb45a8e9ef1ee776 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Wed, 29 Jul 2020 17:13:33 -0500 Subject: [PATCH 20/28] Add a table of contents to some of the docs --- docs/Hook-Examples.md | 16 ++++++++++++++++ docs/Hook-Rules.md | 17 ++++++++++++++++- 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/docs/Hook-Examples.md b/docs/Hook-Examples.md index 23896a7..48ad38b 100644 --- a/docs/Hook-Examples.md +++ b/docs/Hook-Examples.md @@ -1,6 +1,22 @@ # Hook examples This page is still work in progress. Feel free to contribute! +### Table of Contents + +* [Incoming Github webhook](#incoming-github-webhook) +* [Incoming Bitbucket webhook](#incoming-bitbucket-webhook) +* [Incoming Gitlab webhook](#incoming-gitlab-webhook) +* [Incoming Gogs webhook](#incoming-gogs-webhook) +* [Incoming Gitea webhook](#incoming-gitea-webhook) +* [Slack slash command](#slack-slash-command) +* [A simple webhook with a secret key in GET query](#a-simple-webhook-with-a-secret-key-in-get-query) +* [JIRA Webhooks](#jira-webhooks) +* [Pass File-to-command sample](#pass-file-to-command-sample) +* [Incoming Scalr Webhook](#incoming-scalr-webhook) +* [Travis CI webhook](#travis-ci-webhook) +* [XML Payload](#xml-payload) +* [Multipart Form Data](#multipart-form-data) + ## Incoming Github webhook ```json [ diff --git a/docs/Hook-Rules.md b/docs/Hook-Rules.md index e1f92f6..0ff30b3 100644 --- a/docs/Hook-Rules.md +++ b/docs/Hook-Rules.md @@ -1,5 +1,20 @@ # Hook rules +### Table of Contents + +* [And](#and) +* [Or](#or) +* [Not](#not) +* [Multi-level](#multi-level) +* [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 Whitelisted IP range](#match-whitelisted-ip-range) + * [Match scalr-signature](#match-scalr-signature) + ## And *And rule* will evaluate to _true_, if and only if all of the sub rules evaluate to _true_. ```json @@ -267,4 +282,4 @@ Given the time check make sure that NTP is enabled on both your Scalr and webhoo "secret": "Scalr-provided signing key" } } -``` \ No newline at end of file +``` From ae5e9e7894fee973b40ad66d28ead6fb7d368e20 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Fri, 31 Jul 2020 11:58:12 -0500 Subject: [PATCH 21/28] Update ExtractParameterAsString to return JSON on complex types Fixes #448 --- internal/hook/hook.go | 17 +++++++++++++++-- internal/hook/hook_test.go | 7 +++++-- 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/internal/hook/hook.go b/internal/hook/hook.go index dd5546c..2c2d787 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -392,14 +392,27 @@ func GetParameter(s string, params interface{}) (interface{}, error) { return nil, &ParameterNodeError{s} } -// ExtractParameterAsString extracts value from interface{} as string based on the passed string +// ExtractParameterAsString extracts value from interface{} as string based on +// the passed string. Complex data types are rendered as JSON instead of the Go +// Stringer format. func ExtractParameterAsString(s string, params interface{}) (string, error) { pValue, err := GetParameter(s, params) if err != nil { return "", err } - return fmt.Sprintf("%v", pValue), nil + switch v := reflect.ValueOf(pValue); v.Kind() { + case reflect.Array, reflect.Map, reflect.Slice: + r, err := json.Marshal(pValue) + if err != nil { + return "", err + } + + return string(r), nil + + default: + return fmt.Sprintf("%v", pValue), nil + } } // Argument type specifies the parameter key name and the source it should diff --git a/internal/hook/hook_test.go b/internal/hook/hook_test.go index e348930..46bd444 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -223,6 +223,9 @@ var extractParameterTests = []struct { {"a.b.0", map[string]interface{}{"a": map[string]interface{}{"b": []interface{}{"x", "y", "z"}}}, "x", true}, {"a.1.b", map[string]interface{}{"a": []interface{}{map[string]interface{}{"b": "y"}, map[string]interface{}{"b": "z"}}}, "z", true}, {"a.1.b.c", map[string]interface{}{"a": []interface{}{map[string]interface{}{"b": map[string]interface{}{"c": "y"}}, map[string]interface{}{"b": map[string]interface{}{"c": "z"}}}}, "z", true}, + {"b", map[string]interface{}{"b": map[string]interface{}{"z": 1}}, `{"z":1}`, true}, + {"c", map[string]interface{}{"c": []interface{}{"y", "z"}}, `["y","z"]`, true}, + {"d", map[string]interface{}{"d": [2]interface{}{"y", "z"}}, `["y","z"]`, true}, // failures {"check_nil", nil, "", false}, {"a.X", map[string]interface{}{"a": map[string]interface{}{"b": "z"}}, "", false}, // non-existent parameter reference @@ -239,7 +242,7 @@ func TestExtractParameter(t *testing.T) { for _, tt := range extractParameterTests { value, err := ExtractParameterAsString(tt.s, tt.params) if (err == nil) != tt.ok || value != tt.value { - t.Errorf("failed to extract parameter %q:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%s}", tt.s, tt.value, tt.ok, value, err) + t.Errorf("failed to extract parameter %q:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%v}", tt.s, tt.value, tt.ok, value, err) } } } @@ -266,7 +269,7 @@ func TestArgumentGet(t *testing.T) { a := Argument{tt.source, tt.name, "", false} value, err := a.Get(tt.headers, tt.query, tt.payload) if (err == nil) != tt.ok || value != tt.value { - t.Errorf("failed to get {%q, %q}:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%s}", tt.source, tt.name, tt.value, tt.ok, value, err) + t.Errorf("failed to get {%q, %q}:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%v}", tt.source, tt.name, tt.value, tt.ok, value, err) } } } From dd5fa204157215aa9f38fc56929f284cadf321a5 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Tue, 15 Sep 2020 13:05:24 -0500 Subject: [PATCH 22/28] Log stdlib error on failed exec.LookPath The error returned by exec.LookPath was never surfaced to the user. Without that detail, the user can't tell the difference between a non-existent path and a permissions issue. Additionally, when ExecuteCommand is an absolute path, we were still attempting to prepend the CommandWorkingDirectory if the ExecuteCommand was not found, which made it difficult to know which path the user intended to execute. This commit simplifies the logic to avoid multiple attempts with ExecuteCommand is an absolute path and changes the error message from: error locating command: '/path/to/file' to: error in exec: "/path/to/file": stat /path/to/file: no such file or directory error in exec: "/path/to/file": permission denied Fixes #457 --- webhook.go | 14 +++++++------- webhook_test.go | 2 +- 2 files changed, 8 insertions(+), 8 deletions(-) diff --git a/webhook.go b/webhook.go index 6a258c4..26872ed 100644 --- a/webhook.go +++ b/webhook.go @@ -581,16 +581,16 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in var errors []error // check the command exists - cmdPath, err := exec.LookPath(h.ExecuteCommand) - if err != nil { - // give a last chance, maybe is a relative path - relativeToCwd := filepath.Join(h.CommandWorkingDirectory, h.ExecuteCommand) - // check the command exists - cmdPath, err = exec.LookPath(relativeToCwd) + var lookpath string + if filepath.IsAbs(h.ExecuteCommand) || h.CommandWorkingDirectory == "" { + lookpath = h.ExecuteCommand + } else { + lookpath = filepath.Join(h.CommandWorkingDirectory, h.ExecuteCommand) } + cmdPath, err := exec.LookPath(lookpath) if err != nil { - log.Printf("[%s] error locating command: '%s'", rid, h.ExecuteCommand) + log.Printf("[%s] error in %s", rid, err) // check if parameters specified in execute-command by mistake if strings.IndexByte(h.ExecuteCommand, ' ') != -1 { diff --git a/webhook_test.go b/webhook_test.go index 4671c24..353d22d 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -720,7 +720,7 @@ env: HOOK_head_commit.timestamp=2013-03-12T08:14:29-07:00 // Check logs {"static params should pass", "static-params-ok", nil, "POST", nil, "application/json", `{}`, http.StatusOK, "arg: passed\n", `(?s)command output: arg: passed`}, - {"command with space logs warning", "warn-on-space", nil, "POST", nil, "application/json", `{}`, http.StatusInternalServerError, "Error occurred while executing the hook's command. Please check your logs for more details.", `(?s)error locating command.*use 'pass[-]arguments[-]to[-]command' to specify args`}, + {"command with space logs warning", "warn-on-space", nil, "POST", nil, "application/json", `{}`, http.StatusInternalServerError, "Error occurred while executing the hook's command. Please check your logs for more details.", `(?s)error in exec:.*use 'pass[-]arguments[-]to[-]command' to specify args`}, {"unsupported content type error", "github", nil, "POST", map[string]string{"Content-Type": "nonexistent/format"}, "application/json", `{}`, http.StatusBadRequest, `Hook rules were not satisfied.`, `(?s)error parsing body payload due to unsupported content type header:`}, } From c6603894c1215ded1e717594b05dfb7cdd2dd81b Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Fri, 25 Sep 2020 19:46:06 -0500 Subject: [PATCH 23/28] Add Request object to hook package to simplify API To avoid having to pass around so many parameters to the hook package, create a Request object to store all request-specific data. Update APIs accordingly. --- internal/hook/hook.go | 121 ++++++++++++++++---------- internal/hook/hook_test.go | 172 ++++++++++++++++++++++++------------- webhook.go | 153 +++++++++++++++++---------------- webhook_test.go | 6 +- 4 files changed, 269 insertions(+), 183 deletions(-) diff --git a/internal/hook/hook.go b/internal/hook/hook.go index 2c2d787..e6a9272 100644 --- a/internal/hook/hook.go +++ b/internal/hook/hook.go @@ -17,6 +17,7 @@ import ( "log" "math" "net" + "net/http" "net/textproto" "os" "reflect" @@ -47,6 +48,30 @@ const ( EnvNamespace string = "HOOK_" ) +// Request represents a webhook request. +type Request struct { + // The request ID set by the RequestID middleware. + ID string + + // The Content-Type of the request. + ContentType string + + // The raw request body. + Body []byte + + // Headers is a map of the parsed headers. + Headers map[string]interface{} + + // Query is a map of the parsed URL query values. + Query map[string]interface{} + + // Payload is a map of the parsed payload. + Payload map[string]interface{} + + // The underlying HTTP request. + RawRequest *http.Request +} + // ParameterNodeError describes an error walking a parameter node. type ParameterNodeError struct { key string @@ -222,22 +247,26 @@ func CheckPayloadSignature512(payload []byte, secret, signature string) (string, return ValidateMAC(payload, hmac.New(sha512.New, []byte(secret)), signatures) } -func CheckScalrSignature(headers map[string]interface{}, body []byte, signingKey string, checkDate bool) (bool, error) { - // Check for the signature and date headers - if _, ok := headers["X-Signature"]; !ok { +func CheckScalrSignature(r *Request, signingKey string, checkDate bool) (bool, error) { + if r.Headers == nil { return false, nil } - if _, ok := headers["Date"]; !ok { + + // Check for the signature and date headers + if _, ok := r.Headers["X-Signature"]; !ok { + return false, nil + } + if _, ok := r.Headers["Date"]; !ok { return false, nil } if signingKey == "" { return false, errors.New("signature validation signing key can not be empty") } - providedSignature := headers["X-Signature"].(string) - dateHeader := headers["Date"].(string) + providedSignature := r.Headers["X-Signature"].(string) + dateHeader := r.Headers["Date"].(string) mac := hmac.New(sha1.New, []byte(signingKey)) - mac.Write(body) + mac.Write(r.Body) mac.Write([]byte(dateHeader)) expectedSignature := hex.EncodeToString(mac.Sum(nil)) @@ -426,41 +455,41 @@ type Argument struct { // Get Argument method returns the value for the Argument's key name // based on the Argument's source -func (ha *Argument) Get(headers, query, payload *map[string]interface{}) (string, error) { +func (ha *Argument) Get(r *Request) (string, error) { var source *map[string]interface{} key := ha.Name switch ha.Source { case SourceHeader: - source = headers + source = &r.Headers key = textproto.CanonicalMIMEHeaderKey(ha.Name) case SourceQuery, SourceQueryAlias: - source = query + source = &r.Query case SourcePayload: - source = payload + source = &r.Payload case SourceString: return ha.Name, nil case SourceEntirePayload: - r, err := json.Marshal(payload) + res, err := json.Marshal(&r.Payload) if err != nil { return "", err } - return string(r), nil + return string(res), nil case SourceEntireHeaders: - r, err := json.Marshal(headers) + res, err := json.Marshal(&r.Headers) if err != nil { return "", err } - return string(r), nil + return string(res), nil case SourceEntireQuery: - r, err := json.Marshal(query) + res, err := json.Marshal(&r.Query) if err != nil { return "", err } - return string(r), nil + return string(res), nil } if source != nil { @@ -545,11 +574,11 @@ type Hook struct { // ParseJSONParameters decodes specified arguments to JSON objects and replaces the // string with the newly created object -func (h *Hook) ParseJSONParameters(headers, query, payload *map[string]interface{}) []error { +func (h *Hook) ParseJSONParameters(r *Request) []error { errors := make([]error, 0) for i := range h.JSONStringParameters { - arg, err := h.JSONStringParameters[i].Get(headers, query, payload) + arg, err := h.JSONStringParameters[i].Get(r) if err != nil { errors = append(errors, &ArgumentError{h.JSONStringParameters[i]}) } else { @@ -568,11 +597,11 @@ func (h *Hook) ParseJSONParameters(headers, query, payload *map[string]interface switch h.JSONStringParameters[i].Source { case SourceHeader: - source = headers + source = &r.Headers case SourcePayload: - source = payload + source = &r.Payload case SourceQuery, SourceQueryAlias: - source = query + source = &r.Query } if source != nil { @@ -598,14 +627,14 @@ func (h *Hook) ParseJSONParameters(headers, query, payload *map[string]interface // ExtractCommandArguments creates a list of arguments, based on the // PassArgumentsToCommand property that is ready to be used with exec.Command() -func (h *Hook) ExtractCommandArguments(headers, query, payload *map[string]interface{}) ([]string, []error) { +func (h *Hook) ExtractCommandArguments(r *Request) ([]string, []error) { args := make([]string, 0) errors := make([]error, 0) args = append(args, h.ExecuteCommand) for i := range h.PassArgumentsToCommand { - arg, err := h.PassArgumentsToCommand[i].Get(headers, query, payload) + arg, err := h.PassArgumentsToCommand[i].Get(r) if err != nil { args = append(args, "") errors = append(errors, &ArgumentError{h.PassArgumentsToCommand[i]}) @@ -625,11 +654,11 @@ func (h *Hook) ExtractCommandArguments(headers, query, payload *map[string]inter // ExtractCommandArgumentsForEnv creates a list of arguments in key=value // format, based on the PassEnvironmentToCommand property that is ready to be used // with exec.Command(). -func (h *Hook) ExtractCommandArgumentsForEnv(headers, query, payload *map[string]interface{}) ([]string, []error) { +func (h *Hook) ExtractCommandArgumentsForEnv(r *Request) ([]string, []error) { args := make([]string, 0) errors := make([]error, 0) for i := range h.PassEnvironmentToCommand { - arg, err := h.PassEnvironmentToCommand[i].Get(headers, query, payload) + arg, err := h.PassEnvironmentToCommand[i].Get(r) if err != nil { errors = append(errors, &ArgumentError{h.PassEnvironmentToCommand[i]}) continue @@ -661,11 +690,11 @@ type FileParameter struct { // ExtractCommandArgumentsForFile creates a list of arguments in key=value // format, based on the PassFileToCommand property that is ready to be used // with exec.Command(). -func (h *Hook) ExtractCommandArgumentsForFile(headers, query, payload *map[string]interface{}) ([]FileParameter, []error) { +func (h *Hook) ExtractCommandArgumentsForFile(r *Request) ([]FileParameter, []error) { args := make([]FileParameter, 0) errors := make([]error, 0) for i := range h.PassFileToCommand { - arg, err := h.PassFileToCommand[i].Get(headers, query, payload) + arg, err := h.PassFileToCommand[i].Get(r) if err != nil { errors = append(errors, &ArgumentError{h.PassFileToCommand[i]}) continue @@ -772,16 +801,16 @@ type Rules struct { // Evaluate finds the first rule property that is not nil and returns the value // it evaluates to -func (r Rules) Evaluate(headers, query, payload *map[string]interface{}, body *[]byte, remoteAddr string) (bool, error) { +func (r Rules) Evaluate(req *Request) (bool, error) { switch { case r.And != nil: - return r.And.Evaluate(headers, query, payload, body, remoteAddr) + return r.And.Evaluate(req) case r.Or != nil: - return r.Or.Evaluate(headers, query, payload, body, remoteAddr) + return r.Or.Evaluate(req) case r.Not != nil: - return r.Not.Evaluate(headers, query, payload, body, remoteAddr) + return r.Not.Evaluate(req) case r.Match != nil: - return r.Match.Evaluate(headers, query, payload, body, remoteAddr) + return r.Match.Evaluate(req) } return false, nil @@ -791,11 +820,11 @@ func (r Rules) Evaluate(headers, query, payload *map[string]interface{}, body *[ type AndRule []Rules // Evaluate AndRule will return true if and only if all of ChildRules evaluate to true -func (r AndRule) Evaluate(headers, query, payload *map[string]interface{}, body *[]byte, remoteAddr string) (bool, error) { +func (r AndRule) Evaluate(req *Request) (bool, error) { res := true for _, v := range r { - rv, err := v.Evaluate(headers, query, payload, body, remoteAddr) + rv, err := v.Evaluate(req) if err != nil { return false, err } @@ -813,11 +842,11 @@ func (r AndRule) Evaluate(headers, query, payload *map[string]interface{}, body type OrRule []Rules // Evaluate OrRule will return true if any of ChildRules evaluate to true -func (r OrRule) Evaluate(headers, query, payload *map[string]interface{}, body *[]byte, remoteAddr string) (bool, error) { +func (r OrRule) Evaluate(req *Request) (bool, error) { res := false for _, v := range r { - rv, err := v.Evaluate(headers, query, payload, body, remoteAddr) + rv, err := v.Evaluate(req) if err != nil { return false, err } @@ -835,8 +864,8 @@ func (r OrRule) Evaluate(headers, query, payload *map[string]interface{}, body * type NotRule Rules // Evaluate NotRule will return true if and only if ChildRule evaluates to false -func (r NotRule) Evaluate(headers, query, payload *map[string]interface{}, body *[]byte, remoteAddr string) (bool, error) { - rv, err := Rules(r).Evaluate(headers, query, payload, body, remoteAddr) +func (r NotRule) Evaluate(req *Request) (bool, error) { + rv, err := Rules(r).Evaluate(req) return !rv, err } @@ -862,15 +891,15 @@ const ( ) // Evaluate MatchRule will return based on the type -func (r MatchRule) Evaluate(headers, query, payload *map[string]interface{}, body *[]byte, remoteAddr string) (bool, error) { +func (r MatchRule) Evaluate(req *Request) (bool, error) { if r.Type == IPWhitelist { - return CheckIPWhitelist(remoteAddr, r.IPRange) + return CheckIPWhitelist(req.RawRequest.RemoteAddr, r.IPRange) } if r.Type == ScalrSignature { - return CheckScalrSignature(*headers, *body, r.Secret, true) + return CheckScalrSignature(req, r.Secret, true) } - arg, err := r.Parameter.Get(headers, query, payload) + arg, err := r.Parameter.Get(req) if err == nil { switch r.Type { case MatchValue: @@ -878,13 +907,13 @@ func (r MatchRule) Evaluate(headers, query, payload *map[string]interface{}, bod case MatchRegex: return regexp.MatchString(r.Regex, arg) case MatchHashSHA1: - _, err := CheckPayloadSignature(*body, r.Secret, arg) + _, err := CheckPayloadSignature(req.Body, r.Secret, arg) return err == nil, err case MatchHashSHA256: - _, err := CheckPayloadSignature256(*body, r.Secret, arg) + _, err := CheckPayloadSignature256(req.Body, r.Secret, arg) return err == nil, err case MatchHashSHA512: - _, err := CheckPayloadSignature512(*body, r.Secret, arg) + _, 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 index 46bd444..c99edfb 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -1,6 +1,7 @@ package hook import ( + "net/http" "os" "reflect" "strings" @@ -136,7 +137,7 @@ func TestCheckPayloadSignature512(t *testing.T) { var checkScalrSignatureTests = []struct { description string headers map[string]interface{} - payload []byte + body []byte secret string expectedSignature string ok bool @@ -175,7 +176,11 @@ var checkScalrSignatureTests = []struct { func TestCheckScalrSignature(t *testing.T) { for _, testCase := range checkScalrSignatureTests { - valid, err := CheckScalrSignature(testCase.headers, testCase.payload, testCase.secret, false) + r := &Request{ + Headers: testCase.headers, + Body: testCase.body, + } + valid, err := CheckScalrSignature(r, testCase.secret, false) if valid != testCase.ok { t.Errorf("failed to check scalr signature fot test case: %s\nexpected ok:%#v, got ok:%#v}", testCase.description, testCase.ok, valid) @@ -249,25 +254,30 @@ func TestExtractParameter(t *testing.T) { var argumentGetTests = []struct { source, name string - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} value string ok bool }{ - {"header", "a", &map[string]interface{}{"A": "z"}, nil, nil, "z", true}, - {"url", "a", nil, &map[string]interface{}{"a": "z"}, nil, "z", true}, - {"payload", "a", nil, nil, &map[string]interface{}{"a": "z"}, "z", true}, - {"string", "a", nil, nil, &map[string]interface{}{"a": "z"}, "a", true}, + {"header", "a", map[string]interface{}{"A": "z"}, nil, nil, "z", true}, + {"url", "a", nil, map[string]interface{}{"a": "z"}, nil, "z", true}, + {"payload", "a", nil, nil, map[string]interface{}{"a": "z"}, "z", true}, + {"string", "a", nil, nil, map[string]interface{}{"a": "z"}, "a", true}, // failures - {"header", "a", nil, &map[string]interface{}{"a": "z"}, &map[string]interface{}{"a": "z"}, "", false}, // nil headers - {"url", "a", &map[string]interface{}{"A": "z"}, nil, &map[string]interface{}{"a": "z"}, "", false}, // nil query - {"payload", "a", &map[string]interface{}{"A": "z"}, &map[string]interface{}{"a": "z"}, nil, "", false}, // nil payload - {"foo", "a", &map[string]interface{}{"A": "z"}, nil, nil, "", false}, // invalid source + {"header", "a", nil, map[string]interface{}{"a": "z"}, map[string]interface{}{"a": "z"}, "", false}, // nil headers + {"url", "a", map[string]interface{}{"A": "z"}, nil, map[string]interface{}{"a": "z"}, "", false}, // nil query + {"payload", "a", map[string]interface{}{"A": "z"}, map[string]interface{}{"a": "z"}, nil, "", false}, // nil payload + {"foo", "a", map[string]interface{}{"A": "z"}, nil, nil, "", false}, // invalid source } func TestArgumentGet(t *testing.T) { for _, tt := range argumentGetTests { a := Argument{tt.source, tt.name, "", false} - value, err := a.Get(tt.headers, tt.query, tt.payload) + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + } + value, err := a.Get(r) if (err == nil) != tt.ok || value != tt.value { t.Errorf("failed to get {%q, %q}:\nexpected {value:%#v, ok:%#v},\ngot {value:%#v, err:%v}", tt.source, tt.name, tt.value, tt.ok, value, err) } @@ -276,26 +286,31 @@ func TestArgumentGet(t *testing.T) { var hookParseJSONParametersTests = []struct { params []Argument - headers, query, payload *map[string]interface{} - rheaders, rquery, rpayload *map[string]interface{} + headers, query, payload map[string]interface{} + rheaders, rquery, rpayload map[string]interface{} ok bool }{ - {[]Argument{Argument{"header", "a", "", false}}, &map[string]interface{}{"A": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"A": map[string]interface{}{"b": "y"}}, nil, nil, true}, - {[]Argument{Argument{"url", "a", "", false}}, nil, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, nil, true}, - {[]Argument{Argument{"payload", "a", "", false}}, nil, nil, &map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, &map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, true}, - {[]Argument{Argument{"header", "z", "", false}}, &map[string]interface{}{"Z": `{}`}, nil, nil, &map[string]interface{}{"Z": map[string]interface{}{}}, nil, nil, true}, + {[]Argument{Argument{"header", "a", "", false}}, map[string]interface{}{"A": `{"b": "y"}`}, nil, nil, map[string]interface{}{"A": map[string]interface{}{"b": "y"}}, nil, nil, true}, + {[]Argument{Argument{"url", "a", "", false}}, nil, map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, nil, true}, + {[]Argument{Argument{"payload", "a", "", false}}, nil, nil, map[string]interface{}{"a": `{"b": "y"}`}, nil, nil, map[string]interface{}{"a": map[string]interface{}{"b": "y"}}, true}, + {[]Argument{Argument{"header", "z", "", false}}, map[string]interface{}{"Z": `{}`}, nil, nil, map[string]interface{}{"Z": map[string]interface{}{}}, nil, nil, true}, // failures - {[]Argument{Argument{"header", "z", "", false}}, &map[string]interface{}{"Z": ``}, nil, nil, &map[string]interface{}{"Z": ``}, nil, nil, false}, // empty string - {[]Argument{Argument{"header", "y", "", false}}, &map[string]interface{}{"X": `{}`}, nil, nil, &map[string]interface{}{"X": `{}`}, nil, nil, false}, // missing parameter - {[]Argument{Argument{"string", "z", "", false}}, &map[string]interface{}{"Z": ``}, nil, nil, &map[string]interface{}{"Z": ``}, nil, nil, false}, // invalid argument source + {[]Argument{Argument{"header", "z", "", false}}, map[string]interface{}{"Z": ``}, nil, nil, map[string]interface{}{"Z": ``}, nil, nil, false}, // empty string + {[]Argument{Argument{"header", "y", "", false}}, map[string]interface{}{"X": `{}`}, nil, nil, map[string]interface{}{"X": `{}`}, nil, nil, false}, // missing parameter + {[]Argument{Argument{"string", "z", "", false}}, map[string]interface{}{"Z": ``}, nil, nil, map[string]interface{}{"Z": ``}, nil, nil, false}, // invalid argument source } func TestHookParseJSONParameters(t *testing.T) { for _, tt := range hookParseJSONParametersTests { h := &Hook{JSONStringParameters: tt.params} - err := h.ParseJSONParameters(tt.headers, tt.query, tt.payload) + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + } + err := h.ParseJSONParameters(r) if (err == nil) != tt.ok || !reflect.DeepEqual(tt.headers, tt.rheaders) { - t.Errorf("failed to parse %v:\nexpected %#v, ok: %v\ngot %#v, ok: %v", tt.params, *tt.rheaders, tt.ok, *tt.headers, (err == nil)) + t.Errorf("failed to parse %v:\nexpected %#v, ok: %v\ngot %#v, ok: %v", tt.params, tt.rheaders, tt.ok, tt.headers, (err == nil)) } } } @@ -303,19 +318,24 @@ func TestHookParseJSONParameters(t *testing.T) { var hookExtractCommandArgumentsTests = []struct { exec string args []Argument - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} value []string ok bool }{ - {"test", []Argument{Argument{"header", "a", "", false}}, &map[string]interface{}{"A": "z"}, nil, nil, []string{"test", "z"}, true}, + {"test", []Argument{Argument{"header", "a", "", false}}, map[string]interface{}{"A": "z"}, nil, nil, []string{"test", "z"}, true}, // failures - {"fail", []Argument{Argument{"payload", "a", "", false}}, &map[string]interface{}{"A": "z"}, nil, nil, []string{"fail", ""}, false}, + {"fail", []Argument{Argument{"payload", "a", "", false}}, map[string]interface{}{"A": "z"}, nil, nil, []string{"fail", ""}, false}, } func TestHookExtractCommandArguments(t *testing.T) { for _, tt := range hookExtractCommandArgumentsTests { h := &Hook{ExecuteCommand: tt.exec, PassArgumentsToCommand: tt.args} - value, err := h.ExtractCommandArguments(tt.headers, tt.query, tt.payload) + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + } + value, err := h.ExtractCommandArguments(r) if (err == nil) != tt.ok || !reflect.DeepEqual(value, tt.value) { t.Errorf("failed to extract args {cmd=%q, args=%v}:\nexpected %#v, ok: %v\ngot %#v, ok: %v", tt.exec, tt.args, tt.value, tt.ok, value, (err == nil)) } @@ -344,7 +364,7 @@ func TestHookExtractCommandArguments(t *testing.T) { var hookExtractCommandArgumentsForEnvTests = []struct { exec string args []Argument - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} value []string ok bool }{ @@ -352,14 +372,14 @@ var hookExtractCommandArgumentsForEnvTests = []struct { { "test", []Argument{Argument{"header", "a", "", false}}, - &map[string]interface{}{"A": "z"}, nil, nil, + map[string]interface{}{"A": "z"}, nil, nil, []string{"HOOK_a=z"}, true, }, { "test", []Argument{Argument{"header", "a", "MYKEY", false}}, - &map[string]interface{}{"A": "z"}, nil, nil, + map[string]interface{}{"A": "z"}, nil, nil, []string{"MYKEY=z"}, true, }, @@ -367,7 +387,7 @@ var hookExtractCommandArgumentsForEnvTests = []struct { { "fail", []Argument{Argument{"payload", "a", "", false}}, - &map[string]interface{}{"A": "z"}, nil, nil, + map[string]interface{}{"A": "z"}, nil, nil, []string{}, false, }, @@ -376,7 +396,12 @@ var hookExtractCommandArgumentsForEnvTests = []struct { func TestHookExtractCommandArgumentsForEnv(t *testing.T) { for _, tt := range hookExtractCommandArgumentsForEnvTests { h := &Hook{ExecuteCommand: tt.exec, PassEnvironmentToCommand: tt.args} - value, err := h.ExtractCommandArgumentsForEnv(tt.headers, tt.query, tt.payload) + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + } + value, err := h.ExtractCommandArgumentsForEnv(r) if (err == nil) != tt.ok || !reflect.DeepEqual(value, tt.value) { t.Errorf("failed to extract args for env {cmd=%q, args=%v}:\nexpected %#v, ok: %v\ngot %#v, ok: %v", tt.exec, tt.args, tt.value, tt.ok, value, (err == nil)) } @@ -454,24 +479,24 @@ func TestHooksMatch(t *testing.T) { var matchRuleTests = []struct { typ, regex, secret, value, ipRange string param Argument - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} body []byte remoteAddr string ok bool err bool }{ - {"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-hash-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, &map[string]interface{}{"A": "b17e04cbb22afa8ffbff8796fc1894ed27badd9e"}, 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}, + {"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-hash-sha1", "", "secret", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "b17e04cbb22afa8ffbff8796fc1894ed27badd9e"}, 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 + {"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-hash-sha1", "", "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 + {"regex", "*", "", "", "", Argument{"header", "a", "", false}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, "", false, true}, // invalid regex + {"payload-hash-sha1", "", "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 // 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 @@ -490,7 +515,16 @@ var matchRuleTests = []struct { func TestMatchRule(t *testing.T) { for i, tt := range matchRuleTests { r := MatchRule{tt.typ, tt.regex, tt.secret, tt.value, tt.param, tt.ipRange} - ok, err := r.Evaluate(tt.headers, tt.query, tt.payload, &tt.body, tt.remoteAddr) + req := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + Body: tt.body, + RawRequest: &http.Request{ + RemoteAddr: tt.remoteAddr, + }, + } + ok, err := r.Evaluate(req) if ok != tt.ok || (err != nil) != tt.err { t.Errorf("%d failed to match %#v:\nexpected ok: %#v, err: %v\ngot ok: %#v, err: %v", i, r, tt.ok, tt.err, ok, err) } @@ -500,7 +534,7 @@ func TestMatchRule(t *testing.T) { var andRuleTests = []struct { desc string // description of the test case rule AndRule - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} body []byte ok bool err bool @@ -511,7 +545,7 @@ var andRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "z", "B": "y"}, nil, nil, + map[string]interface{}{"A": "z", "B": "y"}, nil, nil, []byte{}, true, false, }, @@ -521,7 +555,7 @@ var andRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "z", "B": "Y"}, nil, nil, + map[string]interface{}{"A": "z", "B": "Y"}, nil, nil, []byte{}, false, false, }, @@ -548,7 +582,7 @@ var andRuleTests = []struct { }, }, }, - &map[string]interface{}{"A": "z", "B": "y", "C": "x", "D": "w", "E": "X", "F": "X"}, nil, nil, + map[string]interface{}{"A": "z", "B": "y", "C": "x", "D": "w", "E": "X", "F": "X"}, nil, nil, []byte{}, true, false, }, @@ -557,14 +591,20 @@ var andRuleTests = []struct { { "invalid rule", AndRule{{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a", "", false}, ""}}}, - &map[string]interface{}{"Y": "z"}, nil, nil, nil, + map[string]interface{}{"Y": "z"}, nil, nil, nil, false, true, }, } func TestAndRule(t *testing.T) { for _, tt := range andRuleTests { - ok, err := tt.rule.Evaluate(tt.headers, tt.query, tt.payload, &tt.body, "") + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + Body: tt.body, + } + ok, err := tt.rule.Evaluate(r) if ok != tt.ok || (err != nil) != tt.err { t.Errorf("failed to match %#v:\nexpected ok: %#v, err: %v\ngot ok: %#v, err: %v", tt.desc, tt.ok, tt.err, ok, err) } @@ -574,7 +614,7 @@ func TestAndRule(t *testing.T) { var orRuleTests = []struct { desc string // description of the test case rule OrRule - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} body []byte ok bool err bool @@ -585,7 +625,7 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "z", "B": "X"}, nil, nil, + map[string]interface{}{"A": "z", "B": "X"}, nil, nil, []byte{}, true, false, }, @@ -595,7 +635,7 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "X", "B": "y"}, nil, nil, + map[string]interface{}{"A": "X", "B": "y"}, nil, nil, []byte{}, true, false, }, @@ -605,7 +645,7 @@ var orRuleTests = []struct { {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, {Match: &MatchRule{"value", "", "", "y", Argument{"header", "b", "", false}, ""}}, }, - &map[string]interface{}{"A": "Z", "B": "Y"}, nil, nil, + map[string]interface{}{"A": "Z", "B": "Y"}, nil, nil, []byte{}, false, false, }, @@ -615,7 +655,7 @@ var orRuleTests = []struct { OrRule{ {Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, }, - &map[string]interface{}{"Y": "Z"}, nil, nil, + map[string]interface{}{"Y": "Z"}, nil, nil, []byte{}, false, true, }, @@ -623,7 +663,13 @@ var orRuleTests = []struct { func TestOrRule(t *testing.T) { for _, tt := range orRuleTests { - ok, err := tt.rule.Evaluate(tt.headers, tt.query, tt.payload, &tt.body, "") + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + Body: tt.body, + } + ok, err := tt.rule.Evaluate(r) if ok != tt.ok || (err != nil) != tt.err { t.Errorf("%#v:\nexpected ok: %#v, err: %v\ngot ok: %#v err: %v", tt.desc, tt.ok, tt.err, ok, err) } @@ -633,18 +679,24 @@ func TestOrRule(t *testing.T) { var notRuleTests = []struct { desc string // description of the test case rule NotRule - headers, query, payload *map[string]interface{} + headers, query, payload map[string]interface{} body []byte ok bool err bool }{ - {"(a=z): !a=X", NotRule{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a", "", false}, ""}}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, true, false}, - {"(a=z): !a=z", NotRule{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, &map[string]interface{}{"A": "z"}, nil, nil, []byte{}, false, false}, + {"(a=z): !a=X", NotRule{Match: &MatchRule{"value", "", "", "X", Argument{"header", "a", "", false}, ""}}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, true, false}, + {"(a=z): !a=z", NotRule{Match: &MatchRule{"value", "", "", "z", Argument{"header", "a", "", false}, ""}}, map[string]interface{}{"A": "z"}, nil, nil, []byte{}, false, false}, } func TestNotRule(t *testing.T) { for _, tt := range notRuleTests { - ok, err := tt.rule.Evaluate(tt.headers, tt.query, tt.payload, &tt.body, "") + r := &Request{ + Headers: tt.headers, + Query: tt.query, + Payload: tt.payload, + Body: tt.body, + } + ok, err := tt.rule.Evaluate(r) if ok != tt.ok || (err != nil) != tt.err { t.Errorf("failed to match %#v:\nexpected ok: %#v, err: %v\ngot ok: %#v, err: %v", tt.rule, tt.ok, tt.err, ok, err) } diff --git a/webhook.go b/webhook.go index 26872ed..f220d4c 100644 --- a/webhook.go +++ b/webhook.go @@ -302,10 +302,14 @@ func main() { } func hookHandler(w http.ResponseWriter, r *http.Request) { - rid := middleware.GetReqID(r.Context()) + req := &hook.Request{ + ID: middleware.GetReqID(r.Context()), + RawRequest: r, + } - log.Printf("[%s] incoming HTTP %s request from %s\n", rid, r.Method, r.RemoteAddr) + log.Printf("[%s] incoming HTTP %s request from %s\n", req.ID, r.Method, r.RemoteAddr) + // TODO: rename this to avoid confusion with Request.ID id := mux.Vars(r)["id"] matchedHook := matchLoadedHook(id) @@ -341,57 +345,54 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { if !allowedMethod { w.WriteHeader(http.StatusMethodNotAllowed) - log.Printf("[%s] HTTP %s method not allowed for hook %q", rid, r.Method, id) + log.Printf("[%s] HTTP %s method not allowed for hook %q", req.ID, r.Method, id) return } - log.Printf("[%s] %s got matched\n", rid, id) + log.Printf("[%s] %s got matched\n", req.ID, id) for _, responseHeader := range responseHeaders { w.Header().Set(responseHeader.Name, responseHeader.Value) } - var ( - body []byte - err error - ) + var err error // set contentType to IncomingPayloadContentType or header value - contentType := r.Header.Get("Content-Type") + req.ContentType = r.Header.Get("Content-Type") if len(matchedHook.IncomingPayloadContentType) != 0 { - contentType = matchedHook.IncomingPayloadContentType + req.ContentType = matchedHook.IncomingPayloadContentType } - isMultipart := strings.HasPrefix(contentType, "multipart/form-data;") + isMultipart := strings.HasPrefix(req.ContentType, "multipart/form-data;") if !isMultipart { - body, err = ioutil.ReadAll(r.Body) + req.Body, err = ioutil.ReadAll(r.Body) if err != nil { - log.Printf("[%s] error reading the request body: %+v\n", rid, err) + log.Printf("[%s] error reading the request body: %+v\n", req.ID, err) } } // parse headers - headers := valuesToMap(r.Header) + req.Headers = valuesToMap(r.Header) // parse query variables - query := valuesToMap(r.URL.Query()) + req.Query = valuesToMap(r.URL.Query()) // parse body - var payload map[string]interface{} + // var payload map[string]interface{} switch { - case strings.Contains(contentType, "json"): - decoder := json.NewDecoder(bytes.NewReader(body)) + case strings.Contains(req.ContentType, "json"): + decoder := json.NewDecoder(bytes.NewReader(req.Body)) decoder.UseNumber() var firstChar byte - for i := 0; i < len(body); i++ { - if unicode.IsSpace(rune(body[i])) { + for i := 0; i < len(req.Body); i++ { + if unicode.IsSpace(rune(req.Body[i])) { continue } - firstChar = body[i] + firstChar = req.Body[i] break } @@ -399,36 +400,36 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { var arrayPayload interface{} err := decoder.Decode(&arrayPayload) if err != nil { - log.Printf("[%s] error parsing JSON array payload %+v\n", rid, err) + log.Printf("[%s] error parsing JSON array payload %+v\n", req.ID, err) } - payload = make(map[string]interface{}, 1) - payload["root"] = arrayPayload + req.Payload = make(map[string]interface{}, 1) + req.Payload["root"] = arrayPayload } else { - err := decoder.Decode(&payload) + err := decoder.Decode(&req.Payload) if err != nil { - log.Printf("[%s] error parsing JSON payload %+v\n", rid, err) + log.Printf("[%s] error parsing JSON payload %+v\n", req.ID, err) } } - case strings.Contains(contentType, "x-www-form-urlencoded"): - fd, err := url.ParseQuery(string(body)) + case strings.Contains(req.ContentType, "x-www-form-urlencoded"): + fd, err := url.ParseQuery(string(req.Body)) if err != nil { - log.Printf("[%s] error parsing form payload %+v\n", rid, err) + log.Printf("[%s] error parsing form payload %+v\n", req.ID, err) } else { - payload = valuesToMap(fd) + req.Payload = valuesToMap(fd) } - case strings.Contains(contentType, "xml"): - payload, err = mxj.NewMapXmlReader(bytes.NewReader(body)) + case strings.Contains(req.ContentType, "xml"): + req.Payload, err = mxj.NewMapXmlReader(bytes.NewReader(req.Body)) if err != nil { - log.Printf("[%s] error parsing XML payload: %+v\n", rid, err) + log.Printf("[%s] error parsing XML payload: %+v\n", req.ID, err) } case isMultipart: err = r.ParseMultipartForm(*maxMultipartMem) if err != nil { - msg := fmt.Sprintf("[%s] error parsing multipart form: %+v\n", rid, err) + msg := fmt.Sprintf("[%s] error parsing multipart form: %+v\n", req.ID, err) log.Println(msg) w.WriteHeader(http.StatusInternalServerError) fmt.Fprint(w, "Error occurred while parsing multipart form.") @@ -436,14 +437,14 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { } for k, v := range r.MultipartForm.Value { - log.Printf("[%s] found multipart form value %q", rid, k) + log.Printf("[%s] found multipart form value %q", req.ID, k) - if payload == nil { - payload = make(map[string]interface{}) + if req.Payload == nil { + req.Payload = make(map[string]interface{}) } // TODO(moorereason): support duplicate, named values - payload[k] = v[0] + req.Payload[k] = v[0] } for k, v := range r.MultipartForm.File { @@ -472,11 +473,11 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { } if parseAsJSON { - log.Printf("[%s] parsing multipart form file %q as JSON\n", rid, k) + log.Printf("[%s] parsing multipart form file %q as JSON\n", req.ID, k) f, err := v[0].Open() if err != nil { - msg := fmt.Sprintf("[%s] error parsing multipart form file: %+v\n", rid, err) + msg := fmt.Sprintf("[%s] error parsing multipart form file: %+v\n", req.ID, err) log.Println(msg) w.WriteHeader(http.StatusInternalServerError) fmt.Fprint(w, "Error occurred while parsing multipart form file.") @@ -489,24 +490,24 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { var part map[string]interface{} err = decoder.Decode(&part) if err != nil { - log.Printf("[%s] error parsing JSON payload file: %+v\n", rid, err) + log.Printf("[%s] error parsing JSON payload file: %+v\n", req.ID, err) } - if payload == nil { - payload = make(map[string]interface{}) + if req.Payload == nil { + req.Payload = make(map[string]interface{}) } - payload[k] = part + req.Payload[k] = part } } default: - log.Printf("[%s] error parsing body payload due to unsupported content type header: %s\n", rid, contentType) + log.Printf("[%s] error parsing body payload due to unsupported content type header: %s\n", req.ID, req.ContentType) } // handle hook - errors := matchedHook.ParseJSONParameters(&headers, &query, &payload) + errors := matchedHook.ParseJSONParameters(req) for _, err := range errors { - log.Printf("[%s] error parsing JSON parameters: %s\n", rid, err) + log.Printf("[%s] error parsing JSON parameters: %s\n", req.ID, err) } var ok bool @@ -514,29 +515,29 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { if matchedHook.TriggerRule == nil { ok = true } else { - ok, err = matchedHook.TriggerRule.Evaluate(&headers, &query, &payload, &body, r.RemoteAddr) + ok, err = matchedHook.TriggerRule.Evaluate(req) if err != nil { if !hook.IsParameterNodeError(err) { - msg := fmt.Sprintf("[%s] error evaluating hook: %s", rid, err) + msg := fmt.Sprintf("[%s] error evaluating hook: %s", req.ID, err) log.Println(msg) w.WriteHeader(http.StatusInternalServerError) fmt.Fprint(w, "Error occurred while evaluating hook rules.") return } - log.Printf("[%s] %v", rid, err) + log.Printf("[%s] %v", req.ID, err) } } if ok { - log.Printf("[%s] %s hook triggered successfully\n", rid, matchedHook.ID) + log.Printf("[%s] %s hook triggered successfully\n", req.ID, matchedHook.ID) for _, responseHeader := range matchedHook.ResponseHeaders { w.Header().Set(responseHeader.Name, responseHeader.Value) } if matchedHook.CaptureCommandOutput { - response, err := handleHook(matchedHook, rid, &headers, &query, &payload, &body) + response, err := handleHook(matchedHook, req) if err != nil { w.WriteHeader(http.StatusInternalServerError) @@ -549,16 +550,16 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { } else { // Check if a success return code is configured for the hook if matchedHook.SuccessHttpResponseCode != 0 { - writeHttpResponseCode(w, rid, matchedHook.ID, matchedHook.SuccessHttpResponseCode) + writeHttpResponseCode(w, req.ID, matchedHook.ID, matchedHook.SuccessHttpResponseCode) } fmt.Fprint(w, response) } } else { - go handleHook(matchedHook, rid, &headers, &query, &payload, &body) + go handleHook(matchedHook, req) // Check if a success return code is configured for the hook if matchedHook.SuccessHttpResponseCode != 0 { - writeHttpResponseCode(w, rid, matchedHook.ID, matchedHook.SuccessHttpResponseCode) + writeHttpResponseCode(w, req.ID, matchedHook.ID, matchedHook.SuccessHttpResponseCode) } fmt.Fprint(w, matchedHook.ResponseMessage) @@ -568,16 +569,16 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { // Check if a return code is configured for the hook if matchedHook.TriggerRuleMismatchHttpResponseCode != 0 { - writeHttpResponseCode(w, rid, matchedHook.ID, matchedHook.TriggerRuleMismatchHttpResponseCode) + writeHttpResponseCode(w, req.ID, matchedHook.ID, matchedHook.TriggerRuleMismatchHttpResponseCode) } // if none of the hooks got triggered - log.Printf("[%s] %s got matched, but didn't get triggered because the trigger rules were not satisfied\n", rid, matchedHook.ID) + log.Printf("[%s] %s got matched, but didn't get triggered because the trigger rules were not satisfied\n", req.ID, matchedHook.ID) fmt.Fprint(w, "Hook rules were not satisfied.") } -func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]interface{}, body *[]byte) (string, error) { +func handleHook(h *hook.Hook, r *hook.Request) (string, error) { var errors []error // check the command exists @@ -590,12 +591,12 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in cmdPath, err := exec.LookPath(lookpath) if err != nil { - log.Printf("[%s] error in %s", rid, err) + log.Printf("[%s] error in %s", r.ID, err) // check if parameters specified in execute-command by mistake if strings.IndexByte(h.ExecuteCommand, ' ') != -1 { s := strings.Fields(h.ExecuteCommand)[0] - log.Printf("[%s] use 'pass-arguments-to-command' to specify args for '%s'", rid, s) + log.Printf("[%s] use 'pass-arguments-to-command' to specify args for '%s'", r.ID, s) } return "", err @@ -604,37 +605,37 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in cmd := exec.Command(cmdPath) cmd.Dir = h.CommandWorkingDirectory - cmd.Args, errors = h.ExtractCommandArguments(headers, query, payload) + cmd.Args, errors = h.ExtractCommandArguments(r) for _, err := range errors { - log.Printf("[%s] error extracting command arguments: %s\n", rid, err) + log.Printf("[%s] error extracting command arguments: %s\n", r.ID, err) } var envs []string - envs, errors = h.ExtractCommandArgumentsForEnv(headers, query, payload) + envs, errors = h.ExtractCommandArgumentsForEnv(r) for _, err := range errors { - log.Printf("[%s] error extracting command arguments for environment: %s\n", rid, err) + log.Printf("[%s] error extracting command arguments for environment: %s\n", r.ID, err) } - files, errors := h.ExtractCommandArgumentsForFile(headers, query, payload) + files, errors := h.ExtractCommandArgumentsForFile(r) for _, err := range errors { - log.Printf("[%s] error extracting command arguments for file: %s\n", rid, err) + log.Printf("[%s] error extracting command arguments for file: %s\n", r.ID, err) } for i := range files { tmpfile, err := ioutil.TempFile(h.CommandWorkingDirectory, files[i].EnvName) if err != nil { - log.Printf("[%s] error creating temp file [%s]", rid, err) + log.Printf("[%s] error creating temp file [%s]", r.ID, err) continue } - log.Printf("[%s] writing env %s file %s", rid, files[i].EnvName, tmpfile.Name()) + log.Printf("[%s] writing env %s file %s", r.ID, files[i].EnvName, tmpfile.Name()) if _, err := tmpfile.Write(files[i].Data); err != nil { - log.Printf("[%s] error writing file %s [%s]", rid, tmpfile.Name(), err) + log.Printf("[%s] error writing file %s [%s]", r.ID, tmpfile.Name(), err) continue } if err := tmpfile.Close(); err != nil { - log.Printf("[%s] error closing file %s [%s]", rid, tmpfile.Name(), err) + log.Printf("[%s] error closing file %s [%s]", r.ID, tmpfile.Name(), err) continue } @@ -644,27 +645,27 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in cmd.Env = append(os.Environ(), envs...) - log.Printf("[%s] executing %s (%s) with arguments %q and environment %s using %s as cwd\n", rid, h.ExecuteCommand, cmd.Path, cmd.Args, envs, cmd.Dir) + log.Printf("[%s] executing %s (%s) with arguments %q and environment %s using %s as cwd\n", r.ID, h.ExecuteCommand, cmd.Path, cmd.Args, envs, cmd.Dir) out, err := cmd.CombinedOutput() - log.Printf("[%s] command output: %s\n", rid, out) + log.Printf("[%s] command output: %s\n", r.ID, out) if err != nil { - log.Printf("[%s] error occurred: %+v\n", rid, err) + log.Printf("[%s] error occurred: %+v\n", r.ID, err) } for i := range files { if files[i].File != nil { - log.Printf("[%s] removing file %s\n", rid, files[i].File.Name()) + log.Printf("[%s] removing file %s\n", r.ID, files[i].File.Name()) err := os.Remove(files[i].File.Name()) if err != nil { - log.Printf("[%s] error removing file %s [%s]", rid, files[i].File.Name(), err) + log.Printf("[%s] error removing file %s [%s]", r.ID, files[i].File.Name(), err) } } } - log.Printf("[%s] finished handling %s\n", rid, h.ID) + log.Printf("[%s] finished handling %s\n", r.ID, h.ID) return string(out), err } diff --git a/webhook_test.go b/webhook_test.go index 353d22d..c8a46cc 100644 --- a/webhook_test.go +++ b/webhook_test.go @@ -53,7 +53,11 @@ func TestStaticParams(t *testing.T) { b := &bytes.Buffer{} log.SetOutput(b) - _, err = handleHook(spHook, "test", &spHeaders, &map[string]interface{}{}, &map[string]interface{}{}, &[]byte{}) + r := &hook.Request{ + ID: "test", + Headers: spHeaders, + } + _, err = handleHook(spHook, r) if err != nil { t.Fatalf("Unexpected error: %v\n", err) } From 6bbf14f7d9fc0f354ef0a313e76e69e93f83a6d2 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Thu, 24 Sep 2020 21:37:49 -0500 Subject: [PATCH 24/28] Transition payload hash option names to hmac The payload-hash-* options are imprecisely named. Clarify their function as HMAC validations by renaming them. The existing options will continue to work but are deprecated. Log a warning if the old options are used. All tests, examples, and documentation are updated. Fixes #289 --- docs/Hook-Examples.md | 4 ++-- docs/Hook-Rules.md | 33 +++++++++++++++++---------------- docs/Templates.md | 4 ++-- hooks.json.example | 2 +- hooks.json.tmpl.example | 2 +- hooks.yaml.example | 2 +- hooks.yaml.tmpl.example | 2 +- internal/hook/hook.go | 12 ++++++++++++ internal/hook/hook_test.go | 6 ++++++ test/hooks.json.tmpl | 4 ++-- test/hooks.yaml.tmpl | 4 ++-- 11 files changed, 47 insertions(+), 28 deletions(-) diff --git a/docs/Hook-Examples.md b/docs/Hook-Examples.md index e02eee7..7ca2600 100644 --- 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 index 0ff30b3..2e5550b 100644 --- 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 ```json { @@ -185,12 +183,13 @@ For the regex syntax, check out } ``` -### 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 index 2adeab3..b7cb972 100644 --- 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 index 00f6ff3..90cd275 100644 --- 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 index 70e05c8..5c00643 100644 --- 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 index 74713e0..ba9d0b7 100644 --- 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 index 2bcfbd6..672c8d0 100644 --- 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 index e6a9272..157c457 100644 --- 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 index c99edfb..32bc848 100644 --- a/internal/hook/hook_test.go +++ b/internal/hook/hook_test.go @@ -487,7 +487,9 @@ 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}, @@ -495,8 +497,12 @@ var matchRuleTests = []struct { {"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 index 7fe7c8c..3075762 100644 --- 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 index ace4277..16aa8c1 100644 --- 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 From d2795059303521cac3fdd0a1c4feeec406512514 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Mon, 28 Sep 2020 14:23:56 -0500 Subject: [PATCH 25/28] Document YAML support Fixes #400 Updates #288 --- README.md | 13 +++++++++++-- docs/Hook-Definition.md | 3 ++- docs/Hook-Examples.md | 8 ++++++-- docs/Templates.md | 6 +++--- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/README.md b/README.md index 1706b79..001835c 100644 --- a/README.md +++ b/README.md @@ -46,7 +46,9 @@ If you are using Debian linux ("stretch" or later), you can install webhook usin Prebuilt binaries for different architectures are available at [GitHub Releases](https://github.com/adnanh/webhook/releases). ## Configuration -Next step is to define some hooks you want [webhook][w] to serve. Begin by creating an empty file named `hooks.json`. This file will contain an array of hooks the [webhook][w] will serve. Check [Hook definition page](docs/Hook-Definition.md) to see the detailed description of what properties a hook can contain, and how to use them. +Next step is to define some hooks you want [webhook][w] to serve. +[webhook][w] supports JSON or YAML configuration files, but we'll focus primarily on JSON in the following example. +Begin by creating an empty file named `hooks.json`. This file will contain an array of hooks the [webhook][w] will serve. Check [Hook definition page](docs/Hook-Definition.md) to see the detailed description of what properties a hook can contain, and how to use them. Let's define a simple hook named `redeploy-webhook` that will run a redeploy script located in `/var/scripts/redeploy.sh`. Make sure that your bash script has `#!/bin/sh` shebang on top. @@ -61,6 +63,13 @@ Our `hooks.json` file will now look like this: ] ``` +**NOTE:** If you prefer YAML, the equivalent `hooks.yaml` file would be: +```yaml +- id: redeploy-webhook + execute-command: "/var/scripts/redeploy.sh" + command-working-directory: "/var/webhook" +``` + You can now run [webhook][w] using ```bash $ /path/to/webhook -hooks hooks.json -verbose @@ -90,7 +99,7 @@ All files are ignored unless they match one of the following criteria: In either case, the given file part will be parsed as JSON and added to the `payload` map. ## Templates -[webhook][w] can parse the `hooks.json` input file as a Go template when given the `-template` [CLI parameter](docs/Webhook-Parameters.md). See the [Templates page](docs/Templates.md) for more details on template usage. +[webhook][w] can parse the hooks configuration file as a Go template when given the `-template` [CLI parameter](docs/Webhook-Parameters.md). See the [Templates page](docs/Templates.md) for more details on template usage. ## Using HTTPS [webhook][w] by default serves hooks using http. If you want [webhook][w] to serve secure content using https, you can use the `-secure` flag while starting [webhook][w]. Files containing a certificate and matching private key for the server must be provided using the `-cert /path/to/cert.pem` and `-key /path/to/key.pem` flags. If the certificate is signed by a certificate authority, the cert file should be the concatenation of the server's certificate followed by the CA's certificate. diff --git a/docs/Hook-Definition.md b/docs/Hook-Definition.md index 7c54a20..925bea1 100644 --- a/docs/Hook-Definition.md +++ b/docs/Hook-Definition.md @@ -1,5 +1,6 @@ # Hook definition -Hooks are defined as JSON objects. Please note that in order to be considered valid, a hook object must contain the `id` and `execute-command` properties. All other properties are considered optional. + +Hooks are defined as objects in the JSON or YAML hooks configuration file. Please note that in order to be considered valid, a hook object must contain the `id` and `execute-command` properties. All other properties are considered optional. ## Properties (keys) diff --git a/docs/Hook-Examples.md b/docs/Hook-Examples.md index 7ca2600..ba581eb 100644 --- a/docs/Hook-Examples.md +++ b/docs/Hook-Examples.md @@ -1,5 +1,9 @@ -# Hook examples -This page is still work in progress. Feel free to contribute! +# Hook Examples + +Hooks are defined in a hooks configuration file in either JSON or YAML format, +although the examples on this page all use the JSON format. + +🌱 This page is still a work in progress. Feel free to contribute! ### Table of Contents diff --git a/docs/Templates.md b/docs/Templates.md index b7cb972..12798f4 100644 --- a/docs/Templates.md +++ b/docs/Templates.md @@ -1,12 +1,12 @@ # Templates in Webhook -[`webhook`][w] can parse the `hooks.json` input file as a Go template when given the `-template` [CLI parameter](Webhook-Parameters.md). +[`webhook`][w] can parse a hooks configuration file as a Go template when given the `-template` [CLI parameter](Webhook-Parameters.md). -In additional to the [built-in Go template functions and features][tt], `webhook` provides a `getenv` template function for inserting environment variables into a `hooks.json` file. +In additional to the [built-in Go template functions and features][tt], `webhook` provides a `getenv` template function for inserting environment variables into a templated configuration file. ## Example Usage -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. +In the example JSON template file below (YAML is also supported), 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. ``` From 0814b10a16f8e53280d9e2138ae3a7b95381514a Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Mon, 28 Sep 2020 17:20:22 -0500 Subject: [PATCH 26/28] Add Github Action to build & run tests --- .github/workflows/build.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .github/workflows/build.yml diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml new file mode 100644 index 0000000..af26c40 --- /dev/null +++ b/.github/workflows/build.yml @@ -0,0 +1,29 @@ +name: build +on: [push, pull_request] +jobs: + build: + strategy: + matrix: + go-version: [1.14.x, 1.15.x] + os: [ubuntu-latest, macos-latest, windows-latest] + + runs-on: ${{ matrix.os }} + + steps: + - name: Setup Go + uses: actions/setup-go@v2 + with: + go-version: ${{ matrix.go-version }} + id: go + + - name: Checkout code + uses: actions/checkout@v2 + + - name: Download dependencies + run: go mod download + + - name: Build + run: go build -v + + - name: Test + run: go test -v ./... From a904537367e0c7d36976e723f1c1a596c3498670 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Mon, 28 Sep 2020 20:41:24 -0500 Subject: [PATCH 27/28] Add build badge to README.md --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 1706b79..35dfce9 100644 --- a/README.md +++ b/README.md @@ -1,4 +1,4 @@ -# What is webhook? +# What is webhook? ![build-status][badge] Webhook @@ -202,3 +202,4 @@ THE SOFTWARE. [w]: https://github.com/adnanh/webhook [wc]: https://github.com/adnanh/webhook-contrib +[badge]: https://github.com/adnanh/webhook/workflows/build/badge.svg From f007fa52809edc645cddb27ce688ce73f58aad92 Mon Sep 17 00:00:00 2001 From: Cameron Moore Date: Mon, 28 Sep 2020 20:51:09 -0500 Subject: [PATCH 28/28] Simplify build workflow --- .github/workflows/build.yml | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index af26c40..75f347d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -10,18 +10,12 @@ jobs: runs-on: ${{ matrix.os }} steps: - - name: Setup Go - uses: actions/setup-go@v2 + - uses: actions/checkout@v2 + - uses: actions/setup-go@v2 with: go-version: ${{ matrix.go-version }} id: go - - name: Checkout code - uses: actions/checkout@v2 - - - name: Download dependencies - run: go mod download - - name: Build run: go build -v