fix: port-25 milter chain, DMARC report validation, auto rate-limit window
test / test (push) Has been cancelled
test / test (push) Has been cancelled
Review pass over the 1.4.0-1.9.1 work. Port 25 no longer inherits main.cf's submission milter chain: smtpd_milters is always overridden on smtp/inet, to the antispam milter or to nothing. Without that, OpenDKIM (strict, tempfail) gated relayed mail during a signing outage, and the journal-milter filed inbound mail into the outbound send log under the sender's domain -- so Deliveries showed other people's mail and a forged From: consumed that domain's level-2 budget. architecture.md already described the intended behaviour; the code now matches. DMARC reports are checked before they are stored: the policy domain must be a configured sending domain, and must match the +tag of a per-domain hosted address. The rua address form is predictable, so anyone able to reach it could file reports under any domain and evict genuine ones through the retention cap. Zip attachments are now decompressed as well as gzip. The auto level-2 ceiling is scaled to the level-1 window before the multiplier is applied; any window other than 3600s shifted the allowance by window/3600. Migration 0010 indexes send_log(app_login, created_at) -- both the per-MAIL FROM rate-limit count and the per-application statistics were scanning the created_at range. e2e now regenerates the Postfix config in-container for all four port-25 flag combinations and requires postfix check to pass; the 1.9.1 crash-loop shipped in 1.4.0 and survived three releases because only the flags-off path was exercised. Migration 0009 stays as shipped -- its conversion branch only fires for a database carrying application-scope allowed_ips from before 1.9.0. The schema reference notes that the old list was permissive and the new column is restrictive; the guide documents the allow-list as a sending control on a fail-open path, not an authentication boundary. Also: gofmt across cmd/ and internal/, unused duplicate validators dropped from internal/web/validate, export no longer swallows store errors. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This commit is contained in:
@@ -29,3 +29,21 @@ func IsHostedOnHostname(addr, hostname string) bool {
|
||||
at := strings.LastIndex(addr, "@")
|
||||
return addr[at+1:] == hostname
|
||||
}
|
||||
|
||||
// TaggedDomain returns the domain a hosted report address is tagged for
|
||||
// (dmarc-reports+<domain>@<hostname> → <domain>), or "" when addr carries no
|
||||
// such tag. Used to check a report's claimed domain against the address it
|
||||
// actually arrived at.
|
||||
func TaggedDomain(addr string) string {
|
||||
addr = strings.ToLower(strings.TrimSpace(addr))
|
||||
at := strings.LastIndex(addr, "@")
|
||||
if at <= 0 {
|
||||
return ""
|
||||
}
|
||||
local := addr[:at]
|
||||
plus := strings.Index(local, "+")
|
||||
if plus < 0 || local[:plus] != DefaultLocalPart {
|
||||
return ""
|
||||
}
|
||||
return local[plus+1:]
|
||||
}
|
||||
|
||||
@@ -18,6 +18,15 @@ import (
|
||||
|
||||
// IngestMessage parses a raw RFC 5322 message from r and stores any DMARC
|
||||
// aggregate attachment it finds.
|
||||
//
|
||||
// The report XML is unauthenticated input: anyone who can reach an allow-listed
|
||||
// rua= address can post one, and the address form is predictable. So the domain
|
||||
// the report claims is checked twice before anything is stored — it must be a
|
||||
// domain this relay actually sends for, and, when the report arrived at a
|
||||
// per-domain hosted address (dmarc-reports+<domain>@<hostname>), it must be the
|
||||
// domain that address is tagged for. Without this a stranger could file reports
|
||||
// under someone else's domain, skew the panel's alignment figures, and push
|
||||
// genuine reports out through the retention cap.
|
||||
func IngestMessage(st *store.Store, r io.Reader, recipient string, receivedAt time.Time) error {
|
||||
raw, err := io.ReadAll(io.LimitReader(r, 12<<20))
|
||||
if err != nil {
|
||||
@@ -31,6 +40,10 @@ func IngestMessage(st *store.Store, r io.Reader, recipient string, receivedAt ti
|
||||
if err != nil {
|
||||
return err
|
||||
}
|
||||
recipient = strings.ToLower(strings.TrimSpace(recipient))
|
||||
if err := checkReportDomain(st, parsed.Domain, recipient); err != nil {
|
||||
return err
|
||||
}
|
||||
rep := store.DMARCReport{
|
||||
Domain: parsed.Domain,
|
||||
Reporter: parsed.Reporter,
|
||||
@@ -46,7 +59,7 @@ func IngestMessage(st *store.Store, r io.Reader, recipient string, receivedAt ti
|
||||
PolicyASPF: parsed.PolicyASPF,
|
||||
PassCount: parsed.PassCount,
|
||||
FailCount: parsed.FailCount,
|
||||
Recipient: strings.ToLower(strings.TrimSpace(recipient)),
|
||||
Recipient: recipient,
|
||||
}
|
||||
for _, rec := range parsed.Records {
|
||||
rep.Records = append(rep.Records, store.DMARCReportRecord{
|
||||
@@ -64,6 +77,26 @@ func IngestMessage(st *store.Store, r io.Reader, recipient string, receivedAt ti
|
||||
return st.PruneDMARCReports()
|
||||
}
|
||||
|
||||
// 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.
|
||||
func checkReportDomain(st *store.Store, domain, recipient string) error {
|
||||
if domain == "" {
|
||||
return fmt.Errorf("dmarc report: 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)
|
||||
}
|
||||
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 nil
|
||||
}
|
||||
|
||||
func extractAggregatePayload(raw []byte) ([]byte, error) {
|
||||
msg, err := mail.ReadMessage(bytes.NewReader(raw))
|
||||
if err != nil {
|
||||
|
||||
+51
-11
@@ -1,6 +1,7 @@
|
||||
package dmarc
|
||||
|
||||
import (
|
||||
"archive/zip"
|
||||
"bytes"
|
||||
"compress/gzip"
|
||||
"encoding/xml"
|
||||
@@ -73,9 +74,9 @@ type feedbackXML struct {
|
||||
} `xml:"record"`
|
||||
}
|
||||
|
||||
// ParseAggregate decodes gzip-compressed or raw DMARC aggregate XML.
|
||||
// ParseAggregate decodes gzip- or zip-compressed, or raw, DMARC aggregate XML.
|
||||
func ParseAggregate(raw []byte) (ParsedReport, error) {
|
||||
data, err := maybeGunzip(raw)
|
||||
data, err := decompress(raw)
|
||||
if err != nil {
|
||||
return ParsedReport{}, err
|
||||
}
|
||||
@@ -128,20 +129,59 @@ func dmarcRecordPasses(r ParsedRecord) bool {
|
||||
return strings.EqualFold(r.SPFResult, "pass") || strings.EqualFold(r.DKIMResult, "pass")
|
||||
}
|
||||
|
||||
func maybeGunzip(raw []byte) ([]byte, error) {
|
||||
if len(raw) >= 2 && raw[0] == 0x1f && raw[1] == 0x8b {
|
||||
zr, err := gzip.NewReader(bytes.NewReader(raw))
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("dmarc gzip: %w", err)
|
||||
// maxAggregateXML bounds the decompressed payload, so a small archive cannot
|
||||
// expand into an unbounded allocation.
|
||||
const maxAggregateXML = 8 << 20
|
||||
|
||||
// decompress unwraps the container the reporter used. Both are seen in the
|
||||
// wild: most senders gzip the XML, some (notably several Microsoft and Yahoo
|
||||
// endpoints) ship it as a single-entry zip.
|
||||
func decompress(raw []byte) ([]byte, error) {
|
||||
switch {
|
||||
case len(raw) >= 2 && raw[0] == 0x1f && raw[1] == 0x8b:
|
||||
return gunzip(raw)
|
||||
case len(raw) >= 4 && raw[0] == 'P' && raw[1] == 'K' && raw[2] == 0x03 && raw[3] == 0x04:
|
||||
return unzip(raw)
|
||||
}
|
||||
return raw, nil
|
||||
}
|
||||
|
||||
func gunzip(raw []byte) ([]byte, error) {
|
||||
zr, err := gzip.NewReader(bytes.NewReader(raw))
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("dmarc gzip: %w", err)
|
||||
}
|
||||
defer zr.Close()
|
||||
data, err := io.ReadAll(io.LimitReader(zr, maxAggregateXML))
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("dmarc gzip read: %w", err)
|
||||
}
|
||||
return data, nil
|
||||
}
|
||||
|
||||
// unzip returns the first .xml entry of a zip archive. Directories and any
|
||||
// other members are skipped rather than trusted.
|
||||
func unzip(raw []byte) ([]byte, error) {
|
||||
zr, err := zip.NewReader(bytes.NewReader(raw), int64(len(raw)))
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("dmarc zip: %w", err)
|
||||
}
|
||||
for _, f := range zr.File {
|
||||
if f.FileInfo().IsDir() || !strings.HasSuffix(strings.ToLower(f.Name), ".xml") {
|
||||
continue
|
||||
}
|
||||
defer zr.Close()
|
||||
data, err := io.ReadAll(io.LimitReader(zr, 8<<20))
|
||||
rc, err := f.Open()
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("dmarc gzip read: %w", err)
|
||||
return nil, fmt.Errorf("dmarc zip open %s: %w", f.Name, err)
|
||||
}
|
||||
data, err := io.ReadAll(io.LimitReader(rc, maxAggregateXML))
|
||||
rc.Close()
|
||||
if err != nil {
|
||||
return nil, fmt.Errorf("dmarc zip read %s: %w", f.Name, err)
|
||||
}
|
||||
return data, nil
|
||||
}
|
||||
return raw, nil
|
||||
return nil, fmt.Errorf("dmarc zip: no xml entry")
|
||||
}
|
||||
|
||||
// TightenPolicyHint summarises whether raising p= looks reasonable.
|
||||
|
||||
@@ -1,6 +1,7 @@
|
||||
package dmarc
|
||||
|
||||
import (
|
||||
"archive/zip"
|
||||
"bytes"
|
||||
"compress/gzip"
|
||||
"testing"
|
||||
@@ -76,6 +77,9 @@ func TestIngestMessageStoresReport(t *testing.T) {
|
||||
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)
|
||||
@@ -114,3 +118,99 @@ func TestHostedReportAddress(t *testing.T) {
|
||||
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()
|
||||
|
||||
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")
|
||||
}
|
||||
list, err := st.ListDMARCReports(nil, 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 err == nil {
|
||||
t.Fatal("report was accepted at an address tagged for another domain")
|
||||
}
|
||||
}
|
||||
|
||||
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()
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user