Repository: honeydipper/honeydipper
Pull request #408 (2021-06-01 16:43:37)

chore(release): release
* feat(drivers): webhook verify signature using hmac



Patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index cc9bbfc..85bac51 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -47,6 +47,23 @@ jobs:
       - run: go build -v ./...
       - *save_mod
       - run: make unit-tests
+  test_coverage:
+    <<: *defaults
+    steps:
+      - run: apk add git openssh-client gcc libc-dev make
+      - checkout
+      - run:
+          name: Setup Code Climate test-reporter
+          command: |
+            # download test reporter as a static binary
+            wget -O cc-test-reporter https://codeclimate.com/downloads/test-reporter/test-reporter-latest-linux-amd64
+            chmod +x ./cc-test-reporter
+      - run: ./cc-test-reporter before-build
+      - *restore_mod
+      - run: go build -v ./...
+      - *save_mod
+      - run: REPORT_TEST_COVERAGE=1 make unit-tests
+      - run: ./cc-test-reporter after-build --prefix=github.com/honeydipper/honeydipper
   integration:
     <<: *defaults
     docker:
@@ -85,13 +102,24 @@ workflows:
     jobs:
       - golangci-lint
       - license_finder
+      - test_coverage:
+          context: honeydipper
+          filters:
+            branches:
+              only: dev
       - test:
           requires:
             - golangci-lint
             - license_finder
+          filters:
+            branches:
+              ignore: dev
       - integration:
           requires:
             - test
+          filters:
+            branches:
+              ignore: dev
   release:
     jobs:
       - semantic-release:
diff --git a/Makefile b/Makefile
index 187e283..fd76ecb 100644
--- a/Makefile
+++ b/Makefile
@@ -44,7 +44,11 @@ run_mockgen: .mockgen_installed .mockgen_files_generated
 
 unit-tests: run_mockgen
 	@echo -e "$(BOLD)Running unit tests$(RESET)"
+ifneq (,$(REPORT_TEST_COVERAGE))
+	@go test -coverprofile=c.out ./...
+else
 	@go test ./...
+endif
 
 integration-tests: run_mockgen
 	@echo -e "$(BOLD)Running integration tests$(RESET)"
diff --git a/drivers/cmd/webhook/main.go b/drivers/cmd/webhook/main.go
index cd9a769..b0160bf 100644
--- a/drivers/cmd/webhook/main.go
+++ b/drivers/cmd/webhook/main.go
@@ -9,18 +9,38 @@ package main
 
 import (
 	"context"
+	"crypto/hmac"
+	"crypto/sha256"
+	"encoding/hex"
 	"flag"
 	"fmt"
 	"io"
 	"net/http"
 	"os"
+	"strconv"
+	"strings"
+	"time"
 
+	"github.com/go-errors/errors"
 	"github.com/honeydipper/honeydipper/pkg/dipper"
 	"github.com/op/go-logging"
 )
 
 var log *logging.Logger
 
+// ErrCalcHash is raised when unable to calculate the hash for validating the webhook request.
+var ErrCalcHash = errors.New("unable to calculate the hash")
+
+// ErrReplayAttack is raised when the timestamp of the request is not within 5 minutes.
+var ErrReplayAttack = errors.New("replay attack detected")
+
+// _SupportedSignatureHeaders is a list of supported signature headers.
+var _SupportedSignatureHeaders = []string{
+	"X-Hub-Signature-256",
+	"X-PagerDuty-Signature",
+	"X-Slack-Signature",
+}
+
 func initFlags() {
 	flag.Usage = func() {
 		fmt.Printf("%s [ -h ] <service name>\n", os.Args[0])
@@ -33,6 +53,7 @@ var (
 	driver *dipper.Driver
 	server *http.Server
 	hooks  map[string]interface{}
+	sysMap map[string]map[string]interface{}
 )
 
 // Addr : listening address and port of the webhook.
@@ -68,6 +89,16 @@ func loadOptions(m *dipper.Message) {
 
 	log.Debugf("[%s] hook data : %+v", driver.Service, hooks)
 
+	sysMap = map[string]map[string]interface{}{}
+	for _, hook := range hooks {
+		for _, collapsed := range hook.([]interface{}) {
+			rule := collapsed.(map[string]interface{})
+			if sys, ok := rule["sysName"]; ok && sys.(string) != "" {
+				sysMap[sys.(string)] = rule["sysData"].(map[string]interface{})
+			}
+		}
+	}
+
 	NewAddr, ok := driver.GetOptionStr("data.Addr")
 	if !ok {
 		NewAddr = ":8080"
@@ -95,6 +126,18 @@ func startWebhook(m *dipper.Message) {
 
 func hookHandler(w http.ResponseWriter, r *http.Request) {
 	eventData := extractEventData(w, r)
+	if eventData == nil {
+		return
+	}
+
+	defer func() {
+		if e := recover(); e != nil {
+			log.Warningf("Resuming after error: %v", e)
+			log.Warning(errors.Wrap(e, 1).ErrorStack())
+			w.WriteHeader(http.StatusInternalServerError)
+			dipper.Must(io.WriteString(w, "Internal server error\n"))
+		}
+	}()
 
 	if eventData["url"] == "/hz/alive" {
 		w.WriteHeader(http.StatusOK)
@@ -102,24 +145,14 @@ func hookHandler(w http.ResponseWriter, r *http.Request) {
 		return
 	}
 
+	verifySystems(eventData)
+
 	log.Debugf("[%s] webhook event data: %+v", driver.Service, eventData)
 	matched := false
 	for _, hook := range hooks {
 		for _, collapsed := range hook.([]interface{}) {
 			condition, _ := dipper.GetMapData(collapsed, "match")
-			auth, ok := dipper.GetMapData(condition, ":auth:")
-			if ok {
-				authDriver := dipper.MustGetMapDataStr(auth, "driver")
-				authResult, err := driver.Call("driver:"+authDriver, "webhookAuth", map[string]interface{}{
-					"event":     eventData,
-					"condition": auth,
-				})
-				if err != nil || string(authResult) != "authenticated" {
-					log.Warningf("[%s] failed to authenticate webhook request with %s error %+v", driver.Service, authDriver, err)
-
-					continue
-				}
-			}
+
 			if dipper.CompareAll(eventData, condition) {
 				matched = true
 
@@ -159,9 +192,95 @@ func extractEventData(w http.ResponseWriter, r *http.Request) map[string]interfa
 	defer func() {
 		if r := recover(); r != nil {
 			badRequest(w)
-			log.Panicf("[%s] invalid json in post body", driver.Service)
+			log.Warningf("Resuming after error: %v", r)
+			log.Warning(errors.Wrap(r, 1).ErrorStack())
 		}
 	}()
 
 	return dipper.ExtractWebRequest(r)
 }
+
+func verifySignature(header, actual, secret string, eventData map[string]interface{}) bool {
+	key := []byte(secret)
+	mac := hmac.New(sha256.New, key)
+	switch header {
+	case "X-Slack-Signature":
+		timestamp := eventData["headers"].(http.Header).Get("X-Slack-Request-Timestamp")
+		if timestamp == "" {
+			panic(ErrCalcHash)
+		}
+		if _, ok := eventData["skip_replay_check"]; !ok {
+			current := time.Now().Unix()
+			requestedAt := dipper.Must(strconv.ParseInt(timestamp, 10, 64)).(int64)
+			if current-requestedAt > 300 || current < requestedAt {
+				panic(ErrReplayAttack)
+			}
+		}
+		dipper.Must(mac.Write([]byte("v0:")))
+		dipper.Must(mac.Write([]byte(timestamp)))
+		dipper.Must(mac.Write([]byte(":")))
+	case "X-PagerDuty-Signature":
+	case "X-Hub-Signature-256": // github signature
+	}
+	dipper.Must(mac.Write(eventData["body"].([]byte)))
+	expected := mac.Sum(nil)
+	log.Infof("[%s] HMAC for %s calculated: %s", driver.Service, header, hex.EncodeToString(expected))
+
+	var hashes []string
+	switch header {
+	case "X-Slack-Signature":
+		hashes = []string{strings.TrimPrefix(actual, "v0=")}
+	case "X-PagerDuty-Signature":
+		for _, sig := range strings.Split(actual, ",") {
+			hashes = append(hashes, strings.TrimPrefix(sig, "v1="))
+		}
+	case "X-Hub-Signature-256": // github signature
+		hashes = []string{strings.TrimPrefix(actual, "sha256=")}
+	}
+
+	for _, hash := range hashes {
+		if hmac.Equal(expected, dipper.Must(hex.DecodeString(hash)).([]byte)) {
+			return true
+		}
+	}
+
+	return false
+}
+
+func verifySystems(eventData map[string]interface{}) {
+	headers := eventData["headers"].(http.Header)
+
+	var signatureHeader, signatureValue string
+	for _, supported := range _SupportedSignatureHeaders {
+		signatureValue = headers.Get(supported)
+		if signatureValue != "" {
+			signatureHeader = supported
+
+			break
+		}
+	}
+
+	if signatureHeader != "" {
+		var verifiedSystem []string
+		for name, sys := range sysMap {
+			expectedHeader, ok := sys["signatureHeader"]
+			if !ok || !strings.EqualFold(expectedHeader.(string), signatureHeader) {
+				// ignore unsupported headers or system without signatureHeader
+				continue
+			}
+
+			secret, ok := dipper.GetMapDataStr(sys, "signatureSecret")
+			if !ok {
+				log.Warningf("[%s] signature secret not defined for system %s", driver.Service, name)
+
+				continue
+			}
+
+			if verifySignature(signatureHeader, signatureValue, secret, eventData) {
+				verifiedSystem = append(verifiedSystem, name)
+			}
+		}
+		log.Infof("[%s] HMAC verified for system(s) %+v", driver.Service, verifiedSystem)
+		eventData["verifiedSystem"] = verifiedSystem
+	}
+}
diff --git a/internal/config/collapsed_trigger.go b/internal/config/collapsed_trigger.go
index 832680f..09b55ca 100644
--- a/internal/config/collapsed_trigger.go
+++ b/internal/config/collapsed_trigger.go
@@ -16,12 +16,14 @@ type CollapsedTrigger struct {
 	Exports    []map[string]interface{} `json:"exports"`
 	SysData    map[string]interface{}   `json:"sysData"`
 	Parameters map[string]interface{}   `json:"parameters"`
+	SysName    string                   `json:"sysName"`
 }
 
 // CollapseTrigger collapses matching criteria, exports and sysData of a trigger and its inheritted triggers.
 func CollapseTrigger(t *Trigger, c *DataSet) (*Trigger, *CollapsedTrigger) {
 	var stack []*Trigger
 	current := t
+
 	for len(current.Source.System) > 0 {
 		stack = append(stack, current)
 		sourceSys := c.Systems[current.Source.System]
@@ -52,11 +54,16 @@ func CollapseTrigger(t *Trigger, c *DataSet) (*Trigger, *CollapsedTrigger) {
 		params = dipper.Interpolate(params, envData).(map[string]interface{})
 	}
 
+	if flag, ok := params["verifySystem"]; ok && dipper.IsTruthy(flag) {
+		match["verifiedSystem"] = t.Source.System
+	}
+
 	return current, &CollapsedTrigger{
 		Match:      match,
 		Exports:    exports,
 		SysData:    sysData,
 		Parameters: params,
+		SysName:    t.Source.System,
 	}
 }
 
diff --git a/pkg/dipper/web.go b/pkg/dipper/web.go
index 0fe67ae..edcabd8 100644
--- a/pkg/dipper/web.go
+++ b/pkg/dipper/web.go
@@ -7,6 +7,7 @@
 package dipper
 
 import (
+	"bytes"
 	"encoding/json"
 	"io/ioutil"
 	"net/http"
@@ -31,14 +32,22 @@ func ExtractWebRequestExceptBody(r *http.Request) map[string]interface{} {
 
 // ExtractWebRequest put needed information from a request in a map.
 func ExtractWebRequest(r *http.Request) map[string]interface{} {
+	// keep the body for sha256
+	var body []byte
+	if r.Body != nil {
+		body = Must(ioutil.ReadAll(r.Body)).([]byte)
+		r.Body.Close()
+		r.Body = ioutil.NopCloser(bytes.NewBuffer(body))
+	}
+
 	req := ExtractWebRequestExceptBody(r)
 
-	if r.Method == http.MethodPost {
-		req["body"] = Must(ioutil.ReadAll(r.Body))
+	if len(body) > 0 {
+		req["body"] = body
 		if strings.HasPrefix(r.Header.Get("content-type"), "application/json") {
-			bodyObj := map[string]interface{}{}
-			Must(json.Unmarshal(req["body"].([]byte), &bodyObj))
-			req["json"] = bodyObj
+			j := map[string]interface{}{}
+			Must(json.Unmarshal(body, &j))
+			req["json"] = j
 		}
 	}
 


Test patch:
diff --git a/drivers/cmd/webhook/main_test.go b/drivers/cmd/webhook/main_test.go
index 393d86f..6d15a2d 100644
--- a/drivers/cmd/webhook/main_test.go
+++ b/drivers/cmd/webhook/main_test.go
@@ -9,8 +9,11 @@
 package main
 
 import (
+	"bytes"
 	"context"
+	"io/ioutil"
 	"net/http"
+	"net/url"
 	"os"
 	"sync"
 	"testing"
@@ -67,3 +70,166 @@ func TestExtractEvent(t *testing.T) {
 	assert.Equalf(t, "127.0.0.1:8999", eventData["host"], "host data mismatch")
 	assert.Containsf(t, eventData["remoteAddr"], "127.0.0.1:", "remoteAddr data mismatch")
 }
+
+func TestVerifySignature(t *testing.T) {
+	result := verifySignature(
+		"X-Hub-Signature-256",
+		"sha256=00",
+		"testsecret",
+		map[string]interface{}{
+			"body": []byte(`{"test": "value"}`),
+		},
+	)
+	assert.Falsef(t, result, "should fail when hmac header is invalid")
+
+	result = verifySignature(
+		"X-Hub-Signature-256",
+		"sha256=95bb6cd808d37348a4bda7857e7558a16de0d0868d83e75574b834660fbc1d3b",
+		"testsecret",
+		map[string]interface{}{
+			"body": []byte(`{"test": "value"}`),
+		},
+	)
+	assert.Truef(t, result, "should succeed with valid github hmac header")
+
+	result = verifySignature(
+		"X-PagerDuty-Signature",
+		"v1=95bb6cd808d37348a4bda7857e7558a16de0d0868d83e75574b834660fbc1d3b",
+		"testsecret",
+		map[string]interface{}{
+			"body": []byte(`{"test": "value"}`),
+		},
+	)
+	assert.Truef(t, result, "should succeed with valid pagerduty hmac header")
+
+	result = verifySignature(
+		"X-Slack-Signature",
+		"v0=57a7e349548bb23285ae98637c893c6beffc9d2f654f386b2b979001167a13a6",
+		"testsecret",
+		map[string]interface{}{
+			"body":              []byte(`{"test": "value"}`),
+			"headers":           http.Header{"X-Slack-Request-Timestamp": []string{"1622172061"}},
+			"skip_replay_check": "yes",
+		},
+	)
+	assert.Truef(t, result, "should succeed with valid slack hmac header")
+
+	f := func() {
+		verifySignature(
+			"X-Slack-Signature",
+			"v0=57a7e349548bb23285ae98637c893c6beffc9d2f654f386b2b979001167a13a6",
+			"testsecret",
+			map[string]interface{}{
+				"body":    []byte(`{"test": "value"}`),
+				"headers": http.Header{"X-Slack-Request-Timestamp": []string{"1622172061"}},
+			},
+		)
+	}
+	assert.PanicsWithErrorf(t, "replay attack detected", f, "should detect replay attack with old slack request timestamp")
+}
+
+type mockResponseWriter struct {
+	status  int
+	content []byte
+	e       error
+	header  http.Header
+}
+
+func (m *mockResponseWriter) Write(c []byte) (int, error) {
+	m.content = c
+	return len(c), m.e
+}
+
+func (m *mockResponseWriter) WriteHeader(s int) {
+	m.status = s
+}
+
+func (m *mockResponseWriter) Header() http.Header {
+	return m.header
+}
+
+func TestHookHandler(t *testing.T) {
+	sysMap = map[string]map[string]interface{}{
+		"sys-missing-secret": map[string]interface{}{
+			"signatureHeader": "x-pagerduty-signature",
+		},
+		"sys": map[string]interface{}{
+			"signatureHeader": "x-pagerduty-signature",
+			"signatureSecret": "test-secret",
+		},
+		"sys-unsupported-header": map[string]interface{}{
+			"signatureHeader": "x-unknown-signature",
+		},
+	}
+
+	hooks = map[string]interface{}{
+		"sys1.webhook": []interface{}{
+			map[string]interface{}{
+				"match": map[string]interface{}{"url": "/test/sys1"},
+			},
+		},
+		"sys2.webhook": []interface{}{
+			map[string]interface{}{
+				"match": map[string]interface{}{"verifiedSystem": "sys-missing-secret"},
+			},
+		},
+		"sys3.webhook": []interface{}{
+			map[string]interface{}{
+				"match": map[string]interface{}{"verifiedSystem": "sys-unsupported-header"},
+			},
+		},
+		"sys4.webhook": []interface{}{
+			map[string]interface{}{
+				"match": map[string]interface{}{"verifiedSystem": "sys"},
+			},
+		},
+	}
+
+	buf := bytes.NewBuffer(make([]byte, 2048))
+	driver = &dipper.Driver{
+		Out: buf,
+	}
+
+	resp := &mockResponseWriter{header: http.Header{}}
+	req := &http.Request{
+		Method: "GET",
+		URL:    &url.URL{Path: "/test/sys1"},
+	}
+	hookHandler(resp, req)
+	msg := dipper.FetchMessage(buf)
+	assert.Equalf(t, 200, resp.status, "should return 200 on success")
+	assert.Equalf(t, "webhook.", msg.Payload.(map[string]interface{})["events"].([]interface{})[0], "should emit webhook event to daemon")
+
+	resp = &mockResponseWriter{header: http.Header{}}
+	req = &http.Request{
+		Method: "GET",
+		URL:    &url.URL{Path: "/test/sys2"},
+	}
+	hookHandler(resp, req)
+	assert.Equalf(t, 404, resp.status, "should return 404 when no trigger is matching")
+	assert.Zerof(t, buf.Len(), "should not emit event to daemon")
+
+	resp = &mockResponseWriter{header: http.Header{}}
+	req = &http.Request{
+		Method: "POST",
+		URL:    &url.URL{Path: "/test/sys2"},
+		Header: http.Header{"X-Pagerduty-Signature": []string{"v1=bcc889a40667cab715e1dc22ad280692cf4bf1c3a280eeeca60d8dbcd8e4b993"}},
+		Body:   ioutil.NopCloser(bytes.NewBufferString("hello")),
+	}
+	hookHandler(resp, req)
+	msg = dipper.FetchMessage(buf)
+	assert.Equalf(t, 200, resp.status, "should return 200 on success with proper signature")
+	assert.Equalf(t, "sys", dipper.MustGetMapDataStr(msg.Payload, "data.verifiedSystem.0"), "should emit webhook event with verifiedSystem")
+	assert.Equalf(t, 1, len(dipper.MustGetMapData(msg.Payload, "data.verifiedSystem").([]interface{})), "should verify only 1 system")
+
+	resp = &mockResponseWriter{header: http.Header{}}
+	req = &http.Request{
+		Method: "POST",
+		URL:    &url.URL{Path: "/test/sys2"},
+		Header: http.Header{"X-Unknown-Signature": []string{"v1=bcc889a40667cab715e1dc22ad280692cf4bf1c3a280eeeca60d8dbcd8e4b993"}},
+		Body:   ioutil.NopCloser(bytes.NewBufferString("hello")),
+	}
+	hookHandler(resp, req)
+	assert.Equalf(t, 404, resp.status, "should return 404 with unsupported signature header")
+	assert.Zerof(t, buf.Len(), "should not emit event to daemon")
+}