Merge pull request #10023 from hashicorp/fix-raw-kv-xss

Add content type headers to raw KV responses

.changelog/10023.txt

@@ -0,0 +1,3 @@
+```release-note:security
+Add content-type headers to raw KV responses to prevent XSS attacks [CVE-2020-25864](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-25864)
+```

agent/kvs_endpoint.go

@@ -80,11 +80,20 @@ func (s *HTTPHandlers) KVSGet(resp http.ResponseWriter, req *http.Request, args
 		return nil, nil
 	}
 
-	// Check if we are in raw mode with a normal get, write out
-	// the raw body
+	// Check if we are in raw mode with a normal get, write out the raw body
+	// while setting the Content-Type, Content-Security-Policy, and
+	// X-Content-Type-Options headers to prevent XSS attacks from malicious KV
+	// entries. Otherwise, the net/http server will sniff the body to set the
+	// Content-Type. The nosniff option then indicates to the browser that it
+	// should also skip sniffing the body, otherwise it might ignore the Content-Type
+	// header in some situations. The sandbox option provides another layer of defense
+	// using the browser's content security policy to prevent code execution.
 	if _, ok := params["raw"]; ok && method == "KVS.Get" {
 		body := out.Entries[0].Value
 		resp.Header().Set("Content-Length", strconv.FormatInt(int64(len(body)), 10))
+		resp.Header().Set("Content-Type", "text/plain")
+		resp.Header().Set("X-Content-Type-Options", "nosniff")
+		resp.Header().Set("Content-Security-Policy", "sandbox")
 		resp.Write(body)
 		return nil, nil
 	}

agent/kvs_endpoint_test.go

@@ -422,6 +422,31 @@ func TestKVSEndpoint_GET_Raw(t *testing.T) {
 	}
 	assertIndex(t, resp)
 
+	// Check the headers
+	contentTypeHdr := resp.Header().Values("Content-Type")
+	if len(contentTypeHdr) != 1 {
+		t.Fatalf("expected 1 value for Content-Type header, got %d: %+v", len(contentTypeHdr), contentTypeHdr)
+	}
+	if contentTypeHdr[0] != "text/plain" {
+		t.Fatalf("expected Content-Type header to be \"text/plain\", got %q", contentTypeHdr[0])
+	}
+
+	optionsHdr := resp.Header().Values("X-Content-Type-Options")
+	if len(optionsHdr) != 1 {
+		t.Fatalf("expected 1 value for X-Content-Type-Options header, got %d: %+v", len(optionsHdr), optionsHdr)
+	}
+	if optionsHdr[0] != "nosniff" {
+		t.Fatalf("expected X-Content-Type-Options header to be \"nosniff\", got %q", optionsHdr[0])
+	}
+
+	cspHeader := resp.Header().Values("Content-Security-Policy")
+	if len(cspHeader) != 1 {
+		t.Fatalf("expected 1 value for Content-Security-Policy header, got %d: %+v", len(optionsHdr), optionsHdr)
+	}
+	if cspHeader[0] != "sandbox" {
+		t.Fatalf("expected X-Content-Type-Options header to be \"sandbox\", got %q", optionsHdr[0])
+	}
+
 	// Check the body
 	if !bytes.Equal(resp.Body.Bytes(), []byte("test")) {
 		t.Fatalf("bad: %s", resp.Body.Bytes())
@@ -447,6 +472,52 @@ func TestKVSEndpoint_PUT_ConflictingFlags(t *testing.T) {
 	}
 }
 
+func TestKVSEndpoint_GET(t *testing.T) {
+	if testing.Short() {
+		t.Skip("too slow for testing.Short")
+	}
+
+	t.Parallel()
+	a := NewTestAgent(t, "")
+	defer a.Shutdown()
+
+	buf := bytes.NewBuffer([]byte("test"))
+	req, _ := http.NewRequest("PUT", "/v1/kv/test", buf)
+	resp := httptest.NewRecorder()
+	obj, err := a.srv.KVSEndpoint(resp, req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	if res := obj.(bool); !res {
+		t.Fatalf("should work")
+	}
+
+	req, _ = http.NewRequest("GET", "/v1/kv/test", nil)
+	resp = httptest.NewRecorder()
+	_, err = a.srv.KVSEndpoint(resp, req)
+	if err != nil {
+		t.Fatalf("err: %v", err)
+	}
+	assertIndex(t, resp)
+
+	// The following headers are only included when returning a raw KV response
+
+	contentTypeHdr := resp.Header().Values("Content-Type")
+	if len(contentTypeHdr) != 0 {
+		t.Fatalf("expected no Content-Type header, got %d: %+v", len(contentTypeHdr), contentTypeHdr)
+	}
+
+	optionsHdr := resp.Header().Values("X-Content-Type-Options")
+	if len(optionsHdr) != 0 {
+		t.Fatalf("expected no X-Content-Type-Options header, got %d: %+v", len(optionsHdr), optionsHdr)
+	}
+
+	cspHeader := resp.Header().Values("Content-Security-Policy")
+	if len(cspHeader) != 0 {
+		t.Fatalf("expected no Content-Security-Policy header, got %d: %+v", len(optionsHdr), optionsHdr)
+	}
+}
+
 func TestKVSEndpoint_DELETE_ConflictingFlags(t *testing.T) {
 	t.Parallel()
 	a := NewTestAgent(t, "")

website/content/api-docs/kv.mdx

@@ -136,7 +136,7 @@ flags or want to implement a key-space explorer.
 #### Raw Response
 
 When using the `?raw` endpoint, the response is not `application/json`, but
-rather the content type of the uploaded content.
+is instead `text/plain`.
 
 ```
 )k<><6B><EFBFBD><EFBFBD><EFBFBD><EFBFBD>z^<5E>-<2D>ɑj<C991>q<EFBFBD><71><EFBFBD><EFBFBD>#u<>-R<>r<EFBFBD><72>T<EFBFBD>D<EFBFBD><44>٬<EFBFBD>Y<EFBFBD><59>l,<2C>ιK<CEB9><4B>Fm<46><6D>}<7D>#e<><65>
@@ -145,6 +145,10 @@ rather the content type of the uploaded content.
 (Yes, that is intentionally a bunch of gibberish characters to showcase the
 response)
 
+!> **Warning:** Consul versions before 1.9.5, 1.8.10 and 1.7.14 detected the content-type
+of the raw KV data which could be used for cross-site scripting (XSS) attacks. This is 
+identified publicly as CVE-2020-25864.
+
 ## Create/Update Key
 
 This endpoint updates the value of the specified key. If no key exists at the given