diff --git a/CHANGELOG.md b/CHANGELOG.md index ce09e68..c74a6ad 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,11 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version each, and the journal milter's rows did not count toward the 10 000-row cut between the six-hourly prunes. Repeats are one row, a message's rows are written in one transaction, and both writers count toward the cut. +- **A domain's custom DMARC report address may not be on the server's own + name** (audit W1): it opened a mailbox the whole internet could file + reports into, under any domain, or another domain's hosted one. Only the + hosted address of a domain that chose it is a mailbox now, and a report + must arrive at the address tagged with its domain. - **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 diff --git a/docs/guide.md b/docs/guide.md index ab7cde1..544f2f9 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -1343,7 +1343,8 @@ settings) and nowhere else: server whose reports are parsed and shown under *DMARC reports*. Offered only while `DMARC_REPORTS_ENABLE=true`. - **A specific address** — any mailbox, typed for this domain; it receives the - raw XML. Beside the field, *Use my e-mail* fills it with the e-mail of your + raw XML. Not one on `SELFPOST_HOSTNAME`: to have this server receive the + reports, choose *SelfPost hosted*. Beside the field, *Use my e-mail* fills it with the e-mail of your profile (Account); it only fills the field, and saving is the form's own button. The button is absent when your profile has no e-mail. @@ -1351,8 +1352,8 @@ Changing the choice changes the DMARC record to publish; the domain page shows the new value. There is no panel-wide or per-user default. Only addresses on `SELFPOST_HOSTNAME` that a domain has chosen are accepted on port 25 - the hosted address `dmarc-reports+@` of a domain set to -*SelfPost hosted*, or a typed address that happens to be on this host. There -is no mailbox for the server as a whole. +*SelfPost hosted* - and a report is kept only when it is for the domain the +address is tagged with. There is no mailbox for the server as a whole. **DNS.** Publish the usual `_dmarc` TXT on each sending domain with `rua=mailto:…` pointing at the address the domain shows. Receivers deliver to the diff --git a/docs/security-audit-2026-10-17.md b/docs/security-audit-2026-10-17.md index e26b978..51f22e8 100644 --- a/docs/security-audit-2026-10-17.md +++ b/docs/security-audit-2026-10-17.md @@ -737,11 +737,14 @@ the argument. into the store (`countInboundWrites`, both writers), `InsertInboundSpamMessage`; TestInboundRecipientRepeatedIsOneRow, TestInboundJournalIsCutWhicheverWriterWrites. -- [ ] **W1.** Refuse a custom report address on the hostname; exact hosted +- [x] **W1.** Refuse a custom report address on the hostname; exact hosted address only in `AllowedRecipients`; a tag required for every recipient on the hostname in `checkReportDomain`; the existing test inverted; the accepted-risk text corrected. Check the live registry for a custom address - on the hostname first (none expected). — **Opus**, 1.5 h. + on the hostname first (none expected). — **Opus**, 1.5 h. *Done + 2026-10-17:* the live registry holds two hosted addresses and no custom + one on the hostname; TestCustomReportAddressOnTheHostnameIsRefused, the + untagged cases in TestIngestMessageRejectsTagMismatch. - [ ] **A5/W2.** Delete a domain's reports in `domain.Service.Delete` (the cheap form); security.md § Roles says reports go with the domain. — **Sonnet**, 0.5 h. (The `domain_id` form is better and 2 h; take it if diff --git a/docs/security.md b/docs/security.md index 771a637..3c4f01b 100644 --- a/docs/security.md +++ b/docs/security.md @@ -636,10 +636,16 @@ deferred item from the roadmap. the alignment statistics the panel shows. Inherent to how `rua` works: reporters are arbitrary third parties and the mailbox address is published in DNS. The ingest already constrains the meaningful part — a report cannot be - filed under a domain this instance does not host, and the claimed domain is - cross-checked against the `+tag` of the delivery address (effective since - 2.0.0: before it the recipient never reached the command) — and the data is - advisory statistics only; nothing enforces off it. What a stranger can do is + filed under a domain this instance does not host, and the claimed domain + must be the `+tag` of the delivery address (checked since 2.0.0: before it + the recipient never reached the command; required since `2.14.4`) — and + the data is advisory statistics only; nothing enforces off it. The only + mailboxes are the hosted addresses, each open while its own domain has + chosen it: a typed report address on the server's own name is refused, and + one stored before is no mailbox (until `2.14.4` a domain user could type + `ops@` and so open an untagged mailbox to the internet whose + reports were filed under any domain, or another domain's hosted address - + security audit 2026-10-17, W1). What a stranger can do is limited to a domain that chose a SelfPost-hosted address, through that address: file reports under it, replace one whose reporter and id they know, and fill that domain's 500 — not another domain's. A message that is not a diff --git a/internal/dmarc/ingest.go b/internal/dmarc/ingest.go index e2cc9b4..e23168e 100644 --- a/internal/dmarc/ingest.go +++ b/internal/dmarc/ingest.go @@ -113,13 +113,19 @@ func IngestMessage(st *store.Store, r io.Reader, recipient string, receivedAt ti } // checkReportDomain refuses a report whose policy domain is not one of this -// relay's sending domains, or that contradicts the +tag of the address it was -// delivered to. +// relay's sending domains, or is not the +tag of the address it was delivered +// to. Every mailbox here is a hosted one, tagged with its domain +// (AllowedRecipients), so an address without a tag is no mailbox of a domain +// and its report belongs to none (security audit 2026-10-17, W1). func checkReportDomain(st *store.Store, domain, recipient string) error { if domain == "" { return rejected(fmt.Errorf("no policy domain")) } - if tagged := TaggedDomain(recipient); tagged != "" && tagged != domain { + tagged := TaggedDomain(recipient) + if tagged == "" { + return rejected(fmt.Errorf("the address it was sent to (%s) is tagged with no domain", recipient)) + } + if tagged != domain { return rejected(fmt.Errorf("domain %q does not match the address it was sent to (%s)", domain, recipient)) } ok, err := st.DomainExists(domain) diff --git a/internal/dmarc/parse_test.go b/internal/dmarc/parse_test.go index 9cd74b1..645be24 100644 --- a/internal/dmarc/parse_test.go +++ b/internal/dmarc/parse_test.go @@ -101,7 +101,7 @@ func TestIngestMessageStoresReport(t *testing.T) { // append raw gzip for simple single-part test msg.Write(gz.Bytes()) - if err := IngestMessage(st, msg, "dmarc-reports@mail.example.com", time.Now().UTC()); err != nil { + if err := IngestMessage(st, msg, "dmarc-reports+example.com@mail.example.com", time.Now().UTC()); err != nil { t.Fatalf("IngestMessage: %v", err) } list, err := st.ListDMARCReports(store.DMARCReportScope{AllDomains: true}, 10) @@ -131,7 +131,7 @@ func TestIngestMessageRejectsUnknownDomain(t *testing.T) { } defer st.Close() - err = IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)), "dmarc-reports@mail.example.com", time.Now().UTC()) + err = IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)), "dmarc-reports+example.com@mail.example.com", time.Now().UTC()) if !errors.Is(err, ErrRejected) { t.Fatalf("report for an unconfigured domain: err = %v, want ErrRejected", err) } @@ -164,6 +164,14 @@ func TestIngestMessageRejectsTagMismatch(t *testing.T) { if !errors.Is(err, ErrRejected) { t.Fatalf("report at an address tagged for another domain: err = %v, want ErrRejected", err) } + // An address with no tag is no domain's mailbox, whatever the report + // claims (security audit 2026-10-17, W1). + for _, untagged := range []string{"ops@mail.example.com", "dmarc-reports@mail.example.com"} { + err = IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)), untagged, time.Now().UTC()) + if !errors.Is(err, ErrRejected) { + t.Errorf("report at the untagged %s: err = %v, want ErrRejected", untagged, err) + } + } } // What cannot be read as a report is rejected, not put off: Postfix would diff --git a/internal/dmarc/service.go b/internal/dmarc/service.go index 97e38b9..f51da0e 100644 --- a/internal/dmarc/service.go +++ b/internal/dmarc/service.go @@ -63,17 +63,20 @@ func (s *Service) AllowedRecipients() ([]string, error) { seen[addr] = true out = append(out, addr) } - // Only what a domain has chosen is open: its report address opens the - // mailbox it names, if that is on this host. There is no mailbox for the - // server as a whole. + // Only what a domain has chosen is open, and only the one mailbox that is + // its own: the hosted address tagged with its name. A stored address on + // this host that is anything else - typed before 2.14.4, when the form + // still took one - opens nothing: it was an untagged mailbox, or another + // domain's, open to the internet (security audit 2026-10-17, W1). There + // is no mailbox for the server as a whole. domains, err := s.store.ListDomains() if err != nil { return nil, err } for _, d := range domains { - // add keeps only addresses on this host, so a domain reporting elsewhere - // (or nowhere) opens nothing here. - add(d.DMARCRua) + if hosted := HostedReportAddress(s.hostname, d.Name); strings.EqualFold(strings.TrimSpace(d.DMARCRua), hosted) { + add(hosted) + } } sort.Strings(out) return out, nil diff --git a/internal/dmarc/service_test.go b/internal/dmarc/service_test.go index 0380ae1..81ecdbd 100644 --- a/internal/dmarc/service_test.go +++ b/internal/dmarc/service_test.go @@ -32,11 +32,21 @@ func TestAllowedRecipientsFollowTheDomainAddresses(t *testing.T) { t.Fatal(err) } + // Another domain's hosted address, stored for this one: no mailbox + // either (W1's second shape). + stolen, _ := st.AddDomain("stolen.example", "sel") + if err := st.SetDomainDMARCAddress(stolen.ID, HostedReportAddress("mail.example.com", "victim.example")); err != nil { + t.Fatal(err) + } + got, err := NewService(st, nil, "mail.example.com", true).AllowedRecipients() if err != nil { t.Fatalf("AllowedRecipients: %v", err) } - want := []string{"dmarc-reports+hosted.example@mail.example.com", "ops@mail.example.com"} + // Only the hosted address of the domain that chose it: an address typed + // on the server's name (ops@) opens nothing (security audit 2026-10-17, + // W1; this test asserted the opposite until then). + want := []string{"dmarc-reports+hosted.example@mail.example.com"} if !reflect.DeepEqual(got, want) { t.Errorf("allowed recipients = %v, want %v", got, want) } diff --git a/internal/web/handlers/handlers_dmarc.go b/internal/web/handlers/handlers_dmarc.go index 6b9ce64..46c7d26 100644 --- a/internal/web/handlers/handlers_dmarc.go +++ b/internal/web/handlers/handlers_dmarc.go @@ -44,6 +44,14 @@ func (h *Handlers) HandleDomainDMARC(w http.ResponseWriter, r *http.Request) { refuse(http.StatusBadRequest, "Enter the report address or choose another option.") return } + // An address on the server's own name is a mailbox of this server: + // the one way to one is the hosted choice, which is tagged with this + // domain. A typed one opened an untagged mailbox - or another + // domain's - to the whole internet (security audit 2026-10-17, W1). + if dmarc.IsHostedOnHostname(email, h.cfg.Hostname) { + refuse(http.StatusBadRequest, "That address is on this server's own name. To have SelfPost receive the reports, choose SelfPost hosted.") + return + } rua = email case ruaHosted: if h.dmarc == nil || !h.dmarc.Enabled() { diff --git a/internal/web/handlers/handlers_dmarc_test.go b/internal/web/handlers/handlers_dmarc_test.go index 7562ab4..e17682f 100644 --- a/internal/web/handlers/handlers_dmarc_test.go +++ b/internal/web/handlers/handlers_dmarc_test.go @@ -356,3 +356,23 @@ func TestDeleteDomainSaysItIsGoneWhenARebuildFails(t *testing.T) { t.Errorf("the domain is still there: %v", err) } } + +// A typed report address on the server's own name is refused: the hosted +// choice is the one way to a mailbox here, tagged with the domain - a typed +// one opened an untagged mailbox, or another domain's, to the internet +// (security audit 2026-10-17, W1). +func TestCustomReportAddressOnTheHostnameIsRefused(t *testing.T) { + s, _ := hostedStand(t) + for _, addr := range []string{"ops@mail.example.org", "dmarc-reports+victim.example@MAIL.example.org", hostedForExample} { + rec := s.saveReports(url.Values{"dmarc_rua_mode": {"custom"}, "dmarc_rua_email": {addr}}) + if rec.Code != http.StatusBadRequest || !strings.Contains(rec.Body.String(), "choose SelfPost hosted") { + t.Errorf("custom %s = %d:\n%.600s", addr, rec.Code, rec.Body.String()) + } + if got := s.reportAddress(t); got != "" { + t.Errorf("custom %s stored %q", addr, got) + } + } + if rec := s.saveReports(url.Values{"dmarc_rua_mode": {"custom"}, "dmarc_rua_email": {"reports@hub.example"}}); rec.Code != http.StatusSeeOther { + t.Errorf("an address elsewhere = %d", rec.Code) + } +}