Merge remote-tracking branch 'origin/main' into HEAD

# Conflicts:
#	client/internal/dnsfwd/forwarder.go

client/internal/dns/host_darwin.go

@@ -267,18 +267,38 @@ func (s *systemConfigurator) getSystemDNSSettings() (SystemDNSSettings, error) {
 		return SystemDNSSettings{}, fmt.Errorf("sending the command: %w", err)
 	}
 
-	var dnsSettings SystemDNSSettings
+	dnsSettings, serverAddresses, err := parseSystemDNSSettings(b)
+	if err != nil {
+		return dnsSettings, err
+	}
+
+	s.mu.Lock()
+	s.origNameservers = serverAddresses
+	s.mu.Unlock()
+
+	return dnsSettings, nil
+}
+
+// parseSystemDNSSettings parses the output of `scutil show State:/Network/Service/<id>/DNS`.
+// Lines that don't match the expected "index : value" shape are skipped: hosts with unusual
+// network services (e.g. orphaned hardware ports) can produce entries without a value.
+func parseSystemDNSSettings(out []byte) (SystemDNSSettings, []netip.Addr, error) {
+	// port is not exposed by scutil, default to 53
+	dnsSettings := SystemDNSSettings{ServerPort: DefaultPort}
 	var serverAddresses []netip.Addr
 	inSearchDomainsArray := false
 	inServerAddressesArray := false
 
-	scanner := bufio.NewScanner(bytes.NewReader(b))
+	scanner := bufio.NewScanner(bytes.NewReader(out))
 	for scanner.Scan() {
 		line := strings.TrimSpace(scanner.Text())
 		switch {
 		case strings.HasPrefix(line, "DomainName :"):
-			domainName := strings.TrimSpace(strings.Split(line, ":")[1])
-			dnsSettings.Domains = append(dnsSettings.Domains, domainName)
+			domainName := strings.TrimSpace(strings.TrimPrefix(line, "DomainName :"))
+			if domainName != "" {
+				dnsSettings.Domains = append(dnsSettings.Domains, domainName)
+			}
+			continue
 		case line == "SearchDomains : <array> {":
 			inSearchDomainsArray = true
 			continue
@@ -288,36 +308,45 @@ func (s *systemConfigurator) getSystemDNSSettings() (SystemDNSSettings, error) {
 		case line == "}":
 			inSearchDomainsArray = false
 			inServerAddressesArray = false
+			continue
+		}
+
+		if !inSearchDomainsArray && !inServerAddressesArray {
+			continue
+		}
+
+		parts := strings.SplitN(line, " : ", 2)
+		if len(parts) != 2 {
+			log.Debugf("skipping unexpected scutil DNS line %q", line)
+			continue
+		}
+		value := strings.TrimSpace(parts[1])
+		if value == "" {
+			continue
 		}
 
 		if inSearchDomainsArray {
-			searchDomain := strings.Split(line, " : ")[1]
-			dnsSettings.Domains = append(dnsSettings.Domains, searchDomain)
-		} else if inServerAddressesArray {
-			address := strings.Split(line, " : ")[1]
-			if ip, err := netip.ParseAddr(address); err == nil && !ip.IsUnspecified() {
-				ip = ip.Unmap()
-				serverAddresses = append(serverAddresses, ip)
-				// Prefer the first IPv4 server as ServerIP since our DNS listener is IPv4.
-				if !dnsSettings.ServerIP.IsValid() && ip.Is4() {
-					dnsSettings.ServerIP = ip
-				}
-			}
+			dnsSettings.Domains = append(dnsSettings.Domains, value)
+			continue
+		}
+
+		ip, err := netip.ParseAddr(value)
+		if err != nil || ip.IsUnspecified() {
+			continue
+		}
+		ip = ip.Unmap()
+		serverAddresses = append(serverAddresses, ip)
+		// Prefer the first IPv4 server as ServerIP since our DNS listener is IPv4.
+		if !dnsSettings.ServerIP.IsValid() && ip.Is4() {
+			dnsSettings.ServerIP = ip
 		}
 	}
 
 	if err := scanner.Err(); err != nil {
-		return dnsSettings, err
+		return dnsSettings, serverAddresses, err
 	}
 
-	// default to 53 port
-	dnsSettings.ServerPort = DefaultPort
-
-	s.mu.Lock()
-	s.origNameservers = serverAddresses
-	s.mu.Unlock()
-
-	return dnsSettings, nil
+	return dnsSettings, serverAddresses, nil
 }
 
 func (s *systemConfigurator) getOriginalNameservers() []netip.Addr {
@@ -435,11 +464,15 @@ func (s *systemConfigurator) getPrimaryService() (string, string, error) {
 	router := ""
 	for scanner.Scan() {
 		text := scanner.Text()
+		parts := strings.SplitN(text, ":", 2)
+		if len(parts) != 2 {
+			continue
+		}
 		if strings.Contains(text, "PrimaryService") {
-			primaryService = strings.TrimSpace(strings.Split(text, ":")[1])
+			primaryService = strings.TrimSpace(parts[1])
 		}
 		if strings.Contains(text, "Router") {
-			router = strings.TrimSpace(strings.Split(text, ":")[1])
+			router = strings.TrimSpace(parts[1])
 		}
 	}
 	if err := scanner.Err(); err != nil && err != io.EOF {

client/internal/dns/host_darwin_test.go

@@ -328,6 +328,120 @@ func removeTestDNSKey(key string) error {
 	return err
 }
 
+func TestParseSystemDNSSettings(t *testing.T) {
+	tests := []struct {
+		name            string
+		output          string
+		expectedDomains []string
+		expectedServers []netip.Addr
+		expectedIP      netip.Addr
+	}{
+		{
+			name: "well_formed",
+			output: `<dictionary> {
+  DomainName : example.com
+  SearchDomains : <array> {
+    0 : example.com
+    1 : corp.example.com
+  }
+  ServerAddresses : <array> {
+    0 : 192.168.1.1
+    1 : fd00::53
+  }
+}
+`,
+			expectedDomains: []string{"example.com", "example.com", "corp.example.com"},
+			expectedServers: []netip.Addr{netip.MustParseAddr("192.168.1.1"), netip.MustParseAddr("fd00::53")},
+			expectedIP:      netip.MustParseAddr("192.168.1.1"),
+		},
+		{
+			// entries without a value after the separator used to panic with
+			// "index out of range [1] with length 1"
+			name: "malformed_array_entries_skipped",
+			output: `<dictionary> {
+  SearchDomains : <array> {
+    0 :
+    (null)
+
+    1 : corp.example.com
+  }
+  ServerAddresses : <array> {
+    0 :
+    1 : 192.168.1.1
+  }
+}
+`,
+			expectedDomains: []string{"corp.example.com"},
+			expectedServers: []netip.Addr{netip.MustParseAddr("192.168.1.1")},
+			expectedIP:      netip.MustParseAddr("192.168.1.1"),
+		},
+		{
+			name: "domain_name_without_value_skipped",
+			output: `<dictionary> {
+  DomainName :
+  ServerAddresses : <array> {
+    0 : 192.168.1.1
+  }
+}
+`,
+			expectedServers: []netip.Addr{netip.MustParseAddr("192.168.1.1")},
+			expectedIP:      netip.MustParseAddr("192.168.1.1"),
+		},
+		{
+			name: "ipv6_first_prefers_ipv4_server_ip",
+			output: `<dictionary> {
+  ServerAddresses : <array> {
+    0 : fd00::53
+    1 : 192.168.1.1
+  }
+}
+`,
+			expectedServers: []netip.Addr{netip.MustParseAddr("fd00::53"), netip.MustParseAddr("192.168.1.1")},
+			expectedIP:      netip.MustParseAddr("192.168.1.1"),
+		},
+		{
+			name: "invalid_and_unspecified_addresses_skipped",
+			output: `<dictionary> {
+  ServerAddresses : <array> {
+    0 : (null)
+    1 : 0.0.0.0
+    2 : 192.168.1.1
+  }
+}
+`,
+			expectedServers: []netip.Addr{netip.MustParseAddr("192.168.1.1")},
+			expectedIP:      netip.MustParseAddr("192.168.1.1"),
+		},
+		{
+			name: "v4_mapped_address_unmapped",
+			output: `<dictionary> {
+  ServerAddresses : <array> {
+    0 : ::ffff:192.168.1.1
+  }
+}
+`,
+			expectedServers: []netip.Addr{netip.MustParseAddr("192.168.1.1")},
+			expectedIP:      netip.MustParseAddr("192.168.1.1"),
+		},
+		{
+			name:   "empty_output",
+			output: "",
+		},
+	}
+
+	for _, tc := range tests {
+		t.Run(tc.name, func(t *testing.T) {
+			settings, servers, err := parseSystemDNSSettings([]byte(tc.output))
+			require.NoError(t, err, "parsing should not fail")
+
+			assert.Equal(t, tc.expectedDomains, settings.Domains, "domains should match")
+			assert.Equal(t, tc.expectedServers, servers, "server addresses should match")
+			assert.Equal(t, tc.expectedIP, settings.ServerIP, "server IP should match")
+			assert.Equal(t, DefaultPort, settings.ServerPort, "server port should default to 53")
+		})
+	}
+}
+
 func TestGetOriginalNameservers(t *testing.T) {
 	configurator := &systemConfigurator{
 		createdKeys: make(map[string]struct{}),

client/internal/dns/host_windows.go

@@ -6,8 +6,10 @@ import (
 	"fmt"
 	"io"
 	"net/netip"
+	"os"
 	"os/exec"
 	"slices"
+	"strconv"
 	"strings"
 	"syscall"
 	"time"
@@ -31,10 +33,52 @@ var (
 	dnsFlushResolverCacheFn = dnsapi.NewProc("DnsFlushResolverCache")
 )
 
+// Registry locations of the host DNS configuration this package programs,
+// exported so a diagnostic reader reports the same locations that are written.
 const (
-	dnsPolicyConfigMatchPath    = `SYSTEM\CurrentControlSet\Services\Dnscache\Parameters\DnsPolicyConfig\NetBird-Match`
-	gpoDnsPolicyRoot            = `SOFTWARE\Policies\Microsoft\Windows NT\DNSClient\DnsPolicyConfig`
-	gpoDnsPolicyConfigMatchPath = gpoDnsPolicyRoot + `\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`
+
+	// GPODNSPolicyConfigRoot holds the NRPT rules of the group policy store,
+	// which takes precedence over the local one when it is present.
+	GPODNSPolicyConfigRoot = `SOFTWARE\Policies\Microsoft\Windows NT\DNSClient\DnsPolicyConfig`
+
+	// InterfaceConfigPath and InterfaceConfigPathV6 hold the per-interface DNS
+	// settings, keyed by interface GUID, in separate hives per address family.
+	InterfaceConfigPath   = `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces`
+	InterfaceConfigPathV6 = `SYSTEM\CurrentControlSet\Services\Tcpip6\Parameters\Interfaces`
+)
+
+const (
+	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
@@ -45,8 +89,6 @@ const (
 
 	nrptMaxDomainsPerRule = 50
 
-	interfaceConfigPath           = `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces`
-	interfaceConfigPathV6         = `SYSTEM\CurrentControlSet\Services\Tcpip6\Parameters\Interfaces`
 	interfaceConfigNameServerKey  = "NameServer"
 	interfaceConfigDhcpNameSrvKey = "DhcpNameServer"
 	interfaceConfigSearchListKey  = "SearchList"
@@ -73,7 +115,6 @@ type registryConfigurator struct {
 	guid            string
 	routingAll      bool
 	gpo             bool
-	nrptEntryCount  int
 	origNameservers []netip.Addr
 }
 
@@ -83,19 +124,9 @@ func newHostManager(wgInterface WGIface) (*registryConfigurator, error) {
 		return nil, err
 	}
 
-	var useGPO bool
-	k, err := registry.OpenKey(registry.LOCAL_MACHINE, gpoDnsPolicyRoot, registry.QUERY_VALUE)
-	if err != nil {
-		log.Debugf("failed to open GPO DNS policy root: %v", err)
-	} else {
-		closer(k)
-		useGPO = true
-		log.Infof("detected GPO DNS policy configuration, using policy store")
-	}
-
 	configurator := &registryConfigurator{
 		guid: guid,
-		gpo:  useGPO,
+		gpo:  useGPOPolicyStore(),
 	}
 
 	origNameservers, err := configurator.captureOriginalNameservers()
@@ -123,7 +154,7 @@ func (r *registryConfigurator) captureOriginalNameservers() ([]netip.Addr, error
 	seen := make(map[netip.Addr]struct{})
 	var out []netip.Addr
 	var merr *multierror.Error
-	for _, root := range []string{interfaceConfigPath, interfaceConfigPathV6} {
+	for _, root := range []string{InterfaceConfigPath, InterfaceConfigPathV6} {
 		addrs, err := r.captureFromTcpipRoot(root)
 		if err != nil {
 			merr = multierror.Append(merr, fmt.Errorf("%s: %w", root, err))
@@ -276,6 +307,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)
@@ -301,19 +339,28 @@ 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 {
-		count, err := r.addDNSMatchPolicy(matchDomains, config.ServerIP)
-		// Update count even on error to ensure cleanup covers partially created rules
-		r.nrptEntryCount = count
-		if err != nil {
+		if err := r.addDNSMatchPolicy(matchDomains, config.ServerIP); err != nil {
 			return fmt.Errorf("add dns match policy: %w", err)
 		}
-	} else {
-		r.nrptEntryCount = 0
 	}
 
 	r.updateState(stateManager)
@@ -329,9 +376,8 @@ func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager
 
 func (r *registryConfigurator) updateState(stateManager *statemanager.Manager) {
 	if err := stateManager.UpdateState(&ShutdownState{
-		Guid:           r.guid,
-		GPO:            r.gpo,
-		NRPTEntryCount: r.nrptEntryCount,
+		Guid: r.guid,
+		GPO:  r.gpo,
 	}); err != nil {
 		log.Errorf("failed to update shutdown state: %s", err)
 	}
@@ -346,7 +392,7 @@ func (r *registryConfigurator) addDNSSetupForAll(ip netip.Addr) error {
 	return nil
 }
 
-func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr) (int, error) {
+func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr) error {
 	// if the gpo key is present, we need to put our DNS settings there, otherwise our config might be ignored
 	// see https://learn.microsoft.com/en-us/openspecs/windows_protocols/ms-gpnrpt/8cc31cb9-20cb-4140-9e85-3e08703b4745
 
@@ -363,19 +409,17 @@ func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr
 		gpoPath := fmt.Sprintf("%s-%d", gpoDnsPolicyConfigMatchPath, ruleIndex)
 
 		if err := r.configureDNSPolicy(localPath, batchDomains, ip); err != nil {
-			return ruleIndex, fmt.Errorf("configure DNS Local policy for rule %d: %w", ruleIndex, err)
+			return fmt.Errorf("configure DNS Local policy for rule %d: %w", ruleIndex, err)
 		}
 
-		// Increment immediately so the caller's cleanup path knows about this rule
-		ruleIndex++
-
 		if r.gpo {
 			if err := r.configureDNSPolicy(gpoPath, batchDomains, ip); err != nil {
-				return ruleIndex, fmt.Errorf("configure gpo DNS policy for rule %d: %w", ruleIndex-1, err)
+				return fmt.Errorf("configure gpo DNS policy for rule %d: %w", ruleIndex, err)
 			}
 		}
 
-		log.Debugf("added NRPT rule %d with %d domains", ruleIndex-1, len(batchDomains))
+		log.Debugf("added NRPT rule %d with %d domains", ruleIndex, len(batchDomains))
+		ruleIndex++
 	}
 
 	if r.gpo {
@@ -385,9 +429,45 @@ func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip netip.Addr
 	}
 
 	log.Infof("added %d NRPT rules for %d domains", ruleIndex, len(domains))
-	return ruleIndex, nil
+	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)
@@ -407,7 +487,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)
 	}
 
@@ -450,7 +534,7 @@ func (r *registryConfigurator) flushDNSCache() {
 
 	ret, _, err := dnsFlushResolverCacheFn.Call()
 	if ret == 0 {
-		if err != nil && !errors.Is(err, syscall.Errno(0)) {
+		if !errors.Is(err, syscall.Errno(0)) {
 			log.Errorf("DnsFlushResolverCache failed: %v", err)
 			return
 		}
@@ -482,21 +566,29 @@ func (r *registryConfigurator) setInterfaceRegistryKeyStringValue(key, value str
 	return nil
 }
 
+// deleteInterfaceRegistryKeyProperty removes a value from the interface key.
+// A value that is already gone, or an interface key that is, is not an error:
+// the caller asked for the value not to be there, and a cleanup that runs twice
+// has to reach its later steps on the second run as well.
 func (r *registryConfigurator) deleteInterfaceRegistryKeyProperty(propertyKey string) error {
 	regKey, err := r.getInterfaceRegistryKey()
-	if err != nil {
+	switch {
+	case errors.Is(err, registry.ErrNotExist), errors.Is(err, syscall.ERROR_PATH_NOT_FOUND):
+		log.Debugf("interface key of %s does not exist, nothing to delete %s from", r.guid, propertyKey)
+		return nil
+	case err != nil:
 		return fmt.Errorf("get interface registry key: %w", err)
 	}
 	defer closer(regKey)
 
-	if err := regKey.DeleteValue(propertyKey); err != nil {
+	if err := regKey.DeleteValue(propertyKey); err != nil && !errors.Is(err, registry.ErrNotExist) {
 		return fmt.Errorf("delete registry key %s: %w", propertyKey, err)
 	}
 	return nil
 }
 
 func (r *registryConfigurator) getInterfaceRegistryKey() (registry.Key, error) {
-	regKeyPath := interfaceConfigPath + "\\" + r.guid
+	regKeyPath := InterfaceConfigPath + "\\" + r.guid
 	regKey, err := registry.OpenKey(registry.LOCAL_MACHINE, regKeyPath, registry.SET_VALUE)
 	if err != nil {
 		return regKey, fmt.Errorf("open HKEY_LOCAL_MACHINE\\%s: %w", regKeyPath, err)
@@ -505,8 +597,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 {
@@ -515,31 +610,36 @@ func (r *registryConfigurator) restoreHostDNS() error {
 
 	go r.flushDNSCache()
 
-	return nil
+	// Last, and only on the way out, once no rule of ours is left: during a
+	// session the store is where the rules of this run live, and emptying it
+	// mid-session would have the next rule recreate it anyway. Propagated so a
+	// failure keeps the shutdown state for the next run to retry, rather than
+	// leaving the store to hold up every rule change from here on.
+	return removeEmptyGPOPolicyStore()
 }
 
+// removeDNSMatchPolicies deletes every NRPT rule this client may have created,
+// from the local and the GPO policy store. The rules are found by enumerating
+// the registry, the only authoritative record of what was written. Cleanup must
+// not depend on a rule count: the in-memory one is scoped to a single
+// registryConfigurator and the persisted one is deleted on every clean
+// disconnect, and a rule left behind keeps resolving names over an interface
+// that is gone, until reboot discards the volatile key.
 func (r *registryConfigurator) removeDNSMatchPolicies() error {
 	var merr *multierror.Error
 
-	// Try to remove the base entries (for backward compatibility)
-	if err := removeRegistryKeyFromDNSPolicyConfig(dnsPolicyConfigMatchPath); err != nil {
-		merr = multierror.Append(merr, fmt.Errorf("remove local base entry: %w", err))
-	}
-
-	if err := removeRegistryKeyFromDNSPolicyConfig(gpoDnsPolicyConfigMatchPath); err != nil {
-		merr = multierror.Append(merr, fmt.Errorf("remove GPO base entry: %w", err))
-	}
-
-	for i := 0; i < r.nrptEntryCount; i++ {
-		localPath := fmt.Sprintf("%s-%d", dnsPolicyConfigMatchPath, i)
-		gpoPath := fmt.Sprintf("%s-%d", gpoDnsPolicyConfigMatchPath, i)
-
-		if err := removeRegistryKeyFromDNSPolicyConfig(localPath); err != nil {
-			merr = multierror.Append(merr, fmt.Errorf("remove local entry %d: %w", i, err))
+	for _, root := range []string{DNSPolicyConfigRoot, GPODNSPolicyConfigRoot} {
+		names, err := listNRPTRuleKeys(root)
+		if err != nil {
+			merr = multierror.Append(merr, fmt.Errorf("list rule keys under %s: %w", root, err))
+			continue
 		}
 
-		if err := removeRegistryKeyFromDNSPolicyConfig(gpoPath); err != nil {
-			merr = multierror.Append(merr, fmt.Errorf("remove GPO entry %d: %w", i, err))
+		for _, name := range names {
+			path := root + `\` + name
+			if err := removeRegistryKeyFromDNSPolicyConfig(path); err != nil {
+				merr = multierror.Append(merr, fmt.Errorf("remove entry %s: %w", path, err))
+			}
 		}
 	}
 
@@ -554,11 +654,119 @@ func (r *registryConfigurator) restoreUncleanShutdownDNS() error {
 	return r.restoreHostDNS()
 }
 
+// useGPOPolicyStore reports whether NRPT rules have to go into the group policy
+// store, and clears an empty one out of the way first.
+//
+// The order is the point. A store left empty by an earlier run would otherwise
+// decide this run too, sending its rules somewhere the resolver only reads when
+// the policy engine next applies DNS client policy. Removing it before the
+// choice is made leaves the local store authoritative for the whole session,
+// including the first one after an upgrade.
+func useGPOPolicyStore() bool {
+	if err := removeEmptyGPOPolicyStore(); err != nil {
+		// Nothing to retry against here: the worst case is the run going
+		// through the group policy store, which is where it would have gone
+		// before this check existed.
+		log.Warnf("%v", err)
+	}
+
+	k, err := registry.OpenKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot, registry.QUERY_VALUE)
+	if err != nil {
+		log.Debugf("failed to open GPO DNS policy root: %v", err)
+		return false
+	}
+	closer(k)
+
+	log.Infof("detected GPO DNS policy configuration, using policy store")
+	return true
+}
+
+// removeEmptyGPOPolicyStore deletes the group policy DnsPolicyConfig key once
+// nothing is left in it. The key survives the deletion of the last rule it
+// held, and the client treats its presence as "group policy configures the
+// NRPT", so an empty one left behind keeps every later run writing rules there.
+// Rules in that store reach the resolver only when the policy engine next
+// applies DNS client policy, and a rule this client writes belongs to no GPO,
+// so nothing schedules that application: both adding and removing a rule are
+// held up by a minute or more, and for a removal that is a catch-all rule
+// resolving every name over an interface that no longer exists. With the store
+// absent the local one is authoritative and a change applies at once.
+//
+// A store that still holds rules, values or subkeys of somebody else's is left
+// alone.
+func removeEmptyGPOPolicyStore() error {
+	k, err := registry.OpenKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot, registry.QUERY_VALUE)
+	switch {
+	case errors.Is(err, registry.ErrNotExist), errors.Is(err, syscall.ERROR_PATH_NOT_FOUND):
+		return nil
+	case err != nil:
+		return fmt.Errorf("open HKEY_LOCAL_MACHINE\\%s: %w", GPODNSPolicyConfigRoot, err)
+	}
+
+	info, err := k.Stat()
+	closer(k)
+	if err != nil {
+		return fmt.Errorf("stat HKEY_LOCAL_MACHINE\\%s: %w", GPODNSPolicyConfigRoot, err)
+	}
+
+	if info.SubKeyCount != 0 || info.ValueCount != 0 {
+		return nil
+	}
+
+	if err := registry.DeleteKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot); err != nil {
+		return fmt.Errorf("delete empty HKEY_LOCAL_MACHINE\\%s: %w", GPODNSPolicyConfigRoot, err)
+	}
+
+	log.Infof("removed the empty GPO DNS policy store, leaving the local one authoritative")
+	return nil
+}
+
+// listNRPTRuleKeys returns the names of our NRPT rule keys under a policy store
+// root. An absent root holds nothing to clean up, which is the normal state of
+// the GPO store on a machine without DNS Client policy.
+func listNRPTRuleKeys(root string) ([]string, error) {
+	k, err := registry.OpenKey(registry.LOCAL_MACHINE, root, registry.ENUMERATE_SUB_KEYS)
+	switch {
+	case errors.Is(err, registry.ErrNotExist), errors.Is(err, syscall.ERROR_PATH_NOT_FOUND):
+		// the GPO store is absent on a machine without DNS client policy
+		log.Debugf("HKEY_LOCAL_MACHINE\\%s does not exist", root)
+		return nil, nil
+	case err != nil:
+		// any other failure has to reach the caller: reporting no rules would
+		// report a successful cleanup while leaving the rules in place
+		return nil, fmt.Errorf("open HKEY_LOCAL_MACHINE\\%s: %w", root, err)
+	}
+	defer closer(k)
+
+	names, err := k.ReadSubKeyNames(-1)
+	if err != nil {
+		return nil, fmt.Errorf("read subkey names: %w", err)
+	}
+
+	var ruleKeys []string
+	for _, name := range names {
+		// registry key names are case insensitive
+		if strings.HasPrefix(strings.ToLower(name), strings.ToLower(NRPTKeyPrefix)) {
+			ruleKeys = append(ruleKeys, name)
+		}
+	}
+
+	return ruleKeys, nil
+}
+
 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)
@@ -585,7 +793,7 @@ func refreshGroupPolicy() error {
 	)
 
 	if ret == 0 {
-		if err != nil && !errors.Is(err, syscall.Errno(0)) {
+		if !errors.Is(err, syscall.Errno(0)) {
 			return fmt.Errorf("RefreshPolicyEx failed: %w", err)
 		}
 		return fmt.Errorf("RefreshPolicyEx failed")
@@ -594,6 +802,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

@@ -8,6 +8,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/sys/windows/registry"
+
+	"github.com/netbirdio/netbird/client/internal/winregistry"
 )
 
 // TestNRPTEntriesCleanupOnConfigChange tests that old NRPT entries are properly cleaned up
@@ -25,7 +27,7 @@ func TestNRPTEntriesCleanupOnConfigChange(t *testing.T) {
 
 	// Create a test interface registry key so updateSearchDomains doesn't fail
 	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()
@@ -56,7 +58,7 @@ func TestNRPTEntriesCleanupOnConfigChange(t *testing.T) {
 	require.NoError(t, err)
 
 	// Verify 3 NRPT rules exist
-	assert.Equal(t, 3, cfg.nrptEntryCount, "Should create 3 NRPT rules for 125 domains")
+	assert.Equal(t, 3, countNRPTRuleKeys(t), "Should create 3 NRPT rules for 125 domains")
 	for i := 0; i < 3; i++ {
 		exists, err := registryKeyExists(fmt.Sprintf("%s-%d", dnsPolicyConfigMatchPath, i))
 		require.NoError(t, err)
@@ -81,7 +83,7 @@ func TestNRPTEntriesCleanupOnConfigChange(t *testing.T) {
 	require.NoError(t, err)
 
 	// Verify first 2 NRPT rules exist
-	assert.Equal(t, 2, cfg.nrptEntryCount, "Should create 2 NRPT rules for 75 domains")
+	assert.Equal(t, 2, countNRPTRuleKeys(t), "Should create 2 NRPT rules for 75 domains")
 	for i := 0; i < 2; i++ {
 		exists, err := registryKeyExists(fmt.Sprintf("%s-%d", dnsPolicyConfigMatchPath, i))
 		require.NoError(t, err)
@@ -94,6 +96,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 {
@@ -106,9 +247,65 @@ func registryKeyExists(path string) (bool, error) {
 	return true, nil
 }
 
+// TestNRPTCleanupWithoutRuleCount verifies that rules written by a previous run
+// are removed by a configurator that has no record of how many there are: an
+// unclean exit loses the in-memory count and a clean disconnect deletes the
+// persisted one, so cleanup cannot depend on either.
+func TestNRPTCleanupWithoutRuleCount(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")
+
+	// 75 domains produce two indexed rules, as the current layout does
+	domains := make([]string, 75)
+	for i := range domains {
+		domains[i] = fmt.Sprintf(".domain%d.com", i+1)
+	}
+
+	previousRun := &registryConfigurator{}
+	require.NoError(t, previousRun.addDNSMatchPolicy(domains, testIP))
+
+	// the unsuffixed key an older version would have written
+	require.NoError(t, previousRun.configureDNSPolicy(dnsPolicyConfigMatchPath, []string{".legacy.example.com"}, testIP))
+
+	// a policy owned by someone else, which cleanup must not touch
+	foreignPath := DNSPolicyConfigRoot + `\DnsPolicyConfigTestForeign`
+	foreignKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, foreignPath, registry.SET_VALUE)
+	require.NoError(t, err, "Should create foreign policy key")
+	foreignKey.Close()
+	defer func() {
+		_ = registry.DeleteKey(registry.LOCAL_MACHINE, foreignPath)
+	}()
+
+	require.Equal(t, 3, countNRPTRuleKeys(t), "Should have two indexed rules and the legacy one")
+
+	// a configurator that never applied a DNS config, as one built after a
+	// restart or from a shutdown state without a count is
+	freshRun := &registryConfigurator{}
+	require.NoError(t, freshRun.removeDNSMatchPolicies())
+
+	assert.Equal(t, 0, countNRPTRuleKeys(t), "Should remove every rule left by the previous run")
+
+	exists, err := registryKeyExists(foreignPath)
+	require.NoError(t, err)
+	assert.True(t, exists, "Should not remove a policy that is not ours")
+}
+
+func countNRPTRuleKeys(t *testing.T) int {
+	t.Helper()
+
+	names, err := listNRPTRuleKeys(DNSPolicyConfigRoot)
+	require.NoError(t, err, "Should list NRPT rule keys")
+	return len(names)
+}
+
 func cleanupRegistryKeys(*testing.T) {
-	// Clean up more entries to account for batching tests with many domains
-	cfg := &registryConfigurator{nrptEntryCount: 20}
+	cfg := &registryConfigurator{}
 	_ = cfg.removeDNSMatchPolicies()
 }
 
@@ -125,7 +322,7 @@ func TestNRPTDomainBatching(t *testing.T) {
 
 	// Create a test interface registry key so updateSearchDomains doesn't fail
 	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()
@@ -193,7 +390,7 @@ func TestNRPTDomainBatching(t *testing.T) {
 			require.NoError(t, err)
 
 			// Verify that exactly expectedRuleCount rules were created
-			assert.Equal(t, tc.expectedRuleCount, cfg.nrptEntryCount,
+			assert.Equal(t, tc.expectedRuleCount, countNRPTRuleKeys(t),
 				"Should create %d NRPT rules for %d domains", tc.expectedRuleCount, tc.domainCount)
 
 			// Verify all expected rules exist
@@ -210,3 +407,130 @@ func TestNRPTDomainBatching(t *testing.T) {
 		})
 	}
 }
+
+// TestRemoveEmptyGPOPolicyStore verifies that cleanup takes the GPO policy
+// store itself with it once our rules are gone, since the store existing keeps
+// the local one from being applied, and that a store with somebody else's rule
+// in it is left alone.
+func TestRemoveEmptyGPOPolicyStore(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping registry integration test in short mode")
+	}
+
+	t.Cleanup(func() { cleanupRegistryKeys(t) })
+	cleanupRegistryKeys(t)
+
+	testIP := netip.MustParseAddr("100.64.0.1")
+	cfg := &registryConfigurator{gpo: true}
+
+	// a store holding a rule of ours is kept, because the rule is still applied
+	require.NoError(t, cfg.addDNSMatchPolicy([]string{".example.com"}, testIP))
+	exists, err := registryKeyExists(gpoDnsPolicyConfigMatchPath + "-0")
+	require.NoError(t, err)
+	require.True(t, exists, "Should write the rule to the GPO policy store")
+
+	require.NoError(t, removeEmptyGPOPolicyStore())
+	exists, err = registryKeyExists(GPODNSPolicyConfigRoot)
+	require.NoError(t, err)
+	assert.True(t, exists, "Should keep a policy store that still holds a rule")
+
+	// once the rules are gone the store goes with them
+	require.NoError(t, cfg.removeDNSMatchPolicies())
+	require.NoError(t, removeEmptyGPOPolicyStore())
+
+	exists, err = registryKeyExists(GPODNSPolicyConfigRoot)
+	require.NoError(t, err)
+	assert.False(t, exists, "Should remove the GPO policy store once it is empty")
+
+	// A store is not ours to remove while somebody else has a rule in it. The
+	// rule is written volatile like our own: the rules above created the parent
+	// chain volatile, and Windows refuses a stable subkey under a volatile
+	// parent.
+	foreignRule := GPODNSPolicyConfigRoot + `\{2A3B4C5D-6E7F-4041-8283-84858687888A}`
+	foreignKey, _, err := winregistry.CreateVolatileKey(registry.LOCAL_MACHINE, foreignRule, registry.SET_VALUE)
+	require.NoError(t, err, "Should create a foreign GPO rule")
+	foreignKey.Close()
+	t.Cleanup(func() {
+		_ = registry.DeleteKey(registry.LOCAL_MACHINE, foreignRule)
+		_ = registry.DeleteKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot)
+	})
+
+	require.NoError(t, cfg.removeDNSMatchPolicies())
+	require.NoError(t, removeEmptyGPOPolicyStore())
+
+	exists, err = registryKeyExists(foreignRule)
+	require.NoError(t, err)
+	assert.True(t, exists, "Should not remove a foreign rule")
+	exists, err = registryKeyExists(GPODNSPolicyConfigRoot)
+	require.NoError(t, err)
+	assert.True(t, exists, "Should keep a policy store that still holds a foreign rule")
+}
+
+// TestDeleteInterfaceRegistryKeyPropertyTwice verifies that removing a value
+// that is already gone, or one on an interface key that is, reports success.
+// Teardown runs again after a failed cleanup, and the steps that follow this
+// one have to be reached on that second run.
+func TestDeleteInterfaceRegistryKeyPropertyTwice(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping registry integration test in short mode")
+	}
+
+	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")
+	testKey.Close()
+	t.Cleanup(func() {
+		_ = registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath)
+	})
+
+	cfg := &registryConfigurator{guid: testGUID}
+
+	require.NoError(t, cfg.setInterfaceRegistryKeyStringValue(interfaceConfigSearchListKey, "example.com"))
+	require.NoError(t, cfg.deleteInterfaceRegistryKeyProperty(interfaceConfigSearchListKey))
+	assert.NoError(t, cfg.deleteInterfaceRegistryKeyProperty(interfaceConfigSearchListKey),
+		"Should report success for a value that is already gone")
+
+	// and with the interface key itself gone, as it is once the adapter is
+	require.NoError(t, registry.DeleteKey(registry.LOCAL_MACHINE, interfacePath))
+	assert.NoError(t, cfg.deleteInterfaceRegistryKeyProperty(interfaceConfigSearchListKey),
+		"Should report success when the interface key does not exist")
+}
+
+// TestUseGPOPolicyStoreClearsEmptyStore verifies that the store is cleared
+// before it is consulted, so an empty one left by an earlier run does not send
+// this run's rules to the group policy store. A store somebody else has a rule
+// in still decides where the rules go.
+func TestUseGPOPolicyStoreClearsEmptyStore(t *testing.T) {
+	if testing.Short() {
+		t.Skip("skipping registry integration test in short mode")
+	}
+
+	t.Cleanup(func() { cleanupRegistryKeys(t) })
+	cleanupRegistryKeys(t)
+
+	// the leftover an earlier run used to keep, which the client read as
+	// "group policy configures the NRPT" for every run after it
+	emptyStore, _, err := winregistry.CreateVolatileKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot, registry.SET_VALUE)
+	require.NoError(t, err, "Should create the GPO policy store")
+	emptyStore.Close()
+
+	assert.False(t, useGPOPolicyStore(), "An empty store should not decide where the rules go")
+	exists, err := registryKeyExists(GPODNSPolicyConfigRoot)
+	require.NoError(t, err)
+	assert.False(t, exists, "Should clear the empty store before consulting it")
+
+	foreignRule := GPODNSPolicyConfigRoot + `\{2A3B4C5D-6E7F-4041-8283-84858687888A}`
+	foreignKey, _, err := winregistry.CreateVolatileKey(registry.LOCAL_MACHINE, foreignRule, registry.SET_VALUE)
+	require.NoError(t, err, "Should create a foreign GPO rule")
+	foreignKey.Close()
+	t.Cleanup(func() {
+		_ = registry.DeleteKey(registry.LOCAL_MACHINE, foreignRule)
+		_ = registry.DeleteKey(registry.LOCAL_MACHINE, GPODNSPolicyConfigRoot)
+	})
+
+	assert.True(t, useGPOPolicyStore(), "A store holding a rule should decide where the rules go")
+	exists, err = registryKeyExists(GPODNSPolicyConfigRoot)
+	require.NoError(t, err)
+	assert.True(t, exists, "Should keep a store that holds a rule")
+}

client/internal/dns/interface_index.go

@@ -0,0 +1,15 @@
+package dns
+
+import (
+	"fmt"
+	"net"
+)
+
+func getInterfaceIndex(interfaceName string) (int, error) {
+	iface, err := net.InterfaceByName(interfaceName)
+	if err != nil {
+		return 0, fmt.Errorf("lookup interface %q: %w", interfaceName, err)
+	}
+
+	return iface.Index, nil
+}

client/internal/dns/interface_index_test.go

@@ -0,0 +1,35 @@
+package dns
+
+import (
+	"net"
+	"testing"
+)
+
+func TestGetInterfaceIndexExisting(t *testing.T) {
+	interfaces, err := net.Interfaces()
+	if err != nil {
+		t.Fatalf("list network interfaces: %v", err)
+	}
+	if len(interfaces) == 0 {
+		t.Fatal("expected at least one network interface")
+	}
+
+	iface := interfaces[0]
+	index, err := getInterfaceIndex(iface.Name)
+	if err != nil {
+		t.Fatalf("look up existing interface %q: %v", iface.Name, err)
+	}
+	if index != iface.Index {
+		t.Fatalf("expected interface index %d, got %d", iface.Index, index)
+	}
+}
+
+func TestGetInterfaceIndexMissing(t *testing.T) {
+	index, err := getInterfaceIndex("netbird-interface-that-does-not-exist")
+	if index != 0 {
+		t.Fatalf("expected missing interface index to be 0, got %d", index)
+	}
+	if err == nil {
+		t.Fatal("expected missing interface lookup to return an error")
+	}
+}

client/internal/dns/mgmt/mgmt_refresh_test.go

@@ -224,6 +224,7 @@ func TestResolver_StaleTriggersAsyncRefresh(t *testing.T) {
 }
 
 func TestResolver_ConcurrentStaleHitsCollapseRefresh(t *testing.T) {
+	semaphore := make(chan struct{})
 	r := NewResolver()
 	chain := newFakeChain()
 	chain.setAnswer("mgmt.example.com.", dns.TypeA, "10.0.0.2")
@@ -239,7 +240,7 @@ func TestResolver_ConcurrentStaleHitsCollapseRefresh(t *testing.T) {
 				break
 			}
 		}
-		time.Sleep(50 * time.Millisecond) // hold inflight long enough to collide
+		<-semaphore // block the call to force request collision
 	}
 
 	r.SetChainResolver(chain, 50)
@@ -255,17 +256,17 @@ func TestResolver_ConcurrentStaleHitsCollapseRefresh(t *testing.T) {
 
 	var wg sync.WaitGroup
 	for i := 0; i < 50; i++ {
-		wg.Add(1)
-		go func() {
-			defer wg.Done()
+		wg.Go(func() {
 			queryA(t, r, "mgmt.example.com.")
-		}()
+		})
 	}
+
+	assert.Eventually(t, func() bool { return inflight.Load() >= 1 }, 2*time.Second, 100*time.Millisecond)
+
+	close(semaphore)
 	wg.Wait()
 
-	waitFor(t, 2*time.Second, func() bool {
-		return inflight.Load() == 0
-	})
+	assert.Eventually(t, func() bool { return inflight.Load() == 0 }, 2*time.Second, 100*time.Millisecond)
 
 	calls := chain.callCount("mgmt.example.com.", dns.TypeA)
 	assert.LessOrEqual(t, calls, 2, "singleflight must collapse concurrent refreshes (got %d)", calls)

client/internal/dns/notifier.go

@@ -51,7 +51,5 @@ func (n *notifier) notify() {
 		return
 	}
 
-	go func(l listener.NetworkChangeListener) {
-		l.OnNetworkChanged("")
-	}(n.listener)
+	n.listener.OnNetworkChanged("")
 }

client/internal/dns/response_writer_test.go

@@ -4,7 +4,7 @@ import (
 	"net"
 	"testing"
 
-	"github.com/golang/mock/gomock"
+	"go.uber.org/mock/gomock"
 	"github.com/google/gopacket"
 	"github.com/google/gopacket/layers"
 	"github.com/miekg/dns"

client/internal/dns/server.go

@@ -252,7 +252,7 @@ func NewDefaultServerPermanentUpstream(
 	ds.hostsDNSHolder.set(hostsDnsList)
 	ds.permanent = true
 	ds.currentConfig = dnsConfigToHostDNSConfig(config, ds.service.RuntimeIP(), ds.service.RuntimePort())
-	ds.searchDomainNotifier = newNotifier(ds.SearchDomains())
+	ds.searchDomainNotifier = newNotifier(ds.searchDomains())
 	ds.searchDomainNotifier.setListener(listener)
 	setServerDns(ds)
 	return ds
@@ -602,6 +602,12 @@ func (s *DefaultServer) UpdateDNSServer(serial uint64, update nbdns.Config) erro
 }
 
 func (s *DefaultServer) SearchDomains() []string {
+	s.mux.Lock()
+	defer s.mux.Unlock()
+	return s.searchDomains()
+}
+
+func (s *DefaultServer) searchDomains() []string {
 	var searchDomains []string
 
 	for _, dConf := range s.currentConfig.Domains {
@@ -686,7 +692,7 @@ func (s *DefaultServer) applyConfiguration(update nbdns.Config) error {
 	}()
 
 	if s.searchDomainNotifier != nil {
-		s.searchDomainNotifier.onNewSearchDomains(s.SearchDomains())
+		s.searchDomainNotifier.onNewSearchDomains(s.searchDomains())
 	}
 
 	s.updateNSGroupStates(update.NameServerGroups)

client/internal/dns/server_privileged_test.go

@@ -9,7 +9,7 @@ import (
 	"os"
 	"testing"
 
-	"github.com/golang/mock/gomock"
+	"go.uber.org/mock/gomock"
 	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
 	"golang.zx2c4.com/wireguard/wgctrl/wgtypes"

client/internal/dns/server_test.go

@@ -423,7 +423,7 @@ func createWgInterfaceWithBind(t *testing.T) (*iface.WGIface, error) {
 		return nil, err
 	}
 
-	pf, err := uspfilter.Create(wgIface, false, flowLogger, iface.DefaultMTU)
+	pf, err := uspfilter.Create(uspfilter.Config{IFace: wgIface, FlowLogger: flowLogger, MTU: iface.DefaultMTU})
 	if err != nil {
 		t.Fatalf("failed to create uspfilter: %v", err)
 		return nil, err

client/internal/dns/service_listener.go

@@ -6,6 +6,7 @@ import (
 	"net"
 	"net/netip"
 	"runtime"
+	"slices"
 	"strconv"
 	"sync"
 	"time"
@@ -17,17 +18,20 @@ import (
 	nberrors "github.com/netbirdio/netbird/client/errors"
 
 	firewall "github.com/netbirdio/netbird/client/firewall/manager"
-	"github.com/netbirdio/netbird/client/internal/ebpf"
-	ebpfMgr "github.com/netbirdio/netbird/client/internal/ebpf/manager"
 )
 
 const (
 	customPort = 5053
+	// randomPortAttempts bounds the search for a port free on both protocols.
+	randomPortAttempts = 5
 )
 
 var (
 	defaultIP = netip.MustParseAddr("127.0.0.1")
 	customIP  = netip.MustParseAddr("127.0.0.153")
+
+	// dnatProtocols are the protocols the port 53 redirect covers.
+	dnatProtocols = []firewall.Protocol{firewall.ProtocolUDP, firewall.ProtocolTCP}
 )
 
 type serviceViaListener struct {
@@ -40,9 +44,20 @@ type serviceViaListener struct {
 	listenPort        uint16
 	listenerIsRunning bool
 	listenerFlagLock  sync.Mutex
-	ebpfService       ebpfMgr.Manager
 	firewall          Firewall
-	tcpDNATConfigured bool
+	// dnatRules holds the port 53 redirects that are installed and not yet
+	// removed, so a removal that fails can be retried.
+	dnatRules []dnatRule
+}
+
+// dnatRule is a port 53 redirect as it was installed. The target is kept with
+// the rule because the listener can come back on a different address or port,
+// and a retried removal has to name the address and port the rule was added
+// with, not the ones in use now.
+type dnatRule struct {
+	protocol firewall.Protocol
+	ip       netip.Addr
+	port     uint16
 }
 
 func newServiceViaListener(wgIface WGIface, customAddr *netip.AddrPort, fw Firewall) *serviceViaListener {
@@ -112,34 +127,93 @@ func (s *serviceViaListener) Listen() error {
 		}
 	}()
 
-	// When eBPF redirects UDP port 53 to our listen port, TCP still needs
-	// a DNAT rule because eBPF only handles UDP.
-	if s.ebpfService != nil && s.firewall != nil && s.listenPort != DefaultPort {
-		if err := s.firewall.AddOutputDNAT(s.listenIP, firewall.ProtocolTCP, DefaultPort, s.listenPort); err != nil {
-			log.Warnf("failed to add DNS TCP DNAT rule, TCP DNS on port 53 will not work: %v", err)
-		} else {
-			s.tcpDNATConfigured = true
-			log.Infof("added DNS TCP DNAT rule: %s:%d -> %s:%d", s.listenIP, DefaultPort, s.listenIP, s.listenPort)
-		}
+	if s.listenPort != DefaultPort {
+		s.setupDNAT()
 	}
 
 	return nil
 }
 
+// setupDNAT redirects port 53 to the port the DNS server actually listens on.
+// Both protocols must be redirected or none: RuntimePort reports port 53 only
+// while the full redirect is in place, so a half-configured redirect would
+// advertise a resolver that answers over one protocol.
+func (s *serviceViaListener) setupDNAT() {
+	if s.firewall == nil {
+		log.Errorf("no firewall manager available to redirect DNS port %d to %d, "+
+			"clients pointed at %s will not reach the resolver", DefaultPort, s.listenPort, s.listenIP)
+		return
+	}
+
+	// Clear whatever an earlier removal left behind first. Those rules can point
+	// at an address or port this listener no longer uses, and they are matched
+	// before anything added now, so adding a redirect on top of one would keep
+	// sending port 53 traffic to the previous listener while reporting the
+	// redirect as complete. The rules stay recorded for a later attempt.
+	if err := s.removeDNAT(); err != nil {
+		log.Errorf("failed to remove stale DNS DNAT rules, leaving port %d redirected to the previous listener: %v",
+			DefaultPort, err)
+		return
+	}
+
+	for _, proto := range dnatProtocols {
+		if err := s.firewall.AddOutputDNAT(s.listenIP, proto, DefaultPort, s.listenPort); err != nil {
+			log.Errorf("failed to add DNS %s DNAT rule, DNS on port %d will not work: %v",
+				proto, DefaultPort, err)
+			if err := s.removeDNAT(); err != nil {
+				log.Warnf("failed to roll back DNS DNAT rules, retrying on stop: %v", err)
+			}
+			return
+		}
+		s.dnatRules = append(s.dnatRules, dnatRule{protocol: proto, ip: s.listenIP, port: s.listenPort})
+	}
+
+	log.Infof("added DNS DNAT rules: %s:%d -> %s:%d (UDP + TCP)", s.listenIP, DefaultPort, s.listenIP, s.listenPort)
+}
+
+// removeDNAT removes every installed port 53 redirect. A rule whose removal
+// fails stays recorded so a later setup or Stop retries it, rather than leaving
+// port 53 pointing at a resolver that is no longer listening.
+func (s *serviceViaListener) removeDNAT() error {
+	if s.firewall == nil {
+		return nil
+	}
+
+	var merr *multierror.Error
+	var remaining []dnatRule
+	for _, rule := range s.dnatRules {
+		if err := s.firewall.RemoveOutputDNAT(rule.ip, rule.protocol, DefaultPort, rule.port); err != nil {
+			merr = multierror.Append(merr, fmt.Errorf("remove DNS %s DNAT rule for %s:%d: %w",
+				rule.protocol, rule.ip, rule.port, err))
+			remaining = append(remaining, rule)
+		}
+	}
+	s.dnatRules = remaining
+
+	return nberrors.FormatErrorOrNil(merr)
+}
+
 func (s *serviceViaListener) Stop() error {
 	s.listenerFlagLock.Lock()
 	defer s.listenerFlagLock.Unlock()
 
+	var merr *multierror.Error
+
+	// Redirects are removed even when the listener is already stopped, so that
+	// a removal which failed earlier is retried instead of leaving port 53
+	// pointing at a resolver that no longer listens.
+	if err := s.removeDNAT(); err != nil {
+		merr = multierror.Append(merr, err)
+	}
+
 	if !s.listenerIsRunning {
-		return nil
+		return nberrors.FormatErrorOrNil(merr)
 	}
 	s.listenerIsRunning = false
 
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()
 
-	var merr *multierror.Error
-
 	if err := s.server.ShutdownContext(ctx); err != nil {
 		merr = multierror.Append(merr, fmt.Errorf("stop DNS UDP server: %w", err))
 	}
@@ -148,19 +222,6 @@ func (s *serviceViaListener) Stop() error {
 		merr = multierror.Append(merr, fmt.Errorf("stop DNS TCP server: %w", err))
 	}
 
-	if s.tcpDNATConfigured && s.firewall != nil {
-		if err := s.firewall.RemoveOutputDNAT(s.listenIP, firewall.ProtocolTCP, DefaultPort, s.listenPort); err != nil {
-			merr = multierror.Append(merr, fmt.Errorf("remove DNS TCP DNAT rule: %w", err))
-		}
-		s.tcpDNATConfigured = false
-	}
-
-	if s.ebpfService != nil {
-		if err := s.ebpfService.FreeDNSFwd(); err != nil {
-			merr = multierror.Append(merr, fmt.Errorf("stop traffic forwarder: %w", err))
-		}
-	}
-
 	return nberrors.FormatErrorOrNil(merr)
 }
 
@@ -177,11 +238,23 @@ func (s *serviceViaListener) RuntimePort() int {
 	s.listenerFlagLock.Lock()
 	defer s.listenerFlagLock.Unlock()
 
-	if s.ebpfService != nil {
+	if s.redirectInstalled() {
 		return DefaultPort
-	} else {
-		return int(s.listenPort)
 	}
+	return int(s.listenPort)
+}
+
+// redirectInstalled reports whether every protocol is redirected from port 53
+// to the address and port the listener currently serves. Rules left over from
+// an earlier listener do not count.
+func (s *serviceViaListener) redirectInstalled() bool {
+	for _, proto := range dnatProtocols {
+		current := dnatRule{protocol: proto, ip: s.listenIP, port: s.listenPort}
+		if !slices.Contains(s.dnatRules, current) {
+			return false
+		}
+	}
+	return true
 }
 
 func (s *serviceViaListener) RuntimeIP() netip.Addr {
@@ -190,30 +263,29 @@ func (s *serviceViaListener) RuntimeIP() netip.Addr {
 
 // evalListenAddress figures out the listen address for the DNS server.
 // IPv4-only: all peers have a v4 overlay address, and DNS config points to v4.
-// First checks port 53 on WG interface or lo, then tries eBPF on a random port,
-// then falls back to port 5053.
+// Prefers port 53 on the overlay interface or lo, so no redirect is needed at
+// all; when it is taken it falls back to port 5053 and then to a random free
+// port, both of which need the port 53 redirect set up by setupDNAT.
 func (s *serviceViaListener) evalListenAddress() (netip.Addr, uint16, error) {
 	if s.customAddr != nil {
 		return s.customAddr.Addr(), s.customAddr.Port(), nil
 	}
 
-	ip, ok := s.testFreePort(DefaultPort)
-	if ok {
+	if ip, ok := s.testFreePort(DefaultPort); ok {
 		return ip, DefaultPort, nil
 	}
 
-	ebpfSrv, port, ok := s.tryToUseeBPF()
-	if ok {
-		s.ebpfService = ebpfSrv
-		return s.wgInterface.Address().IP, port, nil
-	}
-
-	ip, ok = s.testFreePort(customPort)
-	if ok {
+	if ip, ok := s.testFreePort(customPort); ok {
 		return ip, customPort, nil
 	}
 
-	return netip.Addr{}, 0, fmt.Errorf("failed to find a free port for DNS server")
+	ip := s.wgInterface.Address().IP
+	port, err := s.randomFreePort(ip)
+	if err != nil {
+		return netip.Addr{}, 0, fmt.Errorf("find a free port for DNS server: %w", err)
+	}
+
+	return ip, port, nil
 }
 
 func (s *serviceViaListener) testFreePort(port int) (netip.Addr, bool) {
@@ -260,48 +332,25 @@ func (s *serviceViaListener) tryToBind(ip netip.Addr, port int) bool {
 	return true
 }
 
-// tryToUseeBPF decides whether to apply eBPF program to capture DNS traffic on port 53.
-// This is needed because on some operating systems if we start a DNS server not on a default port 53,
-// the domain name  resolution won't work. So, in case we are running on Linux and picked a free
-// port we should fall back to the eBPF solution that will capture traffic on port 53 and forward
-// it to a local DNS server running on the chosen port.
-func (s *serviceViaListener) tryToUseeBPF() (ebpfMgr.Manager, uint16, bool) {
-	if runtime.GOOS != "linux" {
-		return nil, 0, false
+// randomFreePort returns a port that is free on ip for both UDP and TCP, since
+// the DNS server binds both. The probe listeners are closed again, so the port
+// is only likely, not guaranteed, to still be free when the server binds it.
+func (s *serviceViaListener) randomFreePort(ip netip.Addr) (uint16, error) {
+	for range randomPortAttempts {
+		probeListener, err := net.ListenUDP("udp4", &net.UDPAddr{})
+		if err != nil {
+			return 0, fmt.Errorf("bind random port: %w", err)
+		}
+
+		port := uint16(probeListener.LocalAddr().(*net.UDPAddr).Port)
+		if err := probeListener.Close(); err != nil {
+			return 0, fmt.Errorf("free up probed port: %w", err)
+		}
+
+		if s.tryToBind(ip, int(port)) {
+			return port, nil
+		}
 	}
 
-	port, err := s.generateFreePort() //nolint:staticcheck,unused
-	if err != nil {
-		log.Warnf("failed to generate a free port for eBPF DNS forwarder server: %s", err)
-		return nil, 0, false
-	}
-
-	ebpfSrv := ebpf.GetEbpfManagerInstance()
-	err = ebpfSrv.LoadDNSFwd(s.wgInterface.Address().IP, int(port))
-	if err != nil {
-		log.Warnf("failed to load DNS forwarder eBPF program, error: %s", err)
-		return nil, 0, false
-	}
-
-	return ebpfSrv, port, true
-}
-
-func (s *serviceViaListener) generateFreePort() (uint16, error) {
-	ok := s.tryToBind(s.wgInterface.Address().IP, customPort)
-	if ok {
-		return customPort, nil
-	}
-
-	probeListener, err := net.ListenUDP("udp4", &net.UDPAddr{})
-	if err != nil {
-		log.Debugf("failed to bind random port for DNS: %s", err)
-		return 0, err
-	}
-
-	port := uint16(probeListener.LocalAddr().(*net.UDPAddr).Port)
-	if err = probeListener.Close(); err != nil {
-		log.Debugf("failed to free up DNS port: %s", err)
-		return 0, err
-	}
-	return port, nil
+	return 0, fmt.Errorf("no port free for UDP and TCP on %s after %d attempts", ip, randomPortAttempts)
 }

client/internal/dns/service_listener_test.go

@@ -1,6 +1,7 @@
 package dns
 
 import (
+	"errors"
 	"fmt"
 	"net"
 	"net/netip"
@@ -10,6 +11,8 @@ import (
 	"github.com/miekg/dns"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
+
+	firewall "github.com/netbirdio/netbird/client/firewall/manager"
 )
 
 func TestServiceViaListener_TCPAndUDP(t *testing.T) {
@@ -84,3 +87,133 @@ func TestServiceViaListener_TCPAndUDP(t *testing.T) {
 	require.NotEmpty(t, tcpResp.Answer)
 	assert.Contains(t, tcpResp.Answer[0].String(), "192.0.2.1", "TCP response should contain expected IP")
 }
+
+type dnatCall struct {
+	rule  dnatRule
+	added bool
+}
+
+// fakeFirewall records DNAT calls and fails the ones named in addErrs/removeErrs.
+type fakeFirewall struct {
+	calls      []dnatCall
+	addErrs    map[firewall.Protocol]error
+	removeErrs map[firewall.Protocol]error
+}
+
+func (f *fakeFirewall) AddOutputDNAT(ip netip.Addr, protocol firewall.Protocol, _, translatedPort uint16) error {
+	if err := f.addErrs[protocol]; err != nil {
+		return err
+	}
+	f.calls = append(f.calls, dnatCall{rule: dnatRule{protocol: protocol, ip: ip, port: translatedPort}, added: true})
+	return nil
+}
+
+func (f *fakeFirewall) RemoveOutputDNAT(ip netip.Addr, protocol firewall.Protocol, _, translatedPort uint16) error {
+	if err := f.removeErrs[protocol]; err != nil {
+		return err
+	}
+	f.calls = append(f.calls, dnatCall{rule: dnatRule{protocol: protocol, ip: ip, port: translatedPort}})
+	return nil
+}
+
+func newDNATTestService(fw Firewall) *serviceViaListener {
+	return &serviceViaListener{
+		listenIP:   netip.MustParseAddr("100.64.0.1"),
+		listenPort: customPort,
+		firewall:   fw,
+	}
+}
+
+func TestSetupDNAT_BothProtocols(t *testing.T) {
+	svc := newDNATTestService(&fakeFirewall{})
+
+	svc.setupDNAT()
+
+	assert.Len(t, svc.dnatRules, len(dnatProtocols))
+	assert.Equal(t, DefaultPort, svc.RuntimePort(), "port 53 is advertised once both redirects are installed")
+}
+
+func TestSetupDNAT_RollsBackPartialRedirect(t *testing.T) {
+	fw := &fakeFirewall{addErrs: map[firewall.Protocol]error{firewall.ProtocolTCP: errors.New("nftables busy")}}
+	svc := newDNATTestService(fw)
+
+	svc.setupDNAT()
+
+	assert.Empty(t, svc.dnatRules, "the UDP redirect installed before the failure must be rolled back")
+	assert.Equal(t, int(svc.listenPort), svc.RuntimePort(), "an incomplete redirect must not advertise port 53")
+	udp := dnatRule{protocol: firewall.ProtocolUDP, ip: svc.listenIP, port: svc.listenPort}
+	assert.Contains(t, fw.calls, dnatCall{rule: udp}, "UDP removal should have been attempted")
+}
+
+// A rollback that fails must keep the rule recorded, so port 53 is not left
+// redirected to a resolver that no longer listens.
+func TestStop_RetriesFailedDNATRemoval(t *testing.T) {
+	fw := &fakeFirewall{
+		addErrs:    map[firewall.Protocol]error{firewall.ProtocolTCP: errors.New("nftables busy")},
+		removeErrs: map[firewall.Protocol]error{firewall.ProtocolUDP: errors.New("nftables busy")},
+	}
+	svc := newDNATTestService(fw)
+
+	svc.setupDNAT()
+	udp := dnatRule{protocol: firewall.ProtocolUDP, ip: svc.listenIP, port: svc.listenPort}
+	require.Equal(t, []dnatRule{udp}, svc.dnatRules, "a failed rollback keeps the rule for a later retry")
+
+	require.Error(t, svc.Stop(), "the failing removal should be reported")
+	require.Equal(t, []dnatRule{udp}, svc.dnatRules)
+
+	delete(fw.removeErrs, firewall.ProtocolUDP)
+	require.NoError(t, svc.Stop(), "a later stop retries the removal")
+	assert.Empty(t, svc.dnatRules)
+}
+
+// A stale rule that cannot be removed is matched before anything added now, so
+// no new redirect may be installed on top of it and port 53 must not be
+// advertised as reaching this listener.
+func TestSetupDNAT_AbortsWhileStaleRuleRemains(t *testing.T) {
+	fw := &fakeFirewall{removeErrs: map[firewall.Protocol]error{firewall.ProtocolUDP: errors.New("nftables busy")}}
+	svc := newDNATTestService(fw)
+	stalePort := svc.listenPort
+
+	svc.setupDNAT()
+	require.Error(t, svc.Stop())
+	staleUDP := dnatRule{protocol: firewall.ProtocolUDP, ip: svc.listenIP, port: stalePort}
+	require.Equal(t, []dnatRule{staleUDP}, svc.dnatRules)
+
+	svc.listenPort = stalePort + 1
+	fw.calls = nil
+
+	svc.setupDNAT()
+
+	assert.Equal(t, []dnatRule{staleUDP}, svc.dnatRules, "the stale rule stays recorded for a later attempt")
+	for _, call := range fw.calls {
+		assert.False(t, call.added, "no redirect may be installed while a stale one is still in place")
+	}
+	assert.Equal(t, int(svc.listenPort), svc.RuntimePort(), "port 53 must not be advertised")
+}
+
+// A rule left behind by a failed removal must be removed with the address and
+// port it was installed with, even when the listener has since moved to another
+// port, and it must not count towards the redirect the new listener advertises.
+func TestSetupDNAT_ClearsStaleRuleAfterPortChange(t *testing.T) {
+	fw := &fakeFirewall{removeErrs: map[firewall.Protocol]error{firewall.ProtocolUDP: errors.New("nftables busy")}}
+	svc := newDNATTestService(fw)
+	stalePort := svc.listenPort
+
+	svc.setupDNAT()
+	require.Error(t, svc.Stop())
+	staleUDP := dnatRule{protocol: firewall.ProtocolUDP, ip: svc.listenIP, port: stalePort}
+	require.Equal(t, []dnatRule{staleUDP}, svc.dnatRules)
+
+	delete(fw.removeErrs, firewall.ProtocolUDP)
+	svc.listenPort = stalePort + 1
+	fw.calls = nil
+
+	svc.setupDNAT()
+
+	assert.Contains(t, fw.calls, dnatCall{rule: staleUDP}, "the stale rule must be removed with its original port")
+	assert.Len(t, svc.dnatRules, len(dnatProtocols))
+	assert.Equal(t, DefaultPort, svc.RuntimePort(), "the new listener is fully redirected")
+	for _, rule := range svc.dnatRules {
+		assert.Equal(t, svc.listenPort, rule.port, "only rules for the current listener remain")
+	}
+}

client/internal/dns/unclean_shutdown_windows.go

@@ -5,9 +5,8 @@ import (
 )
 
 type ShutdownState struct {
-	Guid           string
-	GPO            bool
-	NRPTEntryCount int
+	Guid string
+	GPO  bool
 }
 
 func (s *ShutdownState) Name() string {
@@ -16,9 +15,8 @@ func (s *ShutdownState) Name() string {
 
 func (s *ShutdownState) Cleanup() error {
 	manager := &registryConfigurator{
-		guid:           s.Guid,
-		gpo:            s.GPO,
-		nrptEntryCount: s.NRPTEntryCount,
+		guid: s.Guid,
+		gpo:  s.GPO,
 	}
 
 	if err := manager.restoreUncleanShutdownDNS(); err != nil {

client/internal/dns/upstream_ios.go

@@ -130,8 +130,3 @@ func GetClientPrivate(iface privateClientIface, upstreamIP netip.Addr, dialTimeo
 	}
 	return client, nil
 }
-
-func getInterfaceIndex(interfaceName string) (int, error) {
-	iface, err := net.InterfaceByName(interfaceName)
-	return iface.Index, err
-}