Skip to content

Commit

Permalink
plugins/rest: masks X-AMZ-SECURITY-TOKEN header in decision logs (#6423)
Browse files Browse the repository at this point in the history
Decision logs had previously been configured to hide the value of the
Authorization header, as that is considered sensitive information.
However, there are cases when additional headers are provided that
contain sensitive information, such as the X-AMZ-SECURITY-TOKEN header.
This PR creates an internal map of headers that should be masked, which
can be expanded if additional headers are required. It then loops over
the headers in a request, and performs a lookup on the internal map
to see if any of them match those that should be masked. If so, it
replaces their values with "REDACTED". An existing test was added to
check both the header keys that should be masked, as well as a key
that should not.

Additional work, out of scope for this PR, would be to open a config
setting that would allow users to pass in a list of headers that should
be masked.

Fixes: #5848

Signed-off-by: Colin Lacy <colinjlacy@gmail.com>
  • Loading branch information
colinjlacy authored Nov 29, 2023
1 parent f66f7e0 commit 0b9bbc5
Show file tree
Hide file tree
Showing 2 changed files with 33 additions and 23 deletions.
22 changes: 13 additions & 9 deletions plugins/rest/rest.go
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,11 @@ const (
grantTypeJwtBearer = "jwt_bearer"
)

var maskedHeaderKeys = map[string]struct{}{
"Authorization": {},
"X-Amz-Security-Token": {},
}

// An HTTPAuthPlugin represents a mechanism to construct and configure HTTP authentication for a REST service
type HTTPAuthPlugin interface {
// implementations can assume NewClient will be called before Prepare
Expand Down Expand Up @@ -322,7 +327,7 @@ func (c Client) Do(ctx context.Context, method, path string) (*http.Response, er
c.loggerFields = map[string]interface{}{
"method": method,
"url": url,
"headers": withMaskedAuthorizationHeader(req.Header),
"headers": withMaskedHeaders(req.Header),
}

c.logger.WithFields(c.loggerFields).Debug("Sending request.")
Expand Down Expand Up @@ -354,15 +359,14 @@ func (c Client) Do(ctx context.Context, method, path string) (*http.Response, er
return resp, err
}

func withMaskedAuthorizationHeader(headers http.Header) http.Header {
authzHeader := headers.Get("Authorization")
if authzHeader != "" {
masked := make(http.Header)
for k, v := range headers {
func withMaskedHeaders(headers http.Header) http.Header {
masked := make(http.Header)
for k, v := range headers {
if _, ok := maskedHeaderKeys[k]; ok {
masked.Set(k, "REDACTED")
} else {
masked[k] = v
}
masked.Set("Authorization", "REDACTED")
return masked
}
return headers
return masked
}
34 changes: 20 additions & 14 deletions plugins/rest/rest_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1881,6 +1881,7 @@ func TestAWSCredentialServiceChain(t *testing.T) {

func TestDebugLoggingRequestMaskAuthorizationHeader(t *testing.T) {
token := "secret"
plaintext := "plaintext"
ts := testServer{t: t, expBearerToken: token}
ts.start()
defer ts.stop()
Expand All @@ -1892,8 +1893,12 @@ func TestDebugLoggingRequestMaskAuthorizationHeader(t *testing.T) {
"bearer": {
"token": %q
}
},
"headers": {
"X-AMZ-SECURITY-TOKEN": %q,
"remains-unmasked": %q
}
}`, ts.server.URL, token)
}`, ts.server.URL, token, token, plaintext)
client, err := New([]byte(config), map[string]*keys.Config{})
if err != nil {
t.Fatalf("Unexpected error: %v", err)
Expand All @@ -1908,22 +1913,23 @@ func TestDebugLoggingRequestMaskAuthorizationHeader(t *testing.T) {
t.Fatalf("Unexpected error: %v", err)
}

var reqLogFound bool
for _, entry := range logger.Entries() {
if entry.Fields["headers"] != nil {
headers := entry.Fields["headers"].(http.Header)
authzHeader := headers.Get("Authorization")
if authzHeader != "" {
reqLogFound = true
if authzHeader != "REDACTED" {
t.Errorf("Excpected redacted Authorization header value, got %v", authzHeader)
}
entries := logger.Entries()
if len(entries) != 2 {
t.Fatalf("Expected 2 log entries, got %d", len(entries))
}

requestEntry := entries[0]
headers := requestEntry.Fields["headers"].(http.Header)
for k := range headers {
v := headers.Get(k)
if _, ok := maskedHeaderKeys[k]; ok {
if v != "REDACTED" {
t.Errorf("Expected redacted %q header value, got %v", k, v)
}
} else if k == "Remains-Unmasked" && v != plaintext {
t.Errorf("Expected %q header to have value %q, got %v", k, plaintext, v)
}
}
if !reqLogFound {
t.Fatalf("Expected log entry from request")
}
}

func newTestClient(t *testing.T, ts *testServer, certPath string, keypath string) *Client {
Expand Down

0 comments on commit 0b9bbc5

Please sign in to comment.