From 0b9bbc5011624af7100e0cc0acdc712bcf52efee Mon Sep 17 00:00:00 2001
From: Colin J Lacy <colinjlacy@gmail.com>
Date: Wed, 29 Nov 2023 11:26:25 -0500
Subject: [PATCH] plugins/rest: masks X-AMZ-SECURITY-TOKEN header in decision
 logs (#6423)

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>
---
 plugins/rest/rest.go      | 22 +++++++++++++---------
 plugins/rest/rest_test.go | 34 ++++++++++++++++++++--------------
 2 files changed, 33 insertions(+), 23 deletions(-)

diff --git a/plugins/rest/rest.go b/plugins/rest/rest.go
index 62164580b4..fd59058ca1 100644
--- a/plugins/rest/rest.go
+++ b/plugins/rest/rest.go
@@ -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
@@ -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.")
@@ -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
 }
diff --git a/plugins/rest/rest_test.go b/plugins/rest/rest_test.go
index 26210cfe1a..27ba37e6cf 100644
--- a/plugins/rest/rest_test.go
+++ b/plugins/rest/rest_test.go
@@ -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()
@@ -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)
@@ -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 {