From 031e59561636dd149890d6d9e39ce225cc0d509c Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Fri, 2 Oct 2026 05:54:23 +0000 Subject: [PATCH] resolver: ask a referral's nameservers that come without addresses (closes #221) Looking up a nameserver's own address followed only the addresses a referral gave, so a nameserver whose zone is delegated without them, such as a.ntpns.org of pool.ntp.org, never resolved. The walk to a name's nameservers looked addresses up only when a referral gave none, so when it gave some it asked only those. Both now ask the nameservers whose addresses the referral gives first and, if none of them gives a usable reply, look up and ask the others; with no addresses given, all are looked up, as before. Looking up all of them at once sent too many queries to the root servers. maxLookupDepth stops lookups two deep, so delegations that point at each other still end. Model: opus-5-5 --- README.md | 7 +- TODO.md | 2 + internal/resolver/export_test.go | 23 +++++- internal/resolver/iterative.go | 125 +++++++++++++++++++++-------- internal/resolver/resolver_test.go | 79 ++++++++++++++++++ 5 files changed, 199 insertions(+), 37 deletions(-) diff --git a/README.md b/README.md index 4f14c1d..d69dbc2 100644 --- a/README.md +++ b/README.md @@ -412,7 +412,12 @@ anew each time, so no one root server gets every first query. A server that does not reply, refuses the query, or gives an error reply such as SERVFAIL or a referral that leads no closer to the name is passed over for the next one. When a referral names a zone's nameservers without their addresses, the addresses of -all of them are looked up, so that each can be asked. +all of them are looked up, so that each can be asked. When it gives addresses +for only some of them, those are asked first, and the others are looked up and +asked only if none of those gives a usable reply. Both hold in the walk to a +name's nameservers and in the lookup of a nameserver's own address. Such a +lookup can need others in turn; lookups go at most two deep, one inside another, +so delegations that point at each other still end. This approach ensures: diff --git a/TODO.md b/TODO.md index 927a598..f8380ad 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ trial run of the finished image: https://git.eeqj.de/sneak/dnswatcher/issues/149 # Completed Steps +- 2026-10-02: nameservers a referral gives no address for are looked up when + needed, two deep at most; `pool.ntp.org`'s nameservers resolve (closes #221). - 2026-10-02: a resolver test that reads one record type from a nameserver's answer asks again when that type is missing from it (closes #218). - 2026-10-02: a plain `docker build .` of a clone stamps its tag or short diff --git a/internal/resolver/export_test.go b/internal/resolver/export_test.go index 17c57d7..9109c7a 100644 --- a/internal/resolver/export_test.go +++ b/internal/resolver/export_test.go @@ -48,12 +48,31 @@ func (r *Resolver) QueryEachNS( return r.queryEachNS(ctx, nameservers, hostname, recordTypes()) } -// ResolveNSIPs exports resolveNSIPs for testing. +// ResolveNSIPs exports resolveNSIPs for testing, looking each name up +// as a lookup that no other lookup started. func (r *Resolver) ResolveNSIPs( ctx context.Context, nsNames []string, ) []string { - return r.resolveNSIPs(ctx, nsNames) + return r.resolveNSIPs(ctx, nsNames, 1) +} + +// MaxLookupDepth exports maxLookupDepth for testing. +const MaxLookupDepth = maxLookupDepth + +// QueryNameservers exports queryNameservers for testing. +func (r *Resolver) QueryNameservers( + ctx context.Context, + given []string, + withoutAddresses []string, + zone string, + name string, + qtype uint16, + depth int, +) (*dns.Msg, error) { + return r.queryNameservers( + ctx, given, withoutAddresses, zone, name, qtype, depth, + ) } // RootServerList exports rootServerList for testing. diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index 1f06a24..ccf6160 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -19,6 +19,14 @@ const ( maxRetries = 2 maxDelegation = 20 timeoutMultiplier = 2 + + // maxLookupDepth is how many lookups of nameserver addresses may be + // under way one inside another. Looking up a nameserver's address + // can meet a referral that names nameservers without their + // addresses, which are then looked up in turn; without a limit, + // delegations that point at each other would never end. Each level + // multiplies the queries sent. + maxLookupDepth = 2 ) // ErrRefused is returned when a DNS server refuses a query. @@ -198,13 +206,15 @@ func (r *Resolver) followDelegation( // servers are the root servers, the servers of zone ".". zone := "." + var withoutAddresses []string + for range maxDelegation { if checkCtx(ctx) != nil { return nil, ErrContextCanceled } - resp, err := r.queryServers( - ctx, servers, zone, domain, dns.TypeNS, + resp, err := r.queryNameservers( + ctx, servers, withoutAddresses, zone, domain, dns.TypeNS, 0, ) if err != nil { return nil, err @@ -229,18 +239,7 @@ func (r *Resolver) followDelegation( return r.resolveNSIterative(ctx, domain) } - glue := extractGlue(resp.Extra) - nextServers := glueIPs(authNS, glue) - - if len(nextServers) == 0 { - nextServers = r.resolveNSIPs(ctx, authNS) - } - - if len(nextServers) == 0 { - return nil, ErrNoNameservers - } - - servers = nextServers + servers, withoutAddresses = referralNameservers(resp) zone = referralZone(resp) } @@ -366,18 +365,80 @@ func nsSetFrom(resp *dns.Msg, domain string) []string { return extractNSSet(resp.Answer) } +// referralNameservers returns the IPv4 addresses that resp, a referral, +// gives for the nameservers it names, and the names of the nameservers +// it gives no address for. +func referralNameservers(resp *dns.Msg) ([]string, []string) { + glue := extractGlue(resp.Extra) + + var given, withoutAddresses []string + + for _, ns := range extractNSSet(resp.Ns) { + ips := glueIPs([]string{ns}, glue) + if len(ips) == 0 { + withoutAddresses = append(withoutAddresses, ns) + } + + given = append(given, ips...) + } + + return given, withoutAddresses +} + +// queryNameservers asks the servers of zone about name as queryServers +// does: first those at given, the addresses a referral gave, and only +// when none of them gives a usable reply, the nameservers named +// withoutAddresses, once their addresses are looked up. depth is how +// many lookups of a nameserver's address are under way, 0 in the walk +// to a domain's nameservers; at maxLookupDepth, no address is looked +// up. +func (r *Resolver) queryNameservers( + ctx context.Context, + given []string, + withoutAddresses []string, + zone string, + name string, + qtype uint16, + depth int, +) (*dns.Msg, error) { + err := fmt.Errorf( + "no address for any nameserver of %s: %w", zone, ErrNoNameservers, + ) + + if len(given) > 0 { + var resp *dns.Msg + + resp, err = r.queryServers(ctx, given, zone, name, qtype) + if err == nil { + return resp, nil + } + } + + if len(withoutAddresses) == 0 || depth >= maxLookupDepth { + return nil, err + } + + lookedUp := r.resolveNSIPs(ctx, withoutAddresses, depth+1) + if len(lookedUp) == 0 { + return nil, err + } + + return r.queryServers(ctx, lookedUp, zone, name, qtype) +} + // resolveNSIPs returns the addresses of every nameserver in nsNames -// whose name resolves, for a referral that carries none. The walk can -// then go on to the zone's other nameservers when one gives no usable -// reply. +// whose name resolves, each looked up at depth (see resolveARecord). +// The walk can then go on to the zone's other nameservers when one +// gives no usable reply. func (r *Resolver) resolveNSIPs( ctx context.Context, nsNames []string, + depth int, ) []string { var ips []string for _, ns := range nsNames { - resolved, err := r.resolveARecord(ctx, ns) + resolved, err := r.resolveARecord(ctx, ns, depth) if err == nil { ips = append(ips, resolved...) } @@ -438,11 +499,14 @@ func (r *Resolver) resolveNSIterative( return nil, ErrNoNameservers } -// resolveARecord resolves a hostname to IPv4 addresses using -// iterative resolution through the delegation chain. +// resolveARecord resolves a hostname, a nameserver's name, to IPv4 +// addresses using iterative resolution through the delegation chain. +// depth is how many lookups of a nameserver's address are under way, +// this one included: 1 for a lookup that no other lookup started. func (r *Resolver) resolveARecord( ctx context.Context, hostname string, + depth int, ) ([]string, error) { if checkCtx(ctx) != nil { return nil, ErrContextCanceled @@ -452,13 +516,16 @@ func (r *Resolver) resolveARecord( servers := rootServerList() zone := "." + var withoutAddresses []string + for range maxDelegation { if checkCtx(ctx) != nil { return nil, ErrContextCanceled } - resp, err := r.queryServers( - ctx, servers, zone, hostname, dns.TypeA, + resp, err := r.queryNameservers( + ctx, servers, withoutAddresses, zone, hostname, dns.TypeA, + depth, ) if err != nil { return nil, fmt.Errorf( @@ -485,17 +552,7 @@ func (r *Resolver) resolveARecord( break } - glue := extractGlue(resp.Extra) - nextServers := glueIPs(authNS, glue) - - if len(nextServers) == 0 { - // Resolve NS IPs iteratively — but guard - // against infinite recursion by using only - // already-resolved servers. - break - } - - servers = nextServers + servers, withoutAddresses = referralNameservers(resp) zone = referralZone(resp) } @@ -584,7 +641,7 @@ func (r *Resolver) queryNameserver( return nil, ErrContextCanceled } - nsIPs, err := r.resolveARecord(ctx, nsHostname) + nsIPs, err := r.resolveARecord(ctx, nsHostname, 1) if err != nil { return nil, fmt.Errorf("resolving NS %s: %w", nsHostname, err) } diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index a07c718..ab4ebf3 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -162,6 +162,85 @@ func TestResolveNSIPs_EveryNameserver(t *testing.T) { assert.ElementsMatch(t, want, got) } +// TestResolveNSIPs_ZoneDelegatedWithoutAddresses looks up the address +// of a.ntpns.org, a nameserver of pool.ntp.org. The org servers delegate +// ntpns.org to nameservers in other zones and give none of their +// addresses, so those are looked up on the way. +func TestResolveNSIPs_ZoneDelegatedWithoutAddresses(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + ips := liveResolveNSIPs(t, r, []string{"a.ntpns.org."}, 1) + + for _, ip := range ips { + assert.NotNil(t, net.ParseIP(ip), "should be valid IP: %s", ip) + } +} + +// TestQueryNameservers_GivenAddressesFail asks the servers of ntp.org +// about pool.ntp.org, as the walk to a name under ntp.org does after the +// org servers' referral. That referral names four nameservers and gives +// an address for ns1.everett.org alone; here the given address is +// 192.0.2.1, where nothing answers, so the other three must be looked +// up and asked. +func TestQueryNameservers_GivenAddressesFail(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + + var resp *dns.Msg + + livednstest.Retry( + t, + "QueryNameservers(192.0.2.1 and three ntp.org nameservers, "+ + "pool.ntp.org)", + func(ctx context.Context) error { + var err error + + resp, err = r.QueryNameservers( + ctx, []string{"192.0.2.1"}, + []string{"anyns.pch.net.", "dns1.udel.edu.", "dns2.udel.edu."}, + "ntp.org.", "pool.ntp.org.", dns.TypeNS, 0, + ) + + return err + }, + ) + + assert.NotEmpty(t, resolver.NSSetFrom(resp, "pool.ntp.org.")) +} + +// TestQueryNameservers_LookupDepth asks the servers of desec.io about +// ns1.desec.io, giving no address and naming ns1.desec.io without one. +// Below maxLookupDepth its address is looked up and it is asked. At the +// limit it is not, so nothing can be asked: this is where lookups of +// nameserver addresses stop when delegations point at each other. +func TestQueryNameservers_LookupDepth(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + withoutAddresses := []string{"ns1.desec.io."} + + livednstest.Retry( + t, + "QueryNameservers(ns1.desec.io without its address, ns1.desec.io)", + func(ctx context.Context) error { + _, err := r.QueryNameservers( + ctx, nil, withoutAddresses, "desec.io.", "ns1.desec.io.", + dns.TypeA, 0, + ) + + return err + }, + ) + + _, err := r.QueryNameservers( + t.Context(), nil, withoutAddresses, "desec.io.", "ns1.desec.io.", + dns.TypeA, resolver.MaxLookupDepth, + ) + require.ErrorIs(t, err, resolver.ErrNoNameservers) +} + // ---------------------------------------------------------------- // QueryNameserver tests // ----------------------------------------------------------------