added missing func.

- Skip systemd-resolved initialization on Android (ChromeOS crash fix)
- Skip system DNS discovery commands on iOS mobile (sandbox restrictions)
- Skip route-based DNS discovery on Android
- Skip systemd resolver on Android
- Add mobile platform checks to prevent sandbox access violations

These changes ensure ctrld works correctly in mobile sandboxed environments
where system commands and file access are restricted.

cmd/cli/cli.go

@@ -451,7 +451,18 @@ func run(appCallback *AppCallback, stopCh chan struct{}) {
 	p.onStopped = append(p.onStopped, func() {
 		// restore static DNS settings or DHCP
 		p.resetDNS(false, true)
-		restoreSavedStaticDNS("", false)
+		// Iterate over all physical interfaces and restore static DNS if a saved static config exists.
+		withEachPhysicalInterfaces("", "restore static DNS", func(i *net.Interface) error {
+			file := savedStaticDnsSettingsFilePath(i)
+			if _, err := os.Stat(file); err == nil {
+				if err := restoreDNS(i); err != nil {
+					mainLog.Load().Error().Err(err).Msgf("Could not restore static DNS on interface %s", i.Name)
+				} else {
+					mainLog.Load().Debug().Msgf("Restored static DNS on interface %s successfully", i.Name)
+				}
+			}
+			return nil
+		})
 	})
 
 	close(waitCh)
@@ -815,7 +826,7 @@ func processLogAndCacheFlags(v *viper.Viper, cfg *ctrld.Config) {
 }
 
 func netInterface(ifaceName string) (*net.Interface, error) {
-	if ifaceName == autoIface {
+	if ifaceName == "auto" {
 		ifaceName = defaultIfaceName()
 	}
 	var iface *net.Interface
@@ -1101,7 +1112,23 @@ func uninstall(p *prog, s service.Service) {
 		}
 		// restore static DNS settings or DHCP
 		p.resetDNS(false, true)
-		restoreSavedStaticDNS(p.runningIface, true)
+
+		// Iterate over all physical interfaces and restore DNS if a saved static config exists.
+		withEachPhysicalInterfaces(p.runningIface, "restore static DNS", func(i *net.Interface) error {
+			file := savedStaticDnsSettingsFilePath(i)
+			if _, err := os.Stat(file); err == nil {
+				if err := restoreDNS(i); err != nil {
+					mainLog.Load().Error().Err(err).Msgf("Could not restore static DNS on interface %s", i.Name)
+				} else {
+					mainLog.Load().Debug().Msgf("Restored static DNS on interface %s successfully", i.Name)
+					err = os.Remove(file)
+					if err != nil {
+						mainLog.Load().Debug().Err(err).Msgf("Could not remove saved static DNS file for interface %s", i.Name)
+					}
+				}
+			}
+			return nil
+		})
 
 		if router.Name() != "" {
 			mainLog.Load().Debug().Msg("Router cleanup")
@@ -1114,26 +1141,6 @@ func uninstall(p *prog, s service.Service) {
 	}
 }
 
-// restoreSavedStaticDNS restores DNS from saved static config files on physical interfaces.
-func restoreSavedStaticDNS(excludeIfaceName string, removeSaved bool) {
-	withEachPhysicalInterfaces(excludeIfaceName, "restore static DNS", func(i *net.Interface) error {
-		file := savedStaticDnsSettingsFilePath(i)
-		if _, err := os.Stat(file); err == nil {
-			if err := restoreDNS(i); err != nil {
-				mainLog.Load().Error().Err(err).Msgf("Could not restore static DNS on interface %s", i.Name)
-			} else {
-				mainLog.Load().Debug().Msgf("Restored static DNS on interface %s successfully", i.Name)
-				if removeSaved {
-					if err := os.Remove(file); err != nil {
-						mainLog.Load().Debug().Err(err).Msgf("Could not remove saved static DNS file for interface %s", i.Name)
-					}
-				}
-			}
-		}
-		return nil
-	})
-}
-
 func validateConfig(cfg *ctrld.Config) error {
 	if err := ctrld.ValidateConfig(validator.New(), cfg); err != nil {
 		var ve validator.ValidationErrors
@@ -2062,23 +2069,6 @@ func doValidateCdRemoteConfig(cdUID string, fatal bool) error {
 	return nil
 }
 
-// ensureRunningIfaceForInvalidUninstall populates p.runningIface before the
-// invalid-device self-uninstall resets DNS. This path can run early during
-// service startup (e.g. right after a reboot) before the running interface is
-// otherwise known. resetDNS, via resetDNSForRunningIface, silently skips DNS
-// restoration when p.runningIface is empty, which would leave the OS pointed at
-// ctrld's local listener after the service is removed. See issue-556.
-func ensureRunningIfaceForInvalidUninstall(p *prog, s service.Service) {
-	if iface == "" {
-		iface = autoIface
-	}
-	p.preRun()
-	if ir := runningIface(s); ir != nil {
-		p.runningIface = ir.Name
-		p.requiredMultiNICsConfig = ir.All
-	}
-}
-
 // uninstallInvalidCdUID performs self-uninstallation because the ControlD device does not exist.
 func uninstallInvalidCdUID(p *prog, logger zerolog.Logger, doStop bool) bool {
 	s, err := newService(p, svcConfig)
@@ -2086,13 +2076,8 @@ func uninstallInvalidCdUID(p *prog, logger zerolog.Logger, doStop bool) bool {
 		logger.Warn().Err(err).Msg("failed to create new service")
 		return false
 	}
-	ensureRunningIfaceForInvalidUninstall(p, s)
 	// restore static DNS settings or DHCP
 	p.resetDNS(false, true)
-	// The invalid-device path may run early during service startup before runningIface
-	// is known. Restore every saved static DNS file so uninstalling does not leave the
-	// OS pointed at ctrld's local listener after the service is removed.
-	restoreSavedStaticDNS("", true)
 
 	tasks := []task{{s.Uninstall, true, "Uninstall"}}
 	if doTasks(tasks) {

cmd/cli/cli_intercept_listener_test.go

@@ -1,10 +1,6 @@
 package cli
 
-import (
-	"testing"
-
-	"github.com/Control-D-Inc/ctrld"
-)
+import "testing"
 
 func TestIsExplicitInterceptListener(t *testing.T) {
 	tests := []struct {
@@ -30,87 +26,3 @@ func TestIsExplicitInterceptListener(t *testing.T) {
 		})
 	}
 }
-
-// TestPreserveBoundListeners is a regression test for #551: on reload, the on-disk
-// generated config still declares 127.0.0.1:53, but the running listener has fallen back
-// to 127.0.0.1:5354. preserveBoundListeners must keep the in-memory config on the actual
-// bound port so pf rdr rules and probes do not target the dead default port.
-func TestPreserveBoundListeners(t *testing.T) {
-	// cur = actual running listener (fell back to 5354); newCfg = freshly read from disk (53).
-	cur := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}
-	newListeners := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 53}}
-
-	preserveBoundListeners(newListeners, cur)
-
-	if got := newListeners["0"].Port; got != 5354 {
-		t.Errorf("listener port after reload = %d, want 5354 (actual bound port)", got)
-	}
-	if got := newListeners["0"].IP; got != "127.0.0.1" {
-		t.Errorf("listener IP after reload = %q, want 127.0.0.1", got)
-	}
-}
-
-// TestPreserveBoundListeners_NoChange verifies that when the on-disk config matches the
-// running listener, the config is left untouched (a legitimate reload with the same port).
-func TestPreserveBoundListeners_NoChange(t *testing.T) {
-	cur := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}
-	newListeners := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}
-
-	preserveBoundListeners(newListeners, cur)
-
-	if got := newListeners["0"].Port; got != 5354 {
-		t.Errorf("listener port = %d, want 5354", got)
-	}
-}
-
-// TestPreserveBoundListeners_MissingCurrent verifies that a listener present on disk but not
-// in the current running set (e.g. newly added) is left as configured.
-func TestPreserveBoundListeners_MissingCurrent(t *testing.T) {
-	cur := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}
-	newListeners := map[string]*ctrld.ListenerConfig{
-		"0": {IP: "127.0.0.1", Port: 53},
-		"1": {IP: "127.0.0.1", Port: 5355},
-	}
-
-	preserveBoundListeners(newListeners, cur)
-
-	if got := newListeners["0"].Port; got != 5354 {
-		t.Errorf("listener 0 port = %d, want 5354 (preserved)", got)
-	}
-	if got := newListeners["1"].Port; got != 5355 {
-		t.Errorf("listener 1 port = %d, want 5355 (unchanged, no current binding)", got)
-	}
-}
-
-// TestPreserveBoundListeners_ExplicitChangeNotMasked verifies that an explicit, non-default
-// listener in the reloaded config is applied rather than reverted to the old bound listener.
-// Reverting an explicit change would make the control-server reload comparison return 200
-// instead of 201, silently dropping the new listener. Regression guard for #551 review.
-func TestPreserveBoundListeners_ExplicitChangeNotMasked(t *testing.T) {
-	// Running listener fell back to 5354; user reloads with an explicit new listener.
-	cur := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}
-	newListeners := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.2", Port: 5399}}
-
-	preserveBoundListeners(newListeners, cur)
-
-	if got := newListeners["0"].IP; got != "127.0.0.2" {
-		t.Errorf("explicit listener IP = %q, want 127.0.0.2 (not reverted)", got)
-	}
-	if got := newListeners["0"].Port; got != 5399 {
-		t.Errorf("explicit listener port = %d, want 5399 (not reverted)", got)
-	}
-}
-
-// TestPreserveBoundListeners_ExplicitDefaultPreserved verifies that the default
-// 127.0.0.1:53 listener remains fallback-eligible: when it diverges from the running
-// fallback port it is still preserved (isExplicitInterceptListener treats :53 as non-explicit).
-func TestPreserveBoundListeners_ExplicitDefaultPreserved(t *testing.T) {
-	cur := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}
-	newListeners := map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 53}}
-
-	preserveBoundListeners(newListeners, cur)
-
-	if got := newListeners["0"].Port; got != 5354 {
-		t.Errorf("default listener port = %d, want 5354 (preserved fallback)", got)
-	}
-}

cmd/cli/commands.go

@@ -399,7 +399,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c
 			reportSetDnsOk := func(sockDir string) {
 				if cc := newSocketControlClient(ctx, s, sockDir); cc != nil {
 					if resp, _ := cc.post(ifacePath, nil); resp != nil && resp.StatusCode == http.StatusOK {
-						if iface == autoIface {
+						if iface == "auto" {
 							iface = defaultIfaceName()
 						}
 						res := &ifaceResponse{}
@@ -748,7 +748,7 @@ NOTE: running "ctrld start" without any arguments will start already installed c
 			startCmd.Run(cmd, args)
 		},
 	}
-	startCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", autoIface, `Update DNS setting for iface, "auto" means the default interface gateway`)
+	startCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", "auto", `Update DNS setting for iface, "auto" means the default interface gateway`)
 	startCmdAlias.Flags().AddFlagSet(startCmd.Flags())
 	rootCmd.AddCommand(startCmdAlias)
 
@@ -833,7 +833,7 @@ func initStopCmd() *cobra.Command {
 			stopCmd.Run(cmd, args)
 		},
 	}
-	stopCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", autoIface, `Reset DNS setting for iface, "auto" means the default interface gateway`)
+	stopCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", "auto", `Reset DNS setting for iface, "auto" means the default interface gateway`)
 	stopCmdAlias.Flags().AddFlagSet(stopCmd.Flags())
 	rootCmd.AddCommand(stopCmdAlias)
 
@@ -865,7 +865,7 @@ func initRestartCmd() *cobra.Command {
 				return
 			}
 			if iface == "" {
-				iface = autoIface
+				iface = "auto"
 			}
 			p.preRun()
 			if ir := runningIface(s); ir != nil {
@@ -1111,7 +1111,7 @@ NOTE: Uninstalling will set DNS to values provided by DHCP.`,
 				return
 			}
 			if iface == "" {
-				iface = autoIface
+				iface = "auto"
 			}
 			p.preRun()
 			if ir := runningIface(s); ir != nil {
@@ -1207,7 +1207,7 @@ NOTE: Uninstalling will set DNS to values provided by DHCP.`,
 			uninstallCmd.Run(cmd, args)
 		},
 	}
-	uninstallCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", autoIface, `Reset DNS setting for iface, "auto" means the default interface gateway`)
+	uninstallCmdAlias.Flags().StringVarP(&ifaceStartStop, "iface", "", "auto", `Reset DNS setting for iface, "auto" means the default interface gateway`)
 	uninstallCmdAlias.Flags().AddFlagSet(uninstallCmd.Flags())
 	rootCmd.AddCommand(uninstallCmdAlias)
 
@@ -1400,7 +1400,7 @@ func initUpgradeCmd() *cobra.Command {
 				return
 			}
 			if iface == "" {
-				iface = autoIface
+				iface = "auto"
 			}
 			p.preRun()
 			if ir := runningIface(s); ir != nil {
@@ -1595,7 +1595,7 @@ func onlyInterceptFlags(args []string) bool {
 			} else {
 				return false
 			}
-		case arg == "--iface="+autoIface || arg == "--iface" || arg == autoIface:
+		case arg == "--iface=auto" || arg == "--iface" || arg == "auto":
 			// Auto-added by startCmdAlias or its value; safe to ignore.
 			continue
 		default:

cmd/cli/dns_intercept_darwin.go

@@ -41,11 +41,6 @@ const (
 	// 2s re-check misses.
 	pfAnchorRecheckDelayLong = 4 * time.Second
 
-	// pfExecFailureBackoff suppresses repeated pfctl/scutil checks briefly after
-	// macOS reports local resource exhaustion. Without this, network-change storms
-	// can turn a pf ruleset race into a fork/file-descriptor exhaustion loop.
-	pfExecFailureBackoff = 5 * time.Second
-
 	// pfVPNInterfacePrefixes lists interface name prefixes that indicate VPN/tunnel
 	// interfaces on macOS. Used to add interface-specific DNS intercept rules so that
 	// VPN software with "pass out quick on <iface>" rules cannot bypass our intercept.
@@ -1308,15 +1303,6 @@ func (p *prog) ensurePFAnchorActive() bool {
 	if p.dnsInterceptState == nil {
 		return false
 	}
-	if !p.pfEnsureRunning.CompareAndSwap(false, true) {
-		mainLog.Load().Debug().Msg("DNS intercept watchdog: check already running, skipping duplicate")
-		return false
-	}
-	defer p.pfEnsureRunning.Store(false)
-
-	if p.pfExecBackoffActive() {
-		return false
-	}
 
 	// While stabilizing (VPN connecting), suppress all restores.
 	// The stabilization loop will restore once pf settles.
@@ -1350,9 +1336,6 @@ func (p *prog) ensurePFAnchorActive() bool {
 	// Check 1: anchor references in the main ruleset.
 	natOut, err := exec.Command("pfctl", "-sn").CombinedOutput()
 	if err != nil {
-		if p.pfBackoffResourceExhaustion(err, natOut, "dump NAT rules") {
-			return false
-		}
 		mainLog.Load().Warn().Err(err).Msg("DNS intercept watchdog: could not dump NAT rules")
 		return false
 	}
@@ -1365,9 +1348,6 @@ func (p *prog) ensurePFAnchorActive() bool {
 	if !needsRestore {
 		filterOut, err := exec.Command("pfctl", "-sr").CombinedOutput()
 		if err != nil {
-			if p.pfBackoffResourceExhaustion(err, filterOut, "dump filter rules") {
-				return false
-			}
 			mainLog.Load().Warn().Err(err).Msg("DNS intercept watchdog: could not dump filter rules")
 			return false
 		}
@@ -1385,9 +1365,6 @@ func (p *prog) ensurePFAnchorActive() bool {
 	if !needsRestore {
 		anchorFilter, err := exec.Command("pfctl", "-a", pfAnchorName, "-sr").CombinedOutput()
 		if err != nil || len(strings.TrimSpace(string(anchorFilter))) == 0 {
-			if p.pfBackoffResourceExhaustion(err, anchorFilter, "dump anchor filter rules") {
-				return false
-			}
 			mainLog.Load().Warn().Msg("DNS intercept watchdog: anchor has no filter rules — content was flushed")
 			needsRestore = true
 		}
@@ -1395,9 +1372,6 @@ func (p *prog) ensurePFAnchorActive() bool {
 	if !needsRestore {
 		anchorNat, err := exec.Command("pfctl", "-a", pfAnchorName, "-sn").CombinedOutput()
 		if err != nil || len(strings.TrimSpace(string(anchorNat))) == 0 {
-			if p.pfBackoffResourceExhaustion(err, anchorNat, "dump anchor NAT rules") {
-				return false
-			}
 			mainLog.Load().Warn().Msg("DNS intercept watchdog: anchor has no rdr rules — translation was flushed (will cause packet loop on lo0)")
 			needsRestore = true
 		}
@@ -1431,7 +1405,6 @@ func (p *prog) ensurePFAnchorActive() bool {
 	// Restore: re-inject anchor references into the main ruleset.
 	mainLog.Load().Info().Msg("DNS intercept watchdog: restoring pf anchor references")
 	if err := p.ensurePFAnchorReference(); err != nil {
-		p.pfBackoffResourceExhaustion(err, nil, "restore anchor references")
 		mainLog.Load().Error().Err(err).Msg("DNS intercept watchdog: failed to restore anchor references")
 		return true
 	}
@@ -1448,7 +1421,6 @@ func (p *prog) ensurePFAnchorActive() bool {
 	if err := os.WriteFile(pfAnchorFile, []byte(rulesStr), 0644); err != nil {
 		mainLog.Load().Error().Err(err).Msg("DNS intercept watchdog: failed to write anchor file")
 	} else if out, err := exec.Command("pfctl", "-a", pfAnchorName, "-f", pfAnchorFile).CombinedOutput(); err != nil {
-		p.pfBackoffResourceExhaustion(err, out, "load rebuilt anchor")
 		mainLog.Load().Error().Err(err).Msgf("DNS intercept watchdog: failed to load rebuilt anchor (output: %s)", strings.TrimSpace(string(out)))
 	} else {
 		flushPFStates()
@@ -1486,49 +1458,6 @@ func (p *prog) scheduleDNSAfterVPNSettleRefresh(reason string, delay time.Durati
 	})
 }
 
-func (p *prog) pfExecBackoffActive() bool {
-	until := p.pfExecBackoffUntil.Load()
-	if until == 0 {
-		return false
-	}
-	remaining := time.Until(time.UnixMilli(until))
-	if remaining <= 0 {
-		p.pfExecBackoffUntil.CompareAndSwap(until, 0)
-		return false
-	}
-	mainLog.Load().Debug().Dur("remaining", remaining).Msg("DNS intercept watchdog: suppressed during pf exec backoff")
-	return true
-}
-
-func (p *prog) pfBackoffResourceExhaustion(err error, output []byte, operation string) bool {
-	if !isResourceExhaustion(err, output) {
-		return false
-	}
-	until := time.Now().Add(pfExecFailureBackoff)
-	p.pfExecBackoffUntil.Store(until.UnixMilli())
-	mainLog.Load().Warn().Err(err).Dur("backoff", pfExecFailureBackoff).Str("operation", operation).
-		Msg("DNS intercept watchdog: backing off after local exec resource exhaustion")
-	return true
-}
-
-func isResourceExhaustion(err error, output []byte) bool {
-	if err == nil && len(output) == 0 {
-		return false
-	}
-	var msg string
-	if err != nil {
-		msg = err.Error()
-	}
-	if len(output) > 0 {
-		msg += "\n" + string(output)
-	}
-	msg = strings.ToLower(msg)
-	return strings.Contains(msg, "resource temporarily unavailable") ||
-		strings.Contains(msg, "too many open files") ||
-		strings.Contains(msg, "too many processes") ||
-		strings.Contains(msg, "cannot allocate memory")
-}
-
 // 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.
@@ -1541,19 +1470,8 @@ func isResourceExhaustion(err error, output []byte) bool {
 //
 // Two delays (2s and 4s) cover both fast and slow VPN teardowns.
 func (p *prog) scheduleDelayedRechecks() {
-	p.pfDelayedRecheckMu.Lock()
-	defer p.pfDelayedRecheckMu.Unlock()
-
-	for _, timer := range p.pfDelayedRecheckTimers {
-		if timer != nil {
-			timer.Stop()
-		}
-	}
-	p.pfDelayedRecheckTimers = p.pfDelayedRecheckTimers[:0]
-
 	for _, delay := range []time.Duration{pfAnchorRecheckDelay, pfAnchorRecheckDelayLong} {
-		delay := delay
-		timer := time.AfterFunc(delay, func() {
+		time.AfterFunc(delay, func() {
 			if p.dnsInterceptState == nil || p.pfStabilizing.Load() {
 				return
 			}
@@ -1567,7 +1485,6 @@ func (p *prog) scheduleDelayedRechecks() {
 				p.vpnDNS.Refresh(true)
 			}
 		})
-		p.pfDelayedRecheckTimers = append(p.pfDelayedRecheckTimers, timer)
 	}
 }
 

cmd/cli/dns_intercept_darwin_test.go

@@ -3,7 +3,6 @@
 package cli
 
 import (
-	"errors"
 	"strings"
 	"testing"
 
@@ -123,35 +122,6 @@ func TestPFBuildAnchorRules_Ordering(t *testing.T) {
 	}
 }
 
-// TestPFBuildAnchorRules_FallbackPort verifies that when the listener falls back
-// to an alternate local port (e.g. 5354 because mDNSResponder owns *:53), the pf
-// rdr rules redirect DNS to the ACTUAL bound port, not the configured default 53.
-// Regression test for #551: pf redirected to a dead port after listener fallback.
-func TestPFBuildAnchorRules_FallbackPort(t *testing.T) {
-	// Configured/generated listener is 127.0.0.1:53, but the runtime bound port is 5354.
-	p := &prog{cfg: &ctrld.Config{Listener: map[string]*ctrld.ListenerConfig{"0": {IP: "127.0.0.1", Port: 5354}}}}
-	rules := p.buildPFAnchorRules(nil)
-
-	// rdr must redirect to the actual bound port 5354.
-	if !strings.Contains(rules, "rdr on lo0 inet proto udp from any to ! 127.0.0.1 port 53 -> 127.0.0.1 port 5354") {
-		t.Errorf("UDP rdr must redirect to bound port 5354, got:\n%s", rules)
-	}
-	if !strings.Contains(rules, "rdr on lo0 inet proto tcp from any to ! 127.0.0.1 port 53 -> 127.0.0.1 port 5354") {
-		t.Errorf("TCP rdr must redirect to bound port 5354, got:\n%s", rules)
-	}
-
-	// The rdr redirect target must NOT point at the dead default port 53.
-	// Match the exact port at line end so "port 5354" is not a false positive.
-	if strings.Contains(rules, "-> 127.0.0.1 port 53\n") {
-		t.Errorf("rdr must not redirect to dead port 53 after fallback, got:\n%s", rules)
-	}
-
-	// The inbound accept rule must also target the actual bound port.
-	if !strings.Contains(rules, "127.0.0.1 port 5354") {
-		t.Errorf("pass in rule must reference bound port 5354, got:\n%s", rules)
-	}
-}
-
 // TestPFAddressFamily tests the pfAddressFamily helper.
 func TestPFAddressFamily(t *testing.T) {
 	tests := []struct {
@@ -171,47 +141,3 @@ func TestPFAddressFamily(t *testing.T) {
 		}
 	}
 }
-
-func TestIsResourceExhaustion(t *testing.T) {
-	tests := []struct {
-		name   string
-		err    error
-		output []byte
-		want   bool
-	}{
-		{
-			name: "exec start failure",
-			err:  errors.New("fork/exec /sbin/pfctl: resource temporarily unavailable"),
-			want: true,
-		},
-		{
-			name:   "fd exhaustion from stderr output",
-			err:    errors.New("exit status 1"),
-			output: []byte("pfctl: Pipe: Too many open files"),
-			want:   true,
-		},
-		{
-			name: "process exhaustion from wrapped restore error",
-			err:  errors.New("failed to dump running filter rules: exit status 1 (output: too many processes)"),
-			want: true,
-		},
-		{
-			name:   "ordinary pf syntax failure",
-			err:    errors.New("exit status 1"),
-			output: []byte("pfctl: syntax error"),
-			want:   false,
-		},
-		{
-			name: "nil error and empty output",
-			want: false,
-		},
-	}
-
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := isResourceExhaustion(tt.err, tt.output); got != tt.want {
-				t.Fatalf("isResourceExhaustion() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}

cmd/cli/dns_intercept_windows.go

@@ -278,9 +278,6 @@ type wfpState struct {
 	loopbackProtectActive bool
 	// loopbackPermitIDs stores the filter IDs for the loopback protect permits.
 	loopbackPermitIDs []uint64
-	// nrptRecoveryLimiter prevents repeated Windows policy/Dnscache signaling
-	// when another agent keeps putting NRPT back into a broken state.
-	nrptRecoveryLimiter nrptRecoveryLimiter
 }
 
 // Lazy-loaded WFP DLL procedures.
@@ -346,10 +343,9 @@ const (
 //   - GP path: SOFTWARE\Policies\...\DnsPolicyConfig (Group Policy)
 //   - Local path: SYSTEM\CurrentControlSet\...\DnsPolicyConfig (service store)
 //
-// If the GP path contains real rules (from IT policy, VPN, MDM, etc.), DNS
-// Client enters "GP mode" and ignores ALL local-path rules entirely. An empty GP
-// parent key is worse: it still puts DNS Client in GP mode, but contributes no
-// usable rule, so our local catch-all is hidden until that empty parent is gone.
+// If ANY rules exist in the GP path (from IT policy, VPN, MDM, etc.), DNS Client
+// enters "GP mode" and ignores ALL local-path rules entirely. Conversely, if the
+// GP path is empty/absent, DNS Client reads from the local path only.
 //
 // Strategy (matching Tailscale's approach):
 //   - Always write to the local path (baseline for non-domain machines).
@@ -399,20 +395,18 @@ func otherGPRulesExist() bool {
 	return false
 }
 
-// cleanGPPath removes only ctrld's GP-path rule and deletes the GP parent when
-// no rules remain. The return value tells callers whether the parent key was
-// actually deleted, which means DNS Client should be signaled once.
-//
-// Do not leave an empty GP parent behind: Windows treats the parent key itself
-// as the policy store boundary, so an empty key can still hide local-path rules.
-func cleanGPPath() bool {
+// cleanGPPath removes our CtrldCatchAll rule from the GP path and deletes
+// the GP DnsPolicyConfig parent key if no other rules remain. Removing the
+// empty GP key is critical: its mere existence forces DNS Client into "GP mode"
+// where local-path rules are ignored.
+func cleanGPPath() {
 	// Delete our specific rule.
 	registry.DeleteKey(registry.LOCAL_MACHINE, nrptBaseKey+`\`+nrptRuleName)
 
 	// If the GP parent key is now empty, delete it entirely to exit "GP mode".
 	k, err := registry.OpenKey(registry.LOCAL_MACHINE, nrptBaseKey, registry.ENUMERATE_SUB_KEYS)
 	if err != nil {
-		return false // Key doesn't exist — clean state.
+		return // Key doesn't exist — clean state.
 	}
 	names, err := k.ReadSubKeyNames(-1)
 	k.Close()
@@ -420,14 +414,12 @@ func cleanGPPath() bool {
 		if len(names) > 0 {
 			mainLog.Load().Debug().Strs("remaining", names).Msg("DNS intercept: GP path has other rules, leaving parent key")
 		}
-		return false
+		return
 	}
 	// Empty — delete it to exit "GP mode".
 	if err := registry.DeleteKey(registry.LOCAL_MACHINE, nrptBaseKey); err == nil {
 		mainLog.Load().Info().Msg("DNS intercept: deleted empty GP DnsPolicyConfig key (exits GP mode)")
-		return true
 	}
-	return false
 }
 
 // writeNRPTRule writes a single NRPT catch-all rule at the given registry keyPath.
@@ -550,11 +542,10 @@ func refreshNRPTPolicy() {
 // Group Policy refresh so NRPT changes take effect immediately.
 // Uses DnsFlushResolverCache from dnsapi.dll + RefreshPolicyEx from userenv.dll.
 func flushDNSCache() {
+	// Step 1: Refresh GP so DNS Client loads the new NRPT rules from registry.
 	refreshNRPTPolicy()
-	flushDNSCacheOnly()
-}
 
-func flushDNSCacheOnly() {
+	// Step 2: Flush the DNS cache so stale entries from pre-NRPT resolution are cleared.
 	if err := dnsapiDLL.Load(); err == nil {
 		if err := procDnsFlushResolverCache.Find(); err == nil {
 			ret, _, _ := procDnsFlushResolverCache.Call()
@@ -564,6 +555,7 @@ func flushDNSCacheOnly() {
 			}
 		}
 	}
+	// Fallback: use ipconfig /flushdns.
 	if out, err := exec.Command("ipconfig", "/flushdns").CombinedOutput(); err != nil {
 		mainLog.Load().Debug().Msgf("DNS intercept: ipconfig /flushdns failed: %v: %s", err, string(out))
 	} else {
@@ -571,12 +563,6 @@ func flushDNSCacheOnly() {
 	}
 }
 
-func signalNRPTChange() {
-	refreshNRPTPolicy()
-	sendParamChange()
-	flushDNSCacheOnly()
-}
-
 // startDNSIntercept activates WFP-based DNS interception on Windows.
 // It creates a WFP sublayer and adds filters that block all outbound DNS (port 53)
 // traffic except to localhost (127.0.0.1/::1), ensuring all DNS queries must go
@@ -612,19 +598,21 @@ func (p *prog) startDNSIntercept() error {
 	mainLog.Load().Info().Msgf("DNS intercept: initializing (mode: %s)", interceptMode)
 	logNRPTParentKeyState("pre-write")
 
-	// Empty parent key recovery: if the GP DnsPolicyConfig key exists but is
-	// empty, DNS Client enters GP mode and hides local rules. Delete empty
-	// parents first, then send one change signal so DNS Client drops stale state.
-	if cleanEmptyNRPTParent() {
-		signalNRPTChange()
-	}
+	// Two-phase empty parent key recovery: if the GP DnsPolicyConfig key exists
+	// but is empty, DNS Client has cached a "no rules" state and won't accept
+	// new rules even after they're written. Delete the empty key and signal DNS
+	// Client to reset before writing our rule.
+	// Two-phase recovery handles its own 2s signaling burst internally.
+	cleanEmptyNRPTParent()
 
 	if err := addNRPTCatchAllRule(listenerIP); err != nil {
 		return fmt.Errorf("dns intercept: failed to add NRPT catch-all rule: %w", err)
 	}
 	logNRPTParentKeyState("post-write")
 	state.nrptActive = true
-	signalNRPTChange()
+	refreshNRPTPolicy()
+	sendParamChange()
+	flushDNSCache()
 	mainLog.Load().Info().Msgf("DNS intercept: NRPT catch-all rule active — all DNS queries directed to %s", listenerIP)
 
 	// Step 2: In hard mode, also set up WFP filters to block non-local DNS.
@@ -1567,7 +1555,7 @@ func (p *prog) scheduleDelayedRechecks() {
 					mainLog.Load().Error().Err(err).Msg("DNS intercept: failed to re-add NRPT catch-all rule")
 					state.nrptActive = false
 				} else {
-					signalNRPTChange()
+					flushDNSCache()
 					mainLog.Load().Info().Msg("DNS intercept: NRPT catch-all rule restored")
 				}
 			}
@@ -1605,22 +1593,14 @@ func (p *prog) nrptHealthMonitor(state *wfpState) {
 
 			// Step 1: Check registry key exists.
 			if !nrptCatchAllRuleExists() {
-				now := time.Now()
-				if ok, wait := state.nrptRecoveryLimiter.allow(now, p.cfg); !ok {
-					if state.nrptRecoveryLimiter.shouldLogSkip(now) {
-						mainLog.Load().Warn().Dur("remaining", wait).
-							Msg("DNS intercept: NRPT rule restore suppressed after repeated recovery flows")
-					}
-					continue
-				}
 				mainLog.Load().Warn().Msg("DNS intercept: NRPT health check — catch-all rule missing, restoring")
 				if err := addNRPTCatchAllRule(state.listenerIP); err != nil {
 					mainLog.Load().Error().Err(err).Msg("DNS intercept: failed to restore NRPT catch-all rule")
 					state.nrptActive = false
 					continue
 				}
-				signalNRPTChange()
-				state.nrptRecoveryLimiter.recordRecoveryFlow(time.Now(), p.cfg)
+				refreshNRPTPolicy()
+				flushDNSCache()
 				mainLog.Load().Info().Msg("DNS intercept: NRPT catch-all rule restored by health monitor")
 				// After restoring, verify it's actually working.
 				go p.nrptProbeAndHeal()
@@ -1632,8 +1612,6 @@ func (p *prog) nrptHealthMonitor(state *wfpState) {
 			if !p.probeNRPT() {
 				mainLog.Load().Warn().Msg("DNS intercept: NRPT health check — rule present but probe failed, running heal cycle")
 				go p.nrptProbeAndHeal()
-			} else {
-				state.nrptRecoveryLimiter.recordStableSuccess()
 			}
 
 			// Step 3: In hard mode, also verify WFP sublayer.
@@ -1732,39 +1710,43 @@ func sendParamChange() {
 }
 
 // cleanEmptyNRPTParent removes empty NRPT parent keys that block activation.
-// Empty GP and local parents have different failure shapes:
-//   - empty GP parent: DNS Client is in GP mode and ignores local-path rules;
-//   - empty local parent: DNS Client can cache an empty local policy store.
+// An empty DnsPolicyConfig key (exists but no subkeys) causes DNS Client to
+// cache "no rules" and ignore subsequently-added rules.
 //
-// This helper only changes registry state. The caller sends the single
-// RefreshPolicyEx/paramchange/flush signal after it knows cleanup occurred.
+// Also cleans the GP path entirely if it has no non-ctrld rules, since the GP
+// path's existence forces DNS Client into "GP mode" where it ignores the local
+// service store path.
 //
-// Returns true if cleanup was performed (caller should signal DNS Client).
+// Returns true if cleanup was performed (caller should add a delay).
 func cleanEmptyNRPTParent() bool {
+	cleaned := false
+
 	// Always clean the GP path — its existence blocks local path activation.
-	cleaned := cleanGPPath()
+	cleanGPPath()
 
 	// Clean empty local/direct path parent key.
-	if !nrptParentKeyEmpty(nrptDirectKey) {
-		return cleaned
-	}
-
-	mainLog.Load().Warn().Msg("DNS intercept: found empty NRPT local parent key (blocks activation) — removing")
-	if err := registry.DeleteKey(registry.LOCAL_MACHINE, nrptDirectKey); err != nil {
-		mainLog.Load().Warn().Err(err).Msg("DNS intercept: failed to delete empty NRPT local parent key")
-		return cleaned
-	}
-	return true
-}
-
-func nrptParentKeyEmpty(keyPath string) bool {
-	k, err := registry.OpenKey(registry.LOCAL_MACHINE, keyPath, registry.ENUMERATE_SUB_KEYS)
+	k, err := registry.OpenKey(registry.LOCAL_MACHINE, nrptDirectKey, registry.ENUMERATE_SUB_KEYS)
 	if err != nil {
 		return false
 	}
 	names, err := k.ReadSubKeyNames(-1)
 	k.Close()
-	return err == nil && len(names) == 0
+	if err != nil || len(names) > 0 {
+		return false
+	}
+
+	mainLog.Load().Warn().Msg("DNS intercept: found empty NRPT local parent key (blocks activation) — removing")
+	if err := registry.DeleteKey(registry.LOCAL_MACHINE, nrptDirectKey); err != nil {
+		mainLog.Load().Warn().Err(err).Msg("DNS intercept: failed to delete empty NRPT local parent key")
+		return false
+	}
+	cleaned = true
+
+	// Signal DNS Client to process the deletion and reset its internal cache.
+	mainLog.Load().Info().Msg("DNS intercept: empty NRPT parent key removed — signaling DNS Client")
+	sendParamChange()
+	flushDNSCache()
+	return cleaned
 }
 
 // logNRPTParentKeyState logs the state of both NRPT registry paths for diagnostics.
@@ -1801,39 +1783,18 @@ func logNRPTParentKeyState(context string) {
 // nrptProbeAndHeal runs the NRPT probe with retries and escalating remediation.
 // Called asynchronously after startup and from the health monitor.
 //
-// Retry sequence:
-//  1. Immediate probe.
-//  2. If the GP parent is empty, clean it immediately, signal once, then probe.
-//     This is intentionally before the normal retry loop: policy refresh and
-//     Dnscache paramchange cannot make local rules visible while GP mode is
-//     selected by an empty GP parent.
-//  3. Otherwise, signal DNS Client with increasing backoff between probes.
+// Retry sequence (each attempt: GP refresh + paramchange + flush → sleep → probe):
+//  1. Immediate probe
+//  2. GP refresh + paramchange + flush → 1s → probe
+//  3. GP refresh + paramchange + flush → 2s → probe
+//  4. GP refresh + paramchange + flush → 4s → probe
 func (p *prog) nrptProbeAndHeal() {
-	state, _ := p.dnsInterceptState.(*wfpState)
-	if state != nil {
-		now := time.Now()
-		if ok, wait := state.nrptRecoveryLimiter.allow(now, p.cfg); !ok {
-			if state.nrptRecoveryLimiter.shouldLogSkip(now) {
-				mainLog.Load().Warn().Dur("remaining", wait).
-					Msg("DNS intercept: NRPT recovery suppressed after repeated failed recovery flows")
-			}
-			return
-		}
-	}
-
 	if !nrptProbeRunning.CompareAndSwap(false, true) {
 		mainLog.Load().Debug().Msg("DNS intercept: NRPT probe already running, skipping")
 		return
 	}
 	defer nrptProbeRunning.Store(false)
 
-	remediated := false
-	defer func() {
-		if remediated && state != nil {
-			state.nrptRecoveryLimiter.recordRecoveryFlow(time.Now(), p.cfg)
-		}
-	}()
-
 	mainLog.Load().Info().Msg("DNS intercept: starting NRPT verification probe sequence")
 
 	// Log parent key state for diagnostics.
@@ -1844,37 +1805,17 @@ func (p *prog) nrptProbeAndHeal() {
 		mainLog.Load().Info().Msg("DNS intercept: NRPT verified working")
 		return
 	}
-	remediated = true
 
-	// If the GP parent exists but is empty, do not burn retries on Windows
-	// signaling. Those retries create SIEM noise but cannot succeed because DNS
-	// Client is still reading the empty GP store instead of the populated local
-	// store. Delete the blocker, send one notification, then re-probe.
-	if nrptParentKeyEmpty(nrptBaseKey) {
-		mainLog.Load().Warn().Msg("DNS intercept: NRPT probe failed with empty GP parent — cleaning before retry signaling")
-		if cleanEmptyNRPTParent() {
-			signalNRPTChange()
-			time.Sleep(1 * time.Second)
-			logNRPTParentKeyState("empty-gp-after-clean")
-			if p.probeNRPT() {
-				mainLog.Load().Info().Msg("DNS intercept: NRPT verified working after empty GP parent cleanup")
-				return
-			}
-		}
-		if nrptParentKeyEmpty(nrptBaseKey) {
-			mainLog.Load().Warn().Msg("DNS intercept: empty GP NRPT parent still present after cleanup; skipping redundant policy refresh retries")
-			return
-		}
-	}
-
-	// Attempts 2-4: signal DNS Client with increasing backoff between probes.
+	// Attempts 2-4: GP refresh + paramchange + flush with increasing backoff
 	delays := []time.Duration{1 * time.Second, 2 * time.Second, 4 * time.Second}
 	for i, delay := range delays {
 		attempt := i + 2
 		mainLog.Load().Info().Int("attempt", attempt).Dur("delay", delay).
-			Msg("DNS intercept: NRPT probe failed, retrying with policy refresh + paramchange")
+			Msg("DNS intercept: NRPT probe failed, retrying with GP refresh + paramchange")
 		logNRPTParentKeyState(fmt.Sprintf("probe-attempt-%d", attempt))
-		signalNRPTChange()
+		refreshNRPTPolicy()
+		sendParamChange()
+		flushDNSCache()
 		time.Sleep(delay)
 		if p.probeNRPT() {
 			mainLog.Load().Info().Int("attempt", attempt).
@@ -1888,7 +1829,7 @@ func (p *prog) nrptProbeAndHeal() {
 	// signal DNS Client to forget it, wait, then re-add and signal again.
 	mainLog.Load().Warn().Msg("DNS intercept: all probes failed — attempting two-phase NRPT recovery (delete → signal → re-add)")
 	listenerIP := "127.0.0.1"
-	if state != nil {
+	if state, ok := p.dnsInterceptState.(*wfpState); ok {
 		listenerIP = state.listenerIP
 	}
 
@@ -1896,7 +1837,9 @@ func (p *prog) nrptProbeAndHeal() {
 	_ = removeNRPTCatchAllRule()
 	// If parent key is now empty after removing our rule, delete it too.
 	cleanEmptyNRPTParent()
-	signalNRPTChange()
+	refreshNRPTPolicy()
+	sendParamChange()
+	flushDNSCache()
 	logNRPTParentKeyState("nuclear-after-delete")
 
 	// Wait for DNS Client to process the deletion.
@@ -1907,7 +1850,9 @@ func (p *prog) nrptProbeAndHeal() {
 		mainLog.Load().Error().Err(err).Msg("DNS intercept: failed to re-add NRPT after nuclear recovery")
 		return
 	}
-	signalNRPTChange()
+	refreshNRPTPolicy()
+	sendParamChange()
+	flushDNSCache()
 	logNRPTParentKeyState("nuclear-after-readd")
 
 	// Final probe after recovery.

cmd/cli/dns_proxy.go

@@ -736,17 +736,6 @@ func (p *prog) proxy(ctx context.Context, req *proxyRequest) *proxyResponse {
 			}
 			continue
 		}
-		// Reject an answer whose question does not match the request before it
-		// can be served or cached. A mismatched question means the upstream
-		// answered a different name/type than asked; caching it would poison
-		// the shared cache with wrong-domain records for the requested name.
-		// See github.com/Control-D-Inc/ctrld/issues/322.
-		if !sameQuestion(req.msg, answer) {
-			ctrld.Log(ctx, mainLog.Load().Debug(),
-				"discarding answer from %s: question mismatch (asked %q, got %q)",
-				upstreams[n], questionString(req.msg), questionString(answer))
-			continue
-		}
 		// We are doing LAN/PTR lookup using private resolver, so always process next one.
 		// Except for the last, we want to send response instead of saying all upstream failed.
 		if answer.Rcode != dns.RcodeSuccess && isLanOrPtrQuery && n != len(upstreamConfigs)-1 {
@@ -910,33 +899,6 @@ func containRcode(rcodes []int, rcode int) bool {
 	return false
 }
 
-// sameQuestion reports whether the upstream answer echoes the request's
-// question. A well-behaved resolver always copies the question section from
-// the query (RFC 1035 section 4.1.2); names are compared case-insensitively
-// because DNS names are case-insensitive. A mismatch means the upstream
-// answered a different name/type than asked - malformed or malicious - and the
-// answer must not be served or cached, or it would poison the shared cache with
-// wrong-domain records. See github.com/Control-D-Inc/ctrld/issues/322.
-func sameQuestion(req, answer *dns.Msg) bool {
-	if req == nil || answer == nil {
-		return false
-	}
-	if len(req.Question) == 0 || len(answer.Question) == 0 {
-		return false
-	}
-	rq, aq := req.Question[0], answer.Question[0]
-	return rq.Qtype == aq.Qtype && rq.Qclass == aq.Qclass && strings.EqualFold(rq.Name, aq.Name)
-}
-
-// questionString renders a message's first question as "name/type" for logging.
-func questionString(msg *dns.Msg) string {
-	if msg == nil || len(msg.Question) == 0 {
-		return "<none>"
-	}
-	q := msg.Question[0]
-	return q.Name + "/" + dns.TypeToString[q.Qtype]
-}
-
 func setCachedAnswerTTL(answer *dns.Msg, now, expiredTime time.Time) {
 	ttlSecs := expiredTime.Sub(now).Seconds()
 	if ttlSecs < 0 {
@@ -1337,8 +1299,7 @@ func isPrivatePtrLookup(m *dns.Msg) bool {
 			return addr.IsPrivate() ||
 				addr.IsLoopback() ||
 				addr.IsLinkLocalUnicast() ||
-				tsaddr.CGNATRange().Contains(addr) ||
-				isServiceContinuityAddr(addr)
+				tsaddr.CGNATRange().Contains(addr)
 		}
 	}
 	return false
@@ -1376,20 +1337,6 @@ func isLanHostname(name string) bool {
 		strings.HasSuffix(name, ".local")
 }
 
-// ipv4ServiceContinuityPrefix is the RFC 7335 IPv4 Service Continuity Prefix
-// (192.0.0.0/29), used by the CLAT in 464XLAT/DS-Lite transition setups. On such
-// networks (common on IPv6-only cellular carriers and iPhone hotspots) the local
-// machine's DNS queries reach ctrld with a source in this range (e.g. 192.0.0.2),
-// so they must be treated as local, not WAN. Go's netip.IsPrivate does not cover
-// this range — the same reason the CGNAT range is special-cased below. See #552.
-var ipv4ServiceContinuityPrefix = netip.MustParsePrefix("192.0.0.0/29")
-
-// isServiceContinuityAddr reports whether ip is in the RFC 7335 IPv4 Service
-// Continuity Prefix (464XLAT/DS-Lite CLAT).
-func isServiceContinuityAddr(ip netip.Addr) bool {
-	return ipv4ServiceContinuityPrefix.Contains(ip)
-}
-
 // isWanClient reports whether the input is a WAN address.
 func isWanClient(na net.Addr) bool {
 	var ip netip.Addr
@@ -1400,8 +1347,7 @@ func isWanClient(na net.Addr) bool {
 		!ip.IsPrivate() &&
 		!ip.IsLinkLocalUnicast() &&
 		!ip.IsLinkLocalMulticast() &&
-		!tsaddr.CGNATRange().Contains(ip) &&
-		!isServiceContinuityAddr(ip)
+		!tsaddr.CGNATRange().Contains(ip)
 }
 
 // isIPv6LoopbackListener reports whether the listener address is [::1].
@@ -1787,15 +1733,6 @@ func (p *prog) checkUpstreamOnce(upstream string, uc *ctrld.UpstreamConfig) erro
 			mainLog.Load().Debug().Err(err).Msgf("Upstream %s check failed after %v (WFP loopback protect active)", upstream, duration)
 			return errOsHealthcheckSuppressed
 		}
-		// A no-route/network-unreachable failure means the endpoint's address
-		// family is available locally but unroutable (e.g. an IPv6 DoH endpoint
-		// while IPv6 is up but has no route). These repeat until the route
-		// returns and are handled by bounded backoff in the recovery loop, so
-		// keep them at debug to avoid sustained error-log spam.
-		if ctrldnet.IsUnreachable(err) {
-			mainLog.Load().Debug().Err(err).Msgf("Upstream %s check failed after %v (network unreachable)", upstream, duration)
-			return err
-		}
 		mainLog.Load().Error().Err(err).Msgf("Upstream %s check failed after %v", upstream, duration)
 		return err
 	}
@@ -2000,9 +1937,6 @@ func (p *prog) handleRecovery(reason RecoveryReason) {
 // waitForUpstreamRecovery checks the provided upstreams concurrently until one recovers.
 // It returns the name of the recovered upstream or an error if the check times out.
 func (p *prog) waitForUpstreamRecovery(ctx context.Context, upstreams map[string]*ctrld.UpstreamConfig) (string, error) {
-	recoveryCtx, cancel := context.WithCancel(ctx)
-	defer cancel()
-
 	recoveredCh := make(chan string, 1)
 	var wg sync.WaitGroup
 
@@ -2014,10 +1948,9 @@ func (p *prog) waitForUpstreamRecovery(ctx context.Context, upstreams map[string
 			defer wg.Done()
 			mainLog.Load().Debug().Msgf("Starting recovery check loop for upstream: %s", name)
 			attempts := 0
-			unreachableStreak := 0
 			for {
 				select {
-				case <-recoveryCtx.Done():
+				case <-ctx.Done():
 					mainLog.Load().Debug().Msgf("Context canceled for upstream %s", name)
 					return
 				default:
@@ -2029,30 +1962,13 @@ func (p *prog) waitForUpstreamRecovery(ctx context.Context, upstreams map[string
 						select {
 						case recoveredCh <- name:
 							mainLog.Load().Debug().Msgf("Sent recovery notification for upstream %s", name)
-							cancel()
 						default:
 							mainLog.Load().Debug().Msg("Recovery channel full, another upstream already recovered")
 						}
 						return
 					}
-					// Back off the retry cadence for an unroutable endpoint so a
-					// host with IPv6 up but no route to the IPv6 DoH endpoint does
-					// not re-bootstrap/re-check every checkUpstreamBackoffSleep and
-					// spam the log. The backoff is bounded (checkUpstreamUnreachableBackoffMax)
-					// so the endpoint is still re-probed and recovers when the route
-					// returns; any other failure resets to the base cadence.
-					sleep := checkUpstreamBackoffSleep
-					if ctrldnet.IsUnreachable(err) {
-						unreachableStreak++
-						sleep = unreachableRecoveryBackoff(unreachableStreak)
-						mainLog.Load().Debug().Msgf("Upstream %s unreachable (streak %d), backing off %s before retry", name, unreachableStreak, sleep)
-					} else {
-						unreachableStreak = 0
-						mainLog.Load().Debug().Msgf("Upstream %s check failed, sleeping before retry", name)
-					}
-					if !sleepWithContext(recoveryCtx, sleep) {
-						return
-					}
+					mainLog.Load().Debug().Msgf("Upstream %s check failed, sleeping before retry", name)
+					time.Sleep(checkUpstreamBackoffSleep)
 
 					// if this is the upstreamOS and it's the 3rd attempt (or multiple of 3),
 					// we should try to reinit the OS resolver to ensure we can recover
@@ -2080,17 +1996,6 @@ func (p *prog) waitForUpstreamRecovery(ctx context.Context, upstreams map[string
 	return recovered, nil
 }
 
-func sleepWithContext(ctx context.Context, d time.Duration) bool {
-	timer := time.NewTimer(d)
-	defer timer.Stop()
-	select {
-	case <-timer.C:
-		return true
-	case <-ctx.Done():
-		return false
-	}
-}
-
 // buildRecoveryUpstreams constructs the map of upstream configurations to test.
 // For OS failures we supply the manual OS resolver upstream configuration.
 // For network change or regular failure we use the upstreams defined in p.cfg (ignoring OS).

cmd/cli/dns_proxy_test.go

@@ -405,8 +405,6 @@ func Test_isPrivatePtrLookup(t *testing.T) {
 		{"CGNAT", newDnsMsgPtr("100.66.27.28", t), true},
 		{"Loopback", newDnsMsgPtr("127.0.0.1", t), true},
 		{"Link Local Unicast", newDnsMsgPtr("fe80::69f6:e16e:8bdb:433f", t), true},
-		// RFC 7335 IPv4 Service Continuity Prefix (464XLAT/DS-Lite CLAT), see #552.
-		{"464XLAT CLAT host", newDnsMsgPtr("192.0.0.2", t), true},
 		{"Public IP", newDnsMsgPtr("8.8.8.8", t), false},
 	}
 	for _, tc := range tests {
@@ -454,11 +452,6 @@ func Test_isWanClient(t *testing.T) {
 		{"CGNAT", &net.UDPAddr{IP: net.ParseIP("100.66.27.28")}, false},
 		{"Loopback", &net.UDPAddr{IP: net.ParseIP("127.0.0.1")}, false},
 		{"Link Local Unicast", &net.UDPAddr{IP: net.ParseIP("fe80::69f6:e16e:8bdb:433f")}, false},
-		// RFC 7335 IPv4 Service Continuity Prefix (464XLAT/DS-Lite CLAT), see #552.
-		{"464XLAT PLAT side", &net.UDPAddr{IP: net.ParseIP("192.0.0.1")}, false},
-		{"464XLAT CLAT host", &net.UDPAddr{IP: net.ParseIP("192.0.0.2")}, false},
-		// Outside the /29 but inside 192.0.0.0/24: still WAN (fix is scoped to /29).
-		{"192.0.0.0/24 outside /29", &net.UDPAddr{IP: net.ParseIP("192.0.0.100")}, true},
 		{"Public", &net.UDPAddr{IP: net.ParseIP("8.8.8.8")}, true},
 	}
 	for _, tc := range tests {
@@ -481,33 +474,3 @@ func Test_prog_queryFromSelf(t *testing.T) {
 		p.queryFromSelf("foo")
 	})
 }
-
-func Test_sameQuestion(t *testing.T) {
-	mk := func(name string, qtype uint16) *dns.Msg {
-		m := new(dns.Msg)
-		m.SetQuestion(name, qtype)
-		return m
-	}
-	tests := []struct {
-		name   string
-		req    *dns.Msg
-		answer *dns.Msg
-		want   bool
-	}{
-		{"identical", mk("example.com.", dns.TypeA), mk("example.com.", dns.TypeA), true},
-		{"case insensitive", mk("Example.COM.", dns.TypeA), mk("example.com.", dns.TypeA), true},
-		{"different name", mk("victim.example.", dns.TypeA), mk("attacker.example.", dns.TypeA), false},
-		{"different type", mk("example.com.", dns.TypeA), mk("example.com.", dns.TypeAAAA), false},
-		{"nil req", nil, mk("example.com.", dns.TypeA), false},
-		{"nil answer", mk("example.com.", dns.TypeA), nil, false},
-		{"empty answer question", mk("example.com.", dns.TypeA), new(dns.Msg), false},
-	}
-	for _, tc := range tests {
-		tc := tc
-		t.Run(tc.name, func(t *testing.T) {
-			if got := sameQuestion(tc.req, tc.answer); got != tc.want {
-				t.Errorf("sameQuestion() = %v, want %v", got, tc.want)
-			}
-		})
-	}
-}

cmd/cli/log_writer.go

@@ -318,12 +318,6 @@ func (p *prog) initInternalLogging(writers []io.Writer) {
 
 // needInternalLogging reports whether prog needs to run internal logging.
 func (p *prog) needInternalLogging() bool {
-	// Do not run in silent mode: the user explicitly asked for no logging, so
-	// ctrld must not create or write the persisted internal log file (nor reset
-	// the global level back to debug). See https://github.com/Control-D-Inc/ctrld/issues/320.
-	if silent {
-		return false
-	}
 	// Do not run in non-cd mode.
 	if cdUID == "" {
 		return false

cmd/cli/log_writer_silent_test.go

@@ -1,66 +0,0 @@
-package cli
-
-import (
-	"os"
-	"path/filepath"
-	"testing"
-
-	"github.com/Control-D-Inc/ctrld"
-)
-
-// Test_needInternalLogging_silent is a regression test for
-// https://github.com/Control-D-Inc/ctrld/issues/320: running with --silent must
-// not enable internal logging, otherwise ctrld creates and writes
-// <homedir>/ctrld.log (and, when verbose==0, resets the global level back to
-// debug) despite the user asking for silence.
-func Test_needInternalLogging_silent(t *testing.T) {
-	origSilent, origCdUID := silent, cdUID
-	t.Cleanup(func() { silent, cdUID = origSilent, origCdUID })
-
-	tests := []struct {
-		name    string
-		silent  bool
-		cdUID   string
-		logPath string
-		want    bool
-	}{
-		{"silent suppresses internal logging in cd mode", true, "test-uid", "", false},
-		{"cd mode enables internal logging", false, "test-uid", "", true},
-		{"non-cd mode disabled", false, "", "", false},
-		{"explicit log path disables internal logging", false, "test-uid", "/var/log/ctrld.log", false},
-	}
-	for _, tt := range tests {
-		tt := tt
-		t.Run(tt.name, func(t *testing.T) {
-			silent = tt.silent
-			cdUID = tt.cdUID
-			p := &prog{cfg: &ctrld.Config{}}
-			p.cfg.Service.LogPath = tt.logPath
-			if got := p.needInternalLogging(); got != tt.want {
-				t.Fatalf("needInternalLogging() = %v, want %v", got, tt.want)
-			}
-		})
-	}
-}
-
-// Test_initInternalLogging_silentCreatesNoFile drives the real initInternalLogging
-// path and asserts that a --silent --cd run does not create <homedir>/ctrld.log,
-// which is the observable failure reported in
-// https://github.com/Control-D-Inc/ctrld/issues/320.
-func Test_initInternalLogging_silentCreatesNoFile(t *testing.T) {
-	origSilent, origCdUID, origHomedir := silent, cdUID, homedir
-	t.Cleanup(func() { silent, cdUID, homedir = origSilent, origCdUID, origHomedir })
-
-	dir := t.TempDir()
-	homedir = dir
-	cdUID = "test-uid" // cd mode, which would otherwise enable internal logging
-	silent = true
-
-	p := &prog{cfg: &ctrld.Config{}}
-	p.initInternalLogging(nil)
-
-	logPath := filepath.Join(dir, logFileName)
-	if _, err := os.Stat(logPath); !os.IsNotExist(err) {
-		t.Fatalf("silent mode must not create %s (stat err = %v)", logPath, err)
-	}
-}

cmd/cli/main.go

@@ -56,9 +56,6 @@ const (
 	cdOrgFlagName          = "cd-org"
 	customHostnameFlagName = "custom-hostname"
 	nextdnsFlagName        = "nextdns"
-
-	// autoIface is the sentinel --iface value meaning "use the default gateway interface".
-	autoIface = "auto"
 )
 
 func init() {

cmd/cli/nrpt_recovery_limiter_windows.go

@@ -1,101 +0,0 @@
-//go:build windows
-
-package cli
-
-import (
-	"sync"
-	"time"
-
-	"github.com/Control-D-Inc/ctrld"
-)
-
-const (
-	// Default to current behavior: keep recovering indefinitely unless configured.
-	defaultNRPTRecoveryMaxAttempts = 0
-	defaultNRPTRecoveryCooldown    = 30 * time.Minute
-
-	// Require more than one good health tick before clearing the circuit. A probe can
-	// pass briefly after delete/re-add even when another agent recreates broken NRPT state.
-	nrptRecoveryStableSuccessesToReset = 2
-)
-
-type nrptRecoveryLimiter struct {
-	mu              sync.Mutex
-	attempts        int
-	stableSuccesses int
-	cooldownUntil   time.Time
-	lastSkipLog     time.Time
-}
-
-func nrptRecoveryMaxAttempts(cfg *ctrld.Config) int {
-	if cfg != nil && cfg.Service.NRPTRecoveryMaxAttempts != nil {
-		return *cfg.Service.NRPTRecoveryMaxAttempts
-	}
-	return defaultNRPTRecoveryMaxAttempts
-}
-
-func nrptRecoveryCooldown(cfg *ctrld.Config) time.Duration {
-	if cfg != nil && cfg.Service.NRPTRecoveryCooldown != nil {
-		return *cfg.Service.NRPTRecoveryCooldown
-	}
-	return defaultNRPTRecoveryCooldown
-}
-
-func (l *nrptRecoveryLimiter) allow(now time.Time, cfg *ctrld.Config) (bool, time.Duration) {
-	maxAttempts := nrptRecoveryMaxAttempts(cfg)
-	if maxAttempts <= 0 {
-		return true, 0
-	}
-
-	l.mu.Lock()
-	defer l.mu.Unlock()
-
-	if now.Before(l.cooldownUntil) {
-		return false, l.cooldownUntil.Sub(now)
-	}
-	return true, 0
-}
-
-func (l *nrptRecoveryLimiter) recordRecoveryFlow(now time.Time, cfg *ctrld.Config) {
-	maxAttempts := nrptRecoveryMaxAttempts(cfg)
-	if maxAttempts <= 0 {
-		return
-	}
-
-	cooldown := nrptRecoveryCooldown(cfg)
-	if cooldown <= 0 {
-		cooldown = defaultNRPTRecoveryCooldown
-	}
-
-	l.mu.Lock()
-	defer l.mu.Unlock()
-
-	l.stableSuccesses = 0
-	l.attempts++
-	if l.attempts >= maxAttempts {
-		l.cooldownUntil = now.Add(cooldown)
-	}
-}
-
-func (l *nrptRecoveryLimiter) recordStableSuccess() {
-	l.mu.Lock()
-	defer l.mu.Unlock()
-
-	l.stableSuccesses++
-	if l.stableSuccesses >= nrptRecoveryStableSuccessesToReset {
-		l.attempts = 0
-		l.cooldownUntil = time.Time{}
-		l.lastSkipLog = time.Time{}
-	}
-}
-
-func (l *nrptRecoveryLimiter) shouldLogSkip(now time.Time) bool {
-	l.mu.Lock()
-	defer l.mu.Unlock()
-
-	if l.lastSkipLog.IsZero() || now.Sub(l.lastSkipLog) >= 5*time.Minute {
-		l.lastSkipLog = now
-		return true
-	}
-	return false
-}

cmd/cli/nrpt_recovery_limiter_windows_test.go

@@ -1,74 +0,0 @@
-//go:build windows
-
-package cli
-
-import (
-	"testing"
-	"time"
-
-	"github.com/Control-D-Inc/ctrld"
-)
-
-func TestNRPTRecoveryLimiterCooldownAndStableReset(t *testing.T) {
-	maxAttempts := 2
-	cooldown := 10 * time.Minute
-	cfg := &ctrld.Config{}
-	cfg.Service.NRPTRecoveryMaxAttempts = &maxAttempts
-	cfg.Service.NRPTRecoveryCooldown = &cooldown
-
-	limiter := &nrptRecoveryLimiter{}
-	now := time.Unix(100, 0)
-
-	if ok, wait := limiter.allow(now, cfg); !ok || wait != 0 {
-		t.Fatalf("initial allow = %v, %v; want true, 0", ok, wait)
-	}
-
-	limiter.recordRecoveryFlow(now, cfg)
-	if ok, wait := limiter.allow(now.Add(time.Second), cfg); !ok || wait != 0 {
-		t.Fatalf("allow after first flow = %v, %v; want true, 0", ok, wait)
-	}
-
-	limiter.recordRecoveryFlow(now.Add(2*time.Second), cfg)
-	if ok, wait := limiter.allow(now.Add(3*time.Second), cfg); ok || wait <= 0 {
-		t.Fatalf("allow after max flows = %v, %v; want false, positive wait", ok, wait)
-	}
-
-	limiter.recordStableSuccess()
-	if ok, _ := limiter.allow(now.Add(4*time.Second), cfg); ok {
-		t.Fatal("one stable success cleared cooldown; want cooldown to remain")
-	}
-
-	limiter.recordStableSuccess()
-	if ok, wait := limiter.allow(now.Add(5*time.Second), cfg); !ok || wait != 0 {
-		t.Fatalf("allow after stable reset = %v, %v; want true, 0", ok, wait)
-	}
-}
-
-func TestNRPTRecoveryLimiterDefaultIsUnlimited(t *testing.T) {
-	cfg := &ctrld.Config{}
-	limiter := &nrptRecoveryLimiter{}
-	now := time.Unix(100, 0)
-
-	for i := 0; i < 10; i++ {
-		limiter.recordRecoveryFlow(now.Add(time.Duration(i)*time.Second), cfg)
-	}
-	if ok, wait := limiter.allow(now.Add(time.Hour), cfg); !ok || wait != 0 {
-		t.Fatalf("default allow after recovery flows = %v, %v; want true, 0", ok, wait)
-	}
-}
-
-func TestNRPTRecoveryLimiterUnlimited(t *testing.T) {
-	maxAttempts := 0
-	cfg := &ctrld.Config{}
-	cfg.Service.NRPTRecoveryMaxAttempts = &maxAttempts
-
-	limiter := &nrptRecoveryLimiter{}
-	now := time.Unix(100, 0)
-
-	for i := 0; i < 10; i++ {
-		limiter.recordRecoveryFlow(now.Add(time.Duration(i)*time.Second), cfg)
-	}
-	if ok, wait := limiter.allow(now.Add(time.Hour), cfg); !ok || wait != 0 {
-		t.Fatalf("unlimited allow = %v, %v; want true, 0", ok, wait)
-	}
-}

cmd/cli/prog.go

@@ -34,7 +34,6 @@ import (
 	"github.com/Control-D-Inc/ctrld/internal/clientinfo"
 	"github.com/Control-D-Inc/ctrld/internal/controld"
 	"github.com/Control-D-Inc/ctrld/internal/dnscache"
-	ctrldnet "github.com/Control-D-Inc/ctrld/internal/net"
 	"github.com/Control-D-Inc/ctrld/internal/router"
 	"github.com/Control-D-Inc/ctrld/internal/router/dnsmasq"
 )
@@ -189,21 +188,6 @@ type prog struct {
 	// interception with exponential backoff and auto-heals if broken.
 	pfMonitorRunning atomic.Bool //lint:ignore U1000 used on darwin
 
-	// pfEnsureRunning ensures only one pf anchor validation/restoration runs at a time.
-	// Network-change callbacks, delayed rechecks, and the periodic watchdog can all
-	// converge during macOS interface churn; concurrent pfctl/scutil exec storms can
-	// exhaust process/file limits and make the outage worse.
-	pfEnsureRunning atomic.Bool //lint:ignore U1000 used on darwin
-
-	// pfExecBackoffUntil suppresses pf anchor validation after pfctl/scutil execs
-	// fail due host resource exhaustion (fork unavailable, too many open files).
-	pfExecBackoffUntil atomic.Int64 //lint:ignore U1000 used on darwin
-
-	// pfDelayedRecheckTimers coalesces delayed DNS-intercept rechecks after noisy
-	// network changes. Protected by pfDelayedRecheckMu.
-	pfDelayedRecheckMu     sync.Mutex    //lint:ignore U1000 used on darwin
-	pfDelayedRecheckTimers []*time.Timer //lint:ignore U1000 used on darwin
-
 	// pfProbeExpected holds the domain name of a pending pf interception probe.
 	// When non-empty, the DNS handler checks incoming queries against this value
 	// and signals pfProbeCh if matched. The probe verifies that pf's rdr rules
@@ -337,18 +321,6 @@ func (p *prog) runWait() {
 
 		p.mu.Lock()
 		*p.cfg = *newCfg
-		// In DNS-intercept mode on macOS, the DNS listener is bound once at startup and is
-		// NOT re-bound on reload (see prog.run: serveDNS is started only when !reload). When
-		// the configured/generated port (e.g. 127.0.0.1:53) is unavailable at startup because
-		// mDNSResponder owns *:53, ctrld falls back to an alternate local port (e.g. 5354).
-		// The on-disk config still declares 53, so adopting it here would revert p.cfg to a
-		// port nothing is listening on, and the pf rdr rules/probes rebuilt from p.cfg would
-		// target a dead port. Since a reload cannot move the running listener anyway, keep
-		// p.cfg pointing at the actual bound listener. The on-disk config (written above) is
-		// left unchanged. See #551.
-		if dnsIntercept && runtime.GOOS == "darwin" {
-			preserveBoundListeners(p.cfg.Listener, curListener)
-		}
 		p.mu.Unlock()
 
 		logger.Notice().Msg("reloading config successfully")
@@ -360,41 +332,8 @@ func (p *prog) runWait() {
 	}
 }
 
-// preserveBoundListeners overrides the IP/Port of each listener in newListeners with the
-// actual bound address from curListeners when they differ, logging the divergence. It is used
-// on config reload in DNS-intercept mode where the running listener is never re-bound, so a
-// port change on disk (e.g. reverting a fallback 5354 back to the generated 53) must not be
-// applied to the in-memory config that drives pf rdr rules and probes.
-//
-// Preservation is limited to fallback-eligible (default/unset, i.e. 127.0.0.1:53) listeners.
-// An explicit, non-default listener in the reloaded config is an intentional change that must
-// be applied: tryUpdateListenerConfigIntercept binds explicit listeners exactly (no fallback),
-// and the control-server reload handler detects the IP/port diff to trigger a restart that
-// re-binds. Reverting an explicit change here would make that comparison return 200 instead of
-// 201, silently dropping the new listener. See #551.
-func preserveBoundListeners(newListeners, curListeners map[string]*ctrld.ListenerConfig) {
-	for n, curLc := range curListeners {
-		newLc := newListeners[n]
-		if newLc == nil || curLc == nil {
-			continue
-		}
-		if newLc.IP == curLc.IP && newLc.Port == curLc.Port {
-			continue
-		}
-		if isExplicitInterceptListener(newLc.IP, newLc.Port) {
-			continue
-		}
-		mainLog.Load().Info().
-			Str("configured", net.JoinHostPort(newLc.IP, strconv.Itoa(newLc.Port))).
-			Str("actual", net.JoinHostPort(curLc.IP, strconv.Itoa(curLc.Port))).
-			Msg("DNS intercept: preserving actual bound listener across reload; on-disk config port not applied to running listener")
-		newLc.IP = curLc.IP
-		newLc.Port = curLc.Port
-	}
-}
-
 func (p *prog) preRun() {
-	if iface == autoIface {
+	if iface == "auto" {
 		iface = defaultIfaceName()
 		p.requiredMultiNICsConfig = requiredMultiNICsConfig()
 	}
@@ -1389,14 +1328,13 @@ func errAddrInUse(err error) bool {
 
 var _ = errAddrInUse
 
-// The unreachable winsock errnos (ENETUNREACH/EHOSTUNREACH) are matched via
-// ctrldnet.IsUnreachable, which owns their definitions.
-//
 // https://learn.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2
 var (
 	windowsECONNREFUSED = syscall.Errno(10061)
+	windowsENETUNREACH  = syscall.Errno(10051)
 	windowsEINVAL       = syscall.Errno(10022)
 	windowsEADDRINUSE   = syscall.Errno(10048)
+	windowsEHOSTUNREACH = syscall.Errno(10065)
 )
 
 func errUrlNetworkError(err error) bool {
@@ -1413,14 +1351,14 @@ func errNetworkError(err error) bool {
 		if opErr.Temporary() {
 			return true
 		}
-		if ctrldnet.IsUnreachable(err) {
-			return true
-		}
 		switch {
 		case errors.Is(opErr.Err, syscall.ECONNREFUSED),
 			errors.Is(opErr.Err, syscall.EINVAL),
+			errors.Is(opErr.Err, syscall.ENETUNREACH),
+			errors.Is(opErr.Err, windowsENETUNREACH),
 			errors.Is(opErr.Err, windowsEINVAL),
-			errors.Is(opErr.Err, windowsECONNREFUSED):
+			errors.Is(opErr.Err, windowsECONNREFUSED),
+			errors.Is(opErr.Err, windowsEHOSTUNREACH):
 			return true
 		}
 	}

cmd/cli/prog_test.go

@@ -1,11 +1,7 @@
 package cli
 
 import (
-	"context"
-	"net"
-	"net/url"
 	"runtime"
-	"syscall"
 	"testing"
 	"time"
 
@@ -16,32 +12,6 @@ import (
 	"github.com/Control-D-Inc/ctrld"
 )
 
-func TestErrNetworkErrorTreatsNoRouteAsNetworkError(t *testing.T) {
-	err := &net.OpError{Op: "dial", Net: "tcp", Err: syscall.EHOSTUNREACH}
-	assert.True(t, errNetworkError(err))
-	assert.True(t, errUrlNetworkError(&url.Error{Op: "Get", URL: "https://dns.controld.com", Err: err}))
-}
-
-func TestSleepWithContext(t *testing.T) {
-	assert.True(t, sleepWithContext(context.Background(), time.Millisecond))
-
-	ctx, cancel := context.WithCancel(context.Background())
-	cancel()
-
-	start := time.Now()
-	assert.False(t, sleepWithContext(ctx, time.Minute))
-	assert.Less(t, time.Since(start), 100*time.Millisecond)
-}
-
-func TestUnreachableRecoveryBackoff(t *testing.T) {
-	// Streak starts at the base cadence and doubles each attempt, capped at the max.
-	assert.Equal(t, checkUpstreamBackoffSleep, unreachableRecoveryBackoff(0))
-	assert.Equal(t, checkUpstreamBackoffSleep, unreachableRecoveryBackoff(1))
-	assert.Equal(t, 2*checkUpstreamBackoffSleep, unreachableRecoveryBackoff(2))
-	assert.Equal(t, 4*checkUpstreamBackoffSleep, unreachableRecoveryBackoff(3))
-	assert.Equal(t, checkUpstreamUnreachableBackoffMax, unreachableRecoveryBackoff(100))
-}
-
 func Test_prog_dnsWatchdogEnabled(t *testing.T) {
 	p := &prog{cfg: &ctrld.Config{}}
 

cmd/cli/uninstall_invalid_test.go

@@ -1,61 +0,0 @@
-package cli
-
-import "testing"
-
-// Test_ensureRunningIfaceForInvalidUninstall is a regression test for issue-556:
-// after a reboot, the invalid-device self-uninstall path could run before the
-// running interface was known. Because resetDNS (via resetDNSForRunningIface)
-// silently skips DNS restoration when p.runningIface is empty, the OS was left
-// pointed at ctrld's local listener with no internet after the service was
-// removed. ensureRunningIfaceForInvalidUninstall must populate p.runningIface
-// before resetDNS runs.
-func Test_ensureRunningIfaceForInvalidUninstall(t *testing.T) {
-	// preRun mutates the package-level iface global; restore it after the test.
-	origIface := iface
-	t.Cleanup(func() { iface = origIface })
-
-	// newService needs the package service config; it is safe to build here
-	// because ensureRunningIfaceForInvalidUninstall only queries the (absent)
-	// control socket via runningIface, which returns nil when ctrld is not
-	// running, and performs no DNS or service mutation.
-	s, err := newService(&prog{}, svcConfig)
-	if err != nil {
-		t.Fatalf("newService: %v", err)
-	}
-
-	t.Run("iface flag already resolved but not yet copied", func(t *testing.T) {
-		iface = "eth-test"
-		p := &prog{}
-
-		// Precondition mirrors the buggy post-reboot state: an empty running
-		// interface would make resetDNS skip restoration entirely.
-		if p.runningIface != "" {
-			t.Fatalf("precondition: runningIface = %q, want empty", p.runningIface)
-		}
-
-		ensureRunningIfaceForInvalidUninstall(p, s)
-
-		if p.runningIface == "" {
-			t.Fatal("runningIface still empty after prepare: resetDNS would skip DNS " +
-				"restoration and leave the OS pointed at ctrld's local listener")
-		}
-		if p.runningIface != "eth-test" {
-			t.Fatalf("runningIface = %q, want the resolved iface %q", p.runningIface, "eth-test")
-		}
-	})
-
-	t.Run("iface unset falls back to auto-detected interface", func(t *testing.T) {
-		iface = ""
-		p := &prog{}
-
-		ensureRunningIfaceForInvalidUninstall(p, s)
-
-		// With iface unset the prep resolves "auto" to the default interface
-		// (defaultIfaceName never returns empty on the supported platforms), so
-		// resetDNS has a concrete interface to restore.
-		if p.runningIface == "" {
-			t.Fatal("runningIface still empty after prepare with iface unset: " +
-				"resetDNS would skip DNS restoration")
-		}
-	})
-}

cmd/cli/upstream_monitor.go

@@ -12,27 +12,8 @@ const (
 	maxFailureRequest = 50
 	// checkUpstreamBackoffSleep is the time interval between each upstream checks.
 	checkUpstreamBackoffSleep = 2 * time.Second
-	// checkUpstreamUnreachableBackoffMax caps the recovery retry interval for an
-	// endpoint that keeps failing with a network-unreachable error. It bounds
-	// the backoff so an unroutable endpoint is still re-probed periodically and
-	// recovers once the route returns.
-	checkUpstreamUnreachableBackoffMax = 60 * time.Second
 )
 
-// unreachableRecoveryBackoff returns the retry interval for the given streak of
-// consecutive network-unreachable failures. It starts at checkUpstreamBackoffSleep
-// and doubles each attempt, capped at checkUpstreamUnreachableBackoffMax.
-func unreachableRecoveryBackoff(streak int) time.Duration {
-	d := checkUpstreamBackoffSleep
-	for i := 1; i < streak; i++ {
-		d *= 2
-		if d >= checkUpstreamUnreachableBackoffMax {
-			return checkUpstreamUnreachableBackoffMax
-		}
-	}
-	return d
-}
-
 // upstreamMonitor performs monitoring upstreams health.
 type upstreamMonitor struct {
 	cfg *ctrld.Config

cmd/cli/vpn_dns.go

@@ -6,7 +6,6 @@ import (
 	"runtime"
 	"strings"
 	"sync"
-	"sync/atomic"
 
 	"github.com/rs/zerolog"
 	"tailscale.com/net/netmon"
@@ -51,9 +50,6 @@ type vpnDNSManager struct {
 	// discoverVPNDNS is injected for tests so Refresh does not depend on the
 	// runner host's real VPN/virtual adapter state.
 	discoverVPNDNS func(context.Context) []ctrld.VPNDNSConfig
-	// refreshRunning keeps noisy network-change storms from running overlapping
-	// scutil/networksetup VPN DNS discovery work.
-	refreshRunning atomic.Bool
 	// Called when VPN DNS server list changes, to update intercept exemptions.
 	onServersChanged vpnDNSExemptFunc
 }
@@ -73,11 +69,6 @@ func newVPNDNSManager(exemptFunc vpnDNSExemptFunc) *vpnDNSManager {
 // Called on network change events.
 func (m *vpnDNSManager) Refresh(guardAgainstNoNameservers bool) {
 	logger := mainLog.Load()
-	if !m.refreshRunning.CompareAndSwap(false, true) {
-		logger.Debug().Msg("VPN DNS refresh already running, skipping duplicate")
-		return
-	}
-	defer m.refreshRunning.Store(false)
 
 	logger.Debug().Msg("Refreshing VPN DNS configurations")
 	discoverVPNDNS := m.discoverVPNDNS

cmd/cli/vpn_dns_test.go

@@ -2,8 +2,6 @@ package cli
 
 import (
 	"context"
-	"sync"
-	"sync/atomic"
 	"testing"
 
 	"github.com/Control-D-Inc/ctrld"
@@ -16,36 +14,6 @@ func withVPNDNSSettlingEnabled(t *testing.T) {
 	t.Cleanup(func() { vpnDNSSettlingEnabled = old })
 }
 
-func TestVPNDNSRefreshSkipsConcurrentDuplicate(t *testing.T) {
-	m := newVPNDNSManager(nil)
-	started := make(chan struct{})
-	release := make(chan struct{})
-	done := make(chan struct{})
-	var once sync.Once
-	var calls atomic.Int32
-
-	m.discoverVPNDNS = func(context.Context) []ctrld.VPNDNSConfig {
-		calls.Add(1)
-		once.Do(func() { close(started) })
-		<-release
-		return nil
-	}
-
-	go func() {
-		defer close(done)
-		m.Refresh(true)
-	}()
-
-	<-started
-	m.Refresh(true)
-	close(release)
-	<-done
-
-	if calls.Load() != 1 {
-		t.Fatalf("expected overlapping refresh to be skipped, got %d discovery calls", calls.Load())
-	}
-}
-
 func TestVPNDNSRefreshRetainsStateForOneGuardedEmptyDiscovery(t *testing.T) {
 	withVPNDNSSettlingEnabled(t)
 	var gotExemptions []vpnDNSExemption

config.go

@@ -241,8 +241,6 @@ type ServiceConfig struct {
 	ForceRefetchWaitTime    *int           `mapstructure:"force_refetch_wait_time" toml:"force_refetch_wait_time,omitempty"`
 	LeakOnUpstreamFailure   *bool          `mapstructure:"leak_on_upstream_failure" toml:"leak_on_upstream_failure,omitempty"`
 	InterceptMode           string         `mapstructure:"intercept_mode" toml:"intercept_mode,omitempty" validate:"omitempty,oneof=off dns hard"`
-	NRPTRecoveryMaxAttempts *int           `mapstructure:"nrpt_recovery_max_attempts" toml:"nrpt_recovery_max_attempts,omitempty" validate:"omitempty,gte=0"`
-	NRPTRecoveryCooldown    *time.Duration `mapstructure:"nrpt_recovery_cooldown" toml:"nrpt_recovery_cooldown,omitempty"`
 	Daemon                  bool           `mapstructure:"-" toml:"-"`
 	AllocateIP              bool           `mapstructure:"-" toml:"-"`
 }

dns.go

@@ -14,12 +14,6 @@ func SetCacheReply(answer, msg *dns.Msg, code int) {
 		// See https://datatracker.ietf.org/doc/html/rfc7873#section-4
 		sCookie.Cookie = cCookie.Cookie[:16] + sCookie.Cookie[16:]
 	}
-	// NOTE: the answer's EDNS Client Subnet (ECS) is intentionally left as the
-	// upstream returned it. Correctness across clients is guaranteed by
-	// partitioning the cache and singleflight keys by ECS (see
-	// dnscache.CanonicalECS), so a cache hit only ever serves an answer that was
-	// resolved for the requester's own subnet. Rewriting the ECS option here
-	// without re-scoping the Answer records would violate RFC 7871 §7.3.
 }
 
 // getEdns0Cookie returns Edns0 cookie from *dns.OPT if present.

dns_test.go

@@ -1,57 +0,0 @@
-package ctrld
-
-import (
-	"net"
-	"testing"
-
-	"github.com/miekg/dns"
-)
-
-// Test_SetCacheReply_DoesNotRewriteECS documents the post-#564 contract: cross-client
-// correctness is guaranteed by partitioning the cache/singleflight keys by ECS
-// (dnscache.CanonicalECS), NOT by rewriting the cached answer's ECS option. Rewriting the
-// ECS metadata while leaving the Answer records scoped to another subnet would violate
-// RFC 7871 §7.3 and make forwarders accept a wrong-subnet answer. SetCacheReply must
-// therefore leave the answer's ECS untouched.
-func Test_SetCacheReply_DoesNotRewriteECS(t *testing.T) {
-	answer := new(dns.Msg)
-	answer.SetQuestion(dns.Fqdn("controld.com"), dns.TypeA)
-	answer.SetEdns0(4096, false)
-	cachedSubnet := &dns.EDNS0_SUBNET{
-		Code:          dns.EDNS0SUBNET,
-		Family:        2,
-		SourceNetmask: 64,
-		SourceScope:   64,
-		Address:       net.ParseIP("2001:db8:1::"),
-	}
-	answer.IsEdns0().Option = append(answer.IsEdns0().Option, cachedSubnet)
-
-	req := new(dns.Msg)
-	req.SetQuestion(dns.Fqdn("controld.com"), dns.TypeA)
-	req.SetEdns0(4096, true)
-	req.IsEdns0().Option = append(req.IsEdns0().Option, &dns.EDNS0_SUBNET{
-		Code:          dns.EDNS0SUBNET,
-		Family:        2,
-		SourceNetmask: 64,
-		Address:       net.ParseIP("2001:db8:2::"),
-	})
-
-	SetCacheReply(answer, req, dns.RcodeSuccess)
-
-	var got *dns.EDNS0_SUBNET
-	for _, o := range answer.IsEdns0().Option {
-		if e, ok := o.(*dns.EDNS0_SUBNET); ok {
-			got = e
-			break
-		}
-	}
-	if got == nil {
-		t.Fatal("SetCacheReply dropped the answer's ECS option")
-	}
-	if want := net.ParseIP("2001:db8:1::"); !got.Address.Equal(want) {
-		t.Fatalf("SetCacheReply rewrote the answer ECS to the requester's subnet: got %v, want %v (unchanged)", got.Address, want)
-	}
-	if got.SourceScope != 64 {
-		t.Fatalf("SetCacheReply altered the answer ECS scope: got %d, want 64 (unchanged)", got.SourceScope)
-	}
-}

docs/config.md

@@ -295,22 +295,6 @@ If a remote upstream fails to resolve a query or is unreachable, `ctrld` will fo
 - Required: no
 - Default: true on Windows, MacOS and non-router Linux.
 
-### nrpt_recovery_max_attempts
-Windows DNS intercept mode uses NRPT health probes and recovery when Windows stops routing queries to the local `ctrld` listener. This limits how many consecutive recovery flows can run before `ctrld` enters a cooldown and stops making policy/Dnscache changes.
-
-Set to `0` to disable this circuit breaker and keep retrying indefinitely.
-
-- Type: integer
-- Required: no
-- Default: 0 (unlimited, current behavior)
-
-### nrpt_recovery_cooldown
-Cooldown duration after `nrpt_recovery_max_attempts` consecutive Windows NRPT recovery flows. During cooldown, `ctrld` logs the suppressed recovery and avoids additional `RefreshPolicyEx`, Dnscache `paramchange`, and DNS cache flush calls.
-
-- Type: time duration string
-- Required: no
-- Default: 30m
-
 ## Upstream
 The `[upstream]` section specifies the DNS upstream servers that `ctrld` will forward DNS requests to.
 

docs/wfp-dns-intercept.md

@@ -91,20 +91,6 @@ ctrld uses an adaptive strategy (matching [Tailscale's approach](https://github.
    the empty GP parent key. This ensures DNS Client stays in "local mode" where
    the local-path rule activates immediately via `paramchange`.
 
-### Reproducing the Empty GP Parent Case
-
-This is a production code reference, so the temporary repro script is not kept in
-the repository. For MR !942 review, the test script and exact before/after steps
-are posted in the MR discussion. The scenario to compare is:
-
-1. Run the same approved PowerShell repro script against a pre-fix build and this
-   branch with the same ctrld config.
-2. Create an empty GP NRPT parent key while ctrld is running in DNS intercept mode.
-3. Confirm pre-fix logs can spend policy refresh/paramchange retries while the GP
-   parent remains empty.
-4. Confirm post-fix logs clean the empty GP parent, send one NRPT-change signal,
-   and re-probe before normal retries.
-
 ### VPN Coexistence
 
 NRPT uses most-specific-match. VPN NRPT rules for specific domains (e.g.,

doh_test.go

@@ -12,6 +12,7 @@ import (
 	"net/url"
 	"runtime"
 	"strings"
+	"sync/atomic"
 	"testing"
 	"time"
 
@@ -267,62 +268,33 @@ func newTestHTTP3Server(t *testing.T, handler http.Handler) *testHTTP3Server {
 	return h3Server
 }
 
-// blockingBodyHandler writes exactly nbytes of body with the given status,
-// flushes them, then blocks until release is closed WITHOUT ever returning.
-// Because the handler does not return, the response stream is never terminated
-// (no EOF/FIN). A client that stops after a bounded prefix therefore completes,
-// while a client that reads to EOF blocks. Tests set nbytes to the exact read
-// cap so the client consumes the whole written body (no half-written frame is
-// left blocking on flow control) yet still never sees EOF.
-func blockingBodyHandler(status, nbytes int, release <-chan struct{}) http.HandlerFunc {
+// oversizedDoHHandler streams `bodyBytes` bytes of zeros with the given
+// HTTP status. The atomic counter records bytes the handler actually
+// wrote, so tests can confirm the client tore down the stream before
+// consuming the whole attacker-controlled body.
+func oversizedDoHHandler(status int, bodyBytes int64, written *atomic.Int64) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Set("Content-Type", headerApplicationDNS)
 		w.WriteHeader(status)
-		if _, err := w.Write(make([]byte, nbytes)); err != nil {
-			return
+		chunk := make([]byte, 64*1024)
+		var sent int64
+		for sent < bodyBytes {
+			n := int64(len(chunk))
+			if remaining := bodyBytes - sent; remaining < n {
+				n = remaining
+			}
+			m, err := w.Write(chunk[:n])
+			if err != nil {
+				return
+			}
+			sent += int64(m)
+			if written != nil {
+				written.Add(int64(m))
+			}
+			if f, ok := w.(http.Flusher); ok {
+				f.Flush()
+			}
 		}
-		if f, ok := w.(http.Flusher); ok {
-			f.Flush()
-		}
-		<-release
-	}
-}
-
-// requireBoundedResolve asserts that r.Resolve returns the expected size/status
-// error while the server is still withholding EOF (the handler is blocked in
-// blockingBodyHandler). Returning under those conditions proves ctrld read only
-// a bounded prefix of the body: a resolver that instead read to EOF would block
-// on the withheld stream and trip the deadline. This is the deterministic
-// regression guard for the issue-312 OOM protections, replacing the earlier
-// flaky server-side byte counter (issue-561).
-func requireBoundedResolve(t *testing.T, r Resolver, msg *dns.Msg, wantErrSubstr string) {
-	t.Helper()
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-	defer cancel()
-
-	type result struct {
-		answer *dns.Msg
-		err    error
-	}
-	done := make(chan result, 1)
-	go func() {
-		answer, err := r.Resolve(ctx, msg)
-		done <- result{answer, err}
-	}()
-
-	select {
-	case res := <-done:
-		if res.err == nil {
-			t.Fatalf("Resolve unexpectedly succeeded; answer=%v", res.answer)
-		}
-		if !strings.Contains(res.err.Error(), wantErrSubstr) {
-			t.Fatalf("error %q does not contain %q", res.err, wantErrSubstr)
-		}
-		if res.answer != nil {
-			t.Fatalf("Resolve returned non-nil answer alongside error: %v", res.answer)
-		}
-	case <-time.After(5 * time.Second):
-		t.Fatal("Resolve did not return while the server withheld EOF: the body is being read to EOF instead of a bounded prefix (issue-312 OOM protection missing)")
 	}
 }
 
@@ -376,12 +348,9 @@ func doh3UpstreamForAddr(t *testing.T, addr string, cert *x509.Certificate) *Ups
 // returning a body larger than the DNS protocol allows must be rejected
 // with an explicit size error rather than buffered into ctrld memory.
 func TestDoHResolve_OversizedBody_Rejected(t *testing.T) {
-	// Write exactly the LimitReader cap, then withhold EOF. ctrld's bounded
-	// read (io.LimitReader of dohMaxResponseSize+1) returns after this prefix;
-	// an unbounded read would block on the missing EOF and trip the deadline.
-	release := make(chan struct{})
-	defer close(release)
-	srv := httptest.NewUnstartedServer(blockingBodyHandler(http.StatusOK, dohMaxResponseSize+1, release))
+	const oversized = 2 * 1024 * 1024 // far past dohMaxResponseSize (~64 KiB)
+	var written atomic.Int64
+	srv := httptest.NewUnstartedServer(oversizedDoHHandler(http.StatusOK, oversized, &written))
 	testCert := generateTestCertificate(t)
 	srv.TLS = &tls.Config{
 		Certificates: []tls.Certificate{testCert.tlsCert},
@@ -401,19 +370,31 @@ func TestDoHResolve_OversizedBody_Rejected(t *testing.T) {
 	msg.SetQuestion("example.com.", dns.TypeA)
 	msg.RecursionDesired = true
 
-	requireBoundedResolve(t, r, msg, "maximum DNS message size")
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	answer, err := r.Resolve(ctx, msg)
+	if err == nil {
+		t.Fatalf("Resolve unexpectedly succeeded; answer=%v", answer)
+	}
+	if !strings.Contains(err.Error(), "maximum DNS message size") {
+		t.Fatalf("error %q does not mention the size cap", err)
+	}
+	if answer != nil {
+		t.Fatalf("Resolve returned non-nil answer alongside error: %v", answer)
+	}
+	if got := written.Load(); got >= int64(oversized) {
+		t.Fatalf("server wrote the entire %d-byte body before client tore down (wrote=%d) — cap not effective", oversized, got)
+	}
 }
 
 // TestDoHResolve_NonOKStatus_BoundedErrorBody locks in that a non-200
 // response with a huge body does not pull the body fully into ctrld
 // memory just to format an error string.
 func TestDoHResolve_NonOKStatus_BoundedErrorBody(t *testing.T) {
-	// Same synchronization as the oversized-body test, but at the error-body
-	// cap: the non-200 path reads through an io.LimitReader of
-	// dohMaxErrorBodySize, so it must return after this prefix without EOF.
-	release := make(chan struct{})
-	defer close(release)
-	srv := httptest.NewUnstartedServer(blockingBodyHandler(http.StatusBadGateway, dohMaxErrorBodySize, release))
+	const huge = 8 * 1024 * 1024
+	var written atomic.Int64
+	srv := httptest.NewUnstartedServer(oversizedDoHHandler(http.StatusBadGateway, huge, &written))
 	testCert := generateTestCertificate(t)
 	srv.TLS = &tls.Config{
 		Certificates: []tls.Certificate{testCert.tlsCert},
@@ -433,22 +414,36 @@ func TestDoHResolve_NonOKStatus_BoundedErrorBody(t *testing.T) {
 	msg.SetQuestion("example.com.", dns.TypeA)
 	msg.RecursionDesired = true
 
-	requireBoundedResolve(t, r, msg, "status: 502")
+	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
+	defer cancel()
+
+	answer, err := r.Resolve(ctx, msg)
+	if err == nil {
+		t.Fatalf("Resolve unexpectedly succeeded; answer=%v", answer)
+	}
+	if !strings.Contains(err.Error(), "status: 502") {
+		t.Fatalf("error %q does not surface the upstream status", err)
+	}
+	if answer != nil {
+		t.Fatalf("Resolve returned non-nil answer alongside error: %v", answer)
+	}
+	if got := written.Load(); got > 1024*1024 {
+		t.Fatalf("server wrote %d bytes before client tore down — error path is reading too much body", got)
+	}
 }
 
 // TestDoHResolve_OversizedBody_DoH3 mirrors the DoH oversized-body check
 // on the HTTP/3 transport, since github-312 specifically reproduced the
 // OOM via DoH3.
 func TestDoHResolve_OversizedBody_DoH3(t *testing.T) {
-	release := make(chan struct{})
-	defer close(release)
+	const oversized = 2 * 1024 * 1024
 	testCert := generateTestCertificate(t)
 	udpConn, err := net.ListenUDP("udp", &net.UDPAddr{IP: net.ParseIP("127.0.0.1"), Port: 0})
 	if err != nil {
 		t.Fatalf("udp listen: %v", err)
 	}
 	h3 := &http3.Server{
-		Handler: blockingBodyHandler(http.StatusOK, dohMaxResponseSize+1, release),
+		Handler: oversizedDoHHandler(http.StatusOK, oversized, nil),
 		TLSConfig: &tls.Config{
 			Certificates: []tls.Certificate{testCert.tlsCert},
 			NextProtos:   []string{"h3"},
@@ -476,5 +471,17 @@ func TestDoHResolve_OversizedBody_DoH3(t *testing.T) {
 	msg.SetQuestion("example.com.", dns.TypeA)
 	msg.RecursionDesired = true
 
-	requireBoundedResolve(t, r, msg, "maximum DNS message size")
+	ctx, cancel := context.WithTimeout(context.Background(), 15*time.Second)
+	defer cancel()
+
+	answer, err := r.Resolve(ctx, msg)
+	if err == nil {
+		t.Fatalf("Resolve unexpectedly succeeded; answer=%v", answer)
+	}
+	if !strings.Contains(err.Error(), "maximum DNS message size") {
+		t.Fatalf("error %q does not mention the size cap", err)
+	}
+	if answer != nil {
+		t.Fatalf("Resolve returned non-nil answer alongside error: %v", answer)
+	}
 }

doq.go

@@ -242,15 +242,7 @@ func (p *doqConnPool) doResolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, er
 	if err := answer.Unpack(buf[2 : 2+respLen]); err != nil {
 		return nil, err
 	}
-	// RFC 9250 section 4.2.1 requires the DNS Message ID to be 0 on the wire,
-	// so restore the downstream transaction ID for the client. Do NOT use
-	// SetReply here: it rewrites the RCODE to NOERROR and overwrites the
-	// Question with the request's, which would mask upstream failures from the
-	// failover logic (a SERVFAIL would look like success) and let a
-	// wrong-question answer pass validation and poison the cache. Preserve the
-	// upstream RCODE, Question, and answer sections untouched so the proxy can
-	// evaluate them. See github.com/Control-D-Inc/ctrld/issues/322.
-	answer.Id = msg.Id
+	answer.SetReply(msg)
 	return answer, nil
 }
 

doq_test.go

@@ -714,105 +714,3 @@ func TestDoQResolve_OversizedResponse_Rejected(t *testing.T) {
 		t.Fatalf("Resolve returned non-nil answer alongside error: %v", answer)
 	}
 }
-
-// frameDoQResponse packs msg and prepends the RFC 9250 2-octet length prefix,
-// producing the exact bytes a DoQ server writes on the wire.
-func frameDoQResponse(t *testing.T, msg *dns.Msg) []byte {
-	t.Helper()
-	b, err := msg.Pack()
-	if err != nil {
-		t.Fatalf("pack response: %v", err)
-	}
-	n := uint16(len(b))
-	return append([]byte{byte(n >> 8), byte(n & 0xFF)}, b...)
-}
-
-// TestDoQResolve_PreservesRcode locks in the fix for
-// github.com/Control-D-Inc/ctrld/issues/322: the DoQ resolver must not rewrite
-// an upstream response with SetReply, which would clobber a SERVFAIL into
-// NOERROR and hide the failure from the proxy's failover logic. The upstream
-// RCODE must survive; only the transaction ID is restored for the client.
-func TestDoQResolve_PreservesRcode(t *testing.T) {
-	t.Parallel()
-
-	// RFC 9250 puts the DNS Message ID at 0 on the wire.
-	resp := new(dns.Msg)
-	resp.SetQuestion("example.com.", dns.TypeA)
-	resp.Response = true
-	resp.Id = 0
-	resp.Rcode = dns.RcodeServerFailure
-
-	server := newMalformedDoQServer(t, frameDoQResponse(t, resp))
-	uc := newMalformedDoQUpstream(t, server.cert, server.addr)
-
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-	defer cancel()
-
-	pool := newDOQConnPool(uc, []string{"127.0.0.1"})
-	t.Cleanup(pool.CloseIdleConnections)
-
-	msg := new(dns.Msg)
-	msg.SetQuestion("example.com.", dns.TypeA)
-	msg.RecursionDesired = true
-
-	answer, err := pool.Resolve(ctx, msg)
-	if err != nil {
-		t.Fatalf("Resolve failed: %v", err)
-	}
-	if answer.Rcode != dns.RcodeServerFailure {
-		t.Fatalf("upstream SERVFAIL was rewritten to %s; failover would be bypassed",
-			dns.RcodeToString[answer.Rcode])
-	}
-	if answer.Id != msg.Id {
-		t.Fatalf("transaction ID not restored: got %d, want %d", answer.Id, msg.Id)
-	}
-}
-
-// TestDoQResolve_PreservesWrongQuestion locks in the fix for
-// github.com/Control-D-Inc/ctrld/issues/322: when an upstream answers a
-// different name than asked, the resolver must preserve the upstream's
-// question rather than rewriting it to the request's question (as SetReply
-// did). Rewriting would hide the mismatch and let wrong-domain records poison
-// the shared cache.
-func TestDoQResolve_PreservesWrongQuestion(t *testing.T) {
-	t.Parallel()
-
-	resp := new(dns.Msg)
-	resp.SetQuestion("attacker.example.", dns.TypeA)
-	resp.Response = true
-	resp.Id = 0
-	resp.Answer = append(resp.Answer, &dns.A{
-		Hdr: dns.RR_Header{
-			Name:   "attacker.example.",
-			Rrtype: dns.TypeA,
-			Class:  dns.ClassINET,
-			Ttl:    300,
-		},
-		A: net.ParseIP("192.0.2.1"),
-	})
-
-	server := newMalformedDoQServer(t, frameDoQResponse(t, resp))
-	uc := newMalformedDoQUpstream(t, server.cert, server.addr)
-
-	ctx, cancel := context.WithTimeout(context.Background(), 10*time.Second)
-	defer cancel()
-
-	pool := newDOQConnPool(uc, []string{"127.0.0.1"})
-	t.Cleanup(pool.CloseIdleConnections)
-
-	msg := new(dns.Msg)
-	msg.SetQuestion("victim.example.", dns.TypeA)
-	msg.RecursionDesired = true
-
-	answer, err := pool.Resolve(ctx, msg)
-	if err != nil {
-		t.Fatalf("Resolve failed: %v", err)
-	}
-	if len(answer.Question) == 0 || !strings.EqualFold(answer.Question[0].Name, "attacker.example.") {
-		t.Fatalf("upstream question was rewritten; got %v, want the upstream's attacker.example.",
-			answer.Question)
-	}
-	if answer.Id != msg.Id {
-		t.Fatalf("transaction ID not restored: got %d, want %d", answer.Id, msg.Id)
-	}
-}

go.mod

@@ -37,7 +37,7 @@ require (
 	github.com/stretchr/testify v1.11.1
 	github.com/vishvananda/netlink v1.3.1
 	golang.org/x/net v0.56.0
-	golang.org/x/sync v0.22.0
+	golang.org/x/sync v0.21.0
 	golang.org/x/sys v0.46.0
 	golang.zx2c4.com/wireguard/windows v0.5.3
 	tailscale.com v1.74.0
@@ -94,9 +94,9 @@ require (
 	go4.org/netipx v0.0.0-20231129151722-fdeea329fbba // indirect
 	golang.org/x/crypto v0.53.0 // indirect
 	golang.org/x/exp v0.0.0-20240119083558-1b970713d09a // indirect
-	golang.org/x/mod v0.37.0 // indirect
-	golang.org/x/text v0.40.0 // indirect
-	golang.org/x/tools v0.47.0 // indirect
+	golang.org/x/mod v0.36.0 // indirect
+	golang.org/x/text v0.38.0 // indirect
+	golang.org/x/tools v0.45.0 // indirect
 	google.golang.org/protobuf v1.33.0 // indirect
 	gopkg.in/ini.v1 v1.67.0 // indirect
 	gopkg.in/yaml.v3 v3.0.1 // indirect

go.sum

@@ -386,8 +386,8 @@ golang.org/x/mod v0.2.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.3.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.0/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
 golang.org/x/mod v0.4.1/go.mod h1:s0Qsj1ACt9ePp/hMypM3fl4fZqREWJwdYDEqhRiZZUA=
-golang.org/x/mod v0.37.0 h1:vF1DjpVEshcIqoEaauuHebaLk1O1forxjxBaVn884JQ=
-golang.org/x/mod v0.37.0/go.mod h1:m8S8VeM9r4dzDwjrKO0a1sZP3YjeMamRRlD+fmR2Q/0=
+golang.org/x/mod v0.36.0 h1:JJjpVx6myfUsUdAzZuOSTTmRE0PfZeNWzzvKrP7amb4=
+golang.org/x/mod v0.36.0/go.mod h1:moc6ELqsWcOw5Ef3xVprK5ul/MvtVvkIXLziUOICjUQ=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
@@ -441,8 +441,8 @@ golang.org/x/sync v0.0.0-20200317015054-43a5402ce75a/go.mod h1:RxMgew5VJxzue5/jJ
 golang.org/x/sync v0.0.0-20200625203802-6e8e738ad208/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20201020160332-67f06af15bc9/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
 golang.org/x/sync v0.0.0-20201207232520-09787c993a3a/go.mod h1:RxMgew5VJxzue5/jJTE5uejpjVlOe/izrB70Jof72aM=
-golang.org/x/sync v0.22.0 h1:SZjpbeLmrCk4xhRSZFNZW5gFUeCeFgjekvI/+gfScek=
-golang.org/x/sync v0.22.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
+golang.org/x/sync v0.21.0 h1:HLII4xRRTtCRkxYp4HNFF0Js/Og6q2i++KXbg0gHCwM=
+golang.org/x/sync v0.21.0/go.mod h1:9xrNwdLfx4jkKbNva9FpL6vEN7evnE43NNNJQ2LF3+0=
 golang.org/x/sys v0.0.0-20180830151530-49385e6e1522/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190215142949-d0b11bdaac8a/go.mod h1:STP8DvDyc/dI5b8T5hshtkjS+E42TnysNCUPdjciGhY=
 golang.org/x/sys v0.0.0-20190312061237-fead79001313/go.mod h1:h1NjWce9XRLGQEsW7wpKNCjG9DtNlClVuFLEZdDNbEs=
@@ -504,8 +504,8 @@ golang.org/x/text v0.3.3/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.4/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.6/go.mod h1:5Zoc/QRtKVWzQhOtBMvqHzDpF6irO9z98xDceosuGiQ=
 golang.org/x/text v0.3.7/go.mod h1:u+2+/6zg+i71rQMx5EYifcz6MCKuco9NR6JIITiCfzQ=
-golang.org/x/text v0.40.0 h1:Ub2Z6/xjgF1WrYQz2nuITOEegKFtiIy+rieRJ5lHZKs=
-golang.org/x/text v0.40.0/go.mod h1:hpnzDAfGV753zIKo+wk3u1bVKCGPbrnF7+7LBF/UHVY=
+golang.org/x/text v0.38.0 h1:sXmwo9DwP3OK9EZ7PqAdaooSGozfl/3a6/xJcbzPRhE=
+golang.org/x/text v0.38.0/go.mod h1:YXZt3QhHUKYT53r2lLKFIVi6Ao1jdzrTR/KQ09qyxF4=
 golang.org/x/time v0.0.0-20181108054448-85acf8d2951c/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/time v0.0.0-20190308202827-9d24e82272b4/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
 golang.org/x/time v0.0.0-20191024005414-555d28b269f0/go.mod h1:tRJNPiyCQ0inRvYxbN9jk5I+vvW/OXSQhTDSoE431IQ=
@@ -556,8 +556,8 @@ golang.org/x/tools v0.0.0-20201208233053-a543418bbed2/go.mod h1:emZCQorbCU4vsT4f
 golang.org/x/tools v0.0.0-20210105154028-b0ab187a4818/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
 golang.org/x/tools v0.0.0-20210108195828-e2f9c7f1fc8e/go.mod h1:emZCQorbCU4vsT4fOWvOPXz4eW1wZW4PmDk9uLelYpA=
 golang.org/x/tools v0.1.0/go.mod h1:xkSsbof2nBLbhDlRMhhhyNLN/zl3eTqcnHD5viDpcZ0=
-golang.org/x/tools v0.47.0 h1:7Kn5x/d1svx/PzryTsqeoZN4TZwqeH5pGWjefhLi/1Q=
-golang.org/x/tools v0.47.0/go.mod h1:dFHnyTvFWY212G+h7ZY4Vsp/K3U4/7W9TyVaAul8uCA=
+golang.org/x/tools v0.45.0 h1:18qN3FAooORvApf5XjCXgsuayZOEtXf6JK18I3+ONa8=
+golang.org/x/tools v0.45.0/go.mod h1:LuUGqqaXcXMEFEruIVJVm5mgDD8vww/z/SR1gQ4uE/0=
 golang.org/x/xerrors v0.0.0-20190717185122-a985d3407aa7/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191011141410-1b5146add898/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=
 golang.org/x/xerrors v0.0.0-20191204190536-9bdfabe68543/go.mod h1:I/5z698sn9Ka8TeJc9MKroUUfqBBauWjQqLJ2OPfmY0=

internal/dnscache/cache.go

@@ -1,8 +1,6 @@
 package dnscache
 
 import (
-	"fmt"
-	"net"
 	"strings"
 	"time"
 
@@ -18,17 +16,11 @@ type Cacher interface {
 }
 
 // Key is the caching key for DNS message.
-//
-// ECS partitions the cache by EDNS Client Subnet so an answer resolved for one
-// subnet is never served to a client in a different subnet. Answer records are
-// scoped to the network that generated them (RFC 7871 §7.3), so they must not
-// be shared across subnets even when the question is otherwise identical.
 type Key struct {
 	Qtype    uint16
 	Qclass   uint16
 	Name     string
 	Upstream string
-	ECS      string
 }
 
 type Value struct {
@@ -68,58 +60,7 @@ func NewLRUCache(size int) (*LRUCache, error) {
 // NewKey creates a new cache key for given DNS message.
 func NewKey(msg *dns.Msg, upstream string) Key {
 	q := msg.Question[0]
-	return Key{Qtype: q.Qtype, Qclass: q.Qclass, Name: normalizeQname(q.Name), Upstream: upstream, ECS: CanonicalECS(msg)}
-}
-
-// CanonicalECS returns a canonical string form of the EDNS Client Subnet (ECS,
-// EDNS option 8) carried by msg, suitable for partitioning cache and
-// singleflight keys. A request with no ECS option returns "", so all ECS-less
-// queries share one partition and behave as before.
-//
-// A request that DOES carry an ECS option is never mapped to "", even at SOURCE
-// PREFIX-LENGTH 0: the /0 query is forwarded with an ECS option, may draw
-// different upstream data than an ECS-less query, and its answer (with the
-// echoed option) must not be served to a client that sent no ECS — RFC 7871
-// §7.3.1 requires /0-cached data to remain distinguishable. The family is part
-// of the token, so an IPv4 /0 and an IPv6 /0 stay distinct too.
-//
-// The address is masked to its source prefix length so only the significant
-// subnet bits contribute to the key: two clients in the same subnet share a
-// partition, while different subnets (or address families) never do. The
-// response-only SCOPE PREFIX-LENGTH is deliberately excluded — it is not part of
-// what the client asked for.
-func CanonicalECS(msg *dns.Msg) string {
-	opt := msg.IsEdns0()
-	if opt == nil {
-		return ""
-	}
-	for _, o := range opt.Option {
-		e, ok := o.(*dns.EDNS0_SUBNET)
-		if !ok {
-			continue
-		}
-		bits := int(e.SourceNetmask)
-		total := 128
-		zero := net.IPv6zero
-		if e.Family == 1 {
-			total = 32
-			zero = net.IPv4zero
-		}
-		addr := e.Address
-		if len(addr) == 0 {
-			// A /0 request commonly carries an empty address; normalize it to
-			// the family zero so its token is stable regardless of encoding.
-			addr = zero
-		}
-		masked := addr
-		if bits <= total {
-			if m := addr.Mask(net.CIDRMask(bits, total)); m != nil {
-				masked = m
-			}
-		}
-		return fmt.Sprintf("%d/%d/%s", e.Family, e.SourceNetmask, masked.String())
-	}
-	return ""
+	return Key{Qtype: q.Qtype, Qclass: q.Qclass, Name: normalizeQname(q.Name), Upstream: upstream}
 }
 
 // NewValue creates a new cache value for given DNS message.

internal/dnscache/cache_test.go

@@ -1,186 +0,0 @@
-package dnscache
-
-import (
-	"net"
-	"testing"
-	"time"
-
-	"github.com/miekg/dns"
-)
-
-// msgWithECS builds an A query for name carrying an EDNS Client Subnet option, or none
-// when family == 0.
-func msgWithECS(name string, family uint16, prefix uint8, addr string) *dns.Msg {
-	m := new(dns.Msg)
-	m.SetQuestion(dns.Fqdn(name), dns.TypeA)
-	if family == 0 {
-		return m
-	}
-	m.SetEdns0(4096, true)
-	m.IsEdns0().Option = append(m.IsEdns0().Option, &dns.EDNS0_SUBNET{
-		Code:          dns.EDNS0SUBNET,
-		Family:        family,
-		SourceNetmask: prefix,
-		Address:       net.ParseIP(addr),
-	})
-	return m
-}
-
-// answerWithA builds a cached answer holding a single A record with the given address.
-func answerWithA(name, a string) *dns.Msg {
-	m := new(dns.Msg)
-	m.SetQuestion(dns.Fqdn(name), dns.TypeA)
-	rr, err := dns.NewRR(dns.Fqdn(name) + " 300 IN A " + a)
-	if err != nil {
-		panic(err)
-	}
-	m.Answer = []dns.RR{rr}
-	return m
-}
-
-func firstA(msg *dns.Msg) string {
-	for _, rr := range msg.Answer {
-		if a, ok := rr.(*dns.A); ok {
-			return a.A.String()
-		}
-	}
-	return ""
-}
-
-// TestCanonicalECS covers the key-partitioning helper: only a request with no ECS option
-// collapses to the shared empty partition, while a carried /0 keeps its own family-scoped
-// token (distinct from no-ECS, per RFC 7871 §7.3.1); host bits below the source prefix do
-// not fragment the key, and different subnets / families produce different keys.
-func TestCanonicalECS(t *testing.T) {
-	tests := []struct {
-		name string
-		msg  *dns.Msg
-		want string
-	}{
-		{"no ecs", msgWithECS("controld.com", 0, 0, ""), ""},
-		// A carried ECS option is never "" (RFC 7871 §7.3.1), and IPv4 /0 vs
-		// IPv6 /0 stay distinct; the address encoding must not change the token.
-		{"ipv4 /0", msgWithECS("controld.com", 1, 0, "0.0.0.0"), "1/0/0.0.0.0"},
-		{"ipv4 /0 empty addr", msgWithECS("controld.com", 1, 0, ""), "1/0/0.0.0.0"},
-		{"ipv6 /0", msgWithECS("controld.com", 2, 0, "::"), "2/0/::"},
-		{"ipv6 /0 empty addr", msgWithECS("controld.com", 2, 0, ""), "2/0/::"},
-		{"ipv4 /24", msgWithECS("controld.com", 1, 24, "203.0.113.0"), "1/24/203.0.113.0"},
-		{"ipv4 host bits masked", msgWithECS("controld.com", 1, 24, "203.0.113.7"), "1/24/203.0.113.0"},
-		{"ipv6 /64", msgWithECS("controld.com", 2, 64, "2001:db8:1::"), "2/64/2001:db8:1::"},
-		{"ipv6 host bits masked", msgWithECS("controld.com", 2, 64, "2001:db8:1::dead:beef"), "2/64/2001:db8:1::"},
-	}
-	for _, tt := range tests {
-		t.Run(tt.name, func(t *testing.T) {
-			if got := CanonicalECS(tt.msg); got != tt.want {
-				t.Fatalf("CanonicalECS = %q, want %q", got, tt.want)
-			}
-		})
-	}
-}
-
-// TestNewKey_ECSPartition verifies the cache key distinguishes subnets while collapsing
-// same-subnet requests, so the LRU cache cannot serve one subnet's records to another.
-func TestNewKey_ECSPartition(t *testing.T) {
-	const up = "https://dns.example/dns-query"
-	subnetA := NewKey(msgWithECS("controld.com", 2, 64, "2001:db8:1::"), up)
-	subnetB := NewKey(msgWithECS("controld.com", 2, 64, "2001:db8:2::"), up)
-	subnetAHost := NewKey(msgWithECS("controld.com", 2, 64, "2001:db8:1::5"), up)
-	ipv4 := NewKey(msgWithECS("controld.com", 1, 24, "203.0.113.0"), up)
-	noECS := NewKey(msgWithECS("controld.com", 0, 0, ""), up)
-
-	if subnetA == subnetB {
-		t.Fatal("different subnets must not share a cache key")
-	}
-	if subnetA != subnetAHost {
-		t.Fatal("same subnet (different host bits) must share a cache key")
-	}
-	if subnetA == ipv4 || subnetA == noECS || ipv4 == noECS {
-		t.Fatal("different families / no-ECS must not collide")
-	}
-}
-
-// TestLRUCache_ECSZeroPrefixDistinctFromNoECS is the regression test for the /0-vs-no-ECS
-// collision (RFC 7871 §7.3.1). A query carrying an ECS /0 option is forwarded WITH ECS and
-// may draw different upstream data, so its cached answer must never be served to a client
-// that sent no ECS at all, nor may IPv4 /0 and IPv6 /0 cross-serve each other.
-func TestLRUCache_ECSZeroPrefixDistinctFromNoECS(t *testing.T) {
-	c, err := NewLRUCache(16)
-	if err != nil {
-		t.Fatalf("NewLRUCache: %v", err)
-	}
-	const up = "https://dns.example/dns-query"
-	expire := time.Now().Add(time.Minute)
-
-	noECS := msgWithECS("controld.com", 0, 0, "")
-	zeroV4 := msgWithECS("controld.com", 1, 0, "0.0.0.0")
-	zeroV6 := msgWithECS("controld.com", 2, 0, "::")
-
-	// Only the IPv4 /0 query's answer is cached.
-	c.Add(NewKey(zeroV4, up), NewValue(answerWithA("controld.com", "192.0.2.1"), expire))
-
-	// A no-ECS client must NOT receive the ECS /0 cached answer.
-	if got := c.Get(NewKey(noECS, up)); got != nil {
-		t.Fatalf("no-ECS client received the ECS /0 cached record %q", firstA(got.Msg))
-	}
-	// An IPv6 /0 client must NOT receive the IPv4 /0 answer.
-	if got := c.Get(NewKey(zeroV6, up)); got != nil {
-		t.Fatalf("IPv6 /0 client received the IPv4 /0 cached record %q", firstA(got.Msg))
-	}
-	// The IPv4 /0 client still hits its own entry.
-	if got := c.Get(NewKey(zeroV4, up)); got == nil || firstA(got.Msg) != "192.0.2.1" {
-		t.Fatalf("IPv4 /0 lost its own cached record: %v", got)
-	}
-
-	// The no-ECS partition is independent and does not corrupt the /0 entry.
-	c.Add(NewKey(noECS, up), NewValue(answerWithA("controld.com", "198.51.100.1"), expire))
-	if got := c.Get(NewKey(noECS, up)); got == nil || firstA(got.Msg) != "198.51.100.1" {
-		t.Fatalf("no-ECS partition wrong record: %v", got)
-	}
-	if got := c.Get(NewKey(zeroV4, up)); got == nil || firstA(got.Msg) != "192.0.2.1" {
-		t.Fatalf("IPv4 /0 record corrupted by no-ECS insert: %v", got)
-	}
-}
-
-// TestLRUCache_ECSNoCrossSubnetServe is the real cache-path regression test for #564:
-// an entry populated for subnet A must never be returned to a client in subnet B, even
-// though the question (name/type/class/upstream) is identical. The two subnets carry
-// different A records; the second client must get its own record or a miss, never A's.
-func TestLRUCache_ECSNoCrossSubnetServe(t *testing.T) {
-	c, err := NewLRUCache(16)
-	if err != nil {
-		t.Fatalf("NewLRUCache: %v", err)
-	}
-	const up = "https://dns.example/dns-query"
-	expire := time.Now().Add(time.Minute)
-
-	reqA := msgWithECS("controld.com", 2, 64, "2001:db8:1::")
-	reqB := msgWithECS("controld.com", 2, 64, "2001:db8:2::")
-
-	// Only subnet A's answer (A record 192.0.2.1) is cached.
-	c.Add(NewKey(reqA, up), NewValue(answerWithA("controld.com", "192.0.2.1"), expire))
-
-	// A client in subnet B must NOT hit subnet A's entry.
-	if got := c.Get(NewKey(reqB, up)); got != nil {
-		t.Fatalf("subnet B received subnet A's cached record %q; cache is not ECS-partitioned", firstA(got.Msg))
-	}
-
-	// Subnet A still hits its own entry.
-	if got := c.Get(NewKey(reqA, up)); got == nil || firstA(got.Msg) != "192.0.2.1" {
-		t.Fatalf("subnet A lost its own cached record: %+v", got)
-	}
-
-	// Now cache subnet B's distinct answer and confirm the two never cross.
-	c.Add(NewKey(reqB, up), NewValue(answerWithA("controld.com", "198.51.100.1"), expire))
-	if got := c.Get(NewKey(reqB, up)); got == nil || firstA(got.Msg) != "198.51.100.1" {
-		t.Fatalf("subnet B got the wrong record: %v", got)
-	}
-	if got := c.Get(NewKey(reqA, up)); got == nil || firstA(got.Msg) != "192.0.2.1" {
-		t.Fatalf("subnet A record corrupted by subnet B insert: %v", got)
-	}
-
-	// A second host within subnet A shares the partition (cache hit with A's record).
-	reqAHost := msgWithECS("controld.com", 2, 64, "2001:db8:1::9")
-	if got := c.Get(NewKey(reqAHost, up)); got == nil || firstA(got.Msg) != "192.0.2.1" {
-		t.Fatalf("same-subnet host missed the shared cache entry: %v", got)
-	}
-}

internal/net/net.go

@@ -157,101 +157,6 @@ type parallelDialerResult struct {
 	err  error
 }
 
-const (
-	// unreachableBackoffBase is the initial suppression window applied to a
-	// dial address after it returns a network-unreachable error (e.g.
-	// "connect: no route to host"). The window grows exponentially up to
-	// unreachableBackoffMax on repeated failures, and is cleared as soon as
-	// the address dials successfully.
-	unreachableBackoffBase = 5 * time.Second
-	// unreachableBackoffMax caps the suppression window so an address is
-	// always re-probed within a bounded interval, preserving recovery when
-	// the route comes back.
-	unreachableBackoffMax = 60 * time.Second
-)
-
-// Windows winsock codes for the unreachable errnos. A failing connect on
-// Windows surfaces these raw WSA codes (WSAENETUNREACH/WSAEHOSTUNREACH), whereas
-// syscall.ENETUNREACH/EHOSTUNREACH are Go's portable "invented" values
-// (APPLICATION_ERROR + iota) that never equal them. Matching these explicitly is
-// therefore required for the classifier to detect unreachable errors on Windows;
-// errors.Is against the syscall.* constants alone would not.
-//
-// https://learn.microsoft.com/en-us/windows/win32/winsock/windows-sockets-error-codes-2
-var (
-	windowsENETUNREACH  = syscall.Errno(10051)
-	windowsEHOSTUNREACH = syscall.Errno(10065)
-)
-
-// IsUnreachable reports whether err indicates the destination network or host
-// has no route (ENETUNREACH/EHOSTUNREACH). These are the errors produced when
-// an endpoint's address family is available locally but unroutable, e.g. an
-// IPv6 DoH endpoint while the host has IPv6 but no route to it.
-func IsUnreachable(err error) bool {
-	if err == nil {
-		return false
-	}
-	var opErr *net.OpError
-	if errors.As(err, &opErr) {
-		return errors.Is(opErr.Err, syscall.ENETUNREACH) ||
-			errors.Is(opErr.Err, syscall.EHOSTUNREACH) ||
-			errors.Is(opErr.Err, windowsENETUNREACH) ||
-			errors.Is(opErr.Err, windowsEHOSTUNREACH)
-	}
-	return false
-}
-
-type unreachableEntry struct {
-	until   time.Time
-	backoff time.Duration
-}
-
-// unreachableTracker records dial addresses that recently failed with a
-// network-unreachable error so ParallelDialer can temporarily stop hammering
-// them. This prevents an unroutable endpoint from generating a sustained dial
-// /health-check storm, while still re-probing each address once its bounded
-// backoff window expires so genuine recovery is never permanently blocked.
-type unreachableTracker struct {
-	mu      sync.Mutex
-	entries map[string]unreachableEntry
-}
-
-var unreachable = &unreachableTracker{entries: make(map[string]unreachableEntry)}
-
-// suppressed reports whether addr is currently within its unreachable backoff
-// window and should be skipped.
-func (t *unreachableTracker) suppressed(addr string, now time.Time) bool {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-	e, ok := t.entries[addr]
-	return ok && now.Before(e.until)
-}
-
-// markUnreachable extends the suppression window for addr using a bounded
-// exponential backoff.
-func (t *unreachableTracker) markUnreachable(addr string, now time.Time) {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-	e := t.entries[addr]
-	if e.backoff == 0 {
-		e.backoff = unreachableBackoffBase
-	} else {
-		e.backoff *= 2
-		if e.backoff > unreachableBackoffMax {
-			e.backoff = unreachableBackoffMax
-		}
-	}
-	e.until = now.Add(e.backoff)
-	t.entries[addr] = e
-}
-
-// markReachable clears any suppression for addr after a successful dial.
-func (t *unreachableTracker) markReachable(addr string) {
-	t.mu.Lock()
-	defer t.mu.Unlock()
-	delete(t.entries, addr)
-}
-
 type ParallelDialer struct {
 	net.Dialer
 }
@@ -260,63 +165,26 @@ func (d *ParallelDialer) DialContext(ctx context.Context, network string, addrs
 	if len(addrs) == 0 {
 		return nil, errors.New("empty addresses")
 	}
-
-	// Skip addresses that recently returned a network-unreachable error so an
-	// unroutable endpoint (e.g. an IPv6 DoH address while the host has IPv6
-	// but no route to it) does not generate a sustained dial storm. Suppression
-	// is bounded: once an address's backoff window expires it is re-probed, so
-	// genuine recovery is preserved.
-	now := time.Now()
-	live := make([]string, 0, len(addrs))
-	var suppressed int
-	for _, addr := range addrs {
-		if unreachable.suppressed(addr, now) {
-			suppressed++
-			continue
-		}
-		live = append(live, addr)
-	}
-	if len(live) == 0 {
-		// Every candidate is within its unreachable backoff window. Fail fast
-		// and quietly instead of re-dialing known-unroutable addresses; the
-		// windows expire and re-probe shortly, so recovery still happens.
-		logger.Debug().Msgf("skipping %d unreachable address(es), all in backoff", suppressed)
-		// TODO: the errno here is hardcoded to EHOSTUNREACH, but the actual
-		// failure that triggered suppression may have been ENETUNREACH. This is
-		// harmless today (IsUnreachable treats both the same and nothing else
-		// inspects the errno), but if these errors are ever recorded/reported we
-		// should retain the real error in unreachableEntry and surface it here.
-		return nil, &net.OpError{Op: "dial", Net: network, Err: syscall.EHOSTUNREACH}
-	}
-	if suppressed > 0 {
-		logger.Debug().Msgf("skipping %d unreachable address(es) in backoff", suppressed)
-	}
-
 	ctx, cancel := context.WithCancel(ctx)
 	defer cancel()
 
 	done := make(chan struct{})
 	defer close(done)
-	ch := make(chan *parallelDialerResult, len(live))
+	ch := make(chan *parallelDialerResult, len(addrs))
 	var wg sync.WaitGroup
-	wg.Add(len(live))
+	wg.Add(len(addrs))
 	go func() {
 		wg.Wait()
 		close(ch)
 	}()
 
-	for _, addr := range live {
+	for _, addr := range addrs {
 		go func(addr string) {
 			defer wg.Done()
 			logger.Debug().Msgf("dialing to %s", addr)
 			conn, err := d.Dialer.DialContext(ctx, network, addr)
 			if err != nil {
 				logger.Debug().Msgf("failed to dial %s: %v", addr, err)
-				if IsUnreachable(err) {
-					unreachable.markUnreachable(addr, time.Now())
-				}
-			} else {
-				unreachable.markReachable(addr)
 			}
 			select {
 			case ch <- &parallelDialerResult{conn: conn, err: err}:

internal/net/net_test.go

@@ -2,77 +2,10 @@ package net
 
 import (
 	"context"
-	"net"
-	"syscall"
 	"testing"
 	"time"
 )
 
-func TestIsUnreachable(t *testing.T) {
-	tests := []struct {
-		name string
-		err  error
-		want bool
-	}{
-		{"nil", nil, false},
-		{"enetunreach", &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ENETUNREACH}, true},
-		{"ehostunreach", &net.OpError{Op: "dial", Net: "tcp", Err: syscall.EHOSTUNREACH}, true},
-		{"windows enetunreach", &net.OpError{Op: "dial", Net: "tcp", Err: windowsENETUNREACH}, true},
-		{"windows ehostunreach", &net.OpError{Op: "dial", Net: "tcp", Err: windowsEHOSTUNREACH}, true},
-		{"connection refused", &net.OpError{Op: "dial", Net: "tcp", Err: syscall.ECONNREFUSED}, false},
-		{"not an opError", syscall.ENETUNREACH, false},
-	}
-	for _, tc := range tests {
-		tc := tc
-		t.Run(tc.name, func(t *testing.T) {
-			if got := IsUnreachable(tc.err); got != tc.want {
-				t.Errorf("IsUnreachable(%v) = %v, want %v", tc.err, got, tc.want)
-			}
-		})
-	}
-}
-
-func TestUnreachableTracker(t *testing.T) {
-	tr := &unreachableTracker{entries: make(map[string]unreachableEntry)}
-	const addr = "[2606:1a40::22]:443"
-	now := time.Unix(0, 0)
-
-	// Not suppressed before any failure.
-	if tr.suppressed(addr, now) {
-		t.Fatal("addr suppressed before any failure")
-	}
-
-	// First failure suppresses for the base window.
-	tr.markUnreachable(addr, now)
-	if !tr.suppressed(addr, now.Add(unreachableBackoffBase-time.Millisecond)) {
-		t.Fatal("addr not suppressed within base backoff window")
-	}
-	if tr.suppressed(addr, now.Add(unreachableBackoffBase)) {
-		t.Fatal("addr still suppressed at end of base backoff window")
-	}
-
-	// Backoff grows exponentially and is capped at the max.
-	tr.markUnreachable(addr, now)
-	if got := tr.entries[addr].backoff; got != 2*unreachableBackoffBase {
-		t.Fatalf("backoff after second failure = %v, want %v", got, 2*unreachableBackoffBase)
-	}
-	for i := 0; i < 10; i++ {
-		tr.markUnreachable(addr, now)
-	}
-	if got := tr.entries[addr].backoff; got != unreachableBackoffMax {
-		t.Fatalf("backoff not capped: got %v, want %v", got, unreachableBackoffMax)
-	}
-
-	// A successful dial clears suppression entirely.
-	tr.markReachable(addr)
-	if tr.suppressed(addr, now) {
-		t.Fatal("addr still suppressed after markReachable")
-	}
-	if _, ok := tr.entries[addr]; ok {
-		t.Fatal("entry not removed after markReachable")
-	}
-}
-
 func TestProbeStackTimeout(t *testing.T) {
 	done := make(chan struct{})
 	started := make(chan struct{})

resolver.go

@@ -19,8 +19,6 @@ import (
 	"golang.org/x/sync/singleflight"
 	"tailscale.com/net/netmon"
 	"tailscale.com/net/tsaddr"
-
-	"github.com/Control-D-Inc/ctrld/internal/dnscache"
 )
 
 const (
@@ -47,12 +45,8 @@ const (
 
 const controldPublicDns = "76.76.2.0"
 
-const maxConcurrentOSResolverExchanges = 128
-
 var controldPublicDnsWithPort = net.JoinHostPort(controldPublicDns, "53")
 
-var osResolverExchangeSem = make(chan struct{}, maxConcurrentOSResolverExchanges)
-
 var localResolver Resolver
 
 func init() {
@@ -142,15 +136,14 @@ 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 {
-	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{}
 	}
 	ns := initializeOsResolver(nameservers)
+	resolverMutex.Lock()
+	defer resolverMutex.Unlock()
 	or = newResolverWithNameserver(ns)
 	return ns
 }
@@ -380,10 +373,8 @@ func (o *osResolver) Resolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, error
 	domain := strings.TrimSuffix(msg.Question[0].Name, ".")
 	qtype := msg.Question[0].Qtype
 
-	// Unique key for the singleflight group. The EDNS Client Subnet is part of
-	// the key so subnet-specific answers are neither coalesced nor hot-cached
-	// across different subnets (RFC 7871 §7.3).
-	key := fmt.Sprintf("%s:%d:%s", domain, qtype, dnscache.CanonicalECS(msg))
+	// Unique key for the singleflight group.
+	key := fmt.Sprintf("%s:%d:", domain, qtype)
 
 	// Checking the cache first.
 	if val, ok := o.cache.Load(key); ok {
@@ -475,13 +466,6 @@ func (o *osResolver) resolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, error
 		for _, server := range servers {
 			go func(server string) {
 				defer wg.Done()
-				release, ok := acquireOSResolverExchangeSlot(ctx)
-				if !ok {
-					ch <- &osResolverResult{err: ctx.Err(), server: server, lan: isLan}
-					return
-				}
-				defer release()
-
 				var answer *dns.Msg
 				var err error
 				var localOSResolverIP net.IP
@@ -592,15 +576,6 @@ func (o *osResolver) resolve(ctx context.Context, msg *dns.Msg) (*dns.Msg, error
 	return nil, errors.Join(errs...)
 }
 
-func acquireOSResolverExchangeSlot(ctx context.Context) (func(), bool) {
-	select {
-	case osResolverExchangeSem <- struct{}{}:
-		return func() { <-osResolverExchangeSem }, true
-	case <-ctx.Done():
-		return nil, false
-	}
-}
-
 func (o *osResolver) removeCache(key string) {
 	o.cache.Delete(key)
 }

resolver_test.go

@@ -282,93 +282,6 @@ func Test_Edns0_CacheReply(t *testing.T) {
 	}
 }
 
-// ecsAnswerHandler returns a distinct A record per EDNS Client Subnet, so a test can
-// prove one subnet never receives another subnet's cached record. It counts upstream
-// calls to confirm the hot cache/singleflight is partitioned by ECS rather than shared.
-func ecsAnswerHandler(call *atomic.Int64) dns.HandlerFunc {
-	return func(w dns.ResponseWriter, msg *dns.Msg) {
-		call.Add(1)
-		a := "203.0.113.1" // no/other subnet
-		if opt := msg.IsEdns0(); opt != nil {
-			for _, o := range opt.Option {
-				if e, ok := o.(*dns.EDNS0_SUBNET); ok {
-					switch {
-					case e.Address.Equal(net.ParseIP("2001:db8:1::")):
-						a = "192.0.2.1"
-					case e.Address.Equal(net.ParseIP("2001:db8:2::")):
-						a = "198.51.100.1"
-					}
-				}
-			}
-		}
-		m := new(dns.Msg)
-		m.SetReply(msg)
-		rr, _ := dns.NewRR(msg.Question[0].Name + " 300 IN A " + a)
-		m.Answer = []dns.RR{rr}
-		w.WriteMsg(m)
-	}
-}
-
-// Test_osResolver_HotCache_ECSPartition is the real cache-path regression test for #564 on
-// the osResolver hot cache / singleflight path: the upstream returns a different A record
-// per subnet, and a client in subnet B must never be served subnet A's hot-cached record.
-func Test_osResolver_HotCache_ECSPartition(t *testing.T) {
-	lanPC, err := net.ListenPacket("udp", "127.0.0.1:0")
-	if err != nil {
-		t.Fatalf("failed to listen on LAN address: %v", err)
-	}
-	call := &atomic.Int64{}
-	lanServer, lanAddr, err := runLocalPacketConnTestServer(t, lanPC, ecsAnswerHandler(call))
-	if err != nil {
-		t.Fatalf("failed to run LAN test server: %v", err)
-	}
-	defer lanServer.Shutdown()
-
-	or := newResolverWithNameserver([]string{lanAddr})
-	query := func(subnet string) string {
-		m := new(dns.Msg)
-		m.SetQuestion(dns.Fqdn("controld.com"), dns.TypeA)
-		m.RecursionDesired = true
-		m.SetEdns0(4096, true)
-		m.IsEdns0().Option = append(m.IsEdns0().Option, &dns.EDNS0_SUBNET{
-			Code:          dns.EDNS0SUBNET,
-			Family:        2,
-			SourceNetmask: 64,
-			Address:       net.ParseIP(subnet),
-		})
-		answer, err := or.Resolve(context.Background(), m)
-		if err != nil {
-			t.Fatal(err)
-		}
-		for _, rr := range answer.Answer {
-			if a, ok := rr.(*dns.A); ok {
-				return a.A.String()
-			}
-		}
-		return ""
-	}
-
-	// Subnet A populates the hot cache; a repeat hits it (upstream called once).
-	if got := query("2001:db8:1::"); got != "192.0.2.1" {
-		t.Fatalf("subnet A: got %q, want 192.0.2.1", got)
-	}
-	if got := query("2001:db8:1::"); got != "192.0.2.1" {
-		t.Fatalf("subnet A repeat: got %q, want 192.0.2.1", got)
-	}
-	if call.Load() != 1 {
-		t.Fatalf("subnet A repeat did not hit the hot cache: %d upstream calls", call.Load())
-	}
-
-	// Subnet B must get ITS OWN record, not subnet A's hot-cached one, and this
-	// requires a fresh upstream call (the cache is partitioned, not shared).
-	if got := query("2001:db8:2::"); got != "198.51.100.1" {
-		t.Fatalf("subnet B was served the wrong record %q (want 198.51.100.1); hot cache is not ECS-partitioned", got)
-	}
-	if call.Load() != 2 {
-		t.Fatalf("subnet B unexpectedly served from subnet A's cache: %d upstream calls, want 2", call.Load())
-	}
-}
-
 // https://github.com/Control-D-Inc/ctrld/issues/255
 func Test_legacyResolverWithBigExtraSection(t *testing.T) {
 	lanPC, err := net.ListenPacket("udp", "127.0.0.1:0") // 127.0.0.1 is considered LAN (loopback)
@@ -470,11 +383,6 @@ func nonSuccessHandlerWithRcode(rcode int) dns.HandlerFunc {
 
 func countHandler(call *atomic.Int64) dns.HandlerFunc {
 	return func(w dns.ResponseWriter, msg *dns.Msg) {
-		// Count the call before writing the reply. The client returns as soon
-		// as it receives the response, so a caller that reads this counter right
-		// after Resolve returns would race an increment done after WriteMsg and
-		// could observe a stale zero.
-		call.Add(1)
 		m := new(dns.Msg)
 		m.SetRcode(msg, dns.RcodeSuccess)
 		if cookie := getEdns0Cookie(msg.IsEdns0()); cookie != nil {
@@ -487,6 +395,7 @@ func countHandler(call *atomic.Int64) dns.HandlerFunc {
 			m.IsEdns0().Option = append(m.IsEdns0().Option, cookieOption)
 		}
 		w.WriteMsg(m)
+		call.Add(1)
 	}
 }