From ad0033f8513562679ee1f20e54989df19d9165ad Mon Sep 17 00:00:00 2001 From: riccardom Date: Wed, 5 Aug 2026 19:05:18 +0200 Subject: [PATCH] [client] Add catch-all NRPT rule when NetBird is the primary DNS resolver --- client/internal/dns/host_windows.go | 81 +++++++++++++++++ client/internal/dns/host_windows_test.go | 109 +++++++++++++++++++++++ 2 files changed, 190 insertions(+) diff --git a/client/internal/dns/host_windows.go b/client/internal/dns/host_windows.go index 4f6ece532..238f2ed3d 100644 --- a/client/internal/dns/host_windows.go +++ b/client/internal/dns/host_windows.go @@ -6,8 +6,10 @@ import ( "fmt" "io" "net/netip" + "os" "os/exec" "slices" + "strconv" "strings" "syscall" "time" @@ -36,6 +38,16 @@ const ( gpoDnsPolicyRoot = `SOFTWARE\Policies\Microsoft\Windows NT\DNSClient\DnsPolicyConfig` gpoDnsPolicyConfigMatchPath = gpoDnsPolicyRoot + `\NetBird-Match` + dnsPolicyConfigCatchAllPath = `SYSTEM\CurrentControlSet\Services\Dnscache\Parameters\DnsPolicyConfig\NetBird-CatchAll` + gpoDnsPolicyConfigCatchAllPath = gpoDnsPolicyRoot + `\NetBird-CatchAll` + + // nrptCatchAllNamespace is the NRPT namespace that matches every name. + nrptCatchAllNamespace = "." + + // envDisableCatchAllNRPT turns off the catch-all NRPT rule, restoring the + // previous behavior where the OS is free to query other adapters' resolvers. + envDisableCatchAllNRPT = "NB_DISABLE_DNS_CATCHALL_NRPT" + dnsPolicyConfigVersionKey = "Version" dnsPolicyConfigVersionValue = 2 dnsPolicyConfigNameKey = "Name" @@ -318,6 +330,12 @@ func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager r.updateState(stateManager) + if config.RouteAll { + if err := r.addDNSCatchAllPolicy(config.ServerIP); err != nil { + return fmt.Errorf("add dns catch-all policy: %w", err) + } + } + if err := r.updateSearchDomains(searchDomains); err != nil { return fmt.Errorf("update search domains: %w", err) } @@ -388,6 +406,44 @@ func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr return ruleIndex, nil } +// addDNSCatchAllPolicy installs an NRPT rule for the root namespace, making our +// resolver the only one the OS consults. +// +// Setting NameServer on the WG adapter is not enough: Windows queries the +// resolvers of every adapter in parallel and takes the first answer that comes +// back (smart multi-homed name resolution). That both leaks every question to +// the local network's resolver and makes the winner non-deterministic, so a +// resolver other than ours can answer for a name we are authoritative for. NRPT +// is evaluated before adapter selection and restricts a matched namespace to the +// servers listed in the rule, which removes the race for every name. +// +// Exclusive by design: there is no fallback to the OS resolvers here. The +// ordered fallback to the pre-takeover nameservers lives inside our own +// resolver (see registerFallback / PriorityFallback), so the walk stays under +// our control instead of being decided by whichever answer arrives first. +func (r *registryConfigurator) addDNSCatchAllPolicy(ip netip.Addr) error { + if parseBoolEnv(envDisableCatchAllNRPT) { + log.Infof("%s is set, not forcing all DNS queries through %s", envDisableCatchAllNRPT, ip) + return nil + } + + if err := r.configureDNSPolicy(dnsPolicyConfigCatchAllPath, []string{nrptCatchAllNamespace}, ip); err != nil { + return fmt.Errorf("configure catch-all DNS policy: %w", err) + } + + if r.gpo { + if err := r.configureDNSPolicy(gpoDnsPolicyConfigCatchAllPath, []string{nrptCatchAllNamespace}, ip); err != nil { + return fmt.Errorf("configure gpo catch-all DNS policy: %w", err) + } + if err := refreshGroupPolicy(); err != nil { + log.Warnf("failed to refresh group policy: %v", err) + } + } + + log.Infof("added catch-all NRPT rule: all DNS queries now resolve exclusively through %s", ip) + return nil +} + func (r *registryConfigurator) configureDNSPolicy(policyPath string, domains []string, ip netip.Addr) error { if err := removeRegistryKeyFromDNSPolicyConfig(policyPath); err != nil { return fmt.Errorf("remove existing dns policy: %w", err) @@ -530,6 +586,17 @@ func (r *registryConfigurator) removeDNSMatchPolicies() error { merr = multierror.Append(merr, fmt.Errorf("remove GPO base entry: %w", err)) } + // Removed unconditionally: the rule needs no bookkeeping to find, and a + // leftover catch-all would send every query to an address we no longer + // serve. Absent keys are not an error. + if err := removeRegistryKeyFromDNSPolicyConfig(dnsPolicyConfigCatchAllPath); err != nil { + merr = multierror.Append(merr, fmt.Errorf("remove local catch-all entry: %w", err)) + } + + if err := removeRegistryKeyFromDNSPolicyConfig(gpoDnsPolicyConfigCatchAllPath); err != nil { + merr = multierror.Append(merr, fmt.Errorf("remove GPO catch-all entry: %w", err)) + } + for i := 0; i < r.nrptEntryCount; i++ { localPath := fmt.Sprintf("%s-%d", dnsPolicyConfigMatchPath, i) gpoPath := fmt.Sprintf("%s-%d", gpoDnsPolicyConfigMatchPath, i) @@ -594,6 +661,20 @@ func refreshGroupPolicy() error { return nil } +func parseBoolEnv(key string) bool { + val := os.Getenv(key) + if val == "" { + return false + } + + parsed, err := strconv.ParseBool(val) + if err != nil { + log.Warnf("failed to parse %s=%q: %v", key, val, err) + return false + } + return parsed +} + func closer(closer io.Closer) { if err := closer.Close(); err != nil { log.Errorf("failed to close: %s", err) diff --git a/client/internal/dns/host_windows_test.go b/client/internal/dns/host_windows_test.go index 3cd2b1bd5..aa6a8bd50 100644 --- a/client/internal/dns/host_windows_test.go +++ b/client/internal/dns/host_windows_test.go @@ -94,6 +94,115 @@ func TestNRPTEntriesCleanupOnConfigChange(t *testing.T) { assert.False(t, exists, "NRPT rule 2 should NOT exist after reducing to 75 domains") } +// TestNRPTCatchAllRule verifies that a catch-all NRPT rule is installed only +// when our resolver is the primary one, that it points at our resolver, and +// that it is removed again when the config stops being primary or is restored. +func TestNRPTCatchAllRule(t *testing.T) { + if testing.Short() { + t.Skip("skipping registry integration test in short mode") + } + + defer cleanupRegistryKeys(t) + cleanupRegistryKeys(t) + + testIP := netip.MustParseAddr("100.64.0.1") + testGUID := "{12345678-1234-1234-1234-123456789ABC}" + interfacePath := `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\` + testGUID + testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE) + require.NoError(t, err, "Should create test interface registry key") + testKey.Close() + defer func() { + _ = registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath) + }() + + cfg := ®istryConfigurator{guid: testGUID} + + matchOnly := HostDNSConfig{ + ServerIP: testIP, + Domains: []DomainConfig{{Domain: "example.com", MatchOnly: true}}, + } + primary := HostDNSConfig{ + ServerIP: testIP, + RouteAll: true, + Domains: []DomainConfig{{Domain: "example.com", MatchOnly: true}}, + } + + // Match-only config: no catch-all, the OS keeps resolving everything else. + require.NoError(t, cfg.applyDNSConfig(matchOnly, nil)) + exists, err := registryKeyExists(dnsPolicyConfigCatchAllPath) + require.NoError(t, err) + assert.False(t, exists, "catch-all rule should not exist for a match-only config") + + // Primary config: catch-all rule for the root namespace, pointing at us. + require.NoError(t, cfg.applyDNSConfig(primary, nil)) + exists, err = registryKeyExists(dnsPolicyConfigCatchAllPath) + require.NoError(t, err) + require.True(t, exists, "catch-all rule should exist when RouteAll is set") + + k, err := registry.OpenKey(registry.LOCAL_MACHINE, dnsPolicyConfigCatchAllPath, registry.QUERY_VALUE) + require.NoError(t, err) + + names, _, err := k.GetStringsValue(dnsPolicyConfigNameKey) + require.NoError(t, err) + assert.Equal(t, []string{nrptCatchAllNamespace}, names, "catch-all rule should match the root namespace") + + servers, _, err := k.GetStringValue(dnsPolicyConfigGenericDNSServersKey) + require.NoError(t, err) + assert.Equal(t, testIP.String(), servers, "catch-all rule should list only our resolver") + + opts, _, err := k.GetIntegerValue(dnsPolicyConfigConfigOptionsKey) + require.NoError(t, err) + assert.EqualValues(t, dnsPolicyConfigConfigOptionsValue, opts) + k.Close() + + // Dropping back to match-only must remove it, otherwise every query would + // keep going to an address we no longer serve. + require.NoError(t, cfg.applyDNSConfig(matchOnly, nil)) + exists, err = registryKeyExists(dnsPolicyConfigCatchAllPath) + require.NoError(t, err) + assert.False(t, exists, "catch-all rule should be removed when RouteAll is cleared") + + // Same on restore. + require.NoError(t, cfg.applyDNSConfig(primary, nil)) + require.NoError(t, cfg.restoreHostDNS()) + exists, err = registryKeyExists(dnsPolicyConfigCatchAllPath) + require.NoError(t, err) + assert.False(t, exists, "catch-all rule should be removed on restore") +} + +// TestNRPTCatchAllRuleDisabledByEnv verifies the kill switch. +func TestNRPTCatchAllRuleDisabledByEnv(t *testing.T) { + if testing.Short() { + t.Skip("skipping registry integration test in short mode") + } + + defer cleanupRegistryKeys(t) + cleanupRegistryKeys(t) + + t.Setenv(envDisableCatchAllNRPT, "true") + + testGUID := "{12345678-1234-1234-1234-123456789ABC}" + interfacePath := `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\` + testGUID + testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE) + require.NoError(t, err, "Should create test interface registry key") + testKey.Close() + defer func() { + _ = registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath) + }() + + cfg := ®istryConfigurator{guid: testGUID} + config := HostDNSConfig{ + ServerIP: netip.MustParseAddr("100.64.0.1"), + RouteAll: true, + } + + require.NoError(t, cfg.applyDNSConfig(config, nil)) + + exists, err := registryKeyExists(dnsPolicyConfigCatchAllPath) + require.NoError(t, err) + assert.False(t, exists, "catch-all rule should not be installed when disabled by env") +} + func registryKeyExists(path string) (bool, error) { k, err := registry.OpenKey(registry.LOCAL_MACHINE, path, registry.QUERY_VALUE) if err != nil {