From 7a8a489be482e9e3cb925ce3ef4a6d18ff17910e Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 6 Jan 2021 19:33:09 +0000 Subject: Add latest changes from gitlab-org/security/gitlab@13-7-stable-ee --- GITLAB_WORKHORSE_VERSION | 2 +- .../security-workhorse-prometheus-13-7.yml | 5 +++ vendor/gitignore/C++.gitignore | 0 vendor/gitignore/Java.gitignore | 0 workhorse/CHANGELOG | 12 ++++++ workhorse/VERSION | 2 +- workhorse/internal/rejectmethods/middleware.go | 38 +++++++++++++++++++ .../internal/rejectmethods/middleware_test.go | 43 ++++++++++++++++++++++ workhorse/internal/upstream/upstream.go | 3 ++ workhorse/main_test.go | 18 +++++++++ 10 files changed, 121 insertions(+), 2 deletions(-) create mode 100644 changelogs/unreleased/security-workhorse-prometheus-13-7.yml mode change 100644 => 100755 vendor/gitignore/C++.gitignore mode change 100644 => 100755 vendor/gitignore/Java.gitignore create mode 100644 workhorse/internal/rejectmethods/middleware.go create mode 100644 workhorse/internal/rejectmethods/middleware_test.go diff --git a/GITLAB_WORKHORSE_VERSION b/GITLAB_WORKHORSE_VERSION index b315ff1896d..8aa8b5f68bd 100644 --- a/GITLAB_WORKHORSE_VERSION +++ b/GITLAB_WORKHORSE_VERSION @@ -1 +1 @@ -8.58.0 +8.58.2 diff --git a/changelogs/unreleased/security-workhorse-prometheus-13-7.yml b/changelogs/unreleased/security-workhorse-prometheus-13-7.yml new file mode 100644 index 00000000000..ab731831033 --- /dev/null +++ b/changelogs/unreleased/security-workhorse-prometheus-13-7.yml @@ -0,0 +1,5 @@ +--- +title: Upgrade Workhorse to 8.58.2 +merge_request: +author: +type: security diff --git a/vendor/gitignore/C++.gitignore b/vendor/gitignore/C++.gitignore old mode 100644 new mode 100755 diff --git a/vendor/gitignore/Java.gitignore b/vendor/gitignore/Java.gitignore old mode 100644 new mode 100755 diff --git a/workhorse/CHANGELOG b/workhorse/CHANGELOG index 489ac4531fc..271928845c1 100644 --- a/workhorse/CHANGELOG +++ b/workhorse/CHANGELOG @@ -1,5 +1,17 @@ # Changelog for gitlab-workhorse +## v8.58.2 + +### Security +- Allow DELETE HTTP method + https://gitlab.com/gitlab-org/gitlab-workhorse/-/merge_requests/ + +## v8.58.1 + +### Security +- Reject unknown http methods + https://gitlab.com/gitlab-org/gitlab-workhorse/-/merge_requests/ + ## v8.58.0 ### Added diff --git a/workhorse/VERSION b/workhorse/VERSION index b315ff1896d..8aa8b5f68bd 100644 --- a/workhorse/VERSION +++ b/workhorse/VERSION @@ -1 +1 @@ -8.58.0 +8.58.2 diff --git a/workhorse/internal/rejectmethods/middleware.go b/workhorse/internal/rejectmethods/middleware.go new file mode 100644 index 00000000000..171463979d5 --- /dev/null +++ b/workhorse/internal/rejectmethods/middleware.go @@ -0,0 +1,38 @@ +package rejectmethods + +import ( + "net/http" + + "github.com/prometheus/client_golang/prometheus" +) + +var acceptedMethods = map[string]bool{ + http.MethodGet: true, + http.MethodHead: true, + http.MethodPost: true, + http.MethodPut: true, + http.MethodPatch: true, + http.MethodDelete: true, + http.MethodConnect: true, + http.MethodOptions: true, + http.MethodTrace: true, +} + +var rejectedRequestsCount = prometheus.NewCounter( + prometheus.CounterOpts{ + Name: "gitlab_workhorse_unknown_method_rejected_requests", + Help: "The number of requests with unknown HTTP method which were rejected", + }, +) + +// 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 { + rejectedRequestsCount.Inc() + http.Error(w, http.StatusText(http.StatusMethodNotAllowed), http.StatusMethodNotAllowed) + } + }) +} diff --git a/workhorse/internal/rejectmethods/middleware_test.go b/workhorse/internal/rejectmethods/middleware_test.go new file mode 100644 index 00000000000..2921975aeef --- /dev/null +++ b/workhorse/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/workhorse/internal/upstream/upstream.go b/workhorse/internal/upstream/upstream.go index fd3f6191a5a..c81a21c0ecd 100644 --- a/workhorse/internal/upstream/upstream.go +++ b/workhorse/internal/upstream/upstream.go @@ -17,6 +17,7 @@ import ( "gitlab.com/gitlab-org/gitlab-workhorse/internal/config" "gitlab.com/gitlab-org/gitlab-workhorse/internal/helper" + "gitlab.com/gitlab-org/gitlab-workhorse/internal/rejectmethods" "gitlab.com/gitlab-org/gitlab-workhorse/internal/upload" "gitlab.com/gitlab-org/gitlab-workhorse/internal/upstream/roundtripper" "gitlab.com/gitlab-org/gitlab-workhorse/internal/urlprefix" @@ -63,6 +64,8 @@ func NewUpstream(cfg config.Config, accessLogger *logrus.Logger) http.Handler { } handler := correlation.InjectCorrelationID(&up, correlationOpts...) + // TODO: move to LabKit https://gitlab.com/gitlab-org/gitlab-workhorse/-/issues/339 + handler = rejectmethods.NewMiddleware(handler) return handler } diff --git a/workhorse/main_test.go b/workhorse/main_test.go index 16fa8ff10b7..d15af1d3e4c 100644 --- a/workhorse/main_test.go +++ b/workhorse/main_test.go @@ -642,6 +642,24 @@ func TestPropagateCorrelationIdHeader(t *testing.T) { } } +func TestRejectUnknownMethod(t *testing.T) { + ts := testhelper.TestServerWithHandler(regexp.MustCompile(`.`), func(w http.ResponseWriter, r *http.Request) { + w.WriteHeader(200) + }) + defer ts.Close() + ws := startWorkhorseServer(ts.URL) + defer ws.Close() + + req, err := http.NewRequest("UNKNOWN", ws.URL+"/api/v3/projects/123/repository/not/special", nil) + require.NoError(t, err) + + resp, err := http.DefaultClient.Do(req) + require.NoError(t, err) + defer resp.Body.Close() + + require.Equal(t, http.StatusMethodNotAllowed, resp.StatusCode) +} + func setupStaticFile(fpath, content string) error { return setupStaticFileHelper(fpath, content, testDocumentRoot) } -- cgit v1.2.3