next (#152)

Reviewed-on: #152

README.md

@@ -561,16 +561,17 @@ This approach ensures:
 - Ability to detect split-horizon or inconsistent responses across authoritative
   servers.
 
-dnswatcher never asks a server for recursion, so a reply marked "recursion
-available" did not come from the server it was sent to. It came from a recursive
-resolver on the network answering in that server's place, as on a network where
-a router rule, a VPN or a DNS filter sends all DNS to one resolver. Such a query
-is sent again over TCP, which many of those networks leave alone, and the reply
-over TCP is used; when that retry fails, the reply over UDP is kept. A root
-server never offers recursion, so a reply from a root server's address that is
-still marked so after that is passed over. When every root server refuses a
-query or gives such a reply, the lookup fails with the error "this network
-intercepts DNS queries".
+A root server never offers recursion, so a reply marked "recursion available"
+from a root server's address came from a recursive resolver on the network
+answering in its place, as on a network where a router rule, a VPN or a DNS
+filter sends all DNS to one resolver. The first such reply from a root server's
+address, a refusal included, stops the lookup at once with the error "this
+network intercepts DNS queries", naming that address, and no other root server
+is asked. A lookup that every root server refuses without that mark fails with
+the same error. Every lookup starts at the root servers, so on such a network
+every lookup fails with that error, whether or not that resolver has the name
+cached, and dnswatcher's tests fail there too. That is by design: nothing asks
+again another way or otherwise works around the interception.
 
 A watched name's records are stored as its nameservers return them, CNAME
 included. When they return a CNAME and no address, the chain of every CNAME

TODO.md

@@ -19,10 +19,10 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
 
 # Completed Steps
 
-- 2026-10-16: a reply over UDP marked "recursion available" is asked again over
-  TCP; such a reply from a root server's address is not used, and when every
-  root server refuses or gives one, the error is "this network intercepts DNS
-  queries" (closes #300).
+- 2026-10-16: the first reply marked "recursion available" from a root server's
+  address, a refusal included, stops the lookup with "this network intercepts
+  DNS queries", so on a network that sends DNS to a resolver dnswatcher and its
+  tests fail, by design (closes #300).
 - 2026-10-15: a query its caller's context ends before it is sent records no
   reason about the nameserver, and a try the context's deadline cuts short is a
   timeout, so a nameserver whose only failures were timeouts is `timeout`, not a

internal/resolver/errors.go

@@ -53,18 +53,13 @@ var (
 		"reply truncated and its retry over TCP failed",
 	)
 
-	// ErrRecursionAvailable is the reason a reply from a root server
-	// marked recursion available is not used. Root servers offer no
-	// recursion, so the reply came from something on the network
-	// answering in their place.
-	ErrRecursionAvailable = errors.New(
-		"reply marked recursion available, so not from a root server",
-	)
-
-	// ErrIntercepted is returned when every root server refused a
-	// query or gave a reply marked recursion available. Root servers do
-	// neither, so those replies came from something on the network
-	// answering in their place.
+	// ErrIntercepted is returned when a root server's address gave a
+	// reply marked recursion available, a refusal included, which ends
+	// the lookup at once, or when every root server refused a query.
+	// Root servers do neither, so those replies came from something on
+	// the network answering in their place. On such a network
+	// dnswatcher fails with it, by design: nothing works around the
+	// interception.
 	ErrIntercepted = errors.New("this network intercepts DNS queries")
 
 	// ErrCNAMEDepthExceeded is returned when a CNAME chain

internal/resolver/iterative.go

@@ -94,18 +94,15 @@ func (r *Resolver) tryExchange(
 }
 
 // retryTCP returns the reply to msg over TCP when resp, its reply over
-// UDP, is truncated or marked recursion available. msg does not ask for
-// recursion, so a reply marked so came from a recursive resolver on the
-// network answering in the server's place, and many networks that send
-// DNS to one do so only for UDP. When the retry fails it returns resp;
-// a truncated one holds only the records that fit.
+// UDP, is truncated. When that fails it returns resp, still truncated,
+// which holds only the records that fit.
 func (r *Resolver) retryTCP(
 	ctx context.Context,
 	msg *dns.Msg,
 	addr string,
 	resp *dns.Msg,
 ) *dns.Msg {
-	if !resp.Truncated && !resp.RecursionAvailable {
+	if !resp.Truncated {
 		return resp
 	}
 
@@ -118,9 +115,8 @@ func (r *Resolver) retryTCP(
 }
 
 // queryDNS sends a DNS query to a specific server IP, never asking it
-// for recursion. When the reply over UDP is truncated or marked
-// recursion available, the query is sent again over TCP (see retryTCP).
-// A reply of REFUSED is returned as ErrRefused.
+// for recursion. A reply of REFUSED is returned together with
+// ErrRefused, so that the caller can still read the reply's flags.
 func (r *Resolver) queryDNS(
 	ctx context.Context,
 	serverIP string,
@@ -143,15 +139,13 @@ func (r *Resolver) queryDNS(
 		return nil, fmt.Errorf("query %s @%s: %w", name, serverIP, err)
 	}
 
-	resp = r.retryTCP(ctx, msg, addr, resp)
-
 	if resp.Rcode == dns.RcodeRefused {
-		return nil, fmt.Errorf(
+		return resp, fmt.Errorf(
 			"query %s @%s: %w", name, serverIP, ErrRefused,
 		)
 	}
 
-	return resp, nil
+	return r.retryTCP(ctx, msg, addr, resp), nil
 }
 
 func extractNSSet(rrs []dns.RR) []string {
@@ -291,11 +285,15 @@ func shuffled(
 }
 
 // queryServers asks servers, the servers of zone, about name in a random
-// order until one gives a reply it uses (see replyError). A server that
-// times out, refuses or gives a reply it does not use is passed over for
-// the next. When every server refused, the error says so. When they are
-// the root servers and each one refused or gave a reply marked recursion
-// available, the error is ErrIntercepted.
+// order until one gives a usable reply. When they are the root servers,
+// the first reply marked recursion available, a refusal included, ends
+// the lookup with ErrIntercepted, naming the address it came from, and
+// no other root server is asked: root servers offer no recursion, so
+// that reply came from something on the network answering in their
+// place. Otherwise a server that times out, refuses or gives a reply
+// that is not usable is passed over for the next. When every server
+// refused, the error says so, and when they are the root servers it is
+// ErrIntercepted.
 func (r *Resolver) queryServers(
 	ctx context.Context,
 	servers []string,
@@ -306,16 +304,27 @@ func (r *Resolver) queryServers(
 	var lastErr error
 
 	refused := 0
-	recursionAvailable := 0
 
 	for _, ip := range shuffled(servers, rand.Shuffle) {
 		if checkCtx(ctx) != nil {
 			return nil, ErrContextCanceled
 		}
 
+		// queryDNS returns a refusal together with its reply, so a
+		// refusal's mark is checked here too.
 		resp, err := r.queryDNS(ctx, ip, name, qtype)
-		if err == nil {
-			err = replyError(resp, ip, zone, name)
+		if resp != nil && zone == "." && resp.RecursionAvailable {
+			return nil, fmt.Errorf(
+				"query %s @%s: reply marked recursion available, "+
+					"so not from a root server: %w",
+				name, ip, ErrIntercepted,
+			)
+		}
+
+		if err == nil && !usableReply(resp, zone, name) {
+			err = fmt.Errorf(
+				"query %s @%s: %w", name, ip, ErrUnusableReply,
+			)
 		}
 
 		if err == nil {
@@ -326,10 +335,6 @@ func (r *Resolver) queryServers(
 			refused++
 		}
 
-		if errors.Is(err, ErrRecursionAvailable) {
-			recursionAvailable++
-		}
-
 		lastErr = err
 	}
 
@@ -340,14 +345,6 @@ func (r *Resolver) queryServers(
 		)
 	}
 
-	if refused+recursionAvailable == len(servers) && zone == "." {
-		return nil, fmt.Errorf(
-			"every root server refused a query for %s or gave a reply "+
-				"marked recursion available: %w",
-			name, ErrIntercepted,
-		)
-	}
-
 	if refused == len(servers) {
 		return nil, fmt.Errorf(
 			"every server of %s refused a query for %s: %w",
@@ -358,27 +355,6 @@ func (r *Resolver) queryServers(
 	return nil, fmt.Errorf("all servers failed: %w", lastErr)
 }
 
-// replyError returns why resp, the reply of the server at ip, one of the
-// servers of zone, to a query about name, is not used, or nil when it
-// is. A reply that is not usable is not used. Nor is a reply from a root
-// server marked recursion available: root servers offer no recursion,
-// so it came from something on the network answering in their place.
-func replyError(resp *dns.Msg, ip string, zone string, name string) error {
-	if zone == "." && resp.RecursionAvailable {
-		return fmt.Errorf(
-			"query %s @%s: %w", name, ip, ErrRecursionAvailable,
-		)
-	}
-
-	if !usableReply(resp, zone, name) {
-		return fmt.Errorf(
-			"query %s @%s: %w", name, ip, ErrUnusableReply,
-		)
-	}
-
-	return nil
-}
-
 // isErrorReply reports whether msg is an error reply: one with any code
 // but NOERROR and NXDOMAIN, such as SERVFAIL, NOTIMP or FORMERR. An error
 // reply says nothing about the name's records.

internal/resolver/resolver_test.go

@@ -851,51 +851,70 @@ func TestQueryServers_NotEveryRootServerRefused(t *testing.T) {
 }
 
 // TestQueryServers_RecursiveResolversAsRootServers passes public
-// recursive resolvers of four operators to QueryServers as the servers
-// of the root zone, as a network that sends every DNS query to a
-// resolver would. Over UDP and over TCP alike, they mark every reply
-// recursion available, which no root server does. Each refuses the
-// query, replies SERVFAIL, or answers from its cache, as 1.1.1.1 does
-// for com.: a reply that would be used if a root server gave it. Either
-// way the error is ErrIntercepted, and since 1.1.1.1 and 8.8.8.8 refuse
-// no such query, it says a reply was marked recursion available.
+// recursive resolvers to QueryServers as the servers of the root zone,
+// as a network that sends every DNS query to a resolver would. Their
+// replies are marked recursion available, which no root server's is:
+// SERVFAIL, REFUSED, or an answer from a resolver's cache, as 1.1.1.1
+// gives for com., which would be used if a root server gave it. The
+// first such reply ends the lookup with ErrIntercepted, naming the
+// resolver that gave it. In the first group the replies differ; every
+// resolver in the second refuses a query that does not ask for
+// recursion, as 9.9.9.9 does, so there a refusal ends the lookup
+// instead of being passed over. Each group has resolvers of four
+// operators, and a resolver that gives no reply is passed over for the
+// next, so one of them answering is enough.
 func TestQueryServers_RecursiveResolversAsRootServers(t *testing.T) {
 	t.Parallel()
 
 	r := newTestResolver(t)
-	resolvers := []string{
-		"1.1.1.1", "8.8.8.8", "9.9.9.9", "208.67.222.222",
+	groups := [][]string{
+		{"1.1.1.1", "8.8.8.8", "9.9.9.9", "208.67.222.222"},
+		{"64.6.64.6", "185.222.222.222", "4.2.2.1", "94.140.14.14"},
 	}
 
-	for _, name := range []string{"com.", "desec.io."} {
-		var err error
+	for _, resolvers := range groups {
+		for _, name := range []string{"com.", "desec.io."} {
+			var err error
 
-		livednstest.Retry(
-			t,
-			"QueryServers(public recursive resolvers as root servers, "+
-				name+")",
-			func(ctx context.Context) error {
-				_, err = r.QueryServers(ctx, resolvers, ".", name, dns.TypeNS)
-
-				// When the error is another, one resolver may have given
-				// no reply at all, so the attempt is tried again.
-				if err != nil && !errors.Is(err, resolver.ErrIntercepted) {
-					return fmt.Errorf(
-						"%w: %w", livednstest.ErrNoAnswer, err,
+			livednstest.Retry(
+				t,
+				"QueryServers("+strings.Join(resolvers, ", ")+
+					" as root servers, "+name+")",
+				func(ctx context.Context) error {
+					_, err = r.QueryServers(
+						ctx, resolvers, ".", name, dns.TypeNS,
 					)
-				}
 
-				return nil
-			},
-		)
+					// When no resolver gave a reply marked recursion
+					// available, none may have replied at all, so the
+					// attempt is tried again.
+					if err != nil && !strings.Contains(
+						err.Error(), "marked recursion available",
+					) {
+						return fmt.Errorf(
+							"%w: %w", livednstest.ErrNoAnswer, err,
+						)
+					}
 
-		require.ErrorIs(t, err, resolver.ErrIntercepted)
-		require.EqualError(
-			t, err,
-			"every root server refused a query for "+name+" or gave a "+
-				"reply marked recursion available: "+
-				"this network intercepts DNS queries",
-		)
+					return nil
+				},
+			)
+
+			require.ErrorIs(t, err, resolver.ErrIntercepted)
+
+			// The error names the resolver whose reply ended the
+			// lookup.
+			messages := make([]string, 0, len(resolvers))
+			for _, ip := range resolvers {
+				messages = append(messages,
+					"query "+name+" @"+ip+": reply marked recursion "+
+						"available, so not from a root server: "+
+						"this network intercepts DNS queries",
+				)
+			}
+
+			require.Contains(t, messages, err.Error())
+		}
 	}
 }