From e3872d7fd5820cb94c5550ccb0af7d66bfe0887e Mon Sep 17 00:00:00 2001 From: Sheena Carswell Date: Tue, 9 Jul 2024 15:58:13 +0100 Subject: [PATCH 1/2] [sc-249267] Redact password in logs if specified as part of URL --- internal/httpconfig/httpconfig.go | 2 +- internal/httpconfig/httpconfig_test.go | 26 ++++++++++++++++++++++---- 2 files changed, 23 insertions(+), 5 deletions(-) diff --git a/internal/httpconfig/httpconfig.go b/internal/httpconfig/httpconfig.go index af6cd2b9..e24ccbc4 100644 --- a/internal/httpconfig/httpconfig.go +++ b/internal/httpconfig/httpconfig.go @@ -44,7 +44,7 @@ func NewHTTPConfig(proxyConfig config.ProxyConfig, authKey credential.SDKCredent return ret, errProxyAuthWithoutProxyURL } if proxyConfig.URL.IsDefined() { - loggers.Infof("Using proxy server at %s", proxyConfig.URL) + loggers.Infof("Using proxy server at %s", proxyConfig.URL.Get().Redacted()) } caCertFiles := proxyConfig.CACertFiles.Values() diff --git a/internal/httpconfig/httpconfig_test.go b/internal/httpconfig/httpconfig_test.go index cf720768..52434e39 100644 --- a/internal/httpconfig/httpconfig_test.go +++ b/internal/httpconfig/httpconfig_test.go @@ -4,6 +4,7 @@ import ( "crypto/x509" "net/http" "net/http/httptest" + "net/url" "os" "testing" @@ -115,25 +116,42 @@ func TestNTLMProxyInvalidConfigs(t *testing.T) { proxyConfig2 := proxyConfig1 proxyConfig2.URL, _ = configtypes.NewOptURLAbsoluteFromString("http://fake-proxy") - _, err = NewHTTPConfig(proxyConfig2, nil, "", ldlog.NewDisabledLoggers()) + mockLog2 := ldlogtest.NewMockLog() + _, err = NewHTTPConfig(proxyConfig2, nil, "", mockLog2.Loggers) assert.Equal(t, errNTLMProxyAuthWithoutCredentials, err) + mockLog2.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://fake-proxy$") proxyConfig3 := proxyConfig2 proxyConfig3.User = "user" - _, err = NewHTTPConfig(proxyConfig3, nil, "", ldlog.NewDisabledLoggers()) + mockLog3 := ldlogtest.NewMockLog() + _, err = NewHTTPConfig(proxyConfig3, nil, "", mockLog3.Loggers) assert.Equal(t, errNTLMProxyAuthWithoutCredentials, err) + mockLog3.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://fake-proxy$") proxyConfig4 := proxyConfig3 proxyConfig4.Password = "pass" - _, err = NewHTTPConfig(proxyConfig4, nil, "", ldlog.NewDisabledLoggers()) + mockLog4 := ldlogtest.NewMockLog() + _, err = NewHTTPConfig(proxyConfig4, nil, "", mockLog4.Loggers) assert.NoError(t, err) + mockLog4.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://fake-proxy$") proxyConfig5 := proxyConfig4 helpers.WithTempFile(func(certFileName string) { proxyConfig5.CACertFiles = configtypes.NewOptStringList([]string{certFileName}) - _, err = NewHTTPConfig(proxyConfig5, nil, "", ldlog.NewDisabledLoggers()) + mockLog5 := ldlogtest.NewMockLog() + _, err = NewHTTPConfig(proxyConfig5, nil, "", mockLog5.Loggers) if assert.Error(t, err) { assert.Contains(t, err.Error(), "invalid CA certificate data") } + mockLog5.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://fake-proxy$") }) + + proxyConfig6 := proxyConfig4 + url6, _ := url.Parse("http://my-user:my-password@my-proxy") + proxyConfig6.URL, _ = configtypes.NewOptURLAbsolute(url6) + mockLog6 := ldlogtest.NewMockLog() + _, err = NewHTTPConfig(proxyConfig6, nil, "", mockLog6.Loggers) + assert.NoError(t, err) + mockLog6.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://my-user:xxxxx@my-proxy$") + } From 96ce70c9b9ca43ff677e21e673b19d83e4ba6c41 Mon Sep 17 00:00:00 2001 From: Sheena Carswell Date: Thu, 11 Jul 2024 13:23:06 +0100 Subject: [PATCH 2/2] [sc-249267] Break out testing log output into separate test --- internal/httpconfig/httpconfig_test.go | 38 ++++++++++++++------------ 1 file changed, 20 insertions(+), 18 deletions(-) diff --git a/internal/httpconfig/httpconfig_test.go b/internal/httpconfig/httpconfig_test.go index 52434e39..f2c6619e 100644 --- a/internal/httpconfig/httpconfig_test.go +++ b/internal/httpconfig/httpconfig_test.go @@ -116,42 +116,44 @@ func TestNTLMProxyInvalidConfigs(t *testing.T) { proxyConfig2 := proxyConfig1 proxyConfig2.URL, _ = configtypes.NewOptURLAbsoluteFromString("http://fake-proxy") - mockLog2 := ldlogtest.NewMockLog() - _, err = NewHTTPConfig(proxyConfig2, nil, "", mockLog2.Loggers) + _, err = NewHTTPConfig(proxyConfig2, nil, "", ldlog.NewDisabledLoggers()) assert.Equal(t, errNTLMProxyAuthWithoutCredentials, err) - mockLog2.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://fake-proxy$") proxyConfig3 := proxyConfig2 proxyConfig3.User = "user" - mockLog3 := ldlogtest.NewMockLog() - _, err = NewHTTPConfig(proxyConfig3, nil, "", mockLog3.Loggers) + _, err = NewHTTPConfig(proxyConfig3, nil, "", ldlog.NewDisabledLoggers()) assert.Equal(t, errNTLMProxyAuthWithoutCredentials, err) - mockLog3.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://fake-proxy$") proxyConfig4 := proxyConfig3 proxyConfig4.Password = "pass" - mockLog4 := ldlogtest.NewMockLog() - _, err = NewHTTPConfig(proxyConfig4, nil, "", mockLog4.Loggers) + _, err = NewHTTPConfig(proxyConfig4, nil, "", ldlog.NewDisabledLoggers()) assert.NoError(t, err) - mockLog4.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://fake-proxy$") proxyConfig5 := proxyConfig4 helpers.WithTempFile(func(certFileName string) { proxyConfig5.CACertFiles = configtypes.NewOptStringList([]string{certFileName}) - mockLog5 := ldlogtest.NewMockLog() - _, err = NewHTTPConfig(proxyConfig5, nil, "", mockLog5.Loggers) + _, err = NewHTTPConfig(proxyConfig5, nil, "", ldlog.NewDisabledLoggers()) if assert.Error(t, err) { assert.Contains(t, err.Error(), "invalid CA certificate data") } - mockLog5.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://fake-proxy$") }) +} - proxyConfig6 := proxyConfig4 - url6, _ := url.Parse("http://my-user:my-password@my-proxy") - proxyConfig6.URL, _ = configtypes.NewOptURLAbsolute(url6) - mockLog6 := ldlogtest.NewMockLog() - _, err = NewHTTPConfig(proxyConfig6, nil, "", mockLog6.Loggers) +func TestLogsRedactConnectionPassword(t *testing.T) { + // Username and password are specified separately in NTLM auth won't show in logs as they're not part of server name + url1, _ := configtypes.NewOptURLAbsoluteFromString("http://my-proxy") + proxyConfig1 := config.ProxyConfig{NTLMAuth: true, URL: url1, User: "my-user", Password: "my-pass"} + mockLog1 := ldlogtest.NewMockLog() + _, err := NewHTTPConfig(proxyConfig1, nil, "", mockLog1.Loggers) assert.NoError(t, err) - mockLog6.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://my-user:xxxxx@my-proxy$") + mockLog1.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://my-proxy$") + // When username and password are configured as part of server name, verify the password is redacted + url2, _ := url.Parse("http://my-user:my-password@my-proxy") + url2Absolute, _ := configtypes.NewOptURLAbsolute(url2) + proxyConfig2 := config.ProxyConfig{URL: url2Absolute} + mockLog2 := ldlogtest.NewMockLog() + _, err = NewHTTPConfig(proxyConfig2, nil, "", mockLog2.Loggers) + assert.NoError(t, err) + mockLog2.AssertMessageMatch(t, true, ldlog.Info, "Using proxy server at http://my-user:xxxxx@my-proxy$") }