Skip to content

Commit 55d517f

Browse files
ralyodioclaude
andcommitted
fix(mailbox): verify SMTP STARTTLS against the mail host, not the dial IP
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.<host>. 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 <noreply@anthropic.com>
1 parent 2526684 commit 55d517f

5 files changed

Lines changed: 175 additions & 10 deletions

File tree

cmd/agentbbs/main.go

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1481,8 +1481,12 @@ func (a *app) mailClientFor(su store.User) (*mailbox.Client, error) {
14811481
cfg := mailbox.IMAPConfig{
14821482
IMAPAddr: env("AGENTBBS_MAIL_IMAP_ADDR", a.mailHost+":993"),
14831483
SMTPAddr: env("AGENTBBS_MAIL_SMTP_ADDR", "127.0.0.1:25"),
1484-
Username: login,
1485-
Password: os.Getenv("AGENTBBS_MAIL_MASTER_PASS"),
1484+
// Dial the loopback relay but verify STARTTLS against the mail host, whose
1485+
// certificate it presents (the relay's cert is never for 127.0.0.1). This
1486+
// avoids the /etc/hosts loopback hack the transactional sender needs.
1487+
SMTPServerName: env("AGENTBBS_MAIL_SMTP_SERVERNAME", a.mailHost),
1488+
Username: login,
1489+
Password: os.Getenv("AGENTBBS_MAIL_MASTER_PASS"),
14861490
// Mailu's front nginx pre-authenticates against its user DB before
14871491
// proxying, which rejects the "<addr>*master" master login. The gateway
14881492
// therefore talks to Dovecot directly over loopback (plaintext, on-host)

internal/mailbox/imap.go

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,11 @@ type IMAPConfig struct {
2424
// SMTPUser/SMTPPass default to Username/Password when empty.
2525
SMTPUser string
2626
SMTPPass string
27+
// SMTPServerName is the TLS server name verified during STARTTLS. Set it when
28+
// the dial host differs from the certificate name — e.g. dialing the trusted
29+
// local relay at 127.0.0.1:25 whose cert is mail.<host>. Empty = use the dial
30+
// host (the net/smtp default).
31+
SMTPServerName string
2732
// Plaintext dials IMAP without TLS. Used only for a co-located backend over
2833
// loopback (the Mailu gateway hitting Dovecot directly on 127.0.0.1, bypassing
2934
// 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
219224
func (t *imapTransport) Send(_ context.Context, from string, d Draft) (SendResult, error) {
220225
msg, msgID := buildRFC822(from, d)
221226
// SMTPUser may be empty for a trusted local relay (no AUTH).
222-
if err := smtpSend(t.cfg.SMTPAddr, t.cfg.SMTPUser, t.cfg.SMTPPass, from, recipients(d), msg); err != nil {
227+
if err := smtpSend(t.cfg.SMTPAddr, t.cfg.SMTPServerName, t.cfg.SMTPUser, t.cfg.SMTPPass, from, recipients(d), msg); err != nil {
223228
return SendResult{}, fmt.Errorf("smtp send: %w", err)
224229
}
225230
// Best-effort copy to Sent so the message shows in the member's mailbox.

internal/mailbox/smtp.go

Lines changed: 45 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package mailbox
22

33
import (
44
"bytes"
5+
"crypto/tls"
56
"fmt"
67
"mime"
78
"net"
@@ -65,17 +66,54 @@ func recipients(d Draft) []string {
6566
return out
6667
}
6768

68-
// smtpSend submits a built message via SMTP. With a non-empty user it does
69-
// STARTTLS + AUTH (e.g. smtp.profullstack.com:587); with an empty user it sends
70-
// unauthenticated, for a trusted local relay (e.g. the co-located Postfix).
71-
func smtpSend(addr, user, pass, from string, rcpts []string, msg []byte) error {
69+
// smtpSend submits a built message via SMTP. It does STARTTLS when the server
70+
// offers it, verifying the certificate against serverName (or the dial host when
71+
// serverName is empty) — this lets us dial a trusted local relay by IP/loopback
72+
// while still validating its real hostname certificate. A non-empty user adds
73+
// AUTH; an empty user sends unauthenticated, for a relay that trusts the source.
74+
//
75+
// This mirrors net/smtp.SendMail but with an overridable TLS ServerName, which
76+
// SendMail does not support (it pins ServerName to the dial host).
77+
func smtpSend(addr, serverName, user, pass, from string, rcpts []string, msg []byte) error {
7278
host, _, err := net.SplitHostPort(addr)
7379
if err != nil {
7480
return fmt.Errorf("smtp addr %q: %w", addr, err)
7581
}
76-
var auth smtp.Auth
82+
if serverName == "" {
83+
serverName = host
84+
}
85+
c, err := smtp.Dial(addr)
86+
if err != nil {
87+
return fmt.Errorf("smtp dial %s: %w", addr, err)
88+
}
89+
defer func() { _ = c.Close() }()
90+
if ok, _ := c.Extension("STARTTLS"); ok {
91+
if err := c.StartTLS(&tls.Config{ServerName: serverName}); err != nil {
92+
return fmt.Errorf("smtp starttls (%s): %w", serverName, err)
93+
}
94+
}
7795
if user != "" {
78-
auth = smtp.PlainAuth("", user, pass, host)
96+
if err := c.Auth(smtp.PlainAuth("", user, pass, serverName)); err != nil {
97+
return fmt.Errorf("smtp auth: %w", err)
98+
}
99+
}
100+
if err := c.Mail(from); err != nil {
101+
return fmt.Errorf("smtp mail from: %w", err)
102+
}
103+
for _, rcpt := range rcpts {
104+
if err := c.Rcpt(rcpt); err != nil {
105+
return fmt.Errorf("smtp rcpt %s: %w", rcpt, err)
106+
}
107+
}
108+
w, err := c.Data()
109+
if err != nil {
110+
return fmt.Errorf("smtp data: %w", err)
111+
}
112+
if _, err := w.Write(msg); err != nil {
113+
return fmt.Errorf("smtp write: %w", err)
114+
}
115+
if err := w.Close(); err != nil {
116+
return fmt.Errorf("smtp close: %w", err)
79117
}
80-
return smtp.SendMail(addr, auth, from, rcpts, msg)
118+
return c.Quit()
81119
}

internal/mailbox/smtp_test.go

Lines changed: 115 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,115 @@
1+
package mailbox
2+
3+
import (
4+
"bufio"
5+
"net"
6+
"strings"
7+
"sync"
8+
"testing"
9+
)
10+
11+
// fakeSMTP is a minimal SMTP server (no STARTTLS advertised) that records the
12+
// envelope and body of one delivered message, so we can exercise smtpSend's
13+
// dial→MAIL→RCPT→DATA→QUIT flow without real TLS.
14+
type fakeSMTP struct {
15+
addr string
16+
ln net.Listener
17+
mu sync.Mutex
18+
from string
19+
rcpts []string
20+
body strings.Builder
21+
gotMailFrom bool
22+
}
23+
24+
func newFakeSMTP(t *testing.T) *fakeSMTP {
25+
t.Helper()
26+
ln, err := net.Listen("tcp", "127.0.0.1:0")
27+
if err != nil {
28+
t.Fatal(err)
29+
}
30+
f := &fakeSMTP{addr: ln.Addr().String(), ln: ln}
31+
go f.serve()
32+
t.Cleanup(func() { _ = ln.Close() })
33+
return f
34+
}
35+
36+
func (f *fakeSMTP) serve() {
37+
conn, err := f.ln.Accept()
38+
if err != nil {
39+
return
40+
}
41+
defer conn.Close()
42+
br := bufio.NewReader(conn)
43+
w := func(s string) { _, _ = conn.Write([]byte(s)) }
44+
w("220 mock ESMTP\r\n")
45+
inData := false
46+
for {
47+
line, err := br.ReadString('\n')
48+
if err != nil {
49+
return
50+
}
51+
if inData {
52+
if strings.TrimRight(line, "\r\n") == "." {
53+
inData = false
54+
w("250 ok\r\n")
55+
continue
56+
}
57+
f.mu.Lock()
58+
f.body.WriteString(line)
59+
f.mu.Unlock()
60+
continue
61+
}
62+
up := strings.ToUpper(strings.TrimSpace(line))
63+
switch {
64+
case strings.HasPrefix(up, "EHLO"), strings.HasPrefix(up, "HELO"):
65+
w("250 mock\r\n") // single line => no extensions (no STARTTLS)
66+
case strings.HasPrefix(up, "MAIL FROM"):
67+
f.mu.Lock()
68+
f.from = strings.TrimSpace(line[len("MAIL FROM:"):])
69+
f.gotMailFrom = true
70+
f.mu.Unlock()
71+
w("250 ok\r\n")
72+
case strings.HasPrefix(up, "RCPT TO"):
73+
f.mu.Lock()
74+
f.rcpts = append(f.rcpts, strings.TrimSpace(line[len("RCPT TO:"):]))
75+
f.mu.Unlock()
76+
w("250 ok\r\n")
77+
case strings.HasPrefix(up, "DATA"):
78+
inData = true
79+
w("354 go ahead\r\n")
80+
case strings.HasPrefix(up, "QUIT"):
81+
w("221 bye\r\n")
82+
return
83+
default:
84+
w("250 ok\r\n")
85+
}
86+
}
87+
}
88+
89+
func TestSMTPSendFlow(t *testing.T) {
90+
srv := newFakeSMTP(t)
91+
92+
msg := []byte("Subject: hi\r\n\r\nbody text\r\n")
93+
err := smtpSend(srv.addr, "", "", "", "alice@bbs.test", []string{"bob@example.com", "carol@example.com"}, msg)
94+
if err != nil {
95+
t.Fatalf("smtpSend: %v", err)
96+
}
97+
98+
srv.mu.Lock()
99+
defer srv.mu.Unlock()
100+
if !srv.gotMailFrom || !strings.Contains(srv.from, "alice@bbs.test") {
101+
t.Fatalf("MAIL FROM wrong: %q", srv.from)
102+
}
103+
if len(srv.rcpts) != 2 {
104+
t.Fatalf("want 2 recipients, got %v", srv.rcpts)
105+
}
106+
if !strings.Contains(srv.body.String(), "body text") {
107+
t.Fatalf("body not delivered: %q", srv.body.String())
108+
}
109+
}
110+
111+
func TestSMTPSendBadAddr(t *testing.T) {
112+
if err := smtpSend("not-a-host-port", "", "", "", "a@b", []string{"c@d"}, []byte("x")); err == nil {
113+
t.Fatal("expected error for malformed addr")
114+
}
115+
}

setup.sh

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1129,6 +1129,9 @@ if [ "$MAIL" = "1" ]; then
11291129
upsert_env AGENTBBS_MAIL_IMAP_ADDR "127.0.0.1:14143"
11301130
upsert_env AGENTBBS_MAIL_IMAP_PLAINTEXT "1"
11311131
upsert_env AGENTBBS_MAIL_SMTP_ADDR "127.0.0.1:25"
1132+
# Dial the loopback relay but verify its STARTTLS cert against the mail host
1133+
# (its cert is for ${MAIL_DOMAIN}, never 127.0.0.1) — no /etc/hosts hack needed.
1134+
upsert_env AGENTBBS_MAIL_SMTP_SERVERNAME "${MAIL_DOMAIN}"
11321135

11331136
# Cert refresher: copy Caddy's mail cert into Mailu on renewal (like news/IRC).
11341137
install -m 0755 "${MAILU_DIR}/refresh-certs.sh" /usr/local/bin/agentbbs-mailu-certs

0 commit comments

Comments
 (0)