From d1024b2b20a5e425a37245e7f5c6abd375382dd5 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 16 Oct 2026 07:44:24 +0000 Subject: [PATCH] resolver: ask again over TCP when a reply offers recursion (closes #300) dnswatcher never asks for recursion, so a reply marked recursion available comes from a resolver on the network answering in the server's place, as on a network that sends all DNS to one resolver. Such a resolver answers only from its cache, or with SERVFAIL or REFUSED, which made make check fail there. A reply over UDP marked so is now asked again over TCP, which many such networks leave alone, and the TCP reply is used. A reply from a root server's address still marked so is not used, and when every root server refuses or gives one, the error is the existing "this network intercepts DNS queries". Model: opus-5-5 --- README.md | 11 +++++ TODO.md | 4 ++ internal/resolver/errors.go | 13 +++++- internal/resolver/iterative.go | 66 ++++++++++++++++++++++++------ internal/resolver/resolver_test.go | 49 ++++++++++++++++++++++ 5 files changed, 128 insertions(+), 15 deletions(-) diff --git a/README.md b/README.md index 49e220a..989cd71 100644 --- a/README.md +++ b/README.md @@ -561,6 +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 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 target they gave is followed (with a depth limit to prevent loops) to the A and diff --git a/TODO.md b/TODO.md index 4a5fdf8..69203b2 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +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-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 diff --git a/internal/resolver/errors.go b/internal/resolver/errors.go index a9b109d..4f3711b 100644 --- a/internal/resolver/errors.go +++ b/internal/resolver/errors.go @@ -53,9 +53,18 @@ 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. Root servers refuse no query, so the refusals came from - // something on the network answering in their place. + // 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 = errors.New("this network intercepts DNS queries") // ErrCNAMEDepthExceeded is returned when a CNAME chain diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index 9156373..fb874a8 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -94,15 +94,18 @@ func (r *Resolver) tryExchange( } // retryTCP returns the reply to msg over TCP when resp, its reply over -// UDP, is truncated. When that fails it returns resp, still truncated, -// which holds only the records that fit. +// 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. func (r *Resolver) retryTCP( ctx context.Context, msg *dns.Msg, addr string, resp *dns.Msg, ) *dns.Msg { - if !resp.Truncated { + if !resp.Truncated && !resp.RecursionAvailable { return resp } @@ -115,7 +118,9 @@ func (r *Resolver) retryTCP( } // queryDNS sends a DNS query to a specific server IP, never asking it -// for recursion. A reply of REFUSED is returned as ErrRefused. +// 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. func (r *Resolver) queryDNS( ctx context.Context, serverIP string, @@ -138,13 +143,15 @@ 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( "query %s @%s: %w", name, serverIP, ErrRefused, ) } - return r.retryTCP(ctx, msg, addr, resp), nil + return resp, nil } func extractNSSet(rrs []dns.RR) []string { @@ -284,10 +291,11 @@ func shuffled( } // queryServers asks servers, the servers of zone, about name in a random -// order until one gives a usable reply. 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. +// 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. func (r *Resolver) queryServers( ctx context.Context, servers []string, @@ -298,6 +306,7 @@ func (r *Resolver) queryServers( var lastErr error refused := 0 + recursionAvailable := 0 for _, ip := range shuffled(servers, rand.Shuffle) { if checkCtx(ctx) != nil { @@ -305,10 +314,8 @@ func (r *Resolver) queryServers( } resp, err := r.queryDNS(ctx, ip, name, qtype) - if err == nil && !usableReply(resp, zone, name) { - err = fmt.Errorf( - "query %s @%s: %w", name, ip, ErrUnusableReply, - ) + if err == nil { + err = replyError(resp, ip, zone, name) } if err == nil { @@ -319,6 +326,10 @@ func (r *Resolver) queryServers( refused++ } + if errors.Is(err, ErrRecursionAvailable) { + recursionAvailable++ + } + lastErr = err } @@ -329,6 +340,14 @@ 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", @@ -339,6 +358,27 @@ 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. diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 9d105b7..b386f3e 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -850,6 +850,55 @@ func TestQueryServers_NotEveryRootServerRefused(t *testing.T) { require.NotContains(t, err.Error(), "refused a query for") } +// 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. +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", + } + + 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, + ) + } + + return nil + }, + ) + + 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", + ) + } +} + func TestQueryNameserver_RecordsSorted(t *testing.T) { t.Parallel()