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] 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}, }