Merge pull request #387 from adnanh/revert-355-master

Revert "Multiple Signature Support"

Author: adnanh

docs/Hook-Rules.md

@@ -186,13 +186,6 @@ For the regex syntax, check out <http://golang.org/pkg/regexp/syntax/>
 }
 ```
 
-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`.

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 "<nil>"
 	}
-
-	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)
+// 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")
 	}
 
-	// There were no commas, so just trim the prefix (if it even exists) and
-	// pass it back.
-	return []string{
-		strings.TrimPrefix(source, prefix),
-	}
-}
+	signature = strings.TrimPrefix(signature, "sha1=")
 
-// 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.
+	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,
+	if !hmac.Equal([]byte(signature), []byte(expectedMAC)) {
+		return expectedMAC, &SignatureError{signature}
 	}
-}
-
-// 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=")
-
-	// Validate the MAC.
-	return ValidateMAC(payload, hmac.New(sha1.New, []byte(secret)), signatures)
+	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=")
+
+	mac := hmac.New(sha256.New, []byte(secret))
+	_, err := mac.Write(payload)
+	if err != nil {
+		return "", err
+	}
+	expectedMAC := hex.EncodeToString(mac.Sum(nil))
 
-	// Validate the MAC.
-	return ValidateMAC(payload, hmac.New(sha256.New, []byte(secret)), signatures)
+	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=")
+
+	mac := hmac.New(sha512.New, []byte(secret))
+	_, err := mac.Write(payload)
+	if err != nil {
+		return "", err
+	}
+	expectedMAC := hex.EncodeToString(mac.Sum(nil))
 
-	// Validate the MAC.
-	return ValidateMAC(payload, hmac.New(sha512.New, []byte(secret)), signatures)
+	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
 }

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