diff --git a/cmd/cli/dns_intercept_darwin.go b/cmd/cli/dns_intercept_darwin.go index 88d7310..a7f751a 100644 --- a/cmd/cli/dns_intercept_darwin.go +++ b/cmd/cli/dns_intercept_darwin.go @@ -1207,7 +1207,6 @@ func stringSlicesEqual(a, b []string) bool { return true } - // pfStartStabilization enters stabilization mode, suppressing all pf restores // until the VPN's ruleset stops changing. This prevents a death spiral where // ctrld and the VPN repeatedly overwrite each other's pf rules. @@ -1284,6 +1283,10 @@ func (p *prog) pfStabilizationLoop(ctx context.Context, stableRequired time.Dura p.pfStabilizing.Store(false) mainLog.Load().Info().Msgf("DNS intercept: pf stable for %s — restoring anchor rules", stableRequired) p.ensurePFAnchorActive() + routes, domainlessServers, exemptions := p.refreshDNSAfterVPNSettle("pf_stabilized") + if routes == 0 && domainlessServers == 0 && exemptions == 0 { + p.scheduleDNSAfterVPNSettleRefresh("pf_stabilized_followup", pfAnchorRecheckDelayLong) + } p.pfLastRestoreTime.Store(time.Now().UnixMilli()) return } @@ -1446,6 +1449,15 @@ func (p *prog) ensurePFAnchorActive() bool { return true } +func (p *prog) scheduleDNSAfterVPNSettleRefresh(reason string, delay time.Duration) { + time.AfterFunc(delay, func() { + if p.dnsInterceptState == nil { + return + } + p.refreshDNSAfterVPNSettle(reason) + }) +} + // pfWatchdog periodically checks that our pf anchor is still active. // Other programs (e.g., Windscribe desktop app, macOS configd) can replace // scheduleDelayedRechecks schedules delayed re-checks after a network change event. diff --git a/cmd/cli/dns_intercept_settle.go b/cmd/cli/dns_intercept_settle.go new file mode 100644 index 0000000..bc8a0d7 --- /dev/null +++ b/cmd/cli/dns_intercept_settle.go @@ -0,0 +1,50 @@ +package cli + +import "github.com/Control-D-Inc/ctrld" + +var initializeOsResolver = ctrld.InitializeOsResolver + +func (p *prog) refreshDNSAfterVPNSettle(reason string) (routes, domainlessServers, exemptions int) { + mainLog.Load().Info().Msgf("DNS intercept: refreshing OS/VPN DNS route state after VPN settle (%s)", reason) + ns := initializeOsResolver(true) + mainLog.Load().Debug().Msgf("DNS intercept: post-settle OS resolver nameservers: %v", ns) + + if p.vpnDNS == nil { + mainLog.Load().Debug().Msg("DNS intercept: post-settle VPN DNS route refresh skipped — manager unavailable") + return 0, 0, 0 + } + + beforeExemptions := p.vpnDNS.CurrentExemptions() + routes, domainlessServers, exemptions = p.vpnDNS.RefreshRoutesOnly() + afterExemptions := p.vpnDNS.CurrentExemptions() + + if vpnDNSExemptionsEqual(beforeExemptions, afterExemptions) { + mainLog.Load().Info().Msgf("DNS intercept: post-settle VPN DNS route refresh completed — %d routes, %d domainless servers, %d exemptions (pf unchanged)", + routes, domainlessServers, exemptions) + return routes, domainlessServers, exemptions + } + + if err := p.exemptVPNDNSServers(afterExemptions); err != nil { + mainLog.Load().Warn().Err(err).Msg("DNS intercept: post-settle VPN DNS exemption update failed") + } else { + mainLog.Load().Info().Msgf("DNS intercept: post-settle VPN DNS exemptions changed — updated pf/WFP with %d exemptions", len(afterExemptions)) + } + return routes, domainlessServers, exemptions +} + +func vpnDNSExemptionsEqual(a, b []vpnDNSExemption) bool { + if len(a) != len(b) { + return false + } + seen := make(map[vpnDNSExemption]int, len(a)) + for _, ex := range a { + seen[ex]++ + } + for _, ex := range b { + if seen[ex] == 0 { + return false + } + seen[ex]-- + } + return true +} diff --git a/cmd/cli/dns_intercept_settle_test.go b/cmd/cli/dns_intercept_settle_test.go new file mode 100644 index 0000000..925806c --- /dev/null +++ b/cmd/cli/dns_intercept_settle_test.go @@ -0,0 +1,49 @@ +package cli + +import ( + "context" + "testing" + + "github.com/Control-D-Inc/ctrld" +) + +func TestRefreshDNSAfterVPNSettleRefreshesOSResolverAndVPNRoutes(t *testing.T) { + oldInitialize := initializeOsResolver + defer func() { initializeOsResolver = oldInitialize }() + + var initialized []bool + initializeOsResolver = func(force bool) []string { + initialized = append(initialized, force) + return []string{"10.102.26.10:53"} + } + + var exemptionUpdates [][]vpnDNSExemption + p := &prog{} + p.vpnDNS = newVPNDNSManager(func(exemptions []vpnDNSExemption) error { + exemptionUpdates = append(exemptionUpdates, append([]vpnDNSExemption{}, exemptions...)) + return nil + }) + p.vpnDNS.discoverVPNDNS = func(context.Context) []ctrld.VPNDNSConfig { + return []ctrld.VPNDNSConfig{{ + InterfaceName: "utun4", + Servers: []string{"10.102.26.10"}, + Domains: []string{"bmwgroup.net"}, + }} + } + + routes, domainlessServers, exemptions := p.refreshDNSAfterVPNSettle("test") + + if routes != 1 || domainlessServers != 0 || exemptions != 1 { + t.Fatalf("expected 1 route, 0 domainless servers, 1 exemption, got routes=%d domainless=%d exemptions=%d", + routes, domainlessServers, exemptions) + } + if len(initialized) != 1 || !initialized[0] { + t.Fatalf("expected forced OS resolver refresh once, got %v", initialized) + } + if got := p.vpnDNS.UpstreamForDomain("jira.cc.bmwgroup.net."); len(got) != 1 || got[0] != "10.102.26.10" { + t.Fatalf("expected refreshed VPN DNS route, got %v", got) + } + if len(exemptionUpdates) != 0 { + t.Fatalf("expected route-only refresh to avoid pf exemption updates, got %+v", exemptionUpdates) + } +} diff --git a/cmd/cli/vpn_dns.go b/cmd/cli/vpn_dns.go index f671d2d..f6f8c20 100644 --- a/cmd/cli/vpn_dns.go +++ b/cmd/cli/vpn_dns.go @@ -7,6 +7,7 @@ import ( "strings" "sync" + "github.com/rs/zerolog" "tailscale.com/net/netmon" "github.com/Control-D-Inc/ctrld" @@ -94,6 +95,8 @@ func (m *vpnDNSManager) Refresh(guardAgainstNoNameservers bool) { m.mu.Lock() defer m.mu.Unlock() + previousExemptions := m.currentExemptionsLocked() + if vpnDNSSettlingEnabled && len(configs) == 0 && guardAgainstNoNameservers && m.hasVPNDNSStateLocked() { if !m.retainedAfterEmptyDiscovery { exemptions := m.currentExemptionsLocked() @@ -180,14 +183,85 @@ func (m *vpnDNSManager) Refresh(guardAgainstNoNameservers bool) { logger.Debug().Msgf("VPN DNS refresh completed: %d configs, %d routes, %d domainless servers, %d unique exemptions", len(m.configs), len(m.routes), len(m.domainlessServers), len(exemptions)) - // Update intercept rules to permit VPN DNS traffic. - // Always call onServersChanged — including when exemptions is empty — so that - // stale exemptions from a previous VPN session get cleared on disconnect. - if m.onServersChanged != nil { - if err := m.onServersChanged(exemptions); err != nil { - logger.Error().Err(err).Msg("Failed to update intercept exemptions for VPN DNS servers") + // Update intercept rules to permit VPN DNS traffic only when the exemption set + // actually changes. Network-change events can fire repeatedly while macOS/VPN + // state is otherwise identical; rewriting pf for identical exemptions can feed + // a self-triggering network-change loop. Empty exemptions are still applied + // when they differ from the previous set, so stale VPN exemptions are cleared + // on disconnect. + m.updateInterceptExemptionsIfChanged(logger, previousExemptions, exemptions, "VPN DNS") +} + +func (m *vpnDNSManager) updateInterceptExemptionsIfChanged(logger *zerolog.Logger, before, after []vpnDNSExemption, reason string) { + if m.onServersChanged == nil { + return + } + if vpnDNSExemptionsEqual(before, after) { + logger.Debug().Msgf("VPN DNS exemptions unchanged after %s refresh; skipping intercept rule update", reason) + return + } + if err := m.onServersChanged(after); err != nil { + logger.Error().Err(err).Msg("Failed to update intercept exemptions for VPN DNS servers") + } +} + +// RefreshRoutesOnly re-discovers VPN DNS configs and updates only ctrld's +// in-memory split-DNS routes. It intentionally does not call onServersChanged, +// so it does not rewrite/reload pf/WFP rules. Use this for post-settle discovery +// checks where we only need to learn late-published VPN search domains. +func (m *vpnDNSManager) RefreshRoutesOnly() (routes, domainlessServers, exemptions int) { + logger := mainLog.Load() + + logger.Debug().Msg("Refreshing VPN DNS route state only") + discoverVPNDNS := m.discoverVPNDNS + if discoverVPNDNS == nil { + discoverVPNDNS = ctrld.DiscoverVPNDNS + } + configs := discoverVPNDNS(context.Background()) + + if dri, err := netmon.DefaultRouteInterface(); err == nil && dri != "" { + for i := range configs { + if configs[i].InterfaceName == dri { + configs[i].IsExitMode = true + } } } + + m.mu.Lock() + defer m.mu.Unlock() + + m.retainedAfterEmptyDiscovery = false + m.configs = configs + m.routes = make(map[string][]string) + + for _, config := range configs { + for _, domain := range config.Domains { + domain = strings.TrimPrefix(domain, "~") + domain = strings.TrimPrefix(domain, ".") + domain = strings.ToLower(domain) + if domain != "" { + m.routes[domain] = append([]string{}, config.Servers...) + } + } + } + + var domainless []string + seenDomainless := make(map[string]bool) + for _, config := range configs { + if len(config.Domains) == 0 && len(config.Servers) > 0 { + for _, server := range config.Servers { + if !seenDomainless[server] { + seenDomainless[server] = true + domainless = append(domainless, server) + } + } + } + } + m.domainlessServers = domainless + + logger.Debug().Msgf("VPN DNS route-only refresh completed: %d configs, %d routes, %d domainless servers, %d exemptions", + len(m.configs), len(m.routes), len(m.domainlessServers), len(m.currentExemptionsLocked())) + return len(m.routes), len(m.domainlessServers), len(m.currentExemptionsLocked()) } func (m *vpnDNSManager) hasVPNDNSStateLocked() bool { diff --git a/cmd/cli/vpn_dns_test.go b/cmd/cli/vpn_dns_test.go index e8c03e6..6a6d520 100644 --- a/cmd/cli/vpn_dns_test.go +++ b/cmd/cli/vpn_dns_test.go @@ -69,6 +69,31 @@ func TestVPNDNSRefreshClearsOnSecondGuardedEmptyDiscovery(t *testing.T) { } } +func TestVPNDNSRefreshSkipsUnchangedInterceptExemptions(t *testing.T) { + var updates [][]vpnDNSExemption + m := newVPNDNSManager(func(exemptions []vpnDNSExemption) error { + updates = append(updates, append([]vpnDNSExemption{}, exemptions...)) + return nil + }) + m.discoverVPNDNS = func(context.Context) []ctrld.VPNDNSConfig { + return []ctrld.VPNDNSConfig{{ + InterfaceName: "utun-test", + Servers: []string{"10.102.26.10"}, + Domains: []string{"example.internal"}, + }} + } + + m.Refresh(true) + m.Refresh(true) + + if len(updates) != 1 { + t.Fatalf("expected exactly one intercept exemption update for unchanged VPN DNS state, got %d", len(updates)) + } + if len(updates[0]) != 1 || updates[0][0].Server != "10.102.26.10" || updates[0][0].Interface != "utun-test" { + t.Fatalf("unexpected exemption update: %+v", updates[0]) + } +} + func TestVPNDNSTransportFailureSuppressesFallbackOnlyWhileRetainingState(t *testing.T) { withVPNDNSSettlingEnabled(t) m := newVPNDNSManager(nil)