From 84f494b3dde5f847924b586b461fa5a4749e6dbe Mon Sep 17 00:00:00 2001 From: Hakan Kaya Date: Fri, 23 Oct 2026 19:25:59 +0200 Subject: [PATCH 1/2] fix(plugin): negotiate SMTP AUTH so LOGIN-only servers work MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit send_email hard-coded PLAIN. Azure Communication Services (and some Exchange relays) advertise only LOGIN/XOAUTH2, so every send failed with "504 5.7.4 Unrecognized authentication type" even though the credential was valid — net/smtp's PlainAuth refuses to transmit when PLAIN is not offered. Read the server's advertised mechanisms after STARTTLS, prefer PLAIN, fall back to AUTH LOGIN, and fail with the advertised list when nothing supported is on offer. The LOGIN challenge handler answers only the two prompts it expects, so a server that asks again cannot harvest the password twice. Verified against smtp.azurecomm.net:587 with a real ACS credential (LOGIN accepted, message accepted for delivery) and by unit tests covering PLAIN+LOGIN, LOGIN only, XOAUTH2 only, and no AUTH advertisement. --- .../api/internal/platform/plugin/email.go | 71 ++++++++++++++++++- .../internal/platform/plugin/email_test.go | 57 +++++++++++++++ 2 files changed, 127 insertions(+), 1 deletion(-) diff --git a/services/api/internal/platform/plugin/email.go b/services/api/internal/platform/plugin/email.go index 7c7b01ec7..50e70d643 100644 --- a/services/api/internal/platform/plugin/email.go +++ b/services/api/internal/platform/plugin/email.go @@ -12,6 +12,7 @@ import ( "net" "net/mail" "net/smtp" + "strings" "time" "github.com/tetratelabs/wazero" @@ -185,7 +186,16 @@ func dialAndSendSMTP(ctx context.Context, req smtpSendRequest) error { } if req.Username != "" { - if err := client.Auth(smtp.PlainAuth("", req.Username, req.Password, req.Host)); err != nil { + // Negotiate from what the server advertises instead of assuming + // PLAIN: net/smtp's PlainAuth refuses to send against a server that + // offers only LOGIN (Azure Communication Services and some Exchange + // relays are exactly that), which surfaced as + // "504 5.7.4 Unrecognized authentication type". + auth, err := smtpAuthFor(advertisedMechanisms(client), req.Username, req.Password, req.Host) + if err != nil { + return fmt.Errorf("send_email: auth: %w", err) + } + if err := client.Auth(auth); err != nil { return fmt.Errorf("send_email: auth: %w", err) } } @@ -212,6 +222,65 @@ func dialAndSendSMTP(ctx context.Context, req smtpSendRequest) error { return client.Quit() } +// advertisedMechanisms returns the AUTH mechanisms the server listed in its +// EHLO response, upper-cased (net/smtp exposes the raw EHLO parameter line). +func advertisedMechanisms(client *smtp.Client) []string { + ok, params := client.Extension("AUTH") + if !ok { + return nil + } + return strings.Fields(strings.ToUpper(params)) +} + +// smtpAuthFor picks an smtp.Auth the server actually supports. PLAIN is +// preferred when offered; LOGIN is the fallback because it is the only +// mechanism Azure Communication Services (and some Exchange relays) accept. +// Returns an error naming the advertised set rather than letting net/smtp +// fail later with a bare "unrecognized authentication type". +func smtpAuthFor(mechanisms []string, username, password, host string) (smtp.Auth, error) { + for _, mech := range mechanisms { + switch mech { + case "PLAIN": + return smtp.PlainAuth("", username, password, host), nil + case "LOGIN": + return &loginAuth{username: username, password: password}, nil + } + } + if len(mechanisms) == 0 { + return nil, fmt.Errorf("server advertises no AUTH mechanism") + } + return nil, fmt.Errorf("no supported AUTH mechanism (server offers %s)", strings.Join(mechanisms, ", ")) +} + +// loginAuth implements the AUTH LOGIN handshake net/smtp does not ship: the +// server prompts for the username, then the password, each as a base64 +// challenge. Start returns "LOGIN" so net/smtp issues AUTH LOGIN without an +// initial response. +type loginAuth struct { + username string + password string +} + +func (a *loginAuth) Start(*smtp.ServerInfo) (string, []byte, error) { + return "LOGIN", nil, nil +} + +func (a *loginAuth) Next(fromServer []byte, more bool) ([]byte, error) { + if !more { + return nil, nil + } + switch strings.ToLower(strings.TrimSpace(string(fromServer))) { + case "username:", "user name:": + return []byte(a.username), nil + case "password:": + return []byte(a.password), nil + default: + // Never echo the challenge: a misbehaving server could ask for the + // password twice and the answer would leak into logs. + return nil, fmt.Errorf("unexpected AUTH LOGIN challenge") + } +} + // buildMIMEMessage renders a minimal RFC 5322 message with a // multipart/alternative text+HTML body (or a single part when only one is // supplied). diff --git a/services/api/internal/platform/plugin/email_test.go b/services/api/internal/platform/plugin/email_test.go index 822c3b2f8..5bcb12c7a 100644 --- a/services/api/internal/platform/plugin/email_test.go +++ b/services/api/internal/platform/plugin/email_test.go @@ -3,6 +3,7 @@ package plugin import ( "context" "net" + "net/smtp" "strings" "testing" ) @@ -51,3 +52,59 @@ func TestDialAndSendSMTP_BlocksPrivateTargets(t *testing.T) { default: } } + +func TestSmtpAuthFor_PicksAdvertisedMechanism(t *testing.T) { + // Azure Communication Services advertises only LOGIN/XOAUTH2; a PLAIN-only + // assumption is what broke the send with "Unrecognized authentication type". + for _, tc := range []struct { + name string + mech []string + wantErr bool + wantKind string + }{ + {"plain preferred when offered", []string{"PLAIN", "LOGIN"}, false, "plain"}, + {"login only", []string{"LOGIN", "XOAUTH2"}, false, "login"}, + {"login after starttls advertisement", []string{"LOGIN"}, false, "login"}, + {"xoauth2 only is unsupported", []string{"XOAUTH2"}, true, ""}, + {"nothing advertised", nil, true, ""}, + } { + t.Run(tc.name, func(t *testing.T) { + auth, err := smtpAuthFor(tc.mech, "user", "secret", "smtp.example.com") + if tc.wantErr { + if err == nil { + t.Fatalf("expected an error for mechanisms %v", tc.mech) + } + return + } + if err != nil { + t.Fatalf("unexpected error: %v", err) + } + _, isLogin := auth.(*loginAuth) + if got := map[bool]string{true: "login", false: "plain"}[isLogin]; got != tc.wantKind { + t.Fatalf("mechanism = %s, want %s", got, tc.wantKind) + } + }) + } +} + +func TestLoginAuth_AnswersChallengesInOrder(t *testing.T) { + auth := &loginAuth{username: "user", password: "secret"} + mech, initial, err := auth.Start(&smtp.ServerInfo{Name: "smtp.example.com", TLS: true}) + if err != nil || mech != "LOGIN" || initial != nil { + t.Fatalf("Start = %q, %v, %v; want LOGIN, nil, nil", mech, initial, err) + } + answer, err := auth.Next([]byte("Username:"), true) + if err != nil || string(answer) != "user" { + t.Fatalf("username challenge = %q, %v", answer, err) + } + answer, err = auth.Next([]byte("Password:"), true) + if err != nil || string(answer) != "secret" { + t.Fatalf("password challenge = %q, %v", answer, err) + } + if _, err := auth.Next(nil, false); err != nil { + t.Fatalf("final call should be a no-op, got %v", err) + } + if answer, err := auth.Next([]byte("Password again:"), true); err == nil { + t.Fatalf("unexpected challenge answered with %q; want an error", answer) + } +} From 3be911373b0c1e40b5a23f72761e8aaea91baaf3 Mon Sep 17 00:00:00 2001 From: Hakan Kaya Date: Fri, 23 Oct 2026 19:44:05 +0200 Subject: [PATCH 2/2] fix(plugin): keep the unencrypted-connection guard for AUTH LOGIN MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review caught that loginAuth.Start ignored ServerInfo, while smtp.PlainAuth refuses to send credentials when the connection was never encrypted and the host is not loopback. dialAndSendSMTP only upgrades opportunistically, so a LOGIN-only server without STARTTLS would have received the password in the clear. Mirror the guard. Also: scan PLAIN across all advertised mechanisms before LOGIN (the loop returned whichever came first, so "LOGIN PLAIN" picked LOGIN), and correct two comments that claimed PlainAuth refuses to send when PLAIN is unadvertised — it sends AUTH PLAIN regardless and the server rejects it with 504. --- .../api/internal/platform/plugin/email.go | 45 +++++++++++++------ .../internal/platform/plugin/email_test.go | 6 +++ 2 files changed, 37 insertions(+), 14 deletions(-) diff --git a/services/api/internal/platform/plugin/email.go b/services/api/internal/platform/plugin/email.go index 50e70d643..14cd5f724 100644 --- a/services/api/internal/platform/plugin/email.go +++ b/services/api/internal/platform/plugin/email.go @@ -7,6 +7,7 @@ import ( "crypto/tls" "encoding/hex" "encoding/json" + "errors" "fmt" "mime" "net" @@ -186,11 +187,10 @@ func dialAndSendSMTP(ctx context.Context, req smtpSendRequest) error { } if req.Username != "" { - // Negotiate from what the server advertises instead of assuming - // PLAIN: net/smtp's PlainAuth refuses to send against a server that - // offers only LOGIN (Azure Communication Services and some Exchange - // relays are exactly that), which surfaced as - // "504 5.7.4 Unrecognized authentication type". + // Negotiate from what the server advertises. net/smtp's PlainAuth + // sends AUTH PLAIN unconditionally, so a server that accepts only + // LOGIN (Azure Communication Services, some Exchange relays) rejects + // the handshake with "504 5.7.4 Unrecognized authentication type". auth, err := smtpAuthFor(advertisedMechanisms(client), req.Username, req.Password, req.Host) if err != nil { return fmt.Errorf("send_email: auth: %w", err) @@ -232,17 +232,21 @@ func advertisedMechanisms(client *smtp.Client) []string { return strings.Fields(strings.ToUpper(params)) } -// smtpAuthFor picks an smtp.Auth the server actually supports. PLAIN is -// preferred when offered; LOGIN is the fallback because it is the only -// mechanism Azure Communication Services (and some Exchange relays) accept. +// smtpAuthFor picks an smtp.Auth the server actually supports, trying PLAIN +// before LOGIN regardless of advertisement order. LOGIN is needed because it +// is the only mechanism Azure Communication Services (and some Exchange +// relays) accept. // Returns an error naming the advertised set rather than letting net/smtp // fail later with a bare "unrecognized authentication type". func smtpAuthFor(mechanisms []string, username, password, host string) (smtp.Auth, error) { - for _, mech := range mechanisms { - switch mech { - case "PLAIN": - return smtp.PlainAuth("", username, password, host), nil - case "LOGIN": + for _, want := range []string{"PLAIN", "LOGIN"} { + for _, mech := range mechanisms { + if mech != want { + continue + } + if want == "PLAIN" { + return smtp.PlainAuth("", username, password, host), nil + } return &loginAuth{username: username, password: password}, nil } } @@ -261,10 +265,23 @@ type loginAuth struct { password string } -func (a *loginAuth) Start(*smtp.ServerInfo) (string, []byte, error) { +func (a *loginAuth) Start(server *smtp.ServerInfo) (string, []byte, error) { + // Mirror smtp.PlainAuth: never put credentials on a link that was never + // encrypted. STARTTLS is opportunistic in dialAndSendSMTP, so a server + // that does not advertise it would otherwise receive the password in the + // clear. + if !server.TLS && !isLocalhost(server.Name) { + return "", nil, errors.New("unencrypted connection") + } return "LOGIN", nil, nil } +// isLocalhost mirrors net/smtp's unexported helper: loopback needs no TLS +// because the traffic never leaves the host. +func isLocalhost(name string) bool { + return name == "localhost" || name == "127.0.0.1" || name == "::1" +} + func (a *loginAuth) Next(fromServer []byte, more bool) ([]byte, error) { if !more { return nil, nil diff --git a/services/api/internal/platform/plugin/email_test.go b/services/api/internal/platform/plugin/email_test.go index 5bcb12c7a..9e53bd605 100644 --- a/services/api/internal/platform/plugin/email_test.go +++ b/services/api/internal/platform/plugin/email_test.go @@ -104,6 +104,12 @@ func TestLoginAuth_AnswersChallengesInOrder(t *testing.T) { if _, err := auth.Next(nil, false); err != nil { t.Fatalf("final call should be a no-op, got %v", err) } + if _, _, err := auth.Start(&smtp.ServerInfo{Name: "smtp.example.com"}); err == nil { + t.Fatalf("credentials must not be offered on an unencrypted connection") + } + if _, _, err := auth.Start(&smtp.ServerInfo{Name: "localhost"}); err != nil { + t.Fatalf("loopback needs no TLS, got %v", err) + } if answer, err := auth.Next([]byte("Password again:"), true); err == nil { t.Fatalf("unexpected challenge answered with %q; want an error", answer) }