From afb0525db373179b89f511455d289b27333644c2 Mon Sep 17 00:00:00 2001 From: riccardom Date: Wed, 5 Aug 2026 19:11:44 +0200 Subject: [PATCH] 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 238f2ed3d..d4b3a8674 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 aa6a8bd50..03f317bc7 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")