From 447dd528f64d8bf481da9ac8445dd446bd4aa5c0 Mon Sep 17 00:00:00 2001
From: Kent 'picat' Gruber <kent@hashicorp.com>
Date: Wed, 14 Apr 2021 18:49:14 -0400
Subject: [PATCH] Merge pull request #10023 from hashicorp/fix-raw-kv-xss

Add content type headers to raw KV responses
---
 .changelog/10023.txt       |  3 ++
 agent/kvs_endpoint.go      | 13 +++++--
 agent/kvs_endpoint_test.go | 71 ++++++++++++++++++++++++++++++++++++++
 3 files changed, 85 insertions(+), 2 deletions(-)
 create mode 100644 .changelog/10023.txt

diff --git a/.changelog/10023.txt b/.changelog/10023.txt
new file mode 100644
index 0000000000..92d85dbd0b
--- /dev/null
+++ b/.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)
+```
\ No newline at end of file
diff --git a/agent/kvs_endpoint.go b/agent/kvs_endpoint.go
index feb6b7bfd2..2b54fb783e 100644
--- a/agent/kvs_endpoint.go
+++ b/agent/kvs_endpoint.go
@@ -80,11 +80,20 @@ func (s *HTTPServer) KVSGet(resp http.ResponseWriter, req *http.Request, args *s
 		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
 	}
diff --git a/agent/kvs_endpoint_test.go b/agent/kvs_endpoint_test.go
index ceb6d907f1..5a3017214a 100644
--- a/agent/kvs_endpoint_test.go
+++ b/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, "")