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 <noreply@anthropic.com>
This commit is contained in:
@@ -5,6 +5,18 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version
|
|||||||
|
|
||||||
## [Unreleased]
|
## [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
|
## [2.0.0] - 2026-10-11
|
||||||
|
|
||||||
The panel redesign. A breaking release: every panel URL changed and the
|
The panel redesign. A breaking release: every panel URL changed and the
|
||||||
|
|||||||
@@ -1,6 +1,7 @@
|
|||||||
package main
|
package main
|
||||||
|
|
||||||
import (
|
import (
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"log"
|
"log"
|
||||||
@@ -28,14 +29,31 @@ func runDMARCIngestMode() error {
|
|||||||
|
|
||||||
recipient := ingestRecipient(os.Args[1:])
|
recipient := ingestRecipient(os.Args[1:])
|
||||||
if err := dmarc.IngestMessage(st, io.LimitReader(os.Stdin, maxIngestStdin), recipient, time.Now().UTC()); err != nil {
|
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 {
|
if incrErr := st.IncrDMARCParseFailures(); incrErr != nil {
|
||||||
log.Printf("dmarc ingest: record failure: %v", incrErr)
|
log.Printf("dmarc ingest: record failure: %v", incrErr)
|
||||||
}
|
}
|
||||||
return err
|
logRejected(err)
|
||||||
}
|
}
|
||||||
return nil
|
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
|
// 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,
|
// 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).
|
// build/postfix-config.sh) - it sets no RECIPIENT variable, that is local(8).
|
||||||
|
|||||||
@@ -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.
|
sees the reports of the domains assigned to them, without the *Ingest* box.
|
||||||
Summaries are pruned (500 kept, 90 days max).
|
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
|
**Not an open relay.** The inbound smtpd offers no SASL. With DMARC ingest
|
||||||
alone, `check_recipient_access` permits only configured report addresses;
|
alone, `check_recipient_access` permits only configured report addresses;
|
||||||
everything else is rejected. With inbound relay enabled too, both allow-lists
|
everything else is rejected. With inbound relay enabled too, both allow-lists
|
||||||
|
|||||||
@@ -3,6 +3,7 @@ package dmarc
|
|||||||
import (
|
import (
|
||||||
"bytes"
|
"bytes"
|
||||||
"encoding/base64"
|
"encoding/base64"
|
||||||
|
"errors"
|
||||||
"fmt"
|
"fmt"
|
||||||
"io"
|
"io"
|
||||||
"mime"
|
"mime"
|
||||||
@@ -16,6 +17,15 @@ import (
|
|||||||
"github.com/mixeme/selfpost/internal/store"
|
"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
|
// IngestMessage parses a raw RFC 5322 message from r and stores any DMARC
|
||||||
// aggregate attachment it finds.
|
// aggregate attachment it finds.
|
||||||
//
|
//
|
||||||
@@ -34,11 +44,11 @@ func IngestMessage(st *store.Store, r io.Reader, recipient string, receivedAt ti
|
|||||||
}
|
}
|
||||||
payload, err := extractAggregatePayload(raw)
|
payload, err := extractAggregatePayload(raw)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return rejected(err)
|
||||||
}
|
}
|
||||||
parsed, err := ParseAggregate(payload)
|
parsed, err := ParseAggregate(payload)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return rejected(err)
|
||||||
}
|
}
|
||||||
recipient = strings.ToLower(strings.TrimSpace(recipient))
|
recipient = strings.ToLower(strings.TrimSpace(recipient))
|
||||||
if err := checkReportDomain(st, parsed.Domain, recipient); err != nil {
|
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.
|
// delivered to.
|
||||||
func checkReportDomain(st *store.Store, domain, recipient string) error {
|
func checkReportDomain(st *store.Store, domain, recipient string) error {
|
||||||
if domain == "" {
|
if domain == "" {
|
||||||
return fmt.Errorf("dmarc report: no policy domain")
|
return rejected(fmt.Errorf("no policy domain"))
|
||||||
}
|
}
|
||||||
if tagged := TaggedDomain(recipient); tagged != "" && tagged != 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)
|
ok, err := st.DomainExists(domain)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
return err
|
return err
|
||||||
}
|
}
|
||||||
if !ok {
|
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
|
return nil
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,6 +4,7 @@ import (
|
|||||||
"archive/zip"
|
"archive/zip"
|
||||||
"bytes"
|
"bytes"
|
||||||
"compress/gzip"
|
"compress/gzip"
|
||||||
|
"errors"
|
||||||
"testing"
|
"testing"
|
||||||
"time"
|
"time"
|
||||||
|
|
||||||
@@ -126,8 +127,9 @@ func TestIngestMessageRejectsUnknownDomain(t *testing.T) {
|
|||||||
}
|
}
|
||||||
defer st.Close()
|
defer st.Close()
|
||||||
|
|
||||||
if err := IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)), "dmarc-reports@mail.example.com", time.Now().UTC()); err == nil {
|
err = IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)), "dmarc-reports@mail.example.com", time.Now().UTC())
|
||||||
t.Fatal("report for an unconfigured domain was accepted")
|
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)
|
list, err := st.ListDMARCReports(store.DMARCReportScope{AllDomains: true}, 10)
|
||||||
if err != nil {
|
if err != nil {
|
||||||
@@ -155,8 +157,47 @@ func TestIngestMessageRejectsTagMismatch(t *testing.T) {
|
|||||||
|
|
||||||
err = IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)),
|
err = IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)),
|
||||||
"dmarc-reports+other.example@mail.example.com", time.Now().UTC())
|
"dmarc-reports+other.example@mail.example.com", time.Now().UTC())
|
||||||
if err == nil {
|
if !errors.Is(err, ErrRejected) {
|
||||||
t.Fatal("report was accepted at an address tagged for another domain")
|
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)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
Reference in New Issue
Block a user