From ad0033f8513562679ee1f20e54989df19d9165ad Mon Sep 17 00:00:00 2001 From: riccardom Date: Wed, 5 Aug 2026 19:05:18 +0200 Subject: [PATCH 1/7] [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 4f6ece53291..238f2ed3d43 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 3cd2b1bd5d5..aa6a8bd5067 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 { From afb0525db373179b89f511455d289b27333644c2 Mon Sep 17 00:00:00 2001 From: riccardom Date: Wed, 5 Aug 2026 19:11:44 +0200 Subject: [PATCH 2/7] Remove obvious comments --- client/internal/dns/host_windows.go | 19 ------------------- client/internal/dns/host_windows_test.go | 9 --------- 2 files changed, 28 deletions(-) diff --git a/client/internal/dns/host_windows.go b/client/internal/dns/host_windows.go index 238f2ed3d43..d4b3a86746d 100644 --- a/client/internal/dns/host_windows.go +++ b/client/internal/dns/host_windows.go @@ -41,7 +41,6 @@ const ( 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 @@ -406,21 +405,6 @@ 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) @@ -586,9 +570,6 @@ 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)) } diff --git a/client/internal/dns/host_windows_test.go b/client/internal/dns/host_windows_test.go index aa6a8bd5067..03f317bc7a8 100644 --- a/client/internal/dns/host_windows_test.go +++ b/client/internal/dns/host_windows_test.go @@ -94,9 +94,6 @@ 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") @@ -127,13 +124,11 @@ func TestNRPTCatchAllRule(t *testing.T) { 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) @@ -155,14 +150,11 @@ func TestNRPTCatchAllRule(t *testing.T) { 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) @@ -170,7 +162,6 @@ func TestNRPTCatchAllRule(t *testing.T) { 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") From f767f82396786a0227519d5e433a20ee46034db2 Mon Sep 17 00:00:00 2001 From: riccardom Date: Mon, 24 Aug 2026 17:38:22 +0200 Subject: [PATCH 3/7] Install the catch-all rule where the adapter's DNS is set addDNSSetupForAll makes us the peer's main DNS forwarder, and the catch-all NRPT rule is the other half of that same job: without it the adapter's NameServer only adds one more resolver to the set Windows queries in parallel. Having the two in one place says that, where a separate block at the end of applyDNSConfig read as an afterthought. The block could not simply move up: removeDNSMatchPolicies deletes the catch-all key too, so installing the rule before it ran would have had the rule deleted moments later. The cleanup now runs first, which is what it was always for - it clears what the previous apply installed before this one installs anything - and keeps being unconditional, so a leftover rule from an earlier run cannot survive into a config that no longer wants it. --- client/internal/dns/host_windows.go | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/client/internal/dns/host_windows.go b/client/internal/dns/host_windows.go index d4b3a86746d..f9560f1c4cf 100644 --- a/client/internal/dns/host_windows.go +++ b/client/internal/dns/host_windows.go @@ -287,6 +287,13 @@ func (r *registryConfigurator) disableWINSForInterface() error { } func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error { + // Clear every rule the previous apply installed before installing any new + // one, including a leftover catch-all: removal is unconditional so a rule + // from an earlier run cannot survive into a config that no longer wants it. + if err := r.removeDNSMatchPolicies(); err != nil { + log.Errorf("cleanup old dns match policies: %s", err) + } + if config.RouteAll { if err := r.addDNSSetupForAll(config.ServerIP); err != nil { return fmt.Errorf("add dns setup: %w", err) @@ -312,10 +319,6 @@ func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager matchDomains = append(matchDomains, "."+strings.TrimSuffix(dConf.Domain, ".")) } - if err := r.removeDNSMatchPolicies(); err != nil { - log.Errorf("cleanup old dns match policies: %s", err) - } - if len(matchDomains) != 0 { count, err := r.addDNSMatchPolicy(matchDomains, config.ServerIP) // Update count even on error to ensure cleanup covers partially created rules @@ -329,12 +332,6 @@ 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) } @@ -360,7 +357,12 @@ func (r *registryConfigurator) addDNSSetupForAll(ip netip.Addr) error { } r.routingAll = true log.Infof("configured %s:%d as main DNS forwarder for this peer", ip, DefaultPort) - return nil + + // The adapter's NameServer alone does not make us the system resolver: + // Windows queries the resolvers of every adapter in parallel and takes the + // first answer back. A catch-all NRPT rule is evaluated before adapter + // selection and restricts every name to the servers it lists. + return r.addDNSCatchAllPolicy(ip) } func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr) (int, error) { From cbc803c1ad5e8c98c94f5a69bcff117e81531598 Mon Sep 17 00:00:00 2001 From: riccardom Date: Tue, 25 Aug 2026 22:59:26 +0200 Subject: [PATCH 4/7] Name the escape hatch after the behaviour it restores NB_DISABLE_DNS_CATCHALL_NRPT described the mechanism it switches off. What an operator reaching for it wants is the behaviour they had before, so name it that: NB_USE_LEGACY_DNS_RESOLUTION, matching NB_USE_LEGACY_ROUTING, the only other legacy switch in the client. Not NB_WIN_LEGACY_FULL_TUNNEL_DNS_RESOLVE, as first suggested: the rule follows a primary nameserver group, not a full tunnel, and putting FULL_TUNNEL in a public variable name would carry that confusion for as long as the variable lives. No OS prefix either, since nothing else in the client has one and this switch is inert anywhere but Windows by construction. Behaviour and default are unchanged: the catch-all rule is on unless the variable says otherwise. --- client/internal/dns/host_windows.go | 13 ++++++++----- client/internal/dns/host_windows_test.go | 6 +++--- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/client/internal/dns/host_windows.go b/client/internal/dns/host_windows.go index f9560f1c4cf..a34e43bbace 100644 --- a/client/internal/dns/host_windows.go +++ b/client/internal/dns/host_windows.go @@ -43,9 +43,12 @@ const ( 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" + // envLegacyDNSResolution restores the pre-catch-all behaviour: the adapter's + // NameServer alone, leaving the OS free to query other adapters' resolvers in + // parallel. An escape hatch for setups that depend on a resolver of theirs + // still being reachable while connected, at the cost of the leak and of the + // race the catch-all rule exists to close. + envLegacyDNSResolution = "NB_USE_LEGACY_DNS_RESOLUTION" dnsPolicyConfigVersionKey = "Version" dnsPolicyConfigVersionValue = 2 @@ -408,8 +411,8 @@ func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr } 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) + if parseBoolEnv(envLegacyDNSResolution) { + log.Infof("%s is set, leaving DNS resolution shared with the other adapters' resolvers instead of forcing it through %s", envLegacyDNSResolution, ip) return nil } diff --git a/client/internal/dns/host_windows_test.go b/client/internal/dns/host_windows_test.go index 03f317bc7a8..553f883e898 100644 --- a/client/internal/dns/host_windows_test.go +++ b/client/internal/dns/host_windows_test.go @@ -162,7 +162,7 @@ func TestNRPTCatchAllRule(t *testing.T) { assert.False(t, exists, "catch-all rule should be removed on restore") } -func TestNRPTCatchAllRuleDisabledByEnv(t *testing.T) { +func TestNRPTCatchAllRuleLegacyEnv(t *testing.T) { if testing.Short() { t.Skip("skipping registry integration test in short mode") } @@ -170,7 +170,7 @@ func TestNRPTCatchAllRuleDisabledByEnv(t *testing.T) { defer cleanupRegistryKeys(t) cleanupRegistryKeys(t) - t.Setenv(envDisableCatchAllNRPT, "true") + t.Setenv(envLegacyDNSResolution, "true") testGUID := "{12345678-1234-1234-1234-123456789ABC}" interfacePath := `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\` + testGUID @@ -191,7 +191,7 @@ func TestNRPTCatchAllRuleDisabledByEnv(t *testing.T) { exists, err := registryKeyExists(dnsPolicyConfigCatchAllPath) require.NoError(t, err) - assert.False(t, exists, "catch-all rule should not be installed when disabled by env") + assert.False(t, exists, "catch-all rule should not be installed when the legacy env var is set") } func registryKeyExists(path string) (bool, error) { From c8d5d47701d47b65966a4e0ae1ac2d1a115096c5 Mon Sep 17 00:00:00 2001 From: riccardom Date: Wed, 26 Aug 2026 11:12:01 +0200 Subject: [PATCH 5/7] Exempt .local from the catch-all rule RFC 6762 reserves .local for multicast DNS and says unicast resolvers must not answer for it. The catch-all rule hands it to us anyway, we forward it to whatever upstream the primary nameserver group points at, and the answer comes back NXDOMAIN for hosts that do exist - printers, NAS boxes, anything announcing itself on the link. Confirmed on a Win11Pro VM: laptop.local resolves with the client down and returns "Nome DNS inesistente" with it up, and the client log shows the query arriving on the catch-all handler and being forwarded to 1.1.1.1. An NRPT rule that names a namespace and lists no servers is an exemption: the DNS client resolves those names as it would with no rule at all. What that looks like in the registry is not what it sounds like. Writing no server value and clearing ConfigOptions produces a rule Windows treats as a no-op - it never appears in Get-DnsClientNrptPolicy -Effective and the catch-all keeps the query. The value has to be present and empty, with ConfigOptions still 0x8: the flag says the server list is the meaningful part of the rule, and an empty list then means "no server, resolve normally". Verified both encodings on the VM. Installed together with the catch-all, since without one nothing captures .local in the first place, and removed with it. Exclusivity is unaffected elsewhere, and a more specific rule still wins - a match domain under .local keeps resolving through NetBird, which is what a legacy Active Directory domain named corp.local needs. Verified separately that a match domain does take precedence over the catch-all: declaring fritz.box against the local router restored laptop.fritz.box while the catch-all was in force. --- client/internal/dns/host_windows.go | 56 +++++++++++++++++++++++- client/internal/dns/host_windows_test.go | 22 ++++++++++ 2 files changed, 77 insertions(+), 1 deletion(-) diff --git a/client/internal/dns/host_windows.go b/client/internal/dns/host_windows.go index a34e43bbace..6b8eaaf4431 100644 --- a/client/internal/dns/host_windows.go +++ b/client/internal/dns/host_windows.go @@ -41,7 +41,14 @@ const ( dnsPolicyConfigCatchAllPath = `SYSTEM\CurrentControlSet\Services\Dnscache\Parameters\DnsPolicyConfig\NetBird-CatchAll` gpoDnsPolicyConfigCatchAllPath = gpoDnsPolicyRoot + `\NetBird-CatchAll` + dnsPolicyConfigExemptLocalPath = `SYSTEM\CurrentControlSet\Services\Dnscache\Parameters\DnsPolicyConfig\NetBird-ExemptLocal` + gpoDnsPolicyConfigExemptLocalPath = gpoDnsPolicyRoot + `\NetBird-ExemptLocal` + nrptCatchAllNamespace = "." + // nrptLocalNamespace is reserved for multicast DNS by RFC 6762: a unicast + // resolver must not answer for it. The catch-all rule would hand it to us + // anyway, so it gets an exemption rule of its own. + nrptLocalNamespace = ".local" // envLegacyDNSResolution restores the pre-catch-all behaviour: the adapter's // NameServer alone, leaving the OS free to query other adapters' resolvers in @@ -430,9 +437,46 @@ func (r *registryConfigurator) addDNSCatchAllPolicy(ip netip.Addr) error { } log.Infof("added catch-all NRPT rule: all DNS queries now resolve exclusively through %s", ip) + + return r.addDNSExemptLocalPolicy() +} + +// addDNSExemptLocalPolicy carves .local back out of the catch-all. RFC 6762 +// reserves it for multicast DNS, so forwarding those names to a unicast +// upstream answers NXDOMAIN for hosts that do exist - printers, NAS boxes, and +// anything else announcing itself on the link - and the answer is authoritative +// enough that Windows stops looking. A rule naming the namespace with no +// servers hands it back to the DNS client untouched. A more specific rule still +// wins, so a match domain under .local keeps going through us. +func (r *registryConfigurator) addDNSExemptLocalPolicy() error { + var noServers netip.Addr + + if err := r.configureDNSPolicy(dnsPolicyConfigExemptLocalPath, []string{nrptLocalNamespace}, noServers); err != nil { + return fmt.Errorf("configure exempt policy for %s: %w", nrptLocalNamespace, err) + } + + if r.gpo { + if err := r.configureDNSPolicy(gpoDnsPolicyConfigExemptLocalPath, []string{nrptLocalNamespace}, noServers); err != nil { + return fmt.Errorf("configure gpo exempt policy for %s: %w", nrptLocalNamespace, err) + } + if err := refreshGroupPolicy(); err != nil { + log.Warnf("failed to refresh group policy: %v", err) + } + } + + log.Infof("added NRPT exemption for %s, leaving it to the OS resolver", nrptLocalNamespace) return nil } +// configureDNSPolicy writes one NRPT rule. An invalid ip writes an exemption +// rule: the namespace with an empty server list, which tells the DNS client to +// resolve those names the way it would without any rule at all. +// +// The empty string is the whole difference, and it has to be written: dropping +// the value and clearing ConfigOptions instead produces a rule Windows treats +// as a no-op, keeps out of Get-DnsClientNrptPolicy -Effective, and ignores in +// favour of the catch-all. 0x8 says the server list is the meaningful part of +// the rule, and an empty list then means "no server, resolve normally". 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) @@ -452,7 +496,11 @@ func (r *registryConfigurator) configureDNSPolicy(policyPath string, domains []s return fmt.Errorf("set %s: %w", dnsPolicyConfigNameKey, err) } - if err := regKey.SetStringValue(dnsPolicyConfigGenericDNSServersKey, ip.String()); err != nil { + var servers string + if ip.IsValid() { + servers = ip.String() + } + if err := regKey.SetStringValue(dnsPolicyConfigGenericDNSServersKey, servers); err != nil { return fmt.Errorf("set %s: %w", dnsPolicyConfigGenericDNSServersKey, err) } @@ -575,6 +623,12 @@ func (r *registryConfigurator) removeDNSMatchPolicies() error { merr = multierror.Append(merr, fmt.Errorf("remove GPO base entry: %w", err)) } + for _, path := range []string{dnsPolicyConfigExemptLocalPath, gpoDnsPolicyConfigExemptLocalPath} { + if err := removeRegistryKeyFromDNSPolicyConfig(path); err != nil { + merr = multierror.Append(merr, fmt.Errorf("remove exempt entry %s: %w", path, err)) + } + } + if err := removeRegistryKeyFromDNSPolicyConfig(dnsPolicyConfigCatchAllPath); err != nil { merr = multierror.Append(merr, fmt.Errorf("remove local catch-all entry: %w", err)) } diff --git a/client/internal/dns/host_windows_test.go b/client/internal/dns/host_windows_test.go index 553f883e898..e66d49d4e92 100644 --- a/client/internal/dns/host_windows_test.go +++ b/client/internal/dns/host_windows_test.go @@ -150,11 +150,33 @@ func TestNRPTCatchAllRule(t *testing.T) { assert.EqualValues(t, dnsPolicyConfigConfigOptionsValue, opts) k.Close() + // .local is carved back out: RFC 6762 reserves it for mDNS, so the rule + // names the namespace and lists no servers. + ek, err := registry.OpenKey(registry.LOCAL_MACHINE, dnsPolicyConfigExemptLocalPath, registry.QUERY_VALUE) + require.NoError(t, err, "exemption rule should exist alongside the catch-all") + + exemptNames, _, err := ek.GetStringsValue(dnsPolicyConfigNameKey) + require.NoError(t, err) + assert.Equal(t, []string{nrptLocalNamespace}, exemptNames) + + exemptServers, _, err := ek.GetStringValue(dnsPolicyConfigGenericDNSServersKey) + require.NoError(t, err, "the value has to be present, empty: without it Windows drops the rule") + assert.Empty(t, exemptServers, "an exemption rule lists no servers") + + exemptOpts, _, err := ek.GetIntegerValue(dnsPolicyConfigConfigOptionsKey) + require.NoError(t, err) + assert.EqualValues(t, dnsPolicyConfigConfigOptionsValue, exemptOpts, "same options as a normal rule; the empty server list is what makes it an exemption") + ek.Close() + 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") + exists, err = registryKeyExists(dnsPolicyConfigExemptLocalPath) + require.NoError(t, err) + assert.False(t, exists, "exemption rule should go with the catch-all it belongs to") + require.NoError(t, cfg.applyDNSConfig(primary, nil)) require.NoError(t, cfg.restoreHostDNS()) exists, err = registryKeyExists(dnsPolicyConfigCatchAllPath) From eea25dd9c5e355456c3788dee080b5bf930a53b1 Mon Sep 17 00:00:00 2001 From: riccardom Date: Thu, 27 Aug 2026 13:20:06 +0200 Subject: [PATCH 6/7] Treat the root namespace as a match domain, not a special case MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The catch-all had a function, a registry key and a call site of its own, which made it look like a different mechanism. It is not: "." is an NRPT namespace like any other, it just happens to match every name. So it goes into the match domain list, and addDNSMatchPolicy writes it along with the rest — batching, GPO variant, volatile keys and cleanup all come for free. The .local exemption stays a rule of its own, and not for symmetry: it is the one rule with a different server list, an empty one. Putting it in the same Name value would give it our resolver and exempt nothing. Windows expands a rule's Name value into one effective namespace each, so a rule carrying {.example.com, .} still shows both as separate rows in Get-DnsClientNrptPolicy -Effective. Nothing is lost for diagnosis by dropping the dedicated key. Suggested by Vik in review. --- client/internal/dns/host_windows.go | 52 +++++++---------- client/internal/dns/host_windows_test.go | 72 ++++++++++++++---------- 2 files changed, 60 insertions(+), 64 deletions(-) diff --git a/client/internal/dns/host_windows.go b/client/internal/dns/host_windows.go index 83d6ee268f4..b67014fd086 100644 --- a/client/internal/dns/host_windows.go +++ b/client/internal/dns/host_windows.go @@ -64,9 +64,6 @@ const ( dnsPolicyConfigMatchPath = DNSPolicyConfigRoot + `\` + nrptMatchKeyName gpoDnsPolicyConfigMatchPath = GPODNSPolicyConfigRoot + `\` + nrptMatchKeyName - dnsPolicyConfigCatchAllPath = DNSPolicyConfigRoot + `\` + NRPTKeyPrefix + `CatchAll` - gpoDnsPolicyConfigCatchAllPath = GPODNSPolicyConfigRoot + `\` + NRPTKeyPrefix + `CatchAll` - dnsPolicyConfigExemptLocalPath = DNSPolicyConfigRoot + `\` + NRPTKeyPrefix + `ExemptLocal` gpoDnsPolicyConfigExemptLocalPath = GPODNSPolicyConfigRoot + `\` + NRPTKeyPrefix + `ExemptLocal` @@ -352,6 +349,24 @@ func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager matchDomains = append(matchDomains, "."+strings.TrimSuffix(dConf.Domain, ".")) } + // The root namespace is a match domain like any other: it just happens to + // match every name. Without it the adapter's NameServer only adds one more + // resolver to the set Windows queries in parallel, keeping whichever answer + // comes back first — which leaks every query to the local network and lets a + // resolver other than ours answer for a name we are authoritative for. + if config.RouteAll { + if parseBoolEnv(envLegacyDNSResolution) { + log.Infof("%s is set, leaving DNS resolution shared with the other adapters' resolvers instead of forcing it through %s", envLegacyDNSResolution, config.ServerIP) + } else { + matchDomains = append(matchDomains, nrptCatchAllNamespace) + log.Infof("routing every namespace through %s: DNS resolution is now exclusive to NetBird", config.ServerIP) + + if err := r.addDNSExemptLocalPolicy(); err != nil { + return fmt.Errorf("add dns exempt policy: %w", err) + } + } + } + if len(matchDomains) != 0 { if err := r.addDNSMatchPolicy(matchDomains, config.ServerIP); err != nil { return fmt.Errorf("add dns match policy: %w", err) @@ -384,12 +399,7 @@ func (r *registryConfigurator) addDNSSetupForAll(ip netip.Addr) error { } r.routingAll = true log.Infof("configured %s:%d as main DNS forwarder for this peer", ip, DefaultPort) - - // The adapter's NameServer alone does not make us the system resolver: - // Windows queries the resolvers of every adapter in parallel and takes the - // first answer back. A catch-all NRPT rule is evaluated before adapter - // selection and restricts every name to the servers it lists. - return r.addDNSCatchAllPolicy(ip) + return nil } func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr) error { @@ -432,30 +442,6 @@ func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr return nil } -func (r *registryConfigurator) addDNSCatchAllPolicy(ip netip.Addr) error { - if parseBoolEnv(envLegacyDNSResolution) { - log.Infof("%s is set, leaving DNS resolution shared with the other adapters' resolvers instead of forcing it through %s", envLegacyDNSResolution, 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 r.addDNSExemptLocalPolicy() -} - // addDNSExemptLocalPolicy carves .local back out of the catch-all. RFC 6762 // reserves it for multicast DNS, so forwarding those names to a unicast // upstream answers NXDOMAIN for hosts that do exist - printers, NAS boxes, and diff --git a/client/internal/dns/host_windows_test.go b/client/internal/dns/host_windows_test.go index 9bd5e3895dd..ab5f2dc8d95 100644 --- a/client/internal/dns/host_windows_test.go +++ b/client/internal/dns/host_windows_test.go @@ -104,7 +104,7 @@ func TestNRPTCatchAllRule(t *testing.T) { testIP := netip.MustParseAddr("100.64.0.1") testGUID := "{12345678-1234-1234-1234-123456789ABC}" - interfacePath := `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\` + testGUID + interfacePath := InterfaceConfigPath + `\` + testGUID testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE) require.NoError(t, err, "Should create test interface registry key") testKey.Close() @@ -123,37 +123,31 @@ func TestNRPTCatchAllRule(t *testing.T) { RouteAll: true, Domains: []DomainConfig{{Domain: "example.com", MatchOnly: true}}, } + firstRule := fmt.Sprintf("%s-0", dnsPolicyConfigMatchPath) + // The root namespace is not a rule of its own: it rides in the match rule, + // which is the point of it not being a special case. 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") + names := ruleNamespaces(t, firstRule) + assert.Contains(t, names, ".example.com") + assert.NotContains(t, names, nrptCatchAllNamespace, "a match-only config must not claim every namespace") 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") + names = ruleNamespaces(t, firstRule) + assert.Contains(t, names, ".example.com") + assert.Contains(t, names, nrptCatchAllNamespace, "RouteAll should add the root namespace to the match rule") - k, err := registry.OpenKey(registry.LOCAL_MACHINE, dnsPolicyConfigCatchAllPath, registry.QUERY_VALUE) - require.NoError(t, err) - - names, _, err := k.GetStringsValue(dnsPolicyConfigNameKey) + k, err := registry.OpenKey(registry.LOCAL_MACHINE, firstRule, registry.QUERY_VALUE) 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) + assert.Equal(t, testIP.String(), servers, "every namespace in the rule resolves through our resolver") k.Close() - // .local is carved back out: RFC 6762 reserves it for mDNS, so the rule - // names the namespace and lists no servers. + // .local is carved back out: RFC 6762 reserves it for mDNS, so it needs a + // rule of its own — it is the one rule with a different server list. ek, err := registry.OpenKey(registry.LOCAL_MACHINE, dnsPolicyConfigExemptLocalPath, registry.QUERY_VALUE) - require.NoError(t, err, "exemption rule should exist alongside the catch-all") + require.NoError(t, err, "exemption rule should exist once the root namespace is claimed") exemptNames, _, err := ek.GetStringsValue(dnsPolicyConfigNameKey) require.NoError(t, err) @@ -169,19 +163,30 @@ func TestNRPTCatchAllRule(t *testing.T) { ek.Close() 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") + names = ruleNamespaces(t, firstRule) + assert.NotContains(t, names, nrptCatchAllNamespace, "clearing RouteAll should drop the root namespace") - exists, err = registryKeyExists(dnsPolicyConfigExemptLocalPath) + exists, err := registryKeyExists(dnsPolicyConfigExemptLocalPath) require.NoError(t, err) - assert.False(t, exists, "exemption rule should go with the catch-all it belongs to") + assert.False(t, exists, "exemption rule should go with the namespace it carves out of") require.NoError(t, cfg.applyDNSConfig(primary, nil)) require.NoError(t, cfg.restoreHostDNS()) - exists, err = registryKeyExists(dnsPolicyConfigCatchAllPath) + exists, err = registryKeyExists(firstRule) require.NoError(t, err) - assert.False(t, exists, "catch-all rule should be removed on restore") + assert.False(t, exists, "restore should leave no rule behind") +} + +// ruleNamespaces returns the namespaces an NRPT rule key claims. +func ruleNamespaces(t *testing.T, path string) []string { + t.Helper() + k, err := registry.OpenKey(registry.LOCAL_MACHINE, path, registry.QUERY_VALUE) + require.NoError(t, err, "rule key %s should exist", path) + defer k.Close() + + names, _, err := k.GetStringsValue(dnsPolicyConfigNameKey) + require.NoError(t, err) + return names } func TestNRPTCatchAllRuleLegacyEnv(t *testing.T) { @@ -195,7 +200,7 @@ func TestNRPTCatchAllRuleLegacyEnv(t *testing.T) { t.Setenv(envLegacyDNSResolution, "true") testGUID := "{12345678-1234-1234-1234-123456789ABC}" - interfacePath := `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\` + testGUID + interfacePath := InterfaceConfigPath + `\` + testGUID testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE) require.NoError(t, err, "Should create test interface registry key") testKey.Close() @@ -211,9 +216,14 @@ func TestNRPTCatchAllRuleLegacyEnv(t *testing.T) { require.NoError(t, cfg.applyDNSConfig(config, nil)) - exists, err := registryKeyExists(dnsPolicyConfigCatchAllPath) + // RouteAll with no match domains and the switch set leaves nothing to write. + exists, err := registryKeyExists(fmt.Sprintf("%s-0", dnsPolicyConfigMatchPath)) + require.NoError(t, err) + assert.False(t, exists, "no rule should be written when the legacy env var is set") + + exists, err = registryKeyExists(dnsPolicyConfigExemptLocalPath) require.NoError(t, err) - assert.False(t, exists, "catch-all rule should not be installed when the legacy env var is set") + assert.False(t, exists, "no exemption without a claimed root namespace") } func registryKeyExists(path string) (bool, error) { From 654fe710d9bdac5fe021eeb3dd3fc0cd5304e914 Mon Sep 17 00:00:00 2001 From: riccardom Date: Thu, 27 Aug 2026 14:10:30 +0200 Subject: [PATCH 7/7] Do not report a failed NRPT cleanup as success MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit removeRegistryKeyFromDNSPolicyConfig returned nil for every OpenKey error, so a permission or registry failure was indistinguishable from a key that was never there. Cleanup then reported success while the rule stayed in force — which is how a rule outlives the interface it points at and keeps sending every query to an address that no longer answers. Distinguish the two, the way listNRPTRuleKeys already does for the policy store root: a missing key is nothing to do, anything else reaches the caller. restoreHostDNS now propagates that error instead of logging it. applyDNSConfig keeps logging on purpose: there we are about to write fresh rules over whatever survived, while restore is the path where a rule left behind is the whole problem. Also addresses review nits on the tests: doc comments on the two added cases, reported Close and DeleteKey errors so a failed cleanup cannot contaminate the next registry test, and a context message on the exemption's namespace assertion. --- client/internal/dns/host_windows.go | 17 ++++++++++++++--- client/internal/dns/host_windows_test.go | 21 ++++++++++++++------- 2 files changed, 28 insertions(+), 10 deletions(-) diff --git a/client/internal/dns/host_windows.go b/client/internal/dns/host_windows.go index b67014fd086..948000a3d91 100644 --- a/client/internal/dns/host_windows.go +++ b/client/internal/dns/host_windows.go @@ -599,8 +599,11 @@ func (r *registryConfigurator) getInterfaceRegistryKey() (registry.Key, error) { } func (r *registryConfigurator) restoreHostDNS() error { + // Propagated, unlike in applyDNSConfig: there we are about to write fresh + // rules over whatever survived, here we are leaving, and a rule left behind + // keeps sending every query to an address that is about to disappear. if err := r.removeDNSMatchPolicies(); err != nil { - log.Errorf("remove dns match policies: %s", err) + return fmt.Errorf("remove dns match policies: %w", err) } if err := r.deleteInterfaceRegistryKeyProperty(interfaceConfigSearchListKey); err != nil { @@ -683,9 +686,17 @@ func listNRPTRuleKeys(root string) ([]string, error) { func removeRegistryKeyFromDNSPolicyConfig(regKeyPath string) error { k, err := registry.OpenKey(registry.LOCAL_MACHINE, regKeyPath, registry.QUERY_VALUE) - if err != nil { - log.Debugf("failed to open HKEY_LOCAL_MACHINE\\%s: %v", regKeyPath, err) + switch { + case errors.Is(err, registry.ErrNotExist), errors.Is(err, syscall.ERROR_PATH_NOT_FOUND): + // nothing to remove, which is the normal case for a rule this config + // never installed + log.Debugf("HKEY_LOCAL_MACHINE\\%s does not exist", regKeyPath) return nil + case err != nil: + // anything else has to reach the caller: reporting success here would + // leave the rule in force while claiming it was removed, which is how a + // stale rule outlives the interface it points at + return fmt.Errorf("open HKEY_LOCAL_MACHINE\\%s: %w", regKeyPath, err) } closer(k) diff --git a/client/internal/dns/host_windows_test.go b/client/internal/dns/host_windows_test.go index ab5f2dc8d95..7aef64590dc 100644 --- a/client/internal/dns/host_windows_test.go +++ b/client/internal/dns/host_windows_test.go @@ -94,6 +94,10 @@ func TestNRPTEntriesCleanupOnConfigChange(t *testing.T) { assert.False(t, exists, "NRPT rule 2 should NOT exist after reducing to 75 domains") } +// TestNRPTCatchAllRule verifies that RouteAll adds the root namespace to the +// match rule instead of a rule of its own, that .local is carved back out with +// an empty server list, and that both go away when RouteAll is cleared or the +// host DNS is restored. func TestNRPTCatchAllRule(t *testing.T) { if testing.Short() { t.Skip("skipping registry integration test in short mode") @@ -107,9 +111,9 @@ func TestNRPTCatchAllRule(t *testing.T) { interfacePath := InterfaceConfigPath + `\` + testGUID testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE) require.NoError(t, err, "Should create test interface registry key") - testKey.Close() + require.NoError(t, testKey.Close(), "close test interface registry key") defer func() { - _ = registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath) + assert.NoError(t, registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath), "delete test interface registry key") }() cfg := ®istryConfigurator{guid: testGUID} @@ -142,7 +146,7 @@ func TestNRPTCatchAllRule(t *testing.T) { servers, _, err := k.GetStringValue(dnsPolicyConfigGenericDNSServersKey) require.NoError(t, err) assert.Equal(t, testIP.String(), servers, "every namespace in the rule resolves through our resolver") - k.Close() + require.NoError(t, k.Close(), "close match rule key") // .local is carved back out: RFC 6762 reserves it for mDNS, so it needs a // rule of its own — it is the one rule with a different server list. @@ -151,7 +155,7 @@ func TestNRPTCatchAllRule(t *testing.T) { exemptNames, _, err := ek.GetStringsValue(dnsPolicyConfigNameKey) require.NoError(t, err) - assert.Equal(t, []string{nrptLocalNamespace}, exemptNames) + assert.Equal(t, []string{nrptLocalNamespace}, exemptNames, "the exemption should name only the mDNS namespace") exemptServers, _, err := ek.GetStringValue(dnsPolicyConfigGenericDNSServersKey) require.NoError(t, err, "the value has to be present, empty: without it Windows drops the rule") @@ -160,7 +164,7 @@ func TestNRPTCatchAllRule(t *testing.T) { exemptOpts, _, err := ek.GetIntegerValue(dnsPolicyConfigConfigOptionsKey) require.NoError(t, err) assert.EqualValues(t, dnsPolicyConfigConfigOptionsValue, exemptOpts, "same options as a normal rule; the empty server list is what makes it an exemption") - ek.Close() + require.NoError(t, ek.Close(), "close exemption rule key") require.NoError(t, cfg.applyDNSConfig(matchOnly, nil)) names = ruleNamespaces(t, firstRule) @@ -189,6 +193,9 @@ func ruleNamespaces(t *testing.T, path string) []string { return names } +// TestNRPTCatchAllRuleLegacyEnv verifies that NB_USE_LEGACY_DNS_RESOLUTION +// leaves the root namespace unclaimed, so no rule is written for a RouteAll +// config that carries no match domains. func TestNRPTCatchAllRuleLegacyEnv(t *testing.T) { if testing.Short() { t.Skip("skipping registry integration test in short mode") @@ -203,9 +210,9 @@ func TestNRPTCatchAllRuleLegacyEnv(t *testing.T) { interfacePath := InterfaceConfigPath + `\` + testGUID testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE) require.NoError(t, err, "Should create test interface registry key") - testKey.Close() + require.NoError(t, testKey.Close(), "close test interface registry key") defer func() { - _ = registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath) + assert.NoError(t, registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath), "delete test interface registry key") }() cfg := ®istryConfigurator{guid: testGUID}