mirror of
https://github.com/Control-D-Inc/ctrld.git
synced 2026-07-16 13:17:19 +02:00
fix: restore v1 intercept parity on master
Restore the previously merged macOS VPN DNS post-settle refresh, unchanged-exemption suppression, forced pf state flush, and self-upgrade test after they were removed by the firewall-mode merge. Also keep Windows NRPT pointed at loopback when the listener is configured on a wildcard address.
This commit is contained in:
committed by
Cuong Manh Le
parent
7f3d332b64
commit
c43739e42d
@@ -1233,6 +1233,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
|
||||
}
|
||||
@@ -1459,6 +1463,15 @@ func isResourceExhaustion(err error, output []byte) bool {
|
||||
strings.Contains(msg, "cannot allocate memory")
|
||||
}
|
||||
|
||||
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.
|
||||
@@ -1826,7 +1839,14 @@ func (p *prog) forceReloadPFMainRuleset() {
|
||||
mainLog.Load().Error().Err(err).Msgf("DNS intercept: force reload — failed to load anchor (output: %s)", strings.TrimSpace(string(out)))
|
||||
}
|
||||
|
||||
// Reset upstream transports — pf reload flushes state table, killing DoH connections.
|
||||
// Flush stale rdr/reply states after the forced ruleset + anchor reload.
|
||||
// Without this, macOS can keep using pre-reload state and try to send
|
||||
// redirected DNS replies directly from loopback to tunnel client addresses
|
||||
// (for example, 127.0.0.1:<listener> -> 100.64.0.0/10), which fails with
|
||||
// "sendmsg: can't assign requested address".
|
||||
flushPFStates()
|
||||
|
||||
// Reset upstream transports — pf reload/state flush kills existing DoH connections.
|
||||
p.resetUpstreamTransports()
|
||||
|
||||
mainLog.Load().Info().Msg("DNS intercept: force reload — pf ruleset and anchor reloaded successfully")
|
||||
|
||||
@@ -0,0 +1,55 @@
|
||||
package cli
|
||||
|
||||
import (
|
||||
"context"
|
||||
|
||||
"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)
|
||||
ctx := ctrld.LoggerCtx(context.Background(), mainLog.Load())
|
||||
ns := initializeOsResolver(ctx, 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(ctx context.Context, force bool) []string {
|
||||
initialized = append(initialized, force)
|
||||
return []string{"10.102.26.10:53"}
|
||||
}
|
||||
|
||||
var exemptionUpdates [][]vpnDNSExemption
|
||||
p := &prog{}
|
||||
p.vpnDNS = newVPNDNSManager(&mainLog, 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)
|
||||
}
|
||||
}
|
||||
@@ -683,8 +683,11 @@ func (p *prog) startDNSIntercept() error {
|
||||
// server, ctrld may have fallen back to 127.0.0.x:53 instead of 127.0.0.1:53.
|
||||
// NRPT must point to whichever address ctrld is actually listening on.
|
||||
listenerIP := "127.0.0.1"
|
||||
if lc := p.cfg.FirstListener(); lc != nil && lc.IP != "" {
|
||||
if lc := p.cfg.FirstListener(); lc != nil && lc.IP != "" && lc.IP != "0.0.0.0" && lc.IP != "::" {
|
||||
listenerIP = lc.IP
|
||||
} else if lc != nil && (lc.IP == "0.0.0.0" || lc.IP == "::") {
|
||||
mainLog.Load().Warn().Str("configured_ip", lc.IP).
|
||||
Msg("DNS intercept: listener configured with wildcard IP, using 127.0.0.1 for NRPT rules")
|
||||
}
|
||||
|
||||
state := &wfpState{
|
||||
|
||||
@@ -2,6 +2,7 @@ package cli
|
||||
|
||||
import (
|
||||
"os"
|
||||
"os/exec"
|
||||
"strings"
|
||||
"testing"
|
||||
|
||||
@@ -28,5 +29,20 @@ func TestMain(m *testing.M) {
|
||||
l := zap.New(core)
|
||||
|
||||
mainLog.Store(&ctrld.Logger{Logger: l})
|
||||
|
||||
// Stub the self-upgrade command builder for the whole test binary. The real
|
||||
// builder execs os.Executable() — which under `go test` IS this test binary
|
||||
// — with positional args ("upgrade", ...). `go test` stops flag parsing at
|
||||
// the first positional arg and ignores the rest, so the child just re-runs
|
||||
// the entire suite, hits the upgrade tests again, and spawns more children:
|
||||
// a fork bomb of detached processes that stalls the host and (on Windows)
|
||||
// holds the test binary's image locked, breaking CI artifact cleanup.
|
||||
// Point it at the test binary with a no-match -test.run so any test that
|
||||
// reaches performUpgrade still exercises the cmd.Start() success path while
|
||||
// the child exits immediately without recursing.
|
||||
newUpgradeCmd = func(exe string) *exec.Cmd {
|
||||
return exec.Command(exe, "-test.run=^$")
|
||||
}
|
||||
|
||||
os.Exit(m.Run())
|
||||
}
|
||||
|
||||
+14
-2
@@ -1659,6 +1659,19 @@ func shouldUpgrade(vt string, cv *semver.Version, logger *ctrld.Logger) bool {
|
||||
return true
|
||||
}
|
||||
|
||||
// newUpgradeCmd builds the detached command used to self-upgrade. It is a
|
||||
// package-level variable so tests can stub it. With the real implementation a
|
||||
// *test* binary would re-exec itself — os.Executable() is the test binary, and
|
||||
// because `go test` stops flag parsing at the first positional arg ("upgrade")
|
||||
// it ignores the args and re-runs the entire suite. That child hits the same
|
||||
// upgrade test and spawns another child, recursively: a fork bomb of detached
|
||||
// processes that pins the host and locks the test binary's image file.
|
||||
var newUpgradeCmd = func(exe string) *exec.Cmd {
|
||||
cmd := exec.Command(exe, "upgrade", "prod", "-vv")
|
||||
cmd.SysProcAttr = sysProcAttrForDetachedChildProcess()
|
||||
return cmd
|
||||
}
|
||||
|
||||
// performUpgrade executes the self-upgrade command.
|
||||
// Returns true if upgrade was initiated successfully, false otherwise.
|
||||
func performUpgrade(vt string, logger *ctrld.Logger) bool {
|
||||
@@ -1667,8 +1680,7 @@ func performUpgrade(vt string, logger *ctrld.Logger) bool {
|
||||
logger.Error().Err(err).Msg("Failed to get executable path, skipped self-upgrade")
|
||||
return false
|
||||
}
|
||||
cmd := exec.Command(exe, "upgrade", "prod", "-vv")
|
||||
cmd.SysProcAttr = sysProcAttrForDetachedChildProcess()
|
||||
cmd := newUpgradeCmd(exe)
|
||||
if err := cmd.Start(); err != nil {
|
||||
logger.Error().Err(err).Msg("Failed to start self-upgrade")
|
||||
return false
|
||||
|
||||
@@ -283,6 +283,8 @@ func Test_performUpgrade(t *testing.T) {
|
||||
},
|
||||
}
|
||||
|
||||
// newUpgradeCmd is stubbed in TestMain so performUpgrade does not re-exec
|
||||
// (and fork-bomb) the test binary; see the comment there.
|
||||
for _, tc := range tests {
|
||||
tc := tc
|
||||
t.Run(tc.name, func(t *testing.T) {
|
||||
|
||||
+79
-6
@@ -110,6 +110,8 @@ func (m *vpnDNSManager) Refresh(ctx context.Context, guardAgainstNoNameservers .
|
||||
m.mu.Lock()
|
||||
defer m.mu.Unlock()
|
||||
|
||||
previousExemptions := m.currentExemptionsLocked()
|
||||
|
||||
if vpnDNSSettlingEnabled && len(configs) == 0 && guardedRefresh && m.hasVPNDNSStateLocked() {
|
||||
if !m.retainedAfterEmptyDiscovery {
|
||||
exemptions := m.currentExemptionsLocked()
|
||||
@@ -198,14 +200,85 @@ func (m *vpnDNSManager) Refresh(ctx context.Context, guardAgainstNoNameservers .
|
||||
ctrld.Log(ctx, logger.Debug(), "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 {
|
||||
ctrld.Log(ctx, logger.Error().Err(err), "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(ctx, logger, previousExemptions, exemptions, "VPN DNS")
|
||||
}
|
||||
|
||||
func (m *vpnDNSManager) updateInterceptExemptionsIfChanged(ctx context.Context, logger *ctrld.Logger, before, after []vpnDNSExemption, reason string) {
|
||||
if m.onServersChanged == nil {
|
||||
return
|
||||
}
|
||||
if vpnDNSExemptionsEqual(before, after) {
|
||||
ctrld.Log(ctx, logger.Debug(), "VPN DNS exemptions unchanged after %s refresh; skipping intercept rule update", reason)
|
||||
return
|
||||
}
|
||||
if err := m.onServersChanged(after); err != nil {
|
||||
ctrld.Log(ctx, logger.Error().Err(err), "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 {
|
||||
|
||||
@@ -101,6 +101,31 @@ func TestVPNDNSRefreshClearsOnSecondGuardedEmptyDiscovery(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
func TestVPNDNSRefreshSkipsUnchangedInterceptExemptions(t *testing.T) {
|
||||
var updates [][]vpnDNSExemption
|
||||
m := newVPNDNSManager(&mainLog, 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(context.Background(), true)
|
||||
m.Refresh(context.Background(), 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(&mainLog, nil)
|
||||
|
||||
Reference in New Issue
Block a user