Skip to content

Commit

Permalink
Merge pull request #229 from signalfx/do_not_log_secure_headers
Browse files Browse the repository at this point in the history
sfxclient: http sink upon error should not log all request headers
  • Loading branch information
jgheewala authored Nov 10, 2021
2 parents 0110f04 + dcaed6c commit b2adab8
Show file tree
Hide file tree
Showing 2 changed files with 47 additions and 1 deletion.
25 changes: 24 additions & 1 deletion sfxclient/httpsink.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"compress/gzip"
"context"
"crypto/sha1"
"encoding/json"
"fmt"
"io"
Expand Down Expand Up @@ -154,6 +155,28 @@ var _ Sink = &HTTPSink{}
// TokenHeaderName is the header key for the auth token in the HTTP request
const TokenHeaderName = "X-Sf-Token"

func getShaValue(values []string) string {
h := sha1.New()
for _, v := range values {
h.Write([]byte(v))
}
return string(h.Sum(nil))
}

// loggableHeaders returns headers that are only allowed to be logged. For instance "X-Sf-Token" should not be logged so
// it will generate a sha value and then log it
func loggableHeaders(headers map[string][]string) (rv map[string][]string) {
rv = make(map[string][]string, len(headers))
for header, value := range headers {
if strings.EqualFold(header, TokenHeaderName) {
rv[header] = []string{getShaValue(value)}
continue
}
rv[header] = value
}
return rv
}

func (h *HTTPSink) doBottom(ctx context.Context, f func() (io.Reader, bool, error), contentType, endpoint string,
respValidator responseValidator) error {
if ctx.Err() != nil {
Expand All @@ -176,7 +199,7 @@ func (h *HTTPSink) doBottom(ctx context.Context, f func() (io.Reader, bool, erro
if err != nil {
// According to docs, resp can be ignored since err is non-nil, so we
// don't have to close body.
return fmt.Errorf("failed to send/receive http request: %w: %v", err, req.Header)
return fmt.Errorf("failed to send/receive http request: %w: %v", err, loggableHeaders(req.Header))
}

return h.handleResponse(resp, respValidator)
Expand Down
23 changes: 23 additions & 0 deletions sfxclient/httpsink_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -727,3 +727,26 @@ func TestSetHeaders(t *testing.T) {
So(req.Header.Get(TokenHeaderName), ShouldEqual, "bar")
})
}

func TestLoggableHeaders(t *testing.T) {
Convey("given a list of request headers we should log all except some", t, func() {
lowerCaseTokenHeader := "x-sf-token"
inputHeaders := map[string][]string{
lowerCaseTokenHeader: {"thisIsValidToken"},
"version": {"test"},
"gzip": {"yes"},
}
expectedHeader := map[string][]string{
lowerCaseTokenHeader: {getShaValue(inputHeaders[lowerCaseTokenHeader])},
"version": {"test"},
"gzip": {"yes"},
}
actualHeaders := loggableHeaders(inputHeaders)
So(len(actualHeaders), ShouldEqual, len(expectedHeader))
So(actualHeaders, ShouldResemble, expectedHeader)
So(actualHeaders[lowerCaseTokenHeader], ShouldNotEqual, inputHeaders[lowerCaseTokenHeader])
for header := range expectedHeader {
So(actualHeaders[header], ShouldNotBeNil)
}
})
}

0 comments on commit b2adab8

Please sign in to comment.