From 24117ae646f6c74a3c7c95456e564b9716aeadc8 Mon Sep 17 00:00:00 2001 From: clawbot <35+clawbot@noreply.example.org> Date: Thu, 15 Oct 2026 10:17:14 +0000 Subject: [PATCH] resolver: a nameserver is asked at each of its addresses in turn (closes #285) A nameserver's queries went only to the first IPv4 address its name resolves to, so one unreachable address made it fail whenever that address came first in the answer. Each record type is now asked at the addresses one at a time until one gives a usable reply, and the next type is asked first at the address that gave it. A type that fails is logged with the reason at each address, and a nameserver is reported as timed out only when every query timed out. The README's DNS Resolution Strategy says which addresses are asked. Model: opus-5-5 --- README.md | 30 +++--- TODO.md | 2 + internal/resolver/export_test.go | 11 ++ internal/resolver/iterative.go | 86 ++++++++++----- internal/resolver/iterative_internal_test.go | 14 +++ internal/resolver/resolver_test.go | 104 +++++++++++++++++++ 6 files changed, 209 insertions(+), 38 deletions(-) diff --git a/README.md b/README.md index dd94301..0c3ab0a 100644 --- a/README.md +++ b/README.md @@ -133,18 +133,19 @@ notification endpoint set, changes show only on the dashboard; see - Queries **each** authoritative nameserver independently for **all** record types: A, AAAA, CNAME, MX, TXT, SRV, CAA, NS. - Each record type is a query of its own. When a nameserver answers some types - but the query for another gets no usable reply (no reply after two tries, an - error reply such as SERVFAIL, a referral, or a reply too large for UDP whose - retry over TCP fails), the failure is logged with the reason, and the type is - listed in the nameserver's `failedTypes` and keeps the records saved for the - nameserver by the previous check. On that check those records are not compared - with the other nameservers', so no record change or inconsistency is reported - for the type; on the next check they are compared with the nameserver's answer - as usual. When the previous check did not know the type's records either, - because the nameserver was new or failing then or the type was already listed - in `unknownTypes`, the type is also listed in `unknownTypes` and left out of - every comparison until it answers. A nameserver none of whose queries got a - usable reply has failed (see NS query failure below). + but the query for another gets no usable reply at any of its addresses (no + reply after two tries, an error reply such as SERVFAIL, a referral, or a reply + too large for UDP whose retry over TCP fails), the failure is logged with the + reason, and the type is listed in the nameserver's `failedTypes` and keeps the + records saved for the nameserver by the previous check. On that check those + records are not compared with the other nameservers', so no record change or + inconsistency is reported for the type; on the next check they are compared + with the nameserver's answer as usual. When the previous check did not know + the type's records either, because the nameserver was new or failing then or + the type was already listed in `unknownTypes`, the type is also listed in + `unknownTypes` and left out of every comparison until it answers. A nameserver + none of whose queries got a usable reply has failed (see NS query failure + below). - Stores results **per nameserver**. The state for a hostname is not a merged view — it is a map from nameserver to record set. - DNS names inside record values (CNAME, MX, SRV and NS targets) are stored in @@ -549,6 +550,11 @@ 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. +In step 4 each record type is asked at the IPv4 addresses the nameserver's name +resolves to, one at a time, until one gives a usable reply; the next type is +asked first at that address. A type's query fails only when none of them gives +one, and the nameserver has failed only when that is so for every type. + This approach ensures: - Independence from any upstream resolver's cache or filtering. diff --git a/TODO.md b/TODO.md index 4fb2e39..9c7daf2 100644 --- a/TODO.md +++ b/TODO.md @@ -19,6 +19,8 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7 # Completed Steps +- 2026-10-15: a nameserver is asked at each of its IPv4 addresses in turn, so it + has failed only when none of them gives a usable reply (closes #285). - 2026-10-13: a hostname or a domain whose zone's nameservers all stop answering gets an NS Failure for each, then an NS Recovery (closes #284). - 2026-10-13: an IPv6 address is written in brackets before its port, as in diff --git a/internal/resolver/export_test.go b/internal/resolver/export_test.go index 23471b8..58ea51b 100644 --- a/internal/resolver/export_test.go +++ b/internal/resolver/export_test.go @@ -96,6 +96,17 @@ func (r *Resolver) QueryServers( return r.queryServers(ctx, servers, zone, name, qtype) } +// QueryTypes exports queryTypes for testing. +func (r *Resolver) QueryTypes( + ctx context.Context, + nsHostname string, + nsIPs []string, + hostname string, + qtypes []uint16, +) (*NameserverResponse, error) { + return r.queryTypes(ctx, nsHostname, nsIPs, hostname, qtypes) +} + // QueryEachNS exports queryEachNS for testing. func (r *Resolver) QueryEachNS( ctx context.Context, diff --git a/internal/resolver/iterative.go b/internal/resolver/iterative.go index e5689f7..9f0cb20 100644 --- a/internal/resolver/iterative.go +++ b/internal/resolver/iterative.go @@ -708,8 +708,9 @@ func (r *Resolver) QueryNameserver( return r.queryNameserver(ctx, nsHostname, hostname, recordTypes()) } -// queryNameserver queries a specific nameserver for the record types -// in qtypes and builds a NameserverResponse. +// queryNameserver queries a specific nameserver, at the IPv4 addresses +// its name resolves to, for the record types in qtypes and builds a +// NameserverResponse. func (r *Resolver) queryNameserver( ctx context.Context, nsHostname string, @@ -727,7 +728,7 @@ func (r *Resolver) queryNameserver( hostname = dns.Fqdn(hostname) - return r.queryTypes(ctx, nsHostname, nsIPs[0], hostname, qtypes) + return r.queryTypes(ctx, nsHostname, nsIPs, hostname, qtypes) } // QueryNameserverIP queries a nameserver by its IP address directly, @@ -744,13 +745,18 @@ func (r *Resolver) QueryNameserverIP( hostname = dns.Fqdn(hostname) - return r.queryTypes(ctx, nsHostname, nsIP, hostname, recordTypes()) + return r.queryTypes( + ctx, nsHostname, []string{nsIP}, hostname, recordTypes(), + ) } +// queryTypes asks the nameserver nsHostname, at its addresses nsIPs, +// about hostname's records of each type in qtypes and builds a +// NameserverResponse. func (r *Resolver) queryTypes( ctx context.Context, nsHostname string, - nsIP string, + nsIPs []string, hostname string, qtypes []uint16, ) (*NameserverResponse, error) { @@ -760,7 +766,7 @@ func (r *Resolver) queryTypes( Status: StatusOK, } - state := r.queryEachType(ctx, nsIP, hostname, qtypes, resp) + state := r.queryEachType(ctx, nsIPs, hostname, qtypes, resp) classifyResponse(resp, state) return resp, nil @@ -778,28 +784,37 @@ type queryState struct { answered bool } -// queryEachType asks the nameserver at nsIP about hostname once for each -// record type in qtypes, and lists in resp.FailedTypes the types whose -// query got no usable reply, logging each with the reason unless ctx was -// cancelled: shutdown cancels it, and a query it cut short did not fail. +// queryEachType asks the nameserver about hostname's records of each type +// in qtypes, at its addresses nsIPs in turn until one gives a usable +// reply, and lists in resp.FailedTypes the types that got none at any of +// them, logging each with the reasons unless ctx was cancelled: shutdown +// cancels it, and a query it cut short did not fail. func (r *Resolver) queryEachType( ctx context.Context, - nsIP string, + nsIPs []string, hostname string, qtypes []uint16, resp *NameserverResponse, ) queryState { var state queryState + // Reordered below; the caller's list is left as it was. + nsIPs = slices.Clone(nsIPs) + for _, qtype := range qtypes { if checkCtx(ctx) != nil { break } - err := r.querySingleType(ctx, nsIP, hostname, qtype, resp, &state) + answeredAt, err := r.querySingleType( + ctx, nsIPs, hostname, qtype, resp, &state, + ) if err == nil { state.answered = true + // The next type is asked first at the address that answered. + nsIPs[0], nsIPs[answeredAt] = nsIPs[answeredAt], nsIPs[0] + continue } @@ -833,20 +848,34 @@ func (r *Resolver) queryEachType( return state } -// querySingleType asks the nameserver at nsIP about hostname's records -// of type qtype. It returns nil when the nameserver answered: with -// records, with none, or with NXDOMAIN; otherwise it returns why not. +// querySingleType asks the nameserver about hostname's records of type +// qtype at its addresses nsIPs, one at a time, until one answers: with +// records, with none, or with NXDOMAIN. It returns the index in nsIPs of +// the address that answered; when none did, it returns why each did not, +// each reason naming its address. func (r *Resolver) querySingleType( ctx context.Context, - nsIP string, + nsIPs []string, hostname string, qtype uint16, resp *NameserverResponse, state *queryState, -) error { - msg, err := r.queryDNS(ctx, nsIP, hostname, qtype) - if err != nil { +) (int, error) { + if len(nsIPs) == 0 { + return 0, ErrNoNameserverAnswered + } + + var errs []error + + for i, nsIP := range nsIPs { + msg, err := r.queryDNS(ctx, nsIP, hostname, qtype) + switch { + case err == nil: + err = readReply(msg, resp, state) + if err != nil { + err = fmt.Errorf("query %s @%s: %w", hostname, nsIP, err) + } case isTimeout(err): state.gotTimeout = true case errors.Is(err, ErrRefused): @@ -855,10 +884,14 @@ func (r *Resolver) querySingleType( state.netErr = err } - return err + if err == nil { + return i, nil + } + + errs = append(errs, err) } - return readReply(msg, resp, state) + return 0, errors.Join(errs...) } // readReply adds to resp the records in msg, a nameserver's reply to a @@ -952,17 +985,15 @@ func isTimeout(err error) bool { } // classifyResponse sets the nameserver's status. One that answered no -// record type has failed, and Error says why; one that answered some has -// the status of those answers. It has no data only when every type +// record type has failed, and Error says why; it says all queries timed +// out only when every query, at every address, did. One that answered +// some has the status of those answers. It has no data only when every type // answered with no records: a type in FailedTypes may have records, so a // nameserver with one stays ok. func classifyResponse(resp *NameserverResponse, state queryState) { switch { case state.gotNXDomain && !state.hasRecords: resp.Status = StatusNXDomain - case state.gotTimeout && !state.answered: - resp.Status = StatusTimeout - resp.Error = "all queries timed out" case state.gotErrorReply && !state.answered: resp.Status = StatusError resp.Error = "server returned " + state.errorReply @@ -975,6 +1006,9 @@ func classifyResponse(resp *NameserverResponse, state queryState) { case state.gotReferral && !state.answered: resp.Status = StatusError resp.Error = "server returned a referral" + case state.gotTimeout && !state.answered: + resp.Status = StatusTimeout + resp.Error = "all queries timed out" // An NXDOMAIN reply with no records was taken by the first case. case !state.hasRecords && len(resp.FailedTypes) == 0: resp.Status = StatusNoData diff --git a/internal/resolver/iterative_internal_test.go b/internal/resolver/iterative_internal_test.go index c756bb0..1749dd6 100644 --- a/internal/resolver/iterative_internal_test.go +++ b/internal/resolver/iterative_internal_test.go @@ -100,6 +100,20 @@ func TestClassifyResponse(t *testing.T) { } } +// TestClassifyResponse_RefusedAndTimedOut sets the status of a +// nameserver one of whose addresses refused every query while another +// never replied. It has failed with the refusal: not every query timed +// out. +func TestClassifyResponse_RefusedAndTimedOut(t *testing.T) { + t.Parallel() + + resp := &NameserverResponse{Status: StatusOK, FailedTypes: []string{"A"}} + classifyResponse(resp, queryState{gotRefused: true, gotTimeout: true}) + + assert.Equal(t, StatusError, resp.Status) + assert.Equal(t, "server returned REFUSED", resp.Error) +} + // TestReadReply checks which replies to a query about one record type, // built here, are an answer: one with the code NOERROR or NXDOMAIN. A // reply with any other code is not, and the type's query has failed; a diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 1c535e7..5203930 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -360,6 +360,110 @@ func TestQueryNameserver_ZoneDelegatedWithoutAddresses(t *testing.T) { assert.Equal(t, resolver.StatusOK, resp.Status) } +// TestQueryTypes_FirstAddressNeverAnswers asks a google.com nameserver +// about google.com's A and AAAA records at a list of addresses that puts +// 192.0.2.1, where nothing answers, before the nameserver's own. The +// nameserver answers at its own address, so it is ok, with its records. +// Each attempt has time for one round of tries at 192.0.2.1, two tries +// of two seconds, and not for two, so AAAA must be asked first at the +// address that answered A. +func TestQueryTypes_FirstAddressNeverAnswers(t *testing.T) { + t.Parallel() + + r := newTestResolver(t) + ns := findOneNSForDomain(t, r, "google.com") + nsIPs := append( + []string{noAnswerAddress}, liveResolveNSIPs(t, r, []string{ns}, 1)..., + ) + + var resp *resolver.NameserverResponse + + livednstest.Retry( + t, + "QueryTypes("+ns+" at "+strings.Join(nsIPs, ", ")+", google.com)", + func(ctx context.Context) error { + ctx, cancel := context.WithTimeout(ctx, 6*time.Second) + defer cancel() + + var err error + + resp, err = r.QueryTypes( + ctx, ns, nsIPs, "google.com.", + []uint16{dns.TypeA, dns.TypeAAAA}, + ) + if err != nil { + return err + } + + if len(resp.FailedTypes) > 0 { + return fmt.Errorf( + "%w: %s gave no usable reply for %s", + livednstest.ErrNoAnswer, ns, + strings.Join(resp.FailedTypes, ", "), + ) + } + + return nil + }, + ) + + assert.Equal(t, resolver.StatusOK, resp.Status) + assert.NotEmpty(t, resp.Records["A"]) + assert.NotEmpty(t, resp.Records["AAAA"]) +} + +// TestQueryTypes_EveryAddressFails asks a google.com nameserver about +// cloudflare.com, a zone it does not serve, at 192.0.2.1, where nothing +// answers, and then at its own address, which refuses. The nameserver +// has failed with the refusal, not a timeout, and the warning about the +// type gives the reason at each address. +func TestQueryTypes_EveryAddressFails(t *testing.T) { + t.Parallel() + + var logs bytes.Buffer + + r := resolver.NewFromLogger(slog.New(slog.NewTextHandler(&logs, nil))) + ns := findOneNSForDomain(t, r, "google.com") + nsIPs := append( + []string{noAnswerAddress}, liveResolveNSIPs(t, r, []string{ns}, 1)..., + ) + + var resp *resolver.NameserverResponse + + livednstest.Retry( + t, + "QueryTypes("+ns+" at "+strings.Join(nsIPs, ", ")+", cloudflare.com)", + func(ctx context.Context) error { + logs.Reset() + + var err error + + resp, err = r.QueryTypes( + ctx, ns, nsIPs, "cloudflare.com.", []uint16{dns.TypeA}, + ) + if err != nil { + return err + } + + // A timeout or a network error at its own address is no + // reply at all. + if resp.Status == resolver.StatusTimeout || + strings.HasPrefix(resp.Error, "network error") { + return fmt.Errorf( + "%w: %s: %s", livednstest.ErrNoAnswer, ns, resp.Error, + ) + } + + return nil + }, + ) + + assert.Equal(t, resolver.StatusError, resp.Status) + assert.Equal(t, "server returned REFUSED", resp.Error) + assert.Contains(t, logs.String(), "@"+noAnswerAddress+": ") + assert.Contains(t, logs.String(), "@"+nsIPs[1]+": ") +} + func TestQueryNameserver_AAAA(t *testing.T) { t.Parallel()