Files
selfpost/internal/dmarc/parse_test.go
T
mixandClaude Opus 5.5 9eddd49602
test / test (push) Canceled after 0s
audit 2c: findings of the two reviews
Technical review (Opus, 11 findings, 2 medium) and security review (Fable,
no high or medium) of 3c86280..HEAD. Both confirm the port gives
dkim_mail_parse()'s result on every input it parses, and that no line the
check passes is signed with another domain's key.

- F1 (medium): OpenDKIM's parser takes lines no mail program reads as one
  address - "ceo@bank.com:me@mine.com", "Name <ceo@bank.com:me@mine.com>",
  "<b@evil.com, a@mine.com>", "ceo@bank.com <me@mine.com>",
  "ceo@bank.com@mine.com" - and signs for the last while a lenient reader
  shows the first. The line must now also be one address in RFC 5322's
  grammar (net/mail, display names in any charset passed through), at the
  domain OpenDKIM signs for. A quoted display name may still hold anything.
- F2 (medium): the bindings were asked about OpenDKIM's form of the
  address, quotes and all, so "x"@mine.com passed for an application
  holding @mine.com while x@mine.com belonged to another. They are now
  asked about the reader's address, its local part unquoted, as Postfix
  holds the envelope's.
- F3 / security (low): Bindings.Allowed and DomainOf fold case in ASCII
  only: Postfix folds as Unicode (casefold.c, ICU), but every key and
  login the panel writes is ASCII, and the Unicode fold let the Kelvin sign
  pass for "k" (net/mail then refuses such a line on the domain test).
- F4 (low): a TERM between "postfix start-fg &" and "child=$!" is no longer
  lost: the wrapper stops Postfix as soon as the pid is known.
- F5 (low): for startsecs after a start any failed reload passed as "not
  running". postfix-reload is now build/postfix-reload.sh: exit 75 when
  master -t says no master runs, else exec postfix reload; RunOnce returns
  EndError with the status, and only 75 is taken as done. supervisor.State
  goes.
- F6 (low): crashexit spares postfix-reload too - a reload that could not
  be spawned is reported to whoever asked, not a reason to stop the
  container.
- F7 (low): a certs/ that is itself a link is resolved before the walk
  (filepath.WalkDir does not follow its root).
- F8 (low): at shutdown the tailer reads what is left of the rotated
  file before saving its place. Not changed: a refusal line still waiting
  on the old file at shutdown is not kept - the panel writes refused rows
  itself, only a kill loses one, and that window is the minute's grace.
- F9 (low): pct read as text - empty or unreadable is 100, the rest held
  to 0..100.
- F10 (low): the directory is synced after sasldb2's rename.
- F11 (low): POSTFIX_STARTSECS by expr, so "010" or "08" in
  MILTER_WAIT_TIMEOUT are decimal, not octal or an error.
- Security (low): pagehide also empties defaultValue - the input's value
  attribute and the textarea's text - not only .value. A sasldb2.new left
  by a killed panel is removed at start (entrypoint) and never archived.
  security.md says that links under certs/ are followed and by whom they
  can be placed.

Tests: sender check cases for F1 (four lines), F2 (a bound address
quoted, for its owner and for another), F3; EndError's state and status;
pct cases. architecture.md and CHANGELOG describe the reader rule. Stand
(2026-10-15, make e2e, exit 0); go vet and go test ./... pass on the
server (24 packages).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
2026-10-15 09:17:33 +03:00

290 lines
8.2 KiB
Go

package dmarc
import (
"archive/zip"
"bytes"
"compress/gzip"
"errors"
"strings"
"testing"
"time"
"github.com/mixeme/selfpost/internal/store"
)
// sampleXML and the messages in this file are composed by hand, in the shape
// RFC 7489 gives (Appendix C, the schema; § 7.2.1.1, application/gzip and the
// .xml.gz name). The org name is any reporter's; no report was copied.
const sampleXML = `<?xml version="1.0"?>
<feedback>
<report_metadata>
<org_name>google.com</org_name>
<email>noreply@google.com</email>
<report_id>12345</report_id>
<date_range><begin>1723593600</begin><end>1723680000</end></date_range>
</report_metadata>
<policy_published>
<domain>example.com</domain>
<p>none</p><sp>none</sp><pct>100</pct>
<adkim>r</adkim><aspf>r</aspf>
</policy_published>
<record>
<row>
<source_ip>203.0.113.10</source_ip>
<count>10</count>
<policy_evaluated>
<disposition>none</disposition>
<dkim>pass</dkim><spf>pass</spf>
</policy_evaluated>
</row>
<identifiers><header_from>example.com</header_from></identifiers>
</record>
<record>
<row>
<source_ip>198.51.100.1</source_ip>
<count>2</count>
<policy_evaluated>
<disposition>none</disposition>
<dkim>fail</dkim><spf>fail</spf>
</policy_evaluated>
</row>
<identifiers><header_from>example.com</header_from></identifiers>
</record>
</feedback>`
func TestParseAggregateGzip(t *testing.T) {
var buf bytes.Buffer
zw := gzip.NewWriter(&buf)
if _, err := zw.Write([]byte(sampleXML)); err != nil {
t.Fatal(err)
}
if err := zw.Close(); err != nil {
t.Fatal(err)
}
got, err := ParseAggregate(buf.Bytes())
if err != nil {
t.Fatalf("ParseAggregate: %v", err)
}
if got.Domain != "example.com" || got.Reporter != "google.com" {
t.Fatalf("metadata = %+v", got)
}
if got.PassCount != 10 || got.FailCount != 2 {
t.Fatalf("counts = %d pass %d fail", got.PassCount, got.FailCount)
}
if !got.PeriodBegin.Equal(time.Unix(1723593600, 0).UTC()) {
t.Fatalf("period begin = %v", got.PeriodBegin)
}
}
func TestIngestMessageStoresReport(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)
}
var gz bytes.Buffer
zw := gzip.NewWriter(&gz)
_, _ = zw.Write([]byte(sampleXML))
_ = zw.Close()
msg := bytes.NewBufferString(
"From: noreply@google.com\r\n" +
"To: dmarc-reports@mail.example.com\r\n" +
"Subject: Report\r\n" +
"Content-Type: application/gzip; name=\"report.xml.gz\"\r\n" +
"\r\n",
)
// 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 {
t.Fatalf("IngestMessage: %v", err)
}
list, err := st.ListDMARCReports(store.DMARCReportScope{AllDomains: true}, 10)
if err != nil {
t.Fatal(err)
}
if len(list) != 1 {
t.Fatalf("reports = %d", len(list))
}
if list[0].PassCount != 10 {
t.Fatalf("pass = %d", list[0].PassCount)
}
}
func TestHostedReportAddress(t *testing.T) {
got := HostedReportAddress("mail.example.com", "Example.COM")
want := "dmarc-reports+example.com@mail.example.com"
if got != want {
t.Fatalf("got %q want %q", got, want)
}
}
func TestIngestMessageRejectsUnknownDomain(t *testing.T) {
st, err := store.Open(t.TempDir() + "/test.db")
if err != nil {
t.Fatal(err)
}
defer st.Close()
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 {
t.Fatal(err)
}
if len(list) != 0 {
t.Fatalf("stored %d report(s) for an unconfigured domain", len(list))
}
}
// A report claiming example.com must not be accepted at the address tagged for
// another domain, even when both domains are configured here.
func TestIngestMessageRejectsTagMismatch(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)
}
if _, err := st.AddDomain("other.example", "selfpost"); err != nil {
t.Fatal(err)
}
err = IngestMessage(st, bytes.NewBufferString(gzipMessage(t, sampleXML)),
"dmarc-reports+other.example@mail.example.com", time.Now().UTC())
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)
}
}
func TestTaggedDomain(t *testing.T) {
cases := map[string]string{
"dmarc-reports+example.com@mail.example.com": "example.com",
"dmarc-reports@mail.example.com": "",
"postmaster+example.com@mail.example.com": "",
"not-an-address": "",
}
for addr, want := range cases {
if got := TaggedDomain(addr); got != want {
t.Errorf("TaggedDomain(%q) = %q, want %q", addr, got, want)
}
}
}
func TestParseAggregateFromZip(t *testing.T) {
var buf bytes.Buffer
zw := zip.NewWriter(&buf)
w, err := zw.Create("report.xml")
if err != nil {
t.Fatal(err)
}
if _, err := w.Write([]byte(sampleXML)); err != nil {
t.Fatal(err)
}
if err := zw.Close(); err != nil {
t.Fatal(err)
}
got, err := ParseAggregate(buf.Bytes())
if err != nil {
t.Fatalf("ParseAggregate(zip): %v", err)
}
if got.Domain != "example.com" || got.PassCount != 10 {
t.Fatalf("zip report = %+v", got)
}
}
// gzipMessage wraps xml in the single-part gzip message the ingest pipe sees.
func gzipMessage(t *testing.T, xml string) string {
t.Helper()
var gz bytes.Buffer
zw := gzip.NewWriter(&gz)
if _, err := zw.Write([]byte(xml)); err != nil {
t.Fatal(err)
}
if err := zw.Close(); err != nil {
t.Fatal(err)
}
return "From: noreply@google.com\r\n" +
"To: dmarc-reports@mail.example.com\r\n" +
"Subject: Report\r\n" +
"Content-Type: application/gzip; name=\"report.xml.gz\"\r\n" +
"\r\n" + gz.String()
}
// pct is 100 when the report leaves it out (RFC 7489 § 6.3), and an explicit
// 0 - a policy applied to no message - stays 0 (assumptions C-M7).
func TestPolicyPct(t *testing.T) {
for _, c := range []struct {
pct string
want int
}{
{"<pct>100</pct>", 100},
{"<pct>0</pct>", 0},
{"<pct>25</pct>", 25},
{"", 100},
{"<pct/>", 100},
{"<pct> 50 </pct>", 50},
{"<pct>150</pct>", 100},
{"<pct>-5</pct>", 0},
{"<pct>x</pct>", 100},
} {
doc := strings.Replace(sampleXML, "<pct>100</pct>", c.pct, 1)
got, err := ParseAggregate([]byte(doc))
if err != nil {
t.Fatalf("%q: %v", c.pct, err)
}
if got.PolicyPct != c.want {
t.Errorf("%q: PolicyPct %d, want %d", c.pct, got.PolicyPct, c.want)
}
}
}