Skip to content

Commit

Permalink
admin: better handling of disabled mta-sts during self-check
Browse files Browse the repository at this point in the history
if admin has disabled mta-sts for a domain, we still check for records &
policies, but won't mark it as error when they don't exist. we do now keep
warning that mta-sts isn't enabled, otherwise we would start showing a green
"ok".

this also fixes the mta-sts code returning ErrNoPolicy when mtasts.<domain>
doesn't exist. the dns lookup is done with the reguler "net" package dns lookup
code, not through adns, so we look for two types of DNSError's.

noticed a while ago when testing with MTA-STS while debugging TLS connection
issues with MS.
  • Loading branch information
mjl- committed Nov 24, 2024
1 parent 726c093 commit 7f5e108
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 5 deletions.
10 changes: 7 additions & 3 deletions dns/dns.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ package dns
import (
"errors"
"fmt"
"net"
"strings"

"golang.org/x/net/idna"
Expand Down Expand Up @@ -148,7 +149,9 @@ func ParseDomainLax(s string) (Domain, error) {
return Domain{ASCII: s}, nil
}

// IsNotFound returns whether an error is an adns.DNSError with IsNotFound set.
// IsNotFound returns whether an error is an adns.DNSError or net.DNSError with
// IsNotFound set.
//
// IsNotFound means the requested type does not exist for the given domain (a
// nodata or nxdomain response). It doesn't not necessarily mean no other types for
// that name exist.
Expand All @@ -158,6 +161,7 @@ func ParseDomainLax(s string) (Domain, error) {
// The adns resolver (just like the Go resolver) returns an IsNotFound error for
// both cases, there is no need to explicitly check for zero entries.
func IsNotFound(err error) bool {
var dnsErr *adns.DNSError
return err != nil && errors.As(err, &dnsErr) && dnsErr.IsNotFound
var adnsErr *adns.DNSError
var dnsErr *net.DNSError
return err != nil && (errors.As(err, &adnsErr) && adnsErr.IsNotFound || errors.As(err, &dnsErr) && dnsErr.IsNotFound)
}
11 changes: 9 additions & 2 deletions webadmin/admin.go
Original file line number Diff line number Diff line change
Expand Up @@ -1281,8 +1281,13 @@ Ensure a DNS TXT record like the following exists:
defer logPanic(ctx)
defer wg.Done()

// The admin has explicitly disabled mta-sts, keep warning about it.
if domConf.MTASTS == nil {
addf(&r.MTASTS.Warnings, "MTA-STS is not configured for this domain.")
}

record, txt, err := mtasts.LookupRecord(ctx, log.Logger, resolver, domain)
if err != nil {
if err != nil && !(domConf.MTASTS == nil && errors.Is(err, mtasts.ErrNoRecord)) {
addf(&r.MTASTS.Errors, "Looking up MTA-STS record: %s", err)
}
r.MTASTS.TXT = txt
Expand All @@ -1292,7 +1297,9 @@ Ensure a DNS TXT record like the following exists:

policy, text, err := mtasts.FetchPolicy(ctx, log.Logger, domain)
if err != nil {
addf(&r.MTASTS.Errors, "Fetching MTA-STS policy: %s", err)
if !(domConf.MTASTS == nil && errors.Is(err, mtasts.ErrNoPolicy)) {
addf(&r.MTASTS.Errors, "Fetching MTA-STS policy: %s", err)
}
} else if policy.Mode == mtasts.ModeNone {
addf(&r.MTASTS.Warnings, "MTA-STS policy is present, but does not require TLS.")
} else if policy.Mode == mtasts.ModeTesting {
Expand Down

0 comments on commit 7f5e108

Please sign in to comment.