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

@@ -62,36 +62,94 @@ func parentHook(hook string) string {
 
 // ClientTraceOption allows customizations to how the httptrace.Client
 // collects information.
-type ClientTraceOption func(*clientTracer)
+type ClientTraceOption interface {
+	apply(*clientTracer)
+}
+
+type clientTraceOptionFunc func(*clientTracer)
+
+func (fn clientTraceOptionFunc) apply(c *clientTracer) {
+	fn(c)
+}
 
-// WithoutSubSpans will modify the httptrace.Client to only collect data
+// WithoutSubSpans will modify the httptrace.ClientTrace to only collect data
 // as Events and Attributes on a span found in the context.  By default
 // sub-spans will be generated.
 func WithoutSubSpans() ClientTraceOption {
-	return func(ct *clientTracer) {
+	return clientTraceOptionFunc(func(ct *clientTracer) {
 		ct.useSpans = false
-	}
+	})
+}
+
+// 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 clientTraceOptionFunc(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 clientTraceOptionFunc(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 clientTraceOptionFunc(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
 }
 
+// NewClientTrace returns an httptrace.ClientTrace implementation that will
+// record OpenTelemetry spans for requests made by an http.Client. By default
+// several spans will be added to the trace for various stages of a request
+// (dns, connection, tls, etc).  Also by default all http headers will be
+// added as attributes to spans, although several headers will be automatically
+// redacted: Authorization, WWW-Authenticate, Proxy-Authenticate,
+// Proxy-Authorization, Cookie, and Set-Cookie.
 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)
+		opt.apply(ct)
 	}
 
 	ct.tr = otel.GetTracerProvider().Tracer(
@@ -258,7 +316,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,74 @@ 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()
+	assert.Equal(t,
+		attribute.StringValue("Bearer token123"),
+		gotAttributes[attribute.Key("http.authorization")],
+	)
 }