Port NRPT recovery limits to master

Also adapts the logging fields to the master branch logger API.
This commit is contained in:
Dev Scribe
2026-07-10 03:05:17 -04:00
committed by Cuong Manh Le
parent eb8756bbe5
commit 3226c2d0e2
6 changed files with 329 additions and 61 deletions
+122 -61
View File
@@ -288,6 +288,9 @@ 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.
@@ -353,9 +356,10 @@ const (
// - GP path: SOFTWARE\Policies\...\DnsPolicyConfig (Group Policy)
// - Local path: SYSTEM\CurrentControlSet\...\DnsPolicyConfig (service store)
//
// 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.
// 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.
//
// Strategy (matching Tailscale's approach):
// - Always write to the local path (baseline for non-domain machines).
@@ -405,18 +409,20 @@ func otherGPRulesExist() bool {
return false
}
// 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() {
// 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 {
// 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 // Key doesn't exist — clean state.
return false // Key doesn't exist — clean state.
}
names, err := k.ReadSubKeyNames(-1)
k.Close()
@@ -424,12 +430,14 @@ func cleanGPPath() {
if len(names) > 0 {
mainLog.Load().Debug().Strs("remaining", names).Msg("DNS intercept: GP path has other rules, leaving parent key")
}
return
return false
}
// 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,6 +558,10 @@ func refreshNRPTPolicy() {
// Group Policy refresh so NRPT changes take effect immediately.
func flushDNSCache() {
refreshNRPTPolicy()
flushDNSCacheOnly()
}
func flushDNSCacheOnly() {
if err := dnsapiDLL.Load(); err == nil {
if err := procDnsFlushResolverCache.Find(); err == nil {
ret, _, _ := procDnsFlushResolverCache.Call()
@@ -566,6 +578,12 @@ func flushDNSCache() {
}
}
func signalNRPTChange() {
refreshNRPTPolicy()
sendParamChange()
flushDNSCacheOnly()
}
// sendParamChange sends SERVICE_CONTROL_PARAMCHANGE to the DNS Client (Dnscache)
// service, signaling it to re-read its configuration including NRPT rules from
// the registry. This is the standard mechanism used by FortiClient, Tailscale,
@@ -581,43 +599,39 @@ func sendParamChange() {
}
// cleanEmptyNRPTParent removes empty NRPT parent keys that block activation.
// An empty DnsPolicyConfig key (exists but no subkeys) causes DNS Client to
// cache "no rules" and ignore subsequently-added rules.
// 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.
//
// 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 local-path rules
// are ignored.
// This helper only changes registry state. The caller sends the single
// RefreshPolicyEx/paramchange/flush signal after it knows cleanup occurred.
//
// Returns true if cleanup was performed (caller should add a delay).
// Returns true if cleanup was performed (caller should signal DNS Client).
func cleanEmptyNRPTParent() bool {
cleaned := false
// Always clean the GP path — its existence blocks local path activation.
cleanGPPath()
cleaned := cleanGPPath()
// Clean empty local/direct path parent key.
k, err := registry.OpenKey(registry.LOCAL_MACHINE, nrptDirectKey, registry.ENUMERATE_SUB_KEYS)
if err != nil {
return false
}
names, err := k.ReadSubKeyNames(-1)
k.Close()
if err != nil || len(names) > 0 {
return false
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)
if err != nil {
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
names, err := k.ReadSubKeyNames(-1)
k.Close()
return err == nil && len(names) == 0
}
// logNRPTParentKeyState logs the state of both NRPT registry paths for diagnostics.
@@ -684,9 +698,12 @@ func (p *prog) startDNSIntercept() error {
logNRPTParentKeyState("pre-write")
// Two-phase empty parent key recovery: if the GP DnsPolicyConfig key exists
// but is empty, it poisons DNS Client's cache. Clean it before writing.
cleanEmptyNRPTParent()
// 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()
}
if err := addNRPTCatchAllRule(listenerIP); err != nil {
return fmt.Errorf("dns intercept: failed to add NRPT catch-all rule: %w", err)
@@ -694,9 +711,7 @@ func (p *prog) startDNSIntercept() error {
logNRPTParentKeyState("post-write")
state.nrptActive = true
refreshNRPTPolicy()
sendParamChange()
flushDNSCache()
signalNRPTChange()
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.
@@ -1640,7 +1655,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 {
flushDNSCache()
signalNRPTChange()
mainLog.Load().Info().Msg("DNS intercept: NRPT catch-all rule restored")
}
}
@@ -1674,14 +1689,22 @@ 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().Str("remaining", wait.String()).
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
}
refreshNRPTPolicy()
flushDNSCache()
signalNRPTChange()
state.nrptRecoveryLimiter.recordRecoveryFlow(time.Now(), p.cfg)
mainLog.Load().Info().Msg("DNS intercept: NRPT catch-all rule restored by health monitor")
// After restoring, verify it's actually working.
go p.nrptProbeAndHeal()
@@ -1693,6 +1716,8 @@ 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.
@@ -1779,19 +1804,39 @@ func (p *prog) probeNRPT() bool {
// nrptProbeAndHeal runs the NRPT probe with retries and escalating remediation.
// Called asynchronously after startup and from the health monitor.
//
// 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
// 5. Nuclear: two-phase delete → signal → re-add → probe
// 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.
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().Str("remaining", wait.String()).
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.
@@ -1802,17 +1847,37 @@ func (p *prog) nrptProbeAndHeal() {
mainLog.Load().Info().Msg("DNS intercept: NRPT verified working")
return
}
remediated = true
// Attempts 2-4: GP refresh + paramchange + flush with increasing backoff
// 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.
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).Str("delay", delay.String()).
Msg("DNS intercept: NRPT probe failed, retrying with GP refresh + paramchange")
Msg("DNS intercept: NRPT probe failed, retrying with policy refresh + paramchange")
logNRPTParentKeyState(fmt.Sprintf("probe-attempt-%d", attempt))
refreshNRPTPolicy()
sendParamChange()
flushDNSCache()
signalNRPTChange()
time.Sleep(delay)
if p.probeNRPT() {
mainLog.Load().Info().Int("attempt", attempt).
@@ -1826,16 +1891,14 @@ 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, ok := p.dnsInterceptState.(*wfpState); ok {
if state != nil {
listenerIP = state.listenerIP
}
// Phase 1: Remove our rule and the parent key if now empty.
_ = removeNRPTCatchAllRule()
cleanEmptyNRPTParent()
refreshNRPTPolicy()
sendParamChange()
flushDNSCache()
signalNRPTChange()
logNRPTParentKeyState("nuclear-after-delete")
// Wait for DNS Client to process the deletion.
@@ -1846,9 +1909,7 @@ func (p *prog) nrptProbeAndHeal() {
mainLog.Load().Error().Err(err).Msg("DNS intercept: failed to re-add NRPT after nuclear recovery")
return
}
refreshNRPTPolicy()
sendParamChange()
flushDNSCache()
signalNRPTChange()
logNRPTParentKeyState("nuclear-after-readd")
// Final probe after recovery.
+101
View File
@@ -0,0 +1,101 @@
//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
}
@@ -0,0 +1,74 @@
//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)
}
}
+2
View File
@@ -247,6 +247,8 @@ 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"`
// FirewallMode controls the DNS-resolved IP allowlist. When "on", only IPs
// that were successfully resolved by ctrld are allowed for outbound connections.
// This closes the "DNS gap" where apps bypass DNS policy using hardcoded IPs.
+16
View File
@@ -291,6 +291,22 @@ If a remote upstream fails to resolve a query or is unreachable, `ctrld` will fo
- Required: no
- Default: true on Windows, MacOS and 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.
+14
View File
@@ -91,6 +91,20 @@ 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.,