infrastructure/mox
1
0
Fork 0
mirror of https://github.com/mjl-/mox.git synced 2025-07-12 17:44:35 +03:00
Code Issues Packages Projects Releases Wiki Activity

outgoing dmarc/tls reporting improvements

Browse Source 
- dmarc reports: add a cid to the log line about one run of sending reports, and log line for each report
- in smtpclient, also handle tls errors from the first read after a handshake. we appear to sometimes get tls alerts about bad certificates on the first read.
- for messages to dmarc/tls reporting addresses that we think should/can not be processed as reports, add an X-Mox- header explaining the reason.
- tls reports: send report messages with From address of postmaster at an actually configured domain for the mail host. and only send reports when dkim signing is configured for that domain. the domain is also the submitter domain. the rfc seems to require dkim-signing with an exact match with the message from and submitter.
- for incoming tls reports, in the smtp server, we do allow a dkim-signature domain that is higher-level (up to publicsuffix) of the message from domain. so we are stricter in what we send than what we receive.
This commit is contained in:
Mechiel Lukkien
2023-11-10 19:34:00 +01:00
parent b2af63b3ec
commit 61bae75228
7 changed files with 136 additions and 91 deletions
Download Patch File Download Diff File Expand all files Collapse all files

44
smtpserver/analyze.go
View File

@ -5,6 +5,7 @@ import (
"fmt"
"net"
"os"
"strings"
"time"
"github.com/mjl-/bstore"
@ -17,6 +18,7 @@ import (
"github.com/mjl-/mox/iprev"
"github.com/mjl-/mox/mlog"
"github.com/mjl-/mox/mox-"
"github.com/mjl-/mox/publicsuffix"
"github.com/mjl-/mox/smtp"
"github.com/mjl-/mox/store"
"github.com/mjl-/mox/subjectpass"
@ -48,6 +50,9 @@ type analysis struct {
tlsReport *tlsrpt.Report // Validated TLS report, not yet stored.
reason string // If non-empty, reason for this decision. Can be one of reputationMethod and a few other tokens.
dmarcOverrideReason string // If set, one of dmarcrpt.PolicyOverride
// Additional headers to add during delivery. Used for reasons a message to a
// dmarc/tls reporting address isn't processed.
headers string
}
const (
@ -81,6 +86,8 @@ func isListDomain(d delivery, ld dns.Domain) bool {
}
func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delivery) analysis {
var headers string
mailbox := d.rcptAcc.destination.Mailbox
if mailbox == "" {
mailbox = "Inbox"
@ -96,7 +103,7 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive
// todo: on temporary failures, reject temporarily?
if isListDomain(d, rs.ListAllowDNSDomain) {
d.m.IsMailingList = true
return analysis{accept: true, mailbox: mailbox, reason: reasonListAllow, dmarcOverrideReason: string(dmarcrpt.PolicyOverrideMailingList)}
return analysis{accept: true, mailbox: mailbox, reason: reasonListAllow, dmarcOverrideReason: string(dmarcrpt.PolicyOverrideMailingList), headers: headers}
}
}
@ -165,7 +172,7 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive
})
})
if mberr != nil {
return analysis{false, mailbox, smtp.C451LocalErr, smtp.SeSys3Other0, false, "error processing", err, nil, nil, reasonReputationError, dmarcOverrideReason}
return analysis{false, mailbox, smtp.C451LocalErr, smtp.SeSys3Other0, false, "error processing", err, nil, nil, reasonReputationError, dmarcOverrideReason, headers}
}
d.m.MailboxID = 0 // We plan to reject, no need to set intended MailboxID.
}
@ -179,7 +186,7 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive
d.m.Seen = true
log.Info("accepting reject to configured mailbox due to ruleset")
}
return analysis{accept, mailbox, code, secode, err == nil, errmsg, err, nil, nil, reason, dmarcOverrideReason}
return analysis{accept, mailbox, code, secode, err == nil, errmsg, err, nil, nil, reason, dmarcOverrideReason, headers}
}
if d.dmarcUse && d.dmarcResult.Reject {
@ -194,14 +201,19 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive
// Messages with DMARC aggregate reports must have a DMARC pass. ../rfc/7489:1866
if d.dmarcResult.Status != dmarc.StatusPass {
log.Info("received dmarc aggregate report without dmarc pass, not processing as dmarc report")
headers += "X-Mox-DMARCReport-Error: no DMARC pass\r\n"
} else if report, err := dmarcrpt.ParseMessageReport(log, store.FileMsgReader(d.m.MsgPrefix, d.dataFile)); err != nil {
log.Infox("parsing dmarc aggregate report", err)
headers += "X-Mox-DMARCReport-Error: could not parse report\r\n"
} else if d, err := dns.ParseDomain(report.PolicyPublished.Domain); err != nil {
log.Infox("parsing domain in dmarc aggregate report", err)
headers += "X-Mox-DMARCReport-Error: could not parse domain in published policy\r\n"
} else if _, ok := mox.Conf.Domain(d); !ok {
log.Info("dmarc aggregate report for domain not configured, ignoring", mlog.Field("domain", d))
headers += "X-Mox-DMARCReport-Error: published policy domain unrecognized\r\n"
} else if report.ReportMetadata.DateRange.End > time.Now().Unix()+60 {
log.Info("dmarc aggregate report with end date in the future, ignoring", mlog.Field("domain", d), mlog.Field("end", time.Unix(report.ReportMetadata.DateRange.End, 0)))
headers += "X-Mox-DMARCReport-Error: report has end date in the future\r\n"
} else {
dmarcReport = report
}
@ -211,6 +223,11 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive
// reputation, defaulting to accept.
var tlsReport *tlsrpt.Report
if d.rcptAcc.destination.HostTLSReports || d.rcptAcc.destination.DomainTLSReports {
matchesDomain := func(sigDomain dns.Domain) bool {
// RFC seems to require exact DKIM domain match with submitt and message From, we
// also allow msgFrom to be subdomain. ../rfc/8460:322
return sigDomain == d.msgFrom.Domain || strings.HasSuffix(d.msgFrom.Domain.ASCII, "."+sigDomain.ASCII) && publicsuffix.Lookup(ctx, d.msgFrom.Domain) == publicsuffix.Lookup(ctx, sigDomain)
}
// Valid DKIM signature for domain must be present. We take "valid" to assume
// "passing", not "syntactically valid". We also check for "tlsrpt" as service.
// This check is optional, but if anyone goes through the trouble to explicitly
@ -220,12 +237,12 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive
for _, r := range d.dkimResults {
// The record should have an allowed service "tlsrpt". The RFC mentions it as if
// the service must be specified explicitly, but the default allowed services for a
// DKIM record are "*", which includes "tlsrpt". Unless a the DKIM record
// explicitly specifies services (e.g. s=email), a record will work for TLS
// reports. The DKIM records seen used for TLS reporting in the wild don't
// explicitly set "s" for services.
// DKIM record are "*", which includes "tlsrpt". Unless a DKIM record explicitly
// specifies services (e.g. s=email), a record will work for TLS reports. The DKIM
// records seen used for TLS reporting in the wild don't explicitly set "s" for
// services.
// ../rfc/8460:326
if r.Status == dkim.StatusPass && r.Sig.Domain == d.msgFrom.Domain && r.Sig.Length < 0 && r.Record.ServiceAllowed("tlsrpt") {
if r.Status == dkim.StatusPass && matchesDomain(r.Sig.Domain) && r.Sig.Length < 0 && r.Record.ServiceAllowed("tlsrpt") {
ok = true
break
}
@ -233,8 +250,10 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive
if !ok {
log.Info("received mail to tlsrpt without acceptable DKIM signature, not processing as tls report")
headers += "X-Mox-TLSReport-Error: no acceptable DKIM signature\r\n"
} else if report, err := tlsrpt.ParseMessage(log, store.FileMsgReader(d.m.MsgPrefix, d.dataFile)); err != nil {
log.Infox("parsing tls report", err)
headers += "X-Mox-TLSReport-Error: could not parse TLS report\r\n"
} else {
var known bool
for _, p := range report.Policies {
@ -248,6 +267,7 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive
}
if !known {
log.Info("tls report without one of configured domains, ignoring")
headers += "X-Mox-TLSReport-Error: report for unknown domain\r\n"
} else {
tlsReport = report
}
@ -279,12 +299,12 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive
log.Info("reputation analyzed", mlog.Field("conclusive", conclusive), mlog.Field("isjunk", isjunk), mlog.Field("method", string(method)))
if conclusive {
if !*isjunk {
return analysis{accept: true, mailbox: mailbox, dmarcReport: dmarcReport, tlsReport: tlsReport, reason: reason, dmarcOverrideReason: dmarcOverrideReason}
return analysis{accept: true, mailbox: mailbox, dmarcReport: dmarcReport, tlsReport: tlsReport, reason: reason, dmarcOverrideReason: dmarcOverrideReason, headers: headers}
}
return reject(smtp.C451LocalErr, smtp.SeSys3Other0, "error processing", err, string(method))
} else if dmarcReport != nil || tlsReport != nil {
log.Info("accepting message with dmarc aggregate report or tls report without reputation")
return analysis{accept: true, mailbox: mailbox, dmarcReport: dmarcReport, tlsReport: tlsReport, reason: reasonReporting, dmarcOverrideReason: dmarcOverrideReason}
return analysis{accept: true, mailbox: mailbox, dmarcReport: dmarcReport, tlsReport: tlsReport, reason: reasonReporting, dmarcOverrideReason: dmarcOverrideReason, headers: headers}
}
// If there was no previous message from sender or its domain, and we have an SPF
// (soft)fail, reject the message.
@ -320,7 +340,7 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive
pass := err == nil
log.Infox("pass by subject token", err, mlog.Field("pass", pass))
if pass {
return analysis{accept: true, mailbox: mailbox, reason: reasonSubjectpass, dmarcOverrideReason: dmarcOverrideReason}
return analysis{accept: true, mailbox: mailbox, reason: reasonSubjectpass, dmarcOverrideReason: dmarcOverrideReason, headers: headers}
}
}
@ -400,7 +420,7 @@ func analyze(ctx context.Context, log *mlog.Log, resolver dns.Resolver, d delive
}
if accept {
return analysis{accept: true, mailbox: mailbox, reason: reasonNoBadSignals, dmarcOverrideReason: dmarcOverrideReason}
return analysis{accept: true, mailbox: mailbox, reason: reasonNoBadSignals, dmarcOverrideReason: dmarcOverrideReason, headers: headers}
}
if subjectpassKey != "" && d.dmarcResult.Status == dmarc.StatusPass && method == methodNone && (dnsblocklisted || junkSubjectpass) {