Release branch v1.5.7

cmd/cli/cli.go

@@ -385,7 +385,7 @@ func run(appCallback *AppCallback, stopCh chan struct{}) {
 	// would silently re-enable interception from config.
 	if updateConfigInterceptMode(&cfg, interceptMode) {
 		updated = true
-		mainLog.Load().Info().Msgf("writing intercept_mode = %q to config", cfg.Service.InterceptMode)
+		mainLog.Load().Info().Msgf("writing intercept_mode = %q to config (requested %q)", cfg.Service.InterceptMode, interceptMode)
 	}
 
 	if updated {

cmd/cli/commands.go

@@ -275,7 +275,7 @@ func initRunCmd() *cobra.Command {
 	_ = runCmd.Flags().MarkHidden("iface")
 	runCmd.Flags().StringVarP(&cdUpstreamProto, "proto", "", ctrld.ResolverTypeDOH, `Control D upstream type, either "doh" or "doh3"`)
 	runCmd.Flags().BoolVarP(&rfc1918, "rfc1918", "", false, "Listen on RFC1918 addresses when 127.0.0.1 is the only listener")
-	runCmd.Flags().StringVarP(&interceptMode, "intercept-mode", "", "", "OS-level DNS interception mode: 'dns' (with VPN split routing) or 'hard' (all DNS through ctrld, no VPN split routing)")
+	runCmd.Flags().StringVarP(&interceptMode, "intercept-mode", "", "", "OS-level DNS interception mode: 'off' (disable interception and clear a persisted intercept_mode), 'dns' (with VPN split routing), or 'hard' (all DNS through ctrld, no VPN split routing)")
 
 	runCmd.FParseErrWhitelist = cobra.FParseErrWhitelist{UnknownFlags: true}
 	rootCmd.AddCommand(runCmd)
@@ -394,8 +394,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c
 			svcExists := serviceConfigFileExists()
 			mainLog.Load().Debug().Msgf("intercept upgrade check: args=%v interceptOnly=%v svcConfigExists=%v interceptMode=%q", osArgsEarly, interceptOnly, svcExists, interceptMode)
 			if interceptOnly && svcExists {
-				// Replace any existing split or --intercept-mode=<value> form. Keep an
-				// explicit "off" argument so it overrides a previously persisted config
+				// An explicit "off" argument must override a previously persisted config
 				// value while the service clears that value on startup.
 				if err := removeServiceFlag("--intercept-mode"); err != nil {
 					mainLog.Load().Fatal().Err(err).Msg("failed to remove existing intercept mode from service arguments")
@@ -778,7 +777,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c
 	startCmd.Flags().BoolVarP(&startOnly, "start_only", "", false, "Do not install new service")
 	_ = startCmd.Flags().MarkHidden("start_only")
 	startCmd.Flags().BoolVarP(&rfc1918, "rfc1918", "", false, "Listen on RFC1918 addresses when 127.0.0.1 is the only listener")
-	startCmd.Flags().StringVarP(&interceptMode, "intercept-mode", "", "", "OS-level DNS interception mode: 'dns' (with VPN split routing) or 'hard' (all DNS through ctrld, no VPN split routing)")
+	startCmd.Flags().StringVarP(&interceptMode, "intercept-mode", "", "", "OS-level DNS interception mode: 'off' (disable interception and clear a persisted intercept_mode), 'dns' (with VPN split routing), or 'hard' (all DNS through ctrld, no VPN split routing)")
 
 	routerCmd := &cobra.Command{
 		Use: "setup",

cmd/cli/dns_intercept_darwin.go

@@ -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.

cmd/cli/dns_proxy.go

@@ -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()
 	}

cmd/cli/dns_target.go

@@ -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
+}