From 11e0031a9f0eeb373f1f3b659946f27b6460f87b Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 15 Nov 2019 15:38:27 -0700 Subject: [PATCH 1/4] feat: added multiple sig support --- internal/hook/hook.go | 73 ++++++++++++++++++++++++++++++++------ internal/hook/hook_test.go | 6 ++++ 2 files changed, 68 insertions(+), 11 deletions(-) diff --git a/internal/hook/hook.go b/internal/hook/hook.go index 1131eb4..09e6946 100644 --- a/internal/hook/hook.go +++ b/internal/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/internal/hook/hook_test.go b/internal/hook/hook_test.go index dfb4874..cccf4de 100644 --- a/internal/hook/hook_test.go +++ b/internal/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 6d3b81fc61095a03e40e2c6fe15897f1c91002ca Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Fri, 15 Nov 2019 15:52:03 -0700 Subject: [PATCH 2/4] fix: simplify implementation --- internal/hook/hook.go | 24 ++++++++---------------- 1 file changed, 8 insertions(+), 16 deletions(-) diff --git a/internal/hook/hook.go b/internal/hook/hook.go index 09e6946..3a52cb2 100644 --- a/internal/hook/hook.go +++ b/internal/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 f8c8932866b7b31027c0c2729e2855a135dd1845 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Mon, 18 Nov 2019 09:31:49 -0700 Subject: [PATCH 3/4] fix: spelling --- internal/hook/hook.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/internal/hook/hook.go b/internal/hook/hook.go index 3a52cb2..65b9c7b 100644 --- a/internal/hook/hook.go +++ b/internal/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 de626ab2bb52097d5708af6ef959bb3cfd180a69 Mon Sep 17 00:00:00 2001 From: Wyatt Johnson Date: Tue, 17 Dec 2019 10:18:08 -0700 Subject: [PATCH 4/4] fix: updated based on review - added support for sha512 - added notes to docs --- docs/Hook-Rules.md | 21 +++++++++++ internal/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/internal/hook/hook.go b/internal/hook/hook.go index 65b9c7b..69df88e 100644 --- a/internal/hook/hook.go +++ b/internal/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) {