dns intercept: handle DNS-less networks and canceled-recovery state leak

macOS intercept mode fails totally on networks that provide no usable
IPv4 DNS (e.g. IPv6-only iPhone tethering with 464XLAT): the pf ruleset
blocks all outbound IPv6 port 53, and with no IPv4 DNS configured
mDNSResponder emits no DNS packets at all, so pf has nothing to
intercept while the Control D upstream stays provably healthy.

When network-change recovery discovers no usable IPv4 DNS on the
default-route service, set 127.0.0.1 as that service DNS so macOS can
emit queries that land directly on the listener. The entry is removed
when the network regains IPv4 DNS and on intercept shutdown; networks
that provide IPv4 DNS are never modified. Runs on the already-debounced
recovery path so interface flaps do not churn networksetup.

Also fix the canceled-recovery state leak: the cancellation early
return never reset recoveryBypass/recoveryRunning, so a flap burst
ending in a canceled recovery left the daemon in bypass forever with
the DNS watchdog disabled. Cleanup is generation-gated so a superseded
recovery never clears state owned by its successor.
This commit is contained in:
Dev Scribe
2026-09-01 11:08:13 +07:00
committed by Cuong Manh Le
parent d2aa155ff3
commit 8013bb72d2
18 changed files with 1392 additions and 83 deletions
+10
View File
@@ -484,6 +484,11 @@ func (p *prog) checkAnchorOrdering(filterLines []string, ourAnchorRef string) {
// stopDNSIntercept removes all pf rules and cleans up the DNS interception.
func (p *prog) stopDNSIntercept() error {
// Remove a loopback DNS target set for a DNS-less network (issue #533)
// before tearing down pf, so the service is returned to its saved or
// empty DNS state.
p.removeInterceptDNSTarget("intercept shutdown")
state, ok := p.dnsInterceptState.(*pfState)
if !ok || state == nil {
mainLog.Load().Debug().Msg("DNS intercept: no pf state to clean up")
@@ -1801,6 +1806,11 @@ func (p *prog) pfWatchdog() {
return
}
// Reconcile the temporary service DNS target even when macOS emits no
// major network delta. This converges both DHCP-return cleanup and a
// later return to a DNS-less network.
ensureInterceptDNSTargetFn(p, []string{})
result := p.ensurePFAnchorActive()
if result == pfAnchorCheckIntact {
// Only an authoritative intact result may trigger the functional probe.
+46 -50
View File
@@ -1852,24 +1852,13 @@ func (p *prog) debounceRecovery() {
func (p *prog) handleRecovery(reason RecoveryReason) {
mainLog.Load().Debug().Msg("Starting recovery process: removing DNS settings")
// For network changes, cancel any existing recovery check because the network state has changed.
recoveryCtx, gen, interceptRecovery, ok := p.beginRecovery(reason)
if !ok {
mainLog.Load().Debug().Msg("Upstream recovery already in progress; skipping duplicate trigger")
return
}
if reason == RecoveryReasonNetworkChange {
p.recoveryCancelMu.Lock()
if p.recoveryCancel != nil {
mainLog.Load().Debug().Msg("Cancelling existing recovery check (network change)")
p.recoveryCancel()
p.recoveryCancel = nil
}
p.recoveryCancelMu.Unlock()
} else {
// For upstream failures, if a recovery is already in progress, do nothing new.
p.recoveryCancelMu.Lock()
if p.recoveryCancel != nil {
mainLog.Load().Debug().Msg("Upstream recovery already in progress; skipping duplicate trigger")
p.recoveryCancelMu.Unlock()
return
}
p.recoveryCancelMu.Unlock()
mainLog.Load().Debug().Msg("Network change recovery now owns shared recovery state")
}
// For network changes, force-reset all upstream transports synchronously.
@@ -1887,32 +1876,28 @@ func (p *prog) handleRecovery(reason RecoveryReason) {
mainLog.Load().Info().Msg("Force-reset upstream transports for network change recovery")
}
// Create a new recovery context without a fixed timeout.
p.recoveryCancelMu.Lock()
recoveryCtx, cancel := context.WithCancel(context.Background())
p.recoveryCancel = cancel
p.recoveryCancelMu.Unlock()
// set recoveryRunning to true to prevent watchdogs from putting the listener back on the interface
p.recoveryRunning.Store(true)
// In DNS intercept mode, don't tear down WFP/pf filters.
// Instead, enable recovery bypass so proxy() forwards queries to
// the OS/DHCP resolver. This handles captive portal authentication
// without the overhead of filter teardown/rebuild.
if dnsIntercept && p.dnsInterceptState != nil {
p.recoveryBypass.Store(true)
if interceptRecovery {
mainLog.Load().Info().Msg("DNS intercept recovery: enabling DHCP bypass (filters stay active)")
// Reinitialize OS resolver to discover DHCP servers on the new network.
mainLog.Load().Debug().Msg("DNS intercept recovery: discovering DHCP nameservers")
dhcpServers := ctrld.InitializeOsResolver(true)
dhcpServers, systemNameservers := ctrld.InitializeOsResolverWithSystemNameservers(true)
if len(dhcpServers) == 0 {
mainLog.Load().Warn().Msg("DNS intercept recovery: no DHCP nameservers found")
} else {
mainLog.Load().Info().Msgf("DNS intercept recovery: found DHCP nameservers: %v", dhcpServers)
}
// If the new network provides no usable IPv4 DNS (e.g. IPv6-only
// tethering with 464XLAT), macOS cannot emit DNS queries at all and
// pf has nothing to intercept. Ensure a loopback DNS target exists
// so the OS keeps sending queries to ctrld's listener (issue #533).
ensureInterceptDNSTargetFn(p, systemNameservers)
// Exempt DHCP nameservers from intercept filters so the OS resolver
// can actually reach them on port 53.
if len(dhcpServers) > 0 {
@@ -1954,21 +1939,20 @@ func (p *prog) handleRecovery(reason RecoveryReason) {
recovered, err := p.waitForUpstreamRecovery(recoveryCtx, upstreams)
if err != nil {
mainLog.Load().Error().Err(err).Msg("Recovery canceled; DNS settings remain removed")
p.recoveryCancelMu.Lock()
p.recoveryCancel = nil
p.recoveryCancelMu.Unlock()
p.recoveryCanceledCleanup(gen)
return
}
if !p.recoveryOwnsState(gen) {
mainLog.Load().Debug().Msgf("Recovery generation %d was superseded after upstream success; skipping stale completion", gen)
return
}
mainLog.Load().Info().Msgf("Upstream %q recovered; re-applying DNS settings", recovered)
// reset the upstream failure count and down state
// Reset the upstream failure count and down state while this generation
// still owns recovery completion.
p.um.reset(recovered)
// In DNS intercept mode, just disable the bypass — filters are still active.
if dnsIntercept && p.dnsInterceptState != nil {
p.recoveryBypass.Store(false)
mainLog.Load().Info().Msg("DNS intercept recovery complete: disabling DHCP bypass, resuming normal flow")
if interceptRecovery {
// Refresh VPN DNS routes in case VPN state changed during recovery.
if p.vpnDNS != nil {
p.vpnDNS.Refresh(true)
@@ -1983,11 +1967,15 @@ func (p *prog) handleRecovery(reason RecoveryReason) {
mainLog.Load().Info().Msgf("Reinitialized OS resolver with nameservers: %v", ns)
}
}
p.recoveryRunning.Store(false)
} else {
// For network changes we also reinitialize the OS resolver.
if reason == RecoveryReasonNetworkChange {
var systemNameservers []string
if dnsIntercept {
// Intercept was requested but no interceptor was active when recovery
// began. Rediscover on every recovery reason before retrying setDNS;
// passing nil could make a successful retry install a loopback target
// on a healthy DHCP network.
systemNameservers = systemNameserversForInterceptRetry()
} else if reason == RecoveryReasonNetworkChange {
ns := ctrld.InitializeOsResolver(true)
if len(ns) == 0 {
mainLog.Load().Warn().Msg("No nameservers found for OS resolver during network-change recovery; using existing values")
@@ -1997,17 +1985,17 @@ func (p *prog) handleRecovery(reason RecoveryReason) {
}
// Apply our DNS settings back and log the interface state.
p.setDNS()
p.setDNS(systemNameservers)
p.logInterfacesState()
// allow watchdogs to put the listener back on the interface if its changed for any reason
p.recoveryRunning.Store(false)
}
// Clear the recovery cancellation for a clean slate.
p.recoveryCancelMu.Lock()
p.recoveryCancel = nil
p.recoveryCancelMu.Unlock()
if !p.completeRecovery(gen) {
mainLog.Load().Debug().Msgf("Recovery generation %d was superseded during completion; preserving successor state", gen)
return
}
if interceptRecovery {
mainLog.Load().Info().Msg("DNS intercept recovery complete: disabling DHCP bypass, resuming normal flow")
}
}
// waitForUpstreamRecovery checks the provided upstreams concurrently until one recovers.
@@ -2085,7 +2073,15 @@ func (p *prog) waitForUpstreamRecovery(ctx context.Context, upstreams map[string
var recovered string
select {
case <-ctx.Done():
return "", ctx.Err()
default:
}
select {
case recovered = <-recoveredCh:
if err := ctx.Err(); err != nil {
return "", err
}
case <-ctx.Done():
return "", ctx.Err()
}
+116
View File
@@ -0,0 +1,116 @@
package cli
import "net"
// interceptDNSRdrTarget is the loopback address used as the macOS service
// DNS value when ctrld's listener is NOT reachable at <listener IP>:53
// directly (non-53 port, e.g. 127.0.0.1:5354 when mDNSResponder holds *:53).
//
// macOS resolvers always send DNS to port 53, so a direct-hit value is
// impossible in that case; delivery must go through the pf rdr rule
// ("rdr on lo0 ... to ! <listenerIP> port 53 -> <listenerIP> port <port>").
// The value therefore must be a loopback address DIFFERENT from the listener
// IP so the rdr's "! <listenerIP>" matches. Any 127/8 address routes via lo0
// on macOS.
const interceptDNSRdrTarget = "127.0.0.53"
// interceptDNSTargetValue returns the nameserver value to set on a DNS-less
// macOS service so the OS emits DNS queries that reach ctrld, respecting the
// configured listener. The listener IP/port derivation mirrors
// buildPFAnchorRulesForTunnels so the value and the pf rules always agree.
//
// - listener on port 53: return the effective listener IP — queries hit the
// listener directly, no pf dependency for this leg.
// - listener on another port: return interceptDNSRdrTarget so the lo0 rdr
// rule fires and rewrites to the real listener address.
func (p *prog) interceptDNSTargetValue() string {
listenerIP := "127.0.0.1"
listenerPort := 53
// FirstListener panics when no listener is configured; guard like the
// startup paths do.
if p.cfg != nil && len(p.cfg.Listener) > 0 {
if lc := p.cfg.FirstListener(); lc != nil {
if lc.IP != "" && lc.IP != "0.0.0.0" && lc.IP != "::" {
listenerIP = lc.IP
}
if lc.Port != 0 {
listenerPort = lc.Port
}
}
}
if listenerPort == 53 {
return listenerIP
}
if listenerIP == interceptDNSRdrTarget {
// Pathological config: the listener itself sits on the rdr target
// address (with a non-53 port). Pick a different loopback so the
// rdr's "! <listenerIP>" still matches.
return "127.0.0.54"
}
return interceptDNSRdrTarget
}
// hasIPv4DNS reports whether any of the given nameserver strings (bare IPs or
// host:port) is an IPv4 address. Loopback counts: an existing local resolver
// is treated conservatively as an intentional emittable DNS target; ctrld does
// not probe or replace another resolver's ownership.
func hasIPv4DNS(nameservers []string) bool {
for _, s := range nameservers {
host := s
if h, _, err := net.SplitHostPort(s); err == nil {
host = h
}
ip := net.ParseIP(host)
if ip == nil {
continue
}
if ip.To4() != nil {
return true
}
}
return false
}
// needsInterceptDNSTarget reports whether the OS is left without any usable
// IPv4 DNS target: neither the default-route service's static DNS nor the
// discovered (DHCP/scutil) nameservers contain an IPv4 address.
//
// IPv6-only DNS is not usable under DNS intercept mode on macOS: the pf
// ruleset blocks all outbound IPv6 port-53 traffic (IPv6 interception is not
// supported, see issues #507/#533), and with no IPv4 DNS configured
// mDNSResponder emits no DNS packets at all — leaving pf nothing to
// intercept despite a healthy upstream. Observed in production on IPv6-only
// iPhone tethering with 464XLAT (issue #533).
func needsInterceptDNSTarget(staticDNS, discovered []string) bool {
return !hasIPv4DNS(staticDNS) && !hasIPv4DNS(discovered)
}
// isInterceptDNSTargetOnly reports whether the given static DNS list is
// exactly the entry ctrld set via ensureInterceptDNSTarget (recorded in
// target), meaning it is safe for ctrld to remove.
func isInterceptDNSTargetOnly(nameservers []string, target string) bool {
return target != "" && len(nameservers) == 1 && nameservers[0] == target
}
// filterOwnTarget returns nameservers with ctrld's own recorded target
// removed. A previously-set target must never be mistaken for user/network
// IPv4 DNS when judging whether the network still needs one — otherwise the
// second recovery on the same DNS-less network would see "IPv4 DNS present"
// and remove the entry, and the third would re-add it, oscillating on every
// recovery.
func filterOwnTarget(nameservers []string, target string) []string {
if target == "" {
return nameservers
}
out := nameservers[:0:0]
for _, s := range nameservers {
host := s
if h, _, err := net.SplitHostPort(s); err == nil {
host = h
}
if host != target {
out = append(out, s)
}
}
return out
}
+234
View File
@@ -0,0 +1,234 @@
//go:build darwin
package cli
import (
"encoding/json"
"net"
"os"
"tailscale.com/net/netmon"
"github.com/Control-D-Inc/ctrld"
)
// interceptDNSTargetStateFile persists which service/value ctrld set, so a
// daemon restart (crash, upgrade, plain restart) does not orphan the entry:
// without it a restarted daemon would not know the entry is ctrld's own and
// could neither remove it on shutdown nor keep its bookkeeping consistent.
const interceptDNSTargetStateFile = ".intercept_dns_target"
var (
interceptDNSTargetStatePathFn = func() string { return absHomeDir(interceptDNSTargetStateFile) }
interceptDefaultRouteInterfaceFn = netmon.DefaultRouteInterface
interceptInterfaceByNameFn = net.InterfaceByName
interceptPatchNetIfaceNameFn = patchNetIfaceName
interceptCurrentStaticDNSFn = currentStaticDNS
interceptSaveCurrentStaticDNSFn = saveCurrentStaticDNS
interceptSetDNSFn = setDNS
interceptSavedStaticNameserversFn = savedStaticNameservers
interceptResetDNSIgnoreUnusableIfaceFn = resetDnsIgnoreUnusableInterface
interceptDHCPNameserversForInterfaceFn = ctrld.DHCPNameserversForInterface
)
type interceptDNSTargetState struct {
Service string `json:"service"`
Value string `json:"value"`
}
// loadInterceptDNSTargetStateLocked hydrates in-memory tracking from the
// state file once (only when memory is empty). Callers must hold
// interceptDNSTargetMu.
func (p *prog) loadInterceptDNSTargetStateLocked() {
if p.interceptDNSTargetService != "" || p.interceptDNSTargetLoaded {
return
}
p.interceptDNSTargetLoaded = true
data, err := os.ReadFile(interceptDNSTargetStatePathFn())
if err != nil {
return
}
var st interceptDNSTargetState
if err := json.Unmarshal(data, &st); err != nil || st.Service == "" || st.Value == "" {
return
}
p.interceptDNSTargetService = st.Service
p.interceptDNSTargetSetValue = st.Value
mainLog.Load().Debug().Msgf("intercept DNS target: restored tracking of %s on %q from previous run", st.Value, st.Service)
}
// persistInterceptDNSTargetStateLocked writes (or clears) the state file to
// match in-memory tracking. Callers must hold interceptDNSTargetMu.
func (p *prog) persistInterceptDNSTargetStateLocked() {
file := interceptDNSTargetStatePathFn()
if p.interceptDNSTargetService == "" {
_ = os.Remove(file)
return
}
data, err := json.Marshal(interceptDNSTargetState{Service: p.interceptDNSTargetService, Value: p.interceptDNSTargetSetValue})
if err != nil {
return
}
if err := os.WriteFile(file, data, 0600); err != nil {
mainLog.Load().Debug().Err(err).Msg("intercept DNS target: could not persist state file")
}
}
// ensureInterceptDNSTarget guarantees macOS always has an emittable DNS
// target while DNS intercept mode is active.
//
// Intercept mode deliberately never manages interface DNS: pf redirects DNS
// packets in flight. But pf can only redirect packets macOS actually sends,
// and mDNSResponder emits none when the active network service has no DNS
// configured. IPv6-only networks (e.g. iPhone tethering with 464XLAT) supply
// no IPv4 DNS, and the pf ruleset blocks all outbound IPv6 port 53, so such
// networks otherwise end in a total DNS outage with a healthy upstream
// (issue #533).
//
// Only when the default-route service has no usable IPv4 DNS at all does
// ctrld set a loopback DNS value on it — chosen by interceptDNSTargetValue to
// respect the configured listener: the listener IP directly when it serves
// port 53, else a distinct loopback address so the pf lo0 rdr rule rewrites
// to the listener's real port. The entry is removed when the network regains
// IPv4 DNS and on intercept shutdown. Networks that provide IPv4 DNS are
// never modified.
//
// Callers pass a non-nil raw system discovery result to prove discovery ran;
// an empty slice is a valid DNS-less result. The decision itself uses static
// DNS plus DHCP option 6 from the default-route interface, so resolvers on a
// second physical interface cannot suppress the target. Invoked during
// startup, debounced network recovery, and periodic pf watchdog reconciliation.
func (p *prog) ensureInterceptDNSTarget(systemDiscovery []string) {
if !dnsIntercept || p.dnsInterceptState == nil {
return
}
if systemDiscovery == nil {
mainLog.Load().Debug().Msg("intercept DNS target: system DNS discovery was not performed; not changing DNS")
return
}
p.interceptDNSTargetMu.Lock()
defer p.interceptDNSTargetMu.Unlock()
p.loadInterceptDNSTargetStateLocked()
drIfaceName, err := interceptDefaultRouteInterfaceFn()
if err != nil || drIfaceName == "" {
// Mid-transition with no default route; the next recovery decides.
return
}
iface, err := interceptInterfaceByNameFn(drIfaceName)
if err != nil || iface == nil {
return
}
// Resolve the network service name (e.g. en5 -> "iPhone USB") so
// networksetup operates on the right service.
if _, err := interceptPatchNetIfaceNameFn(iface); err != nil {
mainLog.Load().Debug().Err(err).Msgf("intercept DNS target: could not resolve network service for %s", drIfaceName)
return
}
staticDNS, err := interceptCurrentStaticDNSFn(iface)
if err != nil {
// Interfaces without a network service (utun/VPN tunnels) land here:
// networksetup cannot address them, ctrld never writes to them, and
// any target set on the underlying physical service stays in place —
// still correct while ctrld runs.
mainLog.Load().Debug().Err(err).Msgf("intercept DNS target: could not read static DNS for %q", iface.Name)
return
}
// Never count ctrld's own previously-set entry as network-provided DNS,
// or the next recovery on the same DNS-less network would remove it and
// the one after re-add it.
if p.interceptDNSTargetService == iface.Name {
staticDNS = filterOwnTarget(staticDNS, p.interceptDNSTargetSetValue)
}
if hasIPv4DNS(staticDNS) {
p.removeInterceptDNSTargetLocked("network has usable static IPv4 DNS")
return
}
routeDHCPDNS, err := interceptDHCPNameserversForInterfaceFn(drIfaceName)
if err != nil {
mainLog.Load().Debug().Err(err).Msgf("intercept DNS target: could not read DHCP DNS for default-route service %q", iface.Name)
return
}
if hasIPv4DNS(routeDHCPDNS) {
// The default-route service regained DHCP option 6. Remove a target
// previously set on this or another service.
p.removeInterceptDNSTargetLocked("network has usable DHCP IPv4 DNS")
return
}
target := p.interceptDNSTargetValue()
if p.interceptDNSTargetService == iface.Name && p.interceptDNSTargetSetValue == target {
return // already set on this service
}
// Default route moved to a different DNS-less service (or the listener
// config changed): clear the stale entry first.
p.removeInterceptDNSTargetLocked("default route service changed")
// Preserve any existing (IPv6-only) static entries for later restore.
// saveCurrentStaticDNS filters loopback on write, and
// savedStaticNameservers filters loopback on read, so ctrld's own
// loopback target can never be recorded or restored as user DNS.
if err := interceptSaveCurrentStaticDNSFn(iface); err != nil {
mainLog.Load().Debug().Err(err).Msgf("intercept DNS target: could not save static DNS for %q", iface.Name)
}
if err := interceptSetDNSFn(iface, []string{target}); err != nil {
mainLog.Load().Warn().Err(err).Msgf("intercept DNS target: could not set %s on %q", target, iface.Name)
return
}
p.interceptDNSTargetService = iface.Name
p.interceptDNSTargetSetValue = target
p.persistInterceptDNSTargetStateLocked()
mainLog.Load().Warn().Msgf("intercept DNS target: service %q provides no usable IPv4 DNS; set %s so macOS can emit DNS queries (removed automatically when the network provides IPv4 DNS)", iface.Name, target)
}
// removeInterceptDNSTarget removes a previously set intercept DNS target,
// restoring the service's saved static DNS (or empty). Safe no-op when no
// target was set.
func (p *prog) removeInterceptDNSTarget(reason string) {
p.interceptDNSTargetMu.Lock()
defer p.interceptDNSTargetMu.Unlock()
p.loadInterceptDNSTargetStateLocked()
p.removeInterceptDNSTargetLocked(reason)
}
// removeInterceptDNSTargetLocked is removeInterceptDNSTarget without locking;
// callers must hold interceptDNSTargetMu.
func (p *prog) removeInterceptDNSTargetLocked(reason string) {
svc := p.interceptDNSTargetService
val := p.interceptDNSTargetSetValue
if svc == "" {
return
}
iface := &net.Interface{Name: svc}
// Only remove what ctrld set. If the service's DNS changed externally,
// leave that value alone and discard our stale ownership record.
cur, err := interceptCurrentStaticDNSFn(iface)
if err != nil {
mainLog.Load().Debug().Err(err).Msgf("intercept DNS target: could not read %q DNS; retaining cleanup state (%s)", svc, reason)
return
}
if !isInterceptDNSTargetOnly(cur, val) {
mainLog.Load().Debug().Msgf("intercept DNS target: %q DNS changed externally; not removing (%s)", svc, reason)
p.clearInterceptDNSTargetStateLocked()
return
}
if saved := interceptSavedStaticNameserversFn(iface); len(saved) > 0 {
if err := interceptSetDNSFn(iface, saved); err != nil {
mainLog.Load().Warn().Err(err).Msgf("intercept DNS target: could not restore saved DNS on %q; retaining cleanup state", svc)
return
}
} else if err := interceptResetDNSIgnoreUnusableIfaceFn(iface); err != nil {
mainLog.Load().Warn().Err(err).Msgf("intercept DNS target: could not reset DNS on %q; retaining cleanup state", svc)
return
}
p.clearInterceptDNSTargetStateLocked()
mainLog.Load().Info().Msgf("intercept DNS target: removed %s from %q (%s)", val, svc, reason)
}
func (p *prog) clearInterceptDNSTargetStateLocked() {
p.interceptDNSTargetService = ""
p.interceptDNSTargetSetValue = ""
p.persistInterceptDNSTargetStateLocked()
}
+248
View File
@@ -0,0 +1,248 @@
//go:build darwin
package cli
import (
"errors"
"net"
"os"
"path/filepath"
"slices"
"testing"
"github.com/Control-D-Inc/ctrld"
)
type interceptTargetHarness struct {
dns map[string][]string
saved map[string][]string
serviceByDev map[string]string
dhcp []string
dhcpErr error
readErr error
setErr error
resetErr error
setCalls []string
resetCalls []string
statePath string
}
func newInterceptTargetHarness(t *testing.T) *interceptTargetHarness {
t.Helper()
h := &interceptTargetHarness{
dns: make(map[string][]string),
saved: make(map[string][]string),
serviceByDev: map[string]string{"en1": "Wi-Fi"},
statePath: filepath.Join(t.TempDir(), interceptDNSTargetStateFile),
}
origPath := interceptDNSTargetStatePathFn
origRoute := interceptDefaultRouteInterfaceFn
origIface := interceptInterfaceByNameFn
origPatch := interceptPatchNetIfaceNameFn
origCurrent := interceptCurrentStaticDNSFn
origSave := interceptSaveCurrentStaticDNSFn
origSet := interceptSetDNSFn
origSaved := interceptSavedStaticNameserversFn
origReset := interceptResetDNSIgnoreUnusableIfaceFn
origDHCP := interceptDHCPNameserversForInterfaceFn
origIntercept := dnsIntercept
t.Cleanup(func() {
interceptDNSTargetStatePathFn = origPath
interceptDefaultRouteInterfaceFn = origRoute
interceptInterfaceByNameFn = origIface
interceptPatchNetIfaceNameFn = origPatch
interceptCurrentStaticDNSFn = origCurrent
interceptSaveCurrentStaticDNSFn = origSave
interceptSetDNSFn = origSet
interceptSavedStaticNameserversFn = origSaved
interceptResetDNSIgnoreUnusableIfaceFn = origReset
interceptDHCPNameserversForInterfaceFn = origDHCP
dnsIntercept = origIntercept
})
dnsIntercept = true
interceptDNSTargetStatePathFn = func() string { return h.statePath }
interceptDefaultRouteInterfaceFn = func() (string, error) { return "en1", nil }
interceptInterfaceByNameFn = func(name string) (*net.Interface, error) { return &net.Interface{Name: name}, nil }
interceptPatchNetIfaceNameFn = func(iface *net.Interface) (bool, error) {
service, ok := h.serviceByDev[iface.Name]
if !ok {
return false, errors.New("unknown network service")
}
iface.Name = service
return true, nil
}
interceptCurrentStaticDNSFn = func(iface *net.Interface) ([]string, error) {
if h.readErr != nil {
return nil, h.readErr
}
return slices.Clone(h.dns[iface.Name]), nil
}
interceptSaveCurrentStaticDNSFn = func(iface *net.Interface) error {
h.saved[iface.Name] = slices.Clone(h.dns[iface.Name])
return nil
}
interceptSetDNSFn = func(iface *net.Interface, nameservers []string) error {
h.setCalls = append(h.setCalls, iface.Name)
if h.setErr != nil {
return h.setErr
}
h.dns[iface.Name] = slices.Clone(nameservers)
return nil
}
interceptSavedStaticNameserversFn = func(iface *net.Interface) []string {
return slices.Clone(h.saved[iface.Name])
}
interceptResetDNSIgnoreUnusableIfaceFn = func(iface *net.Interface) error {
h.resetCalls = append(h.resetCalls, iface.Name)
if h.resetErr != nil {
return h.resetErr
}
h.dns[iface.Name] = nil
return nil
}
interceptDHCPNameserversForInterfaceFn = func(iface string) ([]string, error) {
if iface != "en1" {
return nil, errors.New("DHCP lookup used a non-default interface")
}
return slices.Clone(h.dhcp), h.dhcpErr
}
return h
}
func newInterceptTargetProg() *prog {
return &prog{
cfg: &ctrld.Config{Listener: map[string]*ctrld.ListenerConfig{
"0": {IP: "127.0.0.1", Port: 5354},
}},
dnsInterceptState: &interceptStateStub{},
}
}
func persistInterceptTargetForTest(t *testing.T, p *prog, service, value string) {
t.Helper()
p.interceptDNSTargetMu.Lock()
defer p.interceptDNSTargetMu.Unlock()
p.interceptDNSTargetLoaded = true
p.interceptDNSTargetService = service
p.interceptDNSTargetSetValue = value
p.persistInterceptDNSTargetStateLocked()
}
func TestEnsureInterceptDNSTargetRequiresCompletedDiscovery(t *testing.T) {
h := newInterceptTargetHarness(t)
p := newInterceptTargetProg()
p.ensureInterceptDNSTarget(nil)
if len(h.setCalls) != 0 || len(h.resetCalls) != 0 {
t.Fatal("nil system discovery changed service DNS")
}
}
func TestEnsureInterceptDNSTargetMigratesService(t *testing.T) {
h := newInterceptTargetHarness(t)
p := newInterceptTargetProg()
persistInterceptTargetForTest(t, p, "iPhone USB", "127.0.0.53")
h.dns["iPhone USB"] = []string{"127.0.0.53"}
h.dns["Wi-Fi"] = nil
p.ensureInterceptDNSTarget([]string{})
if len(h.dns["iPhone USB"]) != 0 {
t.Fatalf("old service DNS = %v, want empty", h.dns["iPhone USB"])
}
if got := h.dns["Wi-Fi"]; !slices.Equal(got, []string{"127.0.0.53"}) {
t.Fatalf("new service DNS = %v, want [127.0.0.53]", got)
}
if p.interceptDNSTargetService != "Wi-Fi" || p.interceptDNSTargetSetValue != "127.0.0.53" {
t.Fatalf("tracking = %q/%q, want Wi-Fi/127.0.0.53", p.interceptDNSTargetService, p.interceptDNSTargetSetValue)
}
}
func TestEnsureInterceptDNSTargetUsesDefaultRouteDHCPOnly(t *testing.T) {
t.Run("other interface IPv4 does not suppress target", func(t *testing.T) {
h := newInterceptTargetHarness(t)
p := newInterceptTargetProg()
p.ensureInterceptDNSTarget([]string{"10.10.10.1"})
if got := h.dns["Wi-Fi"]; !slices.Equal(got, []string{"127.0.0.53"}) {
t.Fatalf("other interface DNS suppressed target: %v", got)
}
})
t.Run("returned default route DHCP removes target", func(t *testing.T) {
h := newInterceptTargetHarness(t)
p := newInterceptTargetProg()
persistInterceptTargetForTest(t, p, "Wi-Fi", "127.0.0.53")
h.dns["Wi-Fi"] = []string{"127.0.0.53"}
h.dhcp = []string{"192.168.10.1"}
p.ensureInterceptDNSTarget([]string{"10.10.10.1"})
if len(h.dns["Wi-Fi"]) != 0 || p.interceptDNSTargetService != "" {
t.Fatalf("returned default-route DHCP DNS did not remove target: dns=%v service=%q", h.dns["Wi-Fi"], p.interceptDNSTargetService)
}
})
}
func TestRemoveInterceptDNSTargetRestoresStateFileAfterRestart(t *testing.T) {
h := newInterceptTargetHarness(t)
h.dns["iPhone USB"] = []string{"127.0.0.53"}
if err := os.WriteFile(h.statePath, []byte(`{"service":"iPhone USB","value":"127.0.0.53"}`), 0600); err != nil {
t.Fatal(err)
}
p := newInterceptTargetProg()
p.removeInterceptDNSTarget("intercept mode inactive")
if len(h.dns["iPhone USB"]) != 0 || p.interceptDNSTargetService != "" {
t.Fatalf("restart cleanup failed: dns=%v service=%q", h.dns["iPhone USB"], p.interceptDNSTargetService)
}
if _, err := os.Stat(h.statePath); !os.IsNotExist(err) {
t.Fatalf("state file still exists after cleanup: %v", err)
}
}
func TestRemoveInterceptDNSTargetKeepsExternalDNS(t *testing.T) {
h := newInterceptTargetHarness(t)
p := newInterceptTargetProg()
persistInterceptTargetForTest(t, p, "Wi-Fi", "127.0.0.53")
h.dns["Wi-Fi"] = []string{"8.8.8.8"}
p.removeInterceptDNSTarget("test")
if !slices.Equal(h.dns["Wi-Fi"], []string{"8.8.8.8"}) || len(h.setCalls) != 0 || len(h.resetCalls) != 0 {
t.Fatalf("external DNS was changed: dns=%v set=%v reset=%v", h.dns["Wi-Fi"], h.setCalls, h.resetCalls)
}
if p.interceptDNSTargetService != "" {
t.Fatal("external change left stale ownership tracking")
}
}
func TestRemoveInterceptDNSTargetRetainsStateOnFailure(t *testing.T) {
for _, tc := range []struct {
name string
readErr error
resetErr error
}{
{"read failure", errors.New("networksetup read failed"), nil},
{"restore failure", nil, errors.New("networksetup reset failed")},
} {
t.Run(tc.name, func(t *testing.T) {
h := newInterceptTargetHarness(t)
p := newInterceptTargetProg()
persistInterceptTargetForTest(t, p, "Wi-Fi", "127.0.0.53")
h.dns["Wi-Fi"] = []string{"127.0.0.53"}
h.readErr = tc.readErr
h.resetErr = tc.resetErr
p.removeInterceptDNSTarget("test")
if p.interceptDNSTargetService != "Wi-Fi" || p.interceptDNSTargetSetValue != "127.0.0.53" {
t.Fatal("failed cleanup discarded retry state")
}
if _, err := os.Stat(h.statePath); err != nil {
t.Fatalf("failed cleanup removed persisted retry state: %v", err)
}
})
}
}
+57
View File
@@ -0,0 +1,57 @@
package cli
import "testing"
func TestFilterOwnTarget(t *testing.T) {
tests := []struct {
name string
in []string
target string
wantLen int
}{
// The oscillation guard (MR !997 review): the second recovery on the
// same DNS-less network must not count ctrld's own entry as
// network-provided IPv4 DNS.
{"removes own entry", []string{"127.0.0.1"}, "127.0.0.1", 0},
{"removes own entry with resolver port", []string{"127.0.0.53:53"}, "127.0.0.53", 0},
{"keeps user entries", []string{"127.0.0.1", "1.1.1.1"}, "127.0.0.1", 1},
{"empty target keeps all", []string{"127.0.0.1"}, "", 1},
{"no match keeps all", []string{"1.1.1.1"}, "127.0.0.53", 1},
{"nil input", nil, "127.0.0.1", 0},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
got := filterOwnTarget(tc.in, tc.target)
if len(got) != tc.wantLen {
t.Errorf("filterOwnTarget(%v, %q) = %v, want len %d", tc.in, tc.target, got, tc.wantLen)
}
for _, s := range got {
if tc.target != "" && s == tc.target {
t.Errorf("filterOwnTarget(%v, %q) retained the target entry", tc.in, tc.target)
}
}
})
}
}
// TestFilterOwnTargetStability pins the recovery-cycle contract: on a
// DNS-less network where ctrld already set its target, needsInterceptDNSTarget
// over the filtered list must still report true (entry kept, no oscillation),
// while a genuine user-added IPv4 server must report false (entry removed).
func TestFilterOwnTargetStability(t *testing.T) {
target := "127.0.0.1"
// Second recovery, same tether: only our own entry present. The OS resolver
// reports it with :53, while networksetup reports the bare address.
static := filterOwnTarget([]string{target}, target)
discovered := filterOwnTarget([]string{target + ":53"}, target)
if !needsInterceptDNSTarget(static, discovered) {
t.Error("second recovery on the same DNS-less network would remove the target (oscillation)")
}
// User manually added a public server meanwhile: target no longer needed.
static = filterOwnTarget([]string{target, "1.1.1.1"}, target)
if needsInterceptDNSTarget(static, nil) {
t.Error("user-added IPv4 DNS not recognized; target would be kept unnecessarily")
}
}
+14
View File
@@ -0,0 +1,14 @@
//go:build !darwin
package cli
// ensureInterceptDNSTarget is a no-op on non-Darwin platforms: the DNS-less
// network problem it solves is specific to macOS pf interception blocking
// IPv6 port 53 with no IPv4 fallback (issue #533). Windows intercept mode
// uses NRPT, which routes queries regardless of adapter DNS configuration.
func (p *prog) ensureInterceptDNSTarget(_ []string) {}
// removeInterceptDNSTarget is a no-op on non-Darwin platforms.
//
//lint:ignore U1000 called from Darwin-only intercept shutdown; kept for API symmetry.
func (p *prog) removeInterceptDNSTarget(_ string) {}
+113
View File
@@ -0,0 +1,113 @@
package cli
import (
"testing"
"github.com/Control-D-Inc/ctrld"
)
func TestHasIPv4DNS(t *testing.T) {
tests := []struct {
name string
in []string
want bool
}{
{"empty", nil, false},
{"ipv4", []string{"8.8.8.8"}, true},
{"ipv4 with port", []string{"192.168.1.1:53"}, true},
{"loopback counts", []string{"127.0.0.1"}, true},
{"ipv6 only", []string{"2001:4860:4860::8888"}, false},
{"ipv6 with port", []string{"[2001:4860:4860::8888]:53"}, false},
{"mixed", []string{"2001:4860:4860::8888", "9.9.9.9"}, true},
{"garbage ignored", []string{"not-an-ip", ""}, false},
{"garbage plus v4", []string{"not-an-ip", "1.1.1.1"}, true},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if got := hasIPv4DNS(tc.in); got != tc.want {
t.Errorf("hasIPv4DNS(%v) = %v, want %v", tc.in, got, tc.want)
}
})
}
}
func TestNeedsInterceptDNSTarget(t *testing.T) {
tests := []struct {
name string
static, discovered []string
want bool
}{
{"no dns at all", nil, nil, true},
{"ipv6-only tether (464XLAT, issue #533)", nil, []string{"2605:8d80::1"}, true},
{"static v4 present", []string{"1.1.1.1"}, nil, false},
{"discovered v4 present", nil, []string{"192.168.1.1:53"}, false},
{"existing ctrld target satisfies", []string{"127.0.0.1"}, nil, false},
{"ipv6 static, v4 discovered", []string{"2001:db8::1"}, []string{"10.0.0.1"}, false},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if got := needsInterceptDNSTarget(tc.static, tc.discovered); got != tc.want {
t.Errorf("needsInterceptDNSTarget(%v, %v) = %v, want %v", tc.static, tc.discovered, got, tc.want)
}
})
}
}
func TestIsInterceptDNSTargetOnly(t *testing.T) {
tests := []struct {
name string
in []string
target string
want bool
}{
{"exactly ours (direct listener)", []string{"127.0.0.1"}, "127.0.0.1", true},
{"exactly ours (rdr target)", []string{"127.0.0.53"}, "127.0.0.53", true},
{"empty list", nil, "127.0.0.1", false},
{"empty target never matches", []string{"127.0.0.1"}, "", false},
{"ours plus user entry", []string{"127.0.0.1", "1.1.1.1"}, "127.0.0.1", false},
{"user entry only", []string{"1.1.1.1"}, "127.0.0.1", false},
{"different loopback than ours", []string{"127.0.0.53"}, "127.0.0.1", false},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if got := isInterceptDNSTargetOnly(tc.in, tc.target); got != tc.want {
t.Errorf("isInterceptDNSTargetOnly(%v, %q) = %v, want %v", tc.in, tc.target, got, tc.want)
}
})
}
}
func TestInterceptDNSTargetValue(t *testing.T) {
tests := []struct {
name string
ip string
port int
want string
}{
{"default direct listener :53", "127.0.0.1", 53, "127.0.0.1"},
{"custom loopback listener :53", "127.0.0.2", 53, "127.0.0.2"},
{"non-53 port uses rdr target", "127.0.0.1", 5354, "127.0.0.53"},
{"listener on rdr target with non-53 port", "127.0.0.53", 5354, "127.0.0.54"},
{"wildcard ip :53 falls back to loopback", "0.0.0.0", 53, "127.0.0.1"},
{"wildcard ip non-53 uses rdr target", "0.0.0.0", 5354, "127.0.0.53"},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
p := &prog{cfg: &ctrld.Config{
Listener: map[string]*ctrld.ListenerConfig{
"0": {IP: tc.ip, Port: tc.port},
},
}}
if got := p.interceptDNSTargetValue(); got != tc.want {
t.Errorf("interceptDNSTargetValue() with listener %s:%d = %q, want %q", tc.ip, tc.port, got, tc.want)
}
})
}
}
func TestInterceptDNSTargetValue_NoListener(t *testing.T) {
p := &prog{cfg: &ctrld.Config{}}
if got := p.interceptDNSTargetValue(); got != "127.0.0.1" {
t.Errorf("interceptDNSTargetValue() with no listener = %q, want 127.0.0.1", got)
}
}
+44 -8
View File
@@ -156,6 +156,10 @@ type prog struct {
recoveryCancelMu sync.Mutex
recoveryCancel context.CancelFunc
recoveryRunning atomic.Bool
// recoveryGen counts handleRecovery invocations that reached the
// recovery-context stage; each recovery captures its own generation and
// only touches shared recovery state if it is still the newest (#597).
recoveryGen atomic.Uint64
// recoveryDebounceTimer coalesces rapid NetworkChange recovery triggers
// into a single handleRecovery call. Only handleRecovery is debounced —
@@ -168,6 +172,21 @@ type prog struct {
// instead of using the normal upstream flow.
recoveryBypass atomic.Bool
// interceptDNSTargetService names the macOS network service on which
// ctrld set a loopback DNS value because the service provided no usable
// IPv4 DNS while DNS intercept mode was active (issue #533);
// interceptDNSTargetSetValue records the exact value set. Both empty when
// no target is set. Guarded by interceptDNSTargetMu.
//
//lint:ignore U1000 used in Darwin code.
interceptDNSTargetMu sync.Mutex
//lint:ignore U1000 used in Darwin code.
interceptDNSTargetService string
//lint:ignore U1000 used in Darwin code.
interceptDNSTargetSetValue string
//lint:ignore U1000 used in Darwin code.
interceptDNSTargetLoaded bool
// DNS intercept mode state (platform-specific).
// On Windows: *wfpState, on macOS: *pfState, nil on other platforms.
dnsInterceptState any
@@ -453,9 +472,9 @@ func (p *prog) postRun() {
if !p.skipInitialDNSReset() {
p.resetDNS(false, false)
}
ns := ctrld.InitializeOsResolver(false)
ns, systemNameservers := initializeOsResolverWithSystemNameserversFn(false)
mainLog.Load().Debug().Msgf("initialized OS resolver with nameservers: %v", ns)
p.setDNS()
p.setDNS(systemNameservers)
p.csSetDnsDone <- struct{}{}
close(p.csSetDnsDone)
p.logInterfacesState()
@@ -891,11 +910,14 @@ func (p *prog) deAllocateIP() error {
// NRPT rule, so a test of what happens *after* it fails must not be the thing that
// runs it.
var (
localResolverIPFn = router.LocalResolverIP
startDNSInterceptFn = (*prog).startDNSIntercept
setDnsForRunningIfaceFn = (*prog).setDnsForRunningIface
resetDNSFn = (*prog).resetDNS
refuseFallbackFatal = func(format string, v ...any) {
localResolverIPFn = router.LocalResolverIP
startDNSInterceptFn = (*prog).startDNSIntercept
ensureInterceptDNSTargetFn = (*prog).ensureInterceptDNSTarget
removeInterceptDNSTargetFn = (*prog).removeInterceptDNSTarget
initializeOsResolverWithSystemNameserversFn = ctrld.InitializeOsResolverWithSystemNameservers
setDnsForRunningIfaceFn = (*prog).setDnsForRunningIface
resetDNSFn = (*prog).resetDNS
refuseFallbackFatal = func(format string, v ...any) {
mainLog.Load().Fatal().Msgf(format, v...)
}
)
@@ -922,7 +944,7 @@ func interfaceDNSFallbackViable(lc *ctrld.ListenerConfig, localResolverIP string
return lc == nil || lc.Port == 0 || lc.Port == 53 || localResolverIP != ""
}
func (p *prog) setDNS() {
func (p *prog) setDNS(systemNameservers []string) {
setDnsOK := false
defer func() {
p.csSetDnsOk = setDnsOK
@@ -956,6 +978,7 @@ func (p *prog) setDNS() {
// software that also manages DNS. See issue #489.
if dnsIntercept {
if err := startDNSInterceptFn(p); err != nil {
removeInterceptDNSTargetFn(p, "DNS intercept unavailable")
// This check comes first: it is the one failure where DNS already works
// without ctrld touching anything else, so neither the refusal below nor the
// fallback applies.
@@ -1006,6 +1029,12 @@ func (p *prog) setDNS() {
mainLog.Load().Error().Err(err).Msg("DNS intercept mode failed — falling back to interface DNS settings")
// Fall through to traditional setDNS behavior.
} else {
// Intercept installation alone is insufficient on a DNS-less network:
// without an IPv4 DNS target macOS emits no packet for pf to redirect.
// Do this on startup as well as network-change recovery so starting or
// restarting while already tethered cannot leave DNS offline.
ensureInterceptDNSTargetFn(p, systemNameservers)
if hardIntercept {
mainLog.Load().Info().Msg("Hard intercept mode active — all DNS through ctrld, no VPN split routing")
} else {
@@ -1023,6 +1052,9 @@ func (p *prog) setDNS() {
return
}
}
if !dnsIntercept {
removeInterceptDNSTargetFn(p, "intercept mode inactive")
}
if cfg.Listener == nil {
return
@@ -1258,6 +1290,10 @@ func (p *prog) dnsWatchdog(iface *net.Interface, nameservers []string) {
// resetDNS performs a DNS reset for all interfaces.
// In DNS intercept mode, this tears down the WFP/pf filters instead.
func (p *prog) resetDNS(isStart bool, restoreStatic bool) {
// A previous crash can leave a persisted macOS intercept target even when
// no live interceptor state exists. Cleanup must run for stop/uninstall and
// traditional-mode startup as well as the normal intercept shutdown path.
removeInterceptDNSTargetFn(p, "DNS reset")
if dnsIntercept && p.dnsInterceptState != nil {
if err := p.stopDNSIntercept(); err != nil {
mainLog.Load().Error().Err(err).Msg("Failed to stop DNS intercept mode during reset")
+41 -3
View File
@@ -91,9 +91,12 @@ func TestInterfaceDNSFallbackViable(t *testing.T) {
// than depending on the runner denying a privileged operation.
type interceptFallbackHarness struct {
interceptCalls int
ensureTargetCalls int
ensuredNameservers []string
installedNameservers []string
installCalls int
resetCalls int
removeTargetCalls int
refusals []string
}
@@ -101,12 +104,12 @@ func newInterceptFallbackHarness(t *testing.T, lc *ctrld.ListenerConfig) *interc
t.Helper()
h := &interceptFallbackHarness{}
origStart, origInstall := startDNSInterceptFn, setDnsForRunningIfaceFn
origStart, origEnsure, origRemove, origInstall := startDNSInterceptFn, ensureInterceptDNSTargetFn, removeInterceptDNSTargetFn, setDnsForRunningIfaceFn
origReset, origFatal := resetDNSFn, refuseFallbackFatal
origResolver := localResolverIPFn
origCfg, origMode, origIntercept, origHard := cfg, interceptMode, dnsIntercept, hardIntercept
t.Cleanup(func() {
startDNSInterceptFn, setDnsForRunningIfaceFn = origStart, origInstall
startDNSInterceptFn, ensureInterceptDNSTargetFn, removeInterceptDNSTargetFn, setDnsForRunningIfaceFn = origStart, origEnsure, origRemove, origInstall
resetDNSFn, refuseFallbackFatal = origReset, origFatal
localResolverIPFn = origResolver
cfg, interceptMode, dnsIntercept, hardIntercept = origCfg, origMode, origIntercept, origHard
@@ -121,6 +124,11 @@ func newInterceptFallbackHarness(t *testing.T, lc *ctrld.ListenerConfig) *interc
h.interceptCalls++
return errors.New("dns intercept: injected start failure")
}
ensureInterceptDNSTargetFn = func(_ *prog, nameservers []string) {
h.ensureTargetCalls++
h.ensuredNameservers = slices.Clone(nameservers)
}
removeInterceptDNSTargetFn = func(_ *prog, _ string) { h.removeTargetCalls++ }
setDnsForRunningIfaceFn = func(_ *prog, nameservers []string) *net.Interface {
h.installCalls++
h.installedNameservers = nameservers
@@ -143,7 +151,31 @@ func newInterceptFallbackHarness(t *testing.T, lc *ctrld.ListenerConfig) *interc
func (h *interceptFallbackHarness) run(t *testing.T) {
t.Helper()
p := &prog{cfg: &cfg}
p.setDNS()
p.setDNS(nil)
}
func TestSetDNSEnsuresInterceptTargetAfterSuccessfulStart(t *testing.T) {
h := newInterceptFallbackHarness(t, &ctrld.ListenerConfig{IP: "127.0.0.1", Port: 5354})
startDNSInterceptFn = func(_ *prog) error {
h.interceptCalls++
return nil
}
want := []string{"fe80::1"}
p := &prog{cfg: &cfg}
p.setDNS(want)
if h.interceptCalls != 1 {
t.Fatalf("intercept start called %d time(s), want 1", h.interceptCalls)
}
if h.ensureTargetCalls != 1 {
t.Fatalf("intercept DNS target ensured %d time(s), want 1 after successful start", h.ensureTargetCalls)
}
if !slices.Equal(h.ensuredNameservers, want) {
t.Fatalf("system nameservers = %v, want %v", h.ensuredNameservers, want)
}
if h.installCalls != 0 {
t.Fatalf("interface-DNS fallback installed %d time(s) after successful intercept start", h.installCalls)
}
}
func TestSetDNSExplicitOffOverridesConfig(t *testing.T) {
@@ -160,6 +192,9 @@ func TestSetDNSExplicitOffOverridesConfig(t *testing.T) {
if h.installCalls != 1 {
t.Fatalf("interface DNS installed %d time(s), want 1", h.installCalls)
}
if h.removeTargetCalls != 1 {
t.Fatalf("stale intercept DNS target cleanup called %d time(s), want 1", h.removeTargetCalls)
}
}
// TestSetDNSRefusesUnreachableFallback is the behaviour test for the reported outage: it
@@ -182,6 +217,9 @@ func TestSetDNSRefusesUnreachableFallback(t *testing.T) {
if h.resetCalls == 0 {
t.Error("host DNS was not restored before refusing, leaving the interface pointed at a ctrld that is not serving")
}
if h.removeTargetCalls != 1 {
t.Errorf("stale intercept DNS target cleanup called %d time(s), want 1 after intercept failure", h.removeTargetCalls)
}
if len(h.refusals) == 0 {
t.Fatal("refusal was not surfaced: startup must fail loudly rather than silently skip the fallback")
}
+75
View File
@@ -0,0 +1,75 @@
package cli
import "context"
// beginRecovery atomically transfers ownership of shared recovery state. A
// network change cancels and replaces the current owner without exposing a nil
// recoveryCancel gap; other triggers are coalesced while an owner exists.
func (p *prog) beginRecovery(reason RecoveryReason) (ctx context.Context, gen uint64, intercept bool, ok bool) {
p.recoveryCancelMu.Lock()
defer p.recoveryCancelMu.Unlock()
if reason != RecoveryReasonNetworkChange && p.recoveryCancel != nil {
return nil, 0, false, false
}
if p.recoveryCancel != nil {
p.recoveryCancel()
}
ctx, cancel := context.WithCancel(context.Background())
gen = p.recoveryGen.Add(1)
intercept = dnsIntercept && p.dnsInterceptState != nil
p.recoveryCancel = cancel
p.recoveryRunning.Store(true)
p.recoveryBypass.Store(intercept)
return ctx, gen, intercept, true
}
func (p *prog) recoveryOwnsState(gen uint64) bool {
p.recoveryCancelMu.Lock()
defer p.recoveryCancelMu.Unlock()
return p.recoveryGen.Load() == gen && p.recoveryCancel != nil
}
func systemNameserversForInterceptRetry() []string {
_, system := initializeOsResolverWithSystemNameserversFn(true)
if system == nil {
return []string{}
}
return system
}
// completeRecovery releases shared state only if gen still owns it. The bypass
// reset is unconditional because live intercept state can disappear while a
// recovery is running, but a stale true flag still affects proxy routing.
func (p *prog) completeRecovery(gen uint64) bool {
p.recoveryCancelMu.Lock()
defer p.recoveryCancelMu.Unlock()
if p.recoveryGen.Load() != gen || p.recoveryCancel == nil {
return false
}
p.recoveryBypass.Store(false)
p.recoveryRunning.Store(false)
p.recoveryCancel = nil
return true
}
// recoveryCanceledCleanup resets shared recovery state after a canceled or
// failed recovery, but only when the recovery identified by gen was NOT
// superseded by a newer one (issue #597).
//
// A network-change cancellation is normally followed immediately by a new
// handleRecovery that owns recoveryBypass/recoveryRunning/recoveryCancel;
// clearing them here would disable the successor's bypass mid-flight and
// make it uncancellable. But when the canceled recovery is the LAST one
// (e.g. the tail of a network flap burst), nothing else will ever clear the
// flags: the daemon would stay in recovery bypass forever — every query
// detouring to the OS resolver — and the DNS-settings watchdog would stay
// permanently disabled.
func (p *prog) recoveryCanceledCleanup(gen uint64) {
if !p.completeRecovery(gen) {
// Superseded: the newer recovery owns the shared state.
return
}
mainLog.Load().Info().Msg("Recovery canceled with no successor; cleared recovery state and DHCP bypass")
}
+161
View File
@@ -0,0 +1,161 @@
package cli
import (
"testing"
"time"
)
// interceptStateStub stands in for the platform pfState/wfpState; the
// recovery cleanup path only checks dnsInterceptState != nil.
type interceptStateStub struct{}
// setupInterceptRecovery puts p into "intercept-mode recovery in flight"
// state and restores the package-level dnsIntercept flag on cleanup.
func setupInterceptRecovery(t *testing.T, p *prog) {
t.Helper()
oldIntercept := dnsIntercept
dnsIntercept = true
t.Cleanup(func() { dnsIntercept = oldIntercept })
p.dnsInterceptState = &interceptStateStub{}
p.recoveryBypass.Store(true)
p.recoveryRunning.Store(true)
p.recoveryCancel = func() {}
}
// TestRecoveryCanceledCleanup_LastRecoveryResetsState pins issue #597: a
// canceled recovery with no successor must clear recoveryBypass and
// recoveryRunning, or the daemon stays in bypass forever (every query
// detours to the OS resolver) and the DNS watchdog stays disabled.
func TestRecoveryCanceledCleanup_LastRecoveryResetsState(t *testing.T) {
p := &prog{}
setupInterceptRecovery(t, p)
gen := p.recoveryGen.Add(1)
p.recoveryCanceledCleanup(gen)
if p.recoveryBypass.Load() {
t.Error("recoveryBypass still set after canceled recovery with no successor")
}
if p.recoveryRunning.Load() {
t.Error("recoveryRunning still set after canceled recovery with no successor")
}
p.recoveryCancelMu.Lock()
cancelCleared := p.recoveryCancel == nil
p.recoveryCancelMu.Unlock()
if !cancelCleared {
t.Error("recoveryCancel not cleared after canceled recovery with no successor")
}
}
// TestRecoveryCanceledCleanup_SupersededKeepsSuccessorState pins the
// captive-portal/network-flap contract: when a newer recovery superseded the
// canceled one, the canceled recovery must NOT clear shared state — the
// successor owns bypass for its own duration.
func TestRecoveryCanceledCleanup_SupersededKeepsSuccessorState(t *testing.T) {
p := &prog{}
setupInterceptRecovery(t, p)
gen := p.recoveryGen.Add(1)
// A successor recovery started.
p.recoveryGen.Add(1)
p.recoveryCanceledCleanup(gen)
if !p.recoveryBypass.Load() {
t.Error("superseded canceled recovery cleared recoveryBypass owned by its successor")
}
if !p.recoveryRunning.Load() {
t.Error("superseded canceled recovery cleared recoveryRunning owned by its successor")
}
p.recoveryCancelMu.Lock()
cancelKept := p.recoveryCancel != nil
p.recoveryCancelMu.Unlock()
if !cancelKept {
t.Error("superseded canceled recovery cleared the successor's recoveryCancel")
}
}
// TestRecoveryCanceledCleanup_NonInterceptResetsRunning covers traditional
// (non-intercept) mode: recoveryRunning must still be reset so watchdogs
// resume, while bypass is untouched (it is never set in that mode).
func TestRecoveryCanceledCleanup_NonInterceptResetsRunning(t *testing.T) {
oldIntercept := dnsIntercept
dnsIntercept = false
t.Cleanup(func() { dnsIntercept = oldIntercept })
p := &prog{}
p.recoveryRunning.Store(true)
p.recoveryCancel = func() {}
gen := p.recoveryGen.Add(1)
p.recoveryCanceledCleanup(gen)
if p.recoveryRunning.Load() {
t.Error("recoveryRunning still set after canceled non-intercept recovery")
}
}
func TestBeginRecoveryTransfersOwnershipAtomically(t *testing.T) {
oldIntercept := dnsIntercept
dnsIntercept = true
t.Cleanup(func() { dnsIntercept = oldIntercept })
p := &prog{dnsInterceptState: &interceptStateStub{}}
firstCtx, firstGen, _, ok := p.beginRecovery(RecoveryReasonRegularFailure)
if !ok {
t.Fatal("first recovery did not acquire ownership")
}
if _, _, _, ok := p.beginRecovery(RecoveryReasonRegularFailure); ok {
t.Fatal("duplicate upstream recovery acquired ownership")
}
_, successorGen, intercept, ok := p.beginRecovery(RecoveryReasonNetworkChange)
if !ok || !intercept || successorGen <= firstGen {
t.Fatalf("network recovery did not replace owner: first=%d successor=%d intercept=%v ok=%v", firstGen, successorGen, intercept, ok)
}
select {
case <-firstCtx.Done():
case <-time.After(time.Second):
t.Fatal("successor did not cancel the previous recovery")
}
p.recoveryCanceledCleanup(firstGen)
if !p.recoveryRunning.Load() || !p.recoveryBypass.Load() || !p.recoveryOwnsState(successorGen) {
t.Fatal("stale cleanup changed successor-owned recovery state")
}
if !p.completeRecovery(successorGen) {
t.Fatal("successor could not complete its own recovery state")
}
}
func TestRecoveryCleanupClearsBypassAfterInterceptStateDisappears(t *testing.T) {
p := &prog{}
p.recoveryBypass.Store(true)
p.recoveryRunning.Store(true)
p.recoveryCancel = func() {}
gen := p.recoveryGen.Add(1)
p.recoveryCanceledCleanup(gen)
if p.recoveryBypass.Load() || p.recoveryRunning.Load() {
t.Fatal("cleanup retained recovery flags after intercept state disappeared")
}
}
func TestSystemNameserversForInterceptRetryNormalizesEmptyDiscovery(t *testing.T) {
original := initializeOsResolverWithSystemNameserversFn
called := false
initializeOsResolverWithSystemNameserversFn = func(guard bool) ([]string, []string) {
called = true
if !guard {
t.Error("intercept retry discovery did not guard the existing resolver")
}
return nil, nil
}
t.Cleanup(func() { initializeOsResolverWithSystemNameserversFn = original })
if got := systemNameserversForInterceptRetry(); got == nil || len(got) != 0 {
t.Fatalf("system discovery = %#v, want non-nil empty slice", got)
}
if !called {
t.Fatal("system discovery was not called")
}
}
+19 -15
View File
@@ -10,7 +10,6 @@ import (
"io"
"net"
"os/exec"
"regexp"
"runtime"
"slices"
"strings"
@@ -101,24 +100,29 @@ func getDHCPNameservers(iface string) ([]string, error) {
return nil, fmt.Errorf("ipconfig not available on mobile")
}
// Run the ipconfig command for the given interface.
cmd := exec.Command("ipconfig", "getpacket", iface)
output, err := cmd.Output()
if err != nil {
return nil, fmt.Errorf("error running ipconfig: %v", err)
// getoption returns the selected interface's DHCP option directly and does
// not expose unrelated packet addresses to the parser.
output, err := exec.Command("ipconfig", "getoption", iface, "domain_name_server").Output()
if err == nil {
return parseDHCPOptionNameservers(output), nil
}
// Look for a line like:
// domain_name_servers = 192.168.1.1 8.8.8.8;
re := regexp.MustCompile(`domain_name_servers\s*=\s*(.*);`)
matches := re.FindStringSubmatch(string(output))
if len(matches) < 2 {
return nil, fmt.Errorf("no DHCP nameservers found")
// Older macOS releases can fail getoption while still exposing the packet.
// Parse the real macOS field shape, for example:
// domain_name_server (ip_mult): {192.168.1.1, 8.8.8.8}
output, packetErr := exec.Command("ipconfig", "getpacket", iface).Output()
if packetErr != nil {
if err != nil {
return nil, fmt.Errorf("error reading DHCP DNS option: getoption: %v; getpacket: %v", err, packetErr)
}
return nil, fmt.Errorf("error reading DHCP packet: %v", packetErr)
}
return parseDHCPPacketNameservers(output), nil
}
// Split the nameservers by whitespace.
nameservers := strings.Fields(matches[1])
return nameservers, nil
// DHCPNameserversForInterface returns DHCP option 6 for exactly iface.
func DHCPNameserversForInterface(iface string) ([]string, error) {
return getDHCPNameservers(iface)
}
func getAllDHCPNameservers() []string {
+44
View File
@@ -0,0 +1,44 @@
package ctrld
import (
"net"
"strings"
"unicode"
)
func parseDHCPOptionNameservers(output []byte) []string {
return parseIPv4Nameservers(string(output))
}
func parseDHCPPacketNameservers(output []byte) []string {
for _, line := range strings.Split(string(output), "\n") {
field := strings.TrimSpace(line)
if strings.HasPrefix(field, "domain_name_server ") ||
strings.HasPrefix(field, "domain_name_server:") ||
strings.HasPrefix(field, "domain_name_servers ") ||
strings.HasPrefix(field, "domain_name_servers:") {
return parseIPv4Nameservers(field)
}
}
return nil
}
func parseIPv4Nameservers(value string) []string {
seen := make(map[string]struct{})
var nameservers []string
for _, token := range strings.FieldsFunc(value, func(r rune) bool {
return r != '.' && !unicode.IsDigit(r)
}) {
ip := net.ParseIP(token)
if ip == nil || ip.To4() == nil {
continue
}
ns := ip.String()
if _, ok := seen[ns]; ok {
continue
}
seen[ns] = struct{}{}
nameservers = append(nameservers, ns)
}
return nameservers
}
+64
View File
@@ -0,0 +1,64 @@
package ctrld
import (
"slices"
"testing"
)
func TestParseDHCPOptionNameservers(t *testing.T) {
tests := []struct {
name string
output string
want []string
}{
{"single", "192.168.10.1\n", []string{"192.168.10.1"}},
{"multiple", "192.168.10.1\n1.1.1.1\n", []string{"192.168.10.1", "1.1.1.1"}},
{"deduplicate and reject invalid", "192.168.10.1 999.1.1.1 192.168.10.1", []string{"192.168.10.1"}},
{"empty", "", nil},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if got := parseDHCPOptionNameservers([]byte(tc.output)); !slices.Equal(got, tc.want) {
t.Fatalf("parseDHCPOptionNameservers() = %v, want %v", got, tc.want)
}
})
}
}
func TestParseDHCPPacketNameservers(t *testing.T) {
tests := []struct {
name string
output string
want []string
}{
{
name: "macos singular ip_mult",
output: `op = BOOTREPLY
` +
`yiaddr = 192.168.10.155
` +
`domain_name_server (ip_mult): {192.168.10.1, 1.1.1.1}
` +
`server_identifier (ip): 192.168.10.1
`,
want: []string{"192.168.10.1", "1.1.1.1"},
},
{
name: "legacy plural equals",
output: "domain_name_servers = 192.168.1.1 8.8.8.8;\n",
want: []string{"192.168.1.1", "8.8.8.8"},
},
{
name: "packet addresses without option are ignored",
output: "yiaddr = 192.168.10.155\nserver_identifier (ip): 192.168.10.1\n",
want: nil,
},
}
for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if got := parseDHCPPacketNameservers([]byte(tc.output)); !slices.Equal(got, tc.want) {
t.Fatalf("parseDHCPPacketNameservers() = %v, want %v", got, tc.want)
}
})
}
}
+28 -7
View File
@@ -143,17 +143,38 @@ func availableNameservers() []string {
// It's the caller's responsibility to ensure the system DNS is in a clean state before
// calling this function.
func InitializeOsResolver(guardAgainstNoNameservers bool) []string {
ns, _ := InitializeOsResolverWithSystemNameservers(guardAgainstNoNameservers)
return ns
}
// InitializeOsResolverWithSystemNameservers initializes the OS resolver and
// returns both the effective resolver list and the unmodified nameservers
// discovered from the system. The latter deliberately excludes synthetic
// fallbacks added by initializeOsResolver.
func InitializeOsResolverWithSystemNameservers(guardAgainstNoNameservers bool) (effective, system []string) {
resolverMutex.Lock()
defer resolverMutex.Unlock()
nameservers := availableNameservers()
// if no nameservers, return empty slice so we dont remove all nameservers
if len(nameservers) == 0 && guardAgainstNoNameservers {
return []string{}
system = availableNameservers()
if system == nil {
// A non-nil empty slice means discovery completed and found no DNS.
// Callers use nil to mean that discovery was not attempted.
system = []string{}
}
ns := initializeOsResolver(nameservers)
or = newResolverWithNameserver(ns)
return ns
effective, system, skip := osResolverNameserverSets(system, guardAgainstNoNameservers)
if skip {
return effective, system
}
or = newResolverWithNameserver(effective)
return effective, system
}
func osResolverNameserverSets(system []string, guardAgainstNoNameservers bool) (effective, discovered []string, skip bool) {
// if no nameservers, return empty slice so we dont remove all nameservers
if len(system) == 0 && guardAgainstNoNameservers {
return []string{}, system, true
}
return initializeOsResolver(system), system, false
}
// initializeOsResolver performs logic for choosing OS resolver nameserver.
+25
View File
@@ -392,6 +392,31 @@ func Test_osResolver_ResolveWithNonSuccessAnswer(t *testing.T) {
}
}
func TestOSResolverNameserverSetsKeepsSyntheticFallbackOutOfSystemDiscovery(t *testing.T) {
system := []string{"fe80::1"}
effective, discovered, skip := osResolverNameserverSets(system, false)
if skip {
t.Fatal("non-empty discovery unexpectedly skipped resolver replacement")
}
if len(discovered) != 1 || discovered[0] != system[0] {
t.Fatalf("discovered nameservers = %v, want raw system list %v", discovered, system)
}
if len(effective) != 2 || effective[0] != "[fe80::1]:53" || effective[1] != controldPublicDnsWithPort {
t.Fatalf("effective nameservers = %v, want IPv6 system resolver plus synthetic fallback", effective)
}
}
func TestOSResolverNameserverSetsHonorsEmptyGuard(t *testing.T) {
effective, discovered, skip := osResolverNameserverSets(nil, true)
if len(effective) != 0 || len(discovered) != 0 {
t.Fatalf("guarded empty discovery returned effective=%v discovered=%v", effective, discovered)
}
if !skip {
t.Fatal("guarded empty discovery did not return the skip decision")
}
}
func Test_osResolver_InitializationRace(t *testing.T) {
var wg sync.WaitGroup
n := 10
+53
View File
@@ -0,0 +1,53 @@
#!/bin/sh
set -eu
iface=$(/sbin/route -n get default 2>/dev/null | /usr/bin/awk '/interface:/{print $2; exit}')
printf 'default_interface=%s\n' "$iface"
printf 'ipv4_address='
/usr/sbin/ipconfig getifaddr "$iface" 2>/dev/null || printf '<none>\n'
printf 'dhcp_ipv4_dns='
/usr/sbin/ipconfig getoption "$iface" domain_name_server 2>/dev/null |
/usr/bin/awk '
{
for (i = 1; i <= NF; i++) {
value = $i
gsub(/[{},;]/, "", value)
if (value ~ /^([0-9]{1,3}\.){3}[0-9]{1,3}$/ && !seen[value]++) {
if (found) {
printf ","
}
printf "%s", value
found = 1
}
}
}
END {
if (!found) {
printf "<none>"
}
printf "\n"
}
'
printf 'effective_ipv4_dns='
/usr/sbin/scutil --dns |
/usr/bin/awk '
/nameserver\[[0-9]+\] : [0-9]+\./ {
if (!seen[$3]++) {
if (found) {
printf ","
}
printf "%s", $3
found = 1
}
}
END {
if (!found) {
printf "<none>"
}
printf "\n"
}
'