From aaa94120be0f2c64b03859b5ced697e009ca2b4e Mon Sep 17 00:00:00 2001 From: Mix Date: Thu, 15 Oct 2026 17:24:29 +0300 Subject: [PATCH] port 25 ends DATA only at CRLF.CRLF: smtpd_forbid_bare_newline=normalize (port25-bare-newline) Postfix 3.7.11 leaves its 2023 SMTP-smuggling defence off (default "no" before 3.9, src/smtpd/smtpd.c:807), and the inbound relay's smtp/inet listener did not turn it on: a "." line ended by a bare LF ended DATA and what followed was read as the next transaction. The listener now sets normalize (bare_lf_mask_table, smtpd.c:1656; the DATA loop drops such a "." line, 3649-3652), the setting Postfix recommends and 3.9's default. The submission ports keep the default (own-mta-builds, section 5). Tests: e2e inbound_bare_newline_is_one_message sends the smuggling sequence over raw SMTP (net/smtp's DotWriter would rewrite the bare LF) and finds one message at the upstream with the smuggled part inside; port25 config check asserts the parameter on smtp/inet. On the server: make e2e exit 0 (511 s), go vet + go test 27 packages ok. Docs: CHANGELOG [Unreleased] Security, security.md, architecture.md, guide.md; roadmap: port25-bare-newline, base-trixie and panel-dkim-signing agreed by the owner in this order. Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 15 +++++++++ build/postfix-config.sh | 13 ++++++++ docs/architecture.md | 5 +++ docs/guide.md | 6 +++- docs/roadmap.md | 10 +++--- docs/security.md | 9 ++++- test/e2e/main_test.go | 3 ++ test/e2e/port25_client.go | 51 ++++++++++++++++++++++++++++ test/e2e/port25_config_check.go | 3 ++ test/e2e/port25_features_test.go | 57 ++++++++++++++++++++++++++++++++ 10 files changed, 165 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 53019af..ce40aaf 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,21 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +### Security + +- **Port 25 could be made to read a second transaction out of a message's + body.** Postfix 3.7 leaves its 2023 SMTP-smuggling defence off + (`smtpd_forbid_bare_newline`, default `no` before 3.9), and the inbound + relay's listener did not turn it on: a `.` line ended by a bare newline + ended DATA, and what followed was read as the next transaction's commands. + The listener now sets `normalize` - the setting Postfix recommends and 3.9 + made the default: DATA ends only at the standard `.`, a + bare newline elsewhere is read as a line end. Found while reading for + [own-mta-builds](docs/own-mta-builds-2026-10-15.md) (§ 5); `test/e2e` + sends the sequence and finds one message at the upstream with all of it + inside. Only installations with `INBOUND_RELAY_ENABLE=true` or + `DMARC_REPORTS_ENABLE=true` have the listener. + ## [2.6.0] - 2026-10-15 Stage 5 of the 2026-10-12 audit: the image and Postfix. The supervisor diff --git a/build/postfix-config.sh b/build/postfix-config.sh index 7edf087..1626ba9 100644 --- a/build/postfix-config.sh +++ b/build/postfix-config.sh @@ -643,7 +643,20 @@ if [ "${INBOUND_RELAY_ENABLE}" = "true" ] || [ "${DMARC_REPORTS_ENABLE}" = "true # program name. Only smtpd carries it: cleanup and the rest are other # services and log as postfix/.) postconf -M "smtp/inet=smtp inet n - n - - smtpd" + # smtpd_forbid_bare_newline: the 2023 SMTP-smuggling defence, which this + # Postfix leaves off. normalize takes a bare as a line end but ends + # DATA only at the standard .: a "." line that a bare + # precedes or ends is dropped, not taken as the end of the message + # (Postfix 3.7.11, src/smtpd/smtpd.c:807, the default "no" for Postfix + # < 3.9; :1656, bare_lf_mask_table, normalize = BARE_LF_FLAG_WANT_STD_EOD; + # :3649-3652, the DATA loop; RELEASE_NOTES, "Major changes with Postfix + # 3.8.5, 3.7.10": normalize is the recommended setting and 3.9's + # default). Its exclusions default to $mynetworks, loopback only here + # (above), so no client is spared. The submission ports keep the + # default: their smuggled transaction would still be the authenticated + # session's, under its bindings and milters (own-mta-builds, § 5). postconf -P \ + "smtp/inet/smtpd_forbid_bare_newline=normalize" \ "smtp/inet/syslog_name=postfix/inbound" \ "smtp/inet/smtpd_sasl_auth_enable=no" \ "smtp/inet/smtpd_tls_auth_only=no" \ diff --git a/docs/architecture.md b/docs/architecture.md index d11e137..4f25c63 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -189,6 +189,11 @@ smtp/inet` removes Debian's stock smtpd). Outbound delivery still uses the panel's sender check (§ Panel roles). - **Level-1 rate limit** — `smtpd_client_message_rate_limit` / `anvil_rate_time_unit` from `RATE_LIMIT_*` env vars; independent of milter. +- **Port 25 ends DATA only at `.`** — + `smtpd_forbid_bare_newline=normalize` on the `smtp/inet` listener (Postfix + 3.7.11, `src/smtpd/smtpd.c`, the DATA loop at 3649: a `.` line next to a + bare `` is dropped, not taken as the end); a bare `` elsewhere is + read as ``. The submission ports keep Postfix's default. - **Chroot disabled** for all services (DNS/TLS inside container). - **TLS certs** — read-only mount at `TLS_CERT_FILE` / `TLS_KEY_FILE`, copied at start into `/etc/postfix/tls-internal`, owned by root and `0600` as diff --git a/docs/guide.md b/docs/guide.md index 5ab0ec4..787b2ae 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -1010,7 +1010,11 @@ SelfPost never stores the message locally. container. Until then there is no inbound-relay `smtp inet` listener, no *Inbound* group in the menu, and `/inbound/…` is 404. (Port 25 can still listen when only `DMARC_REPORTS_ENABLE=true` — see [DMARC -reports](#dmarc-reports).) Outbound 465/587 is unchanged. +reports](#dmarc-reports).) Outbound 465/587 is unchanged. Port 25 ends a +message only at the standard `.` (the SMTP-smuggling +defence, since `2.6.1`); a sending server that ends DATA with a bare +newline waits for a reply that does not come, and there is no allow-list +for such a sender. **Panel** (`/inbound/domains`): the global role adds and deletes domains. On a domain's page, set the upstream host/port and TLS to that hop (opportunistic / diff --git a/docs/roadmap.md b/docs/roadmap.md index 67006d2..f9accfb 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -42,9 +42,9 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | settings-save-row | Server › Settings: the Save button no longer stands alone under a long right column | agreed (2026-10-15) | — | — | | dns-record-collapse | A DNS record that is right is shown folded: its name and its tag, the values on request | agreed (2026-10-15) | — | — | | own-mta-builds | Analysis: build Postfix and OpenDKIM ourselves, patched, instead of Debian's packages | analysed 2026-10-15: no own builds; three rows proposed instead | — | [own-mta-builds-2026-10-15.md](own-mta-builds-2026-10-15.md) | -| port25-bare-newline | The inbound relay's port 25 refuses bare newlines in DATA (`smtpd_forbid_bare_newline=normalize`) | candidate (2026-10-15, from own-mta-builds; recommended now, PATCH) | — | — | -| base-trixie | The image's base from Debian bookworm (LTS since 2026-07-12) to trixie: Postfix 3.10 with the 2026 security batches | candidate (2026-10-15, from own-mta-builds; recommended next, MINOR) | — | — | -| panel-dkim-signing | DKIM signing in the panel's own milter; OpenDKIM, the syslog relay and the parser port removed | candidate (2026-10-15, from own-mta-builds; recommended after base-trixie, MINOR) | — | — | +| port25-bare-newline | The inbound relay's port 25 refuses bare newlines in DATA (`smtpd_forbid_bare_newline=normalize`) | agreed (2026-10-15; first of the three, PATCH) | — | — | +| base-trixie | The image's base from Debian bookworm (LTS since 2026-07-12) to trixie: Postfix 3.10 with the 2026 security batches | agreed (2026-10-15; after port25-bare-newline, MINOR) | — | — | +| panel-dkim-signing | DKIM signing in the panel's own milter; OpenDKIM, the syslog relay and the parser port removed | agreed (2026-10-15; after base-trixie, MINOR) | — | — | | phone-desktop-mode | A phone browser in "desktop site" mode gets the phone navigation | agreed (2026-10-13: after stages 1 and 2 of audit-2026-10-12) | — | — | | from-header-binding | Hold a message's `From:` header against the application that sends it | shipped `2.4.0` | 9/9 | [plans/from-header-binding.md](plans/from-header-binding.md) | | panel-redesign | Panel redesign (accepted mockups, design contract) | shipped `2.0.0` | 27/27 | [plans/panel-redesign.md](plans/panel-redesign.md) | @@ -784,8 +784,8 @@ is answered by three rows instead, in this order: Postfix 3.7.11 lacks the three 2026 security batches, which carry no CVE ids and so will not come), and [panel-dkim-signing](#panel-dkim-signing) (signing in the panel's own -milter; OpenDKIM, the syslog relay and the parser port go). Each needs -the owner's OK. +milter; OpenDKIM, the syslog relay and the parser port go). The owner +agreed to the three and their order on 2026-10-15. ## port25-bare-newline diff --git a/docs/security.md b/docs/security.md index aafbefc..a5d3c86 100644 --- a/docs/security.md +++ b/docs/security.md @@ -262,7 +262,14 @@ operator can act on it. is never accepted with nowhere to send it. Prefer recipient mode `list` to refuse unknown addresses at RCPT (no backscatter). OpenDKIM is not attached on inbound. An optional antispam milter is inbound-only; default action is - fail-open (`accept`). + fail-open (`accept`). Since `2.6.1` the listener ends DATA only at the + standard `.` (`smtpd_forbid_bare_newline=normalize`, the + 2023 SMTP-smuggling defence, which Postfix 3.7 leaves off by default): a + `.` line next to a bare newline is body, and what a smuggler would put + behind it is delivered as part of the one message, not read as a second + transaction. The submission ports keep the default - there a smuggled + transaction is still the authenticated session's, under its sender + bindings and milters ([own-mta-builds](own-mta-builds-2026-10-15.md) § 5). - **The panel's milters judge every message of a connection.** Postfix introduces a client to a milter once per connection, and the milter library replaces the milter's session after any reply that is not "continue". Until diff --git a/test/e2e/main_test.go b/test/e2e/main_test.go index 11de5d1..845f343 100644 --- a/test/e2e/main_test.go +++ b/test/e2e/main_test.go @@ -386,6 +386,9 @@ func TestE2E(t *testing.T) { run("inbound_relay_delivers_and_refuses", func(t *testing.T) { testInboundRelay(t, sc) }) + run("inbound_bare_newline_is_one_message", func(t *testing.T) { + testInboundBareNewline(t, sc) + }) } // trimZoneSuffix strips the "e2e.test" suffix a fully-qualified DKIM record diff --git a/test/e2e/port25_client.go b/test/e2e/port25_client.go index edeb46b..b6afead 100644 --- a/test/e2e/port25_client.go +++ b/test/e2e/port25_client.go @@ -4,6 +4,7 @@ import ( "fmt" "net" "net/smtp" + "net/textproto" "time" ) @@ -80,3 +81,53 @@ func waitForPort25Ready(timeout time.Duration) error { return true, nil }) } + +// deliverPort25Raw is deliverPort25 with the DATA bytes written as they +// stand. net/smtp's data writer would turn a bare LF into CRLF and stuff a +// lone "." (net/textproto, DotWriter), which is the very input this is for. +// It returns an error for a reply other than the one each step expects. +func deliverPort25Raw(from, to string, data []byte) error { + conn, err := net.DialTimeout("tcp", port25Addr, 10*time.Second) + if err != nil { + return fmt.Errorf("dial %s: %w", port25Addr, err) + } + defer conn.Close() + _ = conn.SetDeadline(time.Now().Add(60 * time.Second)) + tp := textproto.NewConn(conn) + expect := func(code int, after string) error { + if _, _, err := tp.ReadResponse(code); err != nil { + return fmt.Errorf("after %s: %w", after, err) + } + return nil + } + if err := expect(220, "connect"); err != nil { + return err + } + for _, step := range []struct { + cmd string + code int + }{ + {"HELO mx.stranger.e2e.test", 250}, + {"MAIL FROM:<" + from + ">", 250}, + {"RCPT TO:<" + to + ">", 250}, + {"DATA", 354}, + } { + if err := tp.PrintfLine("%s", step.cmd); err != nil { + return fmt.Errorf("%s: %w", step.cmd, err) + } + if err := expect(step.code, step.cmd); err != nil { + return err + } + } + if _, err := tp.W.Write(data); err != nil { + return fmt.Errorf("DATA content: %w", err) + } + if err := tp.W.Flush(); err != nil { + return fmt.Errorf("DATA content: %w", err) + } + if err := expect(250, "end of data"); err != nil { + return err + } + _ = tp.PrintfLine("QUIT") + return nil +} diff --git a/test/e2e/port25_config_check.go b/test/e2e/port25_config_check.go index 822abb6..cf457bd 100644 --- a/test/e2e/port25_config_check.go +++ b/test/e2e/port25_config_check.go @@ -51,6 +51,9 @@ var port25Cases = []port25Case{ // The log tailer tells this listener's refusals from the // submission ports' by the name they are logged under. {param: "syslog_name", contains: "postfix/inbound", wantExact: true}, + // The SMTP-smuggling defence, off by default on this Postfix + // (own-mta-builds, § 5); testInboundBareNewline drives it. + {param: "smtpd_forbid_bare_newline", contains: "normalize", wantExact: true}, // No antispam milter configured: the inbound journal milter // alone. The submission chain (strict OpenDKIM + journal-milter) // must not reach inbound mail. diff --git a/test/e2e/port25_features_test.go b/test/e2e/port25_features_test.go index 0d83576..c6fb1fa 100644 --- a/test/e2e/port25_features_test.go +++ b/test/e2e/port25_features_test.go @@ -6,6 +6,8 @@ import ( "encoding/base64" "fmt" "net/url" + "os" + "path/filepath" "regexp" "strings" "testing" @@ -456,3 +458,58 @@ func testInboundRelay(t *testing.T, sc *scenario) { t.Fatalf("deny_domain.map after removing %s:\n%s", spammer, got) } } + +// testInboundBareNewline: port 25 ends DATA only at the standard +// .. A "." line that a bare precedes or ends - the SMTP +// smuggling sequence, with a second transaction's commands behind it - is +// body, and the one message delivered carries all of it; nothing is read as +// a second transaction (smtpd_forbid_bare_newline=normalize on smtp/inet, +// build/postfix-config.sh; Postfix 3.7.11, src/smtpd/smtpd.c:3649-3652). +// Runs after testInboundRelay, whose domain, upstream and recipient it uses. +func testInboundBareNewline(t *testing.T, sc *scenario) { + token := uniqueToken("inbound-bare") + smuggled := uniqueToken("smuggled") + data := "From: someone@stranger.e2e.test\r\nTo: " + inboundRecipient + + "\r\nSubject: e2e " + token + "\r\nDate: " + time.Now().UTC().Format(time.RFC1123Z) + + "\r\n\r\nthe first part\n.\n" + + "MAIL FROM:\r\nRCPT TO:<" + inboundRecipient + ">\r\nDATA\r\n" + + "Subject: e2e " + smuggled + "\r\n\r\nthe part a smuggler would send\r\n.\r\n" + if err := deliverPort25Raw("someone@stranger.e2e.test", inboundRecipient, []byte(data)); err != nil { + t.Fatalf("port 25 did not take the message with a bare-newline \".\" inside: %v\n%s", err, mailLogTail(h, "postfix/")) + } + raw, err := findSinkMessage(h.stageDir, token, 30*time.Second) + if err != nil { + t.Fatalf("the message never reached the upstream: %v\n%s", err, mailLogTail(h, "postfix/")) + } + if !bytes.Contains(raw, []byte(smuggled)) { + t.Fatalf("the message was cut at the bare-newline \".\": what followed it is not in the delivered message\n%s", raw) + } + // A second transaction, had Postfix read one, would be delivered right + // behind the first: give it the time and count the files that carry the + // smuggled token. + time.Sleep(3 * time.Second) + if n := sinkMessagesWith(t, h.stageDir, smuggled); n != 1 { + t.Fatalf("%d messages at the upstream carry the smuggled part, want 1\n%s", n, mailLogTail(h, "postfix/")) + } +} + +// sinkMessagesWith counts the sink's dumped messages that contain token. +func sinkMessagesWith(t *testing.T, stageDir, token string) int { + t.Helper() + dir := filepath.Join(stageDir, "mail-stage") + entries, err := os.ReadDir(dir) + if err != nil { + t.Fatalf("read %s: %v", dir, err) + } + n := 0 + for _, e := range entries { + if e.IsDir() { + continue + } + b, err := os.ReadFile(filepath.Join(dir, e.Name())) + if err == nil && bytes.Contains(b, []byte(token)) { + n++ + } + } + return n +}