Backport of [Security] Fix XSS Vulnerability where content-type header wasn't explicitly set into release/1.19.x (#21709)

* backport of commit 52f4b86c5c

* backport of commit ede97520b0

* backport of commit 4446c25617

* backport of commit 957301e092

* backport of commit 55c0ece134

---------

Co-authored-by: Sarah Alsmiller <sarah.alsmiller@hashicorp.com>
pull/21732/head
hc-github-team-consul-core 2024-09-13 11:22:02 -04:00 committed by GitHub
parent 39c00d3271
commit 18354fc302
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
3 changed files with 65 additions and 5 deletions

3
.changelog/21704.txt Normal file
View File

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

View File

@ -6,6 +6,7 @@ package agent
import (
"encoding/json"
"fmt"
"github.com/hashicorp/go-hclog"
"io"
"net"
"net/http"
@ -42,6 +43,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"},
@ -219,6 +225,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
}
@ -227,6 +234,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
}
@ -236,6 +244,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
}
@ -316,6 +325,8 @@ func (s *HTTPHandlers) handler() http.Handler {
}
h = withRemoteAddrHandler(h)
h = ensureContentTypeHeader(h, s.agent.logger)
s.h = &wrappedMux{
mux: mux,
handler: h,
@ -336,6 +347,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
@ -379,6 +404,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
}
@ -410,6 +437,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
@ -606,6 +635,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
}
@ -617,6 +648,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
}
@ -627,7 +660,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)
}

View File

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