From 07fae7bb0be8593cc98c38b1ef4a49ed9188932f Mon Sep 17 00:00:00 2001 From: sarahalsmiller <100602640+sarahalsmiller@users.noreply.github.com> Date: Wed, 11 Sep 2024 14:23:21 -0500 Subject: [PATCH] [Security] Fix XSS Vulnerability where content-type header wasn't explicitly set (#21704) * explicitly add content-type anywhere possible and add middleware to set and warn * added tests, fixed typo * clean up unused constants * changelog * fix call order in middleware --- .changelog/21704.txt | 3 +++ agent/http.go | 35 ++++++++++++++++++++++++++++++++++- agent/http_test.go | 32 ++++++++++++++++++++++++++++---- 3 files changed, 65 insertions(+), 5 deletions(-) create mode 100644 .changelog/21704.txt diff --git a/.changelog/21704.txt b/.changelog/21704.txt new file mode 100644 index 0000000000..4e42741ebb --- /dev/null +++ b/.changelog/21704.txt @@ -0,0 +1,3 @@ +```release-note:security +Explicitly set 'Content-Type' header to mitigate XSS vulnerability. +``` \ No newline at end of file diff --git a/agent/http.go b/agent/http.go index e7ba26825a..506377074a 100644 --- a/agent/http.go +++ b/agent/http.go @@ -6,6 +6,7 @@ package agent import ( "encoding/json" "fmt" + "github.com/hashicorp/go-hclog" "io" "net" "net/http" @@ -43,6 +44,11 @@ import ( "github.com/hashicorp/consul/proto/private/pbcommon" ) +const ( + contentTypeHeader = "Content-Type" + plainContentType = "text/plain; charset=utf-8" +) + var HTTPSummaries = []prometheus.SummaryDefinition{ { Name: []string{"api", "http"}, @@ -220,6 +226,7 @@ func (s *HTTPHandlers) handler() http.Handler { // If enableDebug register wrapped pprof handlers if !s.agent.enableDebug.Load() && s.checkACLDisabled() { resp.WriteHeader(http.StatusNotFound) + resp.Header().Set(contentTypeHeader, plainContentType) return } @@ -228,6 +235,7 @@ func (s *HTTPHandlers) handler() http.Handler { authz, err := s.agent.delegate.ResolveTokenAndDefaultMeta(token, nil, nil) if err != nil { + resp.Header().Set(contentTypeHeader, plainContentType) resp.WriteHeader(http.StatusForbidden) return } @@ -237,6 +245,7 @@ func (s *HTTPHandlers) handler() http.Handler { // TODO(partitions): should this be possible in a partition? // TODO(acl-error-enhancements): We should return error details somehow here. if authz.OperatorRead(nil) != acl.Allow { + resp.Header().Set(contentTypeHeader, plainContentType) resp.WriteHeader(http.StatusForbidden) return } @@ -317,6 +326,8 @@ func (s *HTTPHandlers) handler() http.Handler { } h = withRemoteAddrHandler(h) + h = ensureContentTypeHeader(h, s.agent.logger) + s.h = &wrappedMux{ mux: mux, handler: h, @@ -337,6 +348,20 @@ func withRemoteAddrHandler(next http.Handler) http.Handler { }) } +// Injects content type explicitly if not already set into response to prevent XSS +func ensureContentTypeHeader(next http.Handler, logger hclog.Logger) http.Handler { + + return http.HandlerFunc(func(resp http.ResponseWriter, req *http.Request) { + next.ServeHTTP(resp, req) + + val := resp.Header().Get(contentTypeHeader) + if val == "" { + resp.Header().Set(contentTypeHeader, plainContentType) + logger.Debug("warning: content-type header not explicitly set.", "request-path", req.URL) + } + }) +} + // nodeName returns the node name of the agent func (s *HTTPHandlers) nodeName() string { return s.agent.config.NodeName @@ -380,6 +405,8 @@ func (s *HTTPHandlers) wrap(handler endpoint, methods []string) http.HandlerFunc "from", req.RemoteAddr, "error", err, ) + //set response type to plain to prevent XSS + resp.Header().Set(contentTypeHeader, plainContentType) resp.WriteHeader(http.StatusInternalServerError) return } @@ -406,6 +433,8 @@ func (s *HTTPHandlers) wrap(handler endpoint, methods []string) http.HandlerFunc "from", req.RemoteAddr, "error", errMsg, ) + //set response type to plain to prevent XSS + resp.Header().Set(contentTypeHeader, plainContentType) resp.WriteHeader(http.StatusForbidden) fmt.Fprint(resp, errMsg) return @@ -585,6 +614,8 @@ func (s *HTTPHandlers) wrap(handler endpoint, methods []string) http.HandlerFunc resp.Header().Add("X-Consul-Reason", errPayload.Reason) } } else { + //set response type to plain to prevent XSS + resp.Header().Set(contentTypeHeader, plainContentType) handleErr(err) return } @@ -596,6 +627,8 @@ func (s *HTTPHandlers) wrap(handler endpoint, methods []string) http.HandlerFunc if contentType == "application/json" { buf, err = s.marshalJSON(req, obj) if err != nil { + //set response type to plain to prevent XSS + resp.Header().Set(contentTypeHeader, plainContentType) handleErr(err) return } @@ -606,7 +639,7 @@ func (s *HTTPHandlers) wrap(handler endpoint, methods []string) http.HandlerFunc } } } - resp.Header().Set("Content-Type", contentType) + resp.Header().Set(contentTypeHeader, contentType) resp.WriteHeader(httpCode) resp.Write(buf) } diff --git a/agent/http_test.go b/agent/http_test.go index 607061d868..497789f689 100644 --- a/agent/http_test.go +++ b/agent/http_test.go @@ -639,14 +639,14 @@ func TestHTTPAPIResponseHeaders(t *testing.T) { `) defer a.Shutdown() - requireHasHeadersSet(t, a, "/v1/agent/self") + requireHasHeadersSet(t, a, "/v1/agent/self", "application/json") // Check the Index page that just renders a simple message with UI disabled // also gets the right headers. - requireHasHeadersSet(t, a, "/") + requireHasHeadersSet(t, a, "/", "text/plain; charset=utf-8") } -func requireHasHeadersSet(t *testing.T, a *TestAgent, path string) { +func requireHasHeadersSet(t *testing.T, a *TestAgent, path string, contentType string) { t.Helper() resp := httptest.NewRecorder() @@ -661,6 +661,9 @@ func requireHasHeadersSet(t *testing.T, a *TestAgent, path string) { require.Equal(t, "1; mode=block", hdrs.Get("X-XSS-Protection"), "X-XSS-Protection header value incorrect") + + require.Equal(t, contentType, hdrs.Get("Content-Type"), + "") } func TestUIResponseHeaders(t *testing.T) { @@ -680,7 +683,28 @@ func TestUIResponseHeaders(t *testing.T) { `) defer a.Shutdown() - requireHasHeadersSet(t, a, "/ui") + //response header for the UI appears to be being handled by the UI itself. + requireHasHeadersSet(t, a, "/ui", "text/plain; charset=utf-8") +} + +func TestErrorContentTypeHeaderSet(t *testing.T) { + if testing.Short() { + t.Skip("too slow for testing.Short") + } + + t.Parallel() + a := NewTestAgent(t, ` + http_config { + response_headers = { + "Access-Control-Allow-Origin" = "*" + "X-XSS-Protection" = "1; mode=block" + "X-Frame-Options" = "SAMEORIGIN" + } + } + `) + defer a.Shutdown() + + requireHasHeadersSet(t, a, "/fake-path-doesn't-exist", "text/plain; charset=utf-8") } func TestAcceptEncodingGzip(t *testing.T) {