From b59569465835698a4f9800c923a3f742e9d9c899 Mon Sep 17 00:00:00 2001 From: Hass_SEA Date: Tue, 19 Dec 2017 12:48:10 -0800 Subject: [PATCH 1/3] Update to support Scalr Signature Verification Add a new match rule type that checks for a Scalr webhook signature. Tracking ticket #200 The signature algorithm is described here: https://scalr-wiki.atlassian.net/wiki/spaces/docs/pages/6193247/Webhook+Security+and+Authentication An example match rule ifor a Scalr webhook will look like: "match": { "type": "scalr-signature", "secret": "" } --- hook/hook.go | 47 ++++++++++++++++++++++++++++++++++++++++++++-- hook/hook_test.go | 48 +++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 93 insertions(+), 2 deletions(-) diff --git a/hook/hook.go b/hook/hook.go index 7fd8c1f..fb1b6fc 100644 --- a/hook/hook.go +++ b/hook/hook.go @@ -11,6 +11,7 @@ import ( "errors" "fmt" "io/ioutil" + "math" "log" "net" "net/textproto" @@ -20,6 +21,7 @@ import ( "strconv" "strings" "text/template" + "time" "github.com/ghodss/yaml" ) @@ -127,7 +129,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 + } + // 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) { @@ -704,6 +743,7 @@ const ( MatchHashSHA1 string = "payload-hash-sha1" MatchHashSHA256 string = "payload-hash-sha256" IPWhitelist string = "ip-whitelist" + ScalrSignature string = "scalr-signature" ) // Evaluate MatchRule will return based on the type @@ -711,7 +751,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 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 52f273b..d1134f1 100644 --- a/hook/hook_test.go +++ b/hook/hook_test.go @@ -60,6 +60,54 @@ 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 extractParameterTests = []struct { s string params interface{} From 85889fe37827fe015d27e4629775b0c4e5fda444 Mon Sep 17 00:00:00 2001 From: Adnan Hajdarevic Date: Thu, 21 Dec 2017 13:14:07 +0100 Subject: [PATCH 2/3] Fix nilpointer dereference when file cannot be created --- webhook.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/webhook.go b/webhook.go index fd3ce1d..79c2c7a 100644 --- a/webhook.go +++ b/webhook.go @@ -23,7 +23,7 @@ import ( ) const ( - version = "2.6.7" + version = "2.6.8" ) var ( @@ -368,13 +368,16 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in tmpfile, err := ioutil.TempFile(h.CommandWorkingDirectory, files[i].EnvName) if err != nil { log.Printf("[%s] error creating temp file [%s]", rid, err) + continue } log.Printf("[%s] writing env %s file %s", rid, 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) + continue } if err := tmpfile.Close(); err != nil { log.Printf("[%s] error closing file %s [%s]", rid, tmpfile.Name(), err) + continue } files[i].File = tmpfile From a811db410bf321f0820f4464f51f371ebb012db1 Mon Sep 17 00:00:00 2001 From: Adnan Hajdarevic Date: Thu, 21 Dec 2017 13:25:19 +0100 Subject: [PATCH 3/3] check before removing --- webhook.go | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/webhook.go b/webhook.go index 79c2c7a..71fdc6b 100644 --- a/webhook.go +++ b/webhook.go @@ -397,10 +397,12 @@ func handleHook(h *hook.Hook, rid string, headers, query, payload *map[string]in } for i := range files { - log.Printf("[%s] removing file %s\n", rid, 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) + if files[i].File != nil { + log.Printf("[%s] removing file %s\n", rid, 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) + } } }