agent: prevent XSS by controlling Content-Type

command/agent/agent_endpoint.go

@@ -201,6 +201,14 @@ func (s *HTTPServer) AgentMonitor(resp http.ResponseWriter, req *http.Request) (
 		return nil, CodedError(400, "Cannot target node and server simultaneously")
 	}
 
+	// Force the Content-Type to avoid Go's http.ResponseWriter from
+	// detecting an incorrect or unsafe one.
+	if plainText {
+		resp.Header().Set("Content-Type", "text/plain")
+	} else {
+		resp.Header().Set("Content-Type", "application/json")
+	}
+
 	s.parse(resp, req, &args.QueryOptions.Region, &args.QueryOptions)
 
 	// Make the RPC

command/agent/fs_endpoint.go

@@ -33,12 +33,16 @@ func (s *HTTPServer) FsRequest(resp http.ResponseWriter, req *http.Request) (int
 	case strings.HasPrefix(path, "stat/"):
 		return s.FileStatRequest(resp, req)
 	case strings.HasPrefix(path, "readat/"):
-		return s.FileReadAtRequest(resp, req)
+		return s.wrapUntrustedContent(s.FileReadAtRequest)(resp, req)
 	case strings.HasPrefix(path, "cat/"):
-		return s.FileCatRequest(resp, req)
+		return s.wrapUntrustedContent(s.FileCatRequest)(resp, req)
 	case strings.HasPrefix(path, "stream/"):
 		return s.Stream(resp, req)
 	case strings.HasPrefix(path, "logs/"):
+		// Logs are *not* untrusted content because the endpoint
+		// explicitly sets the Content-Type to text/plain or
+		// application/json depending on the value of the ?plain=
+		// parameter.
 		return s.Logs(resp, req)
 	default:
 		return nil, CodedError(404, ErrInvalidMethod)
@@ -320,6 +324,14 @@ func (s *HTTPServer) Logs(resp http.ResponseWriter, req *http.Request) (interfac
 	}
 	s.parse(resp, req, &fsReq.QueryOptions.Region, &fsReq.QueryOptions)
 
+	// Force the Content-Type to avoid Go's http.ResponseWriter from
+	// detecting an incorrect or unsafe one.
+	if plain {
+		resp.Header().Set("Content-Type", "text/plain")
+	} else {
+		resp.Header().Set("Content-Type", "application/json")
+	}
+
 	// Make the request
 	return s.fsStreamImpl(resp, req, "FileSystem.Logs", fsReq, fsReq.AllocID)
 }

command/agent/http.go

@@ -18,6 +18,7 @@ import (
 	"github.com/gorilla/websocket"
 	"github.com/hashicorp/go-connlimit"
 	log "github.com/hashicorp/go-hclog"
+	"github.com/hashicorp/nomad/helper/noxssrw"
 	"github.com/hashicorp/nomad/helper/tlsutil"
 	"github.com/hashicorp/nomad/nomad/structs"
 	"github.com/rs/cors"
@@ -288,6 +289,12 @@ func (s *HTTPServer) registerHandlers(enableDebug bool) {
 	s.mux.HandleFunc("/v1/agent/servers", s.wrap(s.AgentServersRequest))
 	s.mux.HandleFunc("/v1/agent/keyring/", s.wrap(s.KeyringOperationRequest))
 	s.mux.HandleFunc("/v1/agent/health", s.wrap(s.HealthRequest))
+
+	// Monitor is *not* an untrusted endpoint despite the log contents
+	// potentially containing unsanitized user input. Monitor, like
+	// "/v1/client/fs/logs", explicitly sets a "text/plain" or
+	// "application/json" Content-Type depending on the ?plain= query
+	// parameter.
 	s.mux.HandleFunc("/v1/agent/monitor", s.wrap(s.AgentMonitor))
 
 	s.mux.HandleFunc("/v1/agent/pprof/", s.wrapNonJSON(s.AgentPprofRequest))
@@ -650,6 +657,27 @@ func (s *HTTPServer) parseWriteRequest(req *http.Request, w *structs.WriteReques
 	s.parseRegion(req, &w.Region)
 }
 
+// wrapUntrustedContent wraps handlers in a http.ResponseWriter that prevents
+// setting Content-Types that a browser may render (eg text/html). Any API that
+// returns service-generated content (eg /v1/client/fs/cat) must be wrapped.
+func (s *HTTPServer) wrapUntrustedContent(handler handlerFn) handlerFn {
+	return func(resp http.ResponseWriter, req *http.Request) (interface{}, error) {
+		resp, closeWriter := noxssrw.NewResponseWriter(resp)
+		defer func() {
+			if _, err := closeWriter(); err != nil {
+				// Can't write an error response at this point so just
+				// log. s.wrap does not even log when resp.Write fails,
+				// so log at low level.
+				s.logger.Debug("error writing HTTP response", "error", err,
+					"method", req.Method, "path", req.URL.String())
+			}
+		}()
+
+		// Call the wrapped handler
+		return handler(resp, req)
+	}
+}
+
 // wrapCORS wraps a HandlerFunc in allowCORS and returns a http.Handler
 func wrapCORS(f func(http.ResponseWriter, *http.Request)) http.Handler {
 	return allowCORS.Handler(http.HandlerFunc(f))

helper/noxssrw/noxssrw.go

@@ -0,0 +1,228 @@
+// Package noxssrw (No XSS ResponseWriter) behaves like the Go standard
+// library's ResponseWriter by detecting the Content-Type of a response if it
+// has not been explicitly set. However, unlike the standard library's
+// implementation, this implementation will never return the "text/html"
+// Content-Type and instead return "text/plain".
+package noxssrw
+
+import (
+	"net/http"
+	"strings"
+)
+
+var (
+	// DefaultUnsafeTypes are Content-Types that browsers will render as hypertext.
+	// Any Content-Types that allow Javascript or remote resource fetching must be
+	// converted to a Content-Type that prevents evaluation.
+	//
+	// Types are prefix matched to avoid comparing against specific
+	// character sets (eg "text/html; charset=utf-8") which may be user
+	// controlled.
+	DefaultUnsafeTypes = map[string]string{
+		"text/html":      "text/plain",
+		"text/xhtml":     "text/plain",
+		"text/xhtml+xml": "text/plain",
+	}
+
+	// DefaultHeaders contain CORS headers meant to prevent the execution
+	// of Javascript in compliant browsers.
+	DefaultHeaders = map[string]string{
+		"Content-Security-Policy": "default-src 'none'; style-src 'unsafe-inline'; sandbox",
+		"X-Content-Type-Options":  "nosniff",
+		"X-XSS-Protection":        "1; mode=block",
+	}
+)
+
+// NoXSSResponseWriter implements http.ResponseWriter but prevents renderable
+// Content-Types from being automatically detected. Create with
+// NewResponseWriter.
+type NoXSSResponseWriter struct {
+	// TypeMap maps types unsafe for untrusted content to their safe
+	// version; may be replaced but not mutated.
+	TypeMap map[string]string
+
+	// DefaultHeaders to set on first write if they are not already
+	// explicitly set.
+	DefaultHeaders map[string]string
+
+	// buffer up to 512 bytes before detecting Content-Type and writing
+	// response.
+	buf []byte
+
+	// subsequentWrite is true after the first Write is called
+	subsequentWrite bool
+
+	// flushed is true if Content-Type has been set and Writes may be
+	// passed through.
+	flushed bool
+
+	// original ResponseWriter being wrapped
+	orig http.ResponseWriter
+}
+
+// Header returns the header map that will be sent by
+// WriteHeader. The Header map also is the mechanism with which
+// Handlers can set HTTP trailers.
+//
+// Changing the header map after a call to WriteHeader (or
+// Write) has no effect unless the modified headers are
+// trailers.
+//
+// There are two ways to set Trailers. The preferred way is to
+// predeclare in the headers which trailers you will later
+// send by setting the "Trailer" header to the names of the
+// trailer keys which will come later. In this case, those
+// keys of the Header map are treated as if they were
+// trailers. See the example. The second way, for trailer
+// keys not known to the Handler until after the first Write,
+// is to prefix the Header map keys with the TrailerPrefix
+// constant value. See TrailerPrefix.
+//
+// To suppress automatic response headers (such as "Date"), set
+// their value to nil.
+func (w *NoXSSResponseWriter) Header() http.Header {
+	return w.orig.Header()
+}
+
+// Write writes the data to the connection as part of an HTTP reply.
+//
+// If WriteHeader has not yet been called, Write calls
+// WriteHeader(http.StatusOK) before writing the data. If the Header
+// does not contain a Content-Type line, Write adds a Content-Type set
+// to the result of passing the initial 512 bytes of written data to
+// DetectContentType. Additionally, if the total size of all written
+// data is under a few KB and there are no Flush calls, the
+// Content-Length header is added automatically.
+//
+// Depending on the HTTP protocol version and the client, calling
+// Write or WriteHeader may prevent future reads on the
+// Request.Body. For HTTP/1.x requests, handlers should read any
+// needed request body data before writing the response. Once the
+// headers have been flushed (due to either an explicit Flusher.Flush
+// call or writing enough data to trigger a flush), the request body
+// may be unavailable. For HTTP/2 requests, the Go HTTP server permits
+// handlers to continue to read the request body while concurrently
+// writing the response. However, such behavior may not be supported
+// by all HTTP/2 clients. Handlers should read before writing if
+// possible to maximize compatibility.
+func (w *NoXSSResponseWriter) Write(p []byte) (int, error) {
+	headers := w.Header()
+	// If first write, set any unset default headers. Do this on first write
+	// to allow overriding the default set of headers.
+	if !w.subsequentWrite {
+		for k, v := range w.DefaultHeaders {
+			if headers.Get(k) == "" {
+				headers.Set(k, v)
+			}
+		}
+		w.subsequentWrite = true
+	}
+
+	// If already flushed, write-through and short-circuit
+	if w.flushed {
+		return w.orig.Write(p)
+	}
+
+	// < 512 bytes available, buffer and wait for closing or a subsequent
+	// request
+	if len(w.buf)+len(p) < 512 {
+		w.buf = append(w.buf, p...)
+		return len(p), nil
+	}
+
+	// >= 512 bytes available, set the Content-Type and flush.
+	all := append(w.buf, p...)
+	contentType := http.DetectContentType(all)
+
+	// Prefix match to exclude the character set which may be user
+	// controlled.
+	for prefix, safe := range w.TypeMap {
+		if strings.HasPrefix(contentType, prefix) {
+			contentType = safe
+			break
+		}
+	}
+
+	// Set the Content-Type iff it was not already explicitly set
+	if headers.Get("Content-Type") == "" {
+		headers.Set("Content-Type", contentType)
+	}
+
+	// Write the buffer
+	n, err := w.orig.Write(w.buf)
+	if err != nil {
+		// Throw away part of buffer written successfully and
+		// inform caller p was not written at all
+		w.buf = w.buf[:n]
+		return 0, err
+	}
+
+	// Headers and buffer were written, this writer has been
+	// flushed and can be a passthrough
+	w.flushed = true
+	w.buf = w.buf[:]
+
+	// Write p
+	return w.orig.Write(p)
+}
+
+// Close and flush the writer. Necessary for responses that never reached 512
+// bytes.
+func (w *NoXSSResponseWriter) Close() (int, error) {
+	// If the buffer was already flushed this is a noop
+	if w.flushed {
+		return 0, nil
+	}
+
+	// Prefix match to exclude the character set which may be user
+	// controlled.
+	contentType := http.DetectContentType(w.buf)
+	for prefix, safe := range w.TypeMap {
+		if strings.HasPrefix(contentType, prefix) {
+			contentType = safe
+			break
+		}
+	}
+
+	// Set the Content-Type iff it was not already explicitly set
+	if headers := w.Header(); headers.Get("Content-Type") == "" {
+		headers.Set("Content-Type", contentType)
+	}
+
+	// Write the buffer
+	return w.orig.Write(w.buf)
+}
+
+// WriteHeader sends an HTTP response header with the provided
+// status code.
+//
+// If WriteHeader is not called explicitly, the first call to Write
+// will trigger an implicit WriteHeader(http.StatusOK).
+// Thus explicit calls to WriteHeader are mainly used to
+// send error codes.
+//
+// The provided code must be a valid HTTP 1xx-5xx status code.
+// Only one header may be written. Go does not currently
+// support sending user-defined 1xx informational headers,
+// with the exception of 100-continue response header that the
+// Server sends automatically when the Request.Body is read.
+func (w *NoXSSResponseWriter) WriteHeader(statusCode int) {
+	w.orig.WriteHeader(statusCode)
+}
+
+// NewResponseWriter creates a new ResponseWriter and Close func which will
+// prevent Go's http.ResponseWriter default behavior of detecting the
+// Content-Type.
+//
+// The Close func must be called to ensure that responses < 512 bytes are
+// flushed as up to 512 bytes are buffered without flushing.
+func NewResponseWriter(orig http.ResponseWriter) (http.ResponseWriter, func() (int, error)) {
+	w := &NoXSSResponseWriter{
+		TypeMap:        DefaultUnsafeTypes,
+		DefaultHeaders: DefaultHeaders,
+		buf:            make([]byte, 0, 512),
+		orig:           orig,
+	}
+
+	return w, w.Close
+}