From 55d517feb46722756155dccc11333678ac488f2a Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Tue, 23 Jun 2026 15:54:30 +0000 Subject: [PATCH] fix(mailbox): verify SMTP STARTTLS against the mail host, not the dial IP MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AgentMail compose/send failed with 'cannot validate certificate for 127.0.0.1 because it doesn't contain any IP SANs': the sender dialed the local relay at 127.0.0.1:25 and net/smtp pinned the TLS ServerName to the dial host, but the relay's cert is for mail.. Reimplement smtpSend (mirrors net/smtp.SendMail) with an overridable IMAPConfig.SMTPServerName; default it to the mail host (AGENTBBS_MAIL_SMTP_SERVERNAME). Now we dial the loopback for relay permission yet verify the real hostname cert — no /etc/hosts hack. setup.sh upserts the new var. Tested against a fake SMTP server (full MAIL/RCPT/DATA flow). Co-Authored-By: Claude Opus 4.8 --- cmd/agentbbs/main.go | 8 ++- internal/mailbox/imap.go | 7 ++- internal/mailbox/smtp.go | 54 +++++++++++++--- internal/mailbox/smtp_test.go | 115 ++++++++++++++++++++++++++++++++++ setup.sh | 3 + 5 files changed, 176 insertions(+), 11 deletions(-) create mode 100644 internal/mailbox/smtp_test.go diff --git a/cmd/agentbbs/main.go b/cmd/agentbbs/main.go index fb896c5..3b1754a 100644 --- a/cmd/agentbbs/main.go +++ b/cmd/agentbbs/main.go @@ -1481,8 +1481,12 @@ func (a *app) mailClientFor(su store.User) (*mailbox.Client, error) { cfg := mailbox.IMAPConfig{ IMAPAddr: env("AGENTBBS_MAIL_IMAP_ADDR", a.mailHost+":993"), SMTPAddr: env("AGENTBBS_MAIL_SMTP_ADDR", "127.0.0.1:25"), - Username: login, - Password: os.Getenv("AGENTBBS_MAIL_MASTER_PASS"), + // Dial the loopback relay but verify STARTTLS against the mail host, whose + // certificate it presents (the relay's cert is never for 127.0.0.1). This + // avoids the /etc/hosts loopback hack the transactional sender needs. + SMTPServerName: env("AGENTBBS_MAIL_SMTP_SERVERNAME", a.mailHost), + Username: login, + Password: os.Getenv("AGENTBBS_MAIL_MASTER_PASS"), // Mailu's front nginx pre-authenticates against its user DB before // proxying, which rejects the "*master" master login. The gateway // therefore talks to Dovecot directly over loopback (plaintext, on-host) diff --git a/internal/mailbox/imap.go b/internal/mailbox/imap.go index 2786c1e..8b5b768 100644 --- a/internal/mailbox/imap.go +++ b/internal/mailbox/imap.go @@ -24,6 +24,11 @@ type IMAPConfig struct { // SMTPUser/SMTPPass default to Username/Password when empty. SMTPUser string SMTPPass string + // SMTPServerName is the TLS server name verified during STARTTLS. Set it when + // the dial host differs from the certificate name — e.g. dialing the trusted + // local relay at 127.0.0.1:25 whose cert is mail.. Empty = use the dial + // host (the net/smtp default). + SMTPServerName string // Plaintext dials IMAP without TLS. Used only for a co-located backend over // loopback (the Mailu gateway hitting Dovecot directly on 127.0.0.1, bypassing // the front's auth proxy so master-user login works) — the password never @@ -219,7 +224,7 @@ func (t *imapTransport) Search(_ context.Context, opts SearchOptions) ([]Message func (t *imapTransport) Send(_ context.Context, from string, d Draft) (SendResult, error) { msg, msgID := buildRFC822(from, d) // SMTPUser may be empty for a trusted local relay (no AUTH). - if err := smtpSend(t.cfg.SMTPAddr, t.cfg.SMTPUser, t.cfg.SMTPPass, from, recipients(d), msg); err != nil { + if err := smtpSend(t.cfg.SMTPAddr, t.cfg.SMTPServerName, t.cfg.SMTPUser, t.cfg.SMTPPass, from, recipients(d), msg); err != nil { return SendResult{}, fmt.Errorf("smtp send: %w", err) } // Best-effort copy to Sent so the message shows in the member's mailbox. diff --git a/internal/mailbox/smtp.go b/internal/mailbox/smtp.go index 19b5619..a0ea55d 100644 --- a/internal/mailbox/smtp.go +++ b/internal/mailbox/smtp.go @@ -2,6 +2,7 @@ package mailbox import ( "bytes" + "crypto/tls" "fmt" "mime" "net" @@ -65,17 +66,54 @@ func recipients(d Draft) []string { return out } -// smtpSend submits a built message via SMTP. With a non-empty user it does -// STARTTLS + AUTH (e.g. smtp.profullstack.com:587); with an empty user it sends -// unauthenticated, for a trusted local relay (e.g. the co-located Postfix). -func smtpSend(addr, user, pass, from string, rcpts []string, msg []byte) error { +// smtpSend submits a built message via SMTP. It does STARTTLS when the server +// offers it, verifying the certificate against serverName (or the dial host when +// serverName is empty) — this lets us dial a trusted local relay by IP/loopback +// while still validating its real hostname certificate. A non-empty user adds +// AUTH; an empty user sends unauthenticated, for a relay that trusts the source. +// +// This mirrors net/smtp.SendMail but with an overridable TLS ServerName, which +// SendMail does not support (it pins ServerName to the dial host). +func smtpSend(addr, serverName, user, pass, from string, rcpts []string, msg []byte) error { host, _, err := net.SplitHostPort(addr) if err != nil { return fmt.Errorf("smtp addr %q: %w", addr, err) } - var auth smtp.Auth - if user != "" { - auth = smtp.PlainAuth("", user, pass, host) + if serverName == "" { + serverName = host } - return smtp.SendMail(addr, auth, from, rcpts, msg) + c, err := smtp.Dial(addr) + if err != nil { + return fmt.Errorf("smtp dial %s: %w", addr, err) + } + defer func() { _ = c.Close() }() + if ok, _ := c.Extension("STARTTLS"); ok { + if err := c.StartTLS(&tls.Config{ServerName: serverName}); err != nil { + return fmt.Errorf("smtp starttls (%s): %w", serverName, err) + } + } + if user != "" { + if err := c.Auth(smtp.PlainAuth("", user, pass, serverName)); err != nil { + return fmt.Errorf("smtp auth: %w", err) + } + } + if err := c.Mail(from); err != nil { + return fmt.Errorf("smtp mail from: %w", err) + } + for _, rcpt := range rcpts { + if err := c.Rcpt(rcpt); err != nil { + return fmt.Errorf("smtp rcpt %s: %w", rcpt, err) + } + } + w, err := c.Data() + if err != nil { + return fmt.Errorf("smtp data: %w", err) + } + if _, err := w.Write(msg); err != nil { + return fmt.Errorf("smtp write: %w", err) + } + if err := w.Close(); err != nil { + return fmt.Errorf("smtp close: %w", err) + } + return c.Quit() } diff --git a/internal/mailbox/smtp_test.go b/internal/mailbox/smtp_test.go new file mode 100644 index 0000000..dd5fd9b --- /dev/null +++ b/internal/mailbox/smtp_test.go @@ -0,0 +1,115 @@ +package mailbox + +import ( + "bufio" + "net" + "strings" + "sync" + "testing" +) + +// fakeSMTP is a minimal SMTP server (no STARTTLS advertised) that records the +// envelope and body of one delivered message, so we can exercise smtpSend's +// dial→MAIL→RCPT→DATA→QUIT flow without real TLS. +type fakeSMTP struct { + addr string + ln net.Listener + mu sync.Mutex + from string + rcpts []string + body strings.Builder + gotMailFrom bool +} + +func newFakeSMTP(t *testing.T) *fakeSMTP { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatal(err) + } + f := &fakeSMTP{addr: ln.Addr().String(), ln: ln} + go f.serve() + t.Cleanup(func() { _ = ln.Close() }) + return f +} + +func (f *fakeSMTP) serve() { + conn, err := f.ln.Accept() + if err != nil { + return + } + defer conn.Close() + br := bufio.NewReader(conn) + w := func(s string) { _, _ = conn.Write([]byte(s)) } + w("220 mock ESMTP\r\n") + inData := false + for { + line, err := br.ReadString('\n') + if err != nil { + return + } + if inData { + if strings.TrimRight(line, "\r\n") == "." { + inData = false + w("250 ok\r\n") + continue + } + f.mu.Lock() + f.body.WriteString(line) + f.mu.Unlock() + continue + } + up := strings.ToUpper(strings.TrimSpace(line)) + switch { + case strings.HasPrefix(up, "EHLO"), strings.HasPrefix(up, "HELO"): + w("250 mock\r\n") // single line => no extensions (no STARTTLS) + case strings.HasPrefix(up, "MAIL FROM"): + f.mu.Lock() + f.from = strings.TrimSpace(line[len("MAIL FROM:"):]) + f.gotMailFrom = true + f.mu.Unlock() + w("250 ok\r\n") + case strings.HasPrefix(up, "RCPT TO"): + f.mu.Lock() + f.rcpts = append(f.rcpts, strings.TrimSpace(line[len("RCPT TO:"):])) + f.mu.Unlock() + w("250 ok\r\n") + case strings.HasPrefix(up, "DATA"): + inData = true + w("354 go ahead\r\n") + case strings.HasPrefix(up, "QUIT"): + w("221 bye\r\n") + return + default: + w("250 ok\r\n") + } + } +} + +func TestSMTPSendFlow(t *testing.T) { + srv := newFakeSMTP(t) + + msg := []byte("Subject: hi\r\n\r\nbody text\r\n") + err := smtpSend(srv.addr, "", "", "", "alice@bbs.test", []string{"bob@example.com", "carol@example.com"}, msg) + if err != nil { + t.Fatalf("smtpSend: %v", err) + } + + srv.mu.Lock() + defer srv.mu.Unlock() + if !srv.gotMailFrom || !strings.Contains(srv.from, "alice@bbs.test") { + t.Fatalf("MAIL FROM wrong: %q", srv.from) + } + if len(srv.rcpts) != 2 { + t.Fatalf("want 2 recipients, got %v", srv.rcpts) + } + if !strings.Contains(srv.body.String(), "body text") { + t.Fatalf("body not delivered: %q", srv.body.String()) + } +} + +func TestSMTPSendBadAddr(t *testing.T) { + if err := smtpSend("not-a-host-port", "", "", "", "a@b", []string{"c@d"}, []byte("x")); err == nil { + t.Fatal("expected error for malformed addr") + } +} diff --git a/setup.sh b/setup.sh index 7f02cf3..dbd3bc5 100755 --- a/setup.sh +++ b/setup.sh @@ -1129,6 +1129,9 @@ if [ "$MAIL" = "1" ]; then upsert_env AGENTBBS_MAIL_IMAP_ADDR "127.0.0.1:14143" upsert_env AGENTBBS_MAIL_IMAP_PLAINTEXT "1" upsert_env AGENTBBS_MAIL_SMTP_ADDR "127.0.0.1:25" + # Dial the loopback relay but verify its STARTTLS cert against the mail host + # (its cert is for ${MAIL_DOMAIN}, never 127.0.0.1) — no /etc/hosts hack needed. + upsert_env AGENTBBS_MAIL_SMTP_SERVERNAME "${MAIL_DOMAIN}" # Cert refresher: copy Caddy's mail cert into Mailu on renewal (like news/IRC). install -m 0755 "${MAILU_DIR}/refresh-certs.sh" /usr/local/bin/agentbbs-mailu-certs