diff --git a/README.md b/README.md index daea081..ef8f163 100644 --- a/README.md +++ b/README.md @@ -432,7 +432,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 three 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 f71bbcf..114f8e5 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 names without addresses are looked up, + three deep at most; `pool.ntp.org`'s nameservers resolve (closes #221). - 2026-10-02: the dashboard and `/api/v1/status` show why a nameserver query or a certificate check failed, which only the state file showed (closes #225). - 2026-10-02: a name's CNAME is stored once per nameserver, not once per record diff --git a/internal/resolver/errors.go b/internal/resolver/errors.go index 5e019c0..f603217 100644 --- a/internal/resolver/errors.go +++ b/internal/resolver/errors.go @@ -33,6 +33,13 @@ var ( "CNAME chain depth exceeded", ) + // ErrLookupDepthExceeded is returned when nameserver addresses + // were not looked up because lookups were already maxLookupDepth + // deep, one inside another. + ErrLookupDepthExceeded = errors.New( + "lookups of nameserver addresses go too deep", + ) + // ErrContextCanceled wraps context cancellation for the // resolver's iterative queries. ErrContextCanceled = errors.New("context canceled") diff --git a/internal/resolver/export_test.go b/internal/resolver/export_test.go index 58b88b1..07c293b 100644 --- a/internal/resolver/export_test.go +++ b/internal/resolver/export_test.go @@ -55,12 +55,33 @@ 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) + ips, _ := r.resolveNSIPs(ctx, nsNames, 1) + + return ips +} + +// MaxLookupDepth exports maxLookupDepth for testing. +const MaxLookupDepth = maxLookupDepth + +// QueryZone exports queryZone for testing. +func (r *Resolver) QueryZone( + ctx context.Context, + given []string, + withoutAddresses []string, + zone string, + name string, + qtype uint16, + depth int, +) (*dns.Msg, error) { + return r.queryZone( + 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 6ffedc5..5686622 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -19,6 +19,16 @@ 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. pool.ntp.org needs three: the + // address of its nameserver g.ntpns.org can need a.ntpns.org's, + // which needs a bitnames.com nameserver's. + maxLookupDepth = 3 ) // ErrRefused is returned when a DNS server refuses a query. @@ -198,13 +208,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.queryZone( + ctx, servers, withoutAddresses, zone, domain, dns.TypeNS, 0, ) if err != nil { return nil, err @@ -229,18 +241,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,24 +367,110 @@ func nsSetFrom(resp *dns.Msg, domain string) []string { return extractNSSet(resp.Answer) } -// 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. -func (r *Resolver) resolveNSIPs( - ctx context.Context, - nsNames []string, -) []string { - var ips []string +// 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) - for _, ns := range nsNames { - resolved, err := r.resolveARecord(ctx, ns) + 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 +} + +// queryZone 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. When the limit is why none was found, here or in a lookup this +// one started, the error is ErrLookupDepthExceeded. +func (r *Resolver) queryZone( + 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 { - ips = append(ips, resolved...) + return resp, nil } } - return ips + if len(withoutAddresses) == 0 { + return nil, err + } + + if depth >= maxLookupDepth { + return nil, fmt.Errorf( + "addresses of the nameservers of %s not looked up: %w", + zone, ErrLookupDepthExceeded, + ) + } + + lookedUp, limitErr := r.resolveNSIPs(ctx, withoutAddresses, depth+1) + if limitErr != nil { + return nil, limitErr + } + + 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, 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. When none resolves and the depth limit +// stopped one of the lookups, it returns that lookup's error. +func (r *Resolver) resolveNSIPs( + ctx context.Context, + nsNames []string, + depth int, +) ([]string, error) { + var ( + ips []string + limitErr error + ) + + for _, ns := range nsNames { + resolved, err := r.resolveARecord(ctx, ns, depth) + + switch { + case err == nil: + ips = append(ips, resolved...) + case errors.Is(err, ErrLookupDepthExceeded): + limitErr = err + } + } + + if len(ips) > 0 { + return ips, nil + } + + return nil, limitErr } // resolveNSIterative queries for NS records using iterative @@ -438,11 +525,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 +542,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.queryZone( + ctx, servers, withoutAddresses, zone, hostname, dns.TypeA, + depth, ) if err != nil { return nil, fmt.Errorf( @@ -485,17 +578,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 +667,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..ac45b89 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -2,6 +2,7 @@ package resolver_test import ( "context" + "errors" "fmt" "log/slog" "net" @@ -162,6 +163,112 @@ 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) + } +} + +// TestQueryZone_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 TestQueryZone_GivenAddressesFail(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + + var resp *dns.Msg + + livednstest.Retry( + t, + "QueryZone(192.0.2.1 and three ntp.org nameservers, pool.ntp.org)", + func(ctx context.Context) error { + var err error + + resp, err = r.QueryZone( + 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.")) +} + +// TestQueryZone_LookupDepth asks the servers of g.ntpns.org, a +// nameserver of pool.ntp.org, for its address, as looking that address +// up does when anyns.pch.net, one of the servers of ntpns.org, gives the +// referral to g.ntpns.org without addresses. Their addresses are looked +// up (here only a.ntpns.org's), and that needs a bitnames.com +// nameserver's address, as the org servers delegate ntpns.org without +// addresses. From depth 1, where looking up g.ntpns.org's address +// starts, that makes three lookups and the address is found. From one +// below maxLookupDepth, the bitnames.com lookup would be past the limit, +// so nothing can be asked, and the error says the limit is why. +func TestQueryZone_LookupDepth(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + withoutAddresses := []string{"a.ntpns.org."} + + var resp *dns.Msg + + livednstest.Retry( + t, + "QueryZone(a.ntpns.org without its address, g.ntpns.org)", + func(ctx context.Context) error { + var err error + + resp, err = r.QueryZone( + ctx, nil, withoutAddresses, "g.ntpns.org.", "g.ntpns.org.", + dns.TypeA, 1, + ) + + return err + }, + ) + + assert.NotEmpty(t, resp.Answer) + + var limitErr error + + // Any other error is live DNS not answering, and is retried. + livednstest.Retry( + t, + "QueryZone(a.ntpns.org without its address, g.ntpns.org, "+ + "one below the limit)", + func(ctx context.Context) error { + _, limitErr = r.QueryZone( + ctx, nil, withoutAddresses, "g.ntpns.org.", "g.ntpns.org.", + dns.TypeA, resolver.MaxLookupDepth-1, + ) + if limitErr == nil || + errors.Is(limitErr, resolver.ErrLookupDepthExceeded) { + return nil + } + + return limitErr + }, + ) + + require.ErrorIs(t, limitErr, resolver.ErrLookupDepthExceeded) +} + // ---------------------------------------------------------------- // QueryNameserver tests // ---------------------------------------------------------------- @@ -185,6 +292,20 @@ func TestQueryNameserver_BasicA(t *testing.T) { ) } +// TestQueryNameserver_ZoneDelegatedWithoutAddresses asks a.ntpns.org, a +// nameserver of pool.ntp.org, about pool.ntp.org, as the watcher does. +// The org servers delegate ntpns.org without the addresses of its +// nameservers, so finding a.ntpns.org's address needs a lookup inside +// the one QueryNameserver starts. +func TestQueryNameserver_ZoneDelegatedWithoutAddresses(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + resp := liveQueryNameserver(t, r, "a.ntpns.org.", "pool.ntp.org", "A") + + assert.Equal(t, resolver.StatusOK, resp.Status) +} + func TestQueryNameserver_AAAA(t *testing.T) { t.Parallel() @@ -681,6 +802,21 @@ func TestLookupNS_MatchesFindAuthoritative(t *testing.T) { assert.Equal(t, fromFind, fromLookup) } +// TestLookupNS_ParentZoneDelegatedWithoutAddresses looks up the +// nameservers of g.ntpns.org. The org servers delegate its parent zone, +// ntpns.org, without the addresses of its nameservers, so the walk has +// to look them up to ask them. If it did not, the walk for g.ntpns.org +// would fail and LookupNS would return the nameservers of ntpns.org, +// which a.ntpns.org is not one of. +func TestLookupNS_ParentZoneDelegatedWithoutAddresses(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + nameservers := liveLookupNS(t, r, "g.ntpns.org") + + assert.Contains(t, nameservers, "a.ntpns.org.") +} + // ---------------------------------------------------------------- // ResolveIPAddresses tests // ----------------------------------------------------------------