From 6069a8a0047ed3ce3a0e1934c9a66324ef7cebde Mon Sep 17 00:00:00 2001
From: Vladimir Shushlin <vshushlin@gitlab.com>
Date: Tue, 22 Dec 2020 11:36:37 +0000
Subject: Reject all unknown http methods

---
 app.go                                      |  7 +++++
 internal/rejectmethods/middleware.go        | 30 ++++++++++++++++++++
 internal/rejectmethods/middleware_test.go   | 43 +++++++++++++++++++++++++++++
 metrics/metrics.go                          |  7 +++++
 test/acceptance/unknown_http_method_test.go | 23 +++++++++++++++
 5 files changed, 110 insertions(+)
 create mode 100644 internal/rejectmethods/middleware.go
 create mode 100644 internal/rejectmethods/middleware_test.go
 create mode 100644 test/acceptance/unknown_http_method_test.go

diff --git a/app.go b/app.go
index a704ce00..d3716899 100644
--- a/app.go
+++ b/app.go
@@ -27,6 +27,7 @@ import (
 	"gitlab.com/gitlab-org/gitlab-pages/internal/httperrors"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/logging"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/netutil"
+	"gitlab.com/gitlab-org/gitlab-pages/internal/rejectmethods"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/request"
 	"gitlab.com/gitlab-org/gitlab-pages/internal/source"
 	"gitlab.com/gitlab-org/gitlab-pages/metrics"
@@ -334,6 +335,12 @@ func (a *theApp) buildHandlerPipeline() (http.Handler, error) {
 	// Custom response headers
 	handler = a.customHeadersMiddleware(handler)
 
+	// This MUST be the last handler!
+	// This handler blocks unknown HTTP methods,
+	// being the last means it will be evaluated first
+	// preventing any operation on bogus requests.
+	handler = rejectmethods.NewMiddleware(handler)
+
 	return handler, nil
 }
 
diff --git a/internal/rejectmethods/middleware.go b/internal/rejectmethods/middleware.go
new file mode 100644
index 00000000..e76fc383
--- /dev/null
+++ b/internal/rejectmethods/middleware.go
@@ -0,0 +1,30 @@
+package rejectmethods
+
+import (
+	"net/http"
+
+	"gitlab.com/gitlab-org/gitlab-pages/metrics"
+)
+
+var acceptedMethods = map[string]bool{
+	http.MethodGet:     true,
+	http.MethodHead:    true,
+	http.MethodPost:    true,
+	http.MethodPut:     true,
+	http.MethodPatch:   true,
+	http.MethodConnect: true,
+	http.MethodOptions: true,
+	http.MethodTrace:   true,
+}
+
+// NewMiddleware returns middleware which rejects all unknown http methods
+func NewMiddleware(handler http.Handler) http.Handler {
+	return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		if acceptedMethods[r.Method] {
+			handler.ServeHTTP(w, r)
+		} else {
+			metrics.RejectedRequestsCount.Inc()
+			http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed)
+		}
+	})
+}
diff --git a/internal/rejectmethods/middleware_test.go b/internal/rejectmethods/middleware_test.go
new file mode 100644
index 00000000..2921975a
--- /dev/null
+++ b/internal/rejectmethods/middleware_test.go
@@ -0,0 +1,43 @@
+package rejectmethods
+
+import (
+	"io"
+	"net/http"
+	"net/http/httptest"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestNewMiddleware(t *testing.T) {
+	handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		io.WriteString(w, "OK\n")
+	})
+
+	middleware := NewMiddleware(handler)
+
+	acceptedMethods := []string{"GET", "HEAD", "POST", "PUT", "PATCH", "CONNECT", "OPTIONS", "TRACE"}
+	for _, method := range acceptedMethods {
+		t.Run(method, func(t *testing.T) {
+			tmpRequest, _ := http.NewRequest(method, "/", nil)
+			recorder := httptest.NewRecorder()
+
+			middleware.ServeHTTP(recorder, tmpRequest)
+
+			result := recorder.Result()
+
+			require.Equal(t, http.StatusOK, result.StatusCode)
+		})
+	}
+
+	t.Run("UNKNOWN", func(t *testing.T) {
+		tmpRequest, _ := http.NewRequest("UNKNOWN", "/", nil)
+		recorder := httptest.NewRecorder()
+
+		middleware.ServeHTTP(recorder, tmpRequest)
+
+		result := recorder.Result()
+
+		require.Equal(t, http.StatusMethodNotAllowed, result.StatusCode)
+	})
+}
diff --git a/metrics/metrics.go b/metrics/metrics.go
index db7cae9a..045ff26e 100644
--- a/metrics/metrics.go
+++ b/metrics/metrics.go
@@ -199,6 +199,13 @@ var (
 			Help: "The number of files per zip archive total count over time",
 		},
 	)
+
+	RejectedRequestsCount = prometheus.NewCounter(
+		prometheus.CounterOpts{
+			Name: "gitlab_pages_unknown_method_rejected_requests",
+			Help: "The number of requests with unknown HTTP method which were rejected",
+		},
+	)
 )
 
 // MustRegister collectors with the Prometheus client
diff --git a/test/acceptance/unknown_http_method_test.go b/test/acceptance/unknown_http_method_test.go
new file mode 100644
index 00000000..f454cb41
--- /dev/null
+++ b/test/acceptance/unknown_http_method_test.go
@@ -0,0 +1,23 @@
+package acceptance_test
+
+import (
+	"net/http"
+	"testing"
+
+	"github.com/stretchr/testify/require"
+)
+
+func TestUnknownHTTPMethod(t *testing.T) {
+	skipUnlessEnabled(t)
+	teardown := RunPagesProcess(t, *pagesBinary, listeners, "")
+	defer teardown()
+
+	req, err := http.NewRequest("UNKNOWN", listeners[0].URL(""), nil)
+	require.NoError(t, err)
+	req.Host = ""
+
+	resp, err := DoPagesRequest(t, req)
+	require.NoError(t, err)
+
+	require.Equal(t, http.StatusMethodNotAllowed, resp.StatusCode)
+}
-- 
cgit v1.2.3