From 5dabc6a036d9f7c952582223d705a82e013bd3e6 Mon Sep 17 00:00:00 2001 From: Andrew Wilkins Date: Thu, 25 Oct 2018 18:03:38 +0800 Subject: [PATCH] Record all HTTP request/response headers Record all HTTP request/response headers, with the exception of "Cookie", which we explode and store separately. All headers pass through the field sanitiser. If a header is included multiple times, we store the values as an array of strings. Otherwise, we store just a single string value. --- context.go | 33 ++++++++------ docs/configuration.asciidoc | 4 +- env.go | 2 + model/marshal.go | 68 ++++++++++++++++++++++++++++ model/marshal_fastjson.go | 51 +-------------------- model/marshal_test.go | 30 ++++++------ model/model.go | 26 ++++------- modelwriter.go | 9 +++- module/apmecho/middleware_test.go | 14 +++--- module/apmgin/middleware_test.go | 14 +++--- module/apmgorilla/middleware_test.go | 13 ++++-- module/apmhttp/handler_test.go | 20 +++++--- module/apmhttprouter/handler_test.go | 7 +-- module/apmrestful/filter_test.go | 17 ++++--- sanitizer.go | 37 ++++++++------- sanitizer_test.go | 27 ++++++++--- 16 files changed, 221 insertions(+), 151 deletions(-) diff --git a/context.go b/context.go index 782ac0c8b..db128e747 100644 --- a/context.go +++ b/context.go @@ -3,7 +3,6 @@ package apm import ( "fmt" "net/http" - "strings" "go.elastic.co/apm/internal/apmhttputil" "go.elastic.co/apm/model" @@ -14,10 +13,8 @@ type Context struct { model model.Context request model.Request requestBody model.RequestBody - requestHeaders model.RequestHeaders requestSocket model.RequestSocket response model.Response - responseHeaders model.ResponseHeaders user model.User service model.Service serviceFramework model.Framework @@ -46,6 +43,12 @@ func (c *Context) reset() { *c = Context{ model: modelContext, captureBodyMask: c.captureBodyMask, + request: model.Request{ + Headers: c.request.Headers[:0], + }, + response: model.Response{ + Headers: c.response.Headers[:0], + }, } } @@ -140,13 +143,14 @@ func (c *Context) SetHTTPRequest(req *http.Request) { } c.model.Request = &c.request - c.requestHeaders = model.RequestHeaders{ - ContentType: req.Header.Get("Content-Type"), - Cookie: truncateString(strings.Join(req.Header["Cookie"], ";")), - UserAgent: req.UserAgent(), - } - if c.requestHeaders != (model.RequestHeaders{}) { - c.request.Headers = &c.requestHeaders + for k, values := range req.Header { + if k == "Cookie" { + // We capture cookies in the request structure. + continue + } + c.request.Headers = append(c.request.Headers, model.Header{ + Key: k, Values: values, + }) } c.requestSocket = model.RequestSocket{ @@ -180,9 +184,12 @@ func (c *Context) SetHTTPRequestBody(bc *BodyCapturer) { // SetHTTPResponseHeaders sets the HTTP response headers in the context. func (c *Context) SetHTTPResponseHeaders(h http.Header) { - c.responseHeaders.ContentType = h.Get("Content-Type") - if c.responseHeaders.ContentType != "" { - c.response.Headers = &c.responseHeaders + for k, values := range h { + c.response.Headers = append(c.response.Headers, model.Header{ + Key: k, Values: values, + }) + } + if len(c.response.Headers) != 0 { c.model.Response = &c.response } } diff --git a/docs/configuration.asciidoc b/docs/configuration.asciidoc index a138285b9..12e2b8373 100644 --- a/docs/configuration.asciidoc +++ b/docs/configuration.asciidoc @@ -183,8 +183,8 @@ Prefixing a pattern with `(?-i)` makes the matching case sensitive. [options="header"] |============ -| Environment | Default | Example -| `ELASTIC_APM_SANITIZE_FIELD_NAMES` | `password, passwd, pwd, secret, *key, *token*, *session*, *credit*, *card*` | `sekrits` +| Environment | Default | Example +| `ELASTIC_APM_SANITIZE_FIELD_NAMES` | `password, passwd, pwd, secret, *key, *token*, *session*, *credit*, *card*, authorization, set-cookie` | `sekrits` |============ A list of patterns to match the names of HTTP cookies and POST form fields to refact. diff --git a/env.go b/env.go index 05e6392ff..f4a3f6007 100644 --- a/env.go +++ b/env.go @@ -54,6 +54,8 @@ var ( "*session*", "*credit*", "*card*", + "authorization", + "set-cookie", }, ",")) ) diff --git a/model/marshal.go b/model/marshal.go index de9e06e35..e7b43715b 100644 --- a/model/marshal.go +++ b/model/marshal.go @@ -295,6 +295,74 @@ func (c *Cookies) UnmarshalJSON(data []byte) error { return nil } +func (hs Headers) isZero() bool { + return len(hs) == 0 +} + +// MarshalFastJSON writes the JSON representation of h to w. +func (hs Headers) MarshalFastJSON(w *fastjson.Writer) error { + w.RawByte('{') + for i, h := range hs { + if i != 0 { + w.RawByte(',') + } + w.String(h.Key) + w.RawByte(':') + if len(h.Values) == 1 { + // Just one item, add the item directly. + w.String(h.Values[0]) + } else { + // Zero or multiple items, include them all. + w.RawByte('[') + for i, v := range h.Values { + if i != 0 { + w.RawByte(',') + } + w.String(v) + } + w.RawByte(']') + } + } + w.RawByte('}') + return nil +} + +// MarshalFastJSON writes the JSON representation of h to w. +func (*Header) MarshalFastJSON(w *fastjson.Writer) error { + panic("unreachable") +} + +// UnmarshalJSON unmarshals the JSON data into c. +func (hs *Headers) UnmarshalJSON(data []byte) error { + var m map[string]interface{} + if err := json.Unmarshal(data, &m); err != nil { + return err + } + for k, v := range m { + switch v := v.(type) { + case string: + *hs = append(*hs, Header{Key: k, Values: []string{v}}) + case []interface{}: + var values []string + for _, v := range v { + switch v := v.(type) { + case string: + values = append(values, v) + default: + return errors.Errorf("expected string, got %T", v) + } + } + *hs = append(*hs, Header{Key: k, Values: values}) + default: + return errors.Errorf("expected string or []string, got %T", v) + } + } + sort.Slice(*hs, func(i, j int) bool { + return (*hs)[i].Key < (*hs)[j].Key + }) + return nil +} + // MarshalFastJSON writes the JSON representation of c to w. func (c *ExceptionCode) MarshalFastJSON(w *fastjson.Writer) error { if c.String != "" { diff --git a/model/marshal_fastjson.go b/model/marshal_fastjson.go index 17af42a91..5583619a4 100644 --- a/model/marshal_fastjson.go +++ b/model/marshal_fastjson.go @@ -806,7 +806,7 @@ func (v *Request) MarshalFastJSON(w *fastjson.Writer) error { } w.RawByte('}') } - if v.Headers != nil { + if !v.Headers.isZero() { w.RawString(",\"headers\":") if err := v.Headers.MarshalFastJSON(w); err != nil && firstErr == nil { firstErr = err @@ -826,43 +826,6 @@ func (v *Request) MarshalFastJSON(w *fastjson.Writer) error { return firstErr } -func (v *RequestHeaders) MarshalFastJSON(w *fastjson.Writer) error { - w.RawByte('{') - first := true - if v.ContentType != "" { - const prefix = ",\"content-type\":" - if first { - first = false - w.RawString(prefix[1:]) - } else { - w.RawString(prefix) - } - w.String(v.ContentType) - } - if v.Cookie != "" { - const prefix = ",\"cookie\":" - if first { - first = false - w.RawString(prefix[1:]) - } else { - w.RawString(prefix) - } - w.String(v.Cookie) - } - if v.UserAgent != "" { - const prefix = ",\"user-agent\":" - if first { - first = false - w.RawString(prefix[1:]) - } else { - w.RawString(prefix) - } - w.String(v.UserAgent) - } - w.RawByte('}') - return nil -} - func (v *RequestSocket) MarshalFastJSON(w *fastjson.Writer) error { w.RawByte('{') first := true @@ -904,7 +867,7 @@ func (v *Response) MarshalFastJSON(w *fastjson.Writer) error { } w.Bool(*v.Finished) } - if v.Headers != nil { + if !v.Headers.isZero() { const prefix = ",\"headers\":" if first { first = false @@ -940,16 +903,6 @@ func (v *Response) MarshalFastJSON(w *fastjson.Writer) error { return firstErr } -func (v *ResponseHeaders) MarshalFastJSON(w *fastjson.Writer) error { - w.RawByte('{') - if v.ContentType != "" { - w.RawString("\"content-type\":") - w.String(v.ContentType) - } - w.RawByte('}') - return nil -} - func (v *Metrics) MarshalFastJSON(w *fastjson.Writer) error { var firstErr error w.RawByte('{') diff --git a/model/marshal_test.go b/model/marshal_test.go index ad0b2c45d..a78b7a0b3 100644 --- a/model/marshal_test.go +++ b/model/marshal_test.go @@ -50,8 +50,8 @@ func TestMarshalTransaction(t *testing.T) { }, "method": "GET", "headers": map[string]interface{}{ - "user-agent": "Mosaic/0.2 (Windows 3.1)", - "cookie": "monster=yumyum; random=junk", + "User-Agent": "Mosaic/0.2 (Windows 3.1)", + "Cookie": "monster=yumyum; random=junk", }, "body": "ahoj", "http_version": "1.1", @@ -67,7 +67,7 @@ func TestMarshalTransaction(t *testing.T) { "response": map[string]interface{}{ "status_code": float64(418), "headers": map[string]interface{}{ - "content-type": "text/html", + "Content-Type": "text/html", }, }, "user": map[string]interface{}{ @@ -393,16 +393,17 @@ func TestMarshalResponse(t *testing.T) { headersSent := true response := model.Response{ Finished: &finished, - Headers: &model.ResponseHeaders{ - ContentType: "text/plain", - }, + Headers: model.Headers{{ + Key: "Content-Type", + Values: []string{"text/plain"}, + }}, HeadersSent: &headersSent, StatusCode: 200, } var w fastjson.Writer response.MarshalFastJSON(&w) assert.Equal(t, - `{"finished":true,"headers":{"content-type":"text/plain"},"headers_sent":true,"status_code":200}`, + `{"finished":true,"headers":{"Content-Type":"text/plain"},"headers_sent":true,"status_code":200}`, string(w.Bytes()), ) } @@ -507,10 +508,11 @@ func fakeTransaction() model.Transaction { Hash: "qux", }, Method: "GET", - Headers: &model.RequestHeaders{ - UserAgent: "Mosaic/0.2 (Windows 3.1)", - Cookie: "monster=yumyum; random=junk", - }, + Headers: model.Headers{{ + Key: "Cookie", Values: []string{"monster=yumyum; random=junk"}, + }, { + Key: "User-Agent", Values: []string{"Mosaic/0.2 (Windows 3.1)"}, + }}, Body: &model.RequestBody{ Raw: "ahoj", }, @@ -526,9 +528,9 @@ func fakeTransaction() model.Transaction { }, Response: &model.Response{ StatusCode: 418, - Headers: &model.ResponseHeaders{ - ContentType: "text/html", - }, + Headers: model.Headers{{ + Key: "Content-Type", Values: []string{"text/html"}, + }}, }, User: &model.User{ Username: "wanda", diff --git a/model/model.go b/model/model.go index 75ff47fb1..5e084cb71 100644 --- a/model/model.go +++ b/model/model.go @@ -396,7 +396,7 @@ type Request struct { Method string `json:"method"` // Headers holds the request headers. - Headers *RequestHeaders `json:"headers,omitempty"` + Headers Headers `json:"headers,omitempty"` // Body holds the request body, if body capture is enabled. Body *RequestBody `json:"body,omitempty"` @@ -429,17 +429,13 @@ type RequestBody struct { Form url.Values } -// RequestHeaders holds a limited subset of HTTP request headers. -type RequestHeaders struct { - // ContentType holds the content-type header. - ContentType string `json:"content-type,omitempty"` +// Headers holds a collection of HTTP headers. +type Headers []Header - // Cookie holds the cookies sent with the request, - // delimited by semi-colons. - Cookie string `json:"cookie,omitempty"` - - // UserAgent holds the user-agent header. - UserAgent string `json:"user-agent,omitempty"` +// Header holds an HTTP header, with one or more values. +type Header struct { + Key string + Values []string } // RequestSocket holds transport-level information relating to an HTTP request. @@ -485,7 +481,7 @@ type Response struct { StatusCode int `json:"status_code,omitempty"` // Headers holds the response headers. - Headers *ResponseHeaders `json:"headers,omitempty"` + Headers Headers `json:"headers,omitempty"` // HeadersSent indicates whether or not headers were sent // to the client. @@ -495,12 +491,6 @@ type Response struct { Finished *bool `json:"finished,omitempty"` } -// ResponseHeaders holds a limited subset of HTTP respponse headers. -type ResponseHeaders struct { - // ContentType holds the content-type header. - ContentType string `json:"content-type,omitempty"` -} - // Time is a timestamp, formatted as a number of microseconds since January 1, 1970 UTC. type Time time.Time diff --git a/modelwriter.go b/modelwriter.go index 98768a6fd..feb078645 100644 --- a/modelwriter.go +++ b/modelwriter.go @@ -91,8 +91,13 @@ func (w *modelWriter) buildModelTransaction(out *model.Transaction, tx *Transact } out.Context = tx.Context.build() - if len(w.cfg.sanitizedFieldNames) != 0 && out.Context != nil && out.Context.Request != nil { - sanitizeRequest(out.Context.Request, w.cfg.sanitizedFieldNames) + if len(w.cfg.sanitizedFieldNames) != 0 && out.Context != nil { + if out.Context.Request != nil { + sanitizeRequest(out.Context.Request, w.cfg.sanitizedFieldNames) + } + if out.Context.Response != nil { + sanitizeResponse(out.Context.Response, w.cfg.sanitizedFieldNames) + } } } diff --git a/module/apmecho/middleware_test.go b/module/apmecho/middleware_test.go index cf76f9b0f..965237c32 100644 --- a/module/apmecho/middleware_test.go +++ b/module/apmecho/middleware_test.go @@ -73,15 +73,17 @@ func TestEchoMiddleware(t *testing.T) { }, Method: "GET", HTTPVersion: "1.1", - Headers: &model.RequestHeaders{ - UserAgent: "apmecho_test", - }, + Headers: model.Headers{{ + Key: "User-Agent", + Values: []string{"apmecho_test"}, + }}, }, Response: &model.Response{ StatusCode: 418, - Headers: &model.ResponseHeaders{ - ContentType: "text/plain; charset=UTF-8", - }, + Headers: model.Headers{{ + Key: "Content-Type", + Values: []string{"text/plain; charset=UTF-8"}, + }}, }, }, transaction.Context) } diff --git a/module/apmgin/middleware_test.go b/module/apmgin/middleware_test.go index 5860b7fec..67e05b43f 100644 --- a/module/apmgin/middleware_test.go +++ b/module/apmgin/middleware_test.go @@ -84,16 +84,18 @@ func TestMiddleware(t *testing.T) { Path: "/hello/isbel", }, Method: "GET", - Headers: &model.RequestHeaders{ - UserAgent: "apmgin_test", - }, + Headers: model.Headers{{ + Key: "User-Agent", + Values: []string{"apmgin_test"}, + }}, HTTPVersion: "1.1", }, Response: &model.Response{ StatusCode: 200, - Headers: &model.ResponseHeaders{ - ContentType: "text/plain; charset=utf-8", - }, + Headers: model.Headers{{ + Key: "Content-Type", + Values: []string{"text/plain; charset=utf-8"}, + }}, }, }, transaction.Context) } diff --git a/module/apmgorilla/middleware_test.go b/module/apmgorilla/middleware_test.go index 090240d4b..50a785821 100644 --- a/module/apmgorilla/middleware_test.go +++ b/module/apmgorilla/middleware_test.go @@ -46,14 +46,19 @@ func TestMuxMiddleware(t *testing.T) { Path: "/prefix/articles/fiction/123", Search: "foo=123", }, - Method: "GET", + Method: "GET", + Headers: model.Headers{{ + Key: "X-Real-Ip", + Values: []string{"client.testing"}, + }}, HTTPVersion: "1.1", }, Response: &model.Response{ StatusCode: 200, - Headers: &model.ResponseHeaders{ - ContentType: "text/plain; charset=utf-8", - }, + Headers: model.Headers{{ + Key: "Content-Type", + Values: []string{"text/plain; charset=utf-8"}, + }}, }, }, transaction.Context) } diff --git a/module/apmhttp/handler_test.go b/module/apmhttp/handler_test.go index 2ab36ca65..0dfe2108b 100644 --- a/module/apmhttp/handler_test.go +++ b/module/apmhttp/handler_test.go @@ -77,9 +77,10 @@ func TestHandler(t *testing.T) { Path: "/foo", }, Method: "GET", - Headers: &model.RequestHeaders{ - UserAgent: "apmhttp_test", - }, + Headers: model.Headers{{ + Key: "User-Agent", + Values: []string{"apmhttp_test"}, + }}, HTTPVersion: "1.1", }, Response: &model.Response{ @@ -136,9 +137,16 @@ func TestHandlerHTTP2(t *testing.T) { Path: "/foo", }, Method: "GET", - Headers: &model.RequestHeaders{ - UserAgent: "Go-http-client/2.0", - }, + Headers: model.Headers{{ + Key: "Accept-Encoding", + Values: []string{"gzip"}, + }, { + Key: "User-Agent", + Values: []string{"Go-http-client/2.0"}, + }, { + Key: "X-Real-Ip", + Values: []string{"client.testing"}, + }}, HTTPVersion: "2.0", }, Response: &model.Response{ diff --git a/module/apmhttprouter/handler_test.go b/module/apmhttprouter/handler_test.go index ab8fe4094..0fcfee357 100644 --- a/module/apmhttprouter/handler_test.go +++ b/module/apmhttprouter/handler_test.go @@ -56,9 +56,10 @@ func TestWrap(t *testing.T) { Path: "/hello/go/go/bananas", }, Method: "GET", - Headers: &model.RequestHeaders{ - UserAgent: "apmhttp_test", - }, + Headers: model.Headers{{ + Key: "User-Agent", + Values: []string{"apmhttp_test"}, + }}, HTTPVersion: "1.1", }, Response: &model.Response{ diff --git a/module/apmrestful/filter_test.go b/module/apmrestful/filter_test.go index fe0979064..eda70dac7 100644 --- a/module/apmrestful/filter_test.go +++ b/module/apmrestful/filter_test.go @@ -106,15 +106,20 @@ func TestContainerFilter(t *testing.T) { }, Method: "GET", HTTPVersion: "1.1", - Headers: &model.RequestHeaders{ - UserAgent: "Go-http-client/1.1", - }, + Headers: model.Headers{{ + Key: "Accept-Encoding", + Values: []string{"gzip"}, + }, { + Key: "User-Agent", + Values: []string{"Go-http-client/1.1"}, + }}, }, Response: &model.Response{ StatusCode: 418, - Headers: &model.ResponseHeaders{ - ContentType: "application/json", - }, + Headers: model.Headers{{ + Key: "Content-Type", + Values: []string{"application/json"}, + }}, }, }, transaction.Context) } diff --git a/sanitizer.go b/sanitizer.go index 7378f2da9..a928eb35b 100644 --- a/sanitizer.go +++ b/sanitizer.go @@ -1,36 +1,23 @@ package apm import ( - "bytes" - "go.elastic.co/apm/internal/wildcard" "go.elastic.co/apm/model" ) const redacted = "[REDACTED]" -// sanitizeRequest sanitizes HTTP request data, redacting -// the values of cookies and forms whose corresponding keys +// sanitizeRequest sanitizes HTTP request data, redacting the +// values of cookies, headers and forms whose corresponding keys // match any of the given wildcard patterns. func sanitizeRequest(r *model.Request, matchers wildcard.Matchers) { - var anyCookiesRedacted bool for _, c := range r.Cookies { if !matchers.MatchAny(c.Name) { continue } c.Value = redacted - anyCookiesRedacted = true - } - if anyCookiesRedacted && r.Headers != nil { - var b bytes.Buffer - for i, c := range r.Cookies { - if i != 0 { - b.WriteRune(';') - } - b.WriteString(c.String()) - } - r.Headers.Cookie = b.String() } + sanitizeHeaders(r.Headers, matchers) if r.Body != nil && r.Body.Form != nil { for key, values := range r.Body.Form { if !matchers.MatchAny(key) { @@ -42,3 +29,21 @@ func sanitizeRequest(r *model.Request, matchers wildcard.Matchers) { } } } + +// sanitizeResponse sanitizes HTTP response data, redacting +// the values of response headers whose corresponding keys +// match any of the given wildcard patterns. +func sanitizeResponse(r *model.Response, matchers wildcard.Matchers) { + sanitizeHeaders(r.Headers, matchers) +} + +func sanitizeHeaders(headers model.Headers, matchers wildcard.Matchers) { + for i := range headers { + h := &headers[i] + if !matchers.MatchAny(h.Key) || len(h.Values) == 0 { + continue + } + h.Values = h.Values[:1] + h.Values[0] = redacted + } +} diff --git a/sanitizer_test.go b/sanitizer_test.go index c766c3802..69afd4a13 100644 --- a/sanitizer_test.go +++ b/sanitizer_test.go @@ -12,18 +12,27 @@ import ( "go.elastic.co/apm/transport/transporttest" ) -func TestSanitizeRequest(t *testing.T) { +func TestSanitizeRequestResponse(t *testing.T) { tracer, transport := transporttest.NewRecorderTracer() defer tracer.Close() mux := http.NewServeMux() mux.Handle("/", http.HandlerFunc(func(w http.ResponseWriter, req *http.Request) { + http.SetCookie(w, &http.Cookie{ + Name: "foo", + Value: "bar", + }) + http.SetCookie(w, &http.Cookie{ + Name: "baz", + Value: "qux", + }) w.WriteHeader(http.StatusTeapot) })) h := apmhttp.Wrap(mux, apmhttp.WithTracer(tracer)) w := httptest.NewRecorder() req, _ := http.NewRequest("GET", "http://server.testing/", nil) + req.SetBasicAuth("foo", "bar") for _, c := range []*http.Cookie{ {Name: "secret", Value: "top"}, {Name: "Custom-Credit-Card-Number", Value: "top"}, @@ -44,10 +53,17 @@ func TestSanitizeRequest(t *testing.T) { {Name: "sessionid", Value: "[REDACTED]"}, {Name: "user_id", Value: "456"}, }) - assert.Equal(t, - "secret=[REDACTED];Custom-Credit-Card-Number=[REDACTED];sessionid=[REDACTED];user_id=456", - tx.Context.Request.Headers.Cookie, - ) + assert.Equal(t, model.Headers{{ + Key: "Authorization", + Values: []string{"[REDACTED]"}, + }}, tx.Context.Request.Headers) + + // NOTE: the response includes multiple Set-Cookie headers, + // but we only report a single "[REDACTED]" value. + assert.Equal(t, model.Headers{{ + Key: "Set-Cookie", + Values: []string{"[REDACTED]"}, + }}, tx.Context.Response.Headers) } func TestSetSanitizedFieldNamesNone(t *testing.T) { @@ -85,5 +101,4 @@ func testSetSanitizedFieldNames(t *testing.T, expect string, sanitized ...string assert.Equal(t, tx.Context.Request.Cookies, model.Cookies{ {Name: "secret", Value: expect}, }) - assert.Equal(t, "secret="+expect, tx.Context.Request.Headers.Cookie) }