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()