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.

Signed-off-by: coryb <cbennett@netflix.com>

CHANGELOG.md

@@ -14,9 +14,13 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   The `messaging.message_id` and `messaging.kafka.partition` attributes are now not set if a message was not processed. (#754) (#755)
 - Fix `otelsarama.WrapAsyncProducer` so that the messages from the `Errors` channel contain the original `Metadata`. (#754)
 
+### Added
+
+- Options `WithoutSubSpans`, `WithRedactedHeaders`, `WithoutHeaders`, and `WithInsecureHeaders` added to `NewClientTrace` in the `instrumentation/net/http/httptrace/otelhttptrace` package. (#879)
+
 ### Changed
 
-- Added `WithoutSubSpans` option to `NewClientTrace` in the `instrumentation/net/http/httptrace/otelhttptrace` package (#879)
+- `instrumentation/net/http/httptrace/otelhttptrace` will now redact known sensitive headers by default. (#879)
 
 ## [0.21.0] - 2021-06-18
 

instrumentation/net/http/httptrace/otelhttptrace/clienttrace.go

@@ -73,22 +73,65 @@ 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 attributes for the http headers
+// and values.
+func WithoutHeaders() ClientTraceOption {
+	return func(ct *clientTracer) {
+		ct.addHeaders = false
+	}
+}
+
+// WithInsecureHeaders will add span attributes for all http headers *INCLUDING*
+// the sensitive headers that are redacted by default.  The attribute values
+// will include the raw un-redacted text.  This might be useful for
+// debugging authentication related issues, but should not be used for
+// production deployments.
+func WithInsecureHeaders() ClientTraceOption {
+	return func(ct *clientTracer) {
+		ct.addHeaders = true
+		ct.redactedHeaders = nil
+	}
+}
+
 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 +301,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() {

instrumentation/net/http/httptrace/otelhttptrace/clienttrace_test.go

@@ -255,7 +255,7 @@ func TestEndBeforeStartCreatesSpan(t *testing.T) {
 	require.Len(t, spans, 1)
 }
 
-func TestWithoutSubSpans(t *testing.T) {
+func TestClientTraceOptions(t *testing.T) {
 	sr := &oteltest.SpanRecorder{}
 	otel.SetTracerProvider(
 		oteltest.NewTracerProvider(oteltest.WithSpanRecorder(sr)),
@@ -269,6 +269,8 @@ func TestWithoutSubSpans(t *testing.T) {
 	defer ts.Close()
 	address := ts.Listener.Addr().String()
 
+	// Test WithoutSubSpans
+
 	ctx := context.Background()
 	ctx = httptrace.WithClientTrace(ctx,
 		otelhttptrace.NewClientTrace(ctx,
@@ -293,6 +295,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)
@@ -303,7 +306,7 @@ func TestWithoutSubSpans(t *testing.T) {
 	recSpan := sr.Completed()[0]
 
 	gotAttributes := recSpan.Attributes()
-	require.Len(t, gotAttributes, 3)
+	require.Len(t, gotAttributes, 4)
 	assert.Equal(t,
 		attribute.StringValue("gzip"),
 		gotAttributes[attribute.Key("http.accept-encoding")],
@@ -312,6 +315,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")],
+	)
 	assert.Equal(t,
 		attribute.StringValue(address),
 		gotAttributes[attribute.Key("http.host")],
@@ -364,4 +372,75 @@ func TestWithoutSubSpans(t *testing.T) {
 			})
 		}
 	}
+
+	// Test WithRedactedHeaders
+
+	ctx, span = otel.Tracer("oteltest").Start(context.Background(), "root")
+	ctx = httptrace.WithClientTrace(ctx,
+		otelhttptrace.NewClientTrace(ctx,
+			otelhttptrace.WithoutSubSpans(),
+			otelhttptrace.WithRedactedHeaders("user-agent"),
+		),
+	)
+	req, err = http.NewRequestWithContext(ctx, http.MethodGet, ts.URL, nil)
+	require.NoError(t, err)
+	resp, err = ts.Client().Do(req)
+	require.NoError(t, err)
+	resp.Body.Close()
+	span.End()
+	require.Len(t, sr.Completed(), 2)
+	recSpan = sr.Completed()[1]
+
+	gotAttributes = recSpan.Attributes()
+	assert.Equal(t,
+		attribute.StringValue("****"),
+		gotAttributes[attribute.Key("http.user-agent")],
+	)
+
+	// Test WithoutHeaders
+
+	ctx, span = otel.Tracer("oteltest").Start(context.Background(), "root")
+	ctx = httptrace.WithClientTrace(ctx,
+		otelhttptrace.NewClientTrace(ctx,
+			otelhttptrace.WithoutSubSpans(),
+			otelhttptrace.WithoutHeaders(),
+		),
+	)
+	req, err = http.NewRequestWithContext(ctx, http.MethodGet, ts.URL, nil)
+	require.NoError(t, err)
+	resp, err = ts.Client().Do(req)
+	require.NoError(t, err)
+	resp.Body.Close()
+	span.End()
+	require.Len(t, sr.Completed(), 3)
+	recSpan = sr.Completed()[2]
+
+	gotAttributes = recSpan.Attributes()
+	require.Len(t, gotAttributes, 0)
+
+	// Test WithInsecureHeaders
+
+	ctx, span = otel.Tracer("oteltest").Start(context.Background(), "root")
+	ctx = httptrace.WithClientTrace(ctx,
+		otelhttptrace.NewClientTrace(ctx,
+			otelhttptrace.WithoutSubSpans(),
+			otelhttptrace.WithInsecureHeaders(),
+		),
+	)
+	req, err = http.NewRequestWithContext(ctx, http.MethodGet, ts.URL, nil)
+	req.Header.Set("Authorization", "Bearer token123")
+	require.NoError(t, err)
+	resp, err = ts.Client().Do(req)
+	require.NoError(t, err)
+	resp.Body.Close()
+	span.End()
+	require.Len(t, sr.Completed(), 4)
+	recSpan = sr.Completed()[3]
+
+	gotAttributes = recSpan.Attributes()
+	gotAttributes = recSpan.Attributes()
+	assert.Equal(t,
+		attribute.StringValue("Bearer token123"),
+		gotAttributes[attribute.Key("http.authorization")],
+	)
 }