Compare commits
1
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
540c4590a9 |
@@ -561,18 +561,6 @@ This approach ensures:
|
||||
- Ability to detect split-horizon or inconsistent responses across authoritative
|
||||
servers.
|
||||
|
||||
A root server never offers recursion, so a reply marked "recursion available"
|
||||
from a root server's address came from a recursive resolver on the network
|
||||
answering in its place, as on a network where a router rule, a VPN or a DNS
|
||||
filter sends all DNS to one resolver. The first such reply from a root server's
|
||||
address stops the lookup at once with the error "this network intercepts DNS
|
||||
queries", and no other root server is asked. A lookup that every root server
|
||||
refuses fails with the same error. Every lookup starts at the root servers, so
|
||||
on such a network every lookup fails with that error, whether or not that
|
||||
resolver has the name cached, and dnswatcher's tests fail there too. That is by
|
||||
design: nothing asks again another way or otherwise works around the
|
||||
interception.
|
||||
|
||||
A watched name's records are stored as its nameservers return them, CNAME
|
||||
included. When they return a CNAME and no address, the chain of every CNAME
|
||||
target they gave is followed (with a depth limit to prevent loops) to the A and
|
||||
|
||||
+7
-9
@@ -19,18 +19,16 @@ works correctly in production.
|
||||
### Constraints
|
||||
|
||||
- Tests hit real DNS infrastructure and require network access
|
||||
- Test duration depends on network conditions. `make test` must finish in under
|
||||
60 seconds, the hard cap in `REPO_POLICIES.md`; under 20 seconds is the target
|
||||
- Each query to a nameserver has a 2-second timeout, and a query over UDP that
|
||||
gets no reply is sent once more (`queryTimeoutDuration` and `maxRetries` in
|
||||
`internal/resolver/iterative.go`)
|
||||
- In the walk to a name's nameservers, the servers of each zone are asked one at
|
||||
a time in a random order, as the README's DNS Resolution Strategy describes
|
||||
- Test duration depends on network conditions; timeout tuning keeps the suite
|
||||
within the 60-second target
|
||||
- Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) plus
|
||||
processing margin
|
||||
- Root server fan-out is limited to reduce parallel query load
|
||||
- Live lookups that expect an answer go through `internal/livednstest`, which
|
||||
limits how many run at once in a test binary and retries a lookup that got
|
||||
none
|
||||
- A flaky live test is a robustness problem, fixed with retries with backoff,
|
||||
several independent nameservers or longer timeouts, never with mocks or skip
|
||||
- Flaky failures from transient network issues are acceptable and should be
|
||||
investigated as potential resolver bugs, not papered over with mocks or skip
|
||||
flags
|
||||
|
||||
### What NOT to do
|
||||
|
||||
@@ -19,21 +19,6 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
|
||||
|
||||
# Completed Steps
|
||||
|
||||
- 2026-10-16: the first reply marked "recursion available" from a root server's
|
||||
address stops the lookup with "this network intercepts DNS queries", so on a
|
||||
network that sends DNS to a resolver dnswatcher and its tests fail, by design
|
||||
(closes #300).
|
||||
- 2026-10-15: a query its caller's context ends before it is sent records no
|
||||
reason about the nameserver, and a try the context's deadline cuts short is a
|
||||
timeout, so a nameserver whose only failures were timeouts is `timeout`, not a
|
||||
network error or `ok` (closes #298).
|
||||
- 2026-10-15: the resolver no longer says its query timeout grows on each try;
|
||||
every try waits the same 2 seconds, and the unused `timeoutMultiplier` is gone
|
||||
(closes #296).
|
||||
- 2026-10-15: `TESTING.md` Constraints give the resolver's 2-second query
|
||||
timeout and second try, say a zone's servers are asked one at a time in a
|
||||
random order, give the 60-second cap and 20-second target for `make test`, and
|
||||
say a flaky live test is fixed with robustness, not with mocks (closes #289).
|
||||
- 2026-10-15: a request under `/s/` that names a directory, such as `/s/` or
|
||||
`/s/css`, answers 404 instead of a listing of its files (closes #286).
|
||||
- 2026-10-15: a notification endpoint that answers with a redirect, or any
|
||||
|
||||
@@ -53,12 +53,9 @@ var (
|
||||
"reply truncated and its retry over TCP failed",
|
||||
)
|
||||
|
||||
// ErrIntercepted is returned when a root server's address gave a
|
||||
// reply marked recursion available, which ends the lookup at once,
|
||||
// or when every root server refused a query. Root servers do
|
||||
// neither, so those replies came from something on the network
|
||||
// answering in their place. On such a network dnswatcher fails with
|
||||
// it, by design: nothing works around the interception.
|
||||
// ErrIntercepted is returned when every root server refused a
|
||||
// query. Root servers refuse no query, so the refusals came from
|
||||
// something on the network answering in their place.
|
||||
ErrIntercepted = errors.New("this network intercepts DNS queries")
|
||||
|
||||
// ErrCNAMEDepthExceeded is returned when a CNAME chain
|
||||
|
||||
@@ -19,6 +19,7 @@ const (
|
||||
queryTimeoutDuration = 2 * time.Second
|
||||
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
|
||||
@@ -61,11 +62,19 @@ func checkCtx(ctx context.Context) error {
|
||||
return nil
|
||||
}
|
||||
|
||||
// tryExchange sends msg to addr over UDP, up to maxRetries times, until
|
||||
// a reply comes. Every try waits the same queryTimeoutDuration, or less
|
||||
// when ctx's deadline comes first. When ctx has ended before a further
|
||||
// try, it returns the error of the try already made, such as a timeout,
|
||||
// or ErrContextCanceled when none was made.
|
||||
func (r *Resolver) exchangeWithTimeout(
|
||||
ctx context.Context,
|
||||
msg *dns.Msg,
|
||||
addr string,
|
||||
attempt int,
|
||||
) (*dns.Msg, error) {
|
||||
_ = attempt // timeout escalation handled by client config
|
||||
|
||||
resp, _, err := r.client.ExchangeContext(ctx, msg, addr)
|
||||
|
||||
return resp, err
|
||||
}
|
||||
|
||||
func (r *Resolver) tryExchange(
|
||||
ctx context.Context,
|
||||
msg *dns.Msg,
|
||||
@@ -75,16 +84,14 @@ func (r *Resolver) tryExchange(
|
||||
|
||||
var err error
|
||||
|
||||
for range maxRetries {
|
||||
for attempt := range maxRetries {
|
||||
if checkCtx(ctx) != nil {
|
||||
if err == nil {
|
||||
return nil, ErrContextCanceled
|
||||
}
|
||||
|
||||
return nil, err
|
||||
return nil, ErrContextCanceled
|
||||
}
|
||||
|
||||
resp, _, err = r.client.ExchangeContext(ctx, msg, addr)
|
||||
resp, err = r.exchangeWithTimeout(
|
||||
ctx, msg, addr, attempt,
|
||||
)
|
||||
if err == nil {
|
||||
break
|
||||
}
|
||||
@@ -287,11 +294,7 @@ func shuffled(
|
||||
// order until one gives a usable reply. A server that times out, refuses
|
||||
// or gives a reply that is not usable is passed over for the next. When
|
||||
// every server refused, the error says so, and when they are the root
|
||||
// servers it is ErrIntercepted. When they are the root servers, the
|
||||
// first reply marked recursion available also ends the lookup with
|
||||
// ErrIntercepted, naming the address it came from, and no other root
|
||||
// server is asked: root servers offer no recursion, so that reply came
|
||||
// from something on the network answering in their place.
|
||||
// servers it is ErrIntercepted.
|
||||
func (r *Resolver) queryServers(
|
||||
ctx context.Context,
|
||||
servers []string,
|
||||
@@ -309,14 +312,6 @@ func (r *Resolver) queryServers(
|
||||
}
|
||||
|
||||
resp, err := r.queryDNS(ctx, ip, name, qtype)
|
||||
if err == nil && zone == "." && resp.RecursionAvailable {
|
||||
return nil, fmt.Errorf(
|
||||
"query %s @%s: reply marked recursion available, "+
|
||||
"so not from a root server: %w",
|
||||
name, ip, ErrIntercepted,
|
||||
)
|
||||
}
|
||||
|
||||
if err == nil && !usableReply(resp, zone, name) {
|
||||
err = fmt.Errorf(
|
||||
"query %s @%s: %w", name, ip, ErrUnusableReply,
|
||||
@@ -882,10 +877,6 @@ func (r *Resolver) querySingleType(
|
||||
if err != nil {
|
||||
err = fmt.Errorf("query %s @%s: %w", hostname, nsIP, err)
|
||||
}
|
||||
case errors.Is(err, ErrContextCanceled):
|
||||
// The caller's context ended before the query was sent.
|
||||
// That says nothing about the nameserver, so no reason is
|
||||
// recorded.
|
||||
case isTimeout(err):
|
||||
state.gotTimeout = true
|
||||
case errors.Is(err, ErrRefused):
|
||||
|
||||
@@ -1,8 +1,6 @@
|
||||
package resolver
|
||||
|
||||
import (
|
||||
"context"
|
||||
"log/slog"
|
||||
"strconv"
|
||||
"syscall"
|
||||
"testing"
|
||||
@@ -134,33 +132,3 @@ func TestReadReply(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestQuerySingleType_ContextEnded asks a nameserver about one record
|
||||
// type with a context that has already ended, so no query is sent. A
|
||||
// query the context ended before it was sent says nothing about the
|
||||
// nameserver and records no reason: a nameserver whose earlier query
|
||||
// timed out is still reported as timed out, not as a network error.
|
||||
func TestQuerySingleType_ContextEnded(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
r := NewFromLogger(slog.New(slog.DiscardHandler))
|
||||
|
||||
ctx, cancel := context.WithCancel(t.Context())
|
||||
cancel()
|
||||
|
||||
resp := &NameserverResponse{Records: map[string][]string{}, Status: StatusOK}
|
||||
|
||||
// The query for an earlier record type timed out.
|
||||
state := queryState{gotTimeout: true}
|
||||
|
||||
_, err := r.querySingleType(
|
||||
ctx, []string{"192.0.2.1"}, "example.com.", dns.TypeA, resp, &state,
|
||||
)
|
||||
require.ErrorIs(t, err, ErrContextCanceled)
|
||||
assert.Equal(t, queryState{gotTimeout: true}, state)
|
||||
|
||||
classifyResponse(resp, state)
|
||||
|
||||
assert.Equal(t, StatusTimeout, resp.Status)
|
||||
assert.Equal(t, "all queries timed out", resp.Error)
|
||||
}
|
||||
|
||||
@@ -850,63 +850,6 @@ func TestQueryServers_NotEveryRootServerRefused(t *testing.T) {
|
||||
require.NotContains(t, err.Error(), "refused a query for")
|
||||
}
|
||||
|
||||
// TestQueryServers_RecursiveResolversAsRootServers passes public
|
||||
// recursive resolvers of four operators to QueryServers as the servers
|
||||
// of the root zone, as a network that sends every DNS query to a
|
||||
// resolver would. Their replies are marked recursion available, which
|
||||
// no root server's is: SERVFAIL, or an answer from a resolver's cache,
|
||||
// as 1.1.1.1 gives for com., which would be used if a root server gave
|
||||
// it. The first such reply ends the lookup with ErrIntercepted, naming
|
||||
// the resolver that gave it. A resolver that refuses or gives no reply
|
||||
// is passed over for the next, so one of them answering is enough.
|
||||
func TestQueryServers_RecursiveResolversAsRootServers(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
r := newTestResolver(t)
|
||||
resolvers := []string{
|
||||
"1.1.1.1", "8.8.8.8", "9.9.9.9", "208.67.222.222",
|
||||
}
|
||||
|
||||
for _, name := range []string{"com.", "desec.io."} {
|
||||
var err error
|
||||
|
||||
livednstest.Retry(
|
||||
t,
|
||||
"QueryServers(public recursive resolvers as root servers, "+
|
||||
name+")",
|
||||
func(ctx context.Context) error {
|
||||
_, err = r.QueryServers(ctx, resolvers, ".", name, dns.TypeNS)
|
||||
|
||||
// When no resolver gave a reply marked recursion
|
||||
// available, each refused or gave no reply at all, so
|
||||
// the attempt is tried again.
|
||||
if err != nil &&
|
||||
!strings.Contains(err.Error(), "marked recursion available") {
|
||||
return fmt.Errorf(
|
||||
"%w: %w", livednstest.ErrNoAnswer, err,
|
||||
)
|
||||
}
|
||||
|
||||
return nil
|
||||
},
|
||||
)
|
||||
|
||||
require.ErrorIs(t, err, resolver.ErrIntercepted)
|
||||
|
||||
// The error names the resolver whose reply ended the lookup.
|
||||
messages := make([]string, 0, len(resolvers))
|
||||
for _, ip := range resolvers {
|
||||
messages = append(messages,
|
||||
"query "+name+" @"+ip+": reply marked recursion "+
|
||||
"available, so not from a root server: "+
|
||||
"this network intercepts DNS queries",
|
||||
)
|
||||
}
|
||||
|
||||
require.Contains(t, messages, err.Error())
|
||||
}
|
||||
}
|
||||
|
||||
func TestQueryNameserver_RecordsSorted(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
@@ -1437,10 +1380,10 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
|
||||
r := newTestResolver(t)
|
||||
|
||||
// Nothing answers at 192.0.2.1, a documentation address. The
|
||||
// resolver tries each query twice, and each try gives up after two
|
||||
// seconds. This deadline ends during the second try of the first
|
||||
// record type; TestQueryNameserverIP_DeadlineDuringFirstTry covers
|
||||
// one that ends during the first.
|
||||
// resolver tries each query twice, and the first try gives up
|
||||
// after two seconds. A deadline that ends during the first try
|
||||
// makes the status vary from run to run between error and
|
||||
// timeout, so the deadline must outlast the first try.
|
||||
ctx, cancel := context.WithTimeout(
|
||||
context.Background(), 3*time.Second,
|
||||
)
|
||||
@@ -1456,62 +1399,6 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
|
||||
assert.NotEmpty(t, resp.Error)
|
||||
}
|
||||
|
||||
// TestQueryNameserverIP_DeadlineDuringFirstTry gives a query to
|
||||
// 192.0.2.1, where nothing answers, a deadline that ends during its
|
||||
// first try. That try ends at the deadline as a timeout, so the
|
||||
// nameserver is reported as timed out, not as answering.
|
||||
func TestQueryNameserverIP_DeadlineDuringFirstTry(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
r := newTestResolver(t)
|
||||
|
||||
// Well under the two seconds one try waits.
|
||||
ctx, cancel := context.WithTimeout(
|
||||
context.Background(), 500*time.Millisecond,
|
||||
)
|
||||
t.Cleanup(cancel)
|
||||
|
||||
resp, err := r.QueryNameserverIP(
|
||||
ctx, "unreachable.test.", "192.0.2.1",
|
||||
"example.com",
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Equal(t, resolver.StatusTimeout, resp.Status)
|
||||
assert.NotEmpty(t, resp.Error)
|
||||
}
|
||||
|
||||
// TestQueryNameserverIP_CancelDuringFirstTry cancels the context while
|
||||
// the first try of a query to 192.0.2.1, where nothing answers, waits
|
||||
// for a reply. That try's timeout is all that is learned about the
|
||||
// nameserver, so it is reported as timed out, not as answering.
|
||||
func TestQueryNameserverIP_CancelDuringFirstTry(t *testing.T) {
|
||||
t.Parallel()
|
||||
|
||||
r := newTestResolver(t)
|
||||
|
||||
// The first try starts at once and waits two seconds for a reply;
|
||||
// the cancel comes a second in. The DNS client cuts a try short at
|
||||
// the context's deadline but not at a cancel, so the try still
|
||||
// waits its full two seconds and ends as a timeout a second after
|
||||
// the cancel. By then the context has certainly ended, so no second
|
||||
// try is made. With a deadline instead, the try ends at the moment
|
||||
// the context does, and whether a second try is made depends on
|
||||
// which the resolver sees first.
|
||||
ctx, cancel := context.WithCancel(context.Background())
|
||||
t.Cleanup(cancel)
|
||||
time.AfterFunc(time.Second, cancel)
|
||||
|
||||
resp, err := r.QueryNameserverIP(
|
||||
ctx, "unreachable.test.", "192.0.2.1",
|
||||
"example.com",
|
||||
)
|
||||
require.NoError(t, err)
|
||||
|
||||
assert.Equal(t, resolver.StatusTimeout, resp.Status)
|
||||
assert.NotEmpty(t, resp.Error)
|
||||
}
|
||||
|
||||
// TestQueryNameserverIP_CancelledLogsNothing cancels the context while
|
||||
// a query to 192.0.2.1, where nothing answers, is waiting for a reply,
|
||||
// as shutdown does. The query was cut short, not failed, so nothing is
|
||||
|
||||
Reference in New Issue
Block a user