From 7a88fac3954714279c43769a8a429fac9c41b382 Mon Sep 17 00:00:00 2001 From: Owen Date: Mon, 6 Jul 2026 16:08:10 -0400 Subject: [PATCH] Test the dns server first then fall back --- dns/sysresolver.go | 149 +++++++++++++++++++----- dns/sysresolver_android.go | 18 +++ dns/sysresolver_darwin.go | 2 +- dns/sysresolver_darwin_nosysresolver.go | 17 +++ dns/sysresolver_stub.go | 12 +- dns/sysresolver_test.go | 149 ++++++++++++++++++++++++ olm/olm.go | 55 +++++++-- 7 files changed, 363 insertions(+), 39 deletions(-) create mode 100644 dns/sysresolver_android.go create mode 100644 dns/sysresolver_darwin_nosysresolver.go create mode 100644 dns/sysresolver_test.go diff --git a/dns/sysresolver.go b/dns/sysresolver.go index 0a1c8d7..999047a 100644 --- a/dns/sysresolver.go +++ b/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) { diff --git a/dns/sysresolver_android.go b/dns/sysresolver_android.go new file mode 100644 index 0000000..523c98a --- /dev/null +++ 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 +} diff --git a/dns/sysresolver_darwin.go b/dns/sysresolver_darwin.go index f381dbf..73c89ab 100644 --- a/dns/sysresolver_darwin.go +++ b/dns/sysresolver_darwin.go @@ -1,4 +1,4 @@ -//go:build darwin && !ios +//go:build darwin && !ios && !nosysresolver package dns diff --git a/dns/sysresolver_darwin_nosysresolver.go b/dns/sysresolver_darwin_nosysresolver.go new file mode 100644 index 0000000..8a16ae4 --- /dev/null +++ b/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 +} diff --git a/dns/sysresolver_stub.go b/dns/sysresolver_stub.go index d22774d..6c1f8c9 100644 --- a/dns/sysresolver_stub.go +++ b/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 } diff --git a/dns/sysresolver_test.go b/dns/sysresolver_test.go new file mode 100644 index 0000000..8625f8a --- /dev/null +++ b/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) + } +} diff --git a/olm/olm.go b/olm/olm.go index bc205b7..df4fb3f 100644 --- a/olm/olm.go +++ b/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