diff --git a/CHANGELOG.md b/CHANGELOG.md index 8223b03..e0fc018 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,25 @@ Format follows [Keep a Changelog](https://keepachangelog.com/en/1.1.0/); version ## [Unreleased] +- panel: the domain page now shows the **SPF and DMARC records it expects**, + with host, value and a Copy button, next to the DKIM record it already + showed — previously it only said "also configure SPF and DMARC (see the + documentation)" and the concrete example appeared only once a check had + already failed. The SPF value names the addresses this server's hostname + resolves to (falling back to an `a:` mechanism if it does not resolve), and + the DNS checks below build their remediation advice from the same source, so + the page and its checks cannot recommend different records. +- panel: one appearance for actions. Several controls — a POST wrapped in an + inline form (Re-check, Export domain, Sign out, New password…), the + `
` toggles in the applications table, the delete links — used to + render as bold blue text while everything else was a button, so the same + kind of control looked like two different things, sometimes within one card. + They are all buttons now: filled for a card's own action, compact and + outlined where actions cluster in a table row or the nav bar. An `` is + once again only used for navigation. +- panel: on the domain page, **Applications** and **Add an application** sit + side by side on a wide viewport instead of the form being stranded below the + domain rate limit; they stack again on a narrow one. - ci: hermetic container e2e suite (`test/e2e`, a separate Go module) gates image publishing — `make e2e` locally, and `go test ./...` in `test/e2e` as a required step in `release.yml` before a version tag's image is pushed. diff --git a/internal/dnscheck/dnscheck.go b/internal/dnscheck/dnscheck.go index f0775c2..4fb4184 100644 --- a/internal/dnscheck/dnscheck.go +++ b/internal/dnscheck/dnscheck.go @@ -66,12 +66,13 @@ type Domain struct { // Query describes the domain to check. ExpectedDKIM is the TXT value the panel // tells the operator to publish (domain.DKIMRecord.Value), so the check -// compares DNS against the key this server actually signs with. ServerIPs comes -// from a preceding Server check. +// 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 } diff --git a/internal/dnscheck/examples.go b/internal/dnscheck/examples.go new file mode 100644 index 0000000..2eb3463 --- /dev/null +++ b/internal/dnscheck/examples.go @@ -0,0 +1,49 @@ +package dnscheck + +import ( + "net" + "strings" +) + +// SPF and DMARC are the two records SelfPost cannot generate for the operator: +// unlike the DKIM record they are policy, not a key, and a domain may already +// publish one for other senders. The panel still has to say what "correct" +// looks like, and the checks below have to suggest the same thing when a record +// is missing — so both take their example from here rather than each spelling +// out its own. + +// DMARCRecordName is the name a DMARC record is published at. (SPF has no such +// helper: it is published at the domain itself.) +func DMARCRecordName(domainName string) string { return "_dmarc." + domainName } + +// 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 +// does not resolve) it falls back to an "a:" mechanism naming the host, so the +// panel always has something concrete to show. +func SPFExample(hostname string, serverIPs []string) string { + var mechanisms []string + for _, s := range serverIPs { + ip := net.ParseIP(strings.TrimSpace(s)) + switch { + case ip == nil: + continue + case ip.To4() != nil: + mechanisms = append(mechanisms, "ip4:"+ip.String()) + default: + mechanisms = append(mechanisms, "ip6:"+ip.String()) + } + } + if len(mechanisms) == 0 { + mechanisms = []string{"a:" + hostname} + } + 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 +} diff --git a/internal/dnscheck/examples_test.go b/internal/dnscheck/examples_test.go new file mode 100644 index 0000000..6cc03d4 --- /dev/null +++ b/internal/dnscheck/examples_test.go @@ -0,0 +1,60 @@ +package dnscheck + +import ( + "context" + "strings" + "testing" + + "codeberg.org/mix/selfpost/internal/health" +) + +func TestSPFExample(t *testing.T) { + cases := []struct { + name string + hostname string + ips []string + want string + }{ + {"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"}, + } + for _, c := range cases { + t.Run(c.name, func(t *testing.T) { + if got := SPFExample(c.hostname, c.ips); got != c.want { + t.Errorf("SPFExample = %q, want %q", got, c.want) + } + }) + } +} + +// 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) { + f := &fakeResolver{} + c := newTestChecker(f) + + spf := c.checkSPF(context.Background(), Query{ + Name: "example.com", + Hostname: "mail.example.com", + ServerIPs: []string{"203.0.113.10"}, + }) + if spf.Status != health.StatusError { + t.Fatalf("SPF status = %q, want error (%s)", spf.Status, spf.Detail) + } + if want := SPFExample("mail.example.com", []string{"203.0.113.10"}); !strings.Contains(spf.Detail, want) { + t.Errorf("SPF advice %q does not suggest %q", spf.Detail, want) + } + + dmarc := c.checkDMARC(context.Background(), "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) { + 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 bd15ba8..b9f666b 100644 --- a/internal/dnscheck/records.go +++ b/internal/dnscheck/records.go @@ -61,7 +61,7 @@ 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 := "_dmarc." + domainName + name := DMARCRecordName(domainName) txt, found, err := c.lookupTXT(ctx, name) if err != nil { return lookupFailed("the DMARC record", err) @@ -76,7 +76,7 @@ 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 \"v=DMARC1; p=none; rua=mailto:you@%s\" tells receivers what to do with mail that fails DKIM and gets you reports.", name, domainName), + 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)), } } if len(records) > 1 { diff --git a/internal/dnscheck/spf.go b/internal/dnscheck/spf.go index 9215720..71412fd 100644 --- a/internal/dnscheck/spf.go +++ b/internal/dnscheck/spf.go @@ -47,7 +47,7 @@ func (c *Checker) checkSPF(ctx context.Context, q Query) Result { case !found || len(records) == 0: return Result{ Status: health.StatusError, - Detail: fmt.Sprintf("No SPF record is published for %s. Publish a TXT record such as \"v=spf1 ip4:%s -all\" — without it receivers have nothing authorising this server to send as the domain.", q.Name, ips[0]), + Detail: fmt.Sprintf("No SPF record is published for %s. Publish a TXT record such as %q — without it receivers have nothing authorising this server to send as the domain.", q.Name, SPFExample(q.Hostname, q.ServerIPs)), } case len(records) > 1: return Result{ diff --git a/internal/web/handlers_apps.go b/internal/web/handlers_apps.go index cb690fd..c469aca 100644 --- a/internal/web/handlers_apps.go +++ b/internal/web/handlers_apps.go @@ -96,16 +96,26 @@ func (s *Server) renderDomainDetail(w http.ResponseWriter, r *http.Request, stat return } + // What DNS actually publishes for the domain today, checked against the key + // this server signs with (phase 13.B). Cached by the checker, so + // re-rendering the page after a form post costs nothing. + dns, srv := s.domainDNS(d, record, false) + s.render(w, status, "domain_detail", map[string]any{ "Title": "SelfPost — " + d.Name, "User": currentUser(r), "Active": "domains", "Domain": d, "Record": record, - // What DNS actually publishes for the domain today, checked against the - // key this server signs with (phase 13.B). Cached by the checker, so - // re-rendering the page after a form post costs nothing. - "DNS": s.domainDNS(d, record, false), + "DNS": dns, + // SPF and DMARC are the operator's to write — SelfPost cannot generate + // them the way it generates the DKIM record — so the page shows what + // this server expects rather than leaving it to the documentation. The + // same builders phrase the suggestions in the check messages, so the + // 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), // 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. @@ -131,16 +141,18 @@ func (s *Server) renderDomainDetail(w http.ResponseWriter, r *http.Request, stat // domainDNS resolves what the world sees for a domain: its DKIM, SPF and DMARC // records (phase 13.B). The server's own address comes from the (separately // cached) hostname check, so the SPF heuristic knows which IP it is looking for -// and no extra environment variable is needed. force bypasses the cache, for the -// Re-check button. -func (s *Server) domainDNS(d store.Domain, record domain.DKIMRecord, force bool) dnscheck.Domain { +// 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) { 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, - }, force) + }, force), srv } // handleDomainDNSRecheck re-runs the domain's DNS checks ignoring the cache and diff --git a/internal/web/templates/domain_detail.html b/internal/web/templates/domain_detail.html index 444de74..684b1b4 100644 --- a/internal/web/templates/domain_detail.html +++ b/internal/web/templates/domain_detail.html @@ -44,8 +44,51 @@ -

Also configure SPF and DMARC for the domain (see the - documentation). Mail is signed with selector {{.Domain.DKIMSelector}}.

+

Mail is signed with selector {{.Domain.DKIMSelector}}.

+ + +
+

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.

+ + +
+ {{.Domain.Name}} + +
+ + +
+ {{.SPFExample}} + +
+ +

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.

+ + +
+ {{.DMARCName}} + +
+ + +
+ {{.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.