From dc937c953279bd40645d8ad020f41c6bf93df459 Mon Sep 17 00:00:00 2001 From: Kent 'picat' Gruber 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 +++++++++++++++++++++++++++++++++ website/content/api-docs/kv.mdx | 6 ++- 4 files changed, 90 insertions(+), 3 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 352f79116c..6534718d20 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 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, "") diff --git a/website/content/api-docs/kv.mdx b/website/content/api-docs/kv.mdx index 549abdf683..60a75d8f23 100644 --- a/website/content/api-docs/kv.mdx +++ b/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������z^�-�ɑj�q����#u�-R�r��T�D��٬�Y��l,�ιK��Fm��}�#e�� @@ -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