From efaf016c5fe183ac3434399b4510c91994c1e324 Mon Sep 17 00:00:00 2001 From: Mikhail Yenuchenko Date: Mon, 10 Aug 2026 22:47:34 +0300 Subject: [PATCH] Fix DMARC guidance for send-only relays with optional rua= settings. The panel now suggests policy-only DMARC by default, lets operators configure a default and per-domain report address, and DNS-checks hub _report._dmarc records. Future in-panel report ingestion is tracked as dmarc-reports in the roadmap. Co-authored-by: Cursor --- CHANGELOG.md | 42 ++++++++ docs/guide.md | 12 ++- docs/plans/dmarc-reports.md | 49 +++++++++ docs/roadmap.md | 28 +++++ internal/dnscheck/dnscheck.go | 37 ++++--- internal/dnscheck/dnscheck_test.go | 21 +++- internal/dnscheck/examples.go | 54 ++++++++-- internal/dnscheck/examples_test.go | 42 +++++++- internal/dnscheck/records.go | 59 +++++++++- internal/domain/transfer.go | 14 +++ internal/domain/transfer_test.go | 10 ++ internal/store/admin.go | 17 +-- internal/store/admin_test.go | 17 ++- internal/store/domains.go | 25 ++++- internal/store/domains_dmarc_test.go | 45 ++++++++ .../migrations/0004_dmarc_report_email.sql | 5 + internal/web/handlers_account.go | 101 ++++++++++++------ internal/web/handlers_apps.go | 64 +++++++++-- internal/web/handlers_dmarc.go | 56 ++++++++++ internal/web/handlers_domains.go | 6 +- internal/web/templates/account.html | 40 ++++++- internal/web/templates/domain_detail.html | 64 +++++++++-- internal/web/validate.go | 44 ++++++++ internal/web/validate_email_test.go | 18 ++++ internal/web/web.go | 30 +++--- 25 files changed, 784 insertions(+), 116 deletions(-) create mode 100644 docs/plans/dmarc-reports.md create mode 100644 internal/store/domains_dmarc_test.go create mode 100644 internal/store/migrations/0004_dmarc_report_email.sql create mode 100644 internal/web/handlers_dmarc.go create mode 100644 internal/web/validate_email_test.go diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a901c4..e7f5c80 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,12 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ### Added +- panel: DMARC guidance for send-only relays — the suggested `_dmarc` record + is now `p=none` without `rua=` by default; *Settings* and each domain page + let you configure an optional aggregate-report address (profile default plus + per-domain inherit / none / custom). When `rua=` targets another domain, the + panel shows and DNS-checks the hub's `_report._dmarc` authorisation record. + Domain export/import carries per-domain overrides. - AGPL packaging hygiene: [NOTICE](NOTICE) names the copyright holder and the bundled third-party works (htmx 0BSD, IBM Plex OFL in outlined logos); the panel footer on every page — including login and setup — shows copyright, a @@ -27,6 +33,12 @@ database, or the on-disk layout. Upgrading is a tag bump. ### Added +- panel: DMARC guidance for send-only relays — the suggested `_dmarc` record + is now `p=none` without `rua=` by default; *Settings* and each domain page + let you configure an optional aggregate-report address (profile default plus + per-domain inherit / none / custom). When `rua=` targets another domain, the + panel shows and DNS-checks the hub's `_report._dmarc` authorisation record. + Domain export/import carries per-domain overrides. - `SECURITY.md` — how to report a vulnerability privately (GitHub private vulnerability reporting, `public@mixeme.ru` as fallback), which releases get fixes, and what is in and out of scope for a relay. No response time is @@ -93,6 +105,12 @@ database, or the on-disk layout. Upgrading is a tag bump. ### Added +- panel: DMARC guidance for send-only relays — the suggested `_dmarc` record + is now `p=none` without `rua=` by default; *Settings* and each domain page + let you configure an optional aggregate-report address (profile default plus + per-domain inherit / none / custom). When `rua=` targets another domain, the + panel shows and DNS-checks the hub's `_report._dmarc` authorisation record. + Domain export/import carries per-domain overrides. - A **Delivery log** on each delivery's page (`/deliveries/{id}`): the `mail.log` lines Postfix wrote about that message, oldest first — the connection to the receiving server, its reply, and the status that reply was @@ -214,6 +232,12 @@ database, or the on-disk layout. Upgrading is a tag bump. ### Added +- panel: DMARC guidance for send-only relays — the suggested `_dmarc` record + is now `p=none` without `rua=` by default; *Settings* and each domain page + let you configure an optional aggregate-report address (profile default plus + per-domain inherit / none / custom). When `rua=` targets another domain, the + panel shows and DNS-checks the hub's `_report._dmarc` authorisation record. + Domain export/import carries per-domain overrides. - A page per delivery (`/deliveries/{id}`), reached from the *Details* link on every send-log row. It carries what the log itself no longer shows — the sending domain, the application the message was submitted under, the Postfix @@ -429,6 +453,12 @@ database, or the on-disk layout. Upgrading is a tag bump. ### Added +- panel: DMARC guidance for send-only relays — the suggested `_dmarc` record + is now `p=none` without `rua=` by default; *Settings* and each domain page + let you configure an optional aggregate-report address (profile default plus + per-domain inherit / none / custom). When `rua=` targets another domain, the + panel shows and DNS-checks the hub's `_report._dmarc` authorisation record. + Domain export/import carries per-domain overrides. - Optional password encryption for the two secret-bearing downloads (plan phase 1.5, `docs/code-review.md`): an *Encrypt with a password* checkbox on the full-backup and domain-export forms writes a `.spbk` / `.spde` envelope @@ -530,6 +560,12 @@ database, or the on-disk layout. Upgrading is a tag bump. ### Added +- panel: DMARC guidance for send-only relays — the suggested `_dmarc` record + is now `p=none` without `rua=` by default; *Settings* and each domain page + let you configure an optional aggregate-report address (profile default plus + per-domain inherit / none / custom). When `rua=` targets another domain, the + panel shows and DNS-checks the hub's `_report._dmarc` authorisation record. + Domain export/import carries per-domain overrides. - The project's mark is now in use rather than only on file. The README opens with the full stamp; the panel carries the compact one at the left of its navigation bar, linking to the status page, and the full one above the card @@ -762,6 +798,12 @@ of `docs/implementation-plan.md`). ### Added +- panel: DMARC guidance for send-only relays — the suggested `_dmarc` record + is now `p=none` without `rua=` by default; *Settings* and each domain page + let you configure an optional aggregate-report address (profile default plus + per-domain inherit / none / custom). When `rua=` targets another domain, the + panel shows and DNS-checks the hub's `_report._dmarc` authorisation record. + Domain export/import carries per-domain overrides. - Panel (Go, single static binary) with SQLite persistence, one-time crypto-random setup link, bcrypt admin auth, session cookies. - Domain management with per-domain DKIM (RSA-2048, generated in pure Go) and diff --git a/docs/guide.md b/docs/guide.md index 81770e3..251b0a6 100644 --- a/docs/guide.md +++ b/docs/guide.md @@ -148,8 +148,13 @@ Two different scopes — don't confuse them: behalf (e.g. `v=spf1 a mx ip4: -all`, adjusted to your setup). - **DKIM** — a TXT record with the exact value the panel shows on that domain's page (`domain page → DKIM TXT record`), one selector per domain. -- **DMARC** — a `_dmarc` TXT record (even a conservative `p=none` starts - building reporting/reputation history). +- **DMARC** — a `_dmarc` TXT record. The panel suggests `p=none` (monitoring + only, safe to publish immediately). On a send-only relay the sending domain + often has no inbox, so `rua=` is optional — configure a default report address + in *Settings* or per domain when you have a mailbox that receives inbound mail + elsewhere. If `rua=` points at another domain, publish `_report._dmarc` on that + hub domain too; the panel checks it. Public mail hosts (Gmail, Outlook, …) + cannot be used as external report destinations. Skipping any of the three per-domain records is the single most common reason mail lands in spam even though SelfPost delivered it correctly — DKIM passing @@ -160,7 +165,8 @@ The panel checks both scopes for you and tells you what is actually published: the *Status* page verifies the server's hostname and its reverse record (forward-confirmed reverse DNS), and each domain's page shows a *DNS status* card comparing the published DKIM record against the key this server signs with, -plus the domain's SPF and DMARC records. Results are cached for a few minutes; +plus the domain's SPF, DMARC, and (when configured) DMARC report-authorisation +records. Results are cached for a few minutes; use *Re-check* right after publishing a record. The SPF check is deliberately shallow — it looks for a mechanism that literally covers this server's address and does not follow `include:` or `redirect=`, so a record that authorizes the diff --git a/docs/plans/dmarc-reports.md b/docs/plans/dmarc-reports.md new file mode 100644 index 0000000..eda56a2 --- /dev/null +++ b/docs/plans/dmarc-reports.md @@ -0,0 +1,49 @@ +# Plan: dmarc-reports + +**Status:** candidate + +--- + +## Goal + +SelfPost **receives** DMARC aggregate reports on SMTP, parses them inside the +image, and **shows summaries in the panel** — pass/fail by source, hints when +`tighten p=` is reasonable. No external DMARC SaaS and no IMAP workflow for the +operator. + +## Scope + +**In:** +- Inbound SMTP for configured report addresses only (not a general backup-MX). +- gzip + XML aggregate parsing → SQLite summaries per sending domain. +- Panel page and/or per-domain section: recent reports, third-party senders, + delivery health of report ingestion. +- Reuse `admin.dmarc_report_email` and `domains.dmarc_rua` for DNS templates; + when enabled, suggest a SelfPost-hosted report address. + +**Out:** +- Forensic reports (`ruf=`). +- Full dashboards, APIs, email alerting. +- Mailboxes for people (IMAP/POP3/webmail). + +## Architecture (sketch) + +1. Receiving MTAs → SMTP to SelfPost (hub MX). +2. Postfix virtual alias or dedicated listener → panel ingest worker. +3. Parse XML → `dmarc_reports` table (domain, reporter, counts, date). +4. Panel reads SQLite; links from domain DNS card. + +May share port-25 plumbing with [inbound-relay.md](inbound-relay.md) but must +remain a separate, opt-in feature that does not forward mail upstream. + +## Done when + +- Operator can point `rua=` at an address SelfPost accepts and see parsed + summaries in the panel within one reporting cycle. +- With the feature off, outbound-only behaviour is unchanged. +- Documented in [guide.md](../guide.md); migrations are backward-compatible. + +## Risks + +- Attack surface of accepting mail (mitigate: strict recipient allow-list). +- Report volume and retention (mitigate: caps + pruning). diff --git a/docs/roadmap.md b/docs/roadmap.md index 35e12b8..63b3997 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -31,6 +31,7 @@ in `git log` and [CHANGELOG.md](../CHANGELOG.md). | inbound-relay | Inbound relay (backup-MX / forwarding) | **agreed** | [plans/inbound-relay.md](plans/inbound-relay.md) | | contributing | `CONTRIBUTING.md` | candidate | — | | visual-style | Обновление визуального стиля | candidate | — | +| dmarc-reports | DMARC aggregate report ingestion and panel UI | candidate | [plans/dmarc-reports.md](plans/dmarc-reports.md) | **Recommended order** (not binding): **web-split → domain-admin → inbound-relay** — first the package split, then role-wide authorisation, then @@ -142,3 +143,30 @@ dark schemes remain supported; readability and contrast are preserved. regression across pages; low priority relative to functional work — take up after explicit agreement, independently of the feature roadmap order. **Version:** no bearing on semver. + +--- + +## dmarc-reports + +**Goal:** SelfPost receives DMARC aggregate reports (RFC 7489) on SMTP, +parses the gzip/XML payloads, and shows pass/fail summaries in the panel — so +the operator does not need an external DMARC service or a separate mailbox +workflow. + +**Boundary:** an extension of v1.0 — not IMAP/webmail and not a general +inbound relay. A dedicated inbound path for report messages only; forensic +reports (`ruf=`) out of scope for v1. + +**Done when:** see [plans/dmarc-reports.md](plans/dmarc-reports.md). + +**Dependencies / risks:** inbound SMTP in the image (may share infrastructure +with [inbound-relay](plans/inbound-relay.md) but must not require backup-MX); +storage and retention of parsed summaries; the `admin.dmarc_report_email` and +`domains.dmarc_rua` settings added in the DMARC template work must stay the +source of truth for `rua=` in DNS guidance. + +**Order:** after the DMARC `rua=` settings ship; may follow or overlap with +inbound-relay depending on how port 25 acceptance is structured. + +**Version:** `1.x` MINOR. + diff --git a/internal/dnscheck/dnscheck.go b/internal/dnscheck/dnscheck.go index ba33e4f..59a2181 100644 --- a/internal/dnscheck/dnscheck.go +++ b/internal/dnscheck/dnscheck.go @@ -56,12 +56,13 @@ type Server struct { // Domain is the published-DNS state of one sending domain. type Domain struct { - Name string - DKIM Result - SPF Result - DMARC Result - Overall health.Status - CheckedAt time.Time + Name string + DKIM Result + SPF Result + DMARC Result + DMARCReportAuth Result // zero when external rua= is not used + Overall health.Status + CheckedAt time.Time } // Query describes the domain to check. ExpectedDKIM is the TXT value the panel @@ -69,11 +70,12 @@ type Domain struct { // compares DNS against the key this server actually signs with. Hostname and // ServerIPs identify this server and come from a preceding Server check. type Query struct { - Name string - Selector string - ExpectedDKIM string - Hostname string - ServerIPs []string + Name string + Selector string + ExpectedDKIM string + Hostname string + ServerIPs []string + DMARCReportEmail string // resolved rua= destination; empty = policy-only template } // resolver is the slice of *net.Resolver this package uses, as an interface so @@ -178,12 +180,19 @@ func (c *Checker) Forget(domainName string) { func (c *Checker) checkDomain(ctx context.Context, q Query) Domain { d := Domain{Name: q.Name, CheckedAt: time.Now()} var wg sync.WaitGroup - wg.Add(3) + wg.Add(4) go func() { defer wg.Done(); d.DKIM = c.checkDKIM(ctx, q) }() go func() { defer wg.Done(); d.SPF = c.checkSPF(ctx, q) }() - go func() { defer wg.Done(); d.DMARC = c.checkDMARC(ctx, q.Name) }() + go func() { defer wg.Done(); d.DMARC = c.checkDMARC(ctx, q) }() + go func() { + defer wg.Done() + hub := EmailDomain(q.DMARCReportEmail) + if hub != "" && !strings.EqualFold(hub, q.Name) { + d.DMARCReportAuth = c.checkReportAuth(ctx, hub) + } + }() wg.Wait() - d.Overall = health.Worst(d.DKIM.Status, d.SPF.Status, d.DMARC.Status) + d.Overall = health.Worst(d.DKIM.Status, d.SPF.Status, d.DMARC.Status, d.DMARCReportAuth.Status) return d } diff --git a/internal/dnscheck/dnscheck_test.go b/internal/dnscheck/dnscheck_test.go index 11eb75c..9aae461 100644 --- a/internal/dnscheck/dnscheck_test.go +++ b/internal/dnscheck/dnscheck_test.go @@ -293,7 +293,7 @@ func TestDMARC(t *testing.T) { txt["_dmarc.example.com"] = c.record } f := &fakeResolver{txt: txt} - got := newTestChecker(f).checkDMARC(context.Background(), "example.com") + got := newTestChecker(f).checkDMARC(context.Background(), Query{Name: "example.com"}) if got.Status != c.want { t.Errorf("status = %q, want %q (%s)", got.Status, c.want, got.Detail) } @@ -303,7 +303,7 @@ func TestDMARC(t *testing.T) { func TestDMARCNonePolicyIsExplained(t *testing.T) { f := &fakeResolver{txt: map[string][]string{"_dmarc.example.com": {"v=DMARC1; p=none"}}} - got := newTestChecker(f).checkDMARC(context.Background(), "example.com") + got := newTestChecker(f).checkDMARC(context.Background(), Query{Name: "example.com"}) if !strings.Contains(got.Detail, "monitoring only") { t.Errorf("p=none is not explained: %s", got.Detail) } @@ -357,3 +357,20 @@ func TestForgetDropsTheCachedDomain(t *testing.T) { t.Error("Forget did not drop the cached result") } } + +func TestReportAuth(t *testing.T) { + f := &fakeResolver{txt: map[string][]string{"_report._dmarc.hub.example": {"v=DMARC1;"}}} + got := newTestChecker(f).checkReportAuth(context.Background(), "hub.example") + if got.Status != health.StatusOK { + t.Fatalf("status = %q (%s)", got.Status, got.Detail) + } + + f = &fakeResolver{} + got = newTestChecker(f).checkReportAuth(context.Background(), "hub.example") + if got.Status != health.StatusWarn { + t.Fatalf("missing = %q, want warn", got.Status) + } + if !strings.Contains(got.Detail, ReportAuthExample()) { + t.Errorf("advice %q should cite %q", got.Detail, ReportAuthExample()) + } +} diff --git a/internal/dnscheck/examples.go b/internal/dnscheck/examples.go index 2eb3463..7c3c9be 100644 --- a/internal/dnscheck/examples.go +++ b/internal/dnscheck/examples.go @@ -1,6 +1,7 @@ package dnscheck import ( + "database/sql" "net" "strings" ) @@ -16,6 +17,13 @@ import ( // helper: it is published at the domain itself.) func DMARCRecordName(domainName string) string { return "_dmarc." + domainName } +// ReportAuthRecordName is where a report-receiving domain authorises external +// DMARC aggregate destinations (RFC 7489 §7.1). +func ReportAuthRecordName(hubDomain string) string { return "_report._dmarc." + hubDomain } + +// ReportAuthExample is the TXT value a hub domain publishes to accept reports. +func ReportAuthExample() string { return "v=DMARC1;" } + // SPFExample is the SPF record this server expects for a sending domain: the // addresses its mail actually leaves from, and "-all" to say that nothing else // is authorised. When the server's own addresses are not known (its hostname @@ -40,10 +48,44 @@ func SPFExample(hostname string, serverIPs []string) string { return "v=spf1 " + strings.Join(mechanisms, " ") + " -all" } -// DMARCExample is the least a domain should publish: monitoring only, with an -// address the aggregate reports go to. p=none is deliberate — it changes -// nothing about delivery, so it is safe to publish before the reports have -// shown that DKIM and SPF pass everywhere. -func DMARCExample(domainName string) string { - return "v=DMARC1; p=none; rua=mailto:dmarc@" + domainName +// ResolveDMARCRua picks the rua= mailbox for a sending domain: per-domain +// override wins, then the administrator profile, then policy-only (empty). +func ResolveDMARCRua(domainRua sql.NullString, profileEmail string) string { + if domainRua.Valid { + return domainRua.String + } + return profileEmail +} + +// EmailDomain returns the lower-case domain part of addr, or "" when invalid. +func EmailDomain(addr string) string { + addr = strings.TrimSpace(addr) + at := strings.LastIndex(addr, "@") + if at < 0 || at == len(addr)-1 { + return "" + } + return strings.ToLower(strings.TrimSpace(addr[at+1:])) +} + +// DMARCExample is the DMARC TXT record this server suggests for a sending +// domain. p=none is deliberate — it changes nothing about delivery. rua= is +// included only when reportEmail is set; SelfPost is send-only and most +// operators have no inbox on the sending domain itself. +func DMARCExample(reportEmail string) string { + base := "v=DMARC1; p=none" + if reportEmail == "" { + return base + } + return base + "; rua=mailto:" + reportEmail +} + +// ExternalReportAuth reports whether the hub domain must publish a +// _report._dmarc authorisation for aggregate reports sent to reportEmail from +// sendingDomain. +func ExternalReportAuth(sendingDomain, reportEmail string) (name, value string, ok bool) { + hub := EmailDomain(reportEmail) + if hub == "" || strings.EqualFold(hub, sendingDomain) { + return "", "", false + } + return ReportAuthRecordName(hub), ReportAuthExample(), true } diff --git a/internal/dnscheck/examples_test.go b/internal/dnscheck/examples_test.go index 2a542cf..8dae58b 100644 --- a/internal/dnscheck/examples_test.go +++ b/internal/dnscheck/examples_test.go @@ -2,6 +2,7 @@ package dnscheck import ( "context" + "database/sql" "strings" "testing" @@ -18,8 +19,6 @@ func TestSPFExample(t *testing.T) { {"ipv4", "mail.example.com", []string{"203.0.113.10"}, "v=spf1 ip4:203.0.113.10 -all"}, {"both families", "mail.example.com", []string{"203.0.113.10", "2001:db8::1"}, "v=spf1 ip4:203.0.113.10 ip6:2001:db8::1 -all"}, - // The hostname does not resolve, so there is no address to name; an "a:" - // mechanism still gives the operator a publishable record. {"no addresses", "mail.example.com", nil, "v=spf1 a:mail.example.com -all"}, {"unparsable addresses", "mail.example.com", []string{"not-an-ip"}, "v=spf1 a:mail.example.com -all"}, } @@ -32,6 +31,41 @@ func TestSPFExample(t *testing.T) { } } +func TestDMARCExample(t *testing.T) { + if got := DMARCExample(""); got != "v=DMARC1; p=none" { + t.Errorf("empty = %q", got) + } + want := "v=DMARC1; p=none; rua=mailto:reports@hub.example" + if got := DMARCExample("reports@hub.example"); got != want { + t.Errorf("with rua = %q, want %q", got, want) + } +} + +func TestResolveDMARCRua(t *testing.T) { + inherit := sql.NullString{} + if got := ResolveDMARCRua(inherit, "a@b.com"); got != "a@b.com" { + t.Errorf("inherit profile = %q", got) + } + none := sql.NullString{Valid: true} + if got := ResolveDMARCRua(none, "a@b.com"); got != "" { + t.Errorf("explicit none = %q", got) + } + custom := sql.NullString{Valid: true, String: "x@y.com"} + if got := ResolveDMARCRua(custom, "a@b.com"); got != "x@y.com" { + t.Errorf("custom = %q", got) + } +} + +func TestExternalReportAuth(t *testing.T) { + name, value, ok := ExternalReportAuth("shop.com", "reports@hub.com") + if !ok || name != "_report._dmarc.hub.com" || value != "v=DMARC1;" { + t.Fatalf("external = (%q, %q, %v)", name, value, ok) + } + if _, _, ok := ExternalReportAuth("shop.com", "dmarc@shop.com"); ok { + t.Fatal("same domain should not need external auth") + } +} + // The record the panel shows and the one a failed check suggests must be the // same string, or the operator is told two different things on one page. func TestMissingRecordChecksSuggestTheShownExample(t *testing.T) { @@ -50,11 +84,11 @@ func TestMissingRecordChecksSuggestTheShownExample(t *testing.T) { t.Errorf("SPF advice %q does not suggest %q", spf.Detail, want) } - dmarc := c.checkDMARC(context.Background(), "example.com") + dmarc := c.checkDMARC(context.Background(), Query{Name: "example.com"}) if dmarc.Status != health.StatusWarn { t.Fatalf("DMARC status = %q, want warn (%s)", dmarc.Status, dmarc.Detail) } - if want := DMARCExample("example.com"); !strings.Contains(dmarc.Detail, want) { + if want := DMARCExample(""); !strings.Contains(dmarc.Detail, want) { t.Errorf("DMARC advice %q does not suggest %q", dmarc.Detail, want) } } diff --git a/internal/dnscheck/records.go b/internal/dnscheck/records.go index 6d1c9ae..fe68255 100644 --- a/internal/dnscheck/records.go +++ b/internal/dnscheck/records.go @@ -60,13 +60,15 @@ func (c *Checker) checkDKIM(ctx context.Context, q Query) Result { // checkDMARC reports whether the domain publishes a DMARC policy. DMARC is not // required for delivery, so its absence is advice (warn), not a fault. -func (c *Checker) checkDMARC(ctx context.Context, domainName string) Result { - name := DMARCRecordName(domainName) +func (c *Checker) checkDMARC(ctx context.Context, q Query) Result { + name := DMARCRecordName(q.Name) txt, found, err := c.lookupTXT(ctx, name) if err != nil { return lookupFailed("the DMARC record", err) } + example := DMARCExample(q.DMARCReportEmail) + var records []string for _, rec := range txt { if strings.HasPrefix(strings.ToLower(strings.TrimSpace(rec)), "v=dmarc1") { @@ -74,10 +76,11 @@ func (c *Checker) checkDMARC(ctx context.Context, domainName string) Result { } } if !found || len(records) == 0 { - return Result{ - Status: health.StatusWarn, - Detail: fmt.Sprintf("No DMARC record at %s. Delivery works without one, but publishing at least %q tells receivers what to do with mail that fails DKIM and gets you reports.", name, DMARCExample(domainName)), + detail := fmt.Sprintf("No DMARC record at %s. Delivery works without one, but publishing %q tells receivers what to do with mail that fails authentication.", name, example) + if q.DMARCReportEmail == "" { + detail += " Aggregate reports (rua=) are optional on a send-only relay — omit rua= unless a mailbox that receives inbound mail is configured." } + return Result{Status: health.StatusWarn, Detail: detail} } if len(records) > 1 { return Result{ @@ -102,6 +105,52 @@ func (c *Checker) checkDMARC(ctx context.Context, domainName string) Result { return Result{Status: health.StatusOK, Detail: detail, Records: records} } +// checkReportAuth verifies the hub domain publishes _report._dmarc for external +// aggregate-report destinations. Missing authorisation does not affect outbound +// delivery, only whether reports reach the rua= mailbox. +func (c *Checker) checkReportAuth(ctx context.Context, hubDomain string) Result { + name := ReportAuthRecordName(hubDomain) + expected := ReportAuthExample() + txt, found, err := c.lookupTXT(ctx, name) + if err != nil { + return lookupFailed("the DMARC report-authorisation record", err) + } + + var records []string + for _, rec := range txt { + if strings.HasPrefix(strings.ToLower(strings.TrimSpace(rec)), "v=dmarc1") { + records = append(records, rec) + } + } + if !found || len(records) == 0 { + return Result{ + Status: health.StatusWarn, + Detail: fmt.Sprintf("No report-authorisation record at %s. Aggregate DMARC reports sent to a mailbox on %s will not be delivered until %q is published there.", name, hubDomain, expected), + } + } + if len(records) > 1 { + return Result{ + Status: health.StatusError, + Detail: fmt.Sprintf("More than one DMARC report-authorisation record is published at %s. Keep exactly one.", name), + Records: records, + } + } + return Result{ + Status: health.StatusOK, + Detail: fmt.Sprintf("Published at %s — aggregate reports addressed to %s are authorised.", name, hubDomain), + Records: records, + } +} + +// ReportAuth checks whether hubDomain authorises external DMARC aggregate +// reports. It is used on the settings page for the administrator profile. +func (c *Checker) ReportAuth(ctx context.Context, hubDomain string) Result { + if hubDomain == "" { + return Result{} + } + return c.checkReportAuth(ctx, hubDomain) +} + // publicKeyTag extracts the p= (public key) tag of a DKIM record, with all // whitespace removed: DNS providers and TXT chunking freely insert spaces and // line breaks into the base64, none of which are part of the key. diff --git a/internal/domain/transfer.go b/internal/domain/transfer.go index 300ed55..206819e 100644 --- a/internal/domain/transfer.go +++ b/internal/domain/transfer.go @@ -1,6 +1,7 @@ package domain import ( + "database/sql" "fmt" "github.com/mixeme/selfpost/internal/buildinfo" @@ -23,6 +24,7 @@ type DomainExport struct { Domain string `json:"domain"` DKIMSelector string `json:"dkim_selector"` DKIMPrivateKey string `json:"dkim_private_key"` // PKCS#1 PEM + DMARCRua *string `json:"dmarc_rua,omitempty"` // nil = inherit profile; set = override ("" = none) Applications []AppExport `json:"applications"` } @@ -59,6 +61,10 @@ func (s *Service) Export(id int64) (DomainExport, error) { DKIMPrivateKey: string(pem), Applications: make([]AppExport, 0, len(apps)), } + if d.DMARCRua.Valid { + s := d.DMARCRua.String + exp.DMARCRua = &s + } for _, a := range apps { password, err := s.apps.Secret(a.Login) if err != nil { @@ -110,6 +116,14 @@ func (s *Service) Import(exp DomainExport) (store.Domain, error) { return store.Domain{}, err } + if exp.DMARCRua != nil { + if err := s.store.UpdateDomainDMARCRua(d.ID, sql.NullString{Valid: true, String: *exp.DMARCRua}); err != nil { + s.importRollback(d.ID) + return store.Domain{}, err + } + d.DMARCRua = sql.NullString{Valid: true, String: *exp.DMARCRua} + } + for _, a := range exp.Applications { if err := s.apps.ImportApplication(d.ID, a.Login, a.AddressMode, a.Addresses, a.Password); err != nil { s.importRollback(d.ID) diff --git a/internal/domain/transfer_test.go b/internal/domain/transfer_test.go index 98c8987..ddf08cf 100644 --- a/internal/domain/transfer_test.go +++ b/internal/domain/transfer_test.go @@ -1,6 +1,7 @@ package domain import ( + "database/sql" "errors" "fmt" "testing" @@ -72,6 +73,9 @@ func TestExportImportRoundTrip(t *testing.T) { if _, err := src.store.AddApplication(d.ID, "alerts", store.AddressModeList, []string{"a@example.com"}); err != nil { t.Fatalf("add alerts: %v", err) } + if err := src.store.UpdateDomainDMARCRua(d.ID, sql.NullString{Valid: true, String: "reports@hub.example"}); err != nil { + t.Fatalf("set dmarc rua: %v", err) + } exp, err := src.Export(d.ID) if err != nil { @@ -107,6 +111,12 @@ func TestExportImportRoundTrip(t *testing.T) { if got.Name != "example.com" || got.DKIMSelector != "selfpost" { t.Errorf("imported domain = %+v", got) } + if !got.DMARCRua.Valid || got.DMARCRua.String != "reports@hub.example" { + t.Errorf("imported dmarc rua = %+v", got.DMARCRua) + } + if exp.DMARCRua == nil || *exp.DMARCRua != "reports@hub.example" { + t.Errorf("exported dmarc rua = %v", exp.DMARCRua) + } // The DKIM key was imported byte-for-byte, so the DNS record is unchanged. dstKey, err := dstOdk.ExportKey("example.com", "selfpost") if err != nil { diff --git a/internal/store/admin.go b/internal/store/admin.go index 31fcf77..7b7de03 100644 --- a/internal/store/admin.go +++ b/internal/store/admin.go @@ -12,9 +12,10 @@ var ErrNoAdmin = errors.New("no administrator account") // Admin is the single panel administrator (security.md). type Admin struct { - Username string - PasswordHash string - CreatedAt time.Time + Username string + PasswordHash string + DMARCReportEmail string // default rua= for all sending domains; empty = none + CreatedAt time.Time } // AdminExists reports whether the administrator account has been created. This @@ -47,10 +48,10 @@ func (s *Store) CreateAdmin(username, passwordHash string) error { // SASL logins applications authenticate with, which live in their own table. // ErrNoAdmin is returned if setup has not happened yet, so a change can never // silently create an account. -func (s *Store) UpdateAdmin(username, passwordHash string) error { +func (s *Store) UpdateAdmin(username, passwordHash, dmarcReportEmail string) error { res, err := s.db.Exec( - "UPDATE admin SET username = ?, password_hash = ? WHERE id = 1", - username, passwordHash, + "UPDATE admin SET username = ?, password_hash = ?, dmarc_report_email = ? WHERE id = 1", + username, passwordHash, dmarcReportEmail, ) if err != nil { return fmt.Errorf("update admin: %w", err) @@ -71,8 +72,8 @@ func (s *Store) GetAdmin() (Admin, error) { a Admin createdAt string ) - err := s.db.QueryRow("SELECT username, password_hash, created_at FROM admin WHERE id = 1"). - Scan(&a.Username, &a.PasswordHash, &createdAt) + err := s.db.QueryRow("SELECT username, password_hash, dmarc_report_email, created_at FROM admin WHERE id = 1"). + Scan(&a.Username, &a.PasswordHash, &a.DMARCReportEmail, &createdAt) if errors.Is(err, sql.ErrNoRows) { return Admin{}, ErrNoAdmin } diff --git a/internal/store/admin_test.go b/internal/store/admin_test.go index c240f29..08056d9 100644 --- a/internal/store/admin_test.go +++ b/internal/store/admin_test.go @@ -11,7 +11,7 @@ func TestUpdateAdmin(t *testing.T) { if err := st.CreateAdmin("admin", "hash-one"); err != nil { t.Fatalf("CreateAdmin: %v", err) } - if err := st.UpdateAdmin("operator", "hash-two"); err != nil { + if err := st.UpdateAdmin("operator", "hash-two", "reports@hub.example"); err != nil { t.Fatalf("UpdateAdmin: %v", err) } @@ -19,7 +19,18 @@ func TestUpdateAdmin(t *testing.T) { if err != nil { t.Fatalf("GetAdmin: %v", err) } - if a.Username != "operator" || a.PasswordHash != "hash-two" { + if a.DMARCReportEmail != "reports@hub.example" { + t.Fatalf("dmarc email = %q", a.DMARCReportEmail) + } + + if err := st.UpdateAdmin("operator", "hash-three", ""); err != nil { + t.Fatalf("clear dmarc email: %v", err) + } + a, err = st.GetAdmin() + if err != nil { + t.Fatalf("GetAdmin: %v", err) + } + if a.Username != "operator" || a.PasswordHash != "hash-three" { t.Fatalf("unexpected admin after update: %+v", a) } if a.CreatedAt.IsZero() { @@ -32,7 +43,7 @@ func TestUpdateAdmin(t *testing.T) { func TestUpdateAdminWithoutAdmin(t *testing.T) { st := openTestStore(t) - if err := st.UpdateAdmin("operator", "hash"); !errors.Is(err, ErrNoAdmin) { + if err := st.UpdateAdmin("operator", "hash", ""); !errors.Is(err, ErrNoAdmin) { t.Fatalf("UpdateAdmin without admin = %v, want ErrNoAdmin", err) } exists, err := st.AdminExists() diff --git a/internal/store/domains.go b/internal/store/domains.go index a370476..2fd9872 100644 --- a/internal/store/domains.go +++ b/internal/store/domains.go @@ -24,6 +24,7 @@ type Domain struct { ID int64 Name string DKIMSelector string + DMARCRua sql.NullString // NULL = inherit profile; Valid+empty = no reports CreatedAt time.Time AppCount int } @@ -54,7 +55,7 @@ func (s *Store) AddDomain(name, selector string) (Domain, error) { // ordered by name. func (s *Store) ListDomains() ([]Domain, error) { rows, err := s.db.Query(` - SELECT d.id, d.name, d.dkim_selector, d.created_at, + SELECT d.id, d.name, d.dkim_selector, d.dmarc_rua, d.created_at, (SELECT COUNT(*) FROM applications a WHERE a.domain_id = d.id) FROM domains d ORDER BY d.name`) @@ -78,7 +79,7 @@ func (s *Store) ListDomains() ([]Domain, error) { // ErrDomainNotFound. func (s *Store) GetDomain(id int64) (Domain, error) { row := s.db.QueryRow(` - SELECT d.id, d.name, d.dkim_selector, d.created_at, + SELECT d.id, d.name, d.dkim_selector, d.dmarc_rua, d.created_at, (SELECT COUNT(*) FROM applications a WHERE a.domain_id = d.id) FROM domains d WHERE d.id = ?`, id) @@ -120,7 +121,7 @@ func scanDomain(r scanRow) (Domain, error) { d Domain createdAt string ) - if err := r.Scan(&d.ID, &d.Name, &d.DKIMSelector, &createdAt, &d.AppCount); err != nil { + if err := r.Scan(&d.ID, &d.Name, &d.DKIMSelector, &d.DMARCRua, &createdAt, &d.AppCount); err != nil { return Domain{}, err } d.CreatedAt, _ = time.Parse(time.RFC3339, createdAt) @@ -137,3 +138,21 @@ func isUniqueViolation(err error) bool { } return false } + +// UpdateDomainDMARCRua sets how this domain resolves its DMARC rua= destination. +// NULL means inherit the administrator profile; Valid with an empty string means +// policy-only with no aggregate reports for this domain. +func (s *Store) UpdateDomainDMARCRua(id int64, rua sql.NullString) error { + res, err := s.db.Exec("UPDATE domains SET dmarc_rua = ? WHERE id = ?", rua, id) + if err != nil { + return fmt.Errorf("update domain dmarc rua: %w", err) + } + n, err := res.RowsAffected() + if err != nil { + return fmt.Errorf("update domain dmarc rua rows: %w", err) + } + if n == 0 { + return ErrDomainNotFound + } + return nil +} diff --git a/internal/store/domains_dmarc_test.go b/internal/store/domains_dmarc_test.go new file mode 100644 index 0000000..e2798b1 --- /dev/null +++ b/internal/store/domains_dmarc_test.go @@ -0,0 +1,45 @@ +package store + +import ( + "database/sql" + "testing" +) + +func TestDomainDMARCRua(t *testing.T) { + st := openTestStore(t) + d, err := st.AddDomain("example.com", "sel") + if err != nil { + t.Fatalf("AddDomain: %v", err) + } + got, err := st.GetDomain(d.ID) + if err != nil { + t.Fatalf("GetDomain: %v", err) + } + if got.DMARCRua.Valid { + t.Fatal("new domain should inherit profile") + } + + if err := st.UpdateDomainDMARCRua(d.ID, sql.NullString{Valid: true, String: "reports@hub.com"}); err != nil { + t.Fatalf("UpdateDomainDMARCRua custom: %v", err) + } + got, err = st.GetDomain(d.ID) + if err != nil || got.DMARCRua.String != "reports@hub.com" { + t.Fatalf("custom = %+v, err=%v", got.DMARCRua, err) + } + + if err := st.UpdateDomainDMARCRua(d.ID, sql.NullString{Valid: true}); err != nil { + t.Fatalf("UpdateDomainDMARCRua none: %v", err) + } + got, _ = st.GetDomain(d.ID) + if !got.DMARCRua.Valid || got.DMARCRua.String != "" { + t.Fatalf("none = %+v", got.DMARCRua) + } + + if err := st.UpdateDomainDMARCRua(d.ID, sql.NullString{}); err != nil { + t.Fatalf("UpdateDomainDMARCRua inherit: %v", err) + } + got, _ = st.GetDomain(d.ID) + if got.DMARCRua.Valid { + t.Fatalf("inherit = %+v", got.DMARCRua) + } +} diff --git a/internal/store/migrations/0004_dmarc_report_email.sql b/internal/store/migrations/0004_dmarc_report_email.sql new file mode 100644 index 0000000..f36408a --- /dev/null +++ b/internal/store/migrations/0004_dmarc_report_email.sql @@ -0,0 +1,5 @@ +-- Optional DMARC aggregate-report destination (rua=) for the panel administrator +-- and per-domain overrides. NULL dmarc_rua on a domain means inherit the profile. + +ALTER TABLE admin ADD COLUMN dmarc_report_email TEXT NOT NULL DEFAULT ''; +ALTER TABLE domains ADD COLUMN dmarc_rua TEXT; diff --git a/internal/web/handlers_account.go b/internal/web/handlers_account.go index 9cbdf0a..0f5e5ed 100644 --- a/internal/web/handlers_account.go +++ b/internal/web/handlers_account.go @@ -1,10 +1,13 @@ package web import ( + "context" "errors" "net/http" "strings" + "time" + "github.com/mixeme/selfpost/internal/dnscheck" "github.com/mixeme/selfpost/internal/store" "golang.org/x/crypto/bcrypt" ) @@ -17,7 +20,13 @@ import ( func (s *Server) handleAccount(w http.ResponseWriter, r *http.Request) { switch r.Method { case http.MethodGet: - s.renderAccount(w, r, http.StatusOK, "", currentUser(r)) + admin, err := s.store.GetAdmin() + if err != nil { + logf("panel: account: get admin failed: %v", err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + s.renderAccount(w, r, http.StatusOK, "", admin.Username, admin.DMARCReportEmail) case http.MethodPost: s.submitAccount(w, r) default: @@ -26,16 +35,28 @@ func (s *Server) handleAccount(w http.ResponseWriter, r *http.Request) { } } -// renderAccount draws the settings form. formUsername repopulates the username -// field after a rejected submission; the password fields are never repopulated. -func (s *Server) renderAccount(w http.ResponseWriter, r *http.Request, status int, formErr, formUsername string) { +// renderAccount draws the settings form. formUsername and formDMARCEmail +// repopulate fields after a rejected submission; password fields are never +// repopulated. +func (s *Server) renderAccount(w http.ResponseWriter, r *http.Request, status int, formErr, formUsername, formDMARCEmail string) { + var reportAuth dnscheck.Result + if hub := dnscheck.EmailDomain(formDMARCEmail); hub != "" { + ctx, cancel := context.WithTimeout(r.Context(), 5*time.Second) + reportAuth = s.dns.ReportAuth(ctx, hub) + cancel() + } s.render(w, status, "account", map[string]any{ - "Title": "SelfPost — settings", - "User": currentUser(r), - "Active": "account", - "FormUsername": formUsername, - "Error": formErr, - "Flash": accountFlash(r), + "Title": "SelfPost — settings", + "User": currentUser(r), + "Active": "account", + "FormUsername": formUsername, + "FormDMARCEmail": formDMARCEmail, + "ReportAuthName": dnscheck.ReportAuthRecordName(dnscheck.EmailDomain(formDMARCEmail)), + "ReportAuthExample": dnscheck.ReportAuthExample(), + "ReportAuthDNS": reportAuth, + "ReportAuthHub": dnscheck.EmailDomain(formDMARCEmail), + "Error": formErr, + "Flash": accountFlash(r), }) } @@ -49,6 +70,14 @@ func accountFlash(r *http.Request) string { return "Password changed. Any other signed-in sessions were signed out." case "both": return "Username and password changed. Any other signed-in sessions were signed out." + case "email": + return "DMARC report address updated." + case "username-email": + return "Username and DMARC report address updated." + case "password-email": + return "Password and DMARC report address updated. Any other signed-in sessions were signed out." + case "all": + return "Settings updated. Any other signed-in sessions were signed out." default: return "" } @@ -62,11 +91,11 @@ func accountFlash(r *http.Request) string { func (s *Server) submitAccount(w http.ResponseWriter, r *http.Request) { if !s.loginLimiter.Allow(clientIP(r, s.trustedProxies)) { s.renderAccount(w, r, http.StatusTooManyRequests, - "Too many attempts. Please wait and try again.", currentUser(r)) + "Too many attempts. Please wait and try again.", currentUser(r), "") return } if err := r.ParseForm(); err != nil { - s.renderAccount(w, r, http.StatusBadRequest, "Invalid form submission.", currentUser(r)) + s.renderAccount(w, r, http.StatusBadRequest, "Invalid form submission.", currentUser(r), "") return } @@ -74,6 +103,7 @@ func (s *Server) submitAccount(w http.ResponseWriter, r *http.Request) { current := r.PostFormValue("current_password") password := r.PostFormValue("new_password") confirm := r.PostFormValue("new_password_confirm") + dmarcEmail := strings.TrimSpace(r.PostFormValue("dmarc_report_email")) admin, err := s.store.GetAdmin() if err != nil { @@ -86,34 +116,39 @@ func (s *Server) submitAccount(w http.ResponseWriter, r *http.Request) { } if err := bcrypt.CompareHashAndPassword([]byte(admin.PasswordHash), []byte(current)); err != nil { - s.renderAccount(w, r, http.StatusUnauthorized, "Current password is incorrect.", username) + s.renderAccount(w, r, http.StatusUnauthorized, "Current password is incorrect.", username, dmarcEmail) return } renaming := username != admin.Username if renaming { if err := validateUsername(username); err != nil { - s.renderAccount(w, r, http.StatusBadRequest, err.Error(), username) + s.renderAccount(w, r, http.StatusBadRequest, err.Error(), username, dmarcEmail) return } } - // An empty pair of new-password fields means "leave the password alone", so - // the username can be changed on its own. + if err := validateEmail(dmarcEmail); err != nil { + s.renderAccount(w, r, http.StatusBadRequest, err.Error(), username, dmarcEmail) + return + } + + emailChanging := dmarcEmail != admin.DMARCReportEmail + repassword := password != "" || confirm != "" if repassword { if password != confirm { - s.renderAccount(w, r, http.StatusBadRequest, "New passwords do not match.", username) + s.renderAccount(w, r, http.StatusBadRequest, "New passwords do not match.", username, dmarcEmail) return } if err := validateAdminPassword(password); err != nil { - s.renderAccount(w, r, http.StatusBadRequest, err.Error(), username) + s.renderAccount(w, r, http.StatusBadRequest, err.Error(), username, dmarcEmail) return } } - if !renaming && !repassword { + if !renaming && !repassword && !emailChanging { s.renderAccount(w, r, http.StatusBadRequest, - "Nothing to change: enter a new username, a new password, or both.", username) + "Nothing to change: enter a new username, password, or DMARC report address.", username, dmarcEmail) return } @@ -123,25 +158,22 @@ func (s *Server) submitAccount(w http.ResponseWriter, r *http.Request) { if err != nil { logf("panel: account: hashing password failed: %v", err) s.renderAccount(w, r, http.StatusInternalServerError, - "Internal error. Please try again.", username) + "Internal error. Please try again.", username, dmarcEmail) return } hash = string(newHash) } - if err := s.store.UpdateAdmin(username, hash); err != nil { + if err := s.store.UpdateAdmin(username, hash, dmarcEmail); err != nil { logf("panel: account: update admin failed: %v", err) msg := "Could not save the changes. Please check the logs and try again." if errors.Is(err, store.ErrNoAdmin) { msg = "There is no administrator account to update." } - s.renderAccount(w, r, http.StatusInternalServerError, msg, username) + s.renderAccount(w, r, http.StatusInternalServerError, msg, username, dmarcEmail) return } - // Keep this session usable under the new name, and — when the password - // changed — drop every other session so a cookie captured under the old - // password stops working. if token, ok := s.sessionToken(r); ok { if renaming { s.sessions.Rename(token, username) @@ -151,18 +183,25 @@ func (s *Server) submitAccount(w http.ResponseWriter, r *http.Request) { } } - logf("panel: administrator account updated (username changed: %t, password changed: %t)", renaming, repassword) - http.Redirect(w, r, "/account?updated="+updatedFlag(renaming, repassword), http.StatusSeeOther) + logf("panel: administrator account updated (username: %t, password: %t, dmarc email: %t)", renaming, repassword, emailChanging) + http.Redirect(w, r, "/account?updated="+updatedFlag(renaming, repassword, emailChanging), http.StatusSeeOther) } -// updatedFlag names what changed, for the fixed post-redirect flash message. -func updatedFlag(renamed, repassword bool) string { +func updatedFlag(renamed, repassword, emailChanged bool) string { switch { + case renamed && repassword && emailChanged: + return "all" + case renamed && emailChanged: + return "username-email" + case repassword && emailChanged: + return "password-email" case renamed && repassword: return "both" case renamed: return "username" - default: + case repassword: return "password" + default: + return "email" } } diff --git a/internal/web/handlers_apps.go b/internal/web/handlers_apps.go index 0da9824..aa99997 100644 --- a/internal/web/handlers_apps.go +++ b/internal/web/handlers_apps.go @@ -102,7 +102,35 @@ func (s *Server) renderDomainDetail(w http.ResponseWriter, r *http.Request, stat // What DNS actually publishes for the domain today, checked against the key // this server signs with. Cached by the checker, so re-rendering the page // after a form post costs nothing. - dns, srv := s.domainDNS(d, record, false) + admin, err := s.store.GetAdmin() + if err != nil { + logf("panel: domain %d: get admin: %v", d.ID, err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + profileEmail := admin.DMARCReportEmail + reportEmail := dnscheck.ResolveDMARCRua(d.DMARCRua, profileEmail) + dns, srv := s.domainDNS(d, record, profileEmail, false) + reportAuthName, reportAuthValue, needsReportAuth := dnscheck.ExternalReportAuth(d.Name, reportEmail) + dmarcMode := "inherit" + dmarcCustom := "" + if d.DMARCRua.Valid { + if d.DMARCRua.String == "" { + dmarcMode = "none" + } else { + dmarcMode = "custom" + dmarcCustom = d.DMARCRua.String + } + } + dmarcSource := "policy" + switch { + case dmarcMode == "custom": + dmarcSource = "custom" + case dmarcMode == "none": + dmarcSource = "none" + case profileEmail != "": + dmarcSource = "settings" + } s.render(w, status, "domain_detail", map[string]any{ "Title": "SelfPost — " + d.Name, @@ -118,7 +146,16 @@ func (s *Server) renderDomainDetail(w http.ResponseWriter, r *http.Request, stat // page and the checks below it never recommend different records. "SPFExample": dnscheck.SPFExample(s.cfg.Hostname, srv.IPs), "DMARCName": dnscheck.DMARCRecordName(d.Name), - "DMARCExample": dnscheck.DMARCExample(d.Name), + "DMARCExample": dnscheck.DMARCExample(reportEmail), + "DMARCSource": dmarcSource, + "ProfileDMARCEmail": profileEmail, + "ResolvedDMARCEmail": reportEmail, + "DMARCRuaMode": dmarcMode, + "DMARCRuaCustom": dmarcCustom, + "ReportAuthName": reportAuthName, + "ReportAuthValue": reportAuthValue, + "NeedsReportAuth": needsReportAuth, + "SameDomainRUA": reportEmail != "" && strings.EqualFold(dnscheck.EmailDomain(reportEmail), d.Name), // Client connection settings (the same for every domain on this // instance): the hostname clients connect to, and whether the optional // submission listener is enabled in this deployment. @@ -149,14 +186,15 @@ func (s *Server) renderDomainDetail(w http.ResponseWriter, r *http.Request, stat // and no extra environment variable is needed. That server result is returned // alongside, because the page's suggested SPF record is built from the same // addresses. force bypasses the cache, for the Re-check button. -func (s *Server) domainDNS(d store.Domain, record domain.DKIMRecord, force bool) (dnscheck.Domain, dnscheck.Server) { +func (s *Server) domainDNS(d store.Domain, record domain.DKIMRecord, profileEmail string, force bool) (dnscheck.Domain, dnscheck.Server) { srv := s.dns.Server(s.cfg.Hostname, false) return s.dns.Domain(dnscheck.Query{ - Name: d.Name, - Selector: d.DKIMSelector, - ExpectedDKIM: record.Value, - Hostname: srv.Hostname, - ServerIPs: srv.IPs, + Name: d.Name, + Selector: d.DKIMSelector, + ExpectedDKIM: record.Value, + Hostname: srv.Hostname, + ServerIPs: srv.IPs, + DMARCReportEmail: dnscheck.ResolveDMARCRua(d.DMARCRua, profileEmail), }, force), srv } @@ -173,7 +211,13 @@ func (s *Server) handleDomainDNSRecheck(w http.ResponseWriter, r *http.Request) http.Error(w, "internal error", http.StatusInternalServerError) return } - s.domainDNS(d, record, true) + admin, err := s.store.GetAdmin() + if err != nil { + logf("panel: domain %d: get admin: %v", d.ID, err) + http.Error(w, "internal error", http.StatusInternalServerError) + return + } + s.domainDNS(d, record, admin.DMARCReportEmail, true) http.Redirect(w, r, fmt.Sprintf("/domains/%d?rechecked=1", d.ID), http.StatusSeeOther) } @@ -205,6 +249,8 @@ func detailFlash(r *http.Request) string { return "Application address mode updated." case r.URL.Query().Get("ratelimit") != "": return "Rate limit updated." + case r.URL.Query().Get("dmarc") != "": + return "DMARC report settings updated." case r.URL.Query().Get("imported") != "": return "Domain imported. Its DKIM DNS record is unchanged — no DNS update is needed." case r.URL.Query().Get("rechecked") != "": diff --git a/internal/web/handlers_dmarc.go b/internal/web/handlers_dmarc.go new file mode 100644 index 0000000..938bf15 --- /dev/null +++ b/internal/web/handlers_dmarc.go @@ -0,0 +1,56 @@ +package web + +import ( + "database/sql" + "fmt" + "net/http" + "strings" + +) + +// handleDomainDMARC saves per-domain DMARC rua= settings. +func (s *Server) handleDomainDMARC(w http.ResponseWriter, r *http.Request) { + if r.Method != http.MethodPost { + w.Header().Set("Allow", "POST") + http.Error(w, "method not allowed", http.StatusMethodNotAllowed) + return + } + d, ok := s.lookupDomain(w, r) + if !ok { + return + } + if err := r.ParseForm(); err != nil { + s.renderDomainDetail(w, r, http.StatusBadRequest, d, detailView{FormErr: "Invalid form submission."}) + return + } + + var rua sql.NullString + switch strings.TrimSpace(r.PostFormValue("dmarc_rua_mode")) { + case "inherit": + rua = sql.NullString{} + case "none": + rua = sql.NullString{Valid: true, String: ""} + case "custom": + email := strings.TrimSpace(r.PostFormValue("dmarc_rua_email")) + if err := validateEmail(email); err != nil { + s.renderDomainDetail(w, r, http.StatusBadRequest, d, detailView{FormErr: err.Error()}) + return + } + if email == "" { + s.renderDomainDetail(w, r, http.StatusBadRequest, d, detailView{FormErr: "Enter a custom report address or choose another mode."}) + return + } + rua = sql.NullString{Valid: true, String: email} + default: + s.renderDomainDetail(w, r, http.StatusBadRequest, d, detailView{FormErr: "Choose how aggregate reports are addressed for this domain."}) + return + } + + if err := s.store.UpdateDomainDMARCRua(d.ID, rua); err != nil { + logf("panel: domain %d: save dmarc rua: %v", d.ID, err) + s.renderDomainDetail(w, r, http.StatusInternalServerError, d, detailView{FormErr: "Could not save DMARC settings. Please check the logs and try again."}) + return + } + s.dns.Forget(d.Name) + http.Redirect(w, r, fmt.Sprintf("/domains/%d?dmarc=1", d.ID), http.StatusSeeOther) +} diff --git a/internal/web/handlers_domains.go b/internal/web/handlers_domains.go index 767e4fa..9acd6e4 100644 --- a/internal/web/handlers_domains.go +++ b/internal/web/handlers_domains.go @@ -54,6 +54,10 @@ func (s *Server) renderDashboard(w http.ResponseWriter, r *http.Request, status // so a repeat view of the list costs no lookups at all, and it is the same // cache the domain page fills — opening a domain after the list is free. func (s *Server) domainRows(domains []store.Domain) []domainRow { + profileEmail := "" + if admin, err := s.store.GetAdmin(); err == nil { + profileEmail = admin.DMARCReportEmail + } rows := make([]domainRow, len(domains)) var wg sync.WaitGroup for i, d := range domains { @@ -69,7 +73,7 @@ func (s *Server) domainRows(domains []store.Domain) []domainRow { logf("panel: dashboard: domain %d: dkim record: %v", d.ID, err) return } - dns, _ := s.domainDNS(d, record, false) + dns, _ := s.domainDNS(d, record, profileEmail, false) rows[i].DNS = dns.Overall }() } diff --git a/internal/web/templates/account.html b/internal/web/templates/account.html index 89a5162..e515568 100644 --- a/internal/web/templates/account.html +++ b/internal/web/templates/account.html @@ -23,10 +23,46 @@ +

DMARC aggregate reports

+

Optional default rua= address for every sending + domain (can be overridden per domain). Use a mailbox on a domain that + receives inbound mail. SelfPost is send-only today; a future release will + be able to receive reports in the panel itself.

+ + + + + {{if .FormDMARCEmail}} +

When rua= points at another domain, that hub + domain must publish a report-authorisation record so receivers will deliver + the XML aggregates.

+ + +
+ {{.ReportAuthName}} + +
+ + +
+ {{.ReportAuthExample}} + +
+ + {{if .ReportAuthDNS.Status}} + +

{{.ReportAuthDNS.Detail}}

+ {{if .ReportAuthDNS.Records}}{{range .ReportAuthDNS.Records}}{{.}} +{{end}}{{end}} + {{end}} + {{end}} +

Leave both new-password fields empty to change the username - only. Changing the password signs out every other session; this one stays - signed in.

+ or DMARC address only. Changing the password signs out every other session; + this one stays signed in.

{{end}} diff --git a/internal/web/templates/domain_detail.html b/internal/web/templates/domain_detail.html index 93a7c0a..f9ec373 100644 --- a/internal/web/templates/domain_detail.html +++ b/internal/web/templates/domain_detail.html @@ -51,8 +51,10 @@

SPF and DMARC records

These two are not generated the way the DKIM record above is — they are policy, and the domain may already publish an SPF record for other - senders. What follows is what this server expects to see and what the checks - below look for. Publish both as TXT records.

+ senders. SelfPost is a send-only relay — most operators have no inbox on the + sending domain, so the suggested DMARC record omits rua= unless a + report address is configured below or in Settings. + Publish both as TXT records.

@@ -68,9 +70,23 @@

A domain may have only one SPF record. If it already has one, do not add a second — add this server's mechanism to the existing record - instead, before its all term. -all at the end means - nothing else may send as the domain; use ~all while other senders - are still being sorted out.

+ instead, before its all term.

+ +
+ + + + + + + +
@@ -78,17 +94,36 @@
- +
{{.DMARCExample}}
-

Point rua= at a mailbox that is actually read — - the aggregate reports are how you find out who else sends as the domain. - p=none changes nothing about delivery, so it is safe to publish - straight away; tighten it to p=quarantine and then - p=reject once the reports come back clean.

+ {{if .SameDomainRUA}} +

The report address is on this sending domain. SelfPost does + not receive inbound mail — use a mailbox elsewhere or wait for in-panel report + reception in a future release.

+ {{end}} + + {{if .NeedsReportAuth}} + +
+ {{.ReportAuthName}} + +
+ + +
+ {{.ReportAuthValue}} + +
+ {{end}} + +

p=none changes nothing about delivery. Omit + rua= on a send-only domain, or point it at a mailbox that + receives inbound mail. Tighten to p=quarantine and then + p=reject once aggregate reports look clean.

@@ -116,6 +151,13 @@ {{if .DNS.DMARC.Records}}{{range .DNS.DMARC.Records}}{{.}} {{end}}{{end}} + {{if .DNS.DMARCReportAuth.Status}} + +

{{.DNS.DMARCReportAuth.Detail}}

+ {{if .DNS.DMARCReportAuth.Records}}{{range .DNS.DMARCReportAuth.Records}}{{.}} +{{end}}{{end}} + {{end}} +
diff --git a/internal/web/validate.go b/internal/web/validate.go index a53483b..4951197 100644 --- a/internal/web/validate.go +++ b/internal/web/validate.go @@ -105,3 +105,47 @@ func validateDomainLabel(label string) error { } return nil } + +// freemailDomains lists public mail hosts that cannot publish _report._dmarc +// authorisation for third-party sending domains. +var freemailDomains = map[string]struct{}{ + "gmail.com": {}, + "googlemail.com": {}, + "outlook.com": {}, + "hotmail.com": {}, + "live.com": {}, + "yahoo.com": {}, + "icloud.com": {}, + "me.com": {}, + "proton.me": {}, + "protonmail.com": {}, +} + +// validateEmail checks a DMARC rua= mailbox. Empty is allowed (policy-only). +func validateEmail(addr string) error { + addr = strings.TrimSpace(addr) + if addr == "" { + return nil + } + at := strings.LastIndex(addr, "@") + if at <= 0 || at >= len(addr)-1 { + return fmt.Errorf("enter a valid email address") + } + local := addr[:at] + domain := normalizeDomain(addr[at+1:]) + if err := validateDomain(domain); err != nil { + return fmt.Errorf("email domain is invalid: %w", err) + } + for _, r := range local { + if r > unicode.MaxASCII { + return fmt.Errorf("email address must be ASCII") + } + if !isASCIILetterOrDigit(r) && r != '.' && r != '-' && r != '_' && r != '+' { + return fmt.Errorf("email address contains invalid characters") + } + } + if _, blocked := freemailDomains[domain]; blocked { + return fmt.Errorf("use an address on a domain you control; public mail hosts cannot receive authorised DMARC reports") + } + return nil +} diff --git a/internal/web/validate_email_test.go b/internal/web/validate_email_test.go new file mode 100644 index 0000000..46114fe --- /dev/null +++ b/internal/web/validate_email_test.go @@ -0,0 +1,18 @@ +package web + +import "testing" + +func TestValidateEmail(t *testing.T) { + if err := validateEmail(""); err != nil { + t.Errorf("empty: %v", err) + } + if err := validateEmail("reports@mail.example.com"); err != nil { + t.Errorf("valid: %v", err) + } + if err := validateEmail("bad"); err == nil { + t.Error("bad address accepted") + } + if err := validateEmail("x@gmail.com"); err == nil { + t.Error("gmail accepted") + } +} diff --git a/internal/web/web.go b/internal/web/web.go index 3ba6dda..003c6ea 100644 --- a/internal/web/web.go +++ b/internal/web/web.go @@ -1,4 +1,4 @@ -// Package web implements the SelfPost control panel's HTTP surface: the +// Package web implements the SelfPost control panel's HTTP surface: the // one-time administrator setup flow (security.md), login/session handling // (security.md) and the authenticated shell the later phases build on. package web @@ -24,7 +24,7 @@ var assetsFS embed.FS // Config holds the panel's HTTP-facing configuration. type Config struct { // Hostname is the server's external hostname, used to build the absolute - // setup link shown in the logs (security.md; guide § Environment + // setup link shown in the logs (security.md; guide В§ Environment // variables for SELFPOST_HOSTNAME). Hostname string // CookieSecure sets the Secure attribute on the session cookie. It defaults @@ -33,16 +33,16 @@ type Config struct { CookieSecure bool // SubmissionEnabled mirrors SUBMISSION_ENABLE: whether this deployment also // runs the 587/STARTTLS submission listener next to the primary 465 one - // (architecture.md § Mail path). The panel only reports it on the domain + // (architecture.md В§ Mail path). The panel only reports it on the domain // page's connection settings; it is a deploy-time flag, not something the // panel can verify. SubmissionEnabled bool // MailLogPath is where Postfix's delivery log lives, read by the mail.log - // monitoring view (architecture.md § Panel HTTP surface). It is the same path + // monitoring view (architecture.md В§ Panel HTTP surface). It is the same path // the log-tailer role follows in cmd/panel. MailLogPath string // DataDir and DBPath locate the persistent state a full backup archives - // (architecture.md § Persistence); Version is stamped into the backup + // (architecture.md В§ Persistence); Version is stamped into the backup // manifest. They mirror the panel's own configuration. DataDir string DBPath string @@ -53,7 +53,7 @@ type Config struct { // honoured, so the header can't be spoofed by anyone but a trusted proxy. // Empty (the default) keeps rate-limiting keyed on RemoteAddr only. TrustedProxyCIDRs []*net.IPNet - // TLSCertFile is the certificate Postfix serves on 465/587 (guide § + // TLSCertFile is the certificate Postfix serves on 465/587 (guide В§ // Environment variables), read read-only by the status page to report how // much validity is left. TLSCertFile string @@ -69,8 +69,8 @@ type Config struct { SessionIdleDays int // DNSResolvers are the recursive resolvers the deliverability checks query // (env SELFPOST_DNS_RESOLVERS). Empty uses dnscheck.DefaultResolvers. The - // checks must not go through the system resolver — see dnscheck's - // externalResolver — so this is how a closed network points them at its own. + // checks must not go through the system resolver — see dnscheck's + // externalResolver — so this is how a closed network points them at its own. DNSResolvers []string } @@ -98,9 +98,9 @@ type Server struct { // New builds the panel server. setupTokenPath is where the current setup token // is mirrored on disk (security.md); domains is the sending-domain service -// that owns DKIM keys and the OpenDKIM tables (architecture.md § OpenDKIM); +// that owns DKIM keys and the OpenDKIM tables (architecture.md В§ OpenDKIM); // apps owns application SASL accounts and the Postfix sender map -// (architecture.md § Mail path). +// (architecture.md В§ Mail path). func New(st *store.Store, domains *domain.Service, apps *app.Service, cfg Config, setupTokenPath string) (*Server, error) { tmpl, err := loadTemplates() if err != nil { @@ -183,6 +183,7 @@ func (s *Server) Handler() http.Handler { authed.HandleFunc("POST /domains/{id}/delete", s.handleDeleteDomain) authed.HandleFunc("POST /domains/{id}/applications", s.handleAddApplication) authed.HandleFunc("POST /domains/{id}/ratelimit", s.handleDomainRateLimit) + authed.HandleFunc("POST /domains/{id}/dmarc", s.handleDomainDMARC) authed.HandleFunc("POST /domains/{id}/export", s.handleExportDomain) authed.HandleFunc("POST /applications/{aid}/mode", s.handleUpdateAppMode) authed.HandleFunc("POST /applications/{aid}/password", s.handleRegenPassword) @@ -193,13 +194,13 @@ func (s *Server) Handler() http.Handler { // Administrator's own panel credentials. authed.HandleFunc("/account", s.handleAccount) - // Backup and migration: the page with both actions (architecture.md § + // Backup and migration: the page with both actions (architecture.md В§ // Persistence-B), and the full-server backup download itself. authed.HandleFunc("GET /backup", s.handleBackupPage) authed.HandleFunc("POST /backup", s.handleBackup) - // Monitoring screens (architecture.md § Panel HTTP surface): each page and - // its HTMX polling fragment (architecture.md § Panel HTTP surface — the /rows + // Monitoring screens (architecture.md В§ Panel HTTP surface): each page and + // its HTMX polling fragment (architecture.md В§ Panel HTTP surface — the /rows // and /body endpoints return HTML, not JSON). authed.HandleFunc("GET /deliveries", s.handleDeliveries) authed.HandleFunc("GET /deliveries/rows", s.handleDeliveriesRows) @@ -234,7 +235,7 @@ func handleHealth(w http.ResponseWriter, _ *http.Request) { // clientIP extracts the peer IP for rate-limiting. By default it is the // transport peer (RemoteAddr), which cannot be spoofed. If RemoteAddr matches -// one of trustedProxies, the last entry of X-Forwarded-For is used instead — +// one of trustedProxies, the last entry of X-Forwarded-For is used instead — // that is the address the trusted proxy itself appended, so a client can't // forge it by sending its own XFF header. With no trusted // proxies configured, behind a reverse proxy this is the proxy's own address, @@ -272,3 +273,4 @@ func ipInAny(ip net.IP, nets []*net.IPNet) bool { func logf(format string, args ...any) { log.Printf(format, args...) } +