Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add content type headers to raw KV responses #10023

Merged
merged 4 commits into from
Apr 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .changelog/10023.txt
Original file line number Diff line number Diff line change
@@ -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)
```
13 changes: 11 additions & 2 deletions agent/kvs_endpoint.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down
71 changes: 71 additions & 0 deletions agent/kvs_endpoint_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down Expand Up @@ -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")
Expand Down
6 changes: 5 additions & 1 deletion website/content/api-docs/kv.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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��
Expand All @@ -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
Expand Down