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 60e6c82..1834d71 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-15: the log names a notification endpoint (`slack`, `mattermost`, `ntfy`) and never shows its URL's path or query (closes #288). - 2026-10-13: a hostname or a domain whose zone's nameservers all stop answering diff --git a/internal/resolver/errors.go b/internal/resolver/errors.go index 8c53adb..a9b109d 100644 --- a/internal/resolver/errors.go +++ b/internal/resolver/errors.go @@ -36,6 +36,10 @@ var ( // so whether the name has addresses is unknown. ErrNoNameserverAnswered = errors.New("no nameserver answered") + // ErrNoNameserverAddress is returned when a nameserver is to be + // asked and no address for it was given. + ErrNoNameserverAddress = errors.New("nameserver has no address") + // ErrUnusableReply is returned when a server replied with an // error such as SERVFAIL, or with a referral that leads no // closer to the name asked about. 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..7e5d0e4 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,23 +745,33 @@ 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. With no address there is nothing to ask, and it +// returns ErrNoNameserverAddress. func (r *Resolver) queryTypes( ctx context.Context, nsHostname string, - nsIP string, + nsIPs []string, hostname string, qtypes []uint16, ) (*NameserverResponse, error) { + if len(nsIPs) == 0 { + return nil, fmt.Errorf("%w: %s", ErrNoNameserverAddress, nsHostname) + } + resp := &NameserverResponse{ Nameserver: nsHostname, Records: make(map[string][]string), 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 +789,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 +853,30 @@ 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, of which there is at least one, 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) { + 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 +885,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 +986,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 +1007,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..f0f399c 100644 --- a/internal/resolver/iterative_internal_test.go +++ b/internal/resolver/iterative_internal_test.go @@ -15,8 +15,10 @@ import ( // that answered some record types, even with no records, has not failed // when its query for another type got no usable reply, whatever the // reason, and is ok, not nodata: that type may have records. One whose -// every query got none has failed. Only one whose every type answered -// with no records is nodata. +// every query got none has failed, and its error says all queries timed +// out only when every one did: a reply such as a refusal, or a network +// error, at one of its addresses is given instead. Only one whose every +// type answered with no records is nodata. func TestClassifyResponse(t *testing.T) { t.Parallel() @@ -27,63 +29,46 @@ func TestClassifyResponse(t *testing.T) { wantStatus string wantError string }{ - { - "every type answered with no records", + {"every type answered with no records", queryState{answered: true}, - nil, - StatusNoData, "", - }, - { - "some types answered with no records, another timed out", + nil, StatusNoData, ""}, + {"some types answered with no records, another timed out", queryState{answered: true, gotTimeout: true}, - []string{"A"}, - StatusOK, "", - }, - { - "some types answered with no records, another got SERVFAIL", - queryState{ - answered: true, gotErrorReply: true, errorReply: "SERVFAIL", - }, - []string{"A"}, - StatusOK, "", - }, - { - "some types answered with no records, another was refused", + []string{"A"}, StatusOK, ""}, + {"some types answered with no records, another got SERVFAIL", + queryState{answered: true, gotErrorReply: true, errorReply: "SERVFAIL"}, + []string{"A"}, StatusOK, ""}, + {"some types answered with no records, another was refused", queryState{answered: true, gotRefused: true}, - []string{"A"}, - StatusOK, "", - }, - { - "some types answered with no records, another got a network error", + []string{"A"}, StatusOK, ""}, + {"some types answered with no records, another got a network error", queryState{answered: true, netErr: syscall.ECONNREFUSED}, - []string{"A"}, - StatusOK, "", - }, - { - "some types answered with no records, another's reply was " + - "truncated and its retry over TCP failed", + []string{"A"}, StatusOK, ""}, + {"some types answered with no records, another's reply was " + + "truncated and its retry over TCP failed", queryState{answered: true, netErr: ErrTruncated}, - []string{"TXT"}, - StatusOK, "", - }, - { - "some types answered with no records, another got a referral", + []string{"TXT"}, StatusOK, ""}, + {"some types answered with no records, another got a referral", queryState{answered: true, gotReferral: true}, - []string{"A"}, - StatusOK, "", - }, - { - "every query timed out", + []string{"A"}, StatusOK, ""}, + {"every query timed out", queryState{gotTimeout: true}, - []string{"A", "AAAA", "CNAME"}, - StatusTimeout, "all queries timed out", - }, - { - "every query got NOTIMP", + []string{"A", "AAAA", "CNAME"}, StatusTimeout, "all queries timed out"}, + {"every query got NOTIMP", queryState{gotErrorReply: true, errorReply: "NOTIMP"}, - []string{"A", "AAAA", "CNAME"}, - StatusError, "server returned NOTIMP", - }, + []string{"A", "AAAA", "CNAME"}, StatusError, "server returned NOTIMP"}, + {"every query got SERVFAIL at one address, timed out at another", + queryState{gotErrorReply: true, errorReply: "SERVFAIL", gotTimeout: true}, + []string{"A"}, StatusError, "server returned SERVFAIL"}, + {"every query was refused at one address, timed out at another", + queryState{gotRefused: true, gotTimeout: true}, + []string{"A"}, StatusError, "server returned REFUSED"}, + {"every query got a network error at one address, timed out at another", + queryState{netErr: syscall.ECONNREFUSED, gotTimeout: true}, + []string{"A"}, StatusError, "network error: connection refused"}, + {"every query got a referral at one address, timed out at another", + queryState{gotReferral: true, gotTimeout: true}, + []string{"A"}, StatusError, "server returned a referral"}, } for _, tt := range tests { diff --git a/internal/resolver/resolver_test.go b/internal/resolver/resolver_test.go index 1c535e7..9bd98ed 100644 --- a/internal/resolver/resolver_test.go +++ b/internal/resolver/resolver_test.go @@ -360,6 +360,124 @@ 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]+": ") +} + +// TestQueryTypes_NoAddress asks a nameserver given no address. There is +// nothing to ask it at, so it has failed, with an error that says it has +// no address; it is not ok. +func TestQueryTypes_NoAddress(t *testing.T) { + t.Parallel() + + resp, err := newTestResolver(t).QueryTypes( + t.Context(), "ns1.example.net.", nil, "www.example.net.", + []uint16{dns.TypeA}, + ) + require.ErrorIs(t, err, resolver.ErrNoNameserverAddress) + assert.Nil(t, resp) +} + func TestQueryNameserver_AAAA(t *testing.T) { t.Parallel() @@ -424,8 +542,9 @@ func TestQueryNameserver_TXT(t *testing.T) { // nameserver about google.com with a resolver whose retries over TCP // fail. google.com's TXT records do not fit in a reply over UDP, so TXT // is reported as failed, holding none of the records that fit, and -// logged with the reason, while the nameserver, which answered the other -// types, is ok. +// logged with the reason, which names the address whose reply was +// truncated, while the nameserver, which answered the other types, is +// ok. func TestQueryNameserver_TruncatedReplyWhoseTCPRetryFails(t *testing.T) { t.Parallel() @@ -442,6 +561,13 @@ func TestQueryNameserver_TruncatedReplyWhoseTCPRetryFails(t *testing.T) { assert.Contains(t, logs.String(), "hostname=google.com. nameserver="+ns+" type=TXT error=", ) + + nsIPs := liveResolveNSIPs(t, newTestResolver(t), []string{ns}, 1) + for _, ip := range nsIPs { + assert.Contains(t, logs.String(), + "@"+ip+": "+resolver.ErrTruncated.Error(), + ) + } } func TestQueryNameserver_NXDomain(t *testing.T) { @@ -495,6 +621,57 @@ func TestQueryNameserver_Refused(t *testing.T) { assert.Equal(t, "server returned REFUSED", resp.Error) } +// TestQueryNameserver_EveryAddressRefuses asks ada.ns.cloudflare.com, +// whose name has several IPv4 addresses, about google.com, a zone it +// does not serve, which it refuses. A record type is asked at each of +// the nameserver's addresses before it fails, so the warning about it +// names every one. +func TestQueryNameserver_EveryAddressRefuses(t *testing.T) { + t.Parallel() + + const ns = "ada.ns.cloudflare.com." + + var logs bytes.Buffer + + r := resolver.NewFromLogger(slog.New(slog.NewTextHandler(&logs, nil))) + nsIPs := liveResolveNSIPs(t, r, []string{ns}, 2) + + var resp *resolver.NameserverResponse + + livednstest.Retry( + t, + "QueryNameserver("+ns+", google.com)", + func(ctx context.Context) error { + logs.Reset() + + var err error + + resp, err = r.QueryNameserver(ctx, ns, "google.com") + if err != nil { + return err + } + + // A timeout or a network error at every 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) + + for _, ip := range nsIPs { + assert.Contains(t, logs.String(), "@"+ip+": ") + } +} + // TestQueryServers_RecursiveResolverRefused passes a public recursive // resolver to QueryServers as the server of google.com. These resolvers // refuse a query that does not ask for recursion and answer one that