Skip to content

Commit

Permalink
accept tls reports with both host & recipient domains, and with multi…
Browse files Browse the repository at this point in the history 
…ple recipient domains

embarrassingly, we didn't accept all reports we generated. after the changed
handling of reports about mx/mail host vs recipient domains, would send reports
to mail hosts about multiple recipient domains + the mail host. and we included
a policy domain of the mail host when sending to a recipient domain. we were
still being strict in what we accepted: only a single domain in total in the
entire report, and we still enforced that a report sent to the mx host tlsrpt
address only contained the mx host as policy domain. and likewise for recipient
domains and their tls reporting addresses. those checks would reject reports
generated by a mox instance. this probably only happens with dane configured,
probably most users haven't seen it because of that.

somewhat related to issue #125
  • Loading branch information
@mjl-
mjl- committed Mar 5, 2024
1 parent a9cb6f9 commit e0c36ed
Show file tree
Hide file tree
Showing 2 changed files with 236 additions and 37 deletions.
72 changes: 36 additions & 36 deletions tlsrptdb/report.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,9 @@ func reportDB(ctx context.Context) (rdb *bstore.DB, rerr error) {
// verifiedFromDomain. Using HTTPS for reports is not recommended as there is no
// authentication on the reports origin.
//
// The report is currently required to only cover a single domain in its policy
// domain. Only reports for known domains are added to the database.
// Only reports for known domains are added to the database. Unknown domains are
// ignored without causing an error, unless no known domain was found in the report
// at all.
//
// Prometheus metrics are updated only for configured domains.
func AddReport(ctx context.Context, log mlog.Log, verifiedFromDomain dns.Domain, mailFrom string, hostReport bool, r *tlsrpt.Report) error {
Expand All @@ -92,44 +93,43 @@ func AddReport(ctx context.Context, log mlog.Log, verifiedFromDomain dns.Domain,
return fmt.Errorf("no policies in report")
}

var reportdom, zerodom dns.Domain
record := TLSReportRecord{0, "", verifiedFromDomain.Name(), mailFrom, hostReport, *r}
var inserted int
return db.Write(ctx, func(tx *bstore.Tx) error {
for _, p := range r.Policies {
pp := p.Policy

for _, p := range r.Policies {
pp := p.Policy
d, err := dns.ParseDomain(pp.Domain)
if err != nil {
return fmt.Errorf("invalid domain %v in tls report: %v", d, err)
}

// Check domain, they must all be the same for now. We are not expecting senders to
// coalesce TLS results for different policy domains in a single report.
d, err := dns.ParseDomain(pp.Domain)
if err != nil {
log.Errorx("invalid domain in tls report", err, slog.Any("domain", pp.Domain), slog.String("mailfrom", mailFrom))
continue
}
if hostReport && d != mox.Conf.Static.HostnameDomain {
log.Info("unknown mail host policy domain in tls report, not storing", slog.Any("domain", d), slog.String("mailfrom", mailFrom))
return fmt.Errorf("unknown mail host policy domain")
} else if _, ok := mox.Conf.Domain(d); !hostReport && !ok {
log.Info("unknown recipient policy domain in tls report, not storing", slog.Any("domain", d), slog.String("mailfrom", mailFrom))
return fmt.Errorf("unknown recipient policy domain")
}
if reportdom != zerodom && d != reportdom {
return fmt.Errorf("multiple domains in report %s and %s", reportdom, d)
}
reportdom = d

metricSession.WithLabelValues("success").Add(float64(p.Summary.TotalSuccessfulSessionCount))
for _, f := range p.FailureDetails {
var result string
if _, ok := knownResultTypes[f.ResultType]; ok {
result = string(f.ResultType)
} else {
result = "other"
if _, ok := mox.Conf.Domain(d); !ok && d != mox.Conf.Static.HostnameDomain {
log.Info("unknown host/recipient policy domain in tls report, not storing", slog.Any("domain", d), slog.String("mailfrom", mailFrom))
continue
}

metricSession.WithLabelValues("success").Add(float64(p.Summary.TotalSuccessfulSessionCount))
for _, f := range p.FailureDetails {
var result string
if _, ok := knownResultTypes[f.ResultType]; ok {
result = string(f.ResultType)
} else {
result = "other"
}
metricSession.WithLabelValues(result).Add(float64(f.FailedSessionCount))
}

record := TLSReportRecord{0, d.Name(), verifiedFromDomain.Name(), mailFrom, d == mox.Conf.Static.HostnameDomain, *r}
if err := tx.Insert(&record); err != nil {
return fmt.Errorf("inserting report for domain: %w", err)
}
metricSession.WithLabelValues(result).Add(float64(f.FailedSessionCount))
inserted++
}
}
record.Domain = reportdom.Name()
return db.Insert(ctx, &record)
if inserted == 0 {
return fmt.Errorf("no domains in report recognized")
}
return nil
})
}

// Records returns all TLS reports in the database.
Expand Down
201 changes: 200 additions & 1 deletion tlsrptdb/report_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,14 +62,183 @@ const reportJSON = `{
}]
}`

const reportMultipleJSON = `{
"organization-name": "remote.example",
"date-range": {
"start-datetime": "2024-02-25T00:00:00Z",
"end-datetime": "2024-02-25T23:59:59Z"
},
"contact-info": "postmaster@remote.example",
"report-id": "20240225.mail.mox.example@remote.example",
"policies": [
{
"policy": {
"policy-type": "tlsa",
"policy-string": [
"3 1 1 206d5f55ecb9f8389bc57b5ba14716dd5b23d0834fd2c99fd402f0bda32e9523",
"3 1 1 4201e4b741c746b62ff806c142158c35ecbbbd9ac56b6d791f760e272736f8d0"
],
"policy-domain": "test2.xmox.nl",
"mx-host": [
"mail.mox.example"
]
},
"summary": {
"total-successful-session-count": 1,
"total-failure-session-count": 0
},
"failure-details": []
},
{
"policy": {
"policy-type": "tlsa",
"policy-string": [
"3 1 1 206d5f55ecb9f8389bc57b5ba14716dd5b23d0834fd2c99fd402f0bda32e9523",
"3 1 1 4201e4b741c746b62ff806c142158c35ecbbbd9ac56b6d791f760e272736f8d0"
],
"policy-domain": "test.xmox.nl",
"mx-host": [
"mail.mox.example"
]
},
"summary": {
"total-successful-session-count": 1,
"total-failure-session-count": 0
},
"failure-details": []
}
]
}
`

const reportMixedJSON = `{
"organization-name": "remote.example",
"date-range": {
"start-datetime": "2024-02-25T00:00:00Z",
"end-datetime": "2024-02-25T23:59:59Z"
},
"contact-info": "postmaster@remote.example",
"report-id": "20240225.test.xmox.nl@remote.example",
"policies": [
{
"policy": {
"policy-type": "tlsa",
"policy-string": [
"3 1 1 206d5f55ecb9f8389bc57b5ba14716dd5b23d0834fd2c99fd402f0bda32e9523",
"3 1 1 4201e4b741c746b62ff806c142158c35ecbbbd9ac56b6d791f760e272736f8d0"
],
"policy-domain": "mail.mox.example",
"mx-host": []
},
"summary": {
"total-successful-session-count": 1,
"total-failure-session-count": 0
},
"failure-details": []
},
{
"policy": {
"policy-type": "sts",
"policy-string": [
"version: STSv1",
"mode: enforce",
"max_age: 86400",
"mx: mail.mox.example"
],
"policy-domain": "unknown.xmox.nl",
"mx-host": [
"mail.mox.example"
]
},
"summary": {
"total-successful-session-count": 1,
"total-failure-session-count": 0
},
"failure-details": []
},
{
"policy": {
"policy-type": "sts",
"policy-string": [
"version: STSv1",
"mode: enforce",
"max_age: 86400",
"mx: mail.mox.example"
],
"policy-domain": "test.xmox.nl",
"mx-host": [
"mail.mox.example"
]
},
"summary": {
"total-successful-session-count": 1,
"total-failure-session-count": 0
},
"failure-details": []
}
]
}
`

const reportUnknownJSON = `{
"organization-name": "remote.example",
"date-range": {
"start-datetime": "2024-02-25T00:00:00Z",
"end-datetime": "2024-02-25T23:59:59Z"
},
"contact-info": "postmaster@remote.example",
"report-id": "20240225.test.xmox.nl@remote.example",
"policies": [
{
"policy": {
"policy-type": "tlsa",
"policy-string": [
"3 1 1 206d5f55ecb9f8389bc57b5ba14716dd5b23d0834fd2c99fd402f0bda32e9523",
"3 1 1 4201e4b741c746b62ff806c142158c35ecbbbd9ac56b6d791f760e272736f8d0"
],
"policy-domain": "unknown.mox.example",
"mx-host": []
},
"summary": {
"total-successful-session-count": 1,
"total-failure-session-count": 0
},
"failure-details": []
},
{
"policy": {
"policy-type": "sts",
"policy-string": [
"version: STSv1",
"mode: enforce",
"max_age: 86400",
"mx: mail.mox.example"
],
"policy-domain": "unknown.xmox.nl",
"mx-host": [
"unknown.mox.example"
]
},
"summary": {
"total-successful-session-count": 1,
"total-failure-session-count": 0
},
"failure-details": []
}
]
}
`

func TestReport(t *testing.T) {
mox.Context = ctxbg
mox.Shutdown, mox.ShutdownCancel = context.WithCancel(ctxbg)
mox.ConfigStaticPath = filepath.FromSlash("../testdata/tlsrpt/fake.conf")
mox.Conf.Static.HostnameDomain = dns.Domain{ASCII: "mail.mox.example"}
mox.Conf.Static.DataDir = "."
// Recognize as configured domain.
mox.Conf.Dynamic.Domains = map[string]config.Domain{
"test.xmox.nl": {},
"test.xmox.nl": {},
"test2.xmox.nl": {},
}

dbpath := mox.DataDirPath("tlsrpt.db")
Expand Down Expand Up @@ -133,4 +302,34 @@ func TestReport(t *testing.T) {
if err != nil || len(records) != 1 {
t.Fatalf("got err %v, records %#v, expected no error with 1 record", err, records)
}

// Add report with multiple recipient domains.
reportJSON, err = tlsrpt.Parse(strings.NewReader(reportMultipleJSON))
if err != nil {
t.Fatalf("parsing report: %v", err)
}
report = reportJSON.Convert()
if err := AddReport(ctxbg, pkglog, dns.Domain{ASCII: "remote.example"}, "postmaster@remote.example", false, &report); err != nil {
t.Errorf("adding report to database: %s", err)
}

// Add report with mixed host and domain policies. The unknown domain is ignored.
reportJSON, err = tlsrpt.Parse(strings.NewReader(reportMixedJSON))
if err != nil {
t.Fatalf("parsing report: %v", err)
}
report = reportJSON.Convert()
if err := AddReport(ctxbg, pkglog, dns.Domain{ASCII: "remote.example"}, "postmaster@remote.example", false, &report); err != nil {
t.Errorf("adding report to database: %s", err)
}

// All unknown domains in report should cause error.
reportJSON, err = tlsrpt.Parse(strings.NewReader(reportUnknownJSON))
if err != nil {
t.Fatalf("parsing report: %v", err)
}
report = reportJSON.Convert()
if err := AddReport(ctxbg, pkglog, dns.Domain{ASCII: "remote.example"}, "postmaster@remote.example", false, &report); err == nil {
t.Errorf("adding report with all unknown domains, expected error")
}
}

0 comments on commit e0c36ed

Please sign in to comment.