From 42668f8804eb74f5c5cb257f88719f50f1370903 Mon Sep 17 00:00:00 2001 From: Anders Eknert Date: Sat, 26 Mar 2022 08:25:28 +0100 Subject: [PATCH] logging: mask authorization header value in debug logs (#4496) Fixes #4495 Signed-off-by: Anders Eknert --- plugins/rest/rest.go | 30 +++++++++++++++++------ plugins/rest/rest_test.go | 50 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 73 insertions(+), 7 deletions(-) diff --git a/plugins/rest/rest.go b/plugins/rest/rest.go index eb66543fbb..41fc853dcd 100644 --- a/plugins/rest/rest.go +++ b/plugins/rest/rest.go @@ -293,16 +293,19 @@ func (c Client) Do(ctx context.Context, method, path string) (*http.Response, er return nil, err } - c.loggerFields = map[string]interface{}{ - "method": method, - "url": url, - "headers": req.Header, - } + if c.logger.GetLevel() >= logging.Debug { + c.loggerFields = map[string]interface{}{ + "method": method, + "url": url, + "headers": withMaskedAuthorizationHeader(req.Header), + } - c.logger.WithFields(c.loggerFields).Debug("Sending request.") + c.logger.WithFields(c.loggerFields).Debug("Sending request.") + } resp, err := httpClient.Do(req) - if resp != nil { + + if resp != nil && c.logger.GetLevel() >= logging.Debug { // Only log for debug purposes. If an error occurred, the caller should handle // that. In the non-error case, the caller may not do anything. c.loggerFields["status"] = resp.Status @@ -312,3 +315,16 @@ 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 { + masked[k] = v + } + masked.Set("Authorization", "REDACTED") + return masked + } + return headers +} diff --git a/plugins/rest/rest_test.go b/plugins/rest/rest_test.go index bf54476eee..2c3d497af9 100644 --- a/plugins/rest/rest_test.go +++ b/plugins/rest/rest_test.go @@ -34,9 +34,12 @@ import ( "github.com/open-policy-agent/opa/internal/jwx/jwa" "github.com/open-policy-agent/opa/internal/jwx/jws" "github.com/open-policy-agent/opa/keys" + "github.com/open-policy-agent/opa/logging" "github.com/open-policy-agent/opa/internal/version" "github.com/open-policy-agent/opa/util/test" + + testlogger "github.com/open-policy-agent/opa/logging/test" ) const keyID = "key1" @@ -1514,6 +1517,53 @@ func TestS3SigningInstantiationInitializesLogger(t *testing.T) { } } +func TestDebugLoggingRequestMaskAuthorizationHeader(t *testing.T) { + token := "secret" + ts := testServer{t: t, expBearerToken: token} + ts.start() + defer ts.stop() + + config := fmt.Sprintf(`{ + "name": "foo", + "url": %q, + "credentials": { + "bearer": { + "token": %q + } + } + }`, ts.server.URL, token) + client, err := New([]byte(config), map[string]*keys.Config{}) + if err != nil { + t.Fatalf("Unexpected error: %v", err) + } + + logger := testlogger.New() + logger.SetLevel(logging.Debug) + client.logger = logger + + ctx := context.Background() + if _, err := client.Do(ctx, "GET", "test"); err != nil { + 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) + } + } + } + } + if !reqLogFound { + t.Fatalf("Expected log entry from request") + } +} + func newTestClient(t *testing.T, ts *testServer, certPath string, keypath string) *Client { config := fmt.Sprintf(`{ "name": "foo",