diff --git a/modules/mesh-mail/cmd/mesh-mail/send_test.go b/modules/mesh-mail/cmd/mesh-mail/send_test.go index 59e65438..ca01c877 100644 --- a/modules/mesh-mail/cmd/mesh-mail/send_test.go +++ b/modules/mesh-mail/cmd/mesh-mail/send_test.go @@ -684,7 +684,7 @@ func TestMailSendRefusesAControlCharacterInTheSubjectOrAnAddress(t *testing.T) { func TestSendUsesTheParsedAddressForRcptAndTheFormattedAddressForTheToHeader(t *testing.T) { fake := newFakeSMTPServer(t) acc := Account{Name: "mesh", Address: "jschoubben@novox.be", Outgoing: &Server{Host: "127.0.0.1", Port: 25, Security: "tls"}} - if err := SendMail(fake.dialer(), acc, "the-password", []string{"Ops Team "}, "Status", "body", judgedAt); err != nil { + if err := SendMail(fake.dialer(), acc, "the-password", []string{"Ops Team "}, "Status", "body", judgedAt, "send-abc123"); err != nil { t.Fatalf("SendMail: %v", err) } diff --git a/modules/mesh-mail/cmd/mesh-mail/smtp.go b/modules/mesh-mail/cmd/mesh-mail/smtp.go index bc12d406..0d542ba6 100644 --- a/modules/mesh-mail/cmd/mesh-mail/smtp.go +++ b/modules/mesh-mail/cmd/mesh-mail/smtp.go @@ -8,10 +8,12 @@ package main import ( "crypto/tls" + "errors" "fmt" "mime" "net/mail" "net/smtp" + "net/textproto" "strings" "time" "unicode" @@ -55,10 +57,63 @@ func DialSMTP(server Server) (*smtp.Client, error) { } } -// SendMail sends one message through the account's outgoing server (novox/hq ADR 0361 §1, §2): signs in with the -// password given, states every recipient, and writes the message once. Never logs the password or the body; an -// error names the server and what it said, never either. -func SendMail(dial SMTPDialer, a Account, password string, to []string, subject, body string, now time.Time) error { +// SendOutcome classifies how a send attempt ended (novox/hq ADR 0436 clause 10), so the sender knows +// whether a retry is safe, certainly wrong, or neither: +// +// - NotTaken: no connection, the sign-in refused, or a 4xx reply to MAIL, RCPT or DATA, or to the end of +// data — the server provably never took the mail; retrying is safe. +// - Refused: a 5xx reply before or at the end of data — the server provably refused the mail; never +// retried. +// - NotProven: the connection was lost after the data was written, with no reply at all — the mail may or +// may not have gone out; never retried automatically either way. +type SendOutcome int + +const ( + NotTaken SendOutcome = iota + Refused + NotProven +) + +func (o SendOutcome) String() string { + switch o { + case Refused: + return "refused" + case NotProven: + return "not proven" + default: + return "not taken" + } +} + +// SendError classifies a SendMail failure (novox/hq ADR 0436 clause 10): Outcome says which, and the error +// itself (Unwrap) carries the server's own words where there were any, never the password or the body. +type SendError struct { + Outcome SendOutcome + Err error +} + +func (e *SendError) Error() string { return e.Err.Error() } +func (e *SendError) Unwrap() error { return e.Err } + +// classifyReply is NotTaken for a 4xx reply, Refused for a 5xx, and NotTaken for anything else (no +// connection, or the connection lost before a reply was read at all) — the safe default everywhere but the +// one case clause 10 names its own outcome for: the connection lost after the data was written, which the +// caller classifies as NotProven itself, never through this. +func classifyReply(err error) SendOutcome { + var proto *textproto.Error + if errors.As(err, &proto) && proto.Code >= 500 { + return Refused + } + return NotTaken +} + +// SendMail sends one message through the account's outgoing server (novox/hq ADR 0361 §1, §2; ADR 0436 +// clause 10): signs in with the password given, states every recipient, and writes the message once, its +// Message-ID naming sendID so the mail is never sent twice within the day (clause 10's own guarantee, kept +// by the sender's compare-and-set claim and the bus's duplicate window on the id alone — this is the +// mail's own proof of it). Never logs the password or the body; an error names the server and what it +// said, never either, and is always a *SendError (classifyReply, or NotProven below). +func SendMail(dial SMTPDialer, a Account, password string, to []string, subject, body string, now time.Time, sendID string) error { if a.Outgoing == nil { return fmt.Errorf("the account %s has no outgoing server", a.Name) } @@ -76,44 +131,63 @@ func SendMail(dial SMTPDialer, a Account, password string, to []string, subject, } client, err := dial(*a.Outgoing) if err != nil { - return err + return &SendError{Outcome: NotTaken, Err: err} } defer client.Close() from := a.SignInAs() auth := smtp.PlainAuth("", from, password, a.Outgoing.Host) if err := client.Auth(auth); err != nil { - return fmt.Errorf("signing in to %s failed: %w", a.Outgoing.Address(), err) + // A refused sign-in is its own case, never classified by a reply code: the server never took the + // mail either way (novox/hq ADR 0436 clause 10). + return &SendError{Outcome: NotTaken, Err: fmt.Errorf("signing in to %s failed: %w", a.Outgoing.Address(), err)} } if err := client.Mail(from); err != nil { - return fmt.Errorf("the outgoing server refused the sender: %w", err) + return &SendError{Outcome: classifyReply(err), Err: fmt.Errorf("the outgoing server refused the sender: %w", err)} } for _, rcpt := range rcpts { if err := client.Rcpt(rcpt); err != nil { - return fmt.Errorf("the outgoing server refused the recipient %s: %w", rcpt, err) + return &SendError{Outcome: classifyReply(err), Err: fmt.Errorf("the outgoing server refused the recipient %s: %w", rcpt, err)} } } w, err := client.Data() if err != nil { - return fmt.Errorf("the outgoing server refused the message: %w", err) + return &SendError{Outcome: classifyReply(err), Err: fmt.Errorf("the outgoing server refused the message: %w", err)} } - if _, err := w.Write(composeMessage(from, headers, subject, body, now)); err != nil { - return fmt.Errorf("the message could not be written: %w", err) + if _, err := w.Write(composeMessage(from, headers, subject, body, now, sendID)); err != nil { + // The data was not written whole: never "after the data was written" (clause 10's not-proven case), + // so this is not taken, not not-proven — nothing coherent reached the server. + return &SendError{Outcome: NotTaken, Err: fmt.Errorf("the message could not be written: %w", err)} } if err := w.Close(); err != nil { - return fmt.Errorf("the outgoing server did not accept the message: %w", err) + // Close is what sends the terminating "." and reads the server's one reply to the whole message + // (SMTP's "end of data", clause 10): a real reply here classifies exactly as MAIL or RCPT's does; no + // reply at all (the connection lost after the data was written) is not proven — never retried + // automatically either way. + outcome := NotProven + var proto *textproto.Error + if errors.As(err, &proto) { + outcome = classifyReply(err) + } + return &SendError{Outcome: outcome, Err: fmt.Errorf("the outgoing server did not accept the message: %w", err)} } - return client.Quit() + // The message is already accepted (Close's 250 reply); Quit is a courtesy goodbye, and its own failure + // here never turns an accepted send into a failed one. + _ = client.Quit() + return nil } // composeMessage is a plain-text message, its lines CRLF-terminated as SMTP takes them. to is the recipients // already formatted (mail.Address.String()), the same ones Rcpt was given, never the raw strings a caller gave. -func composeMessage(from string, to []string, subject, body string, now time.Time) []byte { +// Its Message-ID names sendID (novox/hq ADR 0436 clause 10), so the mail carries the proof of which send it +// is, at @. +func composeMessage(from string, to []string, subject, body string, now time.Time, sendID string) []byte { var b strings.Builder header := func(k, v string) { b.WriteString(k + ": " + v + "\r\n") } header("From", from) header("To", strings.Join(to, ", ")) header("Subject", encodeHeader(subject)) header("Date", now.Format(time.RFC1123Z)) + header("Message-Id", "<"+sendID+"@"+messageIDDomain(from)+">") header("MIME-Version", "1.0") header("Content-Type", "text/plain; charset=utf-8") header("Content-Transfer-Encoding", "8bit") @@ -122,6 +196,20 @@ func composeMessage(from string, to []string, subject, body string, now time.Tim return []byte(b.String()) } +// messageIDDomain is the domain a Message-ID is minted under: the part of from after its @, as a bare +// address or parsed out of "Name "; "localhost" when from carries none (never empty, which is not a +// valid Message-ID). +func messageIDDomain(from string) string { + addr := from + if parsed, err := mail.ParseAddress(from); err == nil { + addr = parsed.Address + } + if _, domain, found := strings.Cut(addr, "@"); found && domain != "" { + return domain + } + return "localhost" +} + // encodeHeader word-encodes a header's value when it holds more than ASCII; a header a phone never needs to show. func encodeHeader(s string) string { for _, r := range s { diff --git a/modules/mesh-mail/cmd/mesh-mail/smtp_classify_test.go b/modules/mesh-mail/cmd/mesh-mail/smtp_classify_test.go new file mode 100644 index 00000000..28fcabd0 --- /dev/null +++ b/modules/mesh-mail/cmd/mesh-mail/smtp_classify_test.go @@ -0,0 +1,255 @@ +package main + +// SendMail classifies every failure (novox/hq ADR 0436 clause 10, issue 1197): not taken (no connection, a +// refused sign-in, or a 4xx reply to MAIL, RCPT or DATA or to the end of data), refused (a 5xx before or at +// the end of data), or not proven (the connection lost after the data was written, before any reply). Its +// Message-ID names the send id. + +import ( + "bufio" + "errors" + "fmt" + "net" + "net/smtp" + "strings" + "testing" +) + +// step is one scripted SMTP reply, or a drop (the connection closed instead of a reply). +type step struct { + drop bool + code int + msg string +} + +func reply(code int, msg string) step { return step{code: code, msg: msg} } +func drop() step { return step{drop: true} } + +// scriptedSMTPServer answers EHLO, AUTH, MAIL, RCPT, DATA's own 354 and the end-of-data reply each with a +// step a test sets, defaulting to the ordinary success sequence — so a test overrides exactly the one stage +// its case is about. +type scriptedSMTPServer struct { + ln net.Listener + ehlo, auth, mail, rcpt, dataOpen, dataEnd step +} + +func newScriptedSMTPServer(t *testing.T) *scriptedSMTPServer { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + s := &scriptedSMTPServer{ln: ln, ehlo: reply(250, "fake.local"), auth: reply(235, "authenticated"), + mail: reply(250, "OK"), rcpt: reply(250, "OK"), dataOpen: reply(354, "go ahead"), dataEnd: reply(250, "queued")} + go s.serve() + t.Cleanup(func() { _ = ln.Close() }) + return s +} + +func (s *scriptedSMTPServer) addr() string { return s.ln.Addr().String() } + +func (s *scriptedSMTPServer) dialer() SMTPDialer { + addr := s.addr() + return func(Server) (*smtp.Client, error) { return smtp.Dial(addr) } +} + +func (s *scriptedSMTPServer) send(conn net.Conn, st step) bool { + if st.drop { + _ = conn.Close() + return false + } + fmt.Fprintf(conn, "%d %s\r\n", st.code, st.msg) + return true +} + +func (s *scriptedSMTPServer) serve() { + for { + conn, err := s.ln.Accept() + if err != nil { + return + } + go s.handle(conn) + } +} + +func (s *scriptedSMTPServer) handle(conn net.Conn) { + defer conn.Close() + r := bufio.NewReader(conn) + fmt.Fprintf(conn, "220 fake.local ESMTP\r\n") + for { + line, err := r.ReadString('\n') + if err != nil { + return + } + upper := strings.ToUpper(strings.TrimRight(line, "\r\n")) + switch { + case strings.HasPrefix(upper, "EHLO"), strings.HasPrefix(upper, "HELO"): + if !s.send(conn, s.ehlo) { + return + } + case strings.HasPrefix(upper, "AUTH"): + if !s.send(conn, s.auth) { + return + } + case strings.HasPrefix(upper, "MAIL"): + if !s.send(conn, s.mail) { + return + } + case strings.HasPrefix(upper, "RCPT"): + if !s.send(conn, s.rcpt) { + return + } + case upper == "DATA": + if !s.send(conn, s.dataOpen) { + return + } + if s.dataOpen.code >= 400 { + continue // not entering data mode; the client ends the transaction itself + } + for { + dl, err := r.ReadString('\n') + if err != nil { + return + } + if dl == ".\r\n" || dl == ".\n" { + break + } + } + s.send(conn, s.dataEnd) // a drop here is "the connection lost after the data was written" + return + case upper == "QUIT": + fmt.Fprintf(conn, "221 bye\r\n") + return + default: + fmt.Fprintf(conn, "500 unrecognized\r\n") + } + } +} + +// classifyTestAccount is an account whose outgoing server is the given address: Host is its loopback +// literal, which PlainAuth itself accepts without TLS (its own isLocalhost check). +func classifyTestAccount(addr string) Account { + host, portStr, _ := net.SplitHostPort(addr) + var port int + _, _ = fmt.Sscanf(portStr, "%d", &port) + return Account{Name: "mesh", Address: "jschoubben@novox.be", Outgoing: &Server{Host: host, Port: port, Security: "tls"}} +} + +func outcomeOf(t *testing.T, err error) SendOutcome { + t.Helper() + var se *SendError + if !errors.As(err, &se) { + t.Fatalf("SendMail's error is not a *SendError: %v (%T)", err, err) + } + return se.Outcome +} + +// 1197/AC1: given no connection, a refused sign-in, or a 4xx reply to MAIL, RCPT or DATA or to the end of +// data, when SendMail runs, then it says not taken with the server's words. +func TestSendMailSaysNotTaken(t *testing.T) { + t.Run("no connection", func(t *testing.T) { + s := newScriptedSMTPServer(t) + addr := s.addr() + s.ln.Close() // nothing listens by the time SendMail dials + acc := classifyTestAccount(addr) + dial := func(Server) (*smtp.Client, error) { return smtp.Dial(addr) } + err := SendMail(dial, acc, "pw", []string{"a@b.c"}, "s", "body", judgedAt, "send-1") + if err == nil || outcomeOf(t, err) != NotTaken { + t.Fatalf("err %v, want NotTaken", err) + } + }) + t.Run("a refused sign-in", func(t *testing.T) { + s := newScriptedSMTPServer(t) + s.auth = reply(535, "authentication failed") + acc := classifyTestAccount(s.addr()) + err := SendMail(s.dialer(), acc, "pw", []string{"a@b.c"}, "s", "body", judgedAt, "send-1") + if err == nil || outcomeOf(t, err) != NotTaken || !strings.Contains(err.Error(), "signing in") { + t.Fatalf("err %v, want NotTaken naming the sign-in", err) + } + }) + for _, c := range []struct { + name string + set func(*scriptedSMTPServer) + }{ + {"a 4xx to MAIL", func(s *scriptedSMTPServer) { s.mail = reply(451, "try again later") }}, + {"a 4xx to RCPT", func(s *scriptedSMTPServer) { s.rcpt = reply(450, "mailbox busy") }}, + {"a 4xx to DATA", func(s *scriptedSMTPServer) { s.dataOpen = reply(452, "insufficient storage") }}, + {"a 4xx at the end of data", func(s *scriptedSMTPServer) { s.dataEnd = reply(451, "local error") }}, + } { + t.Run(c.name, func(t *testing.T) { + s := newScriptedSMTPServer(t) + c.set(s) + acc := classifyTestAccount(s.addr()) + err := SendMail(s.dialer(), acc, "pw", []string{"a@b.c"}, "s", "body", judgedAt, "send-1") + if err == nil || outcomeOf(t, err) != NotTaken { + t.Fatalf("err %v, want NotTaken", err) + } + }) + } +} + +// 1197/AC2: given a 5xx reply before or at the end of data, then it says refused with the server's words. +func TestSendMailSaysRefused(t *testing.T) { + for _, c := range []struct { + name string + set func(*scriptedSMTPServer) + }{ + {"a 5xx to MAIL", func(s *scriptedSMTPServer) { s.mail = reply(550, "no such sender") }}, + {"a 5xx to RCPT", func(s *scriptedSMTPServer) { s.rcpt = reply(550, "no such recipient") }}, + {"a 5xx to DATA", func(s *scriptedSMTPServer) { s.dataOpen = reply(554, "transaction failed") }}, + {"a 5xx at the end of data", func(s *scriptedSMTPServer) { s.dataEnd = reply(552, "quota exceeded") }}, + } { + t.Run(c.name, func(t *testing.T) { + s := newScriptedSMTPServer(t) + c.set(s) + acc := classifyTestAccount(s.addr()) + err := SendMail(s.dialer(), acc, "pw", []string{"a@b.c"}, "s", "body", judgedAt, "send-1") + if err == nil || outcomeOf(t, err) != Refused { + t.Fatalf("err %v, want Refused", err) + } + }) + } +} + +// 1197/AC3: given the connection lost after the data was written and before any reply, then it says not +// proven. +func TestSendMailSaysNotProvenWhenTheConnectionIsLostAfterTheDataWasWritten(t *testing.T) { + s := newScriptedSMTPServer(t) + s.dataEnd = drop() + acc := classifyTestAccount(s.addr()) + err := SendMail(s.dialer(), acc, "pw", []string{"a@b.c"}, "s", "body", judgedAt, "send-1") + if err == nil || outcomeOf(t, err) != NotProven { + t.Fatalf("err %v, want NotProven", err) + } +} + +// 1197/AC4: given a send id, when the message is composed, then it carries a Message-ID naming the send id +// (@). Proved both in isolation (composeMessage) and over the wire +// (SendMail, with the existing fakeSMTPServer, which keeps the body it received). +func TestComposeMessageCarriesTheSendIDInItsMessageID(t *testing.T) { + raw := composeMessage("jschoubben@novox.be", []string{"a@b.c"}, "s", "body", judgedAt, "send-abc123") + if !strings.Contains(string(raw), "Message-Id: ") { + t.Fatalf("the message does not carry the send id's Message-ID:\n%s", raw) + } +} + +// composeMessage falls back to "localhost" when From carries no domain at all (never an empty or +// malformed Message-ID). +func TestComposeMessageFallsBackToLocalhostWithNoDomain(t *testing.T) { + raw := composeMessage("postmaster", nil, "s", "body", judgedAt, "send-1") + if !strings.Contains(string(raw), "Message-Id: ") { + t.Fatalf("message: %s", raw) + } +} + +func TestSendMailCarriesTheMessageIDOverTheWire(t *testing.T) { + fake := newFakeSMTPServer(t) + acc := classifyTestAccount(fake.addr()) + if err := SendMail(fake.dialer(), acc, "pw", []string{"a@b.c"}, "s", "body", judgedAt, "send-xyz"); err != nil { + t.Fatalf("SendMail: %v", err) + } + got := fake.last() + if !strings.Contains(got.data, "Message-Id: ") { + t.Fatalf("the message sent does not carry the send id's Message-ID:\n%s", got.data) + } +}