Update client/internal/dns/handler_chain.go

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>

client/internal/dns/handler_chain.go

@@ -1,7 +1,10 @@
 package dns
 
 import (
+	"context"
 	"fmt"
+	"math"
+	"net"
 	"slices"
 	"strconv"
 	"strings"
@@ -192,6 +195,12 @@ func (c *HandlerChain) logHandlers() {
 }
 
 func (c *HandlerChain) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
+	c.dispatch(w, r, math.MaxInt)
+}
+
+// dispatch routes a DNS request through the chain, skipping handlers with
+// priority > maxPriority. Shared by ServeDNS and ResolveInternal.
+func (c *HandlerChain) dispatch(w dns.ResponseWriter, r *dns.Msg, maxPriority int) {
 	if len(r.Question) == 0 {
 		return
 	}
@@ -216,6 +225,9 @@ func (c *HandlerChain) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
 
 	// Try handlers in priority order
 	for _, entry := range handlers {
+		if entry.Priority > maxPriority {
+			continue
+		}
 		if !c.isHandlerMatch(qname, entry) {
 			continue
 		}
@@ -273,6 +285,55 @@ func (c *HandlerChain) logResponse(logger *log.Entry, cw *ResponseWriterChain, q
 		cw.response.Len(), meta, time.Since(startTime))
 }
 
+// ResolveInternal runs an in-process DNS query against the chain, skipping any
+// handler with priority > maxPriority. Used by internal callers (e.g. the mgmt
+// cache refresher) that must bypass themselves to avoid loops. Honors ctx
+// cancellation; on ctx.Done the dispatch goroutine is left to drain on its own
+// (bounded by the invoked handler's internal timeout).
+func (c *HandlerChain) ResolveInternal(ctx context.Context, r *dns.Msg, maxPriority int) (*dns.Msg, error) {
+	if len(r.Question) == 0 {
+		return nil, fmt.Errorf("empty question")
+	}
+
+	base := &internalResponseWriter{}
+	done := make(chan struct{})
+	go func() {
+		c.dispatch(base, r, maxPriority)
+		close(done)
+	}()
+
+	select {
+	case <-done:
+	case <-ctx.Done():
+		// Prefer a completed response if dispatch finished concurrently with cancellation.
+		select {
+		case <-done:
+		default:
+			return nil, fmt.Errorf("resolve %s: %w", strings.ToLower(r.Question[0].Name), ctx.Err())
+		}
+	}
+
+	if base.response == nil || base.response.Rcode == dns.RcodeRefused {
+		return nil, fmt.Errorf("no handler resolved %s at priority ≤ %d",
+			strings.ToLower(r.Question[0].Name), maxPriority)
+	}
+	return base.response, nil
+}
+
+// HasRootHandlerAtOrBelow reports whether any "." handler is registered at
+// priority ≤ maxPriority.
+func (c *HandlerChain) HasRootHandlerAtOrBelow(maxPriority int) bool {
+	c.mu.RLock()
+	defer c.mu.RUnlock()
+
+	for _, h := range c.handlers {
+		if h.Pattern == "." && h.Priority <= maxPriority {
+			return true
+		}
+	}
+	return false
+}
+
 func (c *HandlerChain) isHandlerMatch(qname string, entry HandlerEntry) bool {
 	switch {
 	case entry.Pattern == ".":
@@ -291,3 +352,36 @@ func (c *HandlerChain) isHandlerMatch(qname string, entry HandlerEntry) bool {
 		}
 	}
 }
+
+// internalResponseWriter captures a dns.Msg for in-process chain queries.
+type internalResponseWriter struct {
+	response *dns.Msg
+}
+
+func (w *internalResponseWriter) WriteMsg(m *dns.Msg) error { w.response = m; return nil }
+func (w *internalResponseWriter) LocalAddr() net.Addr       { return nil }
+func (w *internalResponseWriter) RemoteAddr() net.Addr      { return nil }
+
+// Write unpacks raw DNS bytes so handlers that call Write instead of WriteMsg
+// still surface their answer to ResolveInternal.
+func (w *internalResponseWriter) Write(p []byte) (int, error) {
+	msg := new(dns.Msg)
+	if err := msg.Unpack(p); err != nil {
+		return 0, err
+	}
+	w.response = msg
+	return len(p), nil
+}
+
+func (w *internalResponseWriter) Close() error      { return nil }
+func (w *internalResponseWriter) TsigStatus() error { return nil }
+
+// TsigTimersOnly is part of dns.ResponseWriter.
+func (w *internalResponseWriter) TsigTimersOnly(bool) {
+	// no-op: in-process queries carry no TSIG state.
+}
+
+// Hijack is part of dns.ResponseWriter.
+func (w *internalResponseWriter) Hijack() {
+	// no-op: in-process queries have no underlying connection to hand off.
+}

client/internal/dns/handler_chain_test.go

@@ -1,11 +1,15 @@
 package dns_test
 
 import (
+	"context"
+	"net"
 	"testing"
+	"time"
 
 	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/mock"
+	"github.com/stretchr/testify/require"
 
 	nbdns "github.com/netbirdio/netbird/client/internal/dns"
 	"github.com/netbirdio/netbird/client/internal/dns/test"
@@ -1042,3 +1046,163 @@ func TestHandlerChain_AddRemoveRoundtrip(t *testing.T) {
 		})
 	}
 }
+
+// answeringHandler writes a fixed A record to ack the query. Used to verify
+// which handler ResolveInternal dispatches to.
+type answeringHandler struct {
+	name string
+	ip   string
+}
+
+func (h *answeringHandler) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
+	resp := &dns.Msg{}
+	resp.SetReply(r)
+	resp.Answer = []dns.RR{&dns.A{
+		Hdr: dns.RR_Header{Name: r.Question[0].Name, Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 60},
+		A:   net.ParseIP(h.ip).To4(),
+	}}
+	_ = w.WriteMsg(resp)
+}
+
+func (h *answeringHandler) String() string { return h.name }
+
+func TestHandlerChain_ResolveInternal_SkipsAboveMaxPriority(t *testing.T) {
+	chain := nbdns.NewHandlerChain()
+
+	high := &answeringHandler{name: "high", ip: "10.0.0.1"}
+	low := &answeringHandler{name: "low", ip: "10.0.0.2"}
+
+	chain.AddHandler("example.com.", high, nbdns.PriorityMgmtCache)
+	chain.AddHandler("example.com.", low, nbdns.PriorityUpstream)
+
+	r := new(dns.Msg)
+	r.SetQuestion("example.com.", dns.TypeA)
+
+	resp, err := chain.ResolveInternal(context.Background(), r, nbdns.PriorityUpstream)
+	assert.NoError(t, err)
+	assert.NotNil(t, resp)
+	assert.Equal(t, 1, len(resp.Answer))
+	a, ok := resp.Answer[0].(*dns.A)
+	assert.True(t, ok)
+	assert.Equal(t, "10.0.0.2", a.A.String(), "should skip mgmtCache handler and resolve via upstream")
+}
+
+func TestHandlerChain_ResolveInternal_ErrorWhenNoMatch(t *testing.T) {
+	chain := nbdns.NewHandlerChain()
+	high := &answeringHandler{name: "high", ip: "10.0.0.1"}
+	chain.AddHandler("example.com.", high, nbdns.PriorityMgmtCache)
+
+	r := new(dns.Msg)
+	r.SetQuestion("example.com.", dns.TypeA)
+
+	_, err := chain.ResolveInternal(context.Background(), r, nbdns.PriorityUpstream)
+	assert.Error(t, err, "no handler at or below maxPriority should error")
+}
+
+// rawWriteHandler packs a response and calls ResponseWriter.Write directly
+// (instead of WriteMsg), exercising the internalResponseWriter.Write path.
+type rawWriteHandler struct {
+	ip string
+}
+
+func (h *rawWriteHandler) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
+	resp := &dns.Msg{}
+	resp.SetReply(r)
+	resp.Answer = []dns.RR{&dns.A{
+		Hdr: dns.RR_Header{Name: r.Question[0].Name, Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 60},
+		A:   net.ParseIP(h.ip).To4(),
+	}}
+	packed, err := resp.Pack()
+	if err != nil {
+		return
+	}
+	_, _ = w.Write(packed)
+}
+
+func TestHandlerChain_ResolveInternal_CapturesRawWrite(t *testing.T) {
+	chain := nbdns.NewHandlerChain()
+	chain.AddHandler("example.com.", &rawWriteHandler{ip: "10.0.0.3"}, nbdns.PriorityUpstream)
+
+	r := new(dns.Msg)
+	r.SetQuestion("example.com.", dns.TypeA)
+
+	resp, err := chain.ResolveInternal(context.Background(), r, nbdns.PriorityUpstream)
+	assert.NoError(t, err)
+	require.NotNil(t, resp)
+	require.Len(t, resp.Answer, 1)
+	a, ok := resp.Answer[0].(*dns.A)
+	require.True(t, ok)
+	assert.Equal(t, "10.0.0.3", a.A.String(), "handlers calling Write(packed) must still surface their answer")
+}
+
+func TestHandlerChain_ResolveInternal_EmptyQuestion(t *testing.T) {
+	chain := nbdns.NewHandlerChain()
+	_, err := chain.ResolveInternal(context.Background(), new(dns.Msg), nbdns.PriorityUpstream)
+	assert.Error(t, err)
+}
+
+// hangingHandler blocks indefinitely until closed, simulating a wedged upstream.
+type hangingHandler struct {
+	block chan struct{}
+}
+
+func (h *hangingHandler) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
+	<-h.block
+	resp := &dns.Msg{}
+	resp.SetReply(r)
+	_ = w.WriteMsg(resp)
+}
+
+func (h *hangingHandler) String() string { return "hangingHandler" }
+
+func TestHandlerChain_ResolveInternal_HonorsContextTimeout(t *testing.T) {
+	chain := nbdns.NewHandlerChain()
+	h := &hangingHandler{block: make(chan struct{})}
+	defer close(h.block)
+
+	chain.AddHandler("example.com.", h, nbdns.PriorityUpstream)
+
+	r := new(dns.Msg)
+	r.SetQuestion("example.com.", dns.TypeA)
+
+	ctx, cancel := context.WithTimeout(context.Background(), 100*time.Millisecond)
+	defer cancel()
+
+	start := time.Now()
+	_, err := chain.ResolveInternal(ctx, r, nbdns.PriorityUpstream)
+	elapsed := time.Since(start)
+
+	assert.Error(t, err)
+	assert.ErrorIs(t, err, context.DeadlineExceeded)
+	assert.Less(t, elapsed, 500*time.Millisecond, "ResolveInternal must return shortly after ctx deadline")
+}
+
+func TestHandlerChain_HasRootHandlerAtOrBelow(t *testing.T) {
+	chain := nbdns.NewHandlerChain()
+	h := &answeringHandler{name: "h", ip: "10.0.0.1"}
+
+	assert.False(t, chain.HasRootHandlerAtOrBelow(nbdns.PriorityUpstream), "empty chain")
+
+	chain.AddHandler("example.com.", h, nbdns.PriorityUpstream)
+	assert.False(t, chain.HasRootHandlerAtOrBelow(nbdns.PriorityUpstream), "non-root handler does not count")
+
+	chain.AddHandler(".", h, nbdns.PriorityMgmtCache)
+	assert.False(t, chain.HasRootHandlerAtOrBelow(nbdns.PriorityUpstream), "root handler above threshold excluded")
+
+	chain.AddHandler(".", h, nbdns.PriorityDefault)
+	assert.True(t, chain.HasRootHandlerAtOrBelow(nbdns.PriorityUpstream), "root handler at PriorityDefault included")
+
+	chain.RemoveHandler(".", nbdns.PriorityDefault)
+	assert.False(t, chain.HasRootHandlerAtOrBelow(nbdns.PriorityUpstream))
+
+	// Primary nsgroup case: root handler lands at PriorityUpstream.
+	chain.AddHandler(".", h, nbdns.PriorityUpstream)
+	assert.True(t, chain.HasRootHandlerAtOrBelow(nbdns.PriorityUpstream), "root at PriorityUpstream included")
+	chain.RemoveHandler(".", nbdns.PriorityUpstream)
+
+	// Fallback case: original /etc/resolv.conf entries land at PriorityFallback.
+	chain.AddHandler(".", h, nbdns.PriorityFallback)
+	assert.True(t, chain.HasRootHandlerAtOrBelow(nbdns.PriorityUpstream), "root at PriorityFallback included")
+	chain.RemoveHandler(".", nbdns.PriorityFallback)
+	assert.False(t, chain.HasRootHandlerAtOrBelow(nbdns.PriorityUpstream))
+}

client/internal/dns/mgmt/mgmt.go

@@ -2,40 +2,79 @@ package mgmt
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net"
-	"net/netip"
 	"net/url"
+	"os"
 	"strings"
 	"sync"
+	"sync/atomic"
 	"time"
 
 	"github.com/miekg/dns"
 	log "github.com/sirupsen/logrus"
+	"golang.org/x/sync/singleflight"
 
 	dnsconfig "github.com/netbirdio/netbird/client/internal/dns/config"
+	"github.com/netbirdio/netbird/client/internal/dns/resutil"
 	"github.com/netbirdio/netbird/shared/management/domain"
 )
 
-const dnsTimeout = 5 * time.Second
+const (
+	dnsTimeout     = 5 * time.Second
+	defaultTTL     = 300 * time.Second
+	refreshBackoff = 30 * time.Second
 
-// Resolver caches critical NetBird infrastructure domains
+	// envMgmtCacheTTL overrides defaultTTL for integration/dev testing.
+	envMgmtCacheTTL = "NB_MGMT_CACHE_TTL"
+)
+
+// ChainResolver lets the cache refresh stale entries through the DNS handler
+// chain instead of net.DefaultResolver, avoiding loopback when NetBird is the
+// system resolver.
+type ChainResolver interface {
+	ResolveInternal(ctx context.Context, msg *dns.Msg, maxPriority int) (*dns.Msg, error)
+	HasRootHandlerAtOrBelow(maxPriority int) bool
+}
+
+// cachedRecord holds DNS records plus timestamps used for TTL refresh.
+// records and cachedAt are set at construction and treated as immutable;
+// lastFailedRefresh and consecFailures are mutable and must be accessed under
+// Resolver.mutex.
+type cachedRecord struct {
+	records           []dns.RR
+	cachedAt          time.Time
+	lastFailedRefresh time.Time
+	consecFailures    int
+}
+
+// Resolver caches critical NetBird infrastructure domains.
+// records, refreshing, mgmtDomain and serverDomains are all guarded by mutex.
 type Resolver struct {
-	records       map[dns.Question][]dns.RR
+	records       map[dns.Question]*cachedRecord
 	mgmtDomain    *domain.Domain
 	serverDomains *dnsconfig.ServerDomains
 	mutex         sync.RWMutex
-}
 
-type ipsResponse struct {
-	ips []netip.Addr
-	err error
+	chain            ChainResolver
+	chainMaxPriority int
+	refreshGroup     singleflight.Group
+
+	// refreshing tracks questions whose refresh is running via the OS
+	// fallback path. A ServeDNS hit for a question in this map indicates
+	// the OS resolver routed the recursive query back to us (loop). Only
+	// the OS path arms this so chain-path refreshes don't produce false
+	// positives. The atomic bool is CAS-flipped once per refresh to
+	// throttle the warning log.
+	refreshing map[dns.Question]*atomic.Bool
 }
 
 // NewResolver creates a new management domains cache resolver.
 func NewResolver() *Resolver {
 	return &Resolver{
-		records: make(map[dns.Question][]dns.RR),
+		records:    make(map[dns.Question]*cachedRecord),
+		refreshing: make(map[dns.Question]*atomic.Bool),
 	}
 }
 
@@ -44,7 +83,19 @@ func (m *Resolver) String() string {
 	return "MgmtCacheResolver"
 }
 
-// ServeDNS implements dns.Handler interface.
+// SetChainResolver wires the handler chain used to refresh stale cache entries.
+// maxPriority caps which handlers may answer refresh queries (typically
+// PriorityUpstream, so upstream/default/fallback handlers are consulted and
+// mgmt/route/local handlers are skipped).
+func (m *Resolver) SetChainResolver(chain ChainResolver, maxPriority int) {
+	m.mutex.Lock()
+	m.chain = chain
+	m.chainMaxPriority = maxPriority
+	m.mutex.Unlock()
+}
+
+// ServeDNS serves cached A/AAAA records. Stale entries are returned
+// immediately and refreshed asynchronously (stale-while-revalidate).
 func (m *Resolver) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
 	if len(r.Question) == 0 {
 		m.continueToNext(w, r)
@@ -60,7 +111,14 @@ func (m *Resolver) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
 	}
 
 	m.mutex.RLock()
-	records, found := m.records[question]
+	cached, found := m.records[question]
+	inflight := m.refreshing[question]
+	var shouldRefresh bool
+	if found {
+		stale := time.Since(cached.cachedAt) > cacheTTL()
+		inBackoff := !cached.lastFailedRefresh.IsZero() && time.Since(cached.lastFailedRefresh) < refreshBackoff
+		shouldRefresh = stale && !inBackoff
+	}
 	m.mutex.RUnlock()
 
 	if !found {
@@ -68,12 +126,23 @@ func (m *Resolver) ServeDNS(w dns.ResponseWriter, r *dns.Msg) {
 		return
 	}
 
+	if inflight != nil && inflight.CompareAndSwap(false, true) {
+		log.Warnf("mgmt cache: possible resolver loop for domain=%s: served stale while an OS-fallback refresh was inflight (if NetBird is the system resolver, the OS-path predicate is wrong)",
+			question.Name)
+	}
+
+	// Skip scheduling a refresh goroutine if one is already inflight for
+	// this question; singleflight would dedup anyway but skipping avoids
+	// a parked goroutine per stale hit under bursty load.
+	if shouldRefresh && inflight == nil {
+		m.scheduleRefresh(question)
+	}
+
 	resp := &dns.Msg{}
 	resp.SetReply(r)
 	resp.Authoritative = false
 	resp.RecursionAvailable = true
-
-	resp.Answer = append(resp.Answer, records...)
+	resp.Answer = append(resp.Answer, cached.records...)
 
 	log.Debugf("serving %d cached records for domain=%s", len(resp.Answer), question.Name)
 
@@ -98,101 +167,177 @@ func (m *Resolver) continueToNext(w dns.ResponseWriter, r *dns.Msg) {
 	}
 }
 
-// AddDomain manually adds a domain to cache by resolving it.
+// AddDomain resolves a domain and stores its A/AAAA records in the cache.
+// A family that resolves NODATA (nil err, zero records) evicts any stale
+// entry for that qtype.
 func (m *Resolver) AddDomain(ctx context.Context, d domain.Domain) error {
 	dnsName := strings.ToLower(dns.Fqdn(d.PunycodeString()))
 
 	ctx, cancel := context.WithTimeout(ctx, dnsTimeout)
 	defer cancel()
 
-	ips, err := lookupIPWithExtraTimeout(ctx, d)
-	if err != nil {
-		return err
+	aRecords, aaaaRecords, errA, errAAAA := m.lookupBoth(ctx, d, dnsName)
+
+	if errA != nil && errAAAA != nil {
+		return fmt.Errorf("resolve %s: %w", d.SafeString(), errors.Join(errA, errAAAA))
 	}
 
-	var aRecords, aaaaRecords []dns.RR
-	for _, ip := range ips {
-		if ip.Is4() {
-			rr := &dns.A{
-				Hdr: dns.RR_Header{
-					Name:   dnsName,
-					Rrtype: dns.TypeA,
-					Class:  dns.ClassINET,
-					Ttl:    300,
-				},
-				A: ip.AsSlice(),
-			}
-			aRecords = append(aRecords, rr)
-		} else if ip.Is6() {
-			rr := &dns.AAAA{
-				Hdr: dns.RR_Header{
-					Name:   dnsName,
-					Rrtype: dns.TypeAAAA,
-					Class:  dns.ClassINET,
-					Ttl:    300,
-				},
-				AAAA: ip.AsSlice(),
-			}
-			aaaaRecords = append(aaaaRecords, rr)
-		}
+	// Dual NODATA: don't wipe existing entries, let the caller retry.
+	if errA == nil && errAAAA == nil && len(aRecords) == 0 && len(aaaaRecords) == 0 {
+		return fmt.Errorf("resolve %s: no A/AAAA records", d.SafeString())
 	}
 
+	now := time.Now()
 	m.mutex.Lock()
+	defer m.mutex.Unlock()
 
-	if len(aRecords) > 0 {
-		aQuestion := dns.Question{
-			Name:   dnsName,
-			Qtype:  dns.TypeA,
-			Qclass: dns.ClassINET,
-		}
-		m.records[aQuestion] = aRecords
-	}
+	m.applyFamilyRecords(dnsName, dns.TypeA, aRecords, errA, now)
+	m.applyFamilyRecords(dnsName, dns.TypeAAAA, aaaaRecords, errAAAA, now)
 
-	if len(aaaaRecords) > 0 {
-		aaaaQuestion := dns.Question{
-			Name:   dnsName,
-			Qtype:  dns.TypeAAAA,
-			Qclass: dns.ClassINET,
-		}
-		m.records[aaaaQuestion] = aaaaRecords
-	}
-
-	m.mutex.Unlock()
-
-	log.Debugf("added domain=%s with %d A records and %d AAAA records",
+	log.Debugf("added/updated domain=%s with %d A records and %d AAAA records",
 		d.SafeString(), len(aRecords), len(aaaaRecords))
 
 	return nil
 }
 
-func lookupIPWithExtraTimeout(ctx context.Context, d domain.Domain) ([]netip.Addr, error) {
-	log.Infof("looking up IP for mgmt domain=%s", d.SafeString())
-	defer log.Infof("done looking up IP for mgmt domain=%s", d.SafeString())
-	resultChan := make(chan *ipsResponse, 1)
+// applyFamilyRecords writes records, evicts on NODATA, leaves the cache
+// untouched on error. Caller holds m.mutex.
+func (m *Resolver) applyFamilyRecords(dnsName string, qtype uint16, records []dns.RR, err error, now time.Time) {
+	q := dns.Question{Name: dnsName, Qtype: qtype, Qclass: dns.ClassINET}
+	switch {
+	case len(records) > 0:
+		m.records[q] = &cachedRecord{records: records, cachedAt: now}
+	case err == nil:
+		delete(m.records, q)
+	}
+}
 
-	go func() {
-		ips, err := net.DefaultResolver.LookupNetIP(ctx, "ip", d.PunycodeString())
-		resultChan <- &ipsResponse{
-			err: err,
-			ips: ips,
+// scheduleRefresh kicks off an async refresh. DoChan spawns one goroutine per
+// unique in-flight key; bursty stale hits share its channel.
+func (m *Resolver) scheduleRefresh(question dns.Question) {
+	key := question.Name + "|" + dns.TypeToString[question.Qtype]
+	_ = m.refreshGroup.DoChan(key, func() (any, error) {
+		return nil, m.refreshQuestion(question)
+	})
+}
+
+// refreshQuestion replaces the cached records on success, or marks the entry
+// failed (arming the backoff) on failure. While this runs, ServeDNS can detect
+// a resolver loop by spotting a query for this same question arriving on us.
+func (m *Resolver) refreshQuestion(question dns.Question) error {
+	ctx, cancel := context.WithTimeout(context.Background(), dnsTimeout)
+	defer cancel()
+
+	d, err := domain.FromString(strings.TrimSuffix(question.Name, "."))
+	if err != nil {
+		m.markRefreshFailed(question)
+		return fmt.Errorf("parse domain: %w", err)
+	}
+
+	records, err := m.lookupRecords(ctx, d, question)
+	if err != nil {
+		fails := m.markRefreshFailed(question)
+		logf := log.Warnf
+		if fails > 1 {
+			logf = log.Debugf
 		}
-	}()
-
-	var resp *ipsResponse
-
-	select {
-	case <-time.After(dnsTimeout + time.Millisecond*500):
-		log.Warnf("timed out waiting for IP for mgmt domain=%s", d.SafeString())
-		return nil, fmt.Errorf("timed out waiting for ips to be available for domain %s", d.SafeString())
-	case <-ctx.Done():
-		return nil, ctx.Err()
-	case resp = <-resultChan:
+		logf("refresh mgmt cache domain=%s type=%s: %v (consecutive failures=%d)",
+			d.SafeString(), dns.TypeToString[question.Qtype], err, fails)
+		return err
 	}
 
-	if resp.err != nil {
-		return nil, fmt.Errorf("resolve domain %s: %w", d.SafeString(), resp.err)
+	// NOERROR/NODATA: family gone upstream, evict so we stop serving stale.
+	if len(records) == 0 {
+		m.mutex.Lock()
+		delete(m.records, question)
+		m.mutex.Unlock()
+		log.Infof("removed mgmt cache domain=%s type=%s: no records returned",
+			d.SafeString(), dns.TypeToString[question.Qtype])
+		return nil
 	}
-	return resp.ips, nil
+
+	now := time.Now()
+	m.mutex.Lock()
+	if _, stillCached := m.records[question]; !stillCached {
+		m.mutex.Unlock()
+		log.Debugf("skipping refresh write for domain=%s type=%s: entry was removed during refresh",
+			d.SafeString(), dns.TypeToString[question.Qtype])
+		return nil
+	}
+	m.records[question] = &cachedRecord{records: records, cachedAt: now}
+	m.mutex.Unlock()
+
+	log.Infof("refreshed mgmt cache domain=%s type=%s",
+		d.SafeString(), dns.TypeToString[question.Qtype])
+	return nil
+}
+
+func (m *Resolver) markRefreshing(question dns.Question) {
+	m.mutex.Lock()
+	m.refreshing[question] = &atomic.Bool{}
+	m.mutex.Unlock()
+}
+
+func (m *Resolver) clearRefreshing(question dns.Question) {
+	m.mutex.Lock()
+	delete(m.refreshing, question)
+	m.mutex.Unlock()
+}
+
+// markRefreshFailed arms the backoff and returns the new consecutive-failure
+// count so callers can downgrade subsequent failure logs to debug.
+func (m *Resolver) markRefreshFailed(question dns.Question) int {
+	m.mutex.Lock()
+	defer m.mutex.Unlock()
+	c, ok := m.records[question]
+	if !ok {
+		return 0
+	}
+	c.lastFailedRefresh = time.Now()
+	c.consecFailures++
+	return c.consecFailures
+}
+
+// lookupBoth resolves A and AAAA via chain or OS. Per-family errors let
+// callers tell records, NODATA (nil err, no records), and failure apart.
+func (m *Resolver) lookupBoth(ctx context.Context, d domain.Domain, dnsName string) (aRecords, aaaaRecords []dns.RR, errA, errAAAA error) {
+	m.mutex.RLock()
+	chain := m.chain
+	maxPriority := m.chainMaxPriority
+	m.mutex.RUnlock()
+
+	if chain != nil && chain.HasRootHandlerAtOrBelow(maxPriority) {
+		aRecords, errA = lookupViaChain(ctx, chain, maxPriority, dnsName, dns.TypeA)
+		aaaaRecords, errAAAA = lookupViaChain(ctx, chain, maxPriority, dnsName, dns.TypeAAAA)
+		return
+	}
+
+	// TODO: drop once every supported OS registers a fallback resolver. Safe
+	// today: no root handler at priority ≤ PriorityUpstream means NetBird is
+	// not the system resolver, so net.DefaultResolver will not loop back.
+	aRecords, errA = osLookup(ctx, d, dnsName, dns.TypeA)
+	aaaaRecords, errAAAA = osLookup(ctx, d, dnsName, dns.TypeAAAA)
+	return
+}
+
+// lookupRecords resolves a single record type via chain or OS. The OS branch
+// arms the loop detector for the duration of its call so that ServeDNS can
+// spot the OS resolver routing the recursive query back to us.
+func (m *Resolver) lookupRecords(ctx context.Context, d domain.Domain, q dns.Question) ([]dns.RR, error) {
+	m.mutex.RLock()
+	chain := m.chain
+	maxPriority := m.chainMaxPriority
+	m.mutex.RUnlock()
+
+	if chain != nil && chain.HasRootHandlerAtOrBelow(maxPriority) {
+		return lookupViaChain(ctx, chain, maxPriority, q.Name, q.Qtype)
+	}
+
+	// TODO: drop once every supported OS registers a fallback resolver.
+	m.markRefreshing(q)
+	defer m.clearRefreshing(q)
+
+	return osLookup(ctx, d, q.Name, q.Qtype)
 }
 
 // PopulateFromConfig extracts and caches domains from the client configuration.
@@ -224,19 +369,8 @@ func (m *Resolver) RemoveDomain(d domain.Domain) error {
 	m.mutex.Lock()
 	defer m.mutex.Unlock()
 
-	aQuestion := dns.Question{
-		Name:   dnsName,
-		Qtype:  dns.TypeA,
-		Qclass: dns.ClassINET,
-	}
-	delete(m.records, aQuestion)
-
-	aaaaQuestion := dns.Question{
-		Name:   dnsName,
-		Qtype:  dns.TypeAAAA,
-		Qclass: dns.ClassINET,
-	}
-	delete(m.records, aaaaQuestion)
+	delete(m.records, dns.Question{Name: dnsName, Qtype: dns.TypeA, Qclass: dns.ClassINET})
+	delete(m.records, dns.Question{Name: dnsName, Qtype: dns.TypeAAAA, Qclass: dns.ClassINET})
 
 	log.Debugf("removed domain=%s from cache", d.SafeString())
 	return nil
@@ -394,3 +528,82 @@ func (m *Resolver) extractDomainsFromServerDomains(serverDomains dnsconfig.Serve
 
 	return domains
 }
+
+func cacheTTL() time.Duration {
+	if v := os.Getenv(envMgmtCacheTTL); v != "" {
+		if d, err := time.ParseDuration(v); err == nil && d > 0 {
+			return d
+		}
+	}
+	return defaultTTL
+}
+
+// lookupViaChain resolves via the handler chain and rewrites each RR to use
+// dnsName as owner and cacheTTL() as TTL, so CNAME-backed domains don't cache
+// target-owned records or upstream TTLs. NODATA returns (nil, nil).
+func lookupViaChain(ctx context.Context, chain ChainResolver, maxPriority int, dnsName string, qtype uint16) ([]dns.RR, error) {
+	msg := &dns.Msg{}
+	msg.SetQuestion(dnsName, qtype)
+	msg.RecursionDesired = true
+
+	resp, err := chain.ResolveInternal(ctx, msg, maxPriority)
+	if err != nil {
+		return nil, fmt.Errorf("chain resolve: %w", err)
+	}
+	if resp == nil {
+		return nil, fmt.Errorf("chain resolve returned nil response")
+	}
+	if resp.Rcode != dns.RcodeSuccess {
+		return nil, fmt.Errorf("chain resolve rcode=%s", dns.RcodeToString[resp.Rcode])
+	}
+
+	ttl := uint32(cacheTTL().Seconds())
+	var filtered []dns.RR
+	for _, rr := range resp.Answer {
+		if rr.Header().Class != dns.ClassINET {
+			continue
+		}
+		switch r := rr.(type) {
+		case *dns.A:
+			if qtype != dns.TypeA {
+				continue
+			}
+			filtered = append(filtered, &dns.A{
+				Hdr: dns.RR_Header{Name: dnsName, Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: ttl},
+				A:   append(net.IP(nil), r.A...),
+			})
+		case *dns.AAAA:
+			if qtype != dns.TypeAAAA {
+				continue
+			}
+			filtered = append(filtered, &dns.AAAA{
+				Hdr:  dns.RR_Header{Name: dnsName, Rrtype: dns.TypeAAAA, Class: dns.ClassINET, Ttl: ttl},
+				AAAA: append(net.IP(nil), r.AAAA...),
+			})
+		}
+	}
+	return filtered, nil
+}
+
+// osLookup resolves a single family via net.DefaultResolver using resutil,
+// which disambiguates NODATA from NXDOMAIN and Unmaps v4-mapped-v6. NODATA
+// returns (nil, nil).
+func osLookup(ctx context.Context, d domain.Domain, dnsName string, qtype uint16) ([]dns.RR, error) {
+	network := resutil.NetworkForQtype(qtype)
+	if network == "" {
+		return nil, fmt.Errorf("unsupported qtype %s", dns.TypeToString[qtype])
+	}
+
+	log.Infof("looking up IP for mgmt domain=%s type=%s", d.SafeString(), dns.TypeToString[qtype])
+	defer log.Infof("done looking up IP for mgmt domain=%s type=%s", d.SafeString(), dns.TypeToString[qtype])
+
+	result := resutil.LookupIP(ctx, net.DefaultResolver, network, d.PunycodeString(), qtype)
+	if result.Rcode == dns.RcodeSuccess {
+		return resutil.IPsToRRs(dnsName, result.IPs, uint32(cacheTTL().Seconds())), nil
+	}
+
+	if result.Err != nil {
+		return nil, fmt.Errorf("resolve %s type=%s: %w", d.SafeString(), dns.TypeToString[qtype], result.Err)
+	}
+	return nil, fmt.Errorf("resolve %s type=%s: rcode=%s", d.SafeString(), dns.TypeToString[qtype], dns.RcodeToString[result.Rcode])
+}

client/internal/dns/mgmt/mgmt_refresh_test.go

@@ -0,0 +1,359 @@
+package mgmt
+
+import (
+	"context"
+	"errors"
+	"net"
+	"sync"
+	"sync/atomic"
+	"testing"
+	"time"
+
+	"github.com/miekg/dns"
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+
+	"github.com/netbirdio/netbird/client/internal/dns/test"
+	"github.com/netbirdio/netbird/shared/management/domain"
+)
+
+type fakeChain struct {
+	mu       sync.Mutex
+	calls    map[string]int
+	answers  map[string][]dns.RR
+	err      error
+	hasRoot  bool
+	onLookup func()
+}
+
+func newFakeChain() *fakeChain {
+	return &fakeChain{
+		calls:   map[string]int{},
+		answers: map[string][]dns.RR{},
+		hasRoot: true,
+	}
+}
+
+func (f *fakeChain) HasRootHandlerAtOrBelow(maxPriority int) bool {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return f.hasRoot
+}
+
+func (f *fakeChain) ResolveInternal(ctx context.Context, msg *dns.Msg, maxPriority int) (*dns.Msg, error) {
+	f.mu.Lock()
+	q := msg.Question[0]
+	key := q.Name + "|" + dns.TypeToString[q.Qtype]
+	f.calls[key]++
+	answers := f.answers[key]
+	err := f.err
+	onLookup := f.onLookup
+	f.mu.Unlock()
+
+	if onLookup != nil {
+		onLookup()
+	}
+	if err != nil {
+		return nil, err
+	}
+	resp := &dns.Msg{}
+	resp.SetReply(msg)
+	resp.Answer = answers
+	return resp, nil
+}
+
+func (f *fakeChain) setAnswer(name string, qtype uint16, ip string) {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	key := name + "|" + dns.TypeToString[qtype]
+	hdr := dns.RR_Header{Name: name, Rrtype: qtype, Class: dns.ClassINET, Ttl: 60}
+	switch qtype {
+	case dns.TypeA:
+		f.answers[key] = []dns.RR{&dns.A{Hdr: hdr, A: net.ParseIP(ip).To4()}}
+	case dns.TypeAAAA:
+		f.answers[key] = []dns.RR{&dns.AAAA{Hdr: hdr, AAAA: net.ParseIP(ip).To16()}}
+	}
+}
+
+func (f *fakeChain) callCount(name string, qtype uint16) int {
+	f.mu.Lock()
+	defer f.mu.Unlock()
+	return f.calls[name+"|"+dns.TypeToString[qtype]]
+}
+
+// waitFor polls the predicate until it returns true or the deadline passes.
+func waitFor(t *testing.T, d time.Duration, fn func() bool) {
+	t.Helper()
+	deadline := time.Now().Add(d)
+	for time.Now().Before(deadline) {
+		if fn() {
+			return
+		}
+		time.Sleep(5 * time.Millisecond)
+	}
+	t.Fatalf("condition not met within %s", d)
+}
+
+func queryA(t *testing.T, r *Resolver, name string) *dns.Msg {
+	t.Helper()
+	msg := new(dns.Msg)
+	msg.SetQuestion(name, dns.TypeA)
+	w := &test.MockResponseWriter{}
+	r.ServeDNS(w, msg)
+	return w.GetLastResponse()
+}
+
+func firstA(t *testing.T, resp *dns.Msg) string {
+	t.Helper()
+	require.NotNil(t, resp)
+	require.Greater(t, len(resp.Answer), 0, "expected at least one answer")
+	a, ok := resp.Answer[0].(*dns.A)
+	require.True(t, ok, "expected A record")
+	return a.A.String()
+}
+
+func TestResolver_ServeFresh_NoRefresh(t *testing.T) {
+	r := NewResolver()
+	chain := newFakeChain()
+	chain.setAnswer("mgmt.example.com.", dns.TypeA, "10.0.0.2")
+	r.SetChainResolver(chain, 50)
+
+	r.records[dns.Question{Name: "mgmt.example.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET}] = &cachedRecord{
+		records: []dns.RR{&dns.A{
+			Hdr: dns.RR_Header{Name: "mgmt.example.com.", Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 60},
+			A:   net.ParseIP("10.0.0.1").To4(),
+		}},
+		cachedAt: time.Now(), // fresh
+	}
+
+	resp := queryA(t, r, "mgmt.example.com.")
+	assert.Equal(t, "10.0.0.1", firstA(t, resp))
+
+	time.Sleep(20 * time.Millisecond)
+	assert.Equal(t, 0, chain.callCount("mgmt.example.com.", dns.TypeA), "fresh entry must not trigger refresh")
+}
+
+func TestResolver_StaleTriggersAsyncRefresh(t *testing.T) {
+	r := NewResolver()
+	chain := newFakeChain()
+	chain.setAnswer("mgmt.example.com.", dns.TypeA, "10.0.0.2")
+	r.SetChainResolver(chain, 50)
+
+	q := dns.Question{Name: "mgmt.example.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET}
+	r.records[q] = &cachedRecord{
+		records: []dns.RR{&dns.A{
+			Hdr: dns.RR_Header{Name: q.Name, Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 60},
+			A:   net.ParseIP("10.0.0.1").To4(),
+		}},
+		cachedAt: time.Now().Add(-2 * defaultTTL), // stale
+	}
+
+	// First query: serves stale immediately.
+	resp := queryA(t, r, "mgmt.example.com.")
+	assert.Equal(t, "10.0.0.1", firstA(t, resp), "stale entry must be served while refresh runs")
+
+	waitFor(t, time.Second, func() bool {
+		return chain.callCount("mgmt.example.com.", dns.TypeA) >= 1
+	})
+
+	// Next query should now return the refreshed IP.
+	waitFor(t, time.Second, func() bool {
+		resp := queryA(t, r, "mgmt.example.com.")
+		return resp != nil && len(resp.Answer) > 0 && firstA(t, resp) == "10.0.0.2"
+	})
+}
+
+func TestResolver_ConcurrentStaleHitsCollapseRefresh(t *testing.T) {
+	r := NewResolver()
+	chain := newFakeChain()
+	chain.setAnswer("mgmt.example.com.", dns.TypeA, "10.0.0.2")
+
+	var inflight atomic.Int32
+	var maxInflight atomic.Int32
+	chain.onLookup = func() {
+		cur := inflight.Add(1)
+		defer inflight.Add(-1)
+		for {
+			prev := maxInflight.Load()
+			if cur <= prev || maxInflight.CompareAndSwap(prev, cur) {
+				break
+			}
+		}
+		time.Sleep(50 * time.Millisecond) // hold inflight long enough to collide
+	}
+
+	r.SetChainResolver(chain, 50)
+
+	q := dns.Question{Name: "mgmt.example.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET}
+	r.records[q] = &cachedRecord{
+		records: []dns.RR{&dns.A{
+			Hdr: dns.RR_Header{Name: q.Name, Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 60},
+			A:   net.ParseIP("10.0.0.1").To4(),
+		}},
+		cachedAt: time.Now().Add(-2 * defaultTTL),
+	}
+
+	var wg sync.WaitGroup
+	for i := 0; i < 50; i++ {
+		wg.Add(1)
+		go func() {
+			defer wg.Done()
+			queryA(t, r, "mgmt.example.com.")
+		}()
+	}
+	wg.Wait()
+
+	waitFor(t, 2*time.Second, func() bool {
+		return inflight.Load() == 0
+	})
+
+	calls := chain.callCount("mgmt.example.com.", dns.TypeA)
+	assert.LessOrEqual(t, calls, 2, "singleflight must collapse concurrent refreshes (got %d)", calls)
+	assert.Equal(t, int32(1), maxInflight.Load(), "only one refresh should run concurrently")
+}
+
+func TestResolver_RefreshFailureArmsBackoff(t *testing.T) {
+	r := NewResolver()
+	chain := newFakeChain()
+	chain.err = errors.New("boom")
+	r.SetChainResolver(chain, 50)
+
+	q := dns.Question{Name: "mgmt.example.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET}
+	r.records[q] = &cachedRecord{
+		records: []dns.RR{&dns.A{
+			Hdr: dns.RR_Header{Name: q.Name, Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 60},
+			A:   net.ParseIP("10.0.0.1").To4(),
+		}},
+		cachedAt: time.Now().Add(-2 * defaultTTL),
+	}
+
+	// First stale hit triggers a refresh attempt that fails.
+	resp := queryA(t, r, "mgmt.example.com.")
+	assert.Equal(t, "10.0.0.1", firstA(t, resp), "stale entry served while refresh fails")
+
+	waitFor(t, time.Second, func() bool {
+		return chain.callCount("mgmt.example.com.", dns.TypeA) == 1
+	})
+	waitFor(t, time.Second, func() bool {
+		r.mutex.RLock()
+		defer r.mutex.RUnlock()
+		c, ok := r.records[q]
+		return ok && !c.lastFailedRefresh.IsZero()
+	})
+
+	// Subsequent stale hits within backoff window should not schedule more refreshes.
+	for i := 0; i < 10; i++ {
+		queryA(t, r, "mgmt.example.com.")
+	}
+	time.Sleep(50 * time.Millisecond)
+	assert.Equal(t, 1, chain.callCount("mgmt.example.com.", dns.TypeA), "backoff must suppress further refreshes")
+}
+
+func TestResolver_NoRootHandler_SkipsChain(t *testing.T) {
+	r := NewResolver()
+	chain := newFakeChain()
+	chain.hasRoot = false
+	chain.setAnswer("mgmt.example.com.", dns.TypeA, "10.0.0.2")
+	r.SetChainResolver(chain, 50)
+
+	// With hasRoot=false the chain must not be consulted. Use a short
+	// deadline so the OS fallback returns quickly without waiting on a
+	// real network call in CI.
+	ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond)
+	defer cancel()
+	_, _, _, _ = r.lookupBoth(ctx, domain.Domain("mgmt.example.com"), "mgmt.example.com.")
+
+	assert.Equal(t, 0, chain.callCount("mgmt.example.com.", dns.TypeA),
+		"chain must not be used when no root handler is registered at the bound priority")
+}
+
+func TestResolver_ServeDuringRefreshSetsLoopFlag(t *testing.T) {
+	// ServeDNS being invoked for a question while a refresh for that question
+	// is inflight indicates a resolver loop (OS resolver sent the recursive
+	// query back to us). The inflightRefresh.loopLoggedOnce flag must be set.
+	r := NewResolver()
+
+	q := dns.Question{Name: "mgmt.example.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET}
+	r.records[q] = &cachedRecord{
+		records: []dns.RR{&dns.A{
+			Hdr: dns.RR_Header{Name: q.Name, Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 60},
+			A:   net.ParseIP("10.0.0.1").To4(),
+		}},
+		cachedAt: time.Now(),
+	}
+
+	// Simulate an inflight refresh.
+	r.markRefreshing(q)
+	defer r.clearRefreshing(q)
+
+	resp := queryA(t, r, "mgmt.example.com.")
+	assert.Equal(t, "10.0.0.1", firstA(t, resp), "stale entry must still be served to avoid breaking external queries")
+
+	r.mutex.RLock()
+	inflight := r.refreshing[q]
+	r.mutex.RUnlock()
+	require.NotNil(t, inflight)
+	assert.True(t, inflight.Load(), "loop flag must be set once a ServeDNS during refresh was observed")
+}
+
+func TestResolver_LoopFlagOnlyTrippedOncePerRefresh(t *testing.T) {
+	r := NewResolver()
+
+	q := dns.Question{Name: "mgmt.example.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET}
+	r.records[q] = &cachedRecord{
+		records: []dns.RR{&dns.A{
+			Hdr: dns.RR_Header{Name: q.Name, Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 60},
+			A:   net.ParseIP("10.0.0.1").To4(),
+		}},
+		cachedAt: time.Now(),
+	}
+
+	r.markRefreshing(q)
+	defer r.clearRefreshing(q)
+
+	// Multiple ServeDNS calls during the same refresh must not re-set the flag
+	// (CompareAndSwap from false -> true returns true only on the first call).
+	for range 5 {
+		queryA(t, r, "mgmt.example.com.")
+	}
+
+	r.mutex.RLock()
+	inflight := r.refreshing[q]
+	r.mutex.RUnlock()
+	assert.True(t, inflight.Load())
+}
+
+func TestResolver_NoLoopFlagWhenNotRefreshing(t *testing.T) {
+	r := NewResolver()
+
+	q := dns.Question{Name: "mgmt.example.com.", Qtype: dns.TypeA, Qclass: dns.ClassINET}
+	r.records[q] = &cachedRecord{
+		records: []dns.RR{&dns.A{
+			Hdr: dns.RR_Header{Name: q.Name, Rrtype: dns.TypeA, Class: dns.ClassINET, Ttl: 60},
+			A:   net.ParseIP("10.0.0.1").To4(),
+		}},
+		cachedAt: time.Now(),
+	}
+
+	queryA(t, r, "mgmt.example.com.")
+
+	r.mutex.RLock()
+	_, ok := r.refreshing[q]
+	r.mutex.RUnlock()
+	assert.False(t, ok, "no refresh inflight means no loop tracking")
+}
+
+func TestResolver_AddDomain_UsesChainWhenRootRegistered(t *testing.T) {
+	r := NewResolver()
+	chain := newFakeChain()
+	chain.setAnswer("mgmt.example.com.", dns.TypeA, "10.0.0.2")
+	chain.setAnswer("mgmt.example.com.", dns.TypeAAAA, "fd00::2")
+	r.SetChainResolver(chain, 50)
+
+	require.NoError(t, r.AddDomain(context.Background(), domain.Domain("mgmt.example.com")))
+
+	resp := queryA(t, r, "mgmt.example.com.")
+	assert.Equal(t, "10.0.0.2", firstA(t, resp))
+	assert.Equal(t, 1, chain.callCount("mgmt.example.com.", dns.TypeA))
+	assert.Equal(t, 1, chain.callCount("mgmt.example.com.", dns.TypeAAAA))
+}

client/internal/dns/server.go

@@ -212,6 +212,7 @@ func newDefaultServer(
 	ctx, stop := context.WithCancel(ctx)
 
 	mgmtCacheResolver := mgmt.NewResolver()
+	mgmtCacheResolver.SetChainResolver(handlerChain, PriorityUpstream)
 
 	defaultServer := &DefaultServer{
 		ctx:               ctx,

client/internal/engine.go

@@ -570,7 +570,7 @@ func (e *Engine) Start(netbirdConfig *mgmProto.NetbirdConfig, mgmtURL *url.URL)
 	e.connMgr.Start(e.ctx)
 
 	e.srWatcher = guard.NewSRWatcher(e.signal, e.relayManager, e.mobileDep.IFaceDiscover, iceCfg)
-	e.srWatcher.Start()
+	e.srWatcher.Start(peer.IsForceRelayed())
 
 	e.receiveSignalEvents()
 	e.receiveManagementEvents()

client/internal/peer/conn.go

@@ -185,17 +185,20 @@ func (conn *Conn) Open(engineCtx context.Context) error {
 
 	conn.workerRelay = NewWorkerRelay(conn.ctx, conn.Log, isController(conn.config), conn.config, conn, conn.relayManager)
 
-	relayIsSupportedLocally := conn.workerRelay.RelayIsSupportedLocally()
-	workerICE, err := NewWorkerICE(conn.ctx, conn.Log, conn.config, conn, conn.signaler, conn.iFaceDiscover, conn.statusRecorder, relayIsSupportedLocally)
-	if err != nil {
-		return err
+	forceRelay := IsForceRelayed()
+	if !forceRelay {
+		relayIsSupportedLocally := conn.workerRelay.RelayIsSupportedLocally()
+		workerICE, err := NewWorkerICE(conn.ctx, conn.Log, conn.config, conn, conn.signaler, conn.iFaceDiscover, conn.statusRecorder, relayIsSupportedLocally)
+		if err != nil {
+			return err
+		}
+		conn.workerICE = workerICE
 	}
-	conn.workerICE = workerICE
 
 	conn.handshaker = NewHandshaker(conn.Log, conn.config, conn.signaler, conn.workerICE, conn.workerRelay, conn.metricsStages)
 
 	conn.handshaker.AddRelayListener(conn.workerRelay.OnNewOffer)
-	if !isForceRelayed() {
+	if !forceRelay {
 		conn.handshaker.AddICEListener(conn.workerICE.OnNewOffer)
 	}
 
@@ -251,7 +254,9 @@ func (conn *Conn) Close(signalToRemote bool) {
 		conn.wgWatcherCancel()
 	}
 	conn.workerRelay.CloseConn()
-	conn.workerICE.Close()
+	if conn.workerICE != nil {
+		conn.workerICE.Close()
+	}
 
 	if conn.wgProxyRelay != nil {
 		err := conn.wgProxyRelay.CloseConn()
@@ -294,7 +299,9 @@ func (conn *Conn) OnRemoteAnswer(answer OfferAnswer) {
 // OnRemoteCandidate Handles ICE connection Candidate provided by the remote peer.
 func (conn *Conn) OnRemoteCandidate(candidate ice.Candidate, haRoutes route.HAMap) {
 	conn.dumpState.RemoteCandidate()
-	conn.workerICE.OnRemoteCandidate(candidate, haRoutes)
+	if conn.workerICE != nil {
+		conn.workerICE.OnRemoteCandidate(candidate, haRoutes)
+	}
 }
 
 // SetOnConnected sets a handler function to be triggered by Conn when a new connection to a remote peer established
@@ -712,33 +719,35 @@ func (conn *Conn) evalStatus() ConnStatus {
 	return StatusConnecting
 }
 
-func (conn *Conn) isConnectedOnAllWay() (connected bool) {
-	// would be better to protect this with a mutex, but it could cause deadlock with Close function
-
+// isConnectedOnAllWay evaluates the overall connection status based on ICE and Relay transports.
+//
+// The result is a tri-state:
+//   - ConnStatusConnected:          all available transports are up
+//   - ConnStatusPartiallyConnected: relay is up but ICE is still pending/reconnecting
+//   - ConnStatusDisconnected:       no working transport
+func (conn *Conn) isConnectedOnAllWay() (status guard.ConnStatus) {
 	defer func() {
-		if !connected {
+		if status == guard.ConnStatusDisconnected {
 			conn.logTraceConnState()
 		}
 	}()
 
-	// For JS platform: only relay connection is supported
-	if runtime.GOOS == "js" {
-		return conn.statusRelay.Get() == worker.StatusConnected
+	iceWorkerCreated := conn.workerICE != nil
+
+	var iceInProgress bool
+	if iceWorkerCreated {
+		iceInProgress = conn.workerICE.InProgress()
 	}
 
-	// For non-JS platforms: check ICE connection status
-	if conn.statusICE.Get() == worker.StatusDisconnected && !conn.workerICE.InProgress() {
-		return false
-	}
-
-	// If relay is supported with peer, it must also be connected
-	if conn.workerRelay.IsRelayConnectionSupportedWithPeer() {
-		if conn.statusRelay.Get() == worker.StatusDisconnected {
-			return false
-		}
-	}
-
-	return true
+	return evalConnStatus(connStatusInputs{
+		forceRelay:          IsForceRelayed(),
+		peerUsesRelay:       conn.workerRelay.IsRelayConnectionSupportedWithPeer(),
+		relayConnected:      conn.statusRelay.Get() == worker.StatusConnected,
+		remoteSupportsICE:   conn.handshaker.RemoteICESupported(),
+		iceWorkerCreated:    iceWorkerCreated,
+		iceStatusConnecting: conn.statusICE.Get() != worker.StatusDisconnected,
+		iceInProgress:       iceInProgress,
+	})
 }
 
 func (conn *Conn) enableWgWatcherIfNeeded(enabledTime time.Time) {
@@ -926,3 +935,43 @@ func isController(config ConnConfig) bool {
 func isRosenpassEnabled(remoteRosenpassPubKey []byte) bool {
 	return remoteRosenpassPubKey != nil
 }
+
+func evalConnStatus(in connStatusInputs) guard.ConnStatus {
+	// "Relay up and needed" — the peer uses relay and the transport is connected.
+	relayUsedAndUp := in.peerUsesRelay && in.relayConnected
+
+	// Force-relay mode: ICE never runs. Relay is the only transport and must be up.
+	if in.forceRelay {
+		return boolToConnStatus(relayUsedAndUp)
+	}
+
+	// Remote peer doesn't support ICE, or we haven't created the worker yet:
+	// relay is the only possible transport.
+	if !in.remoteSupportsICE || !in.iceWorkerCreated {
+		return boolToConnStatus(relayUsedAndUp)
+	}
+
+	// ICE counts as "up" when the status is anything other than Disconnected, OR
+	// when a negotiation is currently in progress (so we don't spam offers while one is in flight).
+	iceUp := in.iceStatusConnecting || in.iceInProgress
+
+	// Relay side is acceptable if the peer doesn't rely on relay, or relay is connected.
+	relayOK := !in.peerUsesRelay || in.relayConnected
+
+	switch {
+	case iceUp && relayOK:
+		return guard.ConnStatusConnected
+	case relayUsedAndUp:
+		// Relay is up but ICE is down — partially connected.
+		return guard.ConnStatusPartiallyConnected
+	default:
+		return guard.ConnStatusDisconnected
+	}
+}
+
+func boolToConnStatus(connected bool) guard.ConnStatus {
+	if connected {
+		return guard.ConnStatusConnected
+	}
+	return guard.ConnStatusDisconnected
+}

client/internal/peer/conn_status.go

@@ -13,6 +13,20 @@ const (
 	StatusConnected
 )
 
+// connStatusInputs is the primitive-valued snapshot of the state that drives the
+// tri-state connection classification. Extracted so the decision logic can be unit-tested
+// without constructing full Worker/Handshaker objects.
+type connStatusInputs struct {
+	forceRelay          bool // NB_FORCE_RELAY or JS/WASM
+	peerUsesRelay       bool // remote peer advertises relay support AND local has relay
+	relayConnected      bool // statusRelay reports Connected (independent of whether peer uses relay)
+	remoteSupportsICE   bool // remote peer sent ICE credentials
+	iceWorkerCreated    bool // local WorkerICE exists (false in force-relay mode)
+	iceStatusConnecting bool // statusICE is anything other than Disconnected
+	iceInProgress       bool // a negotiation is currently in flight
+}
+
+
 // ConnStatus describe the status of a peer's connection
 type ConnStatus int32
 

client/internal/peer/conn_status_eval_test.go

@@ -0,0 +1,201 @@
+package peer
+
+import (
+	"testing"
+
+	"github.com/netbirdio/netbird/client/internal/peer/guard"
+)
+
+func TestEvalConnStatus_ForceRelay(t *testing.T) {
+	tests := []struct {
+		name string
+		in   connStatusInputs
+		want guard.ConnStatus
+	}{
+		{
+			name: "force relay, peer uses relay, relay up",
+			in: connStatusInputs{
+				forceRelay:     true,
+				peerUsesRelay:  true,
+				relayConnected: true,
+			},
+			want: guard.ConnStatusConnected,
+		},
+		{
+			name: "force relay, peer uses relay, relay down",
+			in: connStatusInputs{
+				forceRelay:     true,
+				peerUsesRelay:  true,
+				relayConnected: false,
+			},
+			want: guard.ConnStatusDisconnected,
+		},
+		{
+			name: "force relay, peer does NOT use relay - disconnected forever",
+			in: connStatusInputs{
+				forceRelay:     true,
+				peerUsesRelay:  false,
+				relayConnected: true,
+			},
+			want: guard.ConnStatusDisconnected,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := evalConnStatus(tc.in); got != tc.want {
+				t.Fatalf("evalConnStatus = %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestEvalConnStatus_ICEUnavailable(t *testing.T) {
+	tests := []struct {
+		name string
+		in   connStatusInputs
+		want guard.ConnStatus
+	}{
+		{
+			name: "remote does not support ICE, peer uses relay, relay up",
+			in: connStatusInputs{
+				peerUsesRelay:     true,
+				relayConnected:    true,
+				remoteSupportsICE: false,
+				iceWorkerCreated:  true,
+			},
+			want: guard.ConnStatusConnected,
+		},
+		{
+			name: "remote does not support ICE, peer uses relay, relay down",
+			in: connStatusInputs{
+				peerUsesRelay:     true,
+				relayConnected:    false,
+				remoteSupportsICE: false,
+				iceWorkerCreated:  true,
+			},
+			want: guard.ConnStatusDisconnected,
+		},
+		{
+			name: "ICE worker not yet created, relay up",
+			in: connStatusInputs{
+				peerUsesRelay:     true,
+				relayConnected:    true,
+				remoteSupportsICE: true,
+				iceWorkerCreated:  false,
+			},
+			want: guard.ConnStatusConnected,
+		},
+		{
+			name: "remote does not support ICE, peer does not use relay",
+			in: connStatusInputs{
+				peerUsesRelay:     false,
+				relayConnected:    false,
+				remoteSupportsICE: false,
+				iceWorkerCreated:  true,
+			},
+			want: guard.ConnStatusDisconnected,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := evalConnStatus(tc.in); got != tc.want {
+				t.Fatalf("evalConnStatus = %v, want %v", got, tc.want)
+			}
+		})
+	}
+}
+
+func TestEvalConnStatus_FullyAvailable(t *testing.T) {
+	base := connStatusInputs{
+		remoteSupportsICE: true,
+		iceWorkerCreated:  true,
+	}
+
+	tests := []struct {
+		name    string
+		mutator func(*connStatusInputs)
+		want    guard.ConnStatus
+	}{
+		{
+			name: "ICE connected, relay connected, peer uses relay",
+			mutator: func(in *connStatusInputs) {
+				in.peerUsesRelay = true
+				in.relayConnected = true
+				in.iceStatusConnecting = true
+			},
+			want: guard.ConnStatusConnected,
+		},
+		{
+			name: "ICE connected, peer does NOT use relay",
+			mutator: func(in *connStatusInputs) {
+				in.peerUsesRelay = false
+				in.relayConnected = false
+				in.iceStatusConnecting = true
+			},
+			want: guard.ConnStatusConnected,
+		},
+		{
+			name: "ICE InProgress only, peer does NOT use relay",
+			mutator: func(in *connStatusInputs) {
+				in.peerUsesRelay = false
+				in.iceStatusConnecting = false
+				in.iceInProgress = true
+			},
+			want: guard.ConnStatusConnected,
+		},
+		{
+			name: "ICE down, relay up, peer uses relay -> partial",
+			mutator: func(in *connStatusInputs) {
+				in.peerUsesRelay = true
+				in.relayConnected = true
+				in.iceStatusConnecting = false
+				in.iceInProgress = false
+			},
+			want: guard.ConnStatusPartiallyConnected,
+		},
+		{
+			name: "ICE down, peer does NOT use relay -> disconnected",
+			mutator: func(in *connStatusInputs) {
+				in.peerUsesRelay = false
+				in.relayConnected = false
+				in.iceStatusConnecting = false
+				in.iceInProgress = false
+			},
+			want: guard.ConnStatusDisconnected,
+		},
+		{
+			name: "ICE up, peer uses relay but relay down -> partial (relay required, ICE ignored)",
+			mutator: func(in *connStatusInputs) {
+				in.peerUsesRelay = true
+				in.relayConnected = false
+				in.iceStatusConnecting = true
+			},
+			// relayOK = false (peer uses relay but it's down), iceUp = true
+			// first switch arm fails (relayOK false), relayUsedAndUp = false (relay down),
+			// falls into default: Disconnected.
+			want: guard.ConnStatusDisconnected,
+		},
+		{
+			name: "ICE down, relay up but peer does not use relay -> disconnected",
+			mutator: func(in *connStatusInputs) {
+				in.peerUsesRelay = false
+				in.relayConnected = true // not actually used since peer doesn't rely on it
+				in.iceStatusConnecting = false
+				in.iceInProgress = false
+			},
+			want: guard.ConnStatusDisconnected,
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			in := base
+			tc.mutator(&in)
+			if got := evalConnStatus(in); got != tc.want {
+				t.Fatalf("evalConnStatus = %v, want %v (inputs: %+v)", got, tc.want, in)
+			}
+		})
+	}
+}

client/internal/peer/env.go

@@ -10,7 +10,7 @@ const (
 	EnvKeyNBForceRelay = "NB_FORCE_RELAY"
 )
 
-func isForceRelayed() bool {
+func IsForceRelayed() bool {
 	if runtime.GOOS == "js" {
 		return true
 	}

client/internal/peer/guard/guard.go

@@ -8,7 +8,19 @@ import (
 	log "github.com/sirupsen/logrus"
 )
 
-type isConnectedFunc func() bool
+// ConnStatus represents the connection state as seen by the guard.
+type ConnStatus int
+
+const (
+	// ConnStatusDisconnected means neither ICE nor Relay is connected.
+	ConnStatusDisconnected ConnStatus = iota
+	// ConnStatusPartiallyConnected means Relay is connected but ICE is not.
+	ConnStatusPartiallyConnected
+	// ConnStatusConnected means all required connections are established.
+	ConnStatusConnected
+)
+
+type connStatusFunc func() ConnStatus
 
 // Guard is responsible for the reconnection logic.
 // It will trigger to send an offer to the peer then has connection issues.
@@ -20,14 +32,14 @@ type isConnectedFunc func() bool
 // - ICE candidate changes
 type Guard struct {
 	log                     *log.Entry
-	isConnectedOnAllWay     isConnectedFunc
+	isConnectedOnAllWay     connStatusFunc
 	timeout                 time.Duration
 	srWatcher               *SRWatcher
 	relayedConnDisconnected chan struct{}
 	iCEConnDisconnected     chan struct{}
 }
 
-func NewGuard(log *log.Entry, isConnectedFn isConnectedFunc, timeout time.Duration, srWatcher *SRWatcher) *Guard {
+func NewGuard(log *log.Entry, isConnectedFn connStatusFunc, timeout time.Duration, srWatcher *SRWatcher) *Guard {
 	return &Guard{
 		log:                     log,
 		isConnectedOnAllWay:     isConnectedFn,
@@ -57,8 +69,17 @@ func (g *Guard) SetICEConnDisconnected() {
 	}
 }
 
-// reconnectLoopWithRetry periodically check the connection status.
-// Try to send offer while the P2P is not established or while the Relay is not connected if is it supported
+// reconnectLoopWithRetry periodically checks the connection status and sends offers to re-establish connectivity.
+//
+// Behavior depends on the connection state reported by isConnectedOnAllWay:
+//   - Connected: no action, the peer is fully reachable.
+//   - Disconnected (neither ICE nor Relay): retries aggressively with exponential backoff (800ms doubling
+//     up to timeout), never gives up. This ensures rapid recovery when the peer has no connectivity at all.
+//   - PartiallyConnected (Relay up, ICE not): retries up to 3 times with exponential backoff, then switches
+//     to one attempt per hour. This limits signaling traffic when relay already provides connectivity.
+//
+// External events (relay/ICE disconnect, signal/relay reconnect, candidate changes) reset the retry
+// counter and backoff ticker, giving ICE a fresh chance after network conditions change.
 func (g *Guard) reconnectLoopWithRetry(ctx context.Context, callback func()) {
 	srReconnectedChan := g.srWatcher.NewListener()
 	defer g.srWatcher.RemoveListener(srReconnectedChan)
@@ -68,36 +89,47 @@ func (g *Guard) reconnectLoopWithRetry(ctx context.Context, callback func()) {
 
 	tickerChannel := ticker.C
 
+	iceState := &iceRetryState{log: g.log}
+	defer iceState.reset()
+
 	for {
 		select {
-		case t := <-tickerChannel:
-			if t.IsZero() {
-				g.log.Infof("retry timed out, stop periodic offer sending")
-				// after backoff timeout the ticker.C will be closed. We need to a dummy channel to avoid loop
-				tickerChannel = make(<-chan time.Time)
-				continue
+		case <-tickerChannel:
+			switch g.isConnectedOnAllWay() {
+			case ConnStatusConnected:
+				// all good, nothing to do
+			case ConnStatusDisconnected:
+				callback()
+			case ConnStatusPartiallyConnected:
+				if iceState.shouldRetry() {
+					callback()
+				} else {
+					iceState.enterHourlyMode()
+					ticker.Stop()
+					tickerChannel = iceState.hourlyC()
+				}
 			}
 
-			if !g.isConnectedOnAllWay() {
-				callback()
-			}
 		case <-g.relayedConnDisconnected:
 			g.log.Debugf("Relay connection changed, reset reconnection ticker")
 			ticker.Stop()
-			ticker = g.prepareExponentTicker(ctx)
+			ticker = g.newReconnectTicker(ctx)
 			tickerChannel = ticker.C
+			iceState.reset()
 
 		case <-g.iCEConnDisconnected:
 			g.log.Debugf("ICE connection changed, reset reconnection ticker")
 			ticker.Stop()
-			ticker = g.prepareExponentTicker(ctx)
+			ticker = g.newReconnectTicker(ctx)
 			tickerChannel = ticker.C
+			iceState.reset()
 
 		case <-srReconnectedChan:
 			g.log.Debugf("has network changes, reset reconnection ticker")
 			ticker.Stop()
-			ticker = g.prepareExponentTicker(ctx)
+			ticker = g.newReconnectTicker(ctx)
 			tickerChannel = ticker.C
+			iceState.reset()
 
 		case <-ctx.Done():
 			g.log.Debugf("context is done, stop reconnect loop")
@@ -120,7 +152,7 @@ func (g *Guard) initialTicker(ctx context.Context) *backoff.Ticker {
 	return backoff.NewTicker(bo)
 }
 
-func (g *Guard) prepareExponentTicker(ctx context.Context) *backoff.Ticker {
+func (g *Guard) newReconnectTicker(ctx context.Context) *backoff.Ticker {
 	bo := backoff.WithContext(&backoff.ExponentialBackOff{
 		InitialInterval:     800 * time.Millisecond,
 		RandomizationFactor: 0.1,

client/internal/peer/guard/ice_retry_state.go

@@ -0,0 +1,61 @@
+package guard
+
+import (
+	"time"
+
+	log "github.com/sirupsen/logrus"
+)
+
+const (
+	// maxICERetries is the maximum number of ICE offer attempts when relay is connected
+	maxICERetries = 3
+	// iceRetryInterval is the periodic retry interval after ICE retries are exhausted
+	iceRetryInterval = 1 * time.Hour
+)
+
+// iceRetryState tracks the limited ICE retry attempts when relay is already connected.
+// After maxICERetries attempts it switches to a periodic hourly retry.
+type iceRetryState struct {
+	log     *log.Entry
+	retries int
+	hourly  *time.Ticker
+}
+
+func (s *iceRetryState) reset() {
+	s.retries = 0
+	if s.hourly != nil {
+		s.hourly.Stop()
+		s.hourly = nil
+	}
+}
+
+// shouldRetry reports whether the caller should send another ICE offer on this tick.
+// Returns false when the per-cycle retry budget is exhausted and the caller must switch
+// to the hourly ticker via enterHourlyMode + hourlyC.
+func (s *iceRetryState) shouldRetry() bool {
+	if s.hourly != nil {
+		s.log.Debugf("hourly ICE retry attempt")
+		return true
+	}
+
+	s.retries++
+	if s.retries <= maxICERetries {
+		s.log.Debugf("ICE retry attempt %d/%d", s.retries, maxICERetries)
+		return true
+	}
+
+	return false
+}
+
+// enterHourlyMode starts the hourly retry ticker. Must be called after shouldRetry returns false.
+func (s *iceRetryState) enterHourlyMode() {
+	s.log.Infof("ICE retries exhausted (%d/%d), switching to hourly retry", maxICERetries, maxICERetries)
+	s.hourly = time.NewTicker(iceRetryInterval)
+}
+
+func (s *iceRetryState) hourlyC() <-chan time.Time {
+	if s.hourly == nil {
+		return nil
+	}
+	return s.hourly.C
+}

client/internal/peer/guard/ice_retry_state_test.go

@@ -0,0 +1,103 @@
+package guard
+
+import (
+	"testing"
+
+	log "github.com/sirupsen/logrus"
+)
+
+func newTestRetryState() *iceRetryState {
+	return &iceRetryState{log: log.NewEntry(log.StandardLogger())}
+}
+
+func TestICERetryState_AllowsInitialBudget(t *testing.T) {
+	s := newTestRetryState()
+
+	for i := 1; i <= maxICERetries; i++ {
+		if !s.shouldRetry() {
+			t.Fatalf("shouldRetry returned false on attempt %d, want true (budget = %d)", i, maxICERetries)
+		}
+	}
+}
+
+func TestICERetryState_ExhaustsAfterBudget(t *testing.T) {
+	s := newTestRetryState()
+
+	for i := 0; i < maxICERetries; i++ {
+		_ = s.shouldRetry()
+	}
+
+	if s.shouldRetry() {
+		t.Fatalf("shouldRetry returned true after budget exhausted, want false")
+	}
+}
+
+func TestICERetryState_HourlyCNilBeforeEnterHourlyMode(t *testing.T) {
+	s := newTestRetryState()
+
+	if s.hourlyC() != nil {
+		t.Fatalf("hourlyC returned non-nil channel before enterHourlyMode")
+	}
+}
+
+func TestICERetryState_EnterHourlyModeArmsTicker(t *testing.T) {
+	s := newTestRetryState()
+	for i := 0; i < maxICERetries+1; i++ {
+		_ = s.shouldRetry()
+	}
+
+	s.enterHourlyMode()
+	defer s.reset()
+
+	if s.hourlyC() == nil {
+		t.Fatalf("hourlyC returned nil after enterHourlyMode")
+	}
+}
+
+func TestICERetryState_ShouldRetryTrueInHourlyMode(t *testing.T) {
+	s := newTestRetryState()
+	s.enterHourlyMode()
+	defer s.reset()
+
+	if !s.shouldRetry() {
+		t.Fatalf("shouldRetry returned false in hourly mode, want true")
+	}
+
+	// Subsequent calls also return true — we keep retrying on each hourly tick.
+	if !s.shouldRetry() {
+		t.Fatalf("second shouldRetry returned false in hourly mode, want true")
+	}
+}
+
+func TestICERetryState_ResetRestoresBudget(t *testing.T) {
+	s := newTestRetryState()
+	for i := 0; i < maxICERetries+1; i++ {
+		_ = s.shouldRetry()
+	}
+	s.enterHourlyMode()
+
+	s.reset()
+
+	if s.hourlyC() != nil {
+		t.Fatalf("hourlyC returned non-nil channel after reset")
+	}
+	if s.retries != 0 {
+		t.Fatalf("retries = %d after reset, want 0", s.retries)
+	}
+
+	for i := 1; i <= maxICERetries; i++ {
+		if !s.shouldRetry() {
+			t.Fatalf("shouldRetry returned false on attempt %d after reset, want true", i)
+		}
+	}
+}
+
+func TestICERetryState_ResetIsIdempotent(t *testing.T) {
+	s := newTestRetryState()
+	s.reset()
+	s.reset() // second call must not panic or re-stop a nil ticker
+
+	if s.hourlyC() != nil {
+		t.Fatalf("hourlyC non-nil after double reset")
+	}
+}

client/internal/peer/guard/sr_watcher.go

@@ -39,7 +39,7 @@ func NewSRWatcher(signalClient chNotifier, relayManager chNotifier, iFaceDiscove
 	return srw
 }
 
-func (w *SRWatcher) Start() {
+func (w *SRWatcher) Start(disableICEMonitor bool) {
 	w.mu.Lock()
 	defer w.mu.Unlock()
 
@@ -50,8 +50,10 @@ func (w *SRWatcher) Start() {
 	ctx, cancel := context.WithCancel(context.Background())
 	w.cancelIceMonitor = cancel
 
-	iceMonitor := NewICEMonitor(w.iFaceDiscover, w.iceConfig, GetICEMonitorPeriod())
-	go iceMonitor.Start(ctx, w.onICEChanged)
+	if !disableICEMonitor {
+		iceMonitor := NewICEMonitor(w.iFaceDiscover, w.iceConfig, GetICEMonitorPeriod())
+		go iceMonitor.Start(ctx, w.onICEChanged)
+	}
 	w.signalClient.SetOnReconnectedListener(w.onReconnected)
 	w.relayManager.SetOnReconnectedListener(w.onReconnected)
 

client/internal/peer/handshaker.go

@@ -4,6 +4,7 @@ import (
 	"context"
 	"errors"
 	"sync"
+	"sync/atomic"
 
 	log "github.com/sirupsen/logrus"
 
@@ -43,6 +44,10 @@ type OfferAnswer struct {
 	SessionID *ICESessionID
 }
 
+func (o *OfferAnswer) hasICECredentials() bool {
+	return o.IceCredentials.UFrag != "" && o.IceCredentials.Pwd != ""
+}
+
 type Handshaker struct {
 	mu            sync.Mutex
 	log           *log.Entry
@@ -59,6 +64,10 @@ type Handshaker struct {
 	relayListener *AsyncOfferListener
 	iceListener   func(remoteOfferAnswer *OfferAnswer)
 
+	// remoteICESupported tracks whether the remote peer includes ICE credentials in its offers/answers.
+	// When false, the local side skips ICE listener dispatch and suppresses ICE credentials in responses.
+	remoteICESupported atomic.Bool
+
 	// remoteOffersCh is a channel used to wait for remote credentials to proceed with the connection
 	remoteOffersCh chan OfferAnswer
 	// remoteAnswerCh is a channel used to wait for remote credentials answer (confirmation of our offer) to proceed with the connection
@@ -66,7 +75,7 @@ type Handshaker struct {
 }
 
 func NewHandshaker(log *log.Entry, config ConnConfig, signaler *Signaler, ice *WorkerICE, relay *WorkerRelay, metricsStages *MetricsStages) *Handshaker {
-	return &Handshaker{
+	h := &Handshaker{
 		log:            log,
 		config:         config,
 		signaler:       signaler,
@@ -76,6 +85,13 @@ func NewHandshaker(log *log.Entry, config ConnConfig, signaler *Signaler, ice *W
 		remoteOffersCh: make(chan OfferAnswer),
 		remoteAnswerCh: make(chan OfferAnswer),
 	}
+	// assume remote supports ICE until we learn otherwise from received offers
+	h.remoteICESupported.Store(ice != nil)
+	return h
+}
+
+func (h *Handshaker) RemoteICESupported() bool {
+	return h.remoteICESupported.Load()
 }
 
 func (h *Handshaker) AddRelayListener(offer func(remoteOfferAnswer *OfferAnswer)) {
@@ -90,18 +106,20 @@ func (h *Handshaker) Listen(ctx context.Context) {
 	for {
 		select {
 		case remoteOfferAnswer := <-h.remoteOffersCh:
-			h.log.Infof("received offer, running version %s, remote WireGuard listen port %d, session id: %s", remoteOfferAnswer.Version, remoteOfferAnswer.WgListenPort, remoteOfferAnswer.SessionIDString())
+			h.log.Infof("received offer, running version %s, remote WireGuard listen port %d, session id: %s, remote ICE supported: %t", remoteOfferAnswer.Version, remoteOfferAnswer.WgListenPort, remoteOfferAnswer.SessionIDString(), remoteOfferAnswer.hasICECredentials())
 
 			// Record signaling received for reconnection attempts
 			if h.metricsStages != nil {
 				h.metricsStages.RecordSignalingReceived()
 			}
 
+			h.updateRemoteICEState(&remoteOfferAnswer)
+
 			if h.relayListener != nil {
 				h.relayListener.Notify(&remoteOfferAnswer)
 			}
 
-			if h.iceListener != nil {
+			if h.iceListener != nil && h.RemoteICESupported() {
 				h.iceListener(&remoteOfferAnswer)
 			}
 
@@ -110,18 +128,20 @@ func (h *Handshaker) Listen(ctx context.Context) {
 				continue
 			}
 		case remoteOfferAnswer := <-h.remoteAnswerCh:
-			h.log.Infof("received answer, running version %s, remote WireGuard listen port %d, session id: %s", remoteOfferAnswer.Version, remoteOfferAnswer.WgListenPort, remoteOfferAnswer.SessionIDString())
+			h.log.Infof("received answer, running version %s, remote WireGuard listen port %d, session id: %s, remote ICE supported: %t", remoteOfferAnswer.Version, remoteOfferAnswer.WgListenPort, remoteOfferAnswer.SessionIDString(), remoteOfferAnswer.hasICECredentials())
 
 			// Record signaling received for reconnection attempts
 			if h.metricsStages != nil {
 				h.metricsStages.RecordSignalingReceived()
 			}
 
+			h.updateRemoteICEState(&remoteOfferAnswer)
+
 			if h.relayListener != nil {
 				h.relayListener.Notify(&remoteOfferAnswer)
 			}
 
-			if h.iceListener != nil {
+			if h.iceListener != nil && h.RemoteICESupported() {
 				h.iceListener(&remoteOfferAnswer)
 			}
 		case <-ctx.Done():
@@ -183,15 +203,18 @@ func (h *Handshaker) sendAnswer() error {
 }
 
 func (h *Handshaker) buildOfferAnswer() OfferAnswer {
-	uFrag, pwd := h.ice.GetLocalUserCredentials()
-	sid := h.ice.SessionID()
 	answer := OfferAnswer{
-		IceCredentials:  IceCredentials{uFrag, pwd},
 		WgListenPort:    h.config.LocalWgPort,
 		Version:         version.NetbirdVersion(),
 		RosenpassPubKey: h.config.RosenpassConfig.PubKey,
 		RosenpassAddr:   h.config.RosenpassConfig.Addr,
-		SessionID:       &sid,
+	}
+
+	if h.ice != nil && h.RemoteICESupported() {
+		uFrag, pwd := h.ice.GetLocalUserCredentials()
+		sid := h.ice.SessionID()
+		answer.IceCredentials = IceCredentials{uFrag, pwd}
+		answer.SessionID = &sid
 	}
 
 	if addr, err := h.relay.RelayInstanceAddress(); err == nil {
@@ -200,3 +223,18 @@ func (h *Handshaker) buildOfferAnswer() OfferAnswer {
 
 	return answer
 }
+
+func (h *Handshaker) updateRemoteICEState(offer *OfferAnswer) {
+	hasICE := offer.hasICECredentials()
+	prev := h.remoteICESupported.Swap(hasICE)
+	if prev != hasICE {
+		if hasICE {
+			h.log.Infof("remote peer started sending ICE credentials")
+		} else {
+			h.log.Infof("remote peer stopped sending ICE credentials")
+			if h.ice != nil {
+				h.ice.Close()
+			}
+		}
+	}
+}

client/internal/peer/signaler.go

@@ -46,9 +46,13 @@ func (s *Signaler) Ready() bool {
 
 // SignalOfferAnswer signals either an offer or an answer to remote peer
 func (s *Signaler) signalOfferAnswer(offerAnswer OfferAnswer, remoteKey string, bodyType sProto.Body_Type) error {
-	sessionIDBytes, err := offerAnswer.SessionID.Bytes()
-	if err != nil {
-		log.Warnf("failed to get session ID bytes: %v", err)
+	var sessionIDBytes []byte
+	if offerAnswer.SessionID != nil {
+		var err error
+		sessionIDBytes, err = offerAnswer.SessionID.Bytes()
+		if err != nil {
+			log.Warnf("failed to get session ID bytes: %v", err)
+		}
 	}
 	msg, err := signal.MarshalCredential(
 		s.wgPrivateKey,