From 91aefe50ab6dc8b59d10aca755d27c2bded5afe9 Mon Sep 17 00:00:00 2001
From: Kent 'picat' Gruber <kent@hashicorp.com>
Date: Wed, 14 Apr 2021 16:20:22 -0400
Subject: [PATCH] Add content type headers to raw KV responses

---
 agent/kvs_endpoint.go      | 13 +++++--
 agent/kvs_endpoint_test.go | 71 ++++++++++++++++++++++++++++++++++++++
 2 files changed, 82 insertions(+), 2 deletions(-)

diff --git a/agent/kvs_endpoint.go b/agent/kvs_endpoint.go
index 352f79116..6534718d2 100644
--- a/agent/kvs_endpoint.go
+++ b/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
 	}
diff --git a/agent/kvs_endpoint_test.go b/agent/kvs_endpoint_test.go
index d00c70f0c..6f4421c6b 100644
--- a/agent/kvs_endpoint_test.go
+++ b/agent/kvs_endpoint_test.go
@@ -450,6 +450,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())
@@ -479,6 +504,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) {
 	if testing.Short() {
 		t.Skip("too slow for testing.Short")