resolver: never resend a refused query asking for recursion (closes #206)

queryDNS resent a query that a server refused, this time asking for recursion, so on a network that intercepts DNS the answers could come from a recursive resolver without anyone knowing. A refusal is now only a refusal, and the server is passed over for the next. When every server of a zone refuses, the error says so. When every root server refuses, the error is ErrIntercepted: root servers refuse no query, so something on the network is answering in their place. FindAuthoritativeNameservers stops at that error instead of trying each parent name, so the watcher's log line says it. A live test asks Quad9, which refuses a query not asking for recursion, so that the resend cannot come back unnoticed. Model: opus-5-5
2026-10-02 06:10:54 +02:00
parent a67fd20e4f
commit 889e17459b
5 changed files with 234 additions and 21 deletions
@@ -11,6 +11,7 @@ import (
 	"testing"
 	"time"

+	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"

@@ -295,6 +296,187 @@ func TestQueryNameserver_Refused(t *testing.T) {
 	assert.Equal(t, "server returned REFUSED", resp.Error)
 }

+// TestQueryNameserverIP_RecursiveResolverRefused asks Quad9, a public
+// recursive resolver, about google.com at both of its addresses. Quad9
+// refuses a query that does not ask for recursion and answers one that
+// does. The resolver never asks for recursion, so it must be reported
+// as refusing, never as answering.
+func TestQueryNameserverIP_RecursiveResolverRefused(t *testing.T) {
+	t.Parallel()
+
+	r := newTestResolver(t)
+
+	for _, ip := range []string{"9.9.9.9", "149.112.112.112"} {
+		var resp *resolver.NameserverResponse
+
+		livednstest.Retry(
+			t,
+			"QueryNameserverIP("+ip+", google.com)",
+			func(ctx context.Context) error {
+				var err error
+
+				resp, err = r.QueryNameserverIP(
+					ctx, ip, ip, "google.com",
+				)
+				if err != nil {
+					return err
+				}
+
+				// A timeout or a network error is no reply at all.
+				if resp.Status == resolver.StatusTimeout ||
+					strings.HasPrefix(resp.Error, "network error") {
+					return fmt.Errorf(
+						"%w: %s: %s",
+						livednstest.ErrNoAnswer, ip, resp.Error,
+					)
+				}
+
+				return nil
+			},
+		)
+
+		assert.Equal(t, resolver.StatusError, resp.Status, ip)
+		assert.Equal(t, "server returned REFUSED", resp.Error, ip)
+	}
+}
+
+// googleNameserverIPv4s returns the IPv4 addresses of google.com's
+// nameservers, the only addresses the resolver asks servers at.
+func googleNameserverIPv4s(t *testing.T, r *resolver.Resolver) []string {
+	t.Helper()
+
+	names := liveFindAuthoritative(t, r, "google.com")
+
+	return liveResolveNSIPs(t, r, names, len(names))
+}
+
+// TestQueryServers_EveryServerRefused asks all of google.com's
+// nameservers about cloudflare.com, a zone they do not serve, which
+// they all refuse. The error says every server refused; it is not
+// ErrIntercepted, which only the root servers refusing shows.
+func TestQueryServers_EveryServerRefused(t *testing.T) {
+	t.Parallel()
+
+	r := newTestResolver(t)
+	servers := googleNameserverIPv4s(t, r)
+
+	var err error
+
+	livednstest.Retry(
+		t,
+		"QueryServers(google.com servers, cloudflare.com)",
+		func(ctx context.Context) error {
+			_, err = r.QueryServers(
+				ctx, servers, "google.com.", "cloudflare.com.",
+				dns.TypeNS,
+			)
+
+			// When not every server refused, one may have given no
+			// reply at all, so the attempt is tried again.
+			if err != nil &&
+				!strings.HasPrefix(err.Error(), "every server of") {
+				return fmt.Errorf(
+					"%w: %w", livednstest.ErrNoAnswer, err,
+				)
+			}
+
+			return nil
+		},
+	)
+
+	require.ErrorIs(t, err, resolver.ErrRefused)
+	require.NotErrorIs(t, err, resolver.ErrIntercepted)
+	require.EqualError(
+		t, err,
+		"every server of google.com. refused a query for "+
+			"cloudflare.com.: dns query refused",
+	)
+}
+
+// TestQueryServers_EveryRootServerRefused passes google.com's
+// nameservers to QueryServers as the servers of the root zone. They
+// refuse a query about cloudflare.com, as root servers would if
+// something on the network answered in their place, so the error is
+// ErrIntercepted.
+func TestQueryServers_EveryRootServerRefused(t *testing.T) {
+	t.Parallel()
+
+	r := newTestResolver(t)
+	servers := googleNameserverIPv4s(t, r)
+
+	var err error
+
+	livednstest.Retry(
+		t,
+		"QueryServers(google.com servers as root servers, cloudflare.com)",
+		func(ctx context.Context) error {
+			_, err = r.QueryServers(
+				ctx, servers, ".", "cloudflare.com.", dns.TypeNS,
+			)
+
+			// When not every server refused, one may have given no
+			// reply at all, so the attempt is tried again. Both errors
+			// for every server refusing say "refused a query for".
+			if err != nil &&
+				!strings.Contains(err.Error(), "refused a query for") {
+				return fmt.Errorf(
+					"%w: %w", livednstest.ErrNoAnswer, err,
+				)
+			}
+
+			return nil
+		},
+	)
+
+	require.ErrorIs(t, err, resolver.ErrIntercepted)
+	require.EqualError(
+		t, err,
+		"every root server refused a query for cloudflare.com.: "+
+			"this network intercepts DNS queries",
+	)
+}
+
+// TestQueryServers_NotEveryRootServerRefused passes google.com's
+// nameservers and 192.0.2.1 to QueryServers as the servers of the root
+// zone. The google.com nameservers refuse a query about cloudflare.com,
+// but nothing answers at 192.0.2.1, a documentation address, so not
+// every server refused, wherever 192.0.2.1 falls in the random order:
+// the error is not ErrIntercepted and does not say every server refused.
+func TestQueryServers_NotEveryRootServerRefused(t *testing.T) {
+	t.Parallel()
+
+	r := newTestResolver(t)
+	servers := googleNameserverIPv4s(t, r)
+	servers = append(servers, "192.0.2.1")
+
+	var err error
+
+	livednstest.Retry(
+		t,
+		"QueryServers(google.com servers and 192.0.2.1, cloudflare.com)",
+		func(ctx context.Context) error {
+			_, err = r.QueryServers(
+				ctx, servers, ".", "cloudflare.com.", dns.TypeNS,
+			)
+
+			// An attempt that ran out of time may not have asked every
+			// server, so it is tried again.
+			if ctx.Err() != nil {
+				return fmt.Errorf(
+					"%w: %w", livednstest.ErrNoAnswer, err,
+				)
+			}
+
+			return nil
+		},
+	)
+
+	require.Error(t, err)
+	require.NotErrorIs(t, err, resolver.ErrIntercepted)
+	// Both errors for every server refusing say "refused a query for".
+	require.NotContains(t, err.Error(), "refused a query for")
+}
+
 func TestQueryNameserver_RecordsSorted(t *testing.T) {
 	t.Parallel()