From 2073db194b6b7bdcf5641d58a6c4f4958bda570e Mon Sep 17 00:00:00 2001 From: Mechiel Lukkien Date: Fri, 10 Nov 2023 20:25:06 +0100 Subject: [PATCH] when checking domain settings, check that dmarc & tls reporting addresses are present if there is a record --- mox-/admin.go | 4 ++-- tlsrpt/lookup_test.go | 4 ++-- tlsrpt/parse.go | 39 +++++++++++++++++++++++++++++++-------- tlsrpt/parse_test.go | 10 +++++----- tlsrptsend/send.go | 2 +- webadmin/admin.go | 40 ++++++++++++++++++++++++++++++++++------ webadmin/adminapi.json | 9 +++++++-- 7 files changed, 82 insertions(+), 26 deletions(-) diff --git a/mox-/admin.go b/mox-/admin.go index a7670e9..8f4eb73 100644 --- a/mox-/admin.go +++ b/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()), diff --git a/tlsrpt/lookup_test.go b/tlsrpt/lookup_test.go index 6e162ef..edcce8f 100644 --- a/tlsrpt/lookup_test.go +++ b/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("one.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: [][]RUA{{"mailto:tlsrpt@basic.example"}}}, nil) test("multiple.example", nil, ErrMultipleRecords) test("absent.example", nil, ErrNoRecord) test("other.example", nil, ErrNoRecord) diff --git a/tlsrpt/parse.go b/tlsrpt/parse.go index bcff02b..f20c785 100644 --- a/tlsrpt/parse.go +++ b/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. - RUAs [][]string + // be a list. + 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 { - fmt.Fprint(b, "; rua="+strings.Join(rua, ",")) + for _, ruas := range r.RUAs { + 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 { - l := []string{p.xuri()} +func (p *parser) xruas() []RUA { + 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) } diff --git a/tlsrpt/parse_test.go b/tlsrpt/parse_test.go index 351d7bb..3f4d1b9 100644 --- a/tlsrpt/parse_test.go +++ b/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 , \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; ext=yes", Record{Version: "TLSRPTv1", RUAs: [][]string{{"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: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: [][]RUA{{"mailto:tlsrpt@mox.example", "https://mox.example/tlsrpt"}}}) + 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: [][]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) diff --git a/tlsrptsend/send.go b/tlsrptsend/send.go index de76742..9bd2280 100644 --- a/tlsrptsend/send.go +++ b/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 diff --git a/webadmin/admin.go b/webadmin/admin.go index 6ed8dbe..ff50017 100644 --- a/webadmin/admin.go +++ b/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() - uristr = strings.ReplaceAll(uristr, ",", "%2C") - uristr = strings.ReplaceAll(uristr, "!", "%21") - uristr = strings.ReplaceAll(uristr, ";", "%3B") + rua := tlsrpt.RUA(uri.String()) 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 { diff --git a/webadmin/adminapi.json b/webadmin/adminapi.json index 2fc5ab9..3579a90 100644 --- a/webadmin/adminapi.json +++ b/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.",