From 2fb7a5223314db57ca6ac791739506142de0e7b6 Mon Sep 17 00:00:00 2001 From: Anthony Ettinger Date: Tue, 22 Sep 2026 14:34:39 +0000 Subject: [PATCH] fix(mail): unbreak join@ registration when the Mailu cert lapses Registration has been dead since 2026-09-13. `ssh join@bbs.profullstack.com` creates the account, then fails at the confirmation step with "couldn't email the code" and disconnects, so nobody can finish signing up. Cause: Caddy owns ACME for mail.profullstack.com and renewed on 2026-08-14 (valid to Nov 12), but Mailu went on serving the certificate it loaded at container start (Jun 15 -> Sep 13). When that lapsed, the STARTTLS handshake from internal/mail started failing verification and every transactional send died with it -- confirmation codes, signup notifications, credential mail. Reproduced against production; 25/465/993 all still present the expired cert while :443 serves the renewed one. Three things let a single stale certificate take registration down: - setup.sh installed the refresher and enabled its *timer*, but never ran it. `systemctl enable --now ` starts the timer, not the service, so a redeploy left a stale cert in place (and did nothing at all if the timer was never scheduled). The news and IRC sections already run theirs at provision time; the Mailu section now does too, which is what repairs the live host. - refresh-certs.sh only compared files, so a copy whose reload silently failed left a fresh cert on disk and an expiring one on the wire -- invisible. It now reads back what the relay actually serves, forces a reload when that disagrees with /certs, refuses to copy a source cert that is itself expired, and no longer swallows the `docker compose restart` failure. It restarts `front` alone, the only container that mounts ./certs. - internal/mail verified the relay's certificate even on loopback, where there is nothing to intercept. It now skips verification for a loopback relay (the reasoning docs/mail.md already applies to the plaintext Dovecot hand-off) and gains AGENTBBS_SMTP_SERVERNAME, mirroring AGENTBBS_MAIL_SMTP_SERVERNAME, so the documented 127.0.0.1:25 config can verify against the mail host instead of an IP literal. A non-loopback relay is still verified. Errors are wrapped with the address and the failing stage so the next failure is one journal line to diagnose rather than nine days of silence. Tests cover the envelope, the unreachable-relay message, and both halves of the TLS decision: a loopback relay with an expired cert delivers, a non-loopback one with the same cert is refused. Co-Authored-By: Claude Opus 5 (1M context) --- deploy/mailu/refresh-certs.sh | 66 +++++-- docs/credentials.md | 1 + docs/mail.md | 28 +++ internal/mail/mail.go | 113 ++++++++++-- internal/mail/mail_test.go | 329 ++++++++++++++++++++++++++++++++++ setup.sh | 11 ++ 6 files changed, 519 insertions(+), 29 deletions(-) create mode 100644 internal/mail/mail_test.go diff --git a/deploy/mailu/refresh-certs.sh b/deploy/mailu/refresh-certs.sh index 37a598f..59e1237 100755 --- a/deploy/mailu/refresh-certs.sh +++ b/deploy/mailu/refresh-certs.sh @@ -1,14 +1,22 @@ #!/usr/bin/env bash # # refresh-certs.sh — copy Caddy's Let's Encrypt cert for mail.$DOMAIN into the -# Mailu certs dir (TLS_FLAVOR=mail), so Postfix/Dovecot TLS on 465/587/993 track -# Caddy's auto-renewals. Mirrors deploy/news-refresh-certs.sh: Caddy is the only -# ACME client on the box (it serves the mail.$DOMAIN site block), and we reuse -# that cert rather than running a second ACME client inside Mailu. +# Mailu certs dir (TLS_FLAVOR=mail), so Postfix/Dovecot TLS on 25/465/587/993 +# track Caddy's auto-renewals. Mirrors deploy/news-refresh-certs.sh: Caddy is the +# only ACME client on the box (it serves the mail.$DOMAIN site block), and we +# reuse that cert rather than running a second ACME client inside Mailu. # # Install to /usr/local/bin/agentbbs-mailu-certs and run from a timer. Reloads -# the Mailu front/smtp/imap so the new cert is picked up. Exits non-zero -# (touching nothing) until Caddy has issued the cert. +# the Mailu front so the new cert is picked up. Exits non-zero (touching +# nothing) until Caddy has issued the cert. +# +# Copying the file is not the whole job. Mailu keeps serving whatever it loaded +# at container start, so a copy whose reload silently failed leaves a fresh cert +# on disk and an expiring one on the wire. That is exactly how join@ stopped +# being able to email confirmation codes: Caddy renewed on 2026-08-14, Mailu went +# on serving the Jun 15 cert, and registration broke when it expired on Sep 13. +# So we also compare what the relay ACTUALLY serves against the file and force a +# reload when they disagree, and we no longer swallow reload failures. set -euo pipefail DOMAIN="${DOMAIN:?set DOMAIN}" @@ -16,13 +24,37 @@ MAIL_HOST="${MAIL_HOST:-mail.${DOMAIN}}" MAILU_DIR="${MAILU_DIR:-/opt/agentbbs/deploy/mailu}" CERT_DIR="${CERT_DIR:-$MAILU_DIR/certs}" CADDY_DATA="${CADDY_DATA:-/var/lib/caddy/.local/share/caddy}" +# Port used to read back the cert the relay is really serving (loopback SMTP). +PROBE_ADDR="${PROBE_ADDR:-127.0.0.1:25}" + +# notAfter of a PEM file, or empty if it can't be read. +cert_not_after() { + openssl x509 -noout -enddate -in "$1" 2>/dev/null | sed 's/^notAfter=//' +} + +# notAfter of the cert the running relay serves over STARTTLS, or empty if the +# probe can't be made (openssl missing, port closed, Mailu down). +served_not_after() { + command -v openssl >/dev/null 2>&1 || return 0 + printf 'QUIT\r\n' \ + | timeout 10 openssl s_client -quiet -starttls smtp \ + -connect "$PROBE_ADDR" -servername "$MAIL_HOST" 2>/dev/null \ + | openssl x509 -noout -enddate 2>/dev/null | sed 's/^notAfter=//' +} # Caddy stores certs under certificates///.{crt,key}; # the ACME directory segment varies (prod vs staging), so glob for it. crt="$(ls "$CADDY_DATA"/certificates/*/"$MAIL_HOST"/"$MAIL_HOST".crt 2>/dev/null | head -1 || true)" key="$(ls "$CADDY_DATA"/certificates/*/"$MAIL_HOST"/"$MAIL_HOST".key 2>/dev/null | head -1 || true)" if [ -z "$crt" ] || [ -z "$key" ]; then - echo "no Caddy cert for $MAIL_HOST yet (looked under $CADDY_DATA/certificates)" + echo "no Caddy cert for $MAIL_HOST yet (looked under $CADDY_DATA/certificates)" >&2 + exit 1 +fi + +# A source cert that is itself expired means Caddy's renewal is broken, which is +# a different fault and one no amount of copying fixes. Say so loudly. +if ! openssl x509 -checkend 0 -noout -in "$crt" >/dev/null 2>&1; then + echo "Caddy's cert for $MAIL_HOST is EXPIRED ($(cert_not_after "$crt")) — fix Caddy's renewal; not copying" >&2 exit 1 fi @@ -33,9 +65,21 @@ changed=0 if ! cmp -s "$crt" "$CERT_DIR/cert.pem"; then install -m 0644 "$crt" "$CERT_DIR/cert.pem"; changed=1; fi if ! cmp -s "$key" "$CERT_DIR/key.pem"; then install -m 0640 "$key" "$CERT_DIR/key.pem"; changed=1; fi -if [ "$changed" = 1 ]; then - echo "updated Mailu TLS cert for $MAIL_HOST; reloading Mailu" - ( cd "$MAILU_DIR" && docker compose restart front smtp imap >/dev/null 2>&1 || true ) +# The file can be current while the running container still serves an older one +# (a reload that never happened, or failed). Trust the wire, not the filesystem. +on_disk="$(cert_not_after "$CERT_DIR/cert.pem")" +on_wire="$(served_not_after)" +stale_on_wire=0 +if [ -n "$on_wire" ] && [ -n "$on_disk" ] && [ "$on_wire" != "$on_disk" ]; then + echo "Mailu is serving a cert that expires '$on_wire' but /certs holds one that expires '$on_disk' — forcing reload" >&2 + stale_on_wire=1 +fi + +if [ "$changed" = 1 ] || [ "$stale_on_wire" = 1 ]; then + echo "updating Mailu TLS cert for $MAIL_HOST (expires $on_disk); reloading Mailu" + # No `|| true`: a reload that fails is the failure mode this whole script + # exists to prevent, so it must surface in `systemctl status` / the journal. + ( cd "$MAILU_DIR" && docker compose restart front ) else - echo "Mailu TLS cert for $MAIL_HOST already current" + echo "Mailu TLS cert for $MAIL_HOST already current (expires $on_disk)" fi diff --git a/docs/credentials.md b/docs/credentials.md index bd641e1..e90ce5e 100644 --- a/docs/credentials.md +++ b/docs/credentials.md @@ -130,6 +130,7 @@ on any failure. | `AGENTBBS_SET_IRC_SUDO` | `1` | chat — invoke the helper via `sudo` (set `0` if the BBS already runs as root, e.g. in tests) | | `AGENTBBS_SMTP_HOST` / `_FROM` | unset | **sending** all of the above emails (required to actually send) | | `AGENTBBS_SMTP_PORT` / `_USER` / `_PASS` | `587` / unset / unset | SMTP submission (STARTTLS) | +| `AGENTBBS_SMTP_SERVERNAME` | unset (= `_HOST`) | name STARTTLS certs are verified against when the relay is dialled on loopback; verification is skipped entirely for a loopback relay | ## Two SMTP paths (and why one is `:25`) diff --git a/docs/mail.md b/docs/mail.md index b4bac4b..0e9ef95 100644 --- a/docs/mail.md +++ b/docs/mail.md @@ -147,9 +147,37 @@ them at the local Mailu relay so codes actually send: AGENTBBS_SMTP_HOST=127.0.0.1 AGENTBBS_SMTP_PORT=25 AGENTBBS_SMTP_FROM=bbs@bbs.profullstack.com +AGENTBBS_SMTP_SERVERNAME=mail.profullstack.com # set by setup.sh # user/pass omitted: the co-located relay accepts local submission unauthenticated ``` +`AGENTBBS_SMTP_SERVERNAME` is the name STARTTLS certificates are verified +against when it differs from the dialled host — the relay answers on +`127.0.0.1` but presents a cert for the mail host. It mirrors +`AGENTBBS_MAIL_SMTP_SERVERNAME` on the mailbox gateway and `setup.sh` sets it +whenever the Mailu stack is enabled. + +On a **loopback** relay the sender skips certificate verification outright. The +connection never leaves the host, so there is nothing to intercept — and tying +`join@` registration to an on-box cert being both name-matched and unexpired is +precisely what broke signups for nine days in September 2026 (see below). + +### When confirmation codes stop sending + +`join@` reporting *"couldn't email the code"* means `internal/mail` could not +hand the message to the relay. The error is in the journal +(`journalctl -u agentbbs -g "send code"`), and it now names the address and the +failing stage. The usual cause is the mail host's TLS cert: Caddy owns ACME for +`mail.$DOMAIN` and `deploy/mailu/refresh-certs.sh` copies it into Mailu, but +Mailu keeps serving whatever it loaded at container start. Check what is +actually on the wire rather than what is on disk: + +```bash +printf 'QUIT\r\n' | openssl s_client -quiet -starttls smtp \ + -connect 127.0.0.1:25 -servername mail.$DOMAIN 2>&1 | grep -i notAfter +sudo /usr/local/bin/agentbbs-mailu-certs # copies + reloads; loud on failure +``` + ## Provisioning member mailboxes Provisioning is automatic at `join@` verification. To create or backfill by hand: diff --git a/internal/mail/mail.go b/internal/mail/mail.go index 228498f..004bd97 100644 --- a/internal/mail/mail.go +++ b/internal/mail/mail.go @@ -5,7 +5,9 @@ package mail import ( + "crypto/tls" "fmt" + "net" "net/smtp" "os" "strings" @@ -18,37 +20,74 @@ type Config struct { User string // auth user; empty = no auth Pass string From string // envelope + From: header + + // ServerName is the name STARTTLS certificates are verified against when it + // differs from Host. A co-located relay is dialled on loopback but presents a + // certificate for its public mail host, never for 127.0.0.1. Mirrors + // AGENTBBS_MAIL_SMTP_SERVERNAME on the mailbox gateway. + ServerName string } -// ConfigFromEnv reads AGENTBBS_SMTP_{HOST,PORT,USER,PASS,FROM}. +// ConfigFromEnv reads AGENTBBS_SMTP_{HOST,PORT,USER,PASS,FROM,SERVERNAME}. func ConfigFromEnv() Config { return Config{ - Host: os.Getenv("AGENTBBS_SMTP_HOST"), - Port: os.Getenv("AGENTBBS_SMTP_PORT"), - User: os.Getenv("AGENTBBS_SMTP_USER"), - Pass: os.Getenv("AGENTBBS_SMTP_PASS"), - From: os.Getenv("AGENTBBS_SMTP_FROM"), + Host: os.Getenv("AGENTBBS_SMTP_HOST"), + Port: os.Getenv("AGENTBBS_SMTP_PORT"), + User: os.Getenv("AGENTBBS_SMTP_USER"), + Pass: os.Getenv("AGENTBBS_SMTP_PASS"), + From: os.Getenv("AGENTBBS_SMTP_FROM"), + ServerName: os.Getenv("AGENTBBS_SMTP_SERVERNAME"), } } // Configured reports whether email can actually be sent. func (c Config) Configured() bool { return c.Host != "" && c.From != "" } -// Send delivers a plain-text message. net/smtp negotiates STARTTLS when the -// server advertises it (the common case on :587). Implicit-TLS :465 is not -// supported — use a STARTTLS port. +// port is the submission port, defaulting to STARTTLS 587. +func (c Config) port() string { + if c.Port == "" { + return "587" + } + return c.Port +} + +// tlsServerName is the identity STARTTLS certificates are checked against. +// AGENTBBS_SMTP_SERVERNAME wins; otherwise the dialled host is used, which is +// only meaningful when that host is a real name. +func (c Config) tlsServerName() string { + if c.ServerName != "" { + return c.ServerName + } + return c.Host +} + +// isLoopback reports whether the relay lives on this host. A loopback +// connection never leaves the box, so there is nothing for certificate +// verification to defend against — the same reasoning docs/mail.md already +// applies to the plaintext Dovecot hand-off. +func isLoopback(host string) bool { + if strings.EqualFold(host, "localhost") { + return true + } + ip := net.ParseIP(strings.Trim(host, "[]")) + return ip != nil && ip.IsLoopback() +} + +// Send delivers a plain-text message. STARTTLS is negotiated whenever the +// server advertises it (the common case on :587 and on a co-located :25). +// Implicit-TLS :465 is not supported — use a STARTTLS port. +// +// Certificates are verified against tlsServerName() EXCEPT on a loopback relay, +// where verification is skipped deliberately. An on-box MTA serves a cert for +// its public mail host and renews it on its own schedule; making join@ signups +// depend on that cert being both name-matched and unexpired is how registration +// silently died for nine days when the Mailu cert lapsed. Loopback traffic is +// not interceptable, so the check bought nothing and cost everything. func (c Config) Send(to, subject, body string) error { if !c.Configured() { return fmt.Errorf("smtp not configured") } - port := c.Port - if port == "" { - port = "587" - } - var auth smtp.Auth - if c.User != "" { - auth = smtp.PlainAuth("", c.User, c.Pass, c.Host) - } + addr := net.JoinHostPort(c.Host, c.port()) msg := "From: " + c.From + "\r\n" + "To: " + to + "\r\n" + "Subject: " + subject + "\r\n" + @@ -56,5 +95,43 @@ func (c Config) Send(to, subject, body string) error { "Content-Type: text/plain; charset=utf-8\r\n" + "\r\n" + strings.ReplaceAll(body, "\n", "\r\n") + "\r\n" - return smtp.SendMail(c.Host+":"+port, auth, c.From, []string{to}, []byte(msg)) + + cl, err := smtp.Dial(addr) + if err != nil { + return fmt.Errorf("smtp dial %s: %w", addr, err) + } + defer func() { _ = cl.Close() }() + + if ok, _ := cl.Extension("STARTTLS"); ok { + name := c.tlsServerName() + conf := &tls.Config{ServerName: name} + if isLoopback(c.Host) { + conf = &tls.Config{ServerName: name, InsecureSkipVerify: true} // #nosec G402 -- loopback relay, see doc comment + } + if err := cl.StartTLS(conf); err != nil { + return fmt.Errorf("smtp starttls %s (servername %q): %w", addr, name, err) + } + } + if c.User != "" { + if err := cl.Auth(smtp.PlainAuth("", c.User, c.Pass, c.tlsServerName())); err != nil { + return fmt.Errorf("smtp auth %s: %w", addr, err) + } + } + if err := cl.Mail(c.From); err != nil { + return fmt.Errorf("smtp mail from %s: %w", c.From, err) + } + if err := cl.Rcpt(to); err != nil { + return fmt.Errorf("smtp rcpt to %s: %w", to, err) + } + w, err := cl.Data() + if err != nil { + return fmt.Errorf("smtp data: %w", err) + } + if _, err := w.Write([]byte(msg)); err != nil { + return fmt.Errorf("smtp write: %w", err) + } + if err := w.Close(); err != nil { + return fmt.Errorf("smtp close: %w", err) + } + return cl.Quit() } diff --git a/internal/mail/mail_test.go b/internal/mail/mail_test.go new file mode 100644 index 0000000..bfe09dc --- /dev/null +++ b/internal/mail/mail_test.go @@ -0,0 +1,329 @@ +package mail + +import ( + "bufio" + "crypto/rand" + "crypto/rsa" + "crypto/tls" + "crypto/x509" + "crypto/x509/pkix" + "math/big" + "net" + "strings" + "testing" + "time" +) + +func TestConfigured(t *testing.T) { + for _, tc := range []struct { + name string + c Config + want bool + }{ + {"host+from", Config{Host: "mail.example.com", From: "bbs@example.com"}, true}, + {"no host", Config{From: "bbs@example.com"}, false}, + {"no from", Config{Host: "mail.example.com"}, false}, + {"empty", Config{}, false}, + } { + if got := tc.c.Configured(); got != tc.want { + t.Errorf("%s: Configured() = %v, want %v", tc.name, got, tc.want) + } + } +} + +func TestPortDefaultsToSubmission(t *testing.T) { + if got := (Config{}).port(); got != "587" { + t.Errorf("port() = %q, want 587", got) + } + if got := (Config{Port: "25"}).port(); got != "25" { + t.Errorf("port() = %q, want 25", got) + } +} + +// A loopback relay presents a certificate for its public mail host, so the +// name STARTTLS is verified against has to be overridable. +func TestTLSServerNameOverride(t *testing.T) { + if got := (Config{Host: "127.0.0.1"}).tlsServerName(); got != "127.0.0.1" { + t.Errorf("tlsServerName() = %q, want the host", got) + } + c := Config{Host: "127.0.0.1", ServerName: "mail.example.com"} + if got := c.tlsServerName(); got != "mail.example.com" { + t.Errorf("tlsServerName() = %q, want the override", got) + } +} + +func TestIsLoopback(t *testing.T) { + for _, tc := range []struct { + host string + want bool + }{ + {"127.0.0.1", true}, + {"127.1.2.3", true}, + {"::1", true}, + {"[::1]", true}, + {"localhost", true}, + {"LocalHost", true}, + {"mail.example.com", false}, + {"10.0.0.5", false}, + {"", false}, + } { + if got := isLoopback(tc.host); got != tc.want { + t.Errorf("isLoopback(%q) = %v, want %v", tc.host, got, tc.want) + } + } +} + +func TestSendUnconfigured(t *testing.T) { + if err := (Config{}).Send("a@example.com", "s", "b"); err == nil { + t.Fatal("Send() on an unconfigured relay should fail") + } +} + +// fakeSMTP is a minimal ESMTP server that does not advertise STARTTLS. It +// records the dialogue so a test can assert on the envelope and body. +func fakeSMTP(t *testing.T) (addr string, got func() string) { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + t.Cleanup(func() { _ = ln.Close() }) + + done := make(chan string, 1) + go func() { + conn, err := ln.Accept() + if err != nil { + done <- "" + return + } + defer func() { _ = conn.Close() }() + var log strings.Builder + w := bufio.NewWriter(conn) + r := bufio.NewReader(conn) + say := func(s string) { + _, _ = w.WriteString(s + "\r\n") + _ = w.Flush() + } + say("220 fake ESMTP ready") + inData := false + for { + line, err := r.ReadString('\n') + if err != nil { + break + } + log.WriteString(line) + line = strings.TrimRight(line, "\r\n") + switch { + case inData: + if line == "." { + inData = false + say("250 2.0.0 queued") + } + case strings.HasPrefix(line, "EHLO"), strings.HasPrefix(line, "HELO"): + say("250-fake") + say("250 8BITMIME") + case strings.HasPrefix(line, "MAIL FROM"), strings.HasPrefix(line, "RCPT TO"): + say("250 2.0.0 ok") + case line == "DATA": + inData = true + say("354 go ahead") + case line == "QUIT": + say("221 2.0.0 bye") + done <- log.String() + return + default: + say("250 2.0.0 ok") + } + } + done <- log.String() + }() + return ln.Addr().String(), func() string { return <-done } +} + +func TestSendDeliversMessage(t *testing.T) { + addr, got := fakeSMTP(t) + host, port, err := net.SplitHostPort(addr) + if err != nil { + t.Fatalf("split: %v", err) + } + c := Config{Host: host, Port: port, From: "bbs@example.com"} + if err := c.Send("member@example.net", "Your AgentBBS confirmation code", "code: 123456\nthanks"); err != nil { + t.Fatalf("Send() = %v, want nil", err) + } + dialogue := got() + for _, want := range []string{ + "MAIL FROM:", + "RCPT TO:", + "Subject: Your AgentBBS confirmation code", + "code: 123456", + } { + if !strings.Contains(dialogue, want) { + t.Errorf("dialogue missing %q:\n%s", want, dialogue) + } + } + // The body must be CRLF-terminated on the wire, not bare LF. + if strings.Contains(dialogue, "code: 123456\nthanks") { + t.Error("body lines were not CRLF-normalised") + } +} + +// A relay that cannot be reached must surface a wrapped error naming the +// address, so the operator can tell "nothing is listening" from "TLS refused". +func TestSendUnreachableRelayNamesAddress(t *testing.T) { + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + addr := ln.Addr().String() + _ = ln.Close() // nothing is listening there now + + host, port, _ := net.SplitHostPort(addr) + err = Config{Host: host, Port: port, From: "bbs@example.com"}.Send("a@example.com", "s", "b") + if err == nil { + t.Fatal("Send() to a dead relay should fail") + } + if !strings.Contains(err.Error(), addr) { + t.Errorf("error %q should name the relay address %q", err, addr) + } +} + +// selfSignedFor mints a certificate for name that is already expired — the +// exact condition that took registration down: an on-box relay whose cert +// lapsed. +func selfSignedFor(t *testing.T, name string) tls.Certificate { + t.Helper() + key, err := rsa.GenerateKey(rand.Reader, 2048) + if err != nil { + t.Fatalf("key: %v", err) + } + tmpl := &x509.Certificate{ + SerialNumber: big.NewInt(1), + Subject: pkix.Name{CommonName: name}, + DNSNames: []string{name}, + NotBefore: time.Now().Add(-90 * 24 * time.Hour), + NotAfter: time.Now().Add(-24 * time.Hour), // expired yesterday + } + der, err := x509.CreateCertificate(rand.Reader, tmpl, tmpl, &key.PublicKey, key) + if err != nil { + t.Fatalf("cert: %v", err) + } + return tls.Certificate{Certificate: [][]byte{der}, PrivateKey: key} +} + +// starttlsSMTP is a fake relay that advertises STARTTLS and serves an expired +// certificate for a name that is not the address being dialled. +func starttlsSMTP(t *testing.T, certName string) (host, port string, delivered <-chan bool) { + t.Helper() + ln, err := net.Listen("tcp", "127.0.0.1:0") + if err != nil { + t.Fatalf("listen: %v", err) + } + t.Cleanup(func() { _ = ln.Close() }) + cert := selfSignedFor(t, certName) + + done := make(chan bool, 1) + go func() { + conn, err := ln.Accept() + if err != nil { + done <- false + return + } + defer func() { _ = conn.Close() }() + + serve := func(c net.Conn, tlsUp bool) bool { + r := bufio.NewReader(c) + w := bufio.NewWriter(c) + say := func(s string) { + _, _ = w.WriteString(s + "\r\n") + _ = w.Flush() + } + if !tlsUp { + say("220 fake ESMTP ready") + } + inData := false + for { + line, err := r.ReadString('\n') + if err != nil { + return false + } + line = strings.TrimRight(line, "\r\n") + switch { + case inData: + if line == "." { + inData = false + say("250 2.0.0 queued") + } + case strings.HasPrefix(line, "EHLO"), strings.HasPrefix(line, "HELO"): + say("250-fake") + if !tlsUp { + say("250 STARTTLS") + } else { + say("250 8BITMIME") + } + case line == "STARTTLS": + return true // caller upgrades and re-serves + case strings.HasPrefix(line, "MAIL FROM"), strings.HasPrefix(line, "RCPT TO"): + say("250 2.0.0 ok") + case line == "DATA": + inData = true + say("354 go ahead") + case line == "QUIT": + say("221 2.0.0 bye") + return false + default: + say("250 2.0.0 ok") + } + } + } + + if !serve(conn, false) { + done <- false + return + } + // Acknowledge STARTTLS, then hand the connection to TLS. + _, _ = conn.Write([]byte("220 2.0.0 ready to start TLS\r\n")) + tc := tls.Server(conn, &tls.Config{Certificates: []tls.Certificate{cert}}) + if err := tc.Handshake(); err != nil { + done <- false + return + } + serve(tc, true) + done <- true + }() + + h, p, err := net.SplitHostPort(ln.Addr().String()) + if err != nil { + t.Fatalf("split: %v", err) + } + return h, p, done +} + +// The regression this guards: an expired/mismatched certificate on a co-located +// relay must NOT be able to block join@ registration. Loopback traffic cannot be +// intercepted, so the sender proceeds instead of failing the signup. +func TestSendLoopbackRelayToleratesBadCert(t *testing.T) { + host, port, delivered := starttlsSMTP(t, "mail.example.com") + c := Config{Host: host, Port: port, From: "bbs@example.com", ServerName: "mail.example.com"} + if err := c.Send("member@example.net", "Your AgentBBS confirmation code", "code: 123456"); err != nil { + t.Fatalf("Send() over a loopback relay with an expired cert = %v, want nil", err) + } + if !<-delivered { + t.Error("relay did not complete the TLS session") + } +} + +// The same expired cert on a relay that is NOT loopback must still be rejected: +// skipping verification is a loopback-only concession, not a blanket opt-out. +func TestSendRemoteRelayRejectsBadCert(t *testing.T) { + _, port, _ := starttlsSMTP(t, "mail.example.com") + // "localhost." (trailing dot) resolves to 127.0.0.1 but is not recognised as + // a loopback literal, so the verified path is exercised against a real dial. + c := Config{Host: "localhost.", Port: port, From: "bbs@example.com"} + err := c.Send("member@example.net", "s", "b") + if err == nil { + t.Fatal("Send() to a non-loopback relay with an expired cert should fail") + } + if !strings.Contains(err.Error(), "starttls") { + t.Errorf("error %q should identify the STARTTLS stage", err) + } +} diff --git a/setup.sh b/setup.sh index 387fbb3..fbedf5a 100755 --- a/setup.sh +++ b/setup.sh @@ -1243,6 +1243,11 @@ if [ "$MAIL_STACK" = "1" ]; then # (its cert is for ${MAIL_DOMAIN}, never 127.0.0.1) — no /etc/hosts hack needed. upsert_env AGENTBBS_MAIL_SMTP_SERVERNAME "${MAIL_DOMAIN}" + # The transactional sender (join@ confirmation codes, notify-creds) verifies + # the relay's STARTTLS cert against the mail host. When it dials the co-located + # relay on loopback there is no name to verify against, so hand it one. + upsert_env AGENTBBS_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 cat > /etc/systemd/system/agentbbs-mailu-certs.service </dev/null 2>&1 || true + # `enable --now` starts the TIMER, not the service, so a redeploy would + # otherwise leave a stale cert in place until the next tick (and do nothing at + # all if the timer was never scheduled). Run the refresher now, like the news + # and IRC sections do -- this is the step that repairs an expired mail cert. + DOMAIN="${DOMAIN#*.}" MAIL_HOST="${MAIL_DOMAIN}" MAILU_DIR="${MAILU_DIR}" \ + /usr/local/bin/agentbbs-mailu-certs || warn "mailu: cert refresh failed — mail TLS may be stale (see: journalctl -u agentbbs-mailu-certs)" # Open the mail ports; bring Mailu up only once the operator has created # mailu.env (it carries SECRET_KEY + admin password — never auto-generated).