Compare commits

..
Author SHA1 Message Date
clawbot db3b09c5d8 handlers: a directory under /s/ answers 404, not a listing (closes #286)
check / check (push) Waiting to run
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
clawbot caa3aa3c12 notify: a reply other than 2xx is a failed delivery (closes #287)
check / check (push) Canceled after 0s
A notification endpoint that answered with a redirect was counted as
delivered: the request is not sent through a client, so the redirect
is never followed, and only a status of 400 or more counted as a
failure. Now any status outside 200-299 fails the delivery for ntfy,
Slack and Mattermost, so it is retried and logged like the others.

Model: opus-5-5
2026-10-15 16:14:20 +02:00
clawbot dee69c4780 resolver: a nameserver is asked at each of its addresses in turn (closes #285)
check / check (push) Canceled after 0s
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; one given no address has
failed. The README's DNS Resolution Strategy says which addresses are
asked.

Model: opus-5-5
2026-10-15 15:48:15 +02:00
clawbot 5e2c456e85 notify: the log names a notification endpoint, not its webhook URL (closes #288)
check / check (push) Canceled after 0s
With debug logging on, each delivery logged the full Slack, Mattermost
or ntfy URL, whose path or query lets anyone who has it post to the
channel or topic. The two debug lines now give the endpoint's name and
the title; sendSlack is told which of Slack or Mattermost it sends to.
A webhook URL that does not parse gave a startup error quoting the
whole URL; it now gives only the reason. Errors from a failed request
come from the transport, not from an HTTP client, and carry no part of
the path or query; a test now checks that too.

Model: opus-5-5
2026-10-15 14:48:17 +02:00
16 changed files with 666 additions and 127 deletions
+20 -13
View File
@@ -133,18 +133,19 @@ notification endpoint set, changes show only on the dashboard; see
- Queries **each** authoritative nameserver independently for **all** record - Queries **each** authoritative nameserver independently for **all** record
types: A, AAAA, CNAME, MX, TXT, SRV, CAA, NS. types: A, AAAA, CNAME, MX, TXT, SRV, CAA, NS.
- Each record type is a query of its own. When a nameserver answers some types - 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 but the query for another gets no usable reply at any of its addresses (no
error reply such as SERVFAIL, a referral, or a reply too large for UDP whose reply after two tries, an error reply such as SERVFAIL, a referral, or a reply
retry over TCP fails), the failure is logged with the reason, and the type is too large for UDP whose retry over TCP fails), the failure is logged with the
listed in the nameserver's `failedTypes` and keeps the records saved for the reason, and the type is listed in the nameserver's `failedTypes` and keeps the
nameserver by the previous check. On that check those records are not compared records saved for the nameserver by the previous check. On that check those
with the other nameservers', so no record change or inconsistency is reported records are not compared with the other nameservers', so no record change or
for the type; on the next check they are compared with the nameserver's answer inconsistency is reported for the type; on the next check they are compared
as usual. When the previous check did not know the type's records either, with the nameserver's answer as usual. When the previous check did not know
because the nameserver was new or failing then or the type was already listed the type's records either, because the nameserver was new or failing then or
in `unknownTypes`, the type is also listed in `unknownTypes` and left out of the type was already listed in `unknownTypes`, the type is also listed in
every comparison until it answers. A nameserver none of whose queries got a `unknownTypes` and left out of every comparison until it answers. A nameserver
usable reply has failed (see NS query failure below). 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 - Stores results **per nameserver**. The state for a hostname is not a merged
view — it is a map from nameserver to record set. view — it is a map from nameserver to record set.
- DNS names inside record values (CNAME, MX, SRV and NS targets) are stored in - DNS names inside record values (CNAME, MX, SRV and NS targets) are stored in
@@ -285,7 +286,7 @@ includes:
Each endpoint is sent each notification on its own, in the background. A Each endpoint is sent each notification on its own, in the background. A
delivery that fails (a network error, no reply within 10 seconds, or an HTTP delivery that fails (a network error, no reply within 10 seconds, or an HTTP
status of 400 or more) is retried up to 5 times: the first retry after about 1 status other than 2xx) is retried up to 5 times: the first retry after about 1
second, each wait after that twice as long up to 60 seconds, every wait varied second, each wait after that twice as long up to 60 seconds, every wait varied
at random by up to 25%. A delivery still failing after that is logged and at random by up to 25%. A delivery still failing after that is logged and
dropped. dropped.
@@ -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 lookup can need others in turn; lookups go at most three deep, one inside
another, so delegations that point at each other still end. 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: This approach ensures:
- Independence from any upstream resolver's cache or filtering. - Independence from any upstream resolver's cache or filtering.
@@ -904,6 +910,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
+5 -7
View File
@@ -19,13 +19,11 @@ 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. `make test` must finish in under - Test duration depends on network conditions; timeout tuning keeps the suite
60 seconds, the hard cap in `REPO_POLICIES.md`; under 20 seconds is the target within the 60-second target
- Each query to a nameserver has a 2-second timeout, and a query over UDP that - Query timeout is calibrated to 3× maximum antipodal RTT (~300ms) plus
gets no reply is sent once more (`queryTimeoutDuration` and `maxRetries` in processing margin
`internal/resolver/iterative.go`) - Root server fan-out is limited to reduce parallel query load
- 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
+8 -4
View File
@@ -19,10 +19,14 @@ milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7
# Completed Steps # Completed Steps
- 2026-10-15: `TESTING.md` Constraints give the resolver's 2-second query - 2026-10-15: a request under `/s/` that names a directory, such as `/s/` or
timeout and second try, say a zone's servers are asked one at a time in a `/s/css`, answers 404 instead of a listing of its files (closes #286).
random order, and give the 60-second cap and 20-second target for `make test` - 2026-10-15: a notification endpoint that answers with a redirect, or any
(closes #289). 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
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 - 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). 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 - 2026-10-13: an IPv6 address is written in brackets before its port, as in
+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)
}
}
+185
View File
@@ -6,10 +6,12 @@ import (
"encoding/json" "encoding/json"
"errors" "errors"
"io" "io"
"log/slog"
"maps" "maps"
"net/http" "net/http"
"net/http/httptest" "net/http/httptest"
"net/url" "net/url"
"slices"
"strings" "strings"
"sync" "sync"
"testing" "testing"
@@ -38,6 +40,13 @@ const (
prioUrgent = "urgent" prioUrgent = "urgent"
) )
// Endpoint names, as the log gives them, used across multiple tests.
const (
endpointNtfy = "ntfy"
endpointSlack = "slack"
endpointMattermost = "mattermost"
)
// testHost is the hostname used in request construction tests. // testHost is the hostname used in request construction tests.
const testHost = "example.com" const testHost = "example.com"
@@ -1094,6 +1103,182 @@ func TestSendNotificationErrorNamesEndpoint(t *testing.T) {
} }
} }
// ── webhook URLs kept out of the log ──────────────────────
// webhookPathAndQuery follows the server's address in every
// endpoint's URL in the tests below. A webhook URL's path or query
// lets anyone post to the channel or topic, so neither word marked
// secret in it may reach the log.
const webhookPathAndQuery = "/hooks/secret-in-path?token=secret-in-query"
// assertNoWebhookSecret fails the test if text holds either word
// marked secret in webhookPathAndQuery.
func assertNoWebhookSecret(t *testing.T, text string) {
t.Helper()
for _, secret := range []string{"secret-in-path", "secret-in-query"} {
if strings.Contains(text, secret) {
t.Errorf("%q from the webhook URL is in:\n%s", secret, text)
}
}
}
// logRecord is the part of a JSON log line the tests below read.
type logRecord struct {
Level string `json:"level"`
Msg string `json:"msg"`
Endpoint string `json:"endpoint"`
Title string `json:"title"`
Error string `json:"error"`
}
// parseLog decodes each line of a JSON log.
func parseLog(t *testing.T, logs string) []logRecord {
t.Helper()
var records []logRecord
for line := range strings.Lines(logs) {
var record logRecord
err := json.Unmarshal([]byte(line), &record)
if err != nil {
t.Fatalf("decoding log line %q: %v", line, err)
}
records = append(records, record)
}
return records
}
// sendToEachEndpoint sends one notification with the given title to
// ntfy, Slack and Mattermost, each at baseURL followed by
// webhookPathAndQuery, and retries a failed delivery once. When
// every delivery has finished it returns the JSON log written,
// debug lines included.
func sendToEachEndpoint(t *testing.T, baseURL, title string) string {
t.Helper()
target, err := url.Parse(baseURL + webhookPathAndQuery)
if err != nil {
t.Fatalf("parsing webhook URL: %v", err)
}
svc, logs := newLoggingService(http.DefaultTransport)
svc.SetNtfyURL(target)
svc.SetSlackWebhookURL(target)
svc.SetMattermostWebhookURL(target)
svc.SetSleepFunc(instantSleep)
svc.SetRetryConfig(notify.RetryConfig{
MaxRetries: 1,
BaseDelay: time.Millisecond,
MaxDelay: time.Millisecond,
})
svc.SendNotification(context.Background(), title, "m", prioInfo)
waitForCondition(t, func() bool {
return svc.OutstandingDeliveries() == 0
})
return logs.String()
}
// assertEachEndpointRetriedThenFailed fails the test unless, for each
// of ntfy, Slack and Mattermost, the lines of logs whose error
// contains reason are the retry line and then the final failure line.
func assertEachEndpointRetriedThenFailed(
t *testing.T, logs, reason string,
) {
t.Helper()
// For each endpoint, the messages of its lines whose error
// contains reason, in the order they were logged.
got := map[string][]string{}
for _, record := range parseLog(t, logs) {
if strings.Contains(record.Error, reason) {
got[record.Endpoint] = append(
got[record.Endpoint], record.Msg,
)
}
}
lines := []string{
"notification delivery failed, retrying",
"failed to send notification after retries",
}
want := map[string][]string{
endpointNtfy: lines,
endpointSlack: lines,
endpointMattermost: lines,
}
if !maps.EqualFunc(got, want, slices.Equal) {
t.Errorf("lines whose error contains %q = %v, want %v",
reason, got, want)
}
}
// TestSendNotificationDebugLogOmitsWebhookURL checks that, with debug
// logging on, a delivery to each endpoint logs the endpoint's name and
// the title, and no part of the URL's path or query.
func TestSendNotificationDebugLogOmitsWebhookURL(t *testing.T) {
t.Parallel()
const title = "Port Change"
srv := httptest.NewServer(
http.HandlerFunc(
func(w http.ResponseWriter, _ *http.Request) {
w.WriteHeader(http.StatusOK)
}),
)
defer srv.Close()
logs := sendToEachEndpoint(t, srv.URL, title)
assertNoWebhookSecret(t, logs)
got := map[string]string{}
for _, record := range parseLog(t, logs) {
if record.Level == slog.LevelDebug.String() {
got[record.Endpoint] = record.Title
}
}
want := map[string]string{
endpointNtfy: title,
endpointSlack: title,
endpointMattermost: title,
}
if !maps.Equal(got, want) {
t.Errorf("debug lines' endpoint and title = %v, want %v", got, want)
}
}
// TestSendNotificationErrorLogOmitsWebhookURL checks that a delivery
// to each endpoint that cannot connect logs the transport's error,
// which names the refused connection, on its retry line and on its
// final failure line, and that no line holds any part of the URL's
// path or query.
func TestSendNotificationErrorLogOmitsWebhookURL(t *testing.T) {
t.Parallel()
// Closed before anything is sent, so every attempt fails to
// connect.
srv := httptest.NewServer(http.NotFoundHandler())
srv.Close()
logs := sendToEachEndpoint(t, srv.URL, "t")
assertNoWebhookSecret(t, logs)
assertEachEndpointRetriedThenFailed(t, logs, "connection refused")
}
// ── SlackPayload JSON marshaling ────────────────────────── // ── SlackPayload JSON marshaling ──────────────────────────
func TestSlackPayloadJSON(t *testing.T) { func TestSlackPayloadJSON(t *testing.T) {
+3 -2
View File
@@ -81,7 +81,8 @@ func (svc *Service) SendNtfy(
return svc.sendNtfy(ctx, topicURL, title, message, priority) return svc.sendNtfy(ctx, topicURL, title, message, priority)
} }
// SendSlack exports sendSlack for testing. // SendSlack exports sendSlack for testing. It sends as the
// "slack" endpoint.
func (svc *Service) SendSlack( func (svc *Service) SendSlack(
ctx context.Context, ctx context.Context,
webhookURL *url.URL, webhookURL *url.URL,
@@ -89,7 +90,7 @@ func (svc *Service) SendSlack(
title, message, priority string, title, message, priority string,
) error { ) error {
return svc.sendSlack( return svc.sendSlack(
ctx, webhookURL, failed, title, message, priority, ctx, webhookURL, "slack", failed, title, message, priority,
) )
} }
+30 -13
View File
@@ -25,9 +25,6 @@ import (
// HTTP client timeout. // HTTP client timeout.
const httpClientTimeout = 10 * time.Second const httpClientTimeout = 10 * time.Second
// HTTP status code thresholds.
const httpStatusClientError = 400
// Sentinel errors for notification failures. // Sentinel errors for notification failures.
var ( var (
// ErrNtfyFailed indicates the ntfy request failed. // ErrNtfyFailed indicates the ntfy request failed.
@@ -56,6 +53,15 @@ func IsAllowedScheme(scheme string) bool {
func ValidateWebhookURL(raw string) (*url.URL, error) { func ValidateWebhookURL(raw string) (*url.URL, error) {
u, err := url.ParseRequestURI(raw) u, err := url.ParseRequestURI(raw)
if err != nil { if err != nil {
// A *url.Error quotes the whole URL, and this error is
// logged at startup. A webhook URL's path or query lets
// anyone post to the channel or topic, so keep only the
// reason.
var urlErr *url.Error
if errors.As(err, &urlErr) {
err = urlErr.Err
}
return nil, fmt.Errorf("invalid URL: %w", err) return nil, fmt.Errorf("invalid URL: %w", err)
} }
@@ -277,7 +283,7 @@ func (svc *Service) dispatchSlack(
svc.dispatch(ctx, "slack", func(c context.Context) error { svc.dispatch(ctx, "slack", func(c context.Context) error {
return svc.sendSlack( return svc.sendSlack(
c, svc.slackWebhookURL, ErrSlackFailed, c, svc.slackWebhookURL, "slack", ErrSlackFailed,
title, message, priority, title, message, priority,
) )
}) })
@@ -295,8 +301,8 @@ func (svc *Service) dispatchMattermost(
ctx, "mattermost", ctx, "mattermost",
func(c context.Context) error { func(c context.Context) error {
return svc.sendSlack( return svc.sendSlack(
c, svc.mattermostWebhookURL, ErrMattermostFailed, c, svc.mattermostWebhookURL, "mattermost",
title, message, priority, ErrMattermostFailed, title, message, priority,
) )
}, },
) )
@@ -307,9 +313,11 @@ func (svc *Service) sendNtfy(
topicURL *url.URL, topicURL *url.URL,
title, message, priority string, title, message, priority string,
) error { ) error {
// The topic URL is left out of the log: it lets anyone who has
// it read and post to the topic.
svc.log.Debug( svc.log.Debug(
"sending ntfy notification", "sending ntfy notification",
"topic", topicURL.String(), "endpoint", "ntfy",
"title", title, "title", title,
) )
@@ -333,7 +341,10 @@ func (svc *Service) sendNtfy(
defer func() { _ = resp.Body.Close() }() defer func() { _ = resp.Body.Close() }()
if resp.StatusCode >= httpStatusClientError { // Only a 2xx reply means ntfy took the notification. A
// redirect is not followed, so it is a failure too.
if resp.StatusCode < http.StatusOK ||
resp.StatusCode >= http.StatusMultipleChoices {
return fmt.Errorf( return fmt.Errorf(
"%w: status %d", "%w: status %d",
ErrNtfyFailed, resp.StatusCode, ErrNtfyFailed, resp.StatusCode,
@@ -372,12 +383,15 @@ type SlackAttachment struct {
} }
// sendSlack posts to a Slack or Mattermost incoming webhook, which // sendSlack posts to a Slack or Mattermost incoming webhook, which
// take the same payload. An HTTP error status is returned wrapped in // take the same payload. endpoint, "slack" or "mattermost", names
// failed, ErrSlackFailed or ErrMattermostFailed, so the error names // the webhook in the log in place of its URL, which lets anyone who
// the endpoint. // has it post to the channel. A reply status other than 2xx is
// returned wrapped in failed, ErrSlackFailed or ErrMattermostFailed,
// so the error names the endpoint.
func (svc *Service) sendSlack( func (svc *Service) sendSlack(
ctx context.Context, ctx context.Context,
webhookURL *url.URL, webhookURL *url.URL,
endpoint string,
failed error, failed error,
title, message, priority string, title, message, priority string,
) error { ) error {
@@ -388,7 +402,7 @@ func (svc *Service) sendSlack(
svc.log.Debug( svc.log.Debug(
"sending webhook notification", "sending webhook notification",
"url", webhookURL.String(), "endpoint", endpoint,
"title", title, "title", title,
) )
@@ -423,7 +437,10 @@ func (svc *Service) sendSlack(
defer func() { _ = resp.Body.Close() }() defer func() { _ = resp.Body.Close() }()
if resp.StatusCode >= httpStatusClientError { // Only a 2xx reply means the webhook took the notification. A
// redirect is not followed, so it is a failure too.
if resp.StatusCode < http.StatusOK ||
resp.StatusCode >= http.StatusMultipleChoices {
return fmt.Errorf( return fmt.Errorf(
"%w: status %d", "%w: status %d",
failed, resp.StatusCode, failed, resp.StatusCode,
+17
View File
@@ -79,6 +79,23 @@ func TestValidateWebhookURLInvalid(t *testing.T) {
} }
} }
// TestValidateWebhookURLErrorOmitsURL checks that the error for a
// webhook URL that does not parse, here because a carriage return
// was left at its end, leaves out the URL's path and query: the
// error is logged at startup.
func TestValidateWebhookURLErrorOmitsURL(t *testing.T) {
t.Parallel()
_, err := notify.ValidateWebhookURL(
"https://hooks.example.com" + webhookPathAndQuery + "\r",
)
if err == nil {
t.Fatal("expected an error for a URL ending in a carriage return")
}
assertNoWebhookSecret(t, err.Error())
}
func TestIsAllowedScheme(t *testing.T) { func TestIsAllowedScheme(t *testing.T) {
t.Parallel() t.Parallel()
+30
View File
@@ -536,3 +536,33 @@ func TestSendNotificationPermanentFailureLogsError(
return attempts >= 3 return attempts >= 3
}) })
} }
// TestSendNotificationRedirectFails checks that a 302 reply is a
// failed delivery at each endpoint: the server is sent the first
// attempt and the retry, and each endpoint logs a retry line and
// then the final failure line, both naming the 302.
func TestSendNotificationRedirectFails(t *testing.T) {
t.Parallel()
var requests atomic.Int64
srv := httptest.NewServer(
http.HandlerFunc(
func(w http.ResponseWriter, r *http.Request) {
requests.Add(1)
http.Redirect(w, r, "/moved", http.StatusFound)
}),
)
defer srv.Close()
logs := sendToEachEndpoint(t, srv.URL, "t")
// ntfy, Slack and Mattermost each send once and retry once.
const wantRequests = 3 * 2
if sent := requests.Load(); sent != wantRequests {
t.Errorf("server got %d requests, want %d", sent, wantRequests)
}
assertEachEndpointRetriedThenFailed(t, logs, "status 302")
}
+4
View File
@@ -36,6 +36,10 @@ var (
// so whether the name has addresses is unknown. // so whether the name has addresses is unknown.
ErrNoNameserverAnswered = errors.New("no nameserver answered") 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 // ErrUnusableReply is returned when a server replied with an
// error such as SERVFAIL, or with a referral that leads no // error such as SERVFAIL, or with a referral that leads no
// closer to the name asked about. // closer to the name asked about.
+11
View File
@@ -96,6 +96,17 @@ func (r *Resolver) QueryServers(
return r.queryServers(ctx, servers, zone, name, qtype) 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. // QueryEachNS exports queryEachNS for testing.
func (r *Resolver) QueryEachNS( func (r *Resolver) QueryEachNS(
ctx context.Context, ctx context.Context,
+61 -26
View File
@@ -708,8 +708,9 @@ func (r *Resolver) QueryNameserver(
return r.queryNameserver(ctx, nsHostname, hostname, recordTypes()) return r.queryNameserver(ctx, nsHostname, hostname, recordTypes())
} }
// queryNameserver queries a specific nameserver for the record types // queryNameserver queries a specific nameserver, at the IPv4 addresses
// in qtypes and builds a NameserverResponse. // its name resolves to, for the record types in qtypes and builds a
// NameserverResponse.
func (r *Resolver) queryNameserver( func (r *Resolver) queryNameserver(
ctx context.Context, ctx context.Context,
nsHostname string, nsHostname string,
@@ -727,7 +728,7 @@ func (r *Resolver) queryNameserver(
hostname = dns.Fqdn(hostname) 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, // QueryNameserverIP queries a nameserver by its IP address directly,
@@ -744,23 +745,33 @@ func (r *Resolver) QueryNameserverIP(
hostname = dns.Fqdn(hostname) 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( func (r *Resolver) queryTypes(
ctx context.Context, ctx context.Context,
nsHostname string, nsHostname string,
nsIP string, nsIPs []string,
hostname string, hostname string,
qtypes []uint16, qtypes []uint16,
) (*NameserverResponse, error) { ) (*NameserverResponse, error) {
if len(nsIPs) == 0 {
return nil, fmt.Errorf("%w: %s", ErrNoNameserverAddress, nsHostname)
}
resp := &NameserverResponse{ resp := &NameserverResponse{
Nameserver: nsHostname, Nameserver: nsHostname,
Records: make(map[string][]string), Records: make(map[string][]string),
Status: StatusOK, Status: StatusOK,
} }
state := r.queryEachType(ctx, nsIP, hostname, qtypes, resp) state := r.queryEachType(ctx, nsIPs, hostname, qtypes, resp)
classifyResponse(resp, state) classifyResponse(resp, state)
return resp, nil return resp, nil
@@ -778,28 +789,37 @@ type queryState struct {
answered bool answered bool
} }
// queryEachType asks the nameserver at nsIP about hostname once for each // queryEachType asks the nameserver about hostname's records of each type
// record type in qtypes, and lists in resp.FailedTypes the types whose // in qtypes, at its addresses nsIPs in turn until one gives a usable
// query got no usable reply, logging each with the reason unless ctx was // reply, and lists in resp.FailedTypes the types that got none at any of
// cancelled: shutdown cancels it, and a query it cut short did not fail. // 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( func (r *Resolver) queryEachType(
ctx context.Context, ctx context.Context,
nsIP string, nsIPs []string,
hostname string, hostname string,
qtypes []uint16, qtypes []uint16,
resp *NameserverResponse, resp *NameserverResponse,
) queryState { ) queryState {
var state queryState var state queryState
// Reordered below; the caller's list is left as it was.
nsIPs = slices.Clone(nsIPs)
for _, qtype := range qtypes { for _, qtype := range qtypes {
if checkCtx(ctx) != nil { if checkCtx(ctx) != nil {
break break
} }
err := r.querySingleType(ctx, nsIP, hostname, qtype, resp, &state) answeredAt, err := r.querySingleType(
ctx, nsIPs, hostname, qtype, resp, &state,
)
if err == nil { if err == nil {
state.answered = true state.answered = true
// The next type is asked first at the address that answered.
nsIPs[0], nsIPs[answeredAt] = nsIPs[answeredAt], nsIPs[0]
continue continue
} }
@@ -833,20 +853,30 @@ func (r *Resolver) queryEachType(
return state return state
} }
// querySingleType asks the nameserver at nsIP about hostname's records // querySingleType asks the nameserver about hostname's records of type
// of type qtype. It returns nil when the nameserver answered: with // qtype at its addresses nsIPs, of which there is at least one, one at a
// records, with none, or with NXDOMAIN; otherwise it returns why not. // 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( func (r *Resolver) querySingleType(
ctx context.Context, ctx context.Context,
nsIP string, nsIPs []string,
hostname string, hostname string,
qtype uint16, qtype uint16,
resp *NameserverResponse, resp *NameserverResponse,
state *queryState, state *queryState,
) error { ) (int, error) {
msg, err := r.queryDNS(ctx, nsIP, hostname, qtype) var errs []error
if err != nil {
for i, nsIP := range nsIPs {
msg, err := r.queryDNS(ctx, nsIP, hostname, qtype)
switch { 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): case isTimeout(err):
state.gotTimeout = true state.gotTimeout = true
case errors.Is(err, ErrRefused): case errors.Is(err, ErrRefused):
@@ -855,10 +885,14 @@ func (r *Resolver) querySingleType(
state.netErr = err 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 // 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 // classifyResponse sets the nameserver's status. One that answered no
// record type has failed, and Error says why; one that answered some has // record type has failed, and Error says why; it says all queries timed
// the status of those answers. It has no data only when every type // 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 // answered with no records: a type in FailedTypes may have records, so a
// nameserver with one stays ok. // nameserver with one stays ok.
func classifyResponse(resp *NameserverResponse, state queryState) { func classifyResponse(resp *NameserverResponse, state queryState) {
switch { switch {
case state.gotNXDomain && !state.hasRecords: case state.gotNXDomain && !state.hasRecords:
resp.Status = StatusNXDomain resp.Status = StatusNXDomain
case state.gotTimeout && !state.answered:
resp.Status = StatusTimeout
resp.Error = "all queries timed out"
case state.gotErrorReply && !state.answered: case state.gotErrorReply && !state.answered:
resp.Status = StatusError resp.Status = StatusError
resp.Error = "server returned " + state.errorReply resp.Error = "server returned " + state.errorReply
@@ -975,6 +1007,9 @@ func classifyResponse(resp *NameserverResponse, state queryState) {
case state.gotReferral && !state.answered: case state.gotReferral && !state.answered:
resp.Status = StatusError resp.Status = StatusError
resp.Error = "server returned a referral" 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. // An NXDOMAIN reply with no records was taken by the first case.
case !state.hasRecords && len(resp.FailedTypes) == 0: case !state.hasRecords && len(resp.FailedTypes) == 0:
resp.Status = StatusNoData resp.Status = StatusNoData
+36 -51
View File
@@ -15,8 +15,10 @@ import (
// that answered some record types, even with no records, has not failed // that answered some record types, even with no records, has not failed
// when its query for another type got no usable reply, whatever the // 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 // 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 // every query got none has failed, and its error says all queries timed
// with no records is nodata. // 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) { func TestClassifyResponse(t *testing.T) {
t.Parallel() t.Parallel()
@@ -27,63 +29,46 @@ func TestClassifyResponse(t *testing.T) {
wantStatus string wantStatus string
wantError string wantError string
}{ }{
{ {"every type answered with no records",
"every type answered with no records",
queryState{answered: true}, queryState{answered: true},
nil, nil, StatusNoData, ""},
StatusNoData, "", {"some types answered with no records, another timed out",
},
{
"some types answered with no records, another timed out",
queryState{answered: true, gotTimeout: true}, queryState{answered: true, gotTimeout: true},
[]string{"A"}, []string{"A"}, StatusOK, ""},
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 got SERVFAIL", {"some types answered with no records, another was refused",
queryState{
answered: true, gotErrorReply: true, errorReply: "SERVFAIL",
},
[]string{"A"},
StatusOK, "",
},
{
"some types answered with no records, another was refused",
queryState{answered: true, gotRefused: true}, queryState{answered: true, gotRefused: true},
[]string{"A"}, []string{"A"}, StatusOK, ""},
StatusOK, "", {"some types answered with no records, another got a network error",
},
{
"some types answered with no records, another got a network error",
queryState{answered: true, netErr: syscall.ECONNREFUSED}, queryState{answered: true, netErr: syscall.ECONNREFUSED},
[]string{"A"}, []string{"A"}, StatusOK, ""},
StatusOK, "", {"some types answered with no records, another's reply was " +
}, "truncated and its retry over TCP failed",
{
"some types answered with no records, another's reply was " +
"truncated and its retry over TCP failed",
queryState{answered: true, netErr: ErrTruncated}, queryState{answered: true, netErr: ErrTruncated},
[]string{"TXT"}, []string{"TXT"}, StatusOK, ""},
StatusOK, "", {"some types answered with no records, another got a referral",
},
{
"some types answered with no records, another got a referral",
queryState{answered: true, gotReferral: true}, queryState{answered: true, gotReferral: true},
[]string{"A"}, []string{"A"}, StatusOK, ""},
StatusOK, "", {"every query timed out",
},
{
"every query timed out",
queryState{gotTimeout: true}, queryState{gotTimeout: true},
[]string{"A", "AAAA", "CNAME"}, []string{"A", "AAAA", "CNAME"}, StatusTimeout, "all queries timed out"},
StatusTimeout, "all queries timed out", {"every query got NOTIMP",
},
{
"every query got NOTIMP",
queryState{gotErrorReply: true, errorReply: "NOTIMP"}, queryState{gotErrorReply: true, errorReply: "NOTIMP"},
[]string{"A", "AAAA", "CNAME"}, []string{"A", "AAAA", "CNAME"}, StatusError, "server returned NOTIMP"},
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 { for _, tt := range tests {
+179 -2
View File
@@ -360,6 +360,124 @@ func TestQueryNameserver_ZoneDelegatedWithoutAddresses(t *testing.T) {
assert.Equal(t, resolver.StatusOK, resp.Status) 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) { func TestQueryNameserver_AAAA(t *testing.T) {
t.Parallel() t.Parallel()
@@ -424,8 +542,9 @@ func TestQueryNameserver_TXT(t *testing.T) {
// nameserver about google.com with a resolver whose retries over TCP // 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 // 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 // is reported as failed, holding none of the records that fit, and
// logged with the reason, while the nameserver, which answered the other // logged with the reason, which names the address whose reply was
// types, is ok. // truncated, while the nameserver, which answered the other types, is
// ok.
func TestQueryNameserver_TruncatedReplyWhoseTCPRetryFails(t *testing.T) { func TestQueryNameserver_TruncatedReplyWhoseTCPRetryFails(t *testing.T) {
t.Parallel() t.Parallel()
@@ -442,6 +561,13 @@ func TestQueryNameserver_TruncatedReplyWhoseTCPRetryFails(t *testing.T) {
assert.Contains(t, logs.String(), assert.Contains(t, logs.String(),
"hostname=google.com. nameserver="+ns+" type=TXT error=", "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) { func TestQueryNameserver_NXDomain(t *testing.T) {
@@ -495,6 +621,57 @@ func TestQueryNameserver_Refused(t *testing.T) {
assert.Equal(t, "server returned REFUSED", resp.Error) 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 // TestQueryServers_RecursiveResolverRefused passes a public recursive
// resolver to QueryServers as the server of google.com. These resolvers // resolver to QueryServers as the server of google.com. These resolvers
// refuse a query that does not ask for recursion and answer one that // refuse a query that does not ask for recursion and answer one that
+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,
)
}
}