From 2ceef5cf0b8d2fc94795eede64d40b4c62bf437f Mon Sep 17 00:00:00 2001
From: coryb <cbennett@netflix.com>
Date: Sun, 11 Jul 2021 23:58:54 -0700
Subject: [PATCH] Add WithRedactedHeaders and WithoutHeaders options to
 NewClientTrace

On testing we learned that sensitive information was being stored
in the traces.  To prevent the security leak several security or
sensitive headers will now be redacted. These are the headers
redacted by default:
    Authorization
    WWW-Authenticate
    Proxy-Authenticate
    Proxy-Authorization
    Cookie
    Set-Cookie

Users can add more headers to redact with WithRedactedHeaders. To
disable adding headers to the span entirely users can use WithoutHeaders.
---
 CHANGELOG.md                                  |  2 +-
 .../httptrace/otelhttptrace/clienttrace.go    | 51 ++++++++++++++++---
 .../otelhttptrace/clienttrace_test.go         |  8 ++-
 3 files changed, 53 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 46684e0b29a..638013b3e3a 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -16,7 +16,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 
 ### Changed
 
-- Added `WithoutSubSpans` option to `NewClientTrace` in the `instrumentation/net/http/httptrace/otelhttptrace` package (#879)
+- Added `WithoutSubSpans`, `WithRedactedHeaders`, and `WithoutHeaders` options to `NewClientTrace` in the `instrumentation/net/http/httptrace/otelhttptrace` package (#879)
 
 ## [0.21.0] - 2021-06-18
 
diff --git a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go
index 52f45a8f76b..7075ea2cdbe 100644
--- a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go
+++ b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go
@@ -73,22 +73,53 @@ func WithoutSubSpans() ClientTraceOption {
 	}
 }
 
+// WithRedactedHeaders will be replaced by fixed '****' values for the header
+// names provided.  These are in addition to the sensitive headers already
+// redacted by default: Authorization, WWW-Authenticate, Proxy-Authenticate
+// Proxy-Authorization, Cookie, Set-Cookie
+func WithRedactedHeaders(headers ...string) ClientTraceOption {
+	return func(ct *clientTracer) {
+		for _, header := range headers {
+			ct.redactedHeaders[strings.ToLower(header)] = struct{}{}
+		}
+	}
+}
+
+// WithoutHeaders will disable adding span annotations for the http headers
+// and values.
+func WithoutHeaders() ClientTraceOption {
+	return func(ct *clientTracer) {
+		ct.addHeaders = false
+	}
+}
+
 type clientTracer struct {
 	context.Context
 
 	tr trace.Tracer
 
-	activeHooks map[string]context.Context
-	root        trace.Span
-	mtx         sync.Mutex
-	useSpans    bool
+	activeHooks     map[string]context.Context
+	root            trace.Span
+	mtx             sync.Mutex
+	redactedHeaders map[string]struct{}
+	addHeaders      bool
+	useSpans        bool
 }
 
 func NewClientTrace(ctx context.Context, opts ...ClientTraceOption) *httptrace.ClientTrace {
 	ct := &clientTracer{
 		Context:     ctx,
 		activeHooks: make(map[string]context.Context),
-		useSpans:    true,
+		redactedHeaders: map[string]struct{}{
+			"authorization":       {},
+			"www-authenticate":    {},
+			"proxy-authenticate":  {},
+			"proxy-authorization": {},
+			"cookie":              {},
+			"set-cookie":          {},
+		},
+		addHeaders: true,
+		useSpans:   true,
 	}
 	for _, opt := range opts {
 		opt(ct)
@@ -258,7 +289,15 @@ func (ct *clientTracer) wroteHeaderField(k string, v []string) {
 	if ct.useSpans && ct.span("http.headers") == nil {
 		ct.start("http.headers", "http.headers")
 	}
-	ct.root.SetAttributes(attribute.String("http."+strings.ToLower(k), sliceToString(v)))
+	if !ct.addHeaders {
+		return
+	}
+	k = strings.ToLower(k)
+	value := sliceToString(v)
+	if _, ok := ct.redactedHeaders[k]; ok {
+		value = "****"
+	}
+	ct.root.SetAttributes(attribute.String("http."+k, value))
 }
 
 func (ct *clientTracer) wroteHeaders() {
diff --git a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace_test.go b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace_test.go
index 7bf19aac5b1..3e97b106153 100644
--- a/instrumentation/net/http/httptrace/otelhttptrace/clienttrace_test.go
+++ b/instrumentation/net/http/httptrace/otelhttptrace/clienttrace_test.go
@@ -293,6 +293,7 @@ func TestWithoutSubSpans(t *testing.T) {
 	)
 	req, err = http.NewRequestWithContext(ctx, http.MethodGet, ts.URL, nil)
 	req.Header.Set("User-Agent", "oteltest/1.1")
+	req.Header.Set("Authorization", "Bearer token123")
 	require.NoError(t, err)
 	resp, err = ts.Client().Do(req)
 	require.NoError(t, err)
@@ -316,7 +317,7 @@ func TestWithoutSubSpans(t *testing.T) {
 	assert.Equal(t, expectedEventNames, gotEventNames)
 
 	gotAttributes := recSpan.Attributes()
-	require.Len(t, gotAttributes, 8)
+	require.Len(t, gotAttributes, 9)
 	assert.Equal(t,
 		attribute.StringValue("gzip"),
 		gotAttributes[attribute.Key("http.accept-encoding")],
@@ -325,6 +326,11 @@ func TestWithoutSubSpans(t *testing.T) {
 		attribute.StringValue("oteltest/1.1"),
 		gotAttributes[attribute.Key("http.user-agent")],
 	)
+	// verify redacted auth headers
+	assert.Equal(t,
+		attribute.StringValue("****"),
+		gotAttributes[attribute.Key("http.authorization")],
+	)
 	// value is dynamic, just verify we have the attribute
 	assert.Contains(t, gotAttributes, attribute.Key("http.conn.idletime"))
 	assert.Equal(t,