when checking domain settings, check that dmarc & tls reporting addresses are present if there is a record

mox-/admin.go

@@ -536,7 +536,7 @@ func DomainRecords(domConf config.Domain, domain dns.Domain, hasDNSSEC bool) ([]
 			Scheme: "mailto",
 			Opaque: smtp.NewAddress(Conf.Static.HostTLSRPT.ParsedLocalpart, Conf.Static.HostnameDomain).Pack(false),
 		}
-		tlsrptr := tlsrpt.Record{Version: "TLSRPTv1", RUAs: [][]string{{uri.String()}}}
+		tlsrptr := tlsrpt.Record{Version: "TLSRPTv1", RUAs: [][]tlsrpt.RUA{{tlsrpt.RUA(uri.String())}}}
 		records = append(records,
 			"; For the machine, only needs to be created once, for the first domain added:",
 			"; ",
@@ -638,7 +638,7 @@ func DomainRecords(domConf config.Domain, domain dns.Domain, hasDNSSEC bool) ([]
 			Scheme: "mailto",
 			Opaque: smtp.NewAddress(domConf.TLSRPT.ParsedLocalpart, domConf.TLSRPT.DNSDomain).Pack(false),
 		}
-		tlsrptr := tlsrpt.Record{Version: "TLSRPTv1", RUAs: [][]string{{uri.String()}}}
+		tlsrptr := tlsrpt.Record{Version: "TLSRPTv1", RUAs: [][]tlsrpt.RUA{{tlsrpt.RUA(uri.String())}}}
 		records = append(records,
 			"; Request reporting about TLS failures.",
 			fmt.Sprintf(`_smtp._tls.%s.         TXT "%s"`, d, tlsrptr.String()),

tlsrpt/lookup_test.go

@@ -36,8 +36,8 @@ func TestLookup(t *testing.T) {
 		}
 	}
 
-	test("basic.example", &Record{Version: "TLSRPTv1", RUAs: [][]string{{"mailto:tlsrpt@basic.example"}}}, nil)
+	test("basic.example", &Record{Version: "TLSRPTv1", RUAs: [][]RUA{{"mailto:tlsrpt@basic.example"}}}, nil)
-	test("one.example", &Record{Version: "TLSRPTv1", RUAs: [][]string{{"mailto:tlsrpt@basic.example"}}}, nil)
+	test("one.example", &Record{Version: "TLSRPTv1", RUAs: [][]RUA{{"mailto:tlsrpt@basic.example"}}}, nil)
 	test("multiple.example", nil, ErrMultipleRecords)
 	test("absent.example", nil, ErrNoRecord)
 	test("other.example", nil, ErrNoRecord)

tlsrpt/parse.go

@@ -21,19 +21,42 @@ type Record struct {
 	Version string // "TLSRPTv1", for "v=".
 
 	// Aggregate reporting URI, for "rua=". "rua=" can occur multiple times, each can
-	// be a list. Must be URL-encoded strings, with ",", "!" and ";" encoded.
+	// be a list.
-	RUAs [][]string
+	RUAs [][]RUA
 	// ../rfc/8460:383
 
 	Extensions []Extension
 }
 
+// RUA is a reporting address with scheme and special characters ",", "!" and
+// ";" not encoded.
+type RUA string
+
+// String returns the RUA with special characters encoded, for inclusion in a
+// TLSRPT record.
+func (rua RUA) String() string {
+	s := string(rua)
+	s = strings.ReplaceAll(s, ",", "%2C")
+	s = strings.ReplaceAll(s, "!", "%21")
+	s = strings.ReplaceAll(s, ";", "%3B")
+	return s
+}
+
+// URI parses a RUA as URI, with either a mailto or https scheme.
+func (rua RUA) URI() (*url.URL, error) {
+	return url.Parse(string(rua))
+}
+
 // String returns a string or use as a TLSRPT DNS TXT record.
 func (r Record) String() string {
 	b := &strings.Builder{}
 	fmt.Fprint(b, "v="+r.Version)
-	for _, rua := range r.RUAs {
+	for _, ruas := range r.RUAs {
-		fmt.Fprint(b, "; rua="+strings.Join(rua, ","))
+		l := make([]string, len(ruas))
+		for i, rua := range ruas {
+			l[i] = rua.String()
+		}
+		fmt.Fprint(b, "; rua="+strings.Join(l, ","))
 	}
 	for _, p := range r.Extensions {
 		fmt.Fprint(b, "; "+p.Key+"="+p.Value)
@@ -204,8 +227,8 @@ func (p *parser) wsp() {
 }
 
 // ../rfc/8460:358
-func (p *parser) xruas() []string {
+func (p *parser) xruas() []RUA {
-	l := []string{p.xuri()}
+	l := []RUA{p.xuri()}
 	p.wsp()
 	for p.take(",") {
 		p.wsp()
@@ -216,7 +239,7 @@ func (p *parser) xruas() []string {
 }
 
 // ../rfc/8460:360
-func (p *parser) xuri() string {
+func (p *parser) xuri() RUA {
 	v := p.xtakefn1(func(b rune, i int) bool {
 		return b != ',' && b != '!' && b != ' ' && b != '\t' && b != ';'
 	})
@@ -227,5 +250,5 @@ func (p *parser) xuri() string {
 	if u.Scheme == "" {
 		p.xerrorf("missing scheme in uri")
 	}
-	return v
+	return RUA(v)
 }

tlsrpt/parse_test.go

@@ -25,10 +25,10 @@ func TestRecord(t *testing.T) {
 		}
 	}
 
-	good("v=TLSRPTv1; rua=mailto:tlsrpt@mox.example;", Record{Version: "TLSRPTv1", RUAs: [][]string{{"mailto:tlsrpt@mox.example"}}})
+	good("v=TLSRPTv1; rua=mailto:tlsrpt@mox.example;", Record{Version: "TLSRPTv1", RUAs: [][]RUA{{"mailto:tlsrpt@mox.example"}}})
-	good("v=TLSRPTv1; rua=mailto:tlsrpt@mox.example , \t\t https://mox.example/tlsrpt  ", Record{Version: "TLSRPTv1", RUAs: [][]string{{"mailto:tlsrpt@mox.example", "https://mox.example/tlsrpt"}}})
+	good("v=TLSRPTv1; rua=mailto:tlsrpt@mox.example , \t\t https://mox.example/tlsrpt  ", Record{Version: "TLSRPTv1", RUAs: [][]RUA{{"mailto:tlsrpt@mox.example", "https://mox.example/tlsrpt"}}})
-	good("v=TLSRPTv1; rua=mailto:tlsrpt@mox.example; ext=yes", Record{Version: "TLSRPTv1", RUAs: [][]string{{"mailto:tlsrpt@mox.example"}}, Extensions: []Extension{{"ext", "yes"}}})
+	good("v=TLSRPTv1; rua=mailto:tlsrpt@mox.example; ext=yes", Record{Version: "TLSRPTv1", RUAs: [][]RUA{{"mailto:tlsrpt@mox.example"}}, Extensions: []Extension{{"ext", "yes"}}})
-	good("v=TLSRPTv1 ; rua=mailto:x@x.example; rua=mailto:y@x.example", Record{Version: "TLSRPTv1", RUAs: [][]string{{"mailto:x@x.example"}, {"mailto:y@x.example"}}})
+	good("v=TLSRPTv1 ; rua=mailto:x@x.example; rua=mailto:y@x.example", Record{Version: "TLSRPTv1", RUAs: [][]RUA{{"mailto:x@x.example"}, {"mailto:y@x.example"}}})
 
 	bad("v=TLSRPTv0")
 	bad("v=TLSRPTv10")
@@ -48,7 +48,7 @@ func TestRecord(t *testing.T) {
 	bad("v=TLSRPTv1; rua=http://bad/%")                                            // bad URI
 
 	const want = `v=TLSRPTv1; rua=mailto:x@mox.example; more=a; ext=2`
-	record := Record{Version: "TLSRPTv1", RUAs: [][]string{{"mailto:x@mox.example"}}, Extensions: []Extension{{"more", "a"}, {"ext", "2"}}}
+	record := Record{Version: "TLSRPTv1", RUAs: [][]RUA{{"mailto:x@mox.example"}}, Extensions: []Extension{{"more", "a"}, {"ext", "2"}}}
 	got := record.String()
 	if got != want {
 		t.Fatalf("record string, got %q, want %q", got, want)

tlsrptsend/send.go

@@ -308,7 +308,7 @@ func sendReportDomain(ctx context.Context, log *mlog.Log, resolver dns.Resolver,
 
 	for _, l := range record.RUAs {
 		for _, s := range l {
-			u, err := url.Parse(s)
+			u, err := url.Parse(string(s))
 			if err != nil {
 				log.Debugx("parsing rua uri in tlsrpt dns record, ignoring", err, mlog.Field("rua", s))
 				continue

webadmin/admin.go

@@ -1118,8 +1118,22 @@ EOF
 				Scheme: "mailto",
 				Opaque: smtp.NewAddress(domConf.DMARC.ParsedLocalpart, domConf.DMARC.DNSDomain).Pack(false),
 			}
+			uristr := uri.String()
 			dmarcr.AggregateReportAddresses = []dmarc.URI{
-				{Address: uri.String(), MaxSize: 10, Unit: "m"},
+				{Address: uristr, MaxSize: 10, Unit: "m"},
+			}
+
+			if record != nil {
+				found := false
+				for _, addr := range record.AggregateReportAddresses {
+					if addr.Address == uristr {
+						found = true
+						break
+					}
+				}
+				if !found {
+					addf(&r.DMARC.Errors, "Configured DMARC reporting address is not present in record.")
+				}
 			}
 		} else {
 			addf(&r.DMARC.Instructions, `Configure a DMARC destination in domain in config file.`)
@@ -1153,13 +1167,10 @@ EOF
 				Scheme: "mailto",
 				Opaque: address.Pack(false),
 			}
-			uristr := uri.String()
+			rua := tlsrpt.RUA(uri.String())
-			uristr = strings.ReplaceAll(uristr, ",", "%2C")
-			uristr = strings.ReplaceAll(uristr, "!", "%21")
-			uristr = strings.ReplaceAll(uristr, ";", "%3B")
 			tlsrptr := &tlsrpt.Record{
 				Version: "TLSRPTv1",
-				RUAs:    [][]string{{uristr}},
+				RUAs:    [][]tlsrpt.RUA{{rua}},
 			}
 			instr += fmt.Sprintf(`
 
@@ -1167,6 +1178,23 @@ Ensure a DNS TXT record like the following exists:
 
 	_smtp._tls TXT %s
 `, mox.TXTStrings(tlsrptr.String()))
+
+			if err == nil {
+				found := false
+			RUA:
+				for _, l := range record.RUAs {
+					for _, e := range l {
+						if e == rua {
+							found = true
+							break RUA
+						}
+					}
+				}
+				if !found {
+					addf(&result.Errors, `Configured reporting address is not present in TLSRPT record.`)
+				}
+			}
+
 		} else if isHost {
 			addf(&result.Errors, `Configure a host TLSRPT localpart in static mox.conf config file.`)
 		} else {

webadmin/adminapi.json

@@ -1759,11 +1759,11 @@
 				},
 				{
 					"Name": "RUAs",
-					"Docs": "Aggregate reporting URI, for \"rua=\". \"rua=\" can occur multiple times, each can be a list. Must be URL-encoded strings, with \",\", \"!\" and \";\" encoded.",
+					"Docs": "Aggregate reporting URI, for \"rua=\". \"rua=\" can occur multiple times, each can be a list.",
 					"Typewords": [
 						"[]",
 						"[]",
-						"string"
+						"RUA"
 					]
 				},
 				{
@@ -3903,6 +3903,11 @@
 				}
 			]
 		},
+		{
+			"Name": "RUA",
+			"Docs": "RUA is a reporting address with scheme and special characters \",\", \"!\" and\n\";\" not encoded.",
+			"Values": null
+		},
 		{
 			"Name": "Mode",
 			"Docs": "Mode indicates how the policy should be interpreted.",