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

@@ -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)",

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
 

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 _, ip := range mox.DomainSPFIPs() {
+				checkSPFIP(ip)
 			}
-			for _, t := range mox.Conf.Static.Transports {
-				if t.Socks != nil {
-					for _, ip := range t.Socks.IPs {
-						checkSPFIP(ip)
-					}
-				}
+
+			if !isHost {
+				spfr.Directives = append(spfr.Directives, spf.Directive{Mechanism: "mx"})
 			}
 
-			spfr.Directives = append(spfr.Directives, spf.Directive{Qualifier: "-", Mechanism: "all"})
+			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 {