admin: better handling of disabled mta-sts during self-check

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.
This commit is contained in:
Mechiel Lukkien 2024-11-24 13:30:29 +01:00
parent 726c0931f7
commit 7f5e1087d4
No known key found for this signature in database
2 changed files with 16 additions and 5 deletions

View file

@ -5,6 +5,7 @@ package dns
import ( import (
"errors" "errors"
"fmt" "fmt"
"net"
"strings" "strings"
"golang.org/x/net/idna" "golang.org/x/net/idna"
@ -148,7 +149,9 @@ func ParseDomainLax(s string) (Domain, error) {
return Domain{ASCII: s}, nil 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 // 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 // nodata or nxdomain response). It doesn't not necessarily mean no other types for
// that name exist. // that name exist.
@ -158,6 +161,7 @@ func ParseDomainLax(s string) (Domain, error) {
// The adns resolver (just like the Go resolver) returns an IsNotFound error for // 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. // both cases, there is no need to explicitly check for zero entries.
func IsNotFound(err error) bool { func IsNotFound(err error) bool {
var dnsErr *adns.DNSError var adnsErr *adns.DNSError
return err != nil && errors.As(err, &dnsErr) && dnsErr.IsNotFound var dnsErr *net.DNSError
return err != nil && (errors.As(err, &adnsErr) && adnsErr.IsNotFound || errors.As(err, &dnsErr) && dnsErr.IsNotFound)
} }

View file

@ -1281,8 +1281,13 @@ Ensure a DNS TXT record like the following exists:
defer logPanic(ctx) defer logPanic(ctx)
defer wg.Done() 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) 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) addf(&r.MTASTS.Errors, "Looking up MTA-STS record: %s", err)
} }
r.MTASTS.TXT = txt r.MTASTS.TXT = txt
@ -1292,7 +1297,9 @@ Ensure a DNS TXT record like the following exists:
policy, text, err := mtasts.FetchPolicy(ctx, log.Logger, domain) policy, text, err := mtasts.FetchPolicy(ctx, log.Logger, domain)
if err != nil { if err != nil {
if !(domConf.MTASTS == nil && errors.Is(err, mtasts.ErrNoPolicy)) {
addf(&r.MTASTS.Errors, "Fetching MTA-STS policy: %s", err) addf(&r.MTASTS.Errors, "Fetching MTA-STS policy: %s", err)
}
} else if policy.Mode == mtasts.ModeNone { } else if policy.Mode == mtasts.ModeNone {
addf(&r.MTASTS.Warnings, "MTA-STS policy is present, but does not require TLS.") addf(&r.MTASTS.Warnings, "MTA-STS policy is present, but does not require TLS.")
} else if policy.Mode == mtasts.ModeTesting { } else if policy.Mode == mtasts.ModeTesting {