[client] Add catch-all NRPT rule when NetBird is the primary DNS resolver (#7071)

* [client] Add catch-all NRPT rule when NetBird is the primary DNS resolver

* Remove obvious comments

* 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.

* 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.

* 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.

* Treat the root namespace as a match domain, not a special case

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.

* Do not report a failed NRPT cleanup as success

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

@@ -6,8 +6,10 @@ import (
 	"fmt"
 	"io"
 	"net/netip"
+	"os"
 	"os/exec"
 	"slices"
+	"strconv"
 	"strings"
 	"syscall"
 	"time"
@@ -34,10 +36,16 @@ var (
 // Registry locations of the host DNS configuration this package programs,
 // exported so a diagnostic reader reports the same locations that are written.
 const (
-	// NRPTKeyPrefix starts the name of every NRPT rule key this client creates.
-	// Older versions used different layouts under the same prefix: a single
-	// unsuffixed key, then one key per domain, now one key per batch of domains.
-	NRPTKeyPrefix = "NetBird-Match"
+	// NRPTKeyPrefix starts the name of every NRPT rule key this client creates:
+	// the match rules, the catch-all, and the .local exemption. Cleanup
+	// enumerates by this prefix, so a new kind of rule is removed by existing
+	// code as long as its key starts here.
+	NRPTKeyPrefix = "NetBird-"
+
+	// nrptMatchKeyName names the match-domain rules. Older versions used
+	// different layouts under the same name: a single unsuffixed key, then one
+	// key per domain, now one key per batch of domains.
+	nrptMatchKeyName = NRPTKeyPrefix + "Match"
 
 	// DNSPolicyConfigRoot holds the NRPT rules of the local policy store.
 	DNSPolicyConfigRoot = `SYSTEM\CurrentControlSet\Services\Dnscache\Parameters\DnsPolicyConfig`
@@ -53,8 +61,24 @@ const (
 )
 
 const (
-	dnsPolicyConfigMatchPath    = DNSPolicyConfigRoot + `\` + NRPTKeyPrefix
-	gpoDnsPolicyConfigMatchPath = GPODNSPolicyConfigRoot + `\` + NRPTKeyPrefix
+	dnsPolicyConfigMatchPath    = DNSPolicyConfigRoot + `\` + nrptMatchKeyName
+	gpoDnsPolicyConfigMatchPath = GPODNSPolicyConfigRoot + `\` + nrptMatchKeyName
+
+	dnsPolicyConfigExemptLocalPath    = DNSPolicyConfigRoot + `\` + NRPTKeyPrefix + `ExemptLocal`
+	gpoDnsPolicyConfigExemptLocalPath = GPODNSPolicyConfigRoot + `\` + NRPTKeyPrefix + `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
+	// 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
@@ -293,6 +317,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)
@@ -318,8 +349,22 @@ 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)
+	// 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 {
@@ -397,6 +442,42 @@ func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr
 	return nil
 }
 
+// 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)
@@ -416,7 +497,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)
 	}
 
@@ -514,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 {
@@ -598,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)
@@ -636,6 +732,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)

client/internal/dns/host_windows_test.go

@@ -94,6 +94,145 @@ 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")
+	}
+
+	defer cleanupRegistryKeys(t)
+	cleanupRegistryKeys(t)
+
+	testIP := netip.MustParseAddr("100.64.0.1")
+	testGUID := "{12345678-1234-1234-1234-123456789ABC}"
+	interfacePath := InterfaceConfigPath + `\` + testGUID
+	testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE)
+	require.NoError(t, err, "Should create test interface registry key")
+	require.NoError(t, testKey.Close(), "close test interface registry key")
+	defer func() {
+		assert.NoError(t, registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath), "delete test interface registry key")
+	}()
+
+	cfg := &registryConfigurator{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}},
+	}
+	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))
+	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))
+	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, firstRule, registry.QUERY_VALUE)
+	require.NoError(t, err)
+	servers, _, err := k.GetStringValue(dnsPolicyConfigGenericDNSServersKey)
+	require.NoError(t, err)
+	assert.Equal(t, testIP.String(), servers, "every namespace in the rule resolves through our resolver")
+	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.
+	ek, err := registry.OpenKey(registry.LOCAL_MACHINE, dnsPolicyConfigExemptLocalPath, registry.QUERY_VALUE)
+	require.NoError(t, err, "exemption rule should exist once the root namespace is claimed")
+
+	exemptNames, _, err := ek.GetStringsValue(dnsPolicyConfigNameKey)
+	require.NoError(t, err)
+	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")
+	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")
+	require.NoError(t, ek.Close(), "close exemption rule key")
+
+	require.NoError(t, cfg.applyDNSConfig(matchOnly, nil))
+	names = ruleNamespaces(t, firstRule)
+	assert.NotContains(t, names, nrptCatchAllNamespace, "clearing RouteAll should drop the root namespace")
+
+	exists, err := registryKeyExists(dnsPolicyConfigExemptLocalPath)
+	require.NoError(t, err)
+	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(firstRule)
+	require.NoError(t, err)
+	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
+}
+
+// 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")
+	}
+
+	defer cleanupRegistryKeys(t)
+	cleanupRegistryKeys(t)
+
+	t.Setenv(envLegacyDNSResolution, "true")
+
+	testGUID := "{12345678-1234-1234-1234-123456789ABC}"
+	interfacePath := InterfaceConfigPath + `\` + testGUID
+	testKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, interfacePath, registry.SET_VALUE)
+	require.NoError(t, err, "Should create test interface registry key")
+	require.NoError(t, testKey.Close(), "close test interface registry key")
+	defer func() {
+		assert.NoError(t, registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath), "delete test interface registry key")
+	}()
+
+	cfg := &registryConfigurator{guid: testGUID}
+	config := HostDNSConfig{
+		ServerIP: netip.MustParseAddr("100.64.0.1"),
+		RouteAll: true,
+	}
+
+	require.NoError(t, cfg.applyDNSConfig(config, nil))
+
+	// 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, "no exemption without a claimed root namespace")
+}
+
 func registryKeyExists(path string) (bool, error) {
 	k, err := registry.OpenKey(registry.LOCAL_MACHINE, path, registry.QUERY_VALUE)
 	if err != nil {