diff --git a/README.md b/README.md index 9439183..78c14c6 100644 --- a/README.md +++ b/README.md @@ -46,10 +46,15 @@ rejected. - Every **1 hour**, performs a full iterative trace from root servers to discover all authoritative nameservers (NS records) for each domain. - Queries **every** discovered authoritative nameserver independently. -- Stores the NS record set as observed by the delegation chain. +- Stores the NS record set as observed by the delegation chain, and the + IPv4 and IPv6 addresses each nameserver's name resolves to. - Any change triggers a notification: - NS added to or removed from the delegation. - - NS IP address changed (glue record change). + - NS address change: a nameserver that stays in the delegation + resolves to different addresses than on the previous check. A + nameserver added or removed gets only the NS change notification. + When the lookup of a nameserver's addresses fails or finds none, + its previous addresses are kept and nothing is sent. ### DNS Hostname Monitoring (Subdomains) @@ -139,6 +144,8 @@ includes: - **DNS record changes**: Which hostname, which nameserver, what record type, old values, new values. - **DNS NS changes**: Which domain, which nameservers were added/removed. +- **NS address changes**: Which domain, which nameserver, its old and + new addresses. - **NS query failures**: Which nameserver failed, error type (timeout, SERVFAIL, REFUSED, network error), which hostname/domain affected. - **NS recoveries**: Which nameserver recovered, which hostname/domain. @@ -418,6 +425,10 @@ not as a merged view, to enable inconsistency detection. "domains": { "example.com": { "nameservers": ["ns1.example.com.", "ns2.example.com."], + "nameserverAddresses": { + "ns1.example.com.": ["192.0.2.53", "2001:db8::53"], + "ns2.example.com.": ["198.51.100.53"] + }, "lastChecked": "2026-02-19T12:00:00Z" } }, @@ -481,6 +492,10 @@ A nameserver that answers NXDOMAIN or with no records has status `ok` and empty `records`. A nameserver whose query failed has status `error`, empty `records`, and the reason in `error`. +`nameserverAddresses` lists, by nameserver, the sorted addresses its name +resolves to. A state file without it loads, and the next check fills it in +without a notification. + --- ## Entrypoints diff --git a/TODO.md b/TODO.md index 20871f3..14cf343 100644 --- a/TODO.md +++ b/TODO.md @@ -15,10 +15,13 @@ on the 1.0 milestone: https://git.eeqj.de/sneak/dnswatcher/milestone/7 # Next Step -nameserver IP address changes: https://git.eeqj.de/sneak/dnswatcher/issues/105 +trial run of the finished image: +https://git.eeqj.de/sneak/dnswatcher/issues/149 # Completed Steps +- 2026-10-01: each nameserver's addresses are saved with its domain, and a + change while it stays in the delegation is notified (closes #105). - 2026-10-01: the watcher saves state when it stops, and shutdown waits for that save, so it no longer relies on the state's own stop hook (closes #114). - 2026-10-01: `DNSWATCHER_SENTRY_DSN` reports panics in HTTP handlers to Sentry, @@ -105,8 +108,6 @@ nameserver IP address changes: https://git.eeqj.de/sneak/dnswatcher/issues/105 # Future Steps -- trial run of the finished image: - https://git.eeqj.de/sneak/dnswatcher/issues/149 - 1.0 readiness: run it with a real config and read the logs: https://git.eeqj.de/sneak/dnswatcher/issues/66 - `goimports` in `make fmt-check`, Markdown formatting: diff --git a/internal/state/state.go b/internal/state/state.go index 417a4d7..a8af3f9 100644 --- a/internal/state/state.go +++ b/internal/state/state.go @@ -35,9 +35,13 @@ type Params struct { } // DomainState holds the monitoring state for an apex domain. +// NameserverAddresses holds the sorted addresses each nameserver's name +// resolves to, by nameserver name. A state file written before it +// existed loads with it nil. type DomainState struct { - Nameservers []string `json:"nameservers"` - LastChecked time.Time `json:"lastChecked"` + Nameservers []string `json:"nameservers"` + NameserverAddresses map[string][]string `json:"nameserverAddresses"` + LastChecked time.Time `json:"lastChecked"` } // NameserverRecordState holds one NS's response for a hostname. diff --git a/internal/state/state_test.go b/internal/state/state_test.go index fabc142..16f709d 100644 --- a/internal/state/state_test.go +++ b/internal/state/state_test.go @@ -4,6 +4,7 @@ import ( "encoding/json" "os" "path/filepath" + "reflect" "strings" "sync" "testing" @@ -37,6 +38,10 @@ func populateState(t *testing.T, s *state.State) { s.SetDomainState("example.com", &state.DomainState{ Nameservers: []string{testNS1, testNS2}, + NameserverAddresses: map[string][]string{ + testNS1: {testIP, testIPv4}, + testNS2: {testIPv4}, + }, LastChecked: now, }) @@ -123,6 +128,64 @@ func TestSaveLoadRoundTrip_Domains(t *testing.T) { if len(dom.Nameservers) != 2 { t.Errorf("expected 2 nameservers, got %d", len(dom.Nameservers)) } + + want := map[string][]string{ + testNS1: {testIP, testIPv4}, + testNS2: {testIPv4}, + } + if !reflect.DeepEqual(dom.NameserverAddresses, want) { + t.Errorf( + "nameserver addresses: got %v, want %v", + dom.NameserverAddresses, want, + ) + } +} + +// TestLoadStateFromBeforeNameserverAddresses loads a state file written +// before nameserver addresses were saved. +func TestLoadStateFromBeforeNameserverAddresses(t *testing.T) { + t.Parallel() + + dir := t.TempDir() + + data := []byte(`{ + "version": 1, + "lastUpdated": "2026-02-19T12:00:00Z", + "domains": { + "example.com": { + "nameservers": ["ns1.example.com.", "ns2.example.com."], + "lastChecked": "2026-02-19T12:00:00Z" + } + } + }`) + + err := os.WriteFile(filepath.Join(dir, "state.json"), data, 0o600) + if err != nil { + t.Fatalf("writing state file: %v", err) + } + + s := state.NewForTestWithDataDir(dir) + + err = s.Load() + if err != nil { + t.Fatalf("Load() error: %v", err) + } + + dom, ok := s.GetDomainState("example.com") + if !ok { + t.Fatal("missing domain example.com") + } + + if !reflect.DeepEqual(dom.Nameservers, []string{testNS1, testNS2}) { + t.Errorf("nameservers: got %v", dom.Nameservers) + } + + if dom.NameserverAddresses != nil { + t.Errorf( + "nameserver addresses: got %v, want none", + dom.NameserverAddresses, + ) + } } // TestSaveLoadRoundTrip_Hostnames verifies hostname data survives a save/load cycle. diff --git a/internal/watcher/export_test.go b/internal/watcher/export_test.go index 5bebe13..772253f 100644 --- a/internal/watcher/export_test.go +++ b/internal/watcher/export_test.go @@ -48,6 +48,25 @@ func (w *Watcher) DetectHostnameChanges( w.detectHostnameChanges(ctx, hostname, prev, current) } +// ResolveNameserverAddresses exports resolveNameserverAddresses for +// testing. +func (w *Watcher) ResolveNameserverAddresses( + ctx context.Context, + nameservers []string, + prev map[string][]string, +) map[string][]string { + return w.resolveNameserverAddresses(ctx, nameservers, prev) +} + +// DetectNSAddressChanges exports detectNSAddressChanges for testing. +func (w *Watcher) DetectNSAddressChanges( + ctx context.Context, + domain string, + prev, current map[string][]string, +) { + w.detectNSAddressChanges(ctx, domain, prev, current) +} + // BuildHostnameState exports buildHostnameState for testing. func BuildHostnameState( results map[string]*resolver.NameserverResponse, diff --git a/internal/watcher/nsaddress_test.go b/internal/watcher/nsaddress_test.go new file mode 100644 index 0000000..6eaf3de --- /dev/null +++ b/internal/watcher/nsaddress_test.go @@ -0,0 +1,151 @@ +package watcher_test + +import ( + "context" + "log/slog" + "reflect" + "testing" + + "sneak.berlin/go/dnswatcher/internal/livednstest" + "sneak.berlin/go/dnswatcher/internal/resolver" + "sneak.berlin/go/dnswatcher/internal/watcher" +) + +const domain = "example.net" + +func TestNSAddressChangeAlerts(t *testing.T) { + t.Parallel() + + // Each case is the nameserver addresses saved by the previous check + // and by the current one. + tests := []struct { + name string + prev, current map[string][]string + want int + }{ + { + "same addresses", + map[string][]string{nsA: {ip1, ip2}}, + map[string][]string{nsA: {ip1, ip2}}, + 0, + }, + { + "same addresses in another order", + map[string][]string{nsA: {ip2, ip1}}, + map[string][]string{nsA: {ip1, ip2}}, + 0, + }, + { + "address replaced", + map[string][]string{nsA: {ip1}}, + map[string][]string{nsA: {ip2}}, + 1, + }, + { + "address added", + map[string][]string{nsA: {ip1}}, + map[string][]string{nsA: {ip1, ip2}}, + 1, + }, + { + "two nameservers changed", + map[string][]string{nsA: {ip1}, nsB: {ip2}}, + map[string][]string{nsA: {ip3}, nsB: {ip3}}, + 2, + }, + { + "nameserver added", + map[string][]string{nsA: {ip1}}, + map[string][]string{nsA: {ip1}, nsB: {ip2}}, + 0, + }, + { + "nameserver removed", + map[string][]string{nsA: {ip1}, nsB: {ip2}}, + map[string][]string{nsA: {ip1}}, + 0, + }, + { + "state file from before addresses were saved", + nil, + map[string][]string{nsA: {ip1}, nsB: {ip2}}, + 0, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + t.Parallel() + + notifier := &mockNotifier{} + w := watcher.NewForTest(nil, nil, nil, nil, nil, notifier) + + w.DetectNSAddressChanges(t.Context(), domain, tt.prev, tt.current) + + got := len(notifier.getNotifications()) + if got != tt.want { + t.Errorf("sent %d address changes, want %d", got, tt.want) + } + }) + } +} + +func TestNSAddressChangeAlertNamesDomainNameserverAndAddresses( + t *testing.T, +) { + t.Parallel() + + notifier := &mockNotifier{} + w := watcher.NewForTest(nil, nil, nil, nil, nil, notifier) + + w.DetectNSAddressChanges( + t.Context(), domain, + map[string][]string{nsA: {ip1}}, + map[string][]string{nsA: {ip2, ip3}}, + ) + + want := notification{ + Title: "NS Address Change: " + domain, + Message: "Domain: " + domain + "\nNameserver: " + nsA + + "\nOld: " + ip1 + "\nNew: " + ip2 + ", " + ip3, + Priority: "warning", + } + + got := notifier.getNotifications() + if len(got) != 1 || got[0] != want { + t.Errorf("sent %v, want %v", got, want) + } +} + +// TestNameserverWithNoAddressKeepsPrevious looks up nameserver names +// with no address: two under .invalid, whose lookup fails with an +// error, and one that does not exist under a real zone, which live DNS +// answers with no address and no error. Each one with addresses saved +// by the previous check keeps them; the one without gets none. +func TestNameserverWithNoAddressKeepsPrevious(t *testing.T) { + t.Parallel() + + w := watcher.NewForTest( + nil, nil, resolver.NewFromLogger(slog.Default()), nil, nil, nil, + ) + + nonexistentNS := "this-surely-does-not-exist-xyz." + testSmallDomain + "." + + prev := map[string][]string{oldNS1: {oldIP}, nonexistentNS: {oldIP}} + + var got map[string][]string + + // The result is the same whether or not live DNS answers, so the + // lookup is not retried. + _ = livednstest.Run(func(ctx context.Context) error { + got = w.ResolveNameserverAddresses( + ctx, []string{oldNS1, oldNS2, nonexistentNS}, prev, + ) + + return nil + }) + + if !reflect.DeepEqual(got, prev) { + t.Errorf("saved %v, want %v", got, prev) + } +} diff --git a/internal/watcher/watcher.go b/internal/watcher/watcher.go index 239351b..2a2c523 100644 --- a/internal/watcher/watcher.go +++ b/internal/watcher/watcher.go @@ -234,13 +234,25 @@ func (w *Watcher) checkDomain( now := time.Now().UTC() prev, hasPrev := w.state.GetDomainState(domain) + + var prevAddresses map[string][]string + if hasPrev { + prevAddresses = prev.NameserverAddresses + } + + addresses := w.resolveNameserverAddresses( + ctx, nameservers, prevAddresses, + ) + if hasPrev && !w.firstRun { w.detectNSChanges(ctx, domain, prev.Nameservers, nameservers) + w.detectNSAddressChanges(ctx, domain, prevAddresses, addresses) } w.state.SetDomainState(domain, &state.DomainState{ - Nameservers: nameservers, - LastChecked: now, + Nameservers: nameservers, + NameserverAddresses: addresses, + LastChecked: now, }) // Also look up A/AAAA records for the apex domain so that @@ -308,6 +320,73 @@ func (w *Watcher) detectNSChanges( ) } +// resolveNameserverAddresses returns the sorted addresses each +// nameserver's name resolves to. A nameserver whose lookup fails or +// finds no address keeps its addresses from prev: the resolver finds no +// address, without an error, when every server it asks times out, and +// that is not an address change. +func (w *Watcher) resolveNameserverAddresses( + ctx context.Context, + nameservers []string, + prev map[string][]string, +) map[string][]string { + addresses := make(map[string][]string, len(nameservers)) + + for _, ns := range nameservers { + ips, err := w.resolver.ResolveIPAddresses(ctx, ns) + if err == nil && len(ips) > 0 { + sort.Strings(ips) + addresses[ns] = ips + + continue + } + + w.log.Error( + "no addresses found for nameserver", + "nameserver", ns, + "error", err, + ) + + if prevIPs, ok := prev[ns]; ok { + addresses[ns] = prevIPs + } + } + + return addresses +} + +// detectNSAddressChanges notifies when a nameserver in both checks +// resolves to different addresses. A nameserver added or removed is +// reported by detectNSChanges alone, and one with no addresses saved by +// the previous check, as in a state file from before they were saved, +// is not compared. +func (w *Watcher) detectNSAddressChanges( + ctx context.Context, + domain string, + prev, current map[string][]string, +) { + for ns, cur := range current { + old, ok := prev[ns] + if !ok || sliceEqual(old, cur) { + continue + } + + msg := fmt.Sprintf( + "Domain: %s\nNameserver: %s\nOld: %s\nNew: %s", + domain, ns, + strings.Join(old, ", "), + strings.Join(cur, ", "), + ) + + w.notify.SendNotification( + ctx, + "NS Address Change: "+domain, + msg, + "warning", + ) + } +} + func (w *Watcher) checkHostname( ctx context.Context, hostname string, diff --git a/internal/watcher/watcher_test.go b/internal/watcher/watcher_test.go index c95a550..c166a3e 100644 --- a/internal/watcher/watcher_test.go +++ b/internal/watcher/watcher_test.go @@ -6,6 +6,7 @@ import ( "log/slog" "os" "slices" + "strings" "sync" "testing" "time" @@ -27,11 +28,17 @@ import ( // so tests assert on what the watcher does with the answers, never on // the records these zones publish. testHost's nameservers and addresses // stay the same from one check to the next, which the tests that check -// it twice rely on. +// it twice rely on, and testSmallDomain's nameservers stay the same +// between a test looking them up and its check. A domain check looks up +// each nameserver's addresses, about a second per nameserver, so the +// tests that check a domain use testSmallDomain, which has two +// nameservers, and check it once. The tests that query testDomain's +// nameservers directly do no domain check. const ( - testDomain = "google.com" - testHost = "cloudflare.com" - testIssuer = "DigiCert" + testDomain = "google.com" + testSmallDomain = "example.com" + testHost = "cloudflare.com" + testIssuer = "DigiCert" ) // Saved-state values that live DNS never returns: nameserver names @@ -206,8 +213,10 @@ func defaultTestConfig(t *testing.T) *config.Config { // checkOnce runs the watcher's checks once and returns an error when a // configured name has no hostname state saved by this check, or that -// state holds no address. Either live DNS gave no answer for the name, -// or the watcher saved no fresh result for it. +// state holds no address, or a configured domain's nameserver has no +// address saved or still has oldIP, which the tests save and live DNS +// never returns. Either live DNS gave no answer for the name, or the +// watcher saved no fresh result for it. func checkOnce( ctx context.Context, w *watcher.Watcher, @@ -231,6 +240,20 @@ func checkOnce( } } + for _, name := range deps.config.Domains { + ds, _ := deps.state.GetDomainState(name) + for _, ns := range ds.Nameservers { + ips := ds.NameserverAddresses[ns] + if len(ips) == 0 || slices.Contains(ips, oldIP) { + return fmt.Errorf( + "%s: nameserver %s: %w, or the watcher saved "+ + "no fresh addresses for it", + name, ns, livednstest.ErrNoAnswer, + ) + } + } + } + return nil } @@ -272,6 +295,26 @@ func runChecks( return deps } +// lookupNameservers returns the nameservers live DNS lists for domain, +// for a test to save in the state its check starts from. +func lookupNameservers(t *testing.T, domain string) []string { + t.Helper() + + res := resolver.NewFromLogger(slog.Default()) + + var nameservers []string + + livednstest.Retry(t, "LookupNS("+domain+")", func(ctx context.Context) error { + var err error + + nameservers, err = res.LookupNS(ctx, domain) + + return err + }) + + return nameservers +} + // addresses returns the A and AAAA values saved for a hostname. func addresses(hs *state.HostnameState) []string { var ips []string @@ -324,7 +367,7 @@ func TestFirstRunBaseline(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Domains = []string{testDomain} + cfg.Domains = []string{testSmallDomain} cfg.Hostnames = []string{testHost} deps := runChecks(t, cfg, nil, nil) @@ -377,7 +420,7 @@ func TestDomainPortAndTLSChecks(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Domains = []string{testDomain} + cfg.Domains = []string{testSmallDomain} deps := runChecks(t, cfg, nil, nil) @@ -416,23 +459,106 @@ func TestNSChangeDetection(t *testing.T) { t.Parallel() cfg := defaultTestConfig(t) - cfg.Domains = []string{testDomain} + cfg.Domains = []string{testSmallDomain} // The saved state lists nameservers that live DNS does not. deps := runChecks(t, cfg, func(deps *testDeps) { - deps.state.SetDomainState(testDomain, &state.DomainState{ + deps.state.SetDomainState(testSmallDomain, &state.DomainState{ Nameservers: []string{oldNS1, oldNS2}, }) }, nil) - assertNotified(t, deps, "NS Change: "+testDomain, "warning") + assertNotified(t, deps, "NS Change: "+testSmallDomain, "warning") - ds, _ := deps.state.GetDomainState(testDomain) + ds, _ := deps.state.GetDomainState(testSmallDomain) if slices.Contains(ds.Nameservers, oldNS1) { t.Errorf("saved nameservers not updated: %v", ds.Nameservers) } } +func TestNSAddressChangeDetection(t *testing.T) { + t.Parallel() + + cfg := defaultTestConfig(t) + cfg.Domains = []string{testSmallDomain} + + nameservers := lookupNameservers(t, testSmallDomain) + + // The saved state lists the nameservers live DNS lists, each at an + // address live DNS never returns. + deps := runChecks(t, cfg, func(deps *testDeps) { + nsAddresses := make(map[string][]string, len(nameservers)) + for _, ns := range nameservers { + nsAddresses[ns] = []string{oldIP} + } + + deps.state.SetDomainState(testSmallDomain, &state.DomainState{ + Nameservers: nameservers, + NameserverAddresses: nsAddresses, + }) + }, nil) + + title := "NS Address Change: " + testSmallDomain + ds, _ := deps.state.GetDomainState(testSmallDomain) + + // One alert per nameserver, naming it and the address it had. + for _, ns := range ds.Nameservers { + prefix := "Domain: " + testSmallDomain + "\nNameserver: " + ns + + "\nOld: " + oldIP + "\nNew: " + + sent := 0 + + for _, n := range deps.notifier.getNotifications() { + if n.Title == title && strings.HasPrefix(n.Message, prefix) { + sent++ + } + } + + if sent != 1 { + t.Errorf("sent %d address changes for %s, want 1", sent, ns) + } + } + + if n := countNotifications(deps, title); n != len(ds.Nameservers) { + t.Errorf( + "sent %d address changes for %d nameservers", + n, len(ds.Nameservers), + ) + } + + if n := countNotifications(deps, "NS Change: "+testSmallDomain); n != 0 { + t.Errorf("sent %d NS changes, want 0", n) + } +} + +func TestNSAddedAndRemovedIsNoAddressChange(t *testing.T) { + t.Parallel() + + cfg := defaultTestConfig(t) + cfg.Domains = []string{testSmallDomain} + + nameservers := lookupNameservers(t, testSmallDomain) + + // The saved state lists oldNS1, which live DNS does not, in place of + // the first nameserver live DNS lists, so that the check finds that + // one added and oldNS1 removed. Only oldNS1 has addresses saved. + deps := runChecks(t, cfg, func(deps *testDeps) { + deps.state.SetDomainState(testSmallDomain, &state.DomainState{ + Nameservers: append([]string{oldNS1}, nameservers[1:]...), + NameserverAddresses: map[string][]string{oldNS1: {oldIP}}, + }) + }, nil) + + if n := countNotifications(deps, "NS Change: "+testSmallDomain); n != 1 { + t.Errorf("sent %d NS changes, want 1", n) + } + + title := "NS Address Change: " + testSmallDomain + if n := countNotifications(deps, title); n != 0 { + t.Errorf("sent %d address changes, want 0", n) + } +} + func TestRecordChangeDetection(t *testing.T) { t.Parallel()