serv/mox
1
0
Fork 0
mirror of https://github.com/mjl-/mox.git synced 2024-12-27 08:53:48 +03:00
Code Issues Projects Releases Packages Wiki Activity Actions

security fix: use correct domain for mta-sts, that of the email address

Browse source 
the original next-hop domain. not anything after resolving cname's, because
then it takes just a single injected dns cname record to lead us to an
unrelated server (that we would verify, but it's the wrong server).

also don't fallback to just strict tls when something is wrong. we must use the
policy to check if an mx host is allowed. the whole idea is that unsigned dns
records cannot be trusted.

i noticed this while implementing dane.
This commit is contained in:
Mechiel Lukkien 2023-10-14 22:30:43 +02:00
parent 42d817ef3d
commit 8ca198882e
No known key found for this signature in database
1 changed files with 10 additions and 11 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

21
queue/direct.go
View file

@ -116,19 +116,18 @@ func deliverDirect(cid int64, qlog *mlog.Log, resolver dns.Resolver, dialer smtp
return return
} }
// Check for MTA-STS policy and enforce it if needed. We have to check the // Check for MTA-STS policy and enforce it if needed.
// effective domain (found after following CNAME record(s)): there will certainly // We must check at the original next-hop, i.e. recipient domain, not following any
// not be an MTA-STS record for the original recipient domain, because that is not // CNAMEs. If we were to follow CNAMEs and ask for MTA-STS at that domain, it
// allowed when a CNAME record is present. // would only take a single CNAME DNS response to direct us to an unrelated domain.
var policy *mtasts.Policy var policy *mtasts.Policy
tlsModeDefault := smtpclient.TLSOpportunistic if !origNextHop.IsZero() {
if !expandedNextHop.IsZero() {
cidctx := context.WithValue(mox.Shutdown, mlog.CidKey, cid) cidctx := context.WithValue(mox.Shutdown, mlog.CidKey, cid)
policy, _, err = mtastsdb.Get(cidctx, resolver, expandedNextHop) policy, _, err = mtastsdb.Get(cidctx, resolver, origNextHop)
if err != nil { if err != nil {
// No need to refuse to deliver if we have some mtasts error. qlog.Infox("mtasts lookup temporary error, aborting delivery attempt", err, mlog.Field("domain", origNextHop))
qlog.Infox("mtasts failed, continuing with strict tls requirement", err, mlog.Field("domain", expandedNextHop)) fail(qlog, m, backoff, false, dsn.NameIP{}, "", err.Error())
tlsModeDefault = smtpclient.TLSStrictStartTLS return
} }
// note: policy can be nil, if a domain does not implement MTA-STS or it's the // note: policy can be nil, if a domain does not implement MTA-STS or it's the
// first time we fetch the policy and if we encountered an error. // first time we fetch the policy and if we encountered an error.
@ -168,7 +167,7 @@ func deliverDirect(cid int64, qlog *mlog.Log, resolver dns.Resolver, dialer smtp
nqlog := qlog.WithCid(cid) nqlog := qlog.WithCid(cid)
var remoteIP net.IP var remoteIP net.IP
tlsMode := tlsModeDefault tlsMode := smtpclient.TLSOpportunistic
if policy != nil && policy.Mode == mtasts.ModeEnforce { if policy != nil && policy.Mode == mtasts.ModeEnforce {
tlsMode = smtpclient.TLSStrictStartTLS tlsMode = smtpclient.TLSStrictStartTLS
} }