From 7f5e1087d42d5d7bf57649c5c6a2155c3706ce39 Mon Sep 17 00:00:00 2001
From: Mechiel Lukkien <mechiel@ueber.net>
Date: Sun, 24 Nov 2024 13:30:29 +0100
Subject: [PATCH] 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.
---
 dns/dns.go        | 10 +++++++---
 webadmin/admin.go | 11 +++++++++--
 2 files changed, 16 insertions(+), 5 deletions(-)

diff --git a/dns/dns.go b/dns/dns.go
index 083b976..f792149 100644
--- a/dns/dns.go
+++ b/dns/dns.go
@@ -5,6 +5,7 @@ package dns
 import (
 	"errors"
 	"fmt"
+	"net"
 	"strings"
 
 	"golang.org/x/net/idna"
@@ -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.
@@ -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)
 }
diff --git a/webadmin/admin.go b/webadmin/admin.go
index 3ea7466..10f38e9 100644
--- a/webadmin/admin.go
+++ b/webadmin/admin.go
@@ -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
@@ -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 {