diff --git a/hook/hook.go b/hook/hook.go index fb1b6fc..5f5b3de 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -11,8 +11,8 @@ import ( "errors" "fmt" "io/ioutil" - "math" "log" + "math" "net" "net/textproto" "os" @@ -94,9 +94,7 @@ func (e *ParseError) Error() string { // CheckPayloadSignature calculates and verifies SHA1 signature of the given payload func CheckPayloadSignature(payload []byte, secret string, signature string) (string, error) { - if strings.HasPrefix(signature, "sha1=") { - signature = signature[5:] - } + signature = strings.TrimPrefix(signature, "sha1=") mac := hmac.New(sha1.New, []byte(secret)) _, err := mac.Write(payload) @@ -113,9 +111,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) { - if strings.HasPrefix(signature, "sha256=") { - signature = signature[7:] - } + signature = strings.TrimPrefix(signature, "sha256=") mac := hmac.New(sha256.New, []byte(secret)) _, err := mac.Write(payload) @@ -129,44 +125,44 @@ func CheckPayloadSignature256(payload []byte, secret string, signature string) ( } return expectedMAC, err } - -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 { - return false, nil - } - if _, ok := headers["Date"]; !ok { - return false, nil - } - providedSignature := headers["X-Signature"].(string) - dateHeader := headers["Date"].(string) - mac := hmac.New(sha1.New, []byte(signingKey)) - mac.Write(body) - mac.Write([]byte(dateHeader)) - expectedSignature := hex.EncodeToString(mac.Sum(nil)) - - if !hmac.Equal([]byte(providedSignature), []byte(expectedSignature)) { - return false, &SignatureError{providedSignature} - } - - if !checkDate { - return true, nil - } - // Example format: Fri 08 Sep 2017 11:24:32 UTC - date, err := time.Parse("Mon 02 Jan 2006 15:04:05 MST", dateHeader) - //date, err := time.Parse(time.RFC1123, dateHeader) - if err != nil { - return false, err - } - now := time.Now() - delta := math.Abs(now.Sub(date).Seconds()) - - if delta > 300 { - return false, &SignatureError{"outdated"} - } - return true, nil - } - + +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 { + return false, nil + } + if _, ok := headers["Date"]; !ok { + return false, nil + } + providedSignature := headers["X-Signature"].(string) + dateHeader := headers["Date"].(string) + mac := hmac.New(sha1.New, []byte(signingKey)) + mac.Write(body) + mac.Write([]byte(dateHeader)) + expectedSignature := hex.EncodeToString(mac.Sum(nil)) + + if !hmac.Equal([]byte(providedSignature), []byte(expectedSignature)) { + return false, &SignatureError{providedSignature} + } + + if !checkDate { + return true, nil + } + // Example format: Fri 08 Sep 2017 11:24:32 UTC + date, err := time.Parse("Mon 02 Jan 2006 15:04:05 MST", dateHeader) + //date, err := time.Parse(time.RFC1123, dateHeader) + if err != nil { + return false, err + } + now := time.Now() + delta := math.Abs(now.Sub(date).Seconds()) + + if delta > 300 { + return false, &SignatureError{"outdated"} + } + return true, nil +} + // 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) { @@ -196,7 +192,7 @@ func CheckIPWhitelist(remoteAddr string, ipRange string) (bool, error) { ipRange = strings.TrimSpace(ipRange) - if strings.Index(ipRange, "/") == -1 { + if !strings.Contains(ipRange, "/") { ipRange = ipRange + "/32" } @@ -687,7 +683,7 @@ func (r AndRule) Evaluate(headers, query, payload *map[string]interface{}, body } res = res && rv - if res == false { + if !res { return res, nil } } @@ -709,7 +705,7 @@ func (r OrRule) Evaluate(headers, query, payload *map[string]interface{}, body * } res = res || rv - if res == true { + if res { return res, nil } } @@ -751,10 +747,10 @@ func (r MatchRule) Evaluate(headers, query, payload *map[string]interface{}, bod if r.Type == IPWhitelist { return CheckIPWhitelist(remoteAddr, r.IPRange) } - if r.Type == ScalrSignature { - return CheckScalrSignature(*headers, *body, r.Secret, true) + if r.Type == ScalrSignature { + return CheckScalrSignature(*headers, *body, r.Secret, true) } - + if arg, ok := r.Parameter.Get(headers, query, payload); ok { switch r.Type { case MatchValue: diff --git a/hook/hook_test.go b/hook/hook_test.go index d1134f1..1794a7c 100644 --- a/hook/hook_test.go +++ b/hook/hook_test.go @@ -60,52 +60,52 @@ func TestCheckPayloadSignature256(t *testing.T) { } } -var checkScalrSignatureTests = []struct { - description string - headers map[string]interface{} - payload []byte - secret string - expectedSignature string - ok bool -}{ - { - "Valid signature", - map[string]interface{}{"Date": "Thu 07 Sep 2017 06:30:04 UTC", "X-Signature": "48e395e38ac48988929167df531eb2da00063a7d"}, - []byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+", - "48e395e38ac48988929167df531eb2da00063a7d", true, - }, - { - "Wrong signature", - map[string]interface{}{"Date": "Thu 07 Sep 2017 06:30:04 UTC", "X-Signature": "999395e38ac48988929167df531eb2da00063a7d"}, - []byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+", - "48e395e38ac48988929167df531eb2da00063a7d", false, - }, - { - "Missing Date header", - map[string]interface{}{"X-Signature": "999395e38ac48988929167df531eb2da00063a7d"}, - []byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+", - "48e395e38ac48988929167df531eb2da00063a7d", false, - }, - { - "Missing X-Signature header", - map[string]interface{}{"Date": "Thu 07 Sep 2017 06:30:04 UTC"}, - []byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+", - "48e395e38ac48988929167df531eb2da00063a7d", false, - }, -} - -func TestCheckScalrSignature(t *testing.T) { - for _, testCase := range checkScalrSignatureTests { - valid, err := CheckScalrSignature(testCase.headers, testCase.payload, 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) - } - - if err != nil && strings.Contains(err.Error(), testCase.expectedSignature) { - t.Errorf("error message should not disclose expected mac: %s on test case %s", err, testCase.description) - } - } +var checkScalrSignatureTests = []struct { + description string + headers map[string]interface{} + payload []byte + secret string + expectedSignature string + ok bool +}{ + { + "Valid signature", + map[string]interface{}{"Date": "Thu 07 Sep 2017 06:30:04 UTC", "X-Signature": "48e395e38ac48988929167df531eb2da00063a7d"}, + []byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+", + "48e395e38ac48988929167df531eb2da00063a7d", true, + }, + { + "Wrong signature", + map[string]interface{}{"Date": "Thu 07 Sep 2017 06:30:04 UTC", "X-Signature": "999395e38ac48988929167df531eb2da00063a7d"}, + []byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+", + "48e395e38ac48988929167df531eb2da00063a7d", false, + }, + { + "Missing Date header", + map[string]interface{}{"X-Signature": "999395e38ac48988929167df531eb2da00063a7d"}, + []byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+", + "48e395e38ac48988929167df531eb2da00063a7d", false, + }, + { + "Missing X-Signature header", + map[string]interface{}{"Date": "Thu 07 Sep 2017 06:30:04 UTC"}, + []byte(`{"a": "b"}`), "bilFGi4ZVZUdG+C6r0NIM9tuRq6PaG33R3eBUVhLwMAErGBaazvXe4Gq2DcJs5q+", + "48e395e38ac48988929167df531eb2da00063a7d", false, + }, +} + +func TestCheckScalrSignature(t *testing.T) { + for _, testCase := range checkScalrSignatureTests { + valid, err := CheckScalrSignature(testCase.headers, testCase.payload, 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) + } + + if err != nil && strings.Contains(err.Error(), testCase.expectedSignature) { + t.Errorf("error message should not disclose expected mac: %s on test case %s", err, testCase.description) + } + } } var extractParameterTests = []struct { diff --git a/test/hookecho.go b/test/hookecho.go index 8e308ff..6e5e9f7 100644 --- a/test/hookecho.go +++ b/test/hookecho.go @@ -5,8 +5,8 @@ package main import ( "fmt" "os" - "strings" "strconv" + "strings" ) func main() { diff --git a/webhook.go b/webhook.go index 71fdc6b..2ca49e0 100644 --- a/webhook.go +++ b/webhook.go @@ -120,7 +120,7 @@ func main() { newHooksFiles := hooksFiles[:0] for _, filePath := range hooksFiles { - if _, ok := loadedHooksFromFiles[filePath]; ok == true { + if _, ok := loadedHooksFromFiles[filePath]; ok { newHooksFiles = append(newHooksFiles, filePath) } } @@ -250,10 +250,9 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { } // handle hook - if errors := matchedHook.ParseJSONParameters(&headers, &query, &payload); errors != nil { - for _, err := range errors { - log.Printf("[%s] error parsing JSON parameters: %s\n", rid, err) - } + errors := matchedHook.ParseJSONParameters(&headers, &query, &payload) + for _, err := range errors { + log.Printf("[%s] error parsing JSON parameters: %s\n", rid, err) } var ok bool @@ -264,7 +263,7 @@ func hookHandler(w http.ResponseWriter, r *http.Request) { ok, err = matchedHook.TriggerRule.Evaluate(&headers, &query, &payload, &body, r.RemoteAddr) if err != nil { msg := fmt.Sprintf("[%s] error evaluating hook: %s", rid, err) - log.Printf(msg) + log.Print(msg) w.WriteHeader(http.StatusInternalServerError) fmt.Fprintf(w, "Error occurred while evaluating hook rules.") return @@ -341,27 +340,21 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in cmd.Dir = h.CommandWorkingDirectory cmd.Args, errors = h.ExtractCommandArguments(headers, query, payload) - if errors != nil { - for _, err := range errors { - log.Printf("[%s] error extracting command arguments: %s\n", rid, err) - } + for _, err := range errors { + log.Printf("[%s] error extracting command arguments: %s\n", rid, err) } var envs []string envs, errors = h.ExtractCommandArgumentsForEnv(headers, query, payload) - if errors != nil { - for _, err := range errors { - log.Printf("[%s] error extracting command arguments for environment: %s\n", rid, err) - } + for _, err := range errors { + log.Printf("[%s] error extracting command arguments for environment: %s\n", rid, err) } files, errors := h.ExtractCommandArgumentsForFile(headers, query, payload) - if errors != nil { - for _, err := range errors { - log.Printf("[%s] error extracting command arguments for file: %s\n", rid, err) - } + for _, err := range errors { + log.Printf("[%s] error extracting command arguments for file: %s\n", rid, err) } for i := range files { @@ -436,7 +429,7 @@ func reloadHooks(hooksFilePath string) { } } - if (matchLoadedHook(hook.ID) != nil && !wasHookIDAlreadyLoaded) || seenHooksIds[hook.ID] == true { + if (matchLoadedHook(hook.ID) != nil && !wasHookIDAlreadyLoaded) || seenHooksIds[hook.ID] { log.Printf("error: hook with the id %s has already been loaded!\nplease check your hooks file for duplicate hooks ids!", hook.ID) log.Println("reverting hooks back to the previous configuration") return