Merge pull request #5 from UKHomeOffice/feature/197-restrict-access

197-restrict-access: Adds auth middleware

.drone.yml

@@ -29,7 +29,7 @@ pipeline:
       - INSECURE_SKIP_TLS_VERIFY=true
     commands:
       - cd kube
-      - kd -f secrets.yml -f deployment.yml -f ingress.yml -f service.yml
+      - kd -f deployment.yml -f ingress.yml -f service.yml
     when:
       event: [deployment, push]
 

.drone.yml.sig

@@ -1 +1 @@
-eyJhbGciOiJIUzI1NiJ9.cGlwZWxpbmU6CgogIG15LWJ1aWxkOgogICAgcHJpdmlsZWdlZDogdHJ1ZQogICAgaW1hZ2U6IGRvY2tlcjoxLjExCiAgICBlbnZpcm9ubWVudDoKICAgICAgLSBET0NLRVJfSE9TVD10Y3A6Ly8xMjcuMC4wLjE6MjM3NQogICAgY29tbWFuZHM6CiAgICAgIC0gZG9ja2VyIGJ1aWxkIC10IHJlcG8tc2VjdXJpdHktc2Nhbm5lciAuCiAgICB3aGVuOgogICAgICBldmVudDogcHVzaAoKICBpbWFnZV90b19xdWF5OgogICAgaW1hZ2U6IGRvY2tlcjoxLjExCiAgICBlbnZpcm9ubWVudDoKICAgICAgLSBET0NLRVJfSE9TVD10Y3A6Ly8xMjcuMC4wLjE6MjM3NQogICAgY29tbWFuZHM6CiAgICAgIC0gZG9ja2VyIGxvZ2luIC11PSJ1a2hvbWVvZmZpY2VkaWdpdGFsK3JlcG9fc2VjdXJpdHlfc2Nhbm5lcl9ib3QiIC1wPSR7RE9DS0VSX1BBU1NXT1JEfSBxdWF5LmlvCiAgICAgIC0gZG9ja2VyIHRhZyByZXBvLXNlY3VyaXR5LXNjYW5uZXIgcXVheS5pby91a2hvbWVvZmZpY2VkaWdpdGFsL3JlcG8tc2VjdXJpdHktc2Nhbm5lcjoke0RST05FX0NPTU1JVF9TSEF9CiAgICAgIC0gZG9ja2VyIHB1c2ggcXVheS5pby91a2hvbWVvZmZpY2VkaWdpdGFsL3JlcG8tc2VjdXJpdHktc2Nhbm5lcjoke0RST05FX0NPTU1JVF9TSEF9CiAgICB3aGVuOgogICAgICBldmVudDogcHVzaAoKICBkZXBsb3lfdG9fZGV2OgogICAgaW1hZ2U6IHF1YXkuaW8vdWtob21lb2ZmaWNlZGlnaXRhbC9rZDpsYXRlc3QKICAgIGVudmlyb25tZW50OgogICAgICAtIEtVQkVfU0VSVkVSPWh0dHBzOi8va3ViZS1kZXYuZHNwLm5vdHByb2QuaG9tZW9mZmljZS5nb3YudWsKICAgICAgLSBLVUJFX05BTUVTUEFDRT1kZXYtaW5kdWN0aW9uCiAgICAgIC0gSU5TRUNVUkVfU0tJUF9UTFNfVkVSSUZZPXRydWUKICAgIGNvbW1hbmRzOgogICAgICAtIGNkIGt1YmUKICAgICAgLSBrZCAtZiBzZWNyZXRzLnltbCAtZiBkZXBsb3ltZW50LnltbCAtZiBpbmdyZXNzLnltbCAtZiBzZXJ2aWNlLnltbAogICAgd2hlbjoKICAgICAgZXZlbnQ6IFtkZXBsb3ltZW50LCBwdXNoXQoKc2VydmljZXM6CiAgZGluZDoKICAgIGltYWdlOiBkb2NrZXI6MS4xMS1kaW5kCiAgICBwcml2aWxlZ2VkOiB0cnVlCiAgICBjb21tYW5kOgogICAgICAtICItcyIKICAgICAgLSAib3ZlcmxheSIK.mQKKnpfBjTJGD7XBNAnOaia1hJTpznbzCsLfn5LUdN8
+eyJhbGciOiJIUzI1NiJ9.cGlwZWxpbmU6CgogIG15LWJ1aWxkOgogICAgcHJpdmlsZWdlZDogdHJ1ZQogICAgaW1hZ2U6IGRvY2tlcjoxLjExCiAgICBlbnZpcm9ubWVudDoKICAgICAgLSBET0NLRVJfSE9TVD10Y3A6Ly8xMjcuMC4wLjE6MjM3NQogICAgY29tbWFuZHM6CiAgICAgIC0gZG9ja2VyIGJ1aWxkIC10IHJlcG8tc2VjdXJpdHktc2Nhbm5lciAuCiAgICB3aGVuOgogICAgICBldmVudDogcHVzaAoKICBpbWFnZV90b19xdWF5OgogICAgaW1hZ2U6IGRvY2tlcjoxLjExCiAgICBlbnZpcm9ubWVudDoKICAgICAgLSBET0NLRVJfSE9TVD10Y3A6Ly8xMjcuMC4wLjE6MjM3NQogICAgY29tbWFuZHM6CiAgICAgIC0gZG9ja2VyIGxvZ2luIC11PSJ1a2hvbWVvZmZpY2VkaWdpdGFsK3JlcG9fc2VjdXJpdHlfc2Nhbm5lcl9ib3QiIC1wPSR7RE9DS0VSX1BBU1NXT1JEfSBxdWF5LmlvCiAgICAgIC0gZG9ja2VyIHRhZyByZXBvLXNlY3VyaXR5LXNjYW5uZXIgcXVheS5pby91a2hvbWVvZmZpY2VkaWdpdGFsL3JlcG8tc2VjdXJpdHktc2Nhbm5lcjoke0RST05FX0NPTU1JVF9TSEF9CiAgICAgIC0gZG9ja2VyIHB1c2ggcXVheS5pby91a2hvbWVvZmZpY2VkaWdpdGFsL3JlcG8tc2VjdXJpdHktc2Nhbm5lcjoke0RST05FX0NPTU1JVF9TSEF9CiAgICB3aGVuOgogICAgICBldmVudDogcHVzaAoKICBkZXBsb3lfdG9fZGV2OgogICAgaW1hZ2U6IHF1YXkuaW8vdWtob21lb2ZmaWNlZGlnaXRhbC9rZDpsYXRlc3QKICAgIGVudmlyb25tZW50OgogICAgICAtIEtVQkVfU0VSVkVSPWh0dHBzOi8va3ViZS1kZXYuZHNwLm5vdHByb2QuaG9tZW9mZmljZS5nb3YudWsKICAgICAgLSBLVUJFX05BTUVTUEFDRT1kZXYtaW5kdWN0aW9uCiAgICAgIC0gSU5TRUNVUkVfU0tJUF9UTFNfVkVSSUZZPXRydWUKICAgIGNvbW1hbmRzOgogICAgICAtIGNkIGt1YmUKICAgICAgLSBrZCAtZiBkZXBsb3ltZW50LnltbCAtZiBpbmdyZXNzLnltbCAtZiBzZXJ2aWNlLnltbAogICAgd2hlbjoKICAgICAgZXZlbnQ6IFtkZXBsb3ltZW50LCBwdXNoXQoKc2VydmljZXM6CiAgZGluZDoKICAgIGltYWdlOiBkb2NrZXI6MS4xMS1kaW5kCiAgICBwcml2aWxlZ2VkOiB0cnVlCiAgICBjb21tYW5kOgogICAgICAtICItcyIKICAgICAgLSAib3ZlcmxheSIK.fGU1SaQhWfl_kQlZaCoU_mVOh0Bl-OU5q4zeHWle19U

.env

@@ -0,0 +1 @@
+export GITHUB_WEBHOOKSECRET=blah

Makefile

@@ -1,5 +1,7 @@
 PORT = 8080
 
+GITHUB_SECRET = $(shell echo $(GITHUB_WEBHOOKSECRET))
+
 USER = ukhomeoffice-bot-test
 REPO = testgithubintegration
 OFFENSES_X0 = 47797c0123bc0f5adfcae3d3467a2ed12e72b2cb
@@ -48,29 +50,34 @@ watch:
 run:
 	@go build -race . && ./repo-security-scanner
 
+mac-diff-file:
+	@cat $(DIFF_FILE) | openssl sha1 -hmac $(GITHUB_SECRET) | sed 's/^.* //'
+
 test-run:
-	@curl \
+	@wget -O- \
 		-X POST \
-		-H "x-github-event: push" \
-		-d @$(DIFF_FILE) \
+		--header="X-GitHub-Event: push" \
+		--header="X-Hub-Signature: sha1=$(shell make mac-diff-file)" \
+		--post-file "$(DIFF_FILE)" \
+		http://localhost:$(PORT)/github
+
+test-run-fail:
+	@wget -O- \
+		-X POST \
+		--header="X-GitHub-Event: push" \
+		--header="X-Hub-Signature: sha1=123456" \
+		--post-file "$(DIFF_FILE)" \
 		http://localhost:$(PORT)/github
 
 test-run-dev:
 	@curl \
 		-X POST \
 		-H "x-github-event: push" \
+		--header="X-Hub-Signature: sha1=$(shell make mac-diff-file)" \
 		-d @$(DIFF_FILE) \
 		http://repo-security-scanner.notprod.homeoffice.gov.uk/github
 
-
 test:
 	@go test
 
-test-cover:
-	@go test -cover
-
-test-race:
-	@go test -race
-
-
 .PHONY: test* run deps install lint rules struct diff

README.md

@@ -1,7 +1,5 @@
 # repo-security-scanner
 
-[Documentation on GoDoc](https://godoc.org/github.com/UKHomeOffice/repo-security-scanner).
-
 ## Installation
 
 ```make install```

TODO.md

@@ -1,7 +1,3 @@
-- [ ] Add middleware - validate = from github origin + tests
-	- use gorilla middleware vs write own
-
-
 - [ ] type []Stats == logger = strings.Stringer() interface -> for creating string for email
 - [ ] Add email notifications (+ interface + tests)
 
@@ -10,5 +6,7 @@
 
 
 - [ ] Analyze body of commits (added/removed lines)
+
+
 - [ ] Add concurrency (parallelize requests to github API)
 - [ ] Add context + timeout to requests to github API

handlers.go

@@ -4,27 +4,20 @@ import (
 	"fmt"
 	"net/http"
 
-	"github.com/julienschmidt/httprouter"
 	"github.com/techjacker/diffence"
 )
 
 const (
-	headerGithubEvt = "x-github-event"
-	msgHealthOk     = "ok"
-	msgBadRequest   = "bad request"
-	msgIgnore       = "Not a push evt; ignoring"
-	msgSuccess      = "Push contains no offenses"
-	msgFail         = "TODO: email list of recipients to be notified of violations"
+	msgHealthOk   = "ok"
+	msgBadRequest = "bad request"
+	msgIgnore     = "Not a push evt; ignoring"
+	msgSuccess    = "Push contains no offenses"
+	msgFail       = "TODO: email list of recipients to be notified of violations"
 )
 
 // GithubHandler is a github integration HTTP handler
-func GithubHandler(dc diffence.Checker, dg DiffGetterHTTP) httprouter.Handle {
-	return func(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
-		if r.Header.Get(headerGithubEvt) != "push" {
-			w.WriteHeader(http.StatusOK)
-			w.Write([]byte(msgIgnore))
-			return
-		}
+func GithubHandler(dc diffence.Checker, dg DiffGetterHTTP) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		// decode github push event payload
 		gitRes := &GithubResponse{}
 		err := DecodeJSON(r.Body, gitRes)
@@ -55,11 +48,11 @@ func GithubHandler(dc diffence.Checker, dg DiffGetterHTTP) httprouter.Handle {
 			return
 		}
 		fmt.Fprintf(w, "%s\n", msgSuccess)
-	}
+	})
 }
 
 // HealthHandler is an endpoint for healthchecks
-func HealthHandler(w http.ResponseWriter, r *http.Request, _ httprouter.Params) {
+func HealthHandler(w http.ResponseWriter, r *http.Request) {
 	w.WriteHeader(http.StatusOK)
 	w.Write([]byte(msgHealthOk))
 }

handlers_test.go

@@ -27,7 +27,6 @@ func TestGithubHandler(t *testing.T) {
 	)
 
 	type args struct {
-		githubEvt         string
 		githubPayloadPath string
 		rulesPath         string
 		diffPath          string
@@ -41,7 +40,6 @@ func TestGithubHandler(t *testing.T) {
 		{
 			name: "Incorrect JSON payload returns 4xx status code",
 			args: args{
-				githubEvt:         "push",
 				githubPayloadPath: "test/fixtures/nonsense.json",
 				rulesPath:         gitrobRules,
 				diffPath:          "",
@@ -52,7 +50,6 @@ func TestGithubHandler(t *testing.T) {
 		{
 			name: "No offenses in diff",
 			args: args{
-				githubEvt:         "push",
 				githubPayloadPath: "test/fixtures/github_event_push.json",
 				rulesPath:         gitrobRules,
 				diffPath:          "test/fixtures/no_offenses.diff",
@@ -63,38 +60,25 @@ func TestGithubHandler(t *testing.T) {
 		{
 			name: "1 offense in diff",
 			args: args{
-				githubEvt:         "push",
 				githubPayloadPath: "test/fixtures/github_event_push.json",
 				rulesPath:         gitrobRules,
 				diffPath:          "test/fixtures/offenses_x1.diff",
 			},
 			wantStatusCode: http.StatusOK,
 			wantResBody:    msgFail,
 		},
-		{
-			name: "Not a push event",
-			args: args{
-				githubEvt:         "integration_installation_repositories",
-				githubPayloadPath: "test/fixtures/github_event_integration.json",
-				rulesPath:         gitrobRules,
-				diffPath:          "test/fixtures/offenses_x1.diff",
-			},
-			wantStatusCode: http.StatusOK,
-			wantResBody:    msgIgnore,
-		},
 	}
 	for _, tt := range tests {
 		t.Run(tt.name, func(t *testing.T) {
 
 			router := httprouter.New()
-			router.POST(testPath, GithubHandler(
+			router.Handler("POST", testPath, GithubHandler(
 				diffence.DiffChecker{getTestRules(t, tt.args.rulesPath)},
 				testDiffGetter{tt.args.diffPath},
 			))
 
 			params := getFixture(tt.args.githubPayloadPath)
 			r, err := http.NewRequest("POST", testPath, params)
-			r.Header.Set(headerGithubEvt, tt.args.githubEvt)
 			if err != nil {
 				t.Error(err)
 			}
@@ -134,7 +118,7 @@ func TestHealthHandler(t *testing.T) {
 		t.Run(tt.name, func(t *testing.T) {
 
 			router := httprouter.New()
-			router.GET(testPath, HealthHandler)
+			router.Handler("GET", testPath, http.HandlerFunc(HealthHandler))
 
 			r, err := http.NewRequest("GET", testPath, nil)
 			if err != nil {

kube/deployment.yml

@@ -29,8 +29,13 @@ spec:
           - name: GITHUB_PRIVATEKEY
             valueFrom:
               secretKeyRef:
-                name: secrets
+                name: repo-security-scanner
                 key: GITHUB_PRIVATEKEY
+          - name: GITHUB_WEBHOOKSECRET
+            valueFrom:
+              secretKeyRef:
+                name: repo-security-scanner
+                key: GITHUB_WEBHOOKSECRET
         resources:
           limits:
             cpu: 250m

kube/secrets.yml

@@ -1,7 +1,8 @@
 apiVersion: v1
 kind: Secret
 metadata:
-  name: secrets
+  name: repo-security-scanner
 type: Opaque
 data:
   GITHUB_PRIVATEKEY: {{.GITHUB_PRIVATEKEY}}
+  GITHUB_WEBHOOKSECRET: {{.GITHUB_WEBHOOKSECRET}}

main.go

@@ -3,6 +3,7 @@ package main
 import (
 	"fmt"
 	"net/http"
+	"os"
 	"path"
 	"runtime"
 
@@ -11,8 +12,11 @@ import (
 )
 
 const (
-	gitrobRules = "rules/gitrob.json"
-	serverPort  = 8080
+	headerGithubMAC     = "X-Hub-Signature"
+	headerGithubEvt     = "X-Github-Event"
+	githubWebhookSecret = "GITHUB_WEBHOOKSECRET"
+	gitrobRules         = "rules/gitrob.json"
+	serverPort          = 8080
 )
 
 func getRules(rulesPath string) *[]diffence.Rule {
@@ -24,13 +28,25 @@ func getRules(rulesPath string) *[]diffence.Rule {
 	return rules
 }
 
+func getEnvVar(name string) []byte {
+	secretStr := os.Getenv(name)
+	if len(secretStr) < 1 {
+		panic(fmt.Sprintf("Env var:%s not set in environment", name))
+	}
+	return []byte(secretStr)
+}
+
 func main() {
 	router := httprouter.New()
-	router.GET("/healthz", HealthHandler)
-	router.POST("/github", GithubHandler(
-		diffence.DiffChecker{getRules(gitrobRules)},
-		diffGetterGithub{},
+	router.Handler("GET", "/healthz", http.HandlerFunc(HealthHandler))
+	router.Handler("POST", "/github", Adapt(
+		GithubHandler(
+			diffence.DiffChecker{getRules(gitrobRules)},
+			diffGetterGithub{},
+		),
+		AuthMiddleware(GithubAuthenticator{getEnvVar(githubWebhookSecret)}),
 	))
+
 	fmt.Printf("Server listening on port: %d", serverPort)
 	http.ListenAndServe(fmt.Sprintf(":%d", serverPort), router)
 }

middleware.go

@@ -0,0 +1,83 @@
+package main
+
+import (
+	"bytes"
+	"crypto/hmac"
+	"crypto/sha1"
+	"encoding/hex"
+	"errors"
+	"io/ioutil"
+	"net/http"
+)
+
+// Adapter defines a middleware handler
+type Adapter func(http.Handler) http.Handler
+
+// Adapt chains a series of middleware handlers
+func Adapt(h http.Handler, adapters ...Adapter) http.Handler {
+	for _, adapter := range adapters {
+		h = adapter(h)
+	}
+	return h
+}
+
+// Middleware wraps a function and returns an http.Handler fn type
+type Middleware func(http.Handler) http.Handler
+
+// Authenticator is an interface for authorizing streams
+type Authenticator interface {
+	CheckMAC([]byte, []byte) (bool, error)
+}
+
+// GithubAuthenticator authorizes a payload with a shared secret
+type GithubAuthenticator struct {
+	secret []byte
+}
+
+// CheckMAC checks text matches a HMAC signature
+func (g GithubAuthenticator) CheckMAC(body, expectedMAC []byte) (bool, error) {
+	if len(g.secret) < 1 {
+		return false, errors.New("secret not set")
+	}
+	mac := hmac.New(sha1.New, g.secret)
+	mac.Write(body)
+	actualMAC := []byte(hex.EncodeToString(mac.Sum(nil)))
+	return hmac.Equal(expectedMAC, append([]byte("sha1="), actualMAC...)), nil
+}
+
+// AuthMiddleware authenticates a request body
+func AuthMiddleware(ag Authenticator) Adapter {
+	return func(next http.Handler) http.Handler {
+		fn := func(w http.ResponseWriter, r *http.Request) {
+			if r.Header.Get(headerGithubEvt) != "push" {
+				w.WriteHeader(http.StatusOK)
+				w.Write([]byte(msgIgnore))
+				return
+			}
+			buf, err := ioutil.ReadAll(r.Body)
+			rdr1 := ioutil.NopCloser(bytes.NewBuffer(buf))
+			r.Body = rdr1
+			if err != nil {
+				w.WriteHeader(http.StatusInternalServerError)
+				w.Write([]byte(http.StatusText(http.StatusInternalServerError)))
+				return
+			}
+			authorized, err := ag.CheckMAC(
+				buf,
+				[]byte(r.Header.Get(headerGithubMAC)),
+			)
+			if err != nil {
+				w.WriteHeader(http.StatusInternalServerError)
+				w.Write([]byte(http.StatusText(http.StatusInternalServerError)))
+				return
+			}
+			if authorized != true {
+				w.WriteHeader(http.StatusUnauthorized)
+				w.Write([]byte(http.StatusText(http.StatusUnauthorized)))
+				return
+			}
+			next.ServeHTTP(w, r)
+		}
+		return http.HandlerFunc(fn)
+	}
+}