Compare commits

...
5 Commits
Author SHA1 Message Date
clawbot b23cf43abf resolver: a reply marked recursion available from a root server's address stops the lookup (closes #300)
check / check (push) Waiting to run
dnswatcher never asks for recursion and a root server never offers it,
so a reply marked recursion available from a root server's address
came from a resolver on the network answering in its place. The first
such reply, a refusal included, now stops the lookup at once with
"this network intercepts DNS queries", naming the address, whether or
not that resolver had the name cached. queryDNS returns a refused reply
along with ErrRefused so the mark can be seen; a refusal without the
mark is handled as before. On such a network dnswatcher and its tests
fail with that error, by design, as the owner ruled on the issue.

Model: opus-5-5
2026-10-16 12:26:52 +02:00
clawbot adc8131191 resolver: a query cut short by the caller's context records no reason (closes #298)
check / check (push) Canceled after 0s
When the caller's context ends, the next query returns
ErrContextCanceled before anything is sent. querySingleType recorded
that as a network error, which outranks a timeout, so a nameserver that
had only timed out could be reported as "network error: context
canceled". It now records no reason for it.

A try the context's deadline cuts short ends as a timeout. When the
context has ended before a further try, tryExchange now returns that
try's error instead of ErrContextCanceled, so such a nameserver is
reported as timed out, not as answering.

Model: opus-5-5
2026-10-15 19:48:08 +02:00
clawbot 06ae357165 resolver: drop the unused timeout multiplier and the comment claiming it (closes #296)
check / check (push) Canceled after 0s
The resolver declared timeoutMultiplier and passed each try's number to
exchangeWithTimeout, which threw it away with a comment saying the
client config grew the timeout. Nothing did: every try waits the same
2-second queryTimeoutDuration. The constant goes, and exchangeWithTimeout
is folded into its one caller, tryExchange, whose comment now says the
timeout is fixed. Behaviour is unchanged.

Model: opus-5-5
2026-10-15 18:31:07 +02:00
clawbot f6b7aefcbf docs: TESTING.md constraints match the resolver and the test-time policy (closes #289)
check / check (push) Canceled after 0s
The Constraints list named a query timeout calibrated to round-trip time and a
limited root-server fan-out; the resolver has neither. It now gives the
2-second timeout and the second try over UDP, says a zone's servers are asked
one at a time in a random order, and gives the 60-second cap and 20-second
target for make test from REPO_POLICIES.md. The line on flaky failures now
says, as the README does, that a flaky live test is fixed with robustness,
never with mocks or skip flags.

Model: opus-5-5
2026-10-15 18:01:51 +02:00
clawbot db3b09c5d8 handlers: a directory under /s/ answers 404, not a listing (closes #286)
check / check (push) Canceled after 0s
http.FileServer answered /s/ and /s/css/ with an HTML listing of the
embedded files, and /s/css with a redirect to that listing. HandleStatic
in internal/handlers now looks the name up in the embedded files first
and answers 404 when it names a directory; files are served as before.
The routes mount it under /s.

Model: opus-5-5
2026-10-15 17:01:52 +02:00
10 changed files with 321 additions and 48 deletions
+13
View File
@@ -561,6 +561,18 @@ This approach ensures:
- Ability to detect split-horizon or inconsistent responses across authoritative - Ability to detect split-horizon or inconsistent responses across authoritative
servers. 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, a refusal included, stops the lookup at once with the error "this
network intercepts DNS queries", naming that address, and no other root server
is asked. A lookup that every root server refuses without that mark 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 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 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 target they gave is followed (with a depth limit to prevent loops) to the A and
@@ -910,6 +922,7 @@ internal/
templates/dashboard.html Dashboard template (embedded) templates/dashboard.html Dashboard template (embedded)
status.go /api/v1/status status.go /api/v1/status
healthcheck.go Health check handler healthcheck.go Health check handler
static.go Static assets under /s/
server/ server/
server.go HTTP server lifecycle server.go HTTP server lifecycle
routes.go Route definitions routes.go Route definitions
+9 -7
View File
@@ -19,16 +19,18 @@ works correctly in production.
### Constraints ### Constraints
- Tests hit real DNS infrastructure and require network access - Tests hit real DNS infrastructure and require network access
- Test duration depends on network conditions; timeout tuning keeps the suite - Test duration depends on network conditions. `make test` must finish in under
within the 60-second target 60 seconds, the hard cap in `REPO_POLICIES.md`; under 20 seconds is the target
- Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) plus - Each query to a nameserver has a 2-second timeout, and a query over UDP that
processing margin gets no reply is sent once more (`queryTimeoutDuration` and `maxRetries` in
- Root server fan-out is limited to reduce parallel query load `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
- Live lookups that expect an answer go through `internal/livednstest`, which - 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 limits how many run at once in a test binary and retries a lookup that got
none none
- Flaky failures from transient network issues are acceptable and should be - A flaky live test is a robustness problem, fixed with retries with backoff,
investigated as potential resolver bugs, not papered over with mocks or skip several independent nameservers or longer timeouts, never with mocks or skip
flags flags
### What NOT to do ### What NOT to do
+17
View File
@@ -19,6 +19,23 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
# Completed Steps # Completed Steps
- 2026-10-16: the first reply marked "recursion available" from a root server's
address, a refusal included, 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 - 2026-10-15: a notification endpoint that answers with a redirect, or any
status other than 2xx, is a failed delivery, retried and logged (closes #287). status other than 2xx, is a failed delivery, retried and logged (closes #287).
- 2026-10-15: a nameserver is asked at each of its IPv4 addresses in turn, so it - 2026-10-15: a nameserver is asked at each of its IPv4 addresses in turn, so it
+36
View File
@@ -0,0 +1,36 @@
package handlers
import (
"io/fs"
"net/http"
"strings"
"sneak.berlin/go/dnswatcher/static"
)
// HandleStatic returns the handler for the embedded static assets. It
// is mounted under /s, with that prefix stripped from the path. A
// request that names a directory answers 404: http.FileServer would
// answer it with a listing of the directory, or with a redirect to
// that listing when the trailing slash is missing.
func (h *Handlers) HandleStatic() http.HandlerFunc {
files := http.FileServer(http.FS(static.Static))
return func(
writer http.ResponseWriter,
request *http.Request,
) {
// fs.Stat refuses a name that is empty or ends in a slash,
// so /s/ and /s/css/ answer 404 here along with /s/css.
name := strings.TrimPrefix(request.URL.Path, "/")
info, err := fs.Stat(static.Static, name)
if err != nil || info.IsDir() {
http.NotFound(writer, request)
return
}
files.ServeHTTP(writer, request)
}
}
+7 -3
View File
@@ -53,9 +53,13 @@ var (
"reply truncated and its retry over TCP failed", "reply truncated and its retry over TCP failed",
) )
// ErrIntercepted is returned when every root server refused a // ErrIntercepted is returned when a root server's address gave a
// query. Root servers refuse no query, so the refusals came from // reply marked recursion available, a refusal included, which ends
// something on the network answering in their place. // 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 = errors.New("this network intercepts DNS queries") ErrIntercepted = errors.New("this network intercepts DNS queries")
// ErrCNAMEDepthExceeded is returned when a CNAME chain // ErrCNAMEDepthExceeded is returned when a CNAME chain
+38 -25
View File
@@ -19,7 +19,6 @@ const (
queryTimeoutDuration = 2 * time.Second queryTimeoutDuration = 2 * time.Second
maxRetries = 2 maxRetries = 2
maxDelegation = 20 maxDelegation = 20
timeoutMultiplier = 2
// maxLookupDepth is how many lookups of nameserver addresses may be // maxLookupDepth is how many lookups of nameserver addresses may be
// under way one inside another. Looking up a nameserver's address // under way one inside another. Looking up a nameserver's address
@@ -62,19 +61,11 @@ func checkCtx(ctx context.Context) error {
return nil return nil
} }
func (r *Resolver) exchangeWithTimeout( // tryExchange sends msg to addr over UDP, up to maxRetries times, until
ctx context.Context, // a reply comes. Every try waits the same queryTimeoutDuration, or less
msg *dns.Msg, // when ctx's deadline comes first. When ctx has ended before a further
addr string, // try, it returns the error of the try already made, such as a timeout,
attempt int, // or ErrContextCanceled when none was made.
) (*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( func (r *Resolver) tryExchange(
ctx context.Context, ctx context.Context,
msg *dns.Msg, msg *dns.Msg,
@@ -84,14 +75,16 @@ func (r *Resolver) tryExchange(
var err error var err error
for attempt := range maxRetries { for range maxRetries {
if checkCtx(ctx) != nil { if checkCtx(ctx) != nil {
return nil, ErrContextCanceled if err == nil {
return nil, ErrContextCanceled
}
return nil, err
} }
resp, err = r.exchangeWithTimeout( resp, _, err = r.client.ExchangeContext(ctx, msg, addr)
ctx, msg, addr, attempt,
)
if err == nil { if err == nil {
break break
} }
@@ -122,7 +115,8 @@ func (r *Resolver) retryTCP(
} }
// queryDNS sends a DNS query to a specific server IP, never asking it // queryDNS sends a DNS query to a specific server IP, never asking it
// for recursion. A reply of REFUSED is returned as ErrRefused. // for recursion. A reply of REFUSED is returned together with
// ErrRefused, so that the caller can still read the reply's flags.
func (r *Resolver) queryDNS( func (r *Resolver) queryDNS(
ctx context.Context, ctx context.Context,
serverIP string, serverIP string,
@@ -146,7 +140,7 @@ func (r *Resolver) queryDNS(
} }
if resp.Rcode == dns.RcodeRefused { if resp.Rcode == dns.RcodeRefused {
return nil, fmt.Errorf( return resp, fmt.Errorf(
"query %s @%s: %w", name, serverIP, ErrRefused, "query %s @%s: %w", name, serverIP, ErrRefused,
) )
} }
@@ -291,10 +285,15 @@ func shuffled(
} }
// queryServers asks servers, the servers of zone, about name in a random // queryServers asks servers, the servers of zone, about name in a random
// order until one gives a usable reply. A server that times out, refuses // order until one gives a usable reply. When they are the root servers,
// or gives a reply that is not usable is passed over for the next. When // the first reply marked recursion available, a refusal included, ends
// every server refused, the error says so, and when they are the root // the lookup with ErrIntercepted, naming the address it came from, and
// servers it is ErrIntercepted. // no other root server is asked: root servers offer no recursion, so
// that reply came from something on the network answering in their
// place. Otherwise 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.
func (r *Resolver) queryServers( func (r *Resolver) queryServers(
ctx context.Context, ctx context.Context,
servers []string, servers []string,
@@ -311,7 +310,17 @@ func (r *Resolver) queryServers(
return nil, ErrContextCanceled return nil, ErrContextCanceled
} }
// queryDNS returns a refusal together with its reply, so a
// refusal's mark is checked here too.
resp, err := r.queryDNS(ctx, ip, name, qtype) resp, err := r.queryDNS(ctx, ip, name, qtype)
if resp != 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) { if err == nil && !usableReply(resp, zone, name) {
err = fmt.Errorf( err = fmt.Errorf(
"query %s @%s: %w", name, ip, ErrUnusableReply, "query %s @%s: %w", name, ip, ErrUnusableReply,
@@ -877,6 +886,10 @@ func (r *Resolver) querySingleType(
if err != nil { if err != nil {
err = fmt.Errorf("query %s @%s: %w", hostname, nsIP, err) 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): case isTimeout(err):
state.gotTimeout = true state.gotTimeout = true
case errors.Is(err, ErrRefused): case errors.Is(err, ErrRefused):
@@ -1,6 +1,8 @@
package resolver package resolver
import ( import (
"context"
"log/slog"
"strconv" "strconv"
"syscall" "syscall"
"testing" "testing"
@@ -132,3 +134,33 @@ 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)
}
+128 -4
View File
@@ -850,6 +850,74 @@ func TestQueryServers_NotEveryRootServerRefused(t *testing.T) {
require.NotContains(t, err.Error(), "refused a query for") require.NotContains(t, err.Error(), "refused a query for")
} }
// TestQueryServers_RecursiveResolversAsRootServers passes public
// recursive resolvers 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, REFUSED, 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. In the first group the replies differ; every
// resolver in the second refuses a query that does not ask for
// recursion, as 9.9.9.9 does, so there a refusal ends the lookup
// instead of being passed over. Each group has resolvers of four
// operators, and a resolver that 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)
groups := [][]string{
{"1.1.1.1", "8.8.8.8", "9.9.9.9", "208.67.222.222"},
{"64.6.64.6", "185.222.222.222", "4.2.2.1", "94.140.14.14"},
}
for _, resolvers := range groups {
for _, name := range []string{"com.", "desec.io."} {
var err error
livednstest.Retry(
t,
"QueryServers("+strings.Join(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, none may have replied 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) { func TestQueryNameserver_RecordsSorted(t *testing.T) {
t.Parallel() t.Parallel()
@@ -1380,10 +1448,10 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
r := newTestResolver(t) r := newTestResolver(t)
// Nothing answers at 192.0.2.1, a documentation address. The // Nothing answers at 192.0.2.1, a documentation address. The
// resolver tries each query twice, and the first try gives up // resolver tries each query twice, and each try gives up after two
// after two seconds. A deadline that ends during the first try // seconds. This deadline ends during the second try of the first
// makes the status vary from run to run between error and // record type; TestQueryNameserverIP_DeadlineDuringFirstTry covers
// timeout, so the deadline must outlast the first try. // one that ends during the first.
ctx, cancel := context.WithTimeout( ctx, cancel := context.WithTimeout(
context.Background(), 3*time.Second, context.Background(), 3*time.Second,
) )
@@ -1399,6 +1467,62 @@ func TestQueryNameserverIP_Timeout(t *testing.T) {
assert.NotEmpty(t, resp.Error) 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 // TestQueryNameserverIP_CancelledLogsNothing cancels the context while
// a query to 192.0.2.1, where nothing answers, is waiting for a reply, // 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 // as shutdown does. The query was cut short, not failed, so nothing is
+1 -9
View File
@@ -8,8 +8,6 @@ import (
"github.com/go-chi/chi/v5" "github.com/go-chi/chi/v5"
chimw "github.com/go-chi/chi/v5/middleware" chimw "github.com/go-chi/chi/v5/middleware"
"github.com/prometheus/client_golang/prometheus/promhttp" "github.com/prometheus/client_golang/prometheus/promhttp"
"sneak.berlin/go/dnswatcher/static"
) )
// requestTimeout is the maximum duration for handling a request. // requestTimeout is the maximum duration for handling a request.
@@ -47,13 +45,7 @@ func (s *Server) SetupRoutes() {
public.Get("/", s.handlers.HandleDashboard()) public.Get("/", s.handlers.HandleDashboard())
// Static assets (embedded CSS/JS) // Static assets (embedded CSS/JS)
public.Mount( public.Mount("/s", http.StripPrefix("/s", s.handlers.HandleStatic()))
"/s",
http.StripPrefix(
"/s",
http.FileServer(http.FS(static.Static)),
),
)
// Health check (standard well-known path) // Health check (standard well-known path)
public.Get( public.Get(
+40
View File
@@ -1,6 +1,7 @@
package server_test package server_test
import ( import (
"mime"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"testing" "testing"
@@ -288,3 +289,42 @@ func TestMetricsRateLimitComesBeforeAuth(t *testing.T) {
t.Errorf("another address: status = %d, want 200", rec.Code) t.Errorf("another address: status = %d, want 200", rec.Code)
} }
} }
// TestStaticDirectoriesAnswerNotFound checks that a request naming a
// directory of the static assets, with or without the trailing slash,
// answers 404 rather than a listing of it or a redirect to one, and
// that the stylesheet is still served as CSS.
func TestStaticDirectoriesAnswerNotFound(t *testing.T) {
viper.Reset()
t.Setenv("DNSWATCHER_TARGETS", "example.com")
srv := routedServer(t)
for _, path := range []string{"/s", "/s/", "/s/css", "/s/css/"} {
rec := serve(srv, httptest.NewRequestWithContext(
t.Context(), http.MethodGet, path, nil,
))
if rec.Code != http.StatusNotFound {
t.Errorf("GET %s: status = %d, want 404", path, rec.Code)
}
}
rec := serve(srv, httptest.NewRequestWithContext(
t.Context(), http.MethodGet, "/s/css/tailwind.min.css", nil,
))
if rec.Code != http.StatusOK {
t.Errorf("GET stylesheet: status = %d, want 200", rec.Code)
}
contentType := rec.Header().Get("Content-Type")
mediaType, _, err := mime.ParseMediaType(contentType)
if err != nil || mediaType != "text/css" {
t.Errorf(
"GET stylesheet: Content-Type = %q, want text/css",
contentType,
)
}
}