From c4a6dafd272ca29364d5da342e333cead23c7888 Mon Sep 17 00:00:00 2001 From: Viktor Liu <17948409+lixmal@users.noreply.github.com> Date: Thu, 13 Feb 2025 18:17:18 +0100 Subject: [PATCH] [client] Use GPO DNS Policy Config to configure DNS if present (#3319) --- client/internal/dns/host_windows.go | 250 +++++++++++------- .../internal/dns/unclean_shutdown_windows.go | 7 +- 2 files changed, 155 insertions(+), 102 deletions(-) diff --git a/client/internal/dns/host_windows.go b/client/internal/dns/host_windows.go index 7ecca8a41..0cd078472 100644 --- a/client/internal/dns/host_windows.go +++ b/client/internal/dns/host_windows.go @@ -1,35 +1,51 @@ package dns import ( + "errors" "fmt" "io" "strings" + "syscall" + "github.com/hashicorp/go-multierror" log "github.com/sirupsen/logrus" "golang.org/x/sys/windows/registry" + nberrors "github.com/netbirdio/netbird/client/errors" "github.com/netbirdio/netbird/client/internal/statemanager" ) +var ( + userenv = syscall.NewLazyDLL("userenv.dll") + + // https://learn.microsoft.com/en-us/windows/win32/api/userenv/nf-userenv-refreshpolicyex + refreshPolicyExFn = userenv.NewProc("RefreshPolicyEx") +) + const ( - dnsPolicyConfigMatchPath = `SYSTEM\CurrentControlSet\Services\Dnscache\Parameters\DnsPolicyConfig\NetBird-Match` + dnsPolicyConfigMatchPath = `SYSTEM\CurrentControlSet\Services\Dnscache\Parameters\DnsPolicyConfig\NetBird-Match` + gpoDnsPolicyRoot = `SOFTWARE\Policies\Microsoft\Windows NT\DNSClient` + gpoDnsPolicyConfigMatchPath = gpoDnsPolicyRoot + `\DnsPolicyConfig\NetBird-Match` + dnsPolicyConfigVersionKey = "Version" dnsPolicyConfigVersionValue = 2 dnsPolicyConfigNameKey = "Name" dnsPolicyConfigGenericDNSServersKey = "GenericDNSServers" dnsPolicyConfigConfigOptionsKey = "ConfigOptions" dnsPolicyConfigConfigOptionsValue = 0x8 -) -const ( interfaceConfigPath = `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces` interfaceConfigNameServerKey = "NameServer" interfaceConfigSearchListKey = "SearchList" + + // RP_FORCE: Reapply all policies even if no policy change was detected + rpForce = 0x1 ) type registryConfigurator struct { guid string routingAll bool + gpo bool } func newHostManager(wgInterface WGIface) (*registryConfigurator, error) { @@ -37,12 +53,20 @@ func newHostManager(wgInterface WGIface) (*registryConfigurator, error) { if err != nil { return nil, err } - return newHostManagerWithGuid(guid) -} -func newHostManagerWithGuid(guid string) (*registryConfigurator, error) { + 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") + } + return ®istryConfigurator{ guid: guid, + gpo: useGPO, }, nil } @@ -51,30 +75,23 @@ func (r *registryConfigurator) supportCustomPort() bool { } func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager *statemanager.Manager) error { - var err error if config.RouteAll { - err = r.addDNSSetupForAll(config.ServerIP) - if err != nil { + if err := r.addDNSSetupForAll(config.ServerIP); err != nil { return fmt.Errorf("add dns setup: %w", err) } } else if r.routingAll { - err = r.deleteInterfaceRegistryKeyProperty(interfaceConfigNameServerKey) - if err != nil { + if err := r.deleteInterfaceRegistryKeyProperty(interfaceConfigNameServerKey); err != nil { return fmt.Errorf("delete interface registry key property: %w", err) } r.routingAll = false log.Infof("removed %s as main DNS forwarder for this peer", config.ServerIP) } - if err := stateManager.UpdateState(&ShutdownState{Guid: r.guid}); err != nil { + if err := stateManager.UpdateState(&ShutdownState{Guid: r.guid, GPO: r.gpo}); err != nil { log.Errorf("failed to update shutdown state: %s", err) } - var ( - searchDomains []string - matchDomains []string - ) - + var searchDomains, matchDomains []string for _, dConf := range config.Domains { if dConf.Disabled { continue @@ -86,16 +103,16 @@ func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager } if len(matchDomains) != 0 { - err = r.addDNSMatchPolicy(matchDomains, config.ServerIP) + if err := r.addDNSMatchPolicy(matchDomains, config.ServerIP); err != nil { + return fmt.Errorf("add dns match policy: %w", err) + } } else { - err = removeRegistryKeyFromDNSPolicyConfig(dnsPolicyConfigMatchPath) - } - if err != nil { - return fmt.Errorf("add dns match policy: %w", err) + if err := r.removeDNSMatchPolicies(); err != nil { + return fmt.Errorf("remove dns match policies: %w", err) + } } - err = r.updateSearchDomains(searchDomains) - if err != nil { + if err := r.updateSearchDomains(searchDomains); err != nil { return fmt.Errorf("update search domains: %w", err) } @@ -103,9 +120,8 @@ func (r *registryConfigurator) applyDNSConfig(config HostDNSConfig, stateManager } func (r *registryConfigurator) addDNSSetupForAll(ip string) error { - err := r.setInterfaceRegistryKeyStringValue(interfaceConfigNameServerKey, ip) - if err != nil { - return fmt.Errorf("adding dns setup for all failed with error: %w", err) + if err := r.setInterfaceRegistryKeyStringValue(interfaceConfigNameServerKey, ip); err != nil { + return fmt.Errorf("adding dns setup for all failed: %w", err) } r.routingAll = true log.Infof("configured %s:53 as main DNS forwarder for this peer", ip) @@ -113,64 +129,54 @@ func (r *registryConfigurator) addDNSSetupForAll(ip string) error { } func (r *registryConfigurator) addDNSMatchPolicy(domains []string, ip string) error { - _, err := registry.OpenKey(registry.LOCAL_MACHINE, dnsPolicyConfigMatchPath, registry.QUERY_VALUE) - if err == nil { - err = registry.DeleteKey(registry.LOCAL_MACHINE, dnsPolicyConfigMatchPath) - if err != nil { - return fmt.Errorf("unable to remove existing key from registry, key: HKEY_LOCAL_MACHINE\\%s, error: %w", dnsPolicyConfigMatchPath, err) + // 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 + policyPath := dnsPolicyConfigMatchPath + if r.gpo { + policyPath = gpoDnsPolicyConfigMatchPath + } + + if err := removeRegistryKeyFromDNSPolicyConfig(policyPath); err != nil { + return fmt.Errorf("remove existing dns policy: %w", err) + } + + regKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, policyPath, registry.SET_VALUE) + if err != nil { + return fmt.Errorf("create registry key HKEY_LOCAL_MACHINE\\%s: %w", policyPath, err) + } + defer closer(regKey) + + if err := regKey.SetDWordValue(dnsPolicyConfigVersionKey, dnsPolicyConfigVersionValue); err != nil { + return fmt.Errorf("set %s: %w", dnsPolicyConfigVersionKey, err) + } + + if err := regKey.SetStringsValue(dnsPolicyConfigNameKey, domains); err != nil { + return fmt.Errorf("set %s: %w", dnsPolicyConfigNameKey, err) + } + + if err := regKey.SetStringValue(dnsPolicyConfigGenericDNSServersKey, ip); err != nil { + return fmt.Errorf("set %s: %w", dnsPolicyConfigGenericDNSServersKey, err) + } + + if err := regKey.SetDWordValue(dnsPolicyConfigConfigOptionsKey, dnsPolicyConfigConfigOptionsValue); err != nil { + return fmt.Errorf("set %s: %w", dnsPolicyConfigConfigOptionsKey, err) + } + + if r.gpo { + if err := refreshGroupPolicy(); err != nil { + log.Warnf("failed to refresh group policy: %v", err) } } - regKey, _, err := registry.CreateKey(registry.LOCAL_MACHINE, dnsPolicyConfigMatchPath, registry.SET_VALUE) - if err != nil { - return fmt.Errorf("unable to create registry key, key: HKEY_LOCAL_MACHINE\\%s, error: %w", dnsPolicyConfigMatchPath, err) - } - - err = regKey.SetDWordValue(dnsPolicyConfigVersionKey, dnsPolicyConfigVersionValue) - if err != nil { - return fmt.Errorf("unable to set registry value for %s, error: %w", dnsPolicyConfigVersionKey, err) - } - - err = regKey.SetStringsValue(dnsPolicyConfigNameKey, domains) - if err != nil { - return fmt.Errorf("unable to set registry value for %s, error: %w", dnsPolicyConfigNameKey, err) - } - - err = regKey.SetStringValue(dnsPolicyConfigGenericDNSServersKey, ip) - if err != nil { - return fmt.Errorf("unable to set registry value for %s, error: %w", dnsPolicyConfigGenericDNSServersKey, err) - } - - err = regKey.SetDWordValue(dnsPolicyConfigConfigOptionsKey, dnsPolicyConfigConfigOptionsValue) - if err != nil { - return fmt.Errorf("unable to set registry value for %s, error: %w", dnsPolicyConfigConfigOptionsKey, err) - } - - log.Infof("added %d match domains to the state. Domain list: %s", len(domains), domains) - - return nil -} - -func (r *registryConfigurator) restoreHostDNS() error { - if err := removeRegistryKeyFromDNSPolicyConfig(dnsPolicyConfigMatchPath); err != nil { - log.Errorf("remove registry key from dns policy config: %s", err) - } - - if err := r.deleteInterfaceRegistryKeyProperty(interfaceConfigSearchListKey); err != nil { - return fmt.Errorf("remove interface registry key: %w", err) - } - + log.Infof("added %d match domains. Domain list: %s", len(domains), domains) return nil } func (r *registryConfigurator) updateSearchDomains(domains []string) error { - err := r.setInterfaceRegistryKeyStringValue(interfaceConfigSearchListKey, strings.Join(domains, ",")) - if err != nil { - return fmt.Errorf("adding search domain failed with error: %w", err) + if err := r.setInterfaceRegistryKeyStringValue(interfaceConfigSearchListKey, strings.Join(domains, ",")); err != nil { + return fmt.Errorf("update search domains: %w", err) } - - log.Infof("updated the search domains in the registry with %d domains. Domain list: %s", len(domains), domains) - + log.Infof("updated search domains: %s", domains) return nil } @@ -181,11 +187,9 @@ func (r *registryConfigurator) setInterfaceRegistryKeyStringValue(key, value str } defer closer(regKey) - err = regKey.SetStringValue(key, value) - if err != nil { - return fmt.Errorf("applying key %s with value \"%s\" for interface failed with error: %w", key, value, err) + if err := regKey.SetStringValue(key, value); err != nil { + return fmt.Errorf("set key %s=%s: %w", key, value, err) } - return nil } @@ -196,43 +200,91 @@ func (r *registryConfigurator) deleteInterfaceRegistryKeyProperty(propertyKey st } defer closer(regKey) - err = regKey.DeleteValue(propertyKey) - if err != nil { - return fmt.Errorf("deleting registry key %s for interface failed with error: %w", propertyKey, err) + if err := regKey.DeleteValue(propertyKey); err != nil { + return fmt.Errorf("delete registry key %s: %w", propertyKey, err) } - return nil } func (r *registryConfigurator) getInterfaceRegistryKey() (registry.Key, error) { - var regKey registry.Key - regKeyPath := interfaceConfigPath + "\\" + r.guid - regKey, err := registry.OpenKey(registry.LOCAL_MACHINE, regKeyPath, registry.SET_VALUE) if err != nil { - return regKey, fmt.Errorf("unable to open the interface registry key, key: HKEY_LOCAL_MACHINE\\%s, error: %w", regKeyPath, err) + return regKey, fmt.Errorf("open HKEY_LOCAL_MACHINE\\%s: %w", regKeyPath, err) } - return regKey, nil } -func (r *registryConfigurator) restoreUncleanShutdownDNS() error { - if err := r.restoreHostDNS(); err != nil { - return fmt.Errorf("restoring dns via registry: %w", err) +func (r *registryConfigurator) restoreHostDNS() error { + if err := r.removeDNSMatchPolicies(); err != nil { + log.Errorf("remove dns match policies: %s", err) } + + if err := r.deleteInterfaceRegistryKeyProperty(interfaceConfigSearchListKey); err != nil { + return fmt.Errorf("remove interface registry key: %w", err) + } + return nil } +func (r *registryConfigurator) removeDNSMatchPolicies() error { + var merr *multierror.Error + if err := removeRegistryKeyFromDNSPolicyConfig(dnsPolicyConfigMatchPath); err != nil { + merr = multierror.Append(merr, fmt.Errorf("remove local registry key: %w", err)) + } + + if err := removeRegistryKeyFromDNSPolicyConfig(gpoDnsPolicyConfigMatchPath); err != nil { + merr = multierror.Append(merr, fmt.Errorf("remove GPO registry key: %w", err)) + } + + if err := refreshGroupPolicy(); err != nil { + merr = multierror.Append(merr, fmt.Errorf("refresh group policy: %w", err)) + } + + return nberrors.FormatErrorOrNil(merr) +} + +func (r *registryConfigurator) restoreUncleanShutdownDNS() error { + return r.restoreHostDNS() +} + func removeRegistryKeyFromDNSPolicyConfig(regKeyPath string) error { k, err := registry.OpenKey(registry.LOCAL_MACHINE, regKeyPath, registry.QUERY_VALUE) - if err == nil { - defer closer(k) - err = registry.DeleteKey(registry.LOCAL_MACHINE, regKeyPath) - if err != nil { - return fmt.Errorf("unable to remove existing key from registry, key: HKEY_LOCAL_MACHINE\\%s, error: %w", regKeyPath, err) - } + if err != nil { + log.Debugf("failed to open HKEY_LOCAL_MACHINE\\%s: %v", regKeyPath, err) + return nil } + + closer(k) + if err := registry.DeleteKey(registry.LOCAL_MACHINE, regKeyPath); err != nil { + return fmt.Errorf("delete HKEY_LOCAL_MACHINE\\%s: %w", regKeyPath, err) + } + + return nil +} + +func refreshGroupPolicy() error { + // refreshPolicyExFn.Call() panics if the func is not found + defer func() { + if r := recover(); r != nil { + log.Errorf("Recovered from panic: %v", r) + } + }() + + ret, _, err := refreshPolicyExFn.Call( + // bMachine = TRUE (computer policy) + uintptr(1), + // dwOptions = RP_FORCE + uintptr(rpForce), + ) + + if ret == 0 { + if err != nil && !errors.Is(err, syscall.Errno(0)) { + return fmt.Errorf("RefreshPolicyEx failed: %w", err) + } + return fmt.Errorf("RefreshPolicyEx failed") + } + return nil } diff --git a/client/internal/dns/unclean_shutdown_windows.go b/client/internal/dns/unclean_shutdown_windows.go index 74e40cc11..ab0b2cc63 100644 --- a/client/internal/dns/unclean_shutdown_windows.go +++ b/client/internal/dns/unclean_shutdown_windows.go @@ -6,6 +6,7 @@ import ( type ShutdownState struct { Guid string + GPO bool } func (s *ShutdownState) Name() string { @@ -13,9 +14,9 @@ func (s *ShutdownState) Name() string { } func (s *ShutdownState) Cleanup() error { - manager, err := newHostManagerWithGuid(s.Guid) - if err != nil { - return fmt.Errorf("create host manager: %w", err) + manager := ®istryConfigurator{ + guid: s.Guid, + gpo: s.GPO, } if err := manager.restoreUncleanShutdownDNS(); err != nil {