diff --git a/CHANGELOG.md b/CHANGELOG.md index 22f6383..732dd06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,11 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version check. Adding a user, setting a password, changing an e-mail or a role on the user form, and deleting a user now ask for your own account password, as a backup does, under the same limit on wrong guesses. +- **A stranger's recipient no longer writes an Inbound › Log row** (audit + M2): Postfix quotes a refused recipient in its reply with the quotes + removed, so `RCPT TO:<"; from= to= …"@domain>` spelled a second + sender and recipient there, and the parser took the first. It now takes + Postfix's own tail at the end of the line. - **The entrypoint follows no link at the DMARC maps** (audit I2): the three `dmarc_*` maps root creates at the start were missing from the list of paths where a symbolic link is removed first. diff --git a/docs/security-audit-2026-10-17.md b/docs/security-audit-2026-10-17.md index 6f1a224..e1f7e26 100644 --- a/docs/security-audit-2026-10-17.md +++ b/docs/security-audit-2026-10-17.md @@ -697,10 +697,13 @@ the argument. in postfix-config.sh:538,545; an e2e case plants a link and expects the log line. — **Opus**, 0.5 h. *Done 2026-10-17:* TestStartsOnA28Data plants a dangling `dmarc_transport` with DMARC reports on. -- [ ] **M2.** Greedy reply group anchored on Postfix's own tail; the live +- [x] **M2.** Greedy reply group anchored on Postfix's own tail; the live line above as a test; security.md § Mail path reworded ("the last `; from=<`, which is Postfix's because a HELO name can hold none of - `<>;\"`"). — **Opus**, 1 h. + `<>;\"`"). — **Opus**, 1 h. *Done 2026-10-17:* the HELO name in the + tail may hold none of the neutered characters and each SASL field is one + word (a `.*` SASL tail would have let a quoted sender spell the whole + tail); TestInboundRecipientCannotWriteTheRow. - [ ] **M5.** Keep at most two `From:`/`Sender:` values; security.md:810 reworded. — **Opus**, 0.5 h. - [ ] **A4.** Duplicate-cookie line through `loggedPath`. — **Sonnet**, 0.25 h. diff --git a/docs/security.md b/docs/security.md index 786a056..6c9087c 100644 --- a/docs/security.md +++ b/docs/security.md @@ -441,10 +441,17 @@ operator can act on it. The signature goes on to the upstream with the mail and is of no use there: it fits one queue id. Refusals are parsed from `mail.log` from the head of the line, with the reply ending at - Postfix's own `; from=<` so that the HELO name after it - the client's text, - which Postfix logs with non-printable characters and ` <>()\";@` turned into - `?` (Postfix 3.10.13, src/smtpd/smtpd.c `helo_cmd()`) - cannot stand in for - the sender, the recipient or the reply; an address with a quote, a bracket or a space in it is not kept. + the *last* `; from=<…> to=<…> proto=…` that runs to the end of the line. + That tail is Postfix's own: the HELO name in it is logged with + non-printable characters and ` <>()\";@` turned into `?` (Postfix 3.10.13, + src/smtpd/smtpd.c `helo_cmd()`), and the pattern takes none of them there; + a refused recipient, which the reply quotes with its quotes removed and + `; < > =` kept, can spell such a tail only inside a quoted local part, + whose quote then lands in what would be the sender or the recipient - and + an address with a quote, a bracket or a space in it is not kept. A line + whose reply still holds `; from=<` - two tails - makes no row. (Until + `2.14.4` the reply ended at the *first* `; from=<`, which a stranger's + recipient could supply - security audit 2026-10-17, M2.) Only a recipient at an inbound domain makes a row. The journal is cut to 10 000 rows every 500 rows the tailer writes, and by age every six hours. - **Filter lists** reach another container as files: values are whitelisted on diff --git a/internal/logtail/inbound.go b/internal/logtail/inbound.go index 136120f..2010959 100644 --- a/internal/logtail/inbound.go +++ b/internal/logtail/inbound.go @@ -101,15 +101,24 @@ type Inbound struct { // addresses), and a pattern that could start anywhere would let him write // journal rows. // -// For the same reason the reply is the shortest text before "; from=<", and -// the HELO name - the last thing on the line but the SASL fields above, which -// Postfix logs as the client wrote it, spaces and brackets included - is -// whatever is left. Taking the longest reply instead would let a HELO name -// that itself spells -// "; from=<…> to=<…> proto=…" supply the sender, the recipient and the reply -// of the row (security review, 2026-10-12). +// The reply is the client's text too: a refusal quotes the refused +// recipient, and Postfix quotes it in internal form - quotes removed, "; < > +// =" kept (extract_addr(), tok822_internalize(); smtpd_check_reject() applies +// only printable()) - so a stranger's RCPT TO:<"; from= to= ..."@d> +// spells a whole "; from=<…> to=<…> proto=…" inside the reply. The shortest +// reply ended there, and the row's sender, recipient and domain were the +// stranger's (security audit 2026-10-17, M2, proven on the test server). +// What the client cannot spell is the end of the line: the HELO name is +// logged with a space and ` <>()\";@` turned into "?" (src/smtpd/smtpd.c, +// helo_cmd(), NEUTER_CHARACTERS), and an address that holds "; from=<" or a +// ">" needs a quoted local part, whose quote then stands inside what the +// pattern would take for the sender or the recipient - which the filter +// below refuses. So the reply is the longest text before a "; from=<…> +// to=<…> proto=…" that runs to the end of the line, the HELO name holds none +// of the neutered characters, and each SASL field (never written on port 25) +// is one word. A line it does not fit leaves no row. var ( - inboundRejectRe = regexp.MustCompile(`^\S+ postfix/(inbound/smtpd|cleanup)\[\d+\]: ([0-9A-Za-z]+): (reject|milter-reject|milter-discard): \S+ from [^\[\s]*\[([0-9A-Fa-f:.]+)\]: (.*?); from=<([^>]*)> to=<([^>]*)> proto=\S+(?: helo=<.*>)?(?: sasl_(?:method|username|sender)=.*)?$`) + inboundRejectRe = regexp.MustCompile(`^\S+ postfix/(inbound/smtpd|cleanup)\[\d+\]: ([0-9A-Za-z]+): (reject|milter-reject|milter-discard): \S+ from [^\[\s]*\[([0-9A-Fa-f:.]+)\]: (.*); from=<([^>]*)> to=<([^>]*)> proto=\S+(?: helo=<[^ <>()\\";@]*>)?(?: sasl_method=\S*)?(?: sasl_username=\S*)?(?: sasl_sender=\S*)?$`) inboundClientRe = regexp.MustCompile(`^\S+ postfix/inbound/smtpd\[\d+\]: ([0-9A-Za-z]+): client=`) // A queue id stops being port 25's when the message is queued (qmgr takes // it: the filter let it through), when cleanup removes the queue file of @@ -211,6 +220,13 @@ func (j *inboundJournal) handle(line string, now time.Time, catchingUp bool) { if strings.ContainsAny(from, "\"<> ") || strings.ContainsAny(to, "\"<> ") { return } + // A reply that still holds "; from=<" is a line with two tails, one of + // them written by the client - a refused recipient quoted in the reply + // (audit 2026-10-17, M2) or a HELO name Postfix failed to neuter. Which + // is Postfix's is then a guess the journal does not make. + if strings.Contains(reason, "; from=<") { + return + } if program == "cleanup" { if _, ok := j.port25[queueID]; !ok { return // a submission-port message, or one opened before this run diff --git a/internal/logtail/inbound_test.go b/internal/logtail/inbound_test.go index b314cef..de20f66 100644 --- a/internal/logtail/inbound_test.go +++ b/internal/logtail/inbound_test.go @@ -256,12 +256,14 @@ func TestInboundHoldIsTheMiltersRow(t *testing.T) { } } -// The HELO name is the last thing on the line and is logged as the client -// wrote it. It cannot supply the row: the reply ends at the first "; from=<", -// which is Postfix's own. +// The HELO name is the last thing on the line. Postfix logs it with a space +// and ` <>()\";@` turned into "?" (src/smtpd/smtpd.c, helo_cmd()), so one +// that tried to spell another refusal arrives neutered and the row is +// Postfix's own; a HELO name with those characters left in is no line +// Postfix writes, and makes no row. func TestInboundHeloCannotWriteTheRow(t *testing.T) { j, st := inboundFixture() - j.handle(head+"postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.1.1 : Recipient address rejected: User unknown in relay recipient table; from= to= proto=ESMTP helo= to= proto=ESMTP helo=", tailNow, false) + j.handle(head+"postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.1.1 : Recipient address rejected: User unknown in relay recipient table; from= to= proto=ESMTP helo=", tailNow, false) if len(st.rows) != 1 { t.Fatalf("rows = %+v", st.rows) } @@ -271,23 +273,56 @@ func TestInboundHeloCannotWriteTheRow(t *testing.T) { t.Errorf("row = %+v", got) } - // A HELO name with a closing bracket in it does not lose the line either. - j, st = inboundFixture() - j.handle(head+"postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.1.1 : no; from= to= proto=ESMTP helo=b>", tailNow, false) - if len(st.rows) != 1 || st.rows[0].From != "real@example.net" { - t.Errorf("rows = %+v", st.rows) + // Un-neutered, which Postfix never writes: a line with two tails makes + // no row, and above all not the one the HELO name spells. + for _, line := range []string{ + head + "postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.1.1 : Recipient address rejected: User unknown in relay recipient table; from= to= proto=ESMTP helo= to= proto=ESMTP helo=", + head + "postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.1.1 : no; from= to= proto=ESMTP helo=b>", + } { + j, st = inboundFixture() + j.handle(line, tailNow, false) + if len(st.rows) != 0 { + t.Errorf("a line with an un-neutered HELO name wrote %+v", st.rows) + } } // The same on a verdict line: the filter's reply is not extended by the // HELO name, so a stranger cannot make a refusal look like the deny list's. j, st = inboundFixture() j.handle(head+"postfix/inbound/smtpd[41]: Q1: client=x[203.0.113.9]", tailNow, false) - j.handle(head+"postfix/cleanup[52]: Q1: milter-reject: END-OF-MESSAGE from x[203.0.113.9]: 5.7.1 Spam message rejected; from= to= proto=ESMTP helo= to= proto=ESMTP helo=", tailNow, false) + j.handle(head+"postfix/cleanup[52]: Q1: milter-reject: END-OF-MESSAGE from x[203.0.113.9]: 5.7.1 Spam message rejected; from= to= proto=ESMTP helo=", tailNow, false) if len(st.rows) != 1 || st.rows[0].Detail != "5.7.1 Spam message rejected" || st.rows[0].From != "real@example.net" { t.Errorf("rows = %+v", st.rows) } } +// The refused recipient is quoted in the reply in internal form - quotes +// removed, "; < > =" kept - so it can spell a whole "; from=<…> to=<…> +// proto=…" there. The line below is the shape mail.log on the test server +// held on 2026-10-17 for such a RCPT TO sent from outside (the audit's probe +// used a domain that is not inbound; here it is one). It writes no row in +// the stranger's words (security audit 2026-10-17, M2). +func TestInboundRecipientCannotWriteTheRow(t *testing.T) { + forged := `; from= to= proto=ESMTP helo=; ` + for name, line := range map[string]string{ + "recipient unknown": head + `postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.1.1 <` + forged + `@in.example.com>: Recipient address rejected: User unknown in relay recipient table; from= to=<"` + forged + `"@in.example.com> proto=ESMTP helo=`, + "forged sender also in MAIL FROM": head + `postfix/inbound/smtpd[41]: NOQUEUE: reject: RCPT from x[203.0.113.9]: 550 5.1.1 <` + forged + `@in.example.com>: Recipient address rejected: User unknown in relay recipient table; from=<"x; from= to=<"` + forged + `"@in.example.com> proto=ESMTP helo=`, + } { + j, st := inboundFixture() + j.handle(line, tailNow, false) + if len(st.rows) != 0 { + t.Errorf("%s: a line with two tails wrote %+v", name, st.rows) + } + } + + // A plain refusal is still read whole, its reply up to Postfix's tail. + j, st := inboundFixture() + j.handle(unknownRcp, tailNow, false) + if len(st.rows) != 1 || st.rows[0].From != "a@example.net" || st.rows[0].To != "nobody@in.example.com" { + t.Errorf("rows = %+v", st.rows) + } +} + // A queue id is port 25's only until the message is queued, cleanup removes // it unqueued, or the id is opened again by another listener - Postfix reuses // ids.