[Security] Fix XSS Vulnerability where content-type header wasn't exp…

…licitly 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

@@ -0,0 +1,3 @@
+```release-note:security
+Explicitly set 'Content-Type' header to mitigate XSS vulnerability.
+```

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)
 	}

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) {