implement accepting dmarc & tls reports for other domains

to accept reports for another domain, first add that domain to the config,
leaving all options empty except DMARC/TLSRPT in which you configure a Domain.

the suggested DNS DMARC/TLSRPT records will show the email address with
configured domain. for DMARC, the dnscheck functionality will verify that the
destination domain has opted in to receiving reports.

there is a new command-line subcommand "mox dmarc checkreportaddrs" that
verifies if dmarc reporting destination addresses have opted in to received
reports.

this also changes the suggested dns records (in quickstart, and through admin
pages and cli subcommand) to take into account whether DMARC and TLSRPT is
configured, and with which localpart/domain (previously it always printed
records as if reporting was enabled for the domain). and when generating the
suggested DNS records, the dmarc.Record and tlsrpt.Record code is used, with
proper uri-escaping.

dmarc/dmarc.go

@@ -146,6 +146,63 @@ func lookupRecord(ctx context.Context, resolver dns.Resolver, domain dns.Domain)
 	return StatusNone, record, text, rerr
 }
 
+func lookupReportsRecord(ctx context.Context, resolver dns.Resolver, dmarcDomain, extDestDomain dns.Domain) (Status, *Record, string, error) {
+	name := dmarcDomain.ASCII + "._report._dmarc." + extDestDomain.ASCII + "."
+	txts, err := dns.WithPackage(resolver, "dmarc").LookupTXT(ctx, name)
+	if err != nil && !dns.IsNotFound(err) {
+		return StatusTemperror, nil, "", fmt.Errorf("%w: %s", ErrDNS, err)
+	}
+	var record *Record
+	var text string
+	var rerr error = ErrNoRecord
+	for _, txt := range txts {
+		r, isdmarc, err := ParseRecordNoRequired(txt)
+		// Examples in the RFC use "v=DMARC1", even though it isn't a valid DMARC record.
+		// Accept the specific example.
+		// ../rfc/7489-eid5440
+		if !isdmarc && txt == "v=DMARC1" {
+			xr := DefaultRecord
+			r, isdmarc, err = &xr, true, nil
+		}
+		if !isdmarc {
+			// ../rfc/7489:1374
+			continue
+		} else if err != nil {
+			return StatusPermerror, nil, text, fmt.Errorf("%w: %s", ErrSyntax, err)
+		}
+		if record != nil {
+			// ../ ../rfc/7489:1388
+			return StatusNone, nil, "", ErrMultipleRecords
+		}
+		text = txt
+		record = r
+		rerr = nil
+	}
+	return StatusNone, record, text, rerr
+}
+
+// LookupExternalReportsAccepted returns whether the extDestDomain has opted in
+// to receiving dmarc reports for dmarcDomain (where the dmarc record was found),
+// through a "._report._dmarc." DNS TXT DMARC record.
+//
+// Callers should look at status for interpretation, not err, because err will
+// be set to ErrNoRecord when the DNS TXT record isn't present, which means the
+// extDestDomain does not opt in (not a failure condition).
+//
+// The normally invalid "v=DMARC1" record is accepted since it is used as
+// example in RFC 7489.
+func LookupExternalReportsAccepted(ctx context.Context, resolver dns.Resolver, dmarcDomain dns.Domain, extDestDomain dns.Domain) (accepts bool, status Status, record *Record, txt string, rerr error) {
+	log := xlog.WithContext(ctx)
+	start := time.Now()
+	defer func() {
+		log.Debugx("dmarc externalreports result", rerr, mlog.Field("accepts", accepts), mlog.Field("dmarcdomain", dmarcDomain), mlog.Field("extdestdomain", extDestDomain), mlog.Field("record", record), mlog.Field("duration", time.Since(start)))
+	}()
+
+	status, record, txt, rerr = lookupReportsRecord(ctx, resolver, dmarcDomain, extDestDomain)
+	accepts = rerr == nil
+	return accepts, status, record, txt, rerr
+}
+
 // Verify evaluates the DMARC policy for the domain in the From-header of a
 // message given the DKIM and SPF evaluation results.
 //

dmarc/dmarc_test.go

@@ -50,6 +50,45 @@ func TestLookup(t *testing.T) {
 	test("sub.example.com", StatusNone, "example.com", &r, nil) // Policy published at organizational domain, public suffix.
 }
 
+func TestLookupExternalReportsAccepted(t *testing.T) {
+	resolver := dns.MockResolver{
+		TXT: map[string][]string{
+			"example.com._report._dmarc.simple.example.":    {"v=DMARC1"},
+			"example.com._report._dmarc.simple2.example.":   {"v=DMARC1;"},
+			"example.com._report._dmarc.one.example.":       {"v=DMARC1; p=none;", "other"},
+			"example.com._report._dmarc.temperror.example.": {"v=DMARC1; p=none;"},
+			"example.com._report._dmarc.multiple.example.":  {"v=DMARC1; p=none;", "v=DMARC1"},
+			"example.com._report._dmarc.malformed.example.": {"v=DMARC1; p=none; bogus;"},
+		},
+		Fail: map[dns.Mockreq]struct{}{
+			{Type: "txt", Name: "example.com._report._dmarc.temperror.example."}: {},
+		},
+	}
+
+	test := func(dom, extdom string, expStatus Status, expAccepts bool, expErr error) {
+		t.Helper()
+
+		accepts, status, _, _, err := LookupExternalReportsAccepted(context.Background(), resolver, dns.Domain{ASCII: dom}, dns.Domain{ASCII: extdom})
+		if (err == nil) != (expErr == nil) || err != nil && !errors.Is(err, expErr) {
+			t.Fatalf("got err %#v, expected %#v", err, expErr)
+		}
+		if status != expStatus || accepts != expAccepts {
+			t.Fatalf("got status %s, accepts %v, expected %v, %v", status, accepts, expStatus, expAccepts)
+		}
+	}
+
+	r := DefaultRecord
+	r.Policy = PolicyNone
+	test("example.com", "simple.example", StatusNone, true, nil)
+	test("example.org", "simple.example", StatusNone, false, ErrNoRecord)
+	test("example.com", "simple2.example", StatusNone, true, nil)
+	test("example.com", "one.example", StatusNone, true, nil)
+	test("example.com", "absent.example", StatusNone, false, ErrNoRecord)
+	test("example.com", "multiple.example", StatusNone, false, ErrMultipleRecords)
+	test("example.com", "malformed.example", StatusPermerror, false, ErrSyntax)
+	test("example.com", "temperror.example", StatusTemperror, false, ErrDNS)
+}
+
 func TestVerify(t *testing.T) {
 	resolver := dns.MockResolver{
 		TXT: map[string][]string{

dmarc/parse.go

@@ -20,6 +20,16 @@ func (e parseErr) Error() string {
 //
 // DefaultRecord provides default values for tags not present in s.
 func ParseRecord(s string) (record *Record, isdmarc bool, rerr error) {
+	return parseRecord(s, true)
+}
+
+// ParseRecordNoRequired is like ParseRecord, but don't check for required fields
+// for regular DMARC records. Useful for checking the _report._dmarc record.
+func ParseRecordNoRequired(s string) (record *Record, isdmarc bool, rerr error) {
+	return parseRecord(s, false)
+}
+
+func parseRecord(s string, checkRequired bool) (record *Record, isdmarc bool, rerr error) {
 	defer func() {
 		x := recover()
 		if x == nil {
@@ -134,7 +144,7 @@ func ParseRecord(s string) (record *Record, isdmarc bool, rerr error) {
 	// ../rfc/7489:1106 says "p" is required, but ../rfc/7489:1407 implies we must be
 	// able to parse a record without a "p" or with invalid "sp" tag.
 	sp := r.SubdomainPolicy
-	if !seen["p"] || sp != PolicyEmpty && sp != PolicyNone && sp != PolicyQuarantine && sp != PolicyReject {
+	if checkRequired && (!seen["p"] || sp != PolicyEmpty && sp != PolicyNone && sp != PolicyQuarantine && sp != PolicyReject) {
 		if len(r.AggregateReportAddresses) > 0 {
 			r.Policy = PolicyNone
 			r.SubdomainPolicy = PolicyEmpty

dmarc/txt.go

@@ -23,7 +23,7 @@ const (
 type URI struct {
 	Address string // Should start with "mailto:".
 	MaxSize uint64 // Optional maximum message size, subject to Unit.
-	Unit    string // "" (b), "k", "g", "t" (case insensitive), unit size, where k is 2^10 etc.
+	Unit    string // "" (b), "k", "m", "g", "t" (case insensitive), unit size, where k is 2^10 etc.
 }
 
 // String returns a string representation of the URI for inclusion in a DMARC
@@ -33,7 +33,7 @@ func (u URI) String() string {
 	s = strings.ReplaceAll(s, ",", "%2C")
 	s = strings.ReplaceAll(s, "!", "%21")
 	if u.MaxSize > 0 {
-		s += fmt.Sprintf("%d", u.MaxSize)
+		s += fmt.Sprintf("!%d", u.MaxSize)
 	}
 	s += u.Unit
 	return s
@@ -109,13 +109,13 @@ func (r Record) String() string {
 		s := strings.Join(l, ",")
 		write(true, "ruf", s)
 	}
-	write(r.ADKIM != "", "adkim", string(r.ADKIM))
-	write(r.ASPF != "", "aspf", string(r.ASPF))
+	write(r.ADKIM != "" && r.ADKIM != "r", "adkim", string(r.ADKIM))
+	write(r.ASPF != "" && r.ASPF != "r", "aspf", string(r.ASPF))
 	write(r.AggregateReportingInterval != DefaultRecord.AggregateReportingInterval, "ri", fmt.Sprintf("%d", r.AggregateReportingInterval))
-	if len(r.FailureReportingOptions) > 1 || (len(r.FailureReportingOptions) == 1 && r.FailureReportingOptions[0] != "0") {
+	if len(r.FailureReportingOptions) > 1 || len(r.FailureReportingOptions) == 1 && r.FailureReportingOptions[0] != "0" {
 		write(true, "fo", strings.Join(r.FailureReportingOptions, ":"))
 	}
-	if len(r.ReportingFormat) > 1 || (len(r.ReportingFormat) == 1 && strings.EqualFold(r.ReportingFormat[0], "afrf")) {
+	if len(r.ReportingFormat) > 1 || len(r.ReportingFormat) == 1 && !strings.EqualFold(r.ReportingFormat[0], "afrf") {
 		write(true, "rf", strings.Join(r.FailureReportingOptions, ":"))
 	}
 	write(r.Percentage != 100, "pct", fmt.Sprintf("%d", r.Percentage))