Merge pull request #312 from wneessen/bug/311_smtp-auth-login-should-follow-ietf-draft-more-closely

Enhance SMTP LOGIN auth and add comprehensive tests
This commit is contained in:
Winni Neessen 2024-10-02 14:18:38 +02:00 committed by GitHub
commit c8e45477bb
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
3 changed files with 211 additions and 42 deletions

View file

@ -620,7 +620,7 @@ func TestClient_DialWithContext(t *testing.T) {
t.Skipf("failed to create test client: %s. Skipping tests", err) t.Skipf("failed to create test client: %s. Skipping tests", err)
} }
ctx := context.Background() ctx := context.Background()
if err := c.DialWithContext(ctx); err != nil { if err = c.DialWithContext(ctx); err != nil {
t.Errorf("failed to dial with context: %s", err) t.Errorf("failed to dial with context: %s", err)
return return
} }
@ -1872,6 +1872,119 @@ func TestClient_AuthSCRAMSHAX(t *testing.T) {
} }
} }
func TestClient_AuthLoginSuccess(t *testing.T) {
tests := []struct {
name string
featureSet string
}{
{"default", "250-AUTH LOGIN\r\n250-X-DEFAULT-LOGIN\r\n250-8BITMIME\r\n250-DSN\r\n250 SMTPUTF8"},
{"mox server", "250-AUTH LOGIN\r\n250-X-MOX-LOGIN\r\n250-8BITMIME\r\n250-DSN\r\n250 SMTPUTF8"},
{"null byte", "250-AUTH LOGIN\r\n250-X-NULLBYTE-LOGIN\r\n250-8BITMIME\r\n250-DSN\r\n250 SMTPUTF8"},
{"bogus responses", "250-AUTH LOGIN\r\n250-X-BOGUS-LOGIN\r\n250-8BITMIME\r\n250-DSN\r\n250 SMTPUTF8"},
{"empty responses", "250-AUTH LOGIN\r\n250-X-EMPTY-LOGIN\r\n250-8BITMIME\r\n250-DSN\r\n250 SMTPUTF8"},
}
for i, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
serverPort := TestServerPortBase + 40 + i
go func() {
if err := simpleSMTPServer(ctx, tt.featureSet, true, serverPort); err != nil {
t.Errorf("failed to start test server: %s", err)
return
}
}()
time.Sleep(time.Millisecond * 300)
client, err := NewClient(TestServerAddr,
WithPort(serverPort),
WithTLSPortPolicy(NoTLS),
WithSMTPAuth(SMTPAuthLogin),
WithUsername("toni@tester.com"),
WithPassword("V3ryS3cr3t+"))
if err != nil {
t.Errorf("unable to create new client: %s", err)
return
}
if err = client.DialWithContext(context.Background()); err != nil {
t.Errorf("failed to dial to test server: %s", err)
}
if err = client.Close(); err != nil {
t.Errorf("failed to close server connection: %s", err)
}
})
}
}
func TestClient_AuthLoginFail(t *testing.T) {
ctx, cancel := context.WithCancel(context.Background())
defer cancel()
serverPort := TestServerPortBase + 50
featureSet := "250-AUTH LOGIN\r\n250-X-DEFAULT-LOGIN\r\n250-8BITMIME\r\n250-DSN\r\n250 SMTPUTF8"
go func() {
if err := simpleSMTPServer(ctx, featureSet, true, serverPort); err != nil {
t.Errorf("failed to start test server: %s", err)
return
}
}()
time.Sleep(time.Millisecond * 300)
client, err := NewClient(TestServerAddr,
WithPort(serverPort),
WithTLSPortPolicy(NoTLS),
WithSMTPAuth(SMTPAuthLogin),
WithUsername("toni@tester.com"),
WithPassword("InvalidPassword"))
if err != nil {
t.Errorf("unable to create new client: %s", err)
return
}
if err = client.DialWithContext(context.Background()); err == nil {
t.Error("expected to fail to dial to test server, but it succeeded")
}
}
func TestClient_AuthLoginFail_noTLS(t *testing.T) {
if os.Getenv("TEST_SKIP_ONLINE") != "" {
t.Skipf("env variable TEST_SKIP_ONLINE is set. Skipping online tests")
}
th := os.Getenv("TEST_HOST")
if th == "" {
t.Skipf("no host set. Skipping online tests")
}
tp := 587
if tps := os.Getenv("TEST_PORT"); tps != "" {
tpi, err := strconv.Atoi(tps)
if err == nil {
tp = tpi
}
}
client, err := NewClient(th, WithPort(tp), WithSMTPAuth(SMTPAuthLogin), WithTLSPolicy(NoTLS))
if err != nil {
t.Errorf("failed to create new client: %s", err)
}
u := os.Getenv("TEST_SMTPAUTH_USER")
if u != "" {
client.SetUsername(u)
}
p := os.Getenv("TEST_SMTPAUTH_PASS")
if p != "" {
client.SetPassword(p)
}
// We don't want to log authentication data in tests
client.SetDebugLog(false)
if err = client.DialWithContext(context.Background()); err == nil {
t.Error("expected to fail to dial to test server, but it succeeded")
}
if !errors.Is(err, smtp.ErrUnencrypted) {
t.Errorf("expected error to be %s, but got %s", smtp.ErrUnencrypted, err)
}
}
func TestClient_AuthSCRAMSHAX_fail(t *testing.T) { func TestClient_AuthSCRAMSHAX_fail(t *testing.T) {
if os.Getenv("TEST_ONLINE_SCRAM") == "" { if os.Getenv("TEST_ONLINE_SCRAM") == "" {
t.Skipf("TEST_ONLINE_SCRAM is not set. Skipping online SCRAM tests") t.Skipf("TEST_ONLINE_SCRAM is not set. Skipping online SCRAM tests")
@ -2491,6 +2604,51 @@ func handleTestServerConnection(connection net.Conn, featureSet string, failRese
break break
} }
_ = writeLine("235 2.7.0 Authentication successful") _ = writeLine("235 2.7.0 Authentication successful")
case strings.HasPrefix(data, "AUTH LOGIN"):
var username, password string
userResp := "VXNlcm5hbWU6"
passResp := "UGFzc3dvcmQ6"
if strings.Contains(featureSet, "250-X-MOX-LOGIN") {
userResp = ""
passResp = "UGFzc3dvcmQ="
}
if strings.Contains(featureSet, "250-X-NULLBYTE-LOGIN") {
userResp = "VXNlciBuYW1lAA=="
passResp = "UGFzc3dvcmQA"
}
if strings.Contains(featureSet, "250-X-BOGUS-LOGIN") {
userResp = "Qm9ndXM="
passResp = "Qm9ndXM="
}
if strings.Contains(featureSet, "250-X-EMPTY-LOGIN") {
userResp = ""
passResp = ""
}
_ = writeLine("334 " + userResp)
ddata, derr := reader.ReadString('\n')
if derr != nil {
fmt.Printf("failed to read username data from connection: %s\n", derr)
break
}
ddata = strings.TrimSpace(ddata)
username = ddata
_ = writeLine("334 " + passResp)
ddata, derr = reader.ReadString('\n')
if derr != nil {
fmt.Printf("failed to read password data from connection: %s\n", derr)
break
}
ddata = strings.TrimSpace(ddata)
password = ddata
if !strings.EqualFold(username, "dG9uaUB0ZXN0ZXIuY29t") ||
!strings.EqualFold(password, "VjNyeVMzY3IzdCs=") {
_ = writeLine("535 5.7.8 Error: authentication failed")
break
}
_ = writeLine("235 2.7.0 Authentication successful")
case strings.EqualFold(data, "DATA"): case strings.EqualFold(data, "DATA"):
_ = writeLine("354 End data with <CR><LF>.<CR><LF>") _ = writeLine("354 End data with <CR><LF>.<CR><LF>")
for { for {

View file

@ -1,4 +1,4 @@
// SPDX-FileCopyrightText: Copyright (c) 2022-2023 The go-mail Authors // SPDX-FileCopyrightText: Copyright (c) 2022-2024 The go-mail Authors
// //
// SPDX-License-Identifier: MIT // SPDX-License-Identifier: MIT
@ -9,57 +9,42 @@ import (
"fmt" "fmt"
) )
// ErrUnencrypted is an error indicating that the connection is not encrypted.
var ErrUnencrypted = errors.New("unencrypted connection")
// loginAuth is the type that satisfies the Auth interface for the "SMTP LOGIN" auth // loginAuth is the type that satisfies the Auth interface for the "SMTP LOGIN" auth
type loginAuth struct { type loginAuth struct {
username, password string username, password string
host string host string
respStep uint8
} }
const (
// LoginXUsernameChallenge represents the Username Challenge response sent by the SMTP server per the AUTH LOGIN
// extension.
//
// See: https://learn.microsoft.com/en-us/openspecs/exchange_server_protocols/ms-xlogin/.
LoginXUsernameChallenge = "Username:"
LoginXUsernameLowerChallenge = "username:"
// LoginXPasswordChallenge represents the Password Challenge response sent by the SMTP server per the AUTH LOGIN
// extension.
//
// See: https://learn.microsoft.com/en-us/openspecs/exchange_server_protocols/ms-xlogin/.
LoginXPasswordChallenge = "Password:"
LoginXPasswordLowerChallenge = "password:"
// LoginXDraftUsernameChallenge represents the Username Challenge response sent by the SMTP server per the IETF
// draft AUTH LOGIN extension. It should be noted this extension is an expired draft which was never formally
// published and was deprecated in favor of the AUTH PLAIN extension.
//
// See: https://datatracker.ietf.org/doc/html/draft-murchison-sasl-login-00.
LoginXDraftUsernameChallenge = "User Name\x00"
// LoginXDraftPasswordChallenge represents the Password Challenge response sent by the SMTP server per the IETF
// draft AUTH LOGIN extension. It should be noted this extension is an expired draft which was never formally
// published and was deprecated in favor of the AUTH PLAIN extension.
//
// See: https://datatracker.ietf.org/doc/html/draft-murchison-sasl-login-00.
LoginXDraftPasswordChallenge = "Password\x00"
)
// LoginAuth returns an [Auth] that implements the LOGIN authentication // LoginAuth returns an [Auth] that implements the LOGIN authentication
// mechanism as it is used by MS Outlook. The Auth works similar to PLAIN // mechanism as it is used by MS Outlook. The Auth works similar to PLAIN
// but instead of sending all in one response, the login is handled within // but instead of sending all in one response, the login is handled within
// 3 steps: // 3 steps:
// - Sending AUTH LOGIN (server responds with "Username:") // - Sending AUTH LOGIN (server might responds with "Username:")
// - Sending the username (server responds with "Password:") // - Sending the username (server might responds with "Password:")
// - Sending the password (server authenticates) // - Sending the password (server authenticates)
// This is the common approach as specified by Microsoft in their MS-XLOGIN spec.
// See: https://msopenspecs.azureedge.net/files/MS-XLOGIN/%5bMS-XLOGIN%5d.pdf
// Yet, there is also an old IETF draft for SMTP AUTH LOGIN that states for clients:
// "The contents of both challenges SHOULD be ignored.".
// See: https://datatracker.ietf.org/doc/html/draft-murchison-sasl-login-00
// Since there is no official standard RFC and we've seen different implementations
// of this mechanism (sending "Username:", "Username", "username", "User name", etc.)
// we follow the IETF-Draft and ignore any server challange to allow compatiblity
// with most mail servers/providers.
// //
// LoginAuth will only send the credentials if the connection is using TLS // LoginAuth will only send the credentials if the connection is using TLS
// or is connected to localhost. Otherwise authentication will fail with an // or is connected to localhost. Otherwise authentication will fail with an
// error, without sending the credentials. // error, without sending the credentials.
func LoginAuth(username, password, host string) Auth { func LoginAuth(username, password, host string) Auth {
return &loginAuth{username, password, host} return &loginAuth{username, password, host, 0}
} }
// Start begins the SMTP authentication process by validating server's TLS status and hostname.
// Returns "LOGIN" on success.
func (a *loginAuth) Start(server *ServerInfo) (string, []byte, error) { func (a *loginAuth) Start(server *ServerInfo) (string, []byte, error) {
// Must have TLS, or else localhost server. // Must have TLS, or else localhost server.
// Note: If TLS is not true, then we can't trust ANYTHING in ServerInfo. // Note: If TLS is not true, then we can't trust ANYTHING in ServerInfo.
@ -67,20 +52,25 @@ func (a *loginAuth) Start(server *ServerInfo) (string, []byte, error) {
// That might just be the attacker saying // That might just be the attacker saying
// "it's ok, you can trust me with your password." // "it's ok, you can trust me with your password."
if !server.TLS && !isLocalhost(server.Name) { if !server.TLS && !isLocalhost(server.Name) {
return "", nil, errors.New("unencrypted connection") return "", nil, ErrUnencrypted
} }
if server.Name != a.host { if server.Name != a.host {
return "", nil, errors.New("wrong host name") return "", nil, errors.New("wrong host name")
} }
a.respStep = 0
return "LOGIN", nil, nil return "LOGIN", nil, nil
} }
// Next processes responses from the server during the SMTP authentication exchange, sending the
// username and password.
func (a *loginAuth) Next(fromServer []byte, more bool) ([]byte, error) { func (a *loginAuth) Next(fromServer []byte, more bool) ([]byte, error) {
if more { if more {
switch string(fromServer) { switch a.respStep {
case LoginXUsernameChallenge, LoginXUsernameLowerChallenge, LoginXDraftUsernameChallenge: case 0:
a.respStep++
return []byte(a.username), nil return []byte(a.username), nil
case LoginXPasswordChallenge, LoginXPasswordLowerChallenge, LoginXDraftPasswordChallenge: case 1:
a.respStep++
return []byte(a.password), nil return []byte(a.password), nil
default: default:
return nil, fmt.Errorf("unexpected server response: %s", string(fromServer)) return nil, fmt.Errorf("unexpected server response: %s", string(fromServer))

View file

@ -57,10 +57,31 @@ var authTests = []authTest{
}, },
{ {
LoginAuth("user", "pass", "testserver"), LoginAuth("user", "pass", "testserver"),
[]string{"Username:", "Password:", "User Name\x00", "Password\x00", "Invalid:"}, []string{"Username:", "Password:"},
"LOGIN", "LOGIN",
[]string{"", "user", "pass", "user", "pass", ""}, []string{"", "user", "pass"},
[]bool{false, false, false, false, true}, []bool{false, false},
},
{
LoginAuth("user", "pass", "testserver"),
[]string{"User Name\x00", "Password\x00"},
"LOGIN",
[]string{"", "user", "pass"},
[]bool{false, false},
},
{
LoginAuth("user", "pass", "testserver"),
[]string{"Invalid", "Invalid:"},
"LOGIN",
[]string{"", "user", "pass"},
[]bool{false, false},
},
{
LoginAuth("user", "pass", "testserver"),
[]string{"Invalid", "Invalid:", "Too many"},
"LOGIN",
[]string{"", "user", "pass", ""},
[]bool{false, false, true},
}, },
{ {
CRAMMD5Auth("user", "pass"), CRAMMD5Auth("user", "pass"),