From 7e54280a9d9df32db894ff11b45fcaeb7c1d24da Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Fri, 28 Jun 2024 14:50:39 +0200 Subject: [PATCH] show the same spf record for a domain in the dnsrecords and dnscheck output/pages before, the suggested records would show "v=spf1 mx ~all", while the dnscheck page would suggest "v=spf1 ip4:... ip6:... -all". the two places now show the same record: explicitly listing the configured ip's (so the common case of a valid message is fast and doesn't require lookups of mx hosts and their addresses), but still including "mx" (may prevent issues while migrating to new ips in the future and doesn't hurt for legit messages), and "~all" (for compatibility with some old systems that don't look at dkim/dmarc when they evaluate spf and reach "-all") based on #176 created by rdelaage, with record mismatch spotted by RobSlgm, thanks! --- mox-/admin.go | 44 ++++++++++++++++++++++++++- testdata/integration/moxacmepebble.sh | 2 +- webadmin/admin.go | 42 ++++++++++++------------- 3 files changed, 63 insertions(+), 25 deletions(-) diff --git a/mox-/admin.go b/mox-/admin.go index 4d0012b..f45349f 100644 --- a/mox-/admin.go +++ b/mox-/admin.go @@ -34,6 +34,7 @@ import ( "github.com/mjl-/mox/mlog" "github.com/mjl-/mox/mtasts" "github.com/mjl-/mox/smtp" + "github.com/mjl-/mox/spf" "github.com/mjl-/mox/tlsrpt" ) @@ -669,6 +670,31 @@ func ConfigSave(ctx context.Context, xmodify func(config *config.Dynamic)) (rerr return nil } +// DomainSPFIPs returns IPs to include in SPF records for domains. It includes the +// IPs on listeners that have SMTP enabled, and includes IPs configured for SOCKS +// transports. +func DomainSPFIPs() (ips []net.IP) { + for _, l := range Conf.Static.Listeners { + if !l.SMTP.Enabled || l.IPsNATed { + continue + } + ipstrs := l.IPs + if len(l.NATIPs) > 0 { + ipstrs = l.NATIPs + } + for _, ipstr := range ipstrs { + ip := net.ParseIP(ipstr) + ips = append(ips, ip) + } + } + for _, t := range Conf.Static.Transports { + if t.Socks != nil { + ips = append(ips, t.Socks.IPs...) + } + } + return ips +} + // todo: find a way to automatically create the dns records as it would greatly simplify setting up email for a domain. we could also dynamically make changes, e.g. providing grace periods after disabling a dkim key, only automatically removing the dkim dns key after a few days. but this requires some kind of api and authentication to the dns server. there doesn't appear to be a single commonly used api for dns management. each of the numerous cloud providers have their own APIs and rather large SKDs to use them. we don't want to link all of them in. // DomainRecords returns text lines describing DNS records required for configuring @@ -823,13 +849,29 @@ func DomainRecords(domConf config.Domain, domain dns.Domain, hasDNSSEC bool, cer {Address: uri.String(), MaxSize: 10, Unit: "m"}, } } + dspfr := spf.Record{Version: "spf1"} + for _, ip := range DomainSPFIPs() { + mech := "ip4" + if ip.To4() == nil { + mech = "ip6" + } + dspfr.Directives = append(dspfr.Directives, spf.Directive{Mechanism: mech, IP: ip}) + } + dspfr.Directives = append(dspfr.Directives, + spf.Directive{Mechanism: "mx"}, + spf.Directive{Qualifier: "~", Mechanism: "all"}, + ) + dspftxt, err := dspfr.Record() + if err != nil { + return nil, fmt.Errorf("making domain spf record: %v", err) + } records = append(records, "", "; Specify the MX host is allowed to send for our domain and for itself (for DSNs).", "; ~all means softfail for anything else, which is done instead of -all to prevent older", "; mail servers from rejecting the message because they never get to looking for a dkim/dmarc pass.", - fmt.Sprintf(`%s. TXT "v=spf1 mx ~all"`, d), + fmt.Sprintf(`%s. TXT "%s"`, d, dspftxt), "", "; Emails that fail the DMARC check (without aligned DKIM and without aligned SPF)", diff --git a/testdata/integration/moxacmepebble.sh b/testdata/integration/moxacmepebble.sh index e268f73..f0d0c90 100755 --- a/testdata/integration/moxacmepebble.sh +++ b/testdata/integration/moxacmepebble.sh @@ -27,7 +27,7 @@ sed -i -e 's/moxtest1@mox1.example: nil/moxtest1@mox1.example: nil\n\t\t\tpostfi cat /integration/example.zone; sed -n '/^;/,/will be suggested/p' output.txt | # allow sending from postfix for mox1.example. - sed 's/mox1.example. *TXT "v=spf1 mx ~all"/mox1.example. TXT "v=spf1 mx ip4:172.28.1.70 ~all"/' + sed 's/mox1.example. *TXT "v=spf1 ip4:172.28.1.10 mx ~all"/mox1.example. TXT "v=spf1 ip4:172.28.1.10 ip4:172.28.1.70 mx ~all"/' ) >/integration/example-integration.zone unbound-control -s 172.28.1.30 reload # reload unbound with zone file changes diff --git a/webadmin/admin.go b/webadmin/admin.go index 0873a4b..586d1b6 100644 --- a/webadmin/admin.go +++ b/webadmin/admin.go @@ -945,7 +945,12 @@ EOF defer wg.Done() // Verify a domain with the configured IPs that do SMTP. - verifySPF := func(kind string, domain dns.Domain) (string, *SPFRecord, spf.Record) { + verifySPF := func(isHost bool, domain dns.Domain) (string, *SPFRecord, spf.Record) { + kind := "domain" + if isHost { + kind = "host" + } + _, txt, record, _, err := spf.Lookup(ctx, log.Logger, resolver, domain) if err != nil { addf(&r.SPF.Errors, "Looking up %s SPF record: %s", kind, err) @@ -986,36 +991,27 @@ EOF } } - for _, l := range mox.Conf.Static.Listeners { - if !l.SMTP.Enabled || l.IPsNATed { - continue - } - ips := l.IPs - if len(l.NATIPs) > 0 { - ips = l.NATIPs - } - for _, ipstr := range ips { - ip := net.ParseIP(ipstr) - checkSPFIP(ip) - } - } - for _, t := range mox.Conf.Static.Transports { - if t.Socks != nil { - for _, ip := range t.Socks.IPs { - checkSPFIP(ip) - } - } + for _, ip := range mox.DomainSPFIPs() { + checkSPFIP(ip) } - spfr.Directives = append(spfr.Directives, spf.Directive{Qualifier: "-", Mechanism: "all"}) + if !isHost { + spfr.Directives = append(spfr.Directives, spf.Directive{Mechanism: "mx"}) + } + + qual := "~" + if isHost { + qual = "-" + } + spfr.Directives = append(spfr.Directives, spf.Directive{Qualifier: qual, Mechanism: "all"}) return txt, xrecord, spfr } // Check SPF record for domain. var dspfr spf.Record - r.SPF.DomainTXT, r.SPF.DomainRecord, dspfr = verifySPF("domain", domain) + r.SPF.DomainTXT, r.SPF.DomainRecord, dspfr = verifySPF(false, domain) // todo: possibly check all hosts for MX records? assuming they are also sending mail servers. - r.SPF.HostTXT, r.SPF.HostRecord, _ = verifySPF("host", mox.Conf.Static.HostnameDomain) + r.SPF.HostTXT, r.SPF.HostRecord, _ = verifySPF(true, mox.Conf.Static.HostnameDomain) dtxt, err := dspfr.Record() if err != nil {