Test the dns server first then fall back

dns/sysresolver.go

@@ -9,10 +9,15 @@ import (
 	"time"
 
 	"github.com/fosrl/newt/logger"
+	"github.com/miekg/dns"
 )
 
 const defaultPollInterval = 30 * time.Second
 
+// dnsHealthCheckTimeout bounds how long we wait for a candidate DNS server to
+// answer a health-check query before considering it unusable.
+const dnsHealthCheckTimeout = 2 * time.Second
+
 // SystemDNSMonitor monitors the host system's DNS configuration and notifies
 // callers when it changes. The reported servers are in "host:port" format
 // (e.g. "8.8.8.8:53") and can be used directly as UpstreamDNS and PublicDNS.
@@ -29,7 +34,8 @@ const defaultPollInterval = 30 * time.Second
 //   - Other platforms: returns an empty list (no-op monitor).
 type SystemDNSMonitor struct {
 	mu         sync.RWMutex
-	current    []string
+	current    []string // last health-checked, applied server list
+	lastRaw    []string // last raw (exclude-filtered but unvalidated) candidate list seen
 	onChange   func(servers []string)
 	interval   time.Duration
 	stopCh     chan struct{}
@@ -65,15 +71,7 @@ func (m *SystemDNSMonitor) SetExcludeIP(ip netip.Addr) {
 // Start reads the current system DNS immediately, fires onChange, then polls
 // in the background until Stop is called or ctx is cancelled.
 func (m *SystemDNSMonitor) Start(ctx context.Context) {
-	servers := m.readFiltered()
-	m.mu.Lock()
-	m.current = servers
-	m.mu.Unlock()
-
-	if m.onChange != nil && len(servers) > 0 {
-		m.onChange(servers)
-	}
-
+	m.applyCandidates(m.readFiltered())
 	go m.run(ctx)
 }
 
@@ -100,18 +98,25 @@ func (m *SystemDNSMonitor) Current() []string {
 // excluded the function returns nil so the caller can retain the last
 // known-good value.
 func (m *SystemDNSMonitor) readFiltered() []string {
-	raw := readSystemDNS()
+	return m.filterExcluded(readSystemDNS())
+}
 
+// filterExcluded removes any addresses that have been excluded via
+// SetExcludeIP from servers. Used both for the internally-polled server list
+// (readFiltered) and for server lists reported externally (ReportExternal) by
+// platforms - Android, iOS - where olm cannot read the OS's DNS configuration
+// itself.
+func (m *SystemDNSMonitor) filterExcluded(servers []string) []string {
 	m.excludeMu.RLock()
 	excludeIPs := m.excludeIPs
 	m.excludeMu.RUnlock()
 
 	if len(excludeIPs) == 0 {
-		return raw
+		return servers
 	}
 
 	var filtered []string
-	for _, s := range raw {
+	for _, s := range servers {
 		host, _, err := net.SplitHostPort(s)
 		if err != nil {
 			filtered = append(filtered, s)
@@ -126,6 +131,16 @@ func (m *SystemDNSMonitor) readFiltered() []string {
 	return filtered
 }
 
+// ReportExternal applies an externally-observed DNS server list (e.g. from
+// Android's ConnectivityManager or iOS's SCDynamicStore, where the platform
+// itself - not olm - must detect the OS's real DNS configuration) through the
+// same exclude-IP filtering, health-check validation, and change-detection as
+// the internal poll loop, firing onChange if the result differs from the last
+// known value.
+func (m *SystemDNSMonitor) ReportExternal(servers []string) {
+	m.applyCandidates(m.filterExcluded(servers))
+}
+
 func (m *SystemDNSMonitor) run(ctx context.Context) {
 	ticker := time.NewTicker(m.interval)
 	defer ticker.Stop()
@@ -137,26 +152,102 @@ func (m *SystemDNSMonitor) run(ctx context.Context) {
 		case <-m.stopCh:
 			return
 		case <-ticker.C:
-			servers := m.readFiltered()
-			if len(servers) == 0 {
-				continue
-			}
-
-			m.mu.Lock()
-			changed := !dnsSlicesEqual(m.current, servers)
-			if changed {
-				m.current = servers
-			}
-			m.mu.Unlock()
-
-			if changed && m.onChange != nil {
-				logger.Info("System DNS changed: %v", servers)
-				m.onChange(servers)
-			}
+			m.applyCandidates(m.readFiltered())
 		}
 	}
 }
 
+// applyCandidates takes an exclude-filtered (but not yet health-checked) list
+// of candidate DNS servers - from either the internal poll loop or
+// ReportExternal - and, only if it differs from the last raw list seen (to
+// avoid re-running network health checks on every 30-second poll tick when
+// nothing has actually changed), health-checks it via filterUnreachable and
+// applies whatever passes, firing onChange if the result changed.
+//
+// If none of the candidates pass the health check, the previous known-good
+// value is retained rather than clobbered - this is what protects against
+// e.g. a carrier reporting a DNS server (such as T-Mobile's internal ULA
+// DNS64 resolvers) that is technically "the system DNS" but not actually
+// reachable/usable from wherever queries are sent.
+func (m *SystemDNSMonitor) applyCandidates(raw []string) {
+	if len(raw) == 0 {
+		return
+	}
+
+	m.mu.Lock()
+	if dnsSlicesEqual(m.lastRaw, raw) {
+		m.mu.Unlock()
+		return
+	}
+	m.lastRaw = raw
+	m.mu.Unlock()
+
+	validated := filterUnreachable(raw)
+	if len(validated) == 0 {
+		logger.Warn("None of the detected DNS servers answered a health-check query, keeping previous value: %v", raw)
+		return
+	}
+
+	m.mu.Lock()
+	changed := !dnsSlicesEqual(m.current, validated)
+	if changed {
+		m.current = validated
+	}
+	m.mu.Unlock()
+
+	if changed && m.onChange != nil {
+		logger.Info("System DNS changed: %v", validated)
+		m.onChange(validated)
+	}
+}
+
+// dnsServerReachable is a seam for tests; production code always uses probeDNSServer.
+var dnsServerReachable = probeDNSServer
+
+// filterUnreachable validates that each candidate server actually answers a
+// DNS query before it's trusted, rather than statically guessing from the
+// address (e.g. rejecting all private/ULA addresses, which would also reject
+// a perfectly valid home router forwarding to a real resolver). Checks run
+// concurrently so multiple candidates don't serialize the timeout.
+func filterUnreachable(servers []string) []string {
+	if len(servers) == 0 {
+		return servers
+	}
+
+	reachable := make([]bool, len(servers))
+	var wg sync.WaitGroup
+	for i, server := range servers {
+		wg.Add(1)
+		go func(i int, server string) {
+			defer wg.Done()
+			reachable[i] = dnsServerReachable(server)
+		}(i, server)
+	}
+	wg.Wait()
+
+	var result []string
+	for i, server := range servers {
+		if reachable[i] {
+			result = append(result, server)
+		} else {
+			logger.Debug("Discarding DNS server %s: failed health check", server)
+		}
+	}
+	return result
+}
+
+// probeDNSServer sends a minimal root NS query to confirm a candidate server
+// actually answers, without depending on any specific external hostname being
+// reachable (which could itself be blocked/filtered independently of whether
+// the resolver works).
+func probeDNSServer(server string) bool {
+	client := &dns.Client{Timeout: dnsHealthCheckTimeout}
+	msg := new(dns.Msg)
+	msg.SetQuestion(".", dns.TypeNS)
+	_, _, err := client.Exchange(msg, server)
+	return err == nil
+}
+
 // dnsSlicesEqual reports whether two server lists are equal regardless of order.
 func dnsSlicesEqual(a, b []string) bool {
 	if len(a) != len(b) {

dns/sysresolver_android.go

@@ -0,0 +1,18 @@
+//go:build android
+
+package dns
+
+// readSystemDNS returns nil on Android: olm cannot read the OS's DNS
+// configuration itself here, so the app detects it (via ConnectivityManager)
+// and pushes it in through Olm.SetSystemDNS instead (see SystemDnsMonitor.java).
+//
+// This is a dedicated file (rather than falling through the general
+// sysresolver_stub.go catch-all) because wireguard-android's build passes
+// "-tags linux" to share Linux netlink code with Android, and that custom tag
+// makes "!linux" evaluate to false even on a real GOOS=android build, which
+// would otherwise make sysresolver_stub.go stop applying and leave
+// readSystemDNS undefined. An explicit "android" constraint isn't affected by
+// that, since nothing passes a conflicting "-tags android".
+func readSystemDNS() []string {
+	return nil
+}

dns/sysresolver_darwin.go

@@ -1,4 +1,4 @@
-//go:build darwin && !ios
+//go:build darwin && !ios && !nosysresolver
 
 package dns
 

dns/sysresolver_darwin_nosysresolver.go

@@ -0,0 +1,17 @@
+//go:build darwin && !ios && nosysresolver
+
+package dns
+
+// readSystemDNS is disabled by the nosysresolver build tag. This is used for
+// the macOS app build: unlike the CLI, the app's PacketTunnel system
+// extension additionally applies NEDNSSettings (see apple/PacketTunnel),
+// which can become the system's primary resolver and make /etc/resolv.conf
+// reflect olm's own proxy IP instead of the real upstream DNS. Rather than
+// have olm poll a value that may be self-referential, the app pushes the
+// real DNS servers in via SetSystemDNS (detected in Swift via
+// SCDynamicStore) exactly like Android and iOS. The CLI keeps the real
+// implementation in sysresolver_darwin.go, since it has no such override
+// mechanism and /etc/resolv.conf always reflects the physical network there.
+func readSystemDNS() []string {
+	return nil
+}

dns/sysresolver_stub.go

@@ -1,10 +1,18 @@
-//go:build !linux && !darwin && !windows
+//go:build !linux && !darwin && !windows && !android
 
 package dns
 
 // readSystemDNS returns nil on platforms where automatic DNS discovery is not
-// implemented (android, ios, freebsd, etc.).  Callers should fall back to a
+// implemented (ios, freebsd, etc.).  Callers should fall back to a
 // statically configured DNS server.
+//
+// android is excluded from this constraint (and has its own sysresolver_android.go
+// with an explicit "android" tag) rather than falling through the "!linux" catch-all
+// here: wireguard-android's build passes "-tags linux" to share Linux netlink code
+// (a deliberate, long-standing convention, since Android's kernel is Linux), and Go's
+// build constraint evaluator can't distinguish a custom "-tags linux" from the real
+// GOOS=linux - so with that tag set, "!linux" is false even though GOOS is actually
+// android, and this file would silently stop applying, leaving readSystemDNS undefined.
 func readSystemDNS() []string {
 	return nil
 }

dns/sysresolver_test.go

@@ -0,0 +1,149 @@
+package dns
+
+import (
+	"net/netip"
+	"reflect"
+	"testing"
+)
+
+// stubReachable overrides dnsServerReachable for the duration of the test so
+// tests don't depend on real network access, restoring the original on
+// cleanup.
+func stubReachable(t *testing.T, fn func(server string) bool) {
+	t.Helper()
+	orig := dnsServerReachable
+	dnsServerReachable = fn
+	t.Cleanup(func() { dnsServerReachable = orig })
+}
+
+func allReachable(t *testing.T) {
+	stubReachable(t, func(string) bool { return true })
+}
+
+func TestReportExternalFiltersExcludedIP(t *testing.T) {
+	allReachable(t)
+
+	var got []string
+	m := &SystemDNSMonitor{
+		excludeIPs: make(map[netip.Addr]bool),
+		onChange: func(servers []string) {
+			got = servers
+		},
+	}
+	m.SetExcludeIP(netip.MustParseAddr("10.0.0.1"))
+
+	m.ReportExternal([]string{"10.0.0.1:53", "8.8.8.8:53"})
+
+	want := []string{"8.8.8.8:53"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("onChange servers = %v, want %v", got, want)
+	}
+	if !reflect.DeepEqual(m.Current(), want) {
+		t.Fatalf("Current() = %v, want %v", m.Current(), want)
+	}
+}
+
+func TestReportExternalAllExcludedIsNoop(t *testing.T) {
+	allReachable(t)
+
+	called := false
+	m := &SystemDNSMonitor{
+		excludeIPs: make(map[netip.Addr]bool),
+		current:    []string{"1.1.1.1:53"},
+		onChange: func(servers []string) {
+			called = true
+		},
+	}
+	m.SetExcludeIP(netip.MustParseAddr("10.0.0.1"))
+
+	m.ReportExternal([]string{"10.0.0.1:53"})
+
+	if called {
+		t.Fatal("onChange should not fire when all reported servers are excluded")
+	}
+	want := []string{"1.1.1.1:53"}
+	if !reflect.DeepEqual(m.Current(), want) {
+		t.Fatalf("Current() = %v, want unchanged %v", m.Current(), want)
+	}
+}
+
+func TestReportExternalOnlyFiresOnChange(t *testing.T) {
+	allReachable(t)
+
+	calls := 0
+	m := &SystemDNSMonitor{
+		excludeIPs: make(map[netip.Addr]bool),
+		onChange: func(servers []string) {
+			calls++
+		},
+	}
+
+	m.ReportExternal([]string{"8.8.8.8:53"})
+	m.ReportExternal([]string{"8.8.8.8:53"})
+
+	if calls != 1 {
+		t.Fatalf("onChange fired %d times, want 1", calls)
+	}
+}
+
+func TestReportExternalDropsUnreachableServer(t *testing.T) {
+	// Simulates e.g. T-Mobile's private ULA DNS64 resolver: technically "the
+	// system DNS" per the OS, but doesn't actually answer queries.
+	stubReachable(t, func(server string) bool {
+		return server != "[fd00:976a::9]:53"
+	})
+
+	var got []string
+	m := &SystemDNSMonitor{
+		excludeIPs: make(map[netip.Addr]bool),
+		onChange: func(servers []string) {
+			got = servers
+		},
+	}
+
+	m.ReportExternal([]string{"[fd00:976a::9]:53", "8.8.8.8:53"})
+
+	want := []string{"8.8.8.8:53"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("onChange servers = %v, want %v", got, want)
+	}
+}
+
+func TestReportExternalKeepsPreviousValueWhenAllUnreachable(t *testing.T) {
+	stubReachable(t, func(string) bool { return true })
+
+	called := false
+	m := &SystemDNSMonitor{
+		excludeIPs: make(map[netip.Addr]bool),
+		current:    []string{"1.1.1.1:53"},
+		onChange: func(servers []string) {
+			called = true
+		},
+	}
+
+	// Now simulate every candidate failing the health check (e.g. moved to a
+	// network where none of the reported servers actually respond).
+	stubReachable(t, func(string) bool { return false })
+
+	m.ReportExternal([]string{"[fd00:976a::9]:53", "[fd00:976a::10]:53"})
+
+	if called {
+		t.Fatal("onChange should not fire when no candidate passes the health check")
+	}
+	want := []string{"1.1.1.1:53"}
+	if !reflect.DeepEqual(m.Current(), want) {
+		t.Fatalf("Current() = %v, want unchanged %v", m.Current(), want)
+	}
+}
+
+func TestFilterUnreachable(t *testing.T) {
+	stubReachable(t, func(server string) bool {
+		return server == "8.8.8.8:53"
+	})
+
+	got := filterUnreachable([]string{"10.0.0.1:53", "8.8.8.8:53", "9.9.9.9:53"})
+	want := []string{"8.8.8.8:53"}
+	if !reflect.DeepEqual(got, want) {
+		t.Fatalf("filterUnreachable() = %v, want %v", got, want)
+	}
+}

olm/olm.go

@@ -43,13 +43,18 @@ type Olm struct {
 	middleDev    *olmDevice.MiddleDevice
 	sharedBind   *bind.SharedBind
 
-	dnsProxy         *dns.DNSProxy
-	dnsMonitor       *dns.SystemDNSMonitor
-	apiServer        *api.API
-	websocket        *websocket.Client
-	holePunchManager *holepunch.Manager
-	peerManager      *peers.PeerManager
-	peerManagerMu    sync.RWMutex
+	dnsProxy   *dns.DNSProxy
+	dnsMonitor *dns.SystemDNSMonitor
+	// pendingSystemDNS holds a SetSystemDNS report received before dnsMonitor exists
+	// (e.g. Android/iOS push a value while the tunnel is still starting up), so it
+	// isn't silently dropped. Drained into dnsMonitor as soon as StartTunnel creates it.
+	pendingSystemDNSMu sync.Mutex
+	pendingSystemDNS   []string
+	apiServer          *api.API
+	websocket          *websocket.Client
+	holePunchManager   *holepunch.Manager
+	peerManager        *peers.PeerManager
+	peerManagerMu      sync.RWMutex
 	// Power mode management
 	currentPowerMode string
 	powerModeMu      sync.Mutex
@@ -432,6 +437,12 @@ func (o *Olm) StartTunnel(config TunnelConfig) {
 	})
 	o.dnsMonitor.Start(o.olmCtx)
 
+	// Apply any SetSystemDNS report that arrived before dnsMonitor existed (e.g. an
+	// Android/iOS push that raced ahead of this goroutine).
+	if pending := o.takePendingSystemDNS(); len(pending) > 0 {
+		o.dnsMonitor.ReportExternal(pending)
+	}
+
 	// Fall back to hardcoded DNS if the system monitor could not detect any.
 	if len(o.tunnelConfig.PublicDNS) == 0 {
 		if o.tunnelConfig.DNS != "" {
@@ -879,6 +890,36 @@ func (o *Olm) SetPostures(data map[string]any) {
 	o.postures = data
 }
 
+// SetSystemDNS reports DNS servers observed by platform-native code. On
+// Android and iOS olm cannot read the OS's DNS configuration itself (unlike
+// Linux/macOS/Windows, see dns.readSystemDNS), so the app/extension detects
+// the real pre-override DNS servers and pushes them here as the network
+// changes. The list is applied through the same exclude-IP filtering and
+// change detection as the internally-polled SystemDNSMonitor.
+func (o *Olm) SetSystemDNS(servers []string) {
+	if o.dnsMonitor == nil {
+		// StartTunnel hasn't created the monitor yet (mobile platforms may push a
+		// value the moment they start observing, before the tunnel goroutine has
+		// gotten far enough to construct it). Stash it so StartTunnel can apply it
+		// instead of falling back to a hardcoded default DNS server.
+		o.pendingSystemDNSMu.Lock()
+		o.pendingSystemDNS = servers
+		o.pendingSystemDNSMu.Unlock()
+		return
+	}
+	o.dnsMonitor.ReportExternal(servers)
+}
+
+// takePendingSystemDNS returns and clears any SetSystemDNS value reported before
+// dnsMonitor existed.
+func (o *Olm) takePendingSystemDNS() []string {
+	o.pendingSystemDNSMu.Lock()
+	defer o.pendingSystemDNSMu.Unlock()
+	pending := o.pendingSystemDNS
+	o.pendingSystemDNS = nil
+	return pending
+}
+
 // SetPowerMode switches between normal and low power modes
 // In low power mode: websocket is closed (stopping pings) and monitoring intervals are set to 10 minutes
 // In normal power mode: websocket is reconnected (restarting pings) and monitoring intervals are restored