diff --git a/client/internal/dns/local/local.go b/client/internal/dns/local/local.go index 4a75a76b6..d13aa672e 100644 --- a/client/internal/dns/local/local.go +++ b/client/internal/dns/local/local.go @@ -26,6 +26,19 @@ type resolver interface { LookupNetIP(ctx context.Context, network, host string) ([]netip.Addr, error) } +// PeerConnectivity reports whether a tunnel IP belongs to a peer the +// client knows about and whether that peer is currently connected. The +// local resolver uses this to suppress A/AAAA answers whose RDATA points +// at a disconnected peer (typical case: a synthesized private-service +// record pointing at an embedded proxy peer that just went offline). +// +// known=false means the IP isn't in the local peerstore at all — the +// record is left alone (it points at something outside our mesh, e.g. +// a non-peer upstream). +type PeerConnectivity interface { + IsConnectedByIP(ip string) (known, connected bool) +} + type Resolver struct { mu sync.RWMutex records map[dns.Question][]dns.RR @@ -33,6 +46,11 @@ type Resolver struct { // zones maps zone domain -> NonAuthoritative (true = non-authoritative, user-created zone) zones map[domain.Domain]bool resolver resolver + // peerConn, when non-nil, is consulted on every A/AAAA answer to + // drop records pointing at disconnected peers. nil disables the + // filter and preserves the legacy "return whatever is registered" + // behaviour for callers that never wire a status source. + peerConn PeerConnectivity ctx context.Context cancel context.CancelFunc @@ -49,6 +67,15 @@ func NewResolver() *Resolver { } } +// SetPeerConnectivity wires the per-IP connectivity check used to filter +// out A/AAAA answers pointing at disconnected peers. Pass nil to disable. +// Safe to call multiple times; the latest value wins. +func (d *Resolver) SetPeerConnectivity(p PeerConnectivity) { + d.mu.Lock() + defer d.mu.Unlock() + d.peerConn = p +} + func (d *Resolver) MatchSubdomains() bool { return true } @@ -95,6 +122,7 @@ func (d *Resolver) ServeDNS(w dns.ResponseWriter, r *dns.Msg) { replyMessage.RecursionAvailable = true result := d.lookupRecords(logger, question) + result.records = d.filterDisconnectedPeerAnswers(logger, question, result.records) replyMessage.Authoritative = !result.hasExternalData replyMessage.Answer = result.records replyMessage.Rcode = d.determineRcode(question, result) @@ -436,6 +464,78 @@ func (d *Resolver) logDNSError(logger *log.Entry, hostname string, qtype uint16, } } +// filterDisconnectedPeerAnswers drops A/AAAA records whose RDATA matches +// a known but disconnected peer. The synthesized private-service zones +// emit one A record per connected proxy peer in a cluster; when a peer +// goes offline, the server-side refresh removes the record from the +// next netmap, but the client may still hold the previous netmap for a +// short window. This filter is the local belt to that braces — even on +// the stale netmap, the resolver hides the offline target. +// +// Records pointing at unknown IPs (outside the local peerstore, e.g. +// non-mesh upstreams) are never dropped. Non-A/AAAA records pass +// through untouched. +// +// Escape hatch: if filtering would leave the answer empty AND at least +// one record was filtered, the original list is returned. Better to +// hand the client a record that may not respond than NXDOMAIN it +// completely when every proxy peer is offline (the upstream may still +// be reachable some other way, or the peerstore may be stale). +func (d *Resolver) filterDisconnectedPeerAnswers(logger *log.Entry, question dns.Question, records []dns.RR) []dns.RR { + if len(records) == 0 { + return records + } + d.mu.RLock() + checker := d.peerConn + d.mu.RUnlock() + if checker == nil { + return records + } + + kept := make([]dns.RR, 0, len(records)) + var dropped int + for _, rr := range records { + ip := extractRecordIP(rr) + if ip == "" { + kept = append(kept, rr) + continue + } + known, connected := checker.IsConnectedByIP(ip) + if known && !connected { + dropped++ + continue + } + kept = append(kept, rr) + } + if dropped == 0 { + return records + } + if len(kept) == 0 { + logger.Debugf("all %d answers for %s point at disconnected peers; returning the original list", dropped, question.Name) + return records + } + logger.Tracef("dropped %d disconnected-peer answer(s) for %s, returning %d", dropped, question.Name, len(kept)) + return kept +} + +// extractRecordIP returns the dotted-decimal / colon-hex IP carried by +// an A or AAAA record, or "" for any other record type. +func extractRecordIP(rr dns.RR) string { + switch r := rr.(type) { + case *dns.A: + if r.A == nil { + return "" + } + return r.A.String() + case *dns.AAAA: + if r.AAAA == nil { + return "" + } + return r.AAAA.String() + } + return "" +} + // Update replaces all zones and their records func (d *Resolver) Update(customZones []nbdns.CustomZone) { d.mu.Lock() diff --git a/client/internal/dns/local/local_test.go b/client/internal/dns/local/local_test.go index 2c6b7dbc3..fdf7f2659 100644 --- a/client/internal/dns/local/local_test.go +++ b/client/internal/dns/local/local_test.go @@ -30,6 +30,21 @@ func (m *mockResolver) LookupNetIP(ctx context.Context, network, host string) ([ return nil, nil } +// mockPeerConnectivity returns canned (known, connected) results per IP. +// Used by the disconnected-peer filter tests below. IPs not in the map +// are reported as unknown so the filter leaves them alone. +type mockPeerConnectivity struct { + byIP map[string]struct{ known, connected bool } +} + +func (m mockPeerConnectivity) IsConnectedByIP(ip string) (known, connected bool) { + v, ok := m.byIP[ip] + if !ok { + return false, false + } + return v.known, v.connected +} + func TestLocalResolver_ServeDNS(t *testing.T) { recordA := nbdns.SimpleRecord{ Name: "peera.netbird.cloud.", @@ -2652,3 +2667,114 @@ func BenchmarkIsInManagedZone_ManyZones(b *testing.B) { resolver.isInManagedZone(qname) } } + +// TestLocalResolver_FilterDisconnectedPeerAnswers verifies the +// connectivity-aware filtering layered on top of lookupRecords: +// when an A record's IP belongs to a known peer that's disconnected, +// the record is dropped from the answer. Records for unknown IPs pass +// through. If filtering would empty the answer entirely and at least +// one record was dropped, the original list is restored (escape hatch +// for the "all proxies offline" case). +func TestLocalResolver_FilterDisconnectedPeerAnswers(t *testing.T) { + zone := "svc.cluster.netbird." + connectedRec := nbdns.SimpleRecord{ + Name: zone, + Type: int(dns.TypeA), + Class: nbdns.DefaultClass, + TTL: 5, + RData: "100.64.0.10", + } + disconnectedRec := nbdns.SimpleRecord{ + Name: zone, + Type: int(dns.TypeA), + Class: nbdns.DefaultClass, + TTL: 5, + RData: "100.64.0.11", + } + unknownRec := nbdns.SimpleRecord{ + Name: zone, + Type: int(dns.TypeA), + Class: nbdns.DefaultClass, + TTL: 5, + RData: "203.0.113.5", + } + + type ipState struct{ known, connected bool } + tests := []struct { + name string + records []nbdns.SimpleRecord + connByIP map[string]ipState + wantInOrder []string + }{ + { + name: "drops disconnected peer, keeps connected", + records: []nbdns.SimpleRecord{connectedRec, disconnectedRec}, + connByIP: map[string]ipState{ + "100.64.0.10": {known: true, connected: true}, + "100.64.0.11": {known: true, connected: false}, + }, + wantInOrder: []string{"100.64.0.10"}, + }, + { + name: "unknown IPs pass through untouched", + records: []nbdns.SimpleRecord{unknownRec, disconnectedRec}, + connByIP: map[string]ipState{ + "100.64.0.11": {known: true, connected: false}, + }, + wantInOrder: []string{"203.0.113.5"}, + }, + { + name: "all disconnected falls back to original list", + records: []nbdns.SimpleRecord{disconnectedRec, connectedRec}, + connByIP: map[string]ipState{ + "100.64.0.10": {known: true, connected: false}, + "100.64.0.11": {known: true, connected: false}, + }, + wantInOrder: []string{"100.64.0.11", "100.64.0.10"}, + }, + { + name: "no checker wired returns all records", + records: []nbdns.SimpleRecord{connectedRec, disconnectedRec}, + connByIP: nil, + wantInOrder: []string{"100.64.0.10", "100.64.0.11"}, + }, + } + + for _, tc := range tests { + t.Run(tc.name, func(t *testing.T) { + resolver := NewResolver() + if tc.connByIP != nil { + cm := mockPeerConnectivity{byIP: make(map[string]struct{ known, connected bool }, len(tc.connByIP))} + for ip, st := range tc.connByIP { + cm.byIP[ip] = struct{ known, connected bool }{st.known, st.connected} + } + resolver.SetPeerConnectivity(cm) + } + resolver.Update([]nbdns.CustomZone{{ + Domain: strings.TrimSuffix(zone, "."), + Records: tc.records, + NonAuthoritative: true, + }}) + + var got *dns.Msg + writer := &test.MockResponseWriter{ + WriteMsgFunc: func(m *dns.Msg) error { + got = m + return nil + }, + } + req := new(dns.Msg).SetQuestion(zone, dns.TypeA) + resolver.ServeDNS(writer, req) + + require.NotNil(t, got, "resolver must produce a response") + require.Len(t, got.Answer, len(tc.wantInOrder), + "answer count must match expected: %v", tc.wantInOrder) + for i, want := range tc.wantInOrder { + a, ok := got.Answer[i].(*dns.A) + require.True(t, ok, "answer[%d] must be an A record", i) + assert.Equal(t, want, a.A.String(), + "answer[%d] expected %s got %s", i, want, a.A.String()) + } + }) + } +} diff --git a/client/internal/dns/server.go b/client/internal/dns/server.go index e689f3586..7a35e56d8 100644 --- a/client/internal/dns/server.go +++ b/client/internal/dns/server.go @@ -301,6 +301,11 @@ func newDefaultServer( warningDelayBase: defaultWarningDelayBase, healthRefresh: make(chan struct{}, 1), } + // Wire the local resolver against the peer status recorder so it can + // suppress A/AAAA answers that point at disconnected peers (typical + // case: synthesised private-service records pointing at an embedded + // proxy peer that just went offline). + defaultServer.localResolver.SetPeerConnectivity(localPeerConnectivity{statusRecorder}) // register with root zone, handler chain takes care of the routing dnsService.RegisterMux(".", handlerChain) @@ -1386,3 +1391,25 @@ func (s *DefaultServer) PopulateManagementDomain(mgmtURL *url.URL) error { } return nil } + +// localPeerConnectivity adapts *peer.Status to local.PeerConnectivity so +// the local resolver can ask "is this IP a known peer and is it +// connected?" without taking on the peer package as a dependency. +// A nil status recorder always reports known=false so the resolver +// short-circuits to the legacy "return everything" path. +type localPeerConnectivity struct { + status *peer.Status +} + +// IsConnectedByIP looks the IP up in the peerstore and surfaces both +// the known and connected bits. Used by Resolver.filterDisconnectedPeerAnswers. +func (l localPeerConnectivity) IsConnectedByIP(ip string) (known, connected bool) { + if l.status == nil { + return false, false + } + state, ok := l.status.PeerStateByIP(ip) + if !ok { + return false, false + } + return true, state.ConnStatus == peer.StatusConnected +}