From 7acfe8015d9b2c2fd674e7e20302b6635585d7ce Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Sat, 12 Oct 2024 20:53:58 +0200 Subject: [PATCH 1/4] Redact authentication logs Add a boolean flag `authIsActive` to manage redaction of sensitive authentication information in debug logs. When this flag is true, authentication details are replaced with ``. --- smtp/smtp.go | 27 ++++++++++++++++++++++++--- 1 file changed, 24 insertions(+), 3 deletions(-) diff --git a/smtp/smtp.go b/smtp/smtp.go index d713f8c..316ddb3 100644 --- a/smtp/smtp.go +++ b/smtp/smtp.go @@ -82,7 +82,8 @@ type Client struct { localName string // the name to use in HELO/EHLO // logger will be used for debug logging - logger log.Logger + logger log.Logger + authIsActive bool // mutex is used to synchronize access to shared resources, ensuring that only one goroutine can access // the resource at a time. @@ -174,7 +175,12 @@ func (c *Client) Hello(localName string) error { func (c *Client) cmd(expectCode int, format string, args ...interface{}) (int, string, error) { c.mutex.Lock() - c.debugLog(log.DirClientToServer, format, args...) + var logMsg []interface{} + logMsg = args + if c.authIsActive { + logMsg = []interface{}{""} + } + c.debugLog(log.DirClientToServer, format, logMsg...) id, err := c.Text.Cmd(format, args...) if err != nil { c.mutex.Unlock() @@ -182,7 +188,12 @@ func (c *Client) cmd(expectCode int, format string, args ...interface{}) (int, s } c.Text.StartResponse(id) code, msg, err := c.Text.ReadResponse(expectCode) - c.debugLog(log.DirServerToClient, "%d %s", code, msg) + + logMsg = []interface{}{code, msg} + if c.authIsActive && code >= 300 { + logMsg = []interface{}{code, ""} + } + c.debugLog(log.DirServerToClient, "%d %s", logMsg...) c.Text.EndResponse(id) c.mutex.Unlock() return code, msg, err @@ -256,6 +267,16 @@ func (c *Client) Auth(a Auth) error { if err := c.hello(); err != nil { return err } + + c.mutex.Lock() + c.authIsActive = true + c.mutex.Unlock() + defer func() { + c.mutex.Lock() + c.authIsActive = false + c.mutex.Unlock() + }() + encoding := base64.StdEncoding mech, resp, err := a.Start(&ServerInfo{c.serverName, c.tls, c.auth}) if err != nil { From 55a5d02fe0379879c82d48f4e35bc483355f95db Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 15 Oct 2024 19:52:31 +0200 Subject: [PATCH 2/4] Add support for configurable SMTP auth data logging Added the `logAuthData` flag to enable conditional logging of SMTP authentication data. Introduced the `SetLogAuthData` method for clients to toggle this flag. Adjusted existing logging logic to respect this new configuration. --- smtp/smtp.go | 36 ++++++++++++++++++++++++++++-------- 1 file changed, 28 insertions(+), 8 deletions(-) diff --git a/smtp/smtp.go b/smtp/smtp.go index 316ddb3..77b5fb0 100644 --- a/smtp/smtp.go +++ b/smtp/smtp.go @@ -54,6 +54,9 @@ type Client struct { // auth supported auth mechanisms auth []string + // authIsActive indicates that the Client is currently during SMTP authentication + authIsActive bool + // keep a reference to the connection so it can be used to create a TLS connection later conn net.Conn @@ -78,12 +81,14 @@ type Client struct { // isConnected indicates if the Client has an active connection isConnected bool + // logAuthData indicates if the Client should include SMTP authentication data in the logs + logAuthData bool + // localName is the name to use in HELO/EHLO localName string // the name to use in HELO/EHLO // logger will be used for debug logging - logger log.Logger - authIsActive bool + logger log.Logger // mutex is used to synchronize access to shared resources, ensuring that only one goroutine can access // the resource at a time. @@ -177,10 +182,13 @@ func (c *Client) cmd(expectCode int, format string, args ...interface{}) (int, s var logMsg []interface{} logMsg = args + logFmt := format if c.authIsActive { - logMsg = []interface{}{""} + logMsg = []interface{}{""} + logFmt = "%s" } - c.debugLog(log.DirClientToServer, format, logMsg...) + c.debugLog(log.DirClientToServer, logFmt, logMsg...) + id, err := c.Text.Cmd(format, args...) if err != nil { c.mutex.Unlock() @@ -190,10 +198,11 @@ func (c *Client) cmd(expectCode int, format string, args ...interface{}) (int, s code, msg, err := c.Text.ReadResponse(expectCode) logMsg = []interface{}{code, msg} - if c.authIsActive && code >= 300 { - logMsg = []interface{}{code, ""} + if c.authIsActive && code >= 300 && code <= 400 { + logMsg = []interface{}{code, ""} } c.debugLog(log.DirServerToClient, "%d %s", logMsg...) + c.Text.EndResponse(id) c.mutex.Unlock() return code, msg, err @@ -269,11 +278,15 @@ func (c *Client) Auth(a Auth) error { } c.mutex.Lock() - c.authIsActive = true + if !c.logAuthData { + c.authIsActive = true + } c.mutex.Unlock() defer func() { c.mutex.Lock() - c.authIsActive = false + if !c.logAuthData { + c.authIsActive = false + } c.mutex.Unlock() }() @@ -577,6 +590,13 @@ func (c *Client) SetLogger(l log.Logger) { c.logger = l } +// SetLogAuthData enables logging of authentication data in the Client. +func (c *Client) SetLogAuthData() { + c.mutex.Lock() + c.logAuthData = true + c.mutex.Unlock() +} + // SetDSNMailReturnOption sets the DSN mail return option for the Mail method func (c *Client) SetDSNMailReturnOption(d string) { c.dsnmrtype = d From 0944296cff95222dfdfc14a4b94c26c3409044c2 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 15 Oct 2024 19:52:59 +0200 Subject: [PATCH 3/4] Enable logging of SMTP authentication data Added a new option and methods to enable logging of SMTP authentication data. Updated documentation to indicate caution when using this feature due to potential data protection risks. --- client.go | 42 +++++++++++++++++++++++++++++++++++++++--- 1 file changed, 39 insertions(+), 3 deletions(-) diff --git a/client.go b/client.go index 45fd764..59fc1b5 100644 --- a/client.go +++ b/client.go @@ -145,6 +145,9 @@ type ( // isEncrypted indicates wether the Client connection is encrypted or not. isEncrypted bool + // logAuthData indicates whether authentication-related data should be logged. + logAuthData bool + // logger is a logger that satisfies the log.Logger interface. logger log.Logger @@ -364,9 +367,10 @@ func WithSSLPort(fallback bool) Option { // WithDebugLog enables debug logging for the Client. // // This function activates debug logging, which logs incoming and outgoing communication between the -// Client and the SMTP server to os.Stderr. Be cautious when using this option, as the logs may include -// unencrypted authentication data, depending on the SMTP authentication method in use, which could -// pose a data protection risk. +// Client and the SMTP server to os.Stderr. By default the debug logging will redact any kind of SMTP +// authentication data. If you need access to the actual authentication data in your logs, you can +// enable authentication data logging with the WithLogAuthData option or by setting it with the +// Client.SetLogAuthData method. // // Returns: // - An Option function that enables debug logging for the Client. @@ -671,6 +675,22 @@ func WithDialContextFunc(dialCtxFunc DialContextFunc) Option { } } +// WithLogAuthData enables logging of authentication data. +// +// This function sets the logAuthData field of the Client to true, enabling the logging of authentication data. +// +// Be cautious when using this option, as the logs may include unencrypted authentication data, depending on +// the SMTP authentication method in use, which could pose a data protection risk. +// +// Returns: +// - An Option function that configures the Client to enable authentication data logging. +func WithLogAuthData() Option { + return func(c *Client) error { + c.logAuthData = true + return nil + } +} + // TLSPolicy returns the TLSPolicy that is currently set on the Client as a string. // // This method retrieves the current TLSPolicy configured for the Client and returns it as a string representation. @@ -865,6 +885,19 @@ func (c *Client) SetSMTPAuthCustom(smtpAuth smtp.Auth) { c.smtpAuthType = SMTPAuthCustom } +// SetLogAuthData sets or overrides the logging of SMTP authentication data for the Client. +// +// This function sets the logAuthData field of the Client to true, enabling the logging of authentication data. +// +// Be cautious when using this option, as the logs may include unencrypted authentication data, depending on +// the SMTP authentication method in use, which could pose a data protection risk. +// +// Parameters: +// - logAuth: Set wether or not to log SMTP authentication data for the Client. +func (c *Client) SetLogAuthData(logAuth bool) { + c.logAuthData = logAuth +} + // DialWithContext establishes a connection to the server using the provided context.Context. // // This function adds a deadline based on the Client's timeout to the provided context.Context @@ -921,6 +954,9 @@ func (c *Client) DialWithContext(dialCtx context.Context) error { if c.useDebugLog { c.smtpClient.SetDebugLog(true) } + if c.logAuthData { + c.smtpClient.SetLogAuthData() + } if err = c.smtpClient.Hello(c.helo); err != nil { return err } From 3234c13277a74427a3d0bc6bc1686a528aede7f0 Mon Sep 17 00:00:00 2001 From: Winni Neessen Date: Tue, 15 Oct 2024 20:02:24 +0200 Subject: [PATCH 4/4] Add tests for SetLogAuthData method Introduced TestClient_SetLogAuthData to verify the proper behavior of the SetLogAuthData method in both client and SMTP tests. This ensures that logAuthData is enabled or disabled as expected, increasing code reliability. --- client_test.go | 18 ++++++++++++++++++ smtp/smtp_test.go | 26 ++++++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/client_test.go b/client_test.go index c767e75..0af9f9c 100644 --- a/client_test.go +++ b/client_test.go @@ -123,6 +123,7 @@ func TestNewClientWithOptions(t *testing.T) { {"WithoutNoop()", WithoutNoop(), false}, {"WithDebugLog()", WithDebugLog(), false}, {"WithLogger()", WithLogger(log.New(os.Stderr, log.LevelDebug)), false}, + {"WithLogger()", WithLogAuthData(), false}, {"WithDialContextFunc()", WithDialContextFunc(func(ctx context.Context, network, address string) (net.Conn, error) { return nil, nil }), false}, @@ -578,6 +579,23 @@ func TestWithoutNoop(t *testing.T) { } } +func TestClient_SetLogAuthData(t *testing.T) { + c, err := NewClient(DefaultHost, WithLogAuthData()) + if err != nil { + t.Errorf("failed to create new client: %s", err) + return + } + if !c.logAuthData { + t.Errorf("WithLogAuthData failed. c.logAuthData expected to be: %t, got: %t", true, + c.logAuthData) + } + c.SetLogAuthData(false) + if c.logAuthData { + t.Errorf("SetLogAuthData failed. c.logAuthData expected to be: %t, got: %t", false, + c.logAuthData) + } +} + // TestSetSMTPAuthCustom tests the SetSMTPAuthCustom method for the Client object func TestSetSMTPAuthCustom(t *testing.T) { tests := []struct { diff --git a/smtp/smtp_test.go b/smtp/smtp_test.go index 4fd32eb..52b6b5a 100644 --- a/smtp/smtp_test.go +++ b/smtp/smtp_test.go @@ -1111,6 +1111,32 @@ func TestClient_SetLogger(t *testing.T) { c.logger.Debugf(log.Log{Direction: log.DirServerToClient, Format: "%s", Messages: []interface{}{"test"}}) } +func TestClient_SetLogAuthData(t *testing.T) { + server := strings.Join(strings.Split(newClientServer, "\n"), "\r\n") + + var cmdbuf strings.Builder + bcmdbuf := bufio.NewWriter(&cmdbuf) + out := func() string { + if err := bcmdbuf.Flush(); err != nil { + t.Errorf("failed to flush: %s", err) + } + return cmdbuf.String() + } + var fake faker + fake.ReadWriter = bufio.NewReadWriter(bufio.NewReader(strings.NewReader(server)), bcmdbuf) + c, err := NewClient(fake, "fake.host") + if err != nil { + t.Fatalf("NewClient: %v\n(after %v)", err, out()) + } + defer func() { + _ = c.Close() + }() + c.SetLogAuthData() + if !c.logAuthData { + t.Error("Expected logAuthData to be true but received false") + } +} + var newClientServer = `220 hello world 250-mx.google.com at your service 250-SIZE 35651584