diff --git a/docs/release-control/v6/internal/records/notification-authentication-reliability-2026-10-18.md b/docs/release-control/v6/internal/records/notification-authentication-reliability-2026-10-18.md new file mode 100644 index 0000000000..d0b2781d60 --- /dev/null +++ b/docs/release-control/v6/internal/records/notification-authentication-reliability-2026-10-18.md @@ -0,0 +1,100 @@ +# Notification authentication investigation, 18 October 2026 + +## Signal and attribution limits + +The fixed production summary retrieved at 10:01:04 UTC is available with no +missing columns. Its clean seven-day cohort has 8,534 rotating IDs. Terminal +failure coverage is 8,455 IDs, of which 498 report a terminal notification +failure. Rolling totals are 653,287 attempts, 246,419 deliveries and 120,040 +terminal failures. Authentication totals 27,959 with classification coverage of +8,113 IDs. Connectivity remains the largest classified sum at 38,088. + +These are each rotating ID's latest rolling counters, not distinct customers +or new incidents. Cohorts and retained counters change between reads and may +predate upgrades. The view exposes no destination, transport, provider or error +text, so neither the earlier SMTP fix nor this source slice can be credited +with a measured fraction of fleet authentication failures or fleet recovery. + +## Source findings + +The baseline is public main +`13c99c17c13659ecce920fd04b24a2617479cca8`, including the prior provider-default +authentication and connectivity repairs. + +LOGIN selected credentials by matching literal Username:/Password: text. +Valid legacy prompts such as User Name/Password and empty prompts therefore +failed before correct credentials could reach the server. The historical +[LOGIN draft, section 2.1](https://datatracker.ietf.org/doc/html/draft-murchison-sasl-login-00#section-2.1) +describes the two responses by exchange order and treats challenge content as +opaque. This is legacy compatibility documentation, not a current RFC. + +AUTH negotiation used substring matches. A server advertising +PLAIN-CLIENTTOKEN and LOGIN could make Pulse choose unadvertised PLAIN and +reject an otherwise usable LOGIN exchange. SMTP AUTH advertises separate +mechanism names, as defined by [RFC 4954, section 3](https://www.rfc-editor.org/rfc/rfc4954.html#section-3). +An unsupported nonempty list also lacked a declared configuration verdict, +misclassifying the capability mismatch as authentication through error prose. + +Connection tests selected implicit TLS for port 465, then could attempt STARTTLS +inside that established TLS session when the explicit TLS flag was false and +STARTTLS remained enabled. Delivery already selected implicit TLS in that case. + +## Canonical repair + +`internal/notifications/email_enhanced.go` remains the shared authentication +owner for notification email, threaded messages and report attachments. +LOGIN now sends username then password independently of prompt text. It resets +on each Start, refuses premature success and excess challenges, and does not +copy receiver-controlled challenges into errors. Mechanism selection matches +whole names and retains PLAIN preference. Unsupported lists fail as typed +configuration errors before AUTH. Missing/empty advertisement retains the +existing legacy PLAIN probe, without laundering underlying greeting failures. + +One implicit-TLS predicate now owns delivery and TestConnection decisions. +Saved settings and the existing TLS trust policy are retained. Actual SMTP +535 replies remain terminal authentication failures and temporary 454 replies +retain retries. Existing queue snapshots, operator retry authority and retained +credentials keep their existing semantics. + +## Executable evidence + +Owned evidence is retained under +`/opt/pulse-release-worker/task-proofs/notification-authentication-20261018/`. +The new controls exercise the production SMTP client against real loopback +plain, TLS and STARTTLS receivers. AUTH is advertised after the STARTTLS upgrade. +Connection tests and real ordinary, threaded and attachment sends use the same +receiver. Only synthetic credentials and mail are used. + +The port-465 control redirects only the configured destination port to an owned +ephemeral receiver through the existing dial seam. It retains the actual TLS +handshake, configured TLS policy and SMTP commands. Negative controls cover +unsupported mechanism names, premature/excess challenges, real bad-password +535 rejection, temporary 454 replies and untrusted certificates. + +The queue control admits a normal alert and reopens its queue and audit after +delivery. Correct LOGIN credentials retain one successful attempt, one receiver +acceptance and no failure. A bad password retains one authentication dead letter, +one failed audit and its authentication telemetry count, without accepted mail. +`baseline-authentication.log` records failures on the unchanged runtime: +nonliteral challenges reject valid credentials across all three transports, +substring selection sends an unadvertised mechanism, unsupported capability +is misclassified as authentication, and the valid queued alert becomes an +authentication dead letter. Classic LOGIN controls pass. +`baseline-tls-mode.log` records the independent port-465 failure with classic +LOGIN prompts: TestConnection returns STARTTLS failed, 503 Already using TLS. +The focused authentication controls pass (`focused-authentication.log`, 0.339s), +as does the complete notification package (`packages-authentication.log`, +155.777s). Targeted authentication, delivery, queue and retry race controls pass +(`race-authentication.log`, 2.957s). The source was then fast-forwarded to +`82b2b3a78b2d7c3c9718681062b873b2a0bea4a6`; its intervening changes are confined +to roadmap governance and do not change the runtime exercised by these proofs. +These controls do not establish native provider acceptance or fleet recovery. + +## Remaining scope + +The existing notification-delivery-terminal-failure-diagnosis gap stays triaged. +Destination-level repeated-failure handling, evidence of destination recovery +and precise delivery-health guidance remain active residual work. Aggregate +authentication counters alone cannot distinguish wrong credentials, provider +policy or a client defect. This slice does not add OAuth, refresh retained +credentials, replay dead letters automatically or publish a release. diff --git a/docs/release-control/v6/internal/status.json b/docs/release-control/v6/internal/status.json index 659b69a0de..0ca8d4096b 100644 --- a/docs/release-control/v6/internal/status.json +++ b/docs/release-control/v6/internal/status.json @@ -10036,7 +10036,7 @@ }, { "id": "notification-delivery-terminal-failure-diagnosis", - "summary": "The 18 October 2026 production clean weekly cohort has 495 of 8,466 covered rotating IDs reporting terminal notification failures, with rolling totals of 122,341 failures and 245,365 deliveries. Connectivity (37,688) and authentication (30,627) are the largest classified sums, with schema-dependent coverage. These aggregates do not identify a transport/provider or prove a release regression. Typed failure classification, deterministic-failure short circuiting and explicit retained-queue recovery already exist. The preceding source slice repairs SMTP provider-default authentication and concurrent envelope mutation. This connectivity slice independently reproduces and repairs stalled leading approved addresses starving healthy fallback connections in both pinned HTTP clients, plus webhook DNS continuing after dial cancellation. Shared bounded dialing, context-aware DNS and real TCP/resolver controls retain destination policy, one-request delivery and persisted queue verdicts. Remaining work is destination-level repeated-failure handling, evidence of destination recovery, and precise delivery-health guidance. These source fixes do not establish how much fleet failure they explain or that the production fleet has recovered.", + "summary": "The 18 October 2026 production summary retrieved at 10:01 UTC has 498 of 8,455 covered rotating IDs reporting terminal notification failures, with rolling totals of 120,040 failures and 246,419 deliveries. Connectivity (38,088) and authentication (27,959) are the largest classified sums; classification coverage is 8,113 IDs. These rolling aggregates do not identify a transport/provider or prove a release regression or recovery. Typed classification, deterministic-failure short circuiting and explicit retained-queue recovery already exist. Prior source slices repair SMTP provider-default authentication and concurrent envelope mutation, pinned HTTP fallback starvation, and webhook DNS continuing after cancellation. This authentication slice independently reproduces and repairs literal LOGIN challenge matching, substring AUTH mechanism selection, and port-465 connection tests attempting STARTTLS inside implicit TLS. Shared exact-name negotiation, a bounded prompt-independent LOGIN exchange and a canonical TLS-mode decision retain real 535 authentication verdicts, 454 retries and persisted queue/audit/telemetry outcomes. Unsupported mechanism lists now have a typed configuration verdict. Remaining work is destination-level repeated-failure handling, evidence of destination recovery, and precise delivery-health guidance. These source proofs do not establish which fleet failures they explain or that the production fleet has recovered.", "owner": "project-owner", "status": "triaged", "recorded_at": "2026-09-03", @@ -10049,6 +10049,11 @@ ], "proposed_resolution": "lane-expansion", "evidence": [ + { + "repo": "pulse", + "path": "docs/release-control/v6/internal/records/notification-authentication-reliability-2026-10-18.md", + "kind": "file" + }, { "repo": "pulse", "path": "docs/release-control/v6/internal/records/notification-connectivity-reliability-2026-10-18.md", diff --git a/docs/release-control/v6/internal/subsystems/api-contracts.md b/docs/release-control/v6/internal/subsystems/api-contracts.md index 8f58276558..36fbc9aaf5 100644 --- a/docs/release-control/v6/internal/subsystems/api-contracts.md +++ b/docs/release-control/v6/internal/subsystems/api-contracts.md @@ -12135,6 +12135,16 @@ and repeatable customer outcomes. ### Report email shares the canonical SMTP retry verdict +Report attachment delivery also inherits complete-token SMTP mechanism +negotiation and the bounded LOGIN exchange in the notification sender. Opaque +challenge text must not reject correct report credentials or select which +credential to send. Unsupported mechanism lists are configuration failures, +while actual SMTP authentication replies keep their typed verdict and retry +policy. `email_authentication_wire_test.go` exercises actual threaded and +attachment delivery through LOGIN over plain, TLS and STARTTLS sessions. It +also verifies that connection tests use the same port-465 TLS rule as delivery. +Report scheduling must not introduce another authentication implementation. + Attachment delivery through `internal/notifications/email_enhanced.go` uses notifications-owned `sendEmailWithOptions`. Reporting must not introduce a separate retry classification. Reporting also inherits the notification-owned diff --git a/docs/release-control/v6/internal/subsystems/notifications.md b/docs/release-control/v6/internal/subsystems/notifications.md index 2efe596773..a9be6d40e5 100644 --- a/docs/release-control/v6/internal/subsystems/notifications.md +++ b/docs/release-control/v6/internal/subsystems/notifications.md @@ -178,6 +178,36 @@ policy, credential, SSRF boundary or notification payload is changed. ### SMTP authentication and envelopes belong to each delivery +SMTP mechanism negotiation matches complete advertised names, prefers PLAIN +over LOGIN and classifies an unsupported nonempty mechanism list as a local +configuration failure. It must not select PLAIN from PLAIN-CLIENTTOKEN or LOGIN +from XLOGIN, nor spend retry budgets on an unsupported mechanism. The existing +PLAIN probe for a missing/empty AUTH advertisement remains available to legacy +servers, retaining the underlying EHLO/connection error if that probe fails. + +LOGIN sends username followed by password according to exchange order. Server +challenges are opaque display text, including empty or localised prompts, and +must neither select a credential nor enter diagnostics. A fresh exchange resets +its progress. Premature success or a third challenge fails closed without DATA +and without another credential response. Genuine SMTP 535 rejection remains an +authentication failure, while 454 retains transient retries and its typed reply. + +Connection tests and delivery share the implicit-TLS rule: an explicit TLS flag +or port 465 selects TLS before SMTP begins. A configured STARTTLS flag must not +request another TLS upgrade inside that established TLS session. Saved flags +remain the submitted configuration, and certificate verification still applies. + +`email_authentication_wire_test.go` uses real plain/TLS/STARTTLS receivers to +check connection tests, ordinary/threaded/attachment delivery, exact mechanism +selection, legacy probing, malformed exchanges, certificate refusal and server +verdicts. Its persistent queue control reopens accepted and bad-password verdicts +with one attempt, retaining the authentication dead letter for bad credentials. +The port-465 control maps only the destination to an owned ephemeral listener +and retains the real TLS handshake and SMTP session. Protocol-level controls +in `email_authentication_test.go` cover opaque challenges and exchange bounds. +These are synthetic source proofs, not native provider acceptance or attribution +of the fleet's aggregate authentication counters. + SMTP managers resolve the provider identity and any supported default username before deciding whether credentials require authentication. Explicit usernames take precedence. Validation, connection tests and real sends use the same diff --git a/internal/notifications/email_authentication_test.go b/internal/notifications/email_authentication_test.go new file mode 100644 index 0000000000..0c83f03f55 --- /dev/null +++ b/internal/notifications/email_authentication_test.go @@ -0,0 +1,95 @@ +package notifications + +import ( + "net/smtp" + "net/textproto" + "strconv" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestSMTPLoginUsesExchangeOrderInsteadOfChallengeText(t *testing.T) { + for _, prompts := range [][2]string{ + {"Username:", "Password:"}, + {"User Name", "Password"}, + {"", ""}, + {"Identifiant", "Mot de passe"}, + {"Password:", "Username:"}, + } { + t.Run(prompts[0]+"/"+prompts[1], func(t *testing.T) { + auth := LoginAuth("synthetic-user", "synthetic-password") + mechanism, initial, err := auth.Start(&smtp.ServerInfo{Name: "localhost"}) + require.NoError(t, err) + require.Equal(t, "LOGIN", mechanism) + require.Nil(t, initial) + username, err := auth.Next([]byte(prompts[0]), true) + require.NoError(t, err) + require.Equal(t, "synthetic-user", string(username)) + password, err := auth.Next([]byte(prompts[1]), true) + require.NoError(t, err) + require.Equal(t, "synthetic-password", string(password)) + response, err := auth.Next(nil, false) + require.NoError(t, err) + require.Nil(t, response) + }) + } +} + +func TestSMTPLoginRejectsIncompleteOrExcessChallenges(t *testing.T) { + for _, challenges := range []int{0, 1, 3} { + t.Run(strconv.Itoa(challenges), func(t *testing.T) { + auth := LoginAuth("synthetic-user", "synthetic-password") + _, _, err := auth.Start(&smtp.ServerInfo{Name: "localhost"}) + require.NoError(t, err) + if challenges >= 1 { + _, err = auth.Next([]byte("Username:"), true) + require.NoError(t, err) + } + if challenges == 3 { + _, err = auth.Next([]byte("Password:"), true) + require.NoError(t, err) + _, err = auth.Next([]byte("receiver-controlled-private-prompt"), true) + } else { + _, err = auth.Next(nil, false) + } + require.Error(t, err) + require.Equal(t, NotificationFailureAuthentication, ClassifyNotificationFailureError(err)) + require.NotContains(t, err.Error(), "receiver-controlled-private-prompt") + }) + } +} + +func TestSMTPLoginStartResetsExchange(t *testing.T) { + auth := LoginAuth("synthetic-user", "synthetic-password") + for attempt := 0; attempt < 2; attempt++ { + _, _, err := auth.Start(&smtp.ServerInfo{Name: "localhost"}) + require.NoError(t, err) + response, err := auth.Next([]byte("Username:"), true) + require.NoError(t, err) + require.Equal(t, "synthetic-user", string(response)) + response, err = auth.Next([]byte("Password:"), true) + require.NoError(t, err) + require.Equal(t, "synthetic-password", string(response)) + _, err = auth.Next(nil, false) + require.NoError(t, err) + } +} + +func TestSMTPAuthenticationPreservesGreetingFailure(t *testing.T) { + client := newSMTPClientWithEHLO(t, []string{"421 authentication service temporarily unavailable"}) + defer client.Close() + manager := NewEnhancedEmailManager(EmailProviderConfig{EmailConfig: EmailConfig{ + Username: "synthetic-user", Password: "synthetic-password", + }}) + // Extension hides hello errors as a missing advertisement. Keep the + // legacy fallback so Auth returns the original structured SMTP error. + auth, err := manager.negotiateAuth(client) + require.NoError(t, err) + require.NotNil(t, auth) + err = client.Auth(auth) + var reply *textproto.Error + require.ErrorAs(t, err, &reply) + require.Equal(t, 421, reply.Code) + require.Equal(t, NotificationFailureServerError, ClassifyNotificationFailureError(err)) +} diff --git a/internal/notifications/email_authentication_wire_test.go b/internal/notifications/email_authentication_wire_test.go new file mode 100644 index 0000000000..14b9c077f3 --- /dev/null +++ b/internal/notifications/email_authentication_wire_test.go @@ -0,0 +1,375 @@ +package notifications + +import ( + "crypto/tls" + "encoding/base64" + "net" + "net/http" + "net/http/httptest" + "net/textproto" + "strconv" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/rcourtman/pulse-go-rewrite/internal/alerts" + "github.com/stretchr/testify/require" +) + +type smtpAuthenticationCounts struct { + connections, attempts, plain, login, credentials, accepted atomic.Int32 +} + +// This receiver checks the actual authentication exchange before accepting any +// envelope or message. STARTTLS advertises AUTH only after the TLS upgrade. +func smtpAuthenticationReceiver(t *testing.T, mode, mechanisms string, prompts []string, verdict int) (EmailProviderConfig, *smtpAuthenticationCounts) { + t.Helper() + certificate := httptest.NewTLSServer(http.HandlerFunc(func(http.ResponseWriter, *http.Request) {})) + tlsConfig := certificate.TLS.Clone() + certificate.Close() + listener, err := net.Listen("tcp4", "127.0.0.1:0") + require.NoError(t, err) + counts := &smtpAuthenticationCounts{} + var sessions sync.WaitGroup + done := make(chan struct{}) + go func() { + defer close(done) + for { + raw, err := listener.Accept() + if err != nil { + return + } + counts.connections.Add(1) + sessions.Add(1) + go func() { + defer sessions.Done() + defer raw.Close() + _ = raw.SetDeadline(time.Now().Add(5 * time.Second)) + wire := raw + if mode == "TLS" { + wire = tls.Server(raw, tlsConfig) + } + conn := textproto.NewConn(wire) + if conn.PrintfLine("220 localhost ESMTP") != nil { + return + } + upgraded, authenticated := mode == "TLS", false + for { + line, err := conn.ReadLine() + if err != nil { + return + } + command, argument, _ := strings.Cut(line, " ") + switch command { + case "EHLO", "HELO": + switch { + case mode == "STARTTLS" && !upgraded: + err = conn.PrintfLine("250-localhost\r\n250 STARTTLS") + case mechanisms != "": + err = conn.PrintfLine("250-localhost\r\n250 AUTH %s", mechanisms) + default: + err = conn.PrintfLine("250 localhost") + } + case "STARTTLS": + if upgraded { + err = conn.PrintfLine("503 Already using TLS") + break + } + if err = conn.PrintfLine("220 Ready for TLS"); err == nil { + conn = textproto.NewConn(tls.Server(raw, tlsConfig)) + upgraded = true + } + case "AUTH": + counts.attempts.Add(1) + mechanism, initial, _ := strings.Cut(argument, " ") + allowed := mechanisms == "" && mechanism == "PLAIN" + for _, candidate := range strings.Fields(mechanisms) { + allowed = allowed || candidate == mechanism + } + if !allowed || mode == "STARTTLS" && !upgraded { + err = conn.PrintfLine("504 Unsupported mechanism") + break + } + valid, cancelled := true, false + switch mechanism { + case "PLAIN": + counts.plain.Add(1) + response, decodeErr := base64.StdEncoding.DecodeString(initial) + valid = decodeErr == nil && string(response) == "\x00synthetic-user\x00synthetic-password" + case "LOGIN": + counts.login.Add(1) + for index, prompt := range prompts { + if conn.PrintfLine("334 %s", base64.StdEncoding.EncodeToString([]byte(prompt))) != nil { + return + } + encoded, readErr := conn.ReadLine() + if readErr != nil { + return + } + if encoded == "*" { + cancelled = true + break + } + counts.credentials.Add(1) + response, decodeErr := base64.StdEncoding.DecodeString(encoded) + want := "synthetic-user" + if index == 1 { + want = "synthetic-password" + } + valid = valid && decodeErr == nil && index < 2 && string(response) == want + } + default: + valid = false + } + if cancelled { + err = conn.PrintfLine("501 Authentication cancelled") + } else if !valid { + err = conn.PrintfLine("535 Invalid credentials") + } else { + authenticated = verdict == 235 + err = conn.PrintfLine("%d Authentication verdict", verdict) + } + case "*": + err = conn.PrintfLine("501 Authentication cancelled") + case "MAIL", "RCPT": + if authenticated { + err = conn.PrintfLine("250 OK") + } else { + err = conn.PrintfLine("530 Authentication required") + } + case "DATA": + if !authenticated { + err = conn.PrintfLine("530 Authentication required") + break + } + if conn.PrintfLine("354 Send message") != nil { + return + } + if _, err = conn.ReadDotBytes(); err != nil { + return + } + counts.accepted.Add(1) + err = conn.PrintfLine("250 Accepted") + case "QUIT": + _ = conn.PrintfLine("221 Bye") + return + default: + t.Errorf("unexpected SMTP command %q", command) + return + } + if err != nil { + return + } + } + }() + } + }() + t.Cleanup(func() { _ = listener.Close(); <-done; sessions.Wait() }) + _, rawPort, err := net.SplitHostPort(listener.Addr().String()) + require.NoError(t, err) + port, err := strconv.Atoi(rawPort) + require.NoError(t, err) + return EmailProviderConfig{EmailConfig: EmailConfig{Enabled: true, SMTPHost: "127.0.0.1", SMTPPort: port, + From: "pulse@example.test", To: []string{"ops@example.test"}, Username: "synthetic-user", Password: "synthetic-password", TLS: mode == "TLS"}, + StartTLS: mode == "STARTTLS", SkipTLSVerify: true, MaxRetries: 2}, counts +} + +func TestSMTPAuthenticationAcceptsLoginChallengeVariants(t *testing.T) { + for _, mode := range []string{"plain", "TLS", "STARTTLS"} { + for _, test := range []struct { + name, mechanisms string + prompts []string + }{ + {"classic", "LOGIN", []string{"Username:", "Password:"}}, + {"legacy", "LOGIN", []string{"User Name", "Password"}}, + {"empty", "LOGIN", []string{"", ""}}, + {"exact mechanism", "PLAIN-CLIENTTOKEN LOGIN", []string{"Username:", "Password:"}}, + } { + for _, action := range []string{"connection test", "delivery"} { + t.Run(mode+"/"+test.name+"/"+action, func(t *testing.T) { + config, counts := smtpAuthenticationReceiver(t, mode, test.mechanisms, test.prompts, 235) + manager := NewEnhancedEmailManager(config) + if action == "connection test" { + require.NoError(t, manager.TestConnection()) + require.Zero(t, counts.accepted.Load()) + } else { + require.NoError(t, manager.SendEmailWithRetry("synthetic authentication test", "
test
", "test")) + require.Equal(t, int32(1), counts.accepted.Load()) + } + require.Equal(t, int32(1), counts.connections.Load()) + require.Equal(t, int32(1), counts.login.Load()) + require.Equal(t, int32(2), counts.credentials.Load()) + }) + } + } + } +} + +func TestSMTPAuthenticationRejectsUnsupportedMechanismsBeforeAUTH(t *testing.T) { + for _, mechanisms := range []string{"CRAM-MD5", "XPLAIN", "XLOGIN"} { + t.Run(mechanisms, func(t *testing.T) { + config, counts := smtpAuthenticationReceiver(t, "plain", mechanisms, nil, 235) + err := NewEnhancedEmailManager(config).SendEmailWithRetry("test", "test
", "test") + require.Error(t, err) + require.Equal(t, NotificationFailureConfiguration, ClassifyNotificationFailureError(err)) + require.Equal(t, int32(1), counts.connections.Load(), "an unsupported mechanism must not spend transport retries") + require.Zero(t, counts.attempts.Load(), "do not attempt an unadvertised mechanism") + require.Zero(t, counts.accepted.Load()) + }) + } +} + +func TestSMTPAuthenticationKeepsPlainPreferenceAndLegacyProbe(t *testing.T) { + for _, mode := range []string{"plain", "TLS", "STARTTLS"} { + for _, mechanisms := range []string{"LOGIN PLAIN", ""} { + t.Run(mode+"/"+mechanisms, func(t *testing.T) { + config, counts := smtpAuthenticationReceiver(t, mode, mechanisms, nil, 235) + require.NoError(t, NewEnhancedEmailManager(config).SendEmailWithRetry("test", "test
", "test")) + require.Equal(t, int32(1), counts.accepted.Load()) + require.Equal(t, int32(1), counts.plain.Load()) + require.Zero(t, counts.login.Load()) + }) + } + } +} + +func TestSMTPAuthenticationRetainsServerVerdicts(t *testing.T) { + for _, mode := range []string{"plain", "TLS", "STARTTLS"} { + for _, verdict := range []int{535, 454} { + t.Run(mode+"/"+strconv.Itoa(verdict), func(t *testing.T) { + config, counts := smtpAuthenticationReceiver(t, mode, "LOGIN", []string{"User Name", "Password"}, verdict) + err := NewEnhancedEmailManager(config).SendEmailWithRetry("test", "test
", "test") + require.Error(t, err) + var reply *textproto.Error + require.ErrorAs(t, err, &reply) + require.Equal(t, verdict, reply.Code) + attempts, class := int32(1), NotificationFailureAuthentication + if verdict == 454 { + attempts, class = 3, NotificationFailureServerError + } + require.Equal(t, class, ClassifyNotificationFailureError(err)) + require.Equal(t, attempts, counts.connections.Load()) + require.Equal(t, attempts*2, counts.credentials.Load()) + require.Zero(t, counts.accepted.Load()) + }) + } + } +} + +func TestSMTPAuthenticationRejectsMalformedLoginExchange(t *testing.T) { + for _, prompts := range [][]string{nil, {"Username:"}, {"Username:", "Password:", "receiver-controlled-private-prompt"}} { + t.Run(strconv.Itoa(len(prompts)), func(t *testing.T) { + config, counts := smtpAuthenticationReceiver(t, "plain", "LOGIN", prompts, 235) + err := NewEnhancedEmailManager(config).SendEmailWithRetry("test", "test
", "test") + require.Error(t, err) + require.Equal(t, NotificationFailureAuthentication, ClassifyNotificationFailureError(err)) + require.NotContains(t, err.Error(), "receiver-controlled-private-prompt") + require.Equal(t, int32(1), counts.connections.Load()) + require.LessOrEqual(t, counts.credentials.Load(), int32(2)) + require.Zero(t, counts.accepted.Load()) + }) + } +} + +func TestSMTPAuthenticationThreadedAndAttachmentDelivery(t *testing.T) { + for _, mode := range []string{"plain", "TLS", "STARTTLS"} { + t.Run(mode, func(t *testing.T) { + config, counts := smtpAuthenticationReceiver(t, mode, "LOGIN", []string{"User Name", "Password"}, 235) + manager := NewEnhancedEmailManager(config) + require.NoError(t, manager.SendEmailThreaded("test", "test
", "test", "synthetic-thread")) + require.NoError(t, manager.SendEmailWithAttachments("test", "test
", "test", []EmailAttachment{{ + Filename: "synthetic-report.csv", ContentType: "text/csv", Data: []byte("resource,status\nsynthetic,ok\n"), + }})) + require.Equal(t, int32(2), counts.accepted.Load()) + require.Equal(t, int32(2), counts.login.Load()) + }) + } +} + +func TestSMTPAuthenticationKeepsCertificateVerification(t *testing.T) { + for _, mode := range []string{"TLS", "STARTTLS"} { + t.Run(mode, func(t *testing.T) { + config, counts := smtpAuthenticationReceiver(t, mode, "LOGIN", []string{"User Name", "Password"}, 235) + config.SkipTLSVerify = false + err := NewEnhancedEmailManager(config).SendEmailWithRetry("test", "test
", "test") + require.Error(t, err) + require.Equal(t, NotificationFailureTLS, ClassifyNotificationFailureError(err)) + require.Zero(t, counts.attempts.Load()) + require.Zero(t, counts.accepted.Load()) + }) + } +} + +func TestSMTPConnectionTestUsesImplicitTLSForPort465(t *testing.T) { + config, counts := smtpAuthenticationReceiver(t, "TLS", "LOGIN", []string{"Username:", "Password:"}, 235) + receiver := net.JoinHostPort(config.SMTPHost, strconv.Itoa(config.SMTPPort)) + original := smtpTLSDialWithDialer + t.Cleanup(func() { smtpTLSDialWithDialer = original }) + smtpTLSDialWithDialer = func(dialer *net.Dialer, network, address string, policy *tls.Config) (*tls.Conn, error) { + require.Equal(t, net.JoinHostPort(config.SMTPHost, "465"), address) + return original(dialer, network, receiver, policy) + } + // Map only the destination port to the owned ephemeral receiver. The real + // TLS handshake, configured policy and SMTP session all remain in use. + config.SMTPPort, config.TLS, config.StartTLS = 465, false, true + manager := NewEnhancedEmailManager(config) + require.NoError(t, manager.TestConnection()) + require.NoError(t, manager.SendEmailWithRetry("test", "test
", "test")) + require.Equal(t, int32(2), counts.login.Load()) + require.Equal(t, int32(1), counts.accepted.Load()) +} + +func TestQueuedSMTPLoginAuthenticationVerdictSurvivesRestart(t *testing.T) { + for _, valid := range []bool{true, false} { + t.Run(strconv.FormatBool(valid), func(t *testing.T) { + config, counts := smtpAuthenticationReceiver(t, "plain", "LOGIN", []string{"User Name", "Password"}, 235) + if !valid { + config.Password = "synthetic-invalid-password" + } + dir := t.TempDir() + manager := NewNotificationManagerWithDeferredQueue("", dir) + t.Cleanup(manager.Stop) + manager.SetEmailConfig(config.EmailConfig) + manager.SetGroupingConfig(false, 0, false, false) + manager.SendAlert(&alerts.Alert{ID: "smtp-login-test", ResourceName: "synthetic-node", Type: "cpu", + Level: alerts.AlertLevelWarning, Message: "synthetic alert", StartTime: time.Now()}) + pending, err := manager.queue.GetPending(10) + require.NoError(t, err) + require.Len(t, pending, 1) + id := pending[0].ID + manager.StartQueueProcessing() + waitVerdictAudit(t, manager.queue, id) + manager.Stop() + reopened := NewNotificationManagerWithDeferredQueue("", dir) + t.Cleanup(reopened.Stop) + var status, failureClass string + var attempts, success int + require.NoError(t, reopened.queue.db.QueryRow(`SELECT status, attempts FROM notification_queue WHERE id = ?`, id).Scan(&status, &attempts)) + require.NoError(t, reopened.queue.db.QueryRow(`SELECT success, failure_class FROM notification_audit WHERE notification_id = ?`, id).Scan(&success, &failureClass)) + stats, err := reopened.GetTelemetryStats(time.Now().Add(-time.Hour)) + require.NoError(t, err) + require.Equal(t, 1, attempts) + require.Equal(t, 1, stats.Attempts) + require.Equal(t, int32(1), counts.connections.Load()) + require.Equal(t, int32(2), counts.credentials.Load(), "the receiver must check both credentials before its verdict") + if valid { + require.Equal(t, string(QueueStatusSent), status) + require.Equal(t, 1, success) + require.Empty(t, failureClass) + require.Equal(t, 1, stats.Deliveries) + require.Zero(t, stats.Failures) + require.Equal(t, int32(1), counts.accepted.Load()) + } else { + require.Equal(t, string(QueueStatusDLQ), status) + require.Zero(t, success) + require.Equal(t, string(NotificationFailureAuthentication), failureClass) + require.Zero(t, stats.Deliveries) + require.Equal(t, 1, stats.Failures) + require.Equal(t, 1, stats.FailureClasses.Authentication) + require.Zero(t, counts.accepted.Load()) + } + }) + } +} diff --git a/internal/notifications/email_enhanced.go b/internal/notifications/email_enhanced.go index a21c466068..24491912b3 100644 --- a/internal/notifications/email_enhanced.go +++ b/internal/notifications/email_enhanced.go @@ -35,29 +35,37 @@ type EmailAttachment struct { // and reject AUTH PLAIN with "504 5.7.4 Unrecognized authentication type". type loginAuth struct { username, password string + step uint8 } // LoginAuth returns an Auth that implements the LOGIN authentication mechanism. func LoginAuth(username, password string) smtp.Auth { - return &loginAuth{username, password} + return &loginAuth{username: username, password: password} } func (a *loginAuth) Start(server *smtp.ServerInfo) (string, []byte, error) { + a.step = 0 return "LOGIN", nil, nil } -func (a *loginAuth) Next(fromServer []byte, more bool) ([]byte, error) { +func (a *loginAuth) Next(_ []byte, more bool) ([]byte, error) { if !more { + if a.step != 2 { + return nil, FailfWithClass(NotificationFailureAuthentication, "SMTP LOGIN ended before credentials were exchanged") + } return nil, nil } - prompt := strings.TrimSpace(strings.ToLower(string(fromServer))) - switch prompt { - case "username:": + // LOGIN sends username then password. Challenges are opaque display text, + // not protocol tokens, and must not enter diagnostics or select credentials. + switch a.step { + case 0: + a.step++ return []byte(a.username), nil - case "password:": + case 1: + a.step++ return []byte(a.password), nil default: - return nil, FailfWithClass(NotificationFailureAuthentication, "unexpected LOGIN prompt: %s", fromServer) + return nil, FailfWithClass(NotificationFailureAuthentication, "SMTP LOGIN received an extra challenge") } } @@ -452,20 +460,28 @@ func (e *EnhancedEmailManager) negotiateAuth(client *smtp.Client) (smtp.Auth, er } hasAuth, mechanismsRaw := client.Extension("AUTH") - mechanisms := strings.ToUpper(mechanismsRaw) + var supportsPlain, supportsLogin bool + for _, mechanism := range strings.Fields(strings.ToUpper(mechanismsRaw)) { + switch mechanism { + case "PLAIN": + supportsPlain = true + case "LOGIN": + supportsLogin = true + } + } - if strings.Contains(mechanisms, "PLAIN") { + if supportsPlain { return &plainAuth{"", username, e.config.Password}, nil } - if strings.Contains(mechanisms, "LOGIN") { + if supportsLogin { return LoginAuth(username, e.config.Password), nil } - if !hasAuth || mechanisms == "" { + if !hasAuth || strings.TrimSpace(mechanismsRaw) == "" { return &plainAuth{"", username, e.config.Password}, nil } - return nil, fmt.Errorf("server advertises AUTH mechanisms [%s] but none are supported (PLAIN, LOGIN)", mechanisms) + return nil, FailfWithClass(NotificationFailureConfiguration, "SMTP authentication mechanisms: none are supported (PLAIN, LOGIN)") } func resolveProviderUsername(config EmailProviderConfig) string { @@ -675,6 +691,12 @@ func (e *EnhancedEmailManager) sendViaProvider(msg []byte) error { return e.sendViaProviderWithAddresses(msg, addresses) } +// Port 465 selects implicit TLS even when the explicit TLS flag is unset. +// Connection tests and delivery must use the same transport decision. +func (e *EnhancedEmailManager) usesImplicitTLS() bool { + return e.config.TLS || e.config.SMTPPort == 465 +} + func (e *EnhancedEmailManager) sendViaProviderWithAddresses(msg []byte, addresses resolvedEmailAddresses) error { addr := net.JoinHostPort(e.config.SMTPHost, strconv.Itoa(e.config.SMTPPort)) @@ -682,7 +704,7 @@ func (e *EnhancedEmailManager) sendViaProviderWithAddresses(msg []byte, addresse // mutating the shared config struct. // Send with TLS configuration — auth is negotiated after connection - if e.config.TLS || e.config.SMTPPort == 465 { + if e.usesImplicitTLS() { return e.sendTLS(addr, msg, addresses) } else if e.config.StartTLS { return e.sendStartTLS(addr, msg, addresses) @@ -837,7 +859,7 @@ func (e *EnhancedEmailManager) TestConnection() error { var conn net.Conn var err error - if e.config.TLS || e.config.SMTPPort == 465 { + if e.usesImplicitTLS() { tlsConfig := &tls.Config{ ServerName: e.config.SMTPHost, InsecureSkipVerify: e.config.SkipTLSVerify, @@ -867,7 +889,7 @@ func (e *EnhancedEmailManager) TestConnection() error { defer client.Close() // Test STARTTLS if configured - if e.config.StartTLS && !e.config.TLS { + if e.config.StartTLS && !e.usesImplicitTLS() { tlsConfig := &tls.Config{ ServerName: e.config.SMTPHost, InsecureSkipVerify: e.config.SkipTLSVerify,