windows: adopt GP-managed NRPT catch-all

cmd/cli/dns_intercept_darwin.go

@@ -19,6 +19,10 @@ import (
 	"github.com/Control-D-Inc/ctrld"
 )
 
+// skipInitialDNSReset is Windows-only; macOS keeps the normal adapter cleanup
+// before installing its pf redirect.
+func (p *prog) skipInitialDNSReset() bool { return false }
+
 const (
 	// pfWatchdogInterval is how often the periodic pf watchdog checks
 	// that our anchor references are still present in the running ruleset.
@@ -1855,14 +1859,9 @@ func (p *prog) probePFIntercept() bool {
 	// Generate unique probe domain
 	probeID := fmt.Sprintf("_pf-probe-%x.%s", time.Now().UnixNano()&0xFFFFFFFF, pfProbeDomain)
 
-	// Register probe so DNS handler can detect and signal it
-	probeCh := make(chan struct{}, 1)
-	p.pfProbeExpected.Store(probeID)
-	p.pfProbeCh.Store(&probeCh)
-	defer func() {
-		p.pfProbeExpected.Store("")
-		p.pfProbeCh.Store((*chan struct{})(nil))
-	}()
+	// Register this attempt's own domain: overlapping probes must not cancel each other.
+	probeCh, deregister := p.registerInterceptProbe(probeID)
+	defer deregister()
 
 	// Build a minimal DNS query packet for the probe domain.
 	// We use exec.Command to send from a subprocess with GID=0 (wheel),

cmd/cli/dns_intercept_lifecycle_windows_test.go

@@ -0,0 +1,319 @@
+//go:build windows
+
+package cli
+
+import (
+	"runtime"
+	"testing"
+	"time"
+)
+
+// newInterceptTestProg returns a prog with a published intercept state, fake NRPT
+// operations already installed, and no WFP engine (engineHandle 0).
+//
+// The fake is installed here, before anything can inspect registry state, and it is the
+// safety boundary - not the empty wfpState. A zero-valued state has owner None, and
+// shutdown's None branch sweeps orphaned ctrld rules, so an unfaked stopDNSIntercept would
+// reach the production nrptCatchAllRuleExists / removeNRPTCatchAllRule / signalNRPTChange.
+// On a host that has ctrld's deterministic key - a developer box, or a CI runner where
+// ctrld is installed - that deletes live policy and forces a Group Policy refresh, a
+// Dnscache paramchange and a cache flush. A green run on a clean runner proves nothing
+// about that.
+func newInterceptTestProg(t *testing.T) (*prog, *wfpState, *fakeNRPTOps) {
+	t.Helper()
+	f := fakeNRPTOpsForTest(t)
+	// Prove the fake is in effect before anything can inspect registry state. Asserting
+	// zero side effects afterwards cannot do that: an uninstalled fake reports zero
+	// whether it was consulted or bypassed.
+	requireFakeNRPTOpsInstalled(t, f)
+	state := &wfpState{stopCh: make(chan struct{}), listenerIP: "127.0.0.1"}
+	p := &prog{}
+	p.dnsInterceptState = state
+	return p, state, f
+}
+
+// assertNoNRPTSideEffects fails when a lifecycle path wrote NRPT policy or signalled the
+// DNS Client. Every test in this file exercises a guard that is supposed to stand down, so
+// any registry write or signal here means the guard did not hold - and, without the fake,
+// would have hit the host's real policy.
+func assertNoNRPTSideEffects(t *testing.T, f *fakeNRPTOps) {
+	t.Helper()
+	add, remove, signal, _ := f.counts()
+	if add != 0 || remove != 0 || signal != 0 {
+		t.Errorf("addRule = %d, removeRule = %d, signal = %d, want 0/0/0: this path must not write NRPT policy",
+			add, remove, signal)
+	}
+	if flush := f.flushCount(); flush != 0 {
+		t.Errorf("flush calls = %d, want 0: this path must not flush the resolver cache", flush)
+	}
+}
+
+// TestStopDNSInterceptRevokesBeforeTeardown pins the ordering the shutdown/monitor race
+// depends on. Teardown deletes our WFP sublayer, and a missing sublayer is precisely what
+// the health monitor treats as "our filters were wiped, rebuild everything". Were the
+// state revoked only after teardown, a monitor tick inside that window would rebuild the
+// intercept during shutdown.
+func TestStopDNSInterceptRevokesBeforeTeardown(t *testing.T) {
+	p, state, f := newInterceptTestProg(t)
+	defer assertNoNRPTSideEffects(t, f)
+
+	if p.interceptStateRevoked(state) {
+		t.Fatal("a freshly published intercept state must not read as retired")
+	}
+	if err := p.stopDNSIntercept(); err != nil {
+		t.Fatalf("stopDNSIntercept() = %v", err)
+	}
+	if !p.interceptStateRevoked(state) {
+		t.Error("state still reads live after shutdown: the monitor and heal flows would keep writing host DNS state")
+	}
+	if p.dnsInterceptState != nil {
+		t.Error("dnsInterceptState survived shutdown")
+	}
+	if p.dnsInterceptStopRequested.Load() {
+		t.Error("stop-requested flag was left set; a later start would see a phantom shutdown")
+	}
+}
+
+// TestRebuildDNSInterceptRefusedAfterShutdown is the regression test for the reported
+// race: SCM stop runs resetDNS -> stopDNSIntercept while the health monitor is mid-tick,
+// and the monitor then reaches the rebuild path before the process exits. The rebuild
+// must refuse - completing it would re-add the NRPT catch-all and the WFP filters moments
+// before ctrld disappears, leaving Windows resolving through a listener that is gone.
+//
+// That refusal is also what keeps this test safe on a real Windows host: a rebuild that
+// did not refuse would run startDNSIntercept and write NRPT policy to the machine
+// running the tests.
+func TestRebuildDNSInterceptRefusedAfterShutdown(t *testing.T) {
+	p, state, f := newInterceptTestProg(t)
+	defer assertNoNRPTSideEffects(t, f)
+	if err := p.stopDNSIntercept(); err != nil {
+		t.Fatalf("stopDNSIntercept() = %v", err)
+	}
+
+	if got := p.rebuildDNSIntercept(state, "WFP sublayer missing during health check"); got != interceptRebuildRetired {
+		t.Fatalf("rebuildDNSIntercept() = %v, want interceptRebuildRetired - a post-shutdown rebuild resurrects DNS interception", got)
+	}
+	if p.dnsInterceptState != nil {
+		t.Error("rebuild published new intercept state after shutdown")
+	}
+}
+
+// TestRebuildDNSInterceptRefusedForReplacedState covers the other stale-owner case: an
+// earlier rebuild already replaced the state, so a goroutine still holding the old one
+// must not tear down its successor.
+func TestRebuildDNSInterceptRefusedForReplacedState(t *testing.T) {
+	p, old, f := newInterceptTestProg(t)
+	defer assertNoNRPTSideEffects(t, f)
+	current := &wfpState{stopCh: make(chan struct{}), listenerIP: "127.0.0.1"}
+	p.dnsInterceptState = current
+
+	if got := p.rebuildDNSIntercept(old, "WFP sublayer missing during health check"); got != interceptRebuildRetired {
+		t.Fatalf("rebuildDNSIntercept() = %v, want interceptRebuildRetired for a superseded state", got)
+	}
+	if p.dnsInterceptState != any(current) {
+		t.Error("a superseded state's rebuild replaced the live intercept")
+	}
+	if p.interceptStateRevoked(current) {
+		t.Error("the live state was revoked by a superseded rebuild")
+	}
+}
+
+// TestRepairMissingWFPStandsDownAfterShutdown checks the monitor's entry point. It must
+// not even query WFP for a retired state - the sublayer it looks for is what teardown
+// just deleted - and it must tell the monitor goroutine to exit.
+func TestRepairMissingWFPStandsDownAfterShutdown(t *testing.T) {
+	p, state, f := newInterceptTestProg(t)
+	defer assertNoNRPTSideEffects(t, f)
+	if err := p.stopDNSIntercept(); err != nil {
+		t.Fatalf("stopDNSIntercept() = %v", err)
+	}
+	// Set the handle only after teardown. A fake handle proves the revocation check
+	// comes first, but must never reach the real WFP calls in cleanupWFPFilters.
+	state.engineHandle = 1
+
+	if !p.repairMissingWFP(state) {
+		t.Error("repairMissingWFP() = false after shutdown; the health monitor would keep running for a dead intercept")
+	}
+	if p.dnsInterceptState != nil {
+		t.Error("repairMissingWFP rebuilt the intercept after shutdown")
+	}
+}
+
+// TestPendingStopSignalsRevocation covers how a stop avoids waiting: while it is blocked
+// on the lifecycle lock it must already read as revoked, so an in-flight NRPT heal
+// abandons its probe backoff instead of making the service stop wait it out. A stop that
+// waits too long is killed by the Service Control Manager, which cleans up nothing.
+func TestPendingStopSignalsRevocation(t *testing.T) {
+	p, state, f := newInterceptTestProg(t)
+	defer assertNoNRPTSideEffects(t, f)
+
+	p.dnsInterceptMu.Lock()
+	stopped := make(chan struct{})
+	go func() {
+		defer close(stopped)
+		_ = p.stopDNSIntercept()
+	}()
+
+	// Wait for the stop to announce itself while it is blocked on the lock.
+	deadline := time.Now().Add(5 * time.Second)
+	for !p.dnsInterceptStopRequested.Load() {
+		if time.Now().After(deadline) {
+			p.dnsInterceptMu.Unlock()
+			<-stopped
+			t.Fatal("stop never announced itself before waiting for the lifecycle lock")
+		}
+		runtime.Gosched()
+	}
+	if !p.interceptStateRevoked(state) {
+		t.Error("a pending stop does not read as revoked; the heal flows would keep it waiting")
+	}
+	p.dnsInterceptMu.Unlock()
+	<-stopped
+
+	if p.dnsInterceptState != nil {
+		t.Error("the pending stop did not tear down the intercept once it acquired the lock")
+	}
+}
+
+// TestInterceptWaitAbandonsPromptlyOnPendingStop is the bound on how long a stop can be
+// delayed by a recovery flow: the heal sequence's waits add up to tens of seconds, and
+// each one must end as soon as a stop is pending.
+func TestInterceptWaitAbandonsPromptlyOnPendingStop(t *testing.T) {
+	p, state, f := newInterceptTestProg(t)
+	defer assertNoNRPTSideEffects(t, f)
+	p.dnsInterceptStopRequested.Store(true)
+
+	start := time.Now()
+	if p.interceptWait(state, 30*time.Second) {
+		t.Fatal("interceptWait() = true with a stop pending; the caller would carry on writing host DNS state")
+	}
+	if elapsed := time.Since(start); elapsed > 2*time.Second {
+		t.Errorf("interceptWait took %v to notice a pending stop; shutdown would inherit that delay", elapsed)
+	}
+}
+
+// TestInterceptWaitRunsToCompletionWhileLive guards the other direction: the cancellable
+// wait must still actually wait, or the recovery flows lose their backoff.
+func TestInterceptWaitRunsToCompletionWhileLive(t *testing.T) {
+	p, state, f := newInterceptTestProg(t)
+	defer assertNoNRPTSideEffects(t, f)
+
+	start := time.Now()
+	if !p.interceptWait(state, 250*time.Millisecond) {
+		t.Fatal("interceptWait() = false for a live intercept")
+	}
+	if elapsed := time.Since(start); elapsed < 250*time.Millisecond {
+		t.Errorf("interceptWait returned after %v, want at least 250ms", elapsed)
+	}
+}
+
+// TestNRPTNeedsCtrldActivation covers the recovery gap that left a machine unfiltered
+// until restart: a failed NRPT write clears ownership, and an owner-None tick used to do
+// nothing at all, so nothing ever retried the write.
+func TestNRPTNeedsCtrldActivation(t *testing.T) {
+	tests := []struct {
+		name       string
+		owner      nrptRuleOwner
+		ruleExists bool
+		want       bool
+	}{
+		{
+			// The reported hole: activation failed, ownership was cleared, and no
+			// other path re-arms it. In hard mode WFP keeps blocking DNS meanwhile.
+			name:  "no owner retries the failed write",
+			owner: nrptRuleOwnerNone,
+			want:  true,
+		},
+		{
+			name:       "no owner retries even if a rule is somehow present",
+			owner:      nrptRuleOwnerNone,
+			ruleExists: true,
+			want:       true,
+		},
+		{
+			name:  "ctrld-owned rule removed externally is re-added",
+			owner: nrptRuleOwnerCtrld,
+			want:  true,
+		},
+		{
+			name:       "healthy ctrld-owned rule is left alone",
+			owner:      nrptRuleOwnerCtrld,
+			ruleExists: true,
+			want:       false,
+		},
+		{
+			// Writing beside external policy would be ambiguous policy, not recovery.
+			name:  "external policy is never overwritten",
+			owner: nrptRuleOwnerGroupPolicy,
+			want:  false,
+		},
+		{
+			name:       "external policy is never overwritten even with a ctrld rule present",
+			owner:      nrptRuleOwnerGroupPolicy,
+			ruleExists: true,
+			want:       false,
+		},
+	}
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			if got := nrptNeedsCtrldActivation(tc.owner, tc.ruleExists); got != tc.want {
+				t.Errorf("nrptNeedsCtrldActivation(%v, %v) = %v, want %v", tc.owner, tc.ruleExists, got, tc.want)
+			}
+		})
+	}
+}
+
+// TestActivateCtrldNRPTFallbackRefusedAfterShutdown guards the worst leftover. A
+// catch-all re-added after shutdown points every DNS query on the machine at a listener
+// that no longer exists, so nothing resolves at all. Refusing early also keeps this test
+// from writing NRPT policy on the machine running it.
+func TestActivateCtrldNRPTFallbackRefusedAfterShutdown(t *testing.T) {
+	p, state, f := newInterceptTestProg(t)
+	defer assertNoNRPTSideEffects(t, f)
+	if err := p.stopDNSIntercept(); err != nil {
+		t.Fatalf("stopDNSIntercept() = %v", err)
+	}
+
+	if p.activateCtrldNRPTFallback(state, "ctrld-owned rule missing during health check") {
+		t.Error("activateCtrldNRPTFallback() = true after shutdown: the catch-all would outlive ctrld")
+	}
+	if owner, _ := state.nrptPolicyOwner(); owner != nrptRuleOwnerNone {
+		t.Errorf("NRPT owner = %v after a refused fallback, want nrptRuleOwnerNone", owner)
+	}
+}
+
+// TestAllowHandbackAttemptRateLimits covers the throttle on testing an external
+// catch-all. Each attempt takes ctrld's rule out of the way for a probe, so a rule that
+// never routes would cost a brief DNS outage on every 30s health tick without this - in
+// hard mode a window where WFP blocks DNS and nothing redirects it.
+func TestHandbackThrottleIsPerRule(t *testing.T) {
+	state := &wfpState{stopCh: make(chan struct{})}
+	now := time.Now()
+
+	if !state.handbackAllowed(now, "{GP-RULE}", nrptHandbackRetryInterval) {
+		t.Fatal("first handback attempt must be allowed")
+	}
+	// Checking alone must not spend the budget: a pre-probe can still abort the attempt
+	// without disturbing NRPT, and that must not cost the rule its next window.
+	if !state.handbackAllowed(now, "{GP-RULE}", nrptHandbackRetryInterval) {
+		t.Error("handbackAllowed must not consume the budget by itself")
+	}
+
+	state.recordHandbackAttempt(now, "{GP-RULE}", nrptHandbackRetryInterval)
+	if state.handbackAllowed(now.Add(nrptHandbackRetryInterval-time.Second), "{GP-RULE}", nrptHandbackRetryInterval) {
+		t.Error("re-testing the same rule inside the interval must be suppressed")
+	}
+	// Group Policy alternating between two names must not erase either one's memory:
+	// with a single slot every swap costs another removal of the live rule.
+	if !state.handbackAllowed(now.Add(time.Second), "{OTHER-RULE}", nrptHandbackRetryInterval) {
+		t.Error("a different rule name means the administrator changed policy: test it now")
+	}
+	state.recordHandbackAttempt(now.Add(time.Second), "{OTHER-RULE}", nrptHandbackRetryInterval)
+	if state.handbackAllowed(now.Add(2*time.Second), "{GP-RULE}", nrptHandbackRetryInterval) {
+		t.Error("testing another rule must not clear the first rule's throttle")
+	}
+
+	if !state.handbackAllowed(now.Add(2*nrptHandbackRetryInterval), "{GP-RULE}", nrptHandbackRetryInterval) {
+		t.Error("the same rule must be testable again after the interval")
+	}
+}

cmd/cli/dns_intercept_others.go

@@ -18,6 +18,9 @@ func (p *prog) stopDNSIntercept() error {
 	return nil
 }
 
+// skipInitialDNSReset is Windows-only; other platforms keep the normal reset.
+func (p *prog) skipInitialDNSReset() bool { return false }
+
 // exemptVPNDNSServers is a no-op on unsupported platforms.
 func (p *prog) exemptVPNDNSServers(exemptions []vpnDNSExemption) error {
 	return nil

cmd/cli/dns_proxy.go

@@ -130,13 +130,7 @@ func (p *prog) serveDNS(listenerNum string) error {
 		// signal the prober and respond NXDOMAIN. Used by both macOS pf probes
 		// (_pf-probe-*) and Windows NRPT probes (_nrpt-probe-*) to verify that
 		// DNS interception is actually routing queries to ctrld's listener.
-		if probeID, ok := p.pfProbeExpected.Load().(string); ok && probeID != "" && domain == probeID {
-			if chPtr, ok := p.pfProbeCh.Load().(*chan struct{}); ok && chPtr != nil {
-				select {
-				case *chPtr <- struct{}{}:
-				default:
-				}
-			}
+		if p.signalInterceptProbe(domain) {
 			answer := new(dns.Msg)
 			answer.SetRcode(m, dns.RcodeNameError) // NXDOMAIN
 			_ = w.WriteMsg(answer)

cmd/cli/intercept_probe.go

@@ -0,0 +1,68 @@
+package cli
+
+// Interception probe registry.
+//
+// A probe sends a DNS query for a unique synthetic domain through the OS resolver and
+// waits for ctrld's own handler to receive it. That is the only way to tell "the rules are
+// present" from "the rules are actually redirecting packets", and both the macOS pf path
+// and the Windows NRPT path use it.
+//
+// Each attempt registers its own domain, so overlapping probes cannot cancel each other,
+// and deregistration only removes the entry it owns.
+
+// registerInterceptProbe registers domain and returns the channel it will be signalled on
+// plus the function that removes the registration.
+//
+//lint:ignore U1000 used on darwin (pf probes) and windows (NRPT probes)
+func (p *prog) registerInterceptProbe(domain string) (<-chan struct{}, func()) {
+	ch := make(chan struct{}, 1)
+
+	p.interceptProbeMu.Lock()
+	current, _ := p.interceptProbes.Load().(map[string]chan struct{})
+	next := make(map[string]chan struct{}, len(current)+1)
+	for k, v := range current {
+		next[k] = v
+	}
+	next[domain] = ch
+	p.interceptProbes.Store(next)
+	p.interceptProbeMu.Unlock()
+
+	return ch, func() {
+		p.interceptProbeMu.Lock()
+		defer p.interceptProbeMu.Unlock()
+		current, _ := p.interceptProbes.Load().(map[string]chan struct{})
+		// Only drop the entry while it is still this attempt's channel. A later probe
+		// that reused the domain owns the slot now, and clearing it would make that one
+		// wait out its timeout for a query it already received.
+		if existing, ok := current[domain]; !ok || existing != ch {
+			return
+		}
+		next := make(map[string]chan struct{}, len(current))
+		for k, v := range current {
+			if k != domain {
+				next[k] = v
+			}
+		}
+		p.interceptProbes.Store(next)
+	}
+}
+
+// signalInterceptProbe reports whether domain is a pending probe, signalling its waiter
+// when it is. Called from the DNS handler for every query, so the common case is a nil or
+// empty map and no allocation.
+func (p *prog) signalInterceptProbe(domain string) bool {
+	probes, _ := p.interceptProbes.Load().(map[string]chan struct{})
+	if len(probes) == 0 {
+		return false
+	}
+	ch, ok := probes[domain]
+	if !ok {
+		return false
+	}
+	select {
+	case ch <- struct{}{}:
+	default:
+		// Buffered channel already holds a signal: the waiter has what it needs.
+	}
+	return true
+}

cmd/cli/nrpt_external_gp.go

@@ -0,0 +1,63 @@
+package cli
+
+import (
+	"errors"
+	"net/netip"
+	"strings"
+)
+
+const nrptRuleName = `CtrldCatchAll`
+
+// errGPNRPTVerified marks an intercept startup failure that happened while an externally
+// managed (Group Policy) NRPT catch-all was proved - by probe, not by registry shape
+// alone - to be routing DNS to this listener. It is the difference between "intercept
+// failed but DNS still reaches ctrld" and "intercept failed and nothing is filtering",
+// which is what decides whether the interface-DNS fallback must run.
+//
+// Only the Windows path produces it, but setDNS is shared, so the sentinel and its
+// predicate live here with the other platform-neutral NRPT helpers.
+var errGPNRPTVerified = errors.New("GP-managed NRPT verified routing to ctrld")
+
+// errGPNRPTIneffective marks a startup that ends with externally managed NRPT owning the
+// namespace while no probe has proved it routes to ctrld. DNS is not reaching ctrld, but
+// adapter DNS was deliberately preserved and no ctrld rule may be written beside an
+// administrator's catch-all - so this is a failed start that must not take the
+// interface-DNS fallback either.
+var errGPNRPTIneffective = errors.New("GP-managed NRPT owns the namespace but no probe reached ctrld")
+
+// interceptFailedWithVerifiedExternalDNS reports whether an intercept startup failure
+// happened while externally managed DNS policy was verified to be routing to ctrld.
+func interceptFailedWithVerifiedExternalDNS(err error) bool {
+	return errors.Is(err, errGPNRPTVerified)
+}
+
+// interceptFailedUnderExternalDNSPolicy reports whether an intercept startup failure
+// happened while externally managed DNS policy owned the namespace, whether or not it was
+// proved to route. Either way the interface-DNS fallback must not run: adapter DNS was
+// preserved on purpose, and rewriting it would violate the policy ctrld just deferred to.
+// Only the verified case is a successful start.
+func interceptFailedUnderExternalDNSPolicy(err error) bool {
+	return errors.Is(err, errGPNRPTVerified) || errors.Is(err, errGPNRPTIneffective)
+}
+
+// isExternalGPCatchAll recognizes only a single catch-all namespace that is not
+// ctrld's deterministic GP key. Registry access stays in the Windows file; this
+// pure classifier is shared with host-runnable tests.
+func isExternalGPCatchAll(ruleName string, namespaces []string) bool {
+	return ruleName != "" && !strings.EqualFold(ruleName, nrptRuleName) && len(namespaces) == 1 && strings.TrimSpace(namespaces[0]) == "."
+}
+
+func isMatchingGPNRPTRule(ruleName string, namespaces []string, dnsServers, listenerIP string) bool {
+	if !isExternalGPCatchAll(ruleName, namespaces) {
+		return false
+	}
+	server, err := netip.ParseAddr(strings.TrimSpace(dnsServers))
+	if err != nil {
+		return false
+	}
+	listener, err := netip.ParseAddr(strings.TrimSpace(listenerIP))
+	if err != nil {
+		return false
+	}
+	return server.Unmap() == listener.Unmap()
+}

cmd/cli/nrpt_external_gp_test.go

@@ -0,0 +1,129 @@
+package cli
+
+import (
+	"errors"
+	"fmt"
+	"testing"
+)
+
+func TestIsMatchingGPNRPTRule(t *testing.T) {
+	tests := []struct {
+		name       string
+		ruleName   string
+		namespaces []string
+		servers    string
+		listener   string
+		want       bool
+	}{
+		{
+			name:       "exact IPv4 catch-all",
+			ruleName:   "{A1B2C3D4}",
+			namespaces: []string{"."},
+			servers:    "127.0.0.1",
+			listener:   "127.0.0.1",
+			want:       true,
+		},
+		{
+			name:       "normalized IPv4-mapped listener",
+			ruleName:   "{A1B2C3D4}",
+			namespaces: []string{"."},
+			servers:    "::ffff:127.0.0.1",
+			listener:   "127.0.0.1",
+			want:       true,
+		},
+		{
+			name:       "ctrld GP key is not external",
+			ruleName:   "ctrldcatchall",
+			namespaces: []string{"."},
+			servers:    "127.0.0.1",
+			listener:   "127.0.0.1",
+		},
+		{
+			name:       "partial namespace",
+			ruleName:   "{A1B2C3D4}",
+			namespaces: []string{"corp.example"},
+			servers:    "127.0.0.1",
+			listener:   "127.0.0.1",
+		},
+		{
+			name:       "multiple namespaces",
+			ruleName:   "{A1B2C3D4}",
+			namespaces: []string{".", "corp.example"},
+			servers:    "127.0.0.1",
+			listener:   "127.0.0.1",
+		},
+		{
+			name:       "wrong listener",
+			ruleName:   "{A1B2C3D4}",
+			namespaces: []string{"."},
+			servers:    "127.0.0.2",
+			listener:   "127.0.0.1",
+		},
+		{
+			name:       "multiple nameservers",
+			ruleName:   "{A1B2C3D4}",
+			namespaces: []string{"."},
+			servers:    "127.0.0.1;127.0.0.2",
+			listener:   "127.0.0.1",
+		},
+		{
+			name:       "malformed nameserver",
+			ruleName:   "{A1B2C3D4}",
+			namespaces: []string{"."},
+			servers:    "localhost",
+			listener:   "127.0.0.1",
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isMatchingGPNRPTRule(tt.ruleName, tt.namespaces, tt.servers, tt.listener); got != tt.want {
+				t.Fatalf("isMatchingGPNRPTRule() = %t, want %t", got, tt.want)
+			}
+		})
+	}
+}
+
+func TestIsExternalGPCatchAll(t *testing.T) {
+	tests := []struct {
+		name       string
+		ruleName   string
+		namespaces []string
+		want       bool
+	}{
+		{name: "external catch-all", ruleName: "{GP-RULE}", namespaces: []string{"."}, want: true},
+		{name: "ctrld key", ruleName: nrptRuleName, namespaces: []string{"."}},
+		{name: "partial namespace", ruleName: "{GP-RULE}", namespaces: []string{"corp.example"}},
+		{name: "multiple namespaces", ruleName: "{GP-RULE}", namespaces: []string{".", "corp.example"}},
+	}
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if got := isExternalGPCatchAll(tt.ruleName, tt.namespaces); got != tt.want {
+				t.Fatalf("isExternalGPCatchAll() = %t, want %t", got, tt.want)
+			}
+		})
+	}
+}
+
+// TestInterceptFailedWithVerifiedExternalDNS covers the distinction the interface-DNS
+// fallback turns on. "A GP rule exists" is not enough: if it is not actually routing and
+// intercept failed too, skipping the fallback leaves the machine with no NRPT, no WFP and
+// no adapter DNS - that is, unfiltered. Only a probe-verified route earns the skip.
+func TestInterceptFailedWithVerifiedExternalDNS(t *testing.T) {
+	wfpErr := errors.New("FwpmEngineOpen0 failed: HRESULT 0x5")
+
+	verified := fmt.Errorf("dns intercept: WFP setup failed: %w: %w", wfpErr, errGPNRPTVerified)
+	if !interceptFailedWithVerifiedExternalDNS(verified) {
+		t.Error("a failure carrying errGPNRPTVerified must skip the interface-DNS fallback")
+	}
+	if !errors.Is(verified, wfpErr) {
+		t.Error("the underlying cause must stay inspectable for logs and callers")
+	}
+
+	if interceptFailedWithVerifiedExternalDNS(fmt.Errorf("dns intercept: WFP setup failed: %w", wfpErr)) {
+		t.Error("an unverified failure must take the interface-DNS fallback rather than leave the machine unfiltered")
+	}
+	if interceptFailedWithVerifiedExternalDNS(nil) {
+		t.Error("no error must not read as a verified external route")
+	}
+}

cmd/cli/nrpt_external_gp_windows_test.go

@@ -0,0 +1,20 @@
+//go:build windows
+
+package cli
+
+import "testing"
+
+func TestWFPStateNRPTPolicyOwner(t *testing.T) {
+	state := &wfpState{}
+	state.setNRPTPolicyOwner(nrptRuleOwnerGroupPolicy, "{GP-RULE}")
+	owner, ruleName := state.nrptPolicyOwner()
+	if owner != nrptRuleOwnerGroupPolicy || ruleName != "{GP-RULE}" {
+		t.Fatalf("owner = %v, rule = %q", owner, ruleName)
+	}
+
+	state.setNRPTPolicyOwner(nrptRuleOwnerCtrld, "")
+	owner, ruleName = state.nrptPolicyOwner()
+	if owner != nrptRuleOwnerCtrld || ruleName != "" {
+		t.Fatalf("owner = %v, rule = %q", owner, ruleName)
+	}
+}

cmd/cli/prog.go

@@ -172,6 +172,22 @@ type prog struct {
 	// On Windows: *wfpState, on macOS: *pfState, nil on other platforms.
 	dnsInterceptState any
 
+	// dnsInterceptMu serializes DNS intercept lifecycle transitions - start, stop and
+	// the health monitor's rebuild - and guards every write to dnsInterceptState, so a
+	// service stop can never interleave with a monitor-driven rebuild.
+	dnsInterceptMu sync.Mutex //lint:ignore U1000 used on windows
+
+	// dnsInterceptStopRequested is set while a stop waits for dnsInterceptMu. The
+	// health and recovery flows read it as a shutdown signal and abandon their work,
+	// rather than making the stop wait out their probe backoffs.
+	dnsInterceptStopRequested atomic.Bool //lint:ignore U1000 used on windows
+
+	// nrptTransitionMu makes one NRPT ownership transition - observe, mutate, signal,
+	// record owner - atomic against shutdown and against another transition. It is
+	// deliberately finer-grained than dnsInterceptMu: it is taken for the duration of a
+	// single transition, never across the recovery flows' probe backoffs.
+	nrptTransitionMu sync.Mutex //lint:ignore U1000 used on windows
+
 	// lastTunnelIfaces tracks the tunnel set included in the last successfully loaded
 	// pf anchor. Pending tunnel state is kept separately so failed PF work is retried
 	// instead of being mistaken for an applied update. Protected by mu.
@@ -220,15 +236,22 @@ type prog struct {
 	// existing delayed checks provide a trailing reconciliation after churn.
 	pfIgnoredChangeLastReconcile atomic.Int64 //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
-	// are actually translating packets (not just present in rule text).
-	pfProbeExpected atomic.Value // string
-
-	// pfProbeCh is signaled when the DNS handler receives the expected probe query.
-	// The channel is created by probePFIntercept() and closed when the probe arrives.
-	pfProbeCh atomic.Value // *chan struct{}
+	// interceptProbes maps the domain of each pending interception probe to the channel
+	// that probe waits on. A probe verifies that interception is actually translating or
+	// redirecting packets, not merely present in rule text: the DNS handler looks up
+	// incoming queries here and signals the matching waiter.
+	//
+	// It holds one entry per in-flight probe rather than a single slot, because probes do
+	// overlap - the health monitor, a handback and a heal cycle can each have one out at
+	// the same time - and a single slot means the last registration wins and the loser
+	// waits out its timeout for a query that was answered. A false failure then triggers
+	// recovery work that was not needed.
+	//
+	// Registrations are rare and lookups happen on every query, so the map is stored as
+	// an immutable snapshot behind an atomic: readers never take a lock, writers copy
+	// under interceptProbeMu.
+	interceptProbes  atomic.Value // map[string]chan struct{}
+	interceptProbeMu sync.Mutex   //lint:ignore U1000 written only by registerInterceptProbe, used on darwin/windows
 
 	// VPN DNS manager for split DNS routing when intercept mode is active.
 	vpnDNS *vpnDNSManager
@@ -424,7 +447,12 @@ func (p *prog) postRun() {
 			p.runningOnDomainController = isDC
 			mainLog.Load().Debug().Msgf("running on domain controller: %t, role: %d", p.runningOnDomainController, roleInt)
 		}
-		p.resetDNS(false, false)
+		// A Windows organization can install a GP-owned NRPT catch-all before
+		// starting ctrld. Detect that policy before resetDNS touches adapter DNS;
+		// startDNSIntercept will then prove the rule functionally before adopting it.
+		if !p.skipInitialDNSReset() {
+			p.resetDNS(false, false)
+		}
 		ns := ctrld.InitializeOsResolver(false)
 		mainLog.Load().Debug().Msgf("initialized OS resolver with nameservers: %v", ns)
 		p.setDNS()
@@ -907,7 +935,7 @@ func (p *prog) setDNS() {
 		mainLog.Load().Fatal().Msgf("invalid --intercept-mode value %q: must be 'off', 'dns', or 'hard'", interceptMode)
 	}
 	if interceptMode == "" || interceptMode == "off" {
-		interceptMode = cfg.Service.InterceptMode
+		interceptMode = p.configuredInterceptMode()
 		if interceptMode != "" && interceptMode != "off" {
 			mainLog.Load().Info().Msgf("Intercept mode enabled via config (intercept_mode = %q)", interceptMode)
 		}
@@ -927,6 +955,30 @@ func (p *prog) setDNS() {
 	// software that also manages DNS. See issue #489.
 	if dnsIntercept {
 		if err := startDNSInterceptFn(p); err != nil {
+			// This check comes first: it is the one failure where DNS already works
+			// without ctrld touching anything else, so neither the refusal below nor the
+			// fallback applies.
+			//
+			// An externally managed rule was proved - by probe, not by registry shape -
+			// to be routing DNS to this listener. Falling through would rewrite adapter
+			// DNS after explicitly preserving it, and DNS still works, so stop here.
+			//
+			// Only a verified route earns this. A rule that merely exists does not: if it
+			// is not actually routing and intercept failed too, the machine would be left
+			// with no NRPT, no WFP and no adapter fallback - that is, unfiltered - so
+			// every other failure takes the paths below.
+			if interceptFailedUnderExternalDNSPolicy(err) {
+				if interceptFailedWithVerifiedExternalDNS(err) {
+					mainLog.Load().Error().Err(err).Msg("DNS intercept mode failed but externally managed DNS policy is verified routing to ctrld — not falling back to interface DNS settings")
+				} else {
+					// Owned by external policy but not proved to route: DNS is not
+					// reaching ctrld. Adapter DNS still stays as the organization set it,
+					// and setDnsOK stays false, so this start reports as failed until a
+					// probe succeeds.
+					mainLog.Load().Error().Err(err).Msg("DNS intercept mode failed and externally managed DNS policy is not routing to ctrld — leaving interface DNS settings untouched; the service is not ready")
+				}
+				return
+			}
 			// Interface DNS cannot express a port: macOS interface settings and Windows
 			// NRPT rules both name a resolver by IP alone. So it is only a usable
 			// fallback when the listener actually bound :53. When something else owns
@@ -1037,6 +1089,17 @@ func (p *prog) setDNS() {
 	}
 }
 
+// configuredInterceptMode resolves the service's effective intercept mode without
+// mutating package state. Platform startup preflights use the same precedence as
+// setDNS so they do not make adapter-DNS decisions from a different mode value.
+func (p *prog) configuredInterceptMode() string {
+	im := interceptMode
+	if im == "" || im == "off" {
+		im = p.cfg.Service.InterceptMode
+	}
+	return im
+}
+
 func (p *prog) setDnsForRunningIface(nameservers []string) (runningIface *net.Interface) {
 	if p.runningIface == "" {
 		return