dmarc: no custom report address on the server's own name (audit W1)

Security audit 2026-10-17, finding W1, stage 1. HandleDomainDMARC took any
address validate.Email takes; AllowedRecipients opened every stored address
on the hostname, not only the domain's hosted one; checkReportDomain
cross-checked the claimed domain only against a dmarc-reports+<tag> local
part. So a domain user of tenant A could store ops@<hostname> and from then
on anyone could mail a report claiming tenant B's domain to it, stored and
shown under B and counted toward B's 500; or store B's hosted address and
open B's mailbox without B choosing hosted. TestAllowedRecipients asserted
ops@<host> was allowed - a choice, now reversed.

Now: a custom address on the hostname is refused on the form ("choose
SelfPost hosted"); AllowedRecipients adds a domain's address only when it
is that domain's own hosted address; checkReportDomain requires a tag and
the claimed domain to equal it. The live registry (checked 2026-10-17)
holds two hosted addresses and no custom one on the hostname, so nothing
there changes.

Tests: TestCustomReportAddressOnTheHostnameIsRefused (typed, another
domain's hosted, the domain's own hosted typed as custom; an address
elsewhere still saves); TestAllowedRecipientsFollowTheDomainAddresses
inverted plus another domain's hosted address stored; untagged addresses
rejected in TestIngestMessageRejectsTagMismatch; the ingest tests deliver
to tagged addresses.

Docs: security.md § Accepted risks (DMARC), guide § DMARC reports;
CHANGELOG; plan.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
mix
2026-10-17 21:47:34 +03:00
co-authored by Claude Opus 5.5
parent c50314fb4c
commit 626bd337be
10 changed files with 91 additions and 21 deletions
+9 -3
View File
@@ -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)
+10 -2
View File
@@ -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
+9 -6
View File
@@ -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
+11 -1
View File
@@ -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)
}