Skip to content

Commit

Permalink
fix verifying dane-ta connections for outgoing email where the dane-t…
Browse files Browse the repository at this point in the history
…a record is not for the first certificate in the chain after the leaf certifiate.

tls servers send a list of certificates for the connection. the first is the
leaf certificate. that's the one for the server itself. that's the one we want
to verify. the others are intermediate CA's. and possibly even the root CA
certificate that it hopes is trusted at the client (though sending it doesn't
make it trusted). with dane-ta, the public key of an intermediate or root CA
certificate is listed in the TSLA record. when verifying, we add any
intermediate/root CA that matches a dane-ta tlsa record to the trusted root CA
certs. we should also have added CA certs that didn't match a TLSA record to
the "intermediates" of x509.VerifyOptions. because we didn't,
x509.Certificate.Verify couldn't verify the chain from the trusted dane-ta ca
cert to the leaf cert. we would only properly verify a dane-ta connection
correctly if the dane-ta-trusted ca cert was the one immediately following the
leaf cert. not when there were one or more additional intermediate certs.

this showed when connecting to mx.runbox.com.

problem reported by robbo5000 on matrix, thanks!
  • Loading branch information
mjl- committed Dec 21, 2024
1 parent aa9a066 commit f7666d1
Showing 1 changed file with 5 additions and 2 deletions.
7 changes: 5 additions & 2 deletions dane/dane.go
Original file line number Diff line number Diff line change
Expand Up @@ -448,7 +448,8 @@ func verifySingle(log mlog.Log, tlsa adns.TLSA, cs tls.ConnectionState, allowedH
// We set roots, so the system defaults don't get used. Verify checks the host name
// (set below) and checks for expiration.
opts := x509.VerifyOptions{
Roots: x509.NewCertPool(),
Intermediates: x509.NewCertPool(),
Roots: x509.NewCertPool(),
}

// If the full certificate was included, we must add it to the valid roots, the TLS
Expand All @@ -465,11 +466,13 @@ func verifySingle(log mlog.Log, tlsa adns.TLSA, cs tls.ConnectionState, allowedH
}
}

for _, cert := range cs.PeerCertificates {
for i, cert := range cs.PeerCertificates {
if match(cert) {
opts.Roots.AddCert(cert)
found = true
break
} else if i > 0 {
opts.Intermediates.AddCert(cert)
}
}
if !found {
Expand Down

0 comments on commit f7666d1

Please sign in to comment.