mirror of
https://github.com/Control-D-Inc/ctrld.git
synced 2026-07-16 13:17:19 +02:00
Refresh macOS VPN DNS after pf stabilization
This commit is contained in:
committed by
Cuong Manh Le
parent
da454db8ef
commit
1e1c998c89
@@ -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.
|
||||
|
||||
@@ -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
|
||||
}
|
||||
@@ -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)
|
||||
}
|
||||
}
|
||||
+80
-6
@@ -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 {
|
||||
|
||||
@@ -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)
|
||||
|
||||
Reference in New Issue
Block a user