From 09d9625f32334086d50875aa320ec0b963c3b57a Mon Sep 17 00:00:00 2001 From: Mix Date: Sun, 11 Oct 2026 16:28:12 +0300 Subject: [PATCH] dmarc-ingest: what is not a report this relay keeps is dropped, not retried Owner, 2026-10-11: foreign and malformed reports are turned away. The command answered every failure with EX_TEMPFAIL, so Postfix kept a message that could never be stored and offered it again for days, each attempt adding to the parse-failure count. IngestMessage now marks those failures (unreadable as an aggregate report, a domain not sent from here, another domain's report address) with ErrRejected; the command counts one failure, writes the reason where Postfix puts it on the delivery line of the mail log and reports the delivery as done. No bounce: the envelope sender of such a message is whatever its author wrote, and the report addresses are public. A failing store is still EX_TEMPFAIL. go vet ./cmd/... ./internal/dmarc/: ok go test ./cmd/panel/ ./internal/dmarc/ ./internal/web/...: ok Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 12 +++++++++ cmd/panel/dmarc_ingest.go | 20 ++++++++++++++- docs/guide.md | 6 +++++ internal/dmarc/ingest.go | 20 +++++++++++---- internal/dmarc/parse_test.go | 49 +++++++++++++++++++++++++++++++++--- 5 files changed, 97 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 11d5564..14b7b3f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,18 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +### Changed + +- **A message that is not a report this relay keeps is dropped at once.** + One that cannot be read as an aggregate report, reports on a domain not + sent from here, or came to another domain's report address used to be put + off and offered again by Postfix for days, counted as a parse failure each + time. It is now accepted and discarded - no retry, and no bounce, since the + envelope sender of such a message is whatever its author wrote. It counts + as one parse failure and the reason is in the mail log, on the message's + own delivery line (Server › System log, `dropped: …`). Only a failure of the database is still + retried. + ## [2.0.0] - 2026-10-11 The panel redesign. A breaking release: every panel URL changed and the diff --git a/cmd/panel/dmarc_ingest.go b/cmd/panel/dmarc_ingest.go index 9d7699b..17421b4 100644 --- a/cmd/panel/dmarc_ingest.go +++ b/cmd/panel/dmarc_ingest.go @@ -1,6 +1,7 @@ package main import ( + "errors" "fmt" "io" "log" @@ -28,14 +29,31 @@ func runDMARCIngestMode() error { recipient := ingestRecipient(os.Args[1:]) if err := dmarc.IngestMessage(st, io.LimitReader(os.Stdin, maxIngestStdin), recipient, time.Now().UTC()); err != nil { + if !errors.Is(err, dmarc.ErrRejected) { + return err + } + // Not a report this relay keeps. It is dropped: deferring would have + // Postfix try it again for days, and a bounce would go to an envelope + // sender anyone can write - the report addresses are public. if incrErr := st.IncrDMARCParseFailures(); incrErr != nil { log.Printf("dmarc ingest: record failure: %v", incrErr) } - return err + logRejected(err) } return nil } +// logRejected leaves the reason in the mail log. Postfix puts the start of +// what a delivery command wrote - about a hundred bytes - into its own line +// for the message ("status=sent (delivered via dmarc-ingest service (...))"), +// so the reason goes out first and bare: no prefix, no date, no recipient, +// all of which that line already has. postlog is no use here: pipe(8) starts +// the command without the group that may write to Postfix's log socket. +func logRejected(reason error) { + text := strings.TrimPrefix(reason.Error(), dmarc.ErrRejected.Error()+": ") + fmt.Fprintln(os.Stderr, "dropped: "+text) +} + // ingestRecipient is the report address the message was delivered to. Postfix's // pipe(8) hands it over as an argument (${recipient} in the service's argv, // build/postfix-config.sh) - it sets no RECIPIENT variable, that is local(8). diff --git a/docs/guide.md b/docs/guide.md index 7836555..923b1cd 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -893,6 +893,12 @@ from *DMARC reports* in the domain page's menu. A user with the *domain* role sees the reports of the domains assigned to them, without the *Ingest* box. Summaries are pruned (500 kept, 90 days max). +A message that is not a report the relay keeps — unreadable as an aggregate +report, about a domain not sent from here, or delivered to another domain's +report address — is dropped without a retry and without a bounce. It adds one +to *parse failures*, and the reason is on the message's delivery line in the mail log +(*Server › System log*, `dropped: …`). + **Not an open relay.** The inbound smtpd offers no SASL. With DMARC ingest alone, `check_recipient_access` permits only configured report addresses; everything else is rejected. With inbound relay enabled too, both allow-lists diff --git a/internal/dmarc/ingest.go b/internal/dmarc/ingest.go index ece9f5f..39ac310 100644 --- a/internal/dmarc/ingest.go +++ b/internal/dmarc/ingest.go @@ -3,6 +3,7 @@ package dmarc import ( "bytes" "encoding/base64" + "errors" "fmt" "io" "mime" @@ -16,6 +17,15 @@ import ( "github.com/mixeme/selfpost/internal/store" ) +// ErrRejected marks a message that is not a report this relay keeps and never +// will be: it cannot be read as an aggregate report, or it reports on a domain +// that is not sent from here, or it came to another domain's address. Sending +// it again changes nothing, so the caller drops it. Any other error is the +// store's and worth another try. +var ErrRejected = errors.New("dmarc report rejected") + +func rejected(err error) error { return fmt.Errorf("%w: %w", ErrRejected, err) } + // IngestMessage parses a raw RFC 5322 message from r and stores any DMARC // aggregate attachment it finds. // @@ -34,11 +44,11 @@ func IngestMessage(st *store.Store, r io.Reader, recipient string, receivedAt ti } payload, err := extractAggregatePayload(raw) if err != nil { - return err + return rejected(err) } parsed, err := ParseAggregate(payload) if err != nil { - return err + return rejected(err) } recipient = strings.ToLower(strings.TrimSpace(recipient)) if err := checkReportDomain(st, parsed.Domain, recipient); err != nil { @@ -82,17 +92,17 @@ func IngestMessage(st *store.Store, r io.Reader, recipient string, receivedAt ti // delivered to. func checkReportDomain(st *store.Store, domain, recipient string) error { if domain == "" { - return fmt.Errorf("dmarc report: no policy domain") + return rejected(fmt.Errorf("no policy domain")) } if tagged := TaggedDomain(recipient); tagged != "" && tagged != domain { - return fmt.Errorf("dmarc report: domain %q does not match the address it was sent to (%s)", domain, recipient) + return rejected(fmt.Errorf("domain %q does not match the address it was sent to (%s)", domain, recipient)) } ok, err := st.DomainExists(domain) if err != nil { return err } if !ok { - return fmt.Errorf("dmarc report: %q is not a domain configured on this relay", domain) + return rejected(fmt.Errorf("%q is not a domain configured on this relay", domain)) } return nil } diff --git a/internal/dmarc/parse_test.go b/internal/dmarc/parse_test.go index 283583c..a49aaf7 100644 --- a/internal/dmarc/parse_test.go +++ b/internal/dmarc/parse_test.go @@ -4,6 +4,7 @@ import ( "archive/zip" "bytes" "compress/gzip" + "errors" "testing" "time" @@ -126,8 +127,9 @@ func TestIngestMessageRejectsUnknownDomain(t *testing.T) { } defer st.Close() - if err := IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)), "dmarc-reports@mail.example.com", time.Now().UTC()); err == nil { - t.Fatal("report for an unconfigured domain was accepted") + err = IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)), "dmarc-reports@mail.example.com", time.Now().UTC()) + if !errors.Is(err, ErrRejected) { + t.Fatalf("report for an unconfigured domain: err = %v, want ErrRejected", err) } list, err := st.ListDMARCReports(store.DMARCReportScope{AllDomains: true}, 10) if err != nil { @@ -155,8 +157,47 @@ func TestIngestMessageRejectsTagMismatch(t *testing.T) { err = IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)), "dmarc-reports+other.example@mail.example.com", time.Now().UTC()) - if err == nil { - t.Fatal("report was accepted at an address tagged for another domain") + if !errors.Is(err, ErrRejected) { + t.Fatalf("report at an address tagged for another domain: err = %v, want ErrRejected", err) + } +} + +// What cannot be read as a report is rejected, not put off: Postfix would +// offer the same bytes again for days. +func TestIngestMessageRejectsWhatIsNotAReport(t *testing.T) { + st, err := store.Open(t.TempDir() + "/test.db") + if err != nil { + t.Fatal(err) + } + defer st.Close() + if _, err := st.AddDomain("example.com", "selfpost"); err != nil { + t.Fatal(err) + } + for name, msg := range map[string]string{ + "plain text": "From: someone@example.net\r\nSubject: hello\r\n\r\nnot a report\r\n", + "no headers": "garbage", + "broken gzip": "From: a@example.net\r\nContent-Type: application/gzip\r\n\r\nnot gzip at all", + } { + err := IngestMessage(st, bytes.NewBufferString(msg), "dmarc-reports+example.com@mail.example.com", time.Now().UTC()) + if !errors.Is(err, ErrRejected) { + t.Errorf("%s: err = %v, want ErrRejected", name, err) + } + } +} + +// A failing store is not the report's fault: the message is to be tried again. +func TestIngestMessageStoreFailureIsNotARejection(t *testing.T) { + st, err := store.Open(t.TempDir() + "/test.db") + if err != nil { + t.Fatal(err) + } + if _, err := st.AddDomain("example.com", "selfpost"); err != nil { + t.Fatal(err) + } + st.Close() + err = IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)), "dmarc-reports+example.com@mail.example.com", time.Now().UTC()) + if err == nil || errors.Is(err, ErrRejected) { + t.Fatalf("closed store: err = %v, want an error that is not ErrRejected", err) } }