net/dns: change windows DNS manager to use pointer receiver

This is safer given that we need to close the NRPT database.

Signed-off-by: Aaron Klotz <aaron@tailscale.com>

net/dns/manager_windows.go

@@ -42,7 +42,7 @@ type windowsManager struct {
 }
 
 func NewOSConfigurator(logf logger.Logf, interfaceName string) (OSConfigurator, error) {
-	ret := windowsManager{
+	ret := &windowsManager{
 		logf:       logf,
 		guid:       interfaceName,
 		wslManager: newWSLManager(logf),
@@ -62,7 +62,7 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (OSConfigurator,
 	return ret, nil
 }
 
-func (m windowsManager) openInterfaceKey(pfx winutil.RegistryPathPrefix) (registry.Key, error) {
+func (m *windowsManager) openInterfaceKey(pfx winutil.RegistryPathPrefix) (registry.Key, error) {
 	path := pfx.WithSuffix(m.guid)
 	key, err := winutil.OpenKeyWait(registry.LOCAL_MACHINE, path, registry.SET_VALUE)
 	if err != nil {
@@ -83,7 +83,7 @@ func delValue(key registry.Key, name string) error {
 // system's "primary" resolver.
 //
 // If no resolvers are provided, the Tailscale NRPT rules are deleted.
-func (m windowsManager) setSplitDNS(resolvers []netip.Addr, domains []dnsname.FQDN) error {
+func (m *windowsManager) setSplitDNS(resolvers []netip.Addr, domains []dnsname.FQDN) error {
 	if m.nrptDB == nil {
 		if resolvers == nil {
 			// Just a no-op in this case.
@@ -152,7 +152,7 @@ func setTailscaleHosts(prevHostsFile []byte, hosts []*HostEntry) ([]byte, error)
 }
 
 // setHosts sets the hosts file to contain the given host entries.
-func (m windowsManager) setHosts(hosts []*HostEntry) error {
+func (m *windowsManager) setHosts(hosts []*HostEntry) error {
 	systemDir, err := windows.GetSystemDirectory()
 	if err != nil {
 		return err
@@ -176,7 +176,7 @@ func (m windowsManager) setHosts(hosts []*HostEntry) error {
 // "primary" resolvers.
 // domains can be set without resolvers, which just contributes new
 // paths to the global DNS search list.
-func (m windowsManager) setPrimaryDNS(resolvers []netip.Addr, domains []dnsname.FQDN) error {
+func (m *windowsManager) setPrimaryDNS(resolvers []netip.Addr, domains []dnsname.FQDN) error {
 	var ipsv4 []string
 	var ipsv6 []string
 
@@ -250,7 +250,7 @@ func (m windowsManager) setPrimaryDNS(resolvers []netip.Addr, domains []dnsname.
 	return nil
 }
 
-func (m windowsManager) SetDNS(cfg OSConfig) error {
+func (m *windowsManager) SetDNS(cfg OSConfig) error {
 	// We can configure Windows DNS in one of two ways:
 	//
 	//  - In primary DNS mode, we set the NameServer and SearchList
@@ -372,14 +372,15 @@ func (m windowsManager) SetDNS(cfg OSConfig) error {
 	return nil
 }
 
-func (m windowsManager) SupportsSplitDNS() bool {
+func (m *windowsManager) SupportsSplitDNS() bool {
 	return m.nrptDB != nil
 }
 
-func (m windowsManager) Close() error {
+func (m *windowsManager) Close() error {
 	err := m.SetDNS(OSConfig{})
 	if m.nrptDB != nil {
 		m.nrptDB.Close()
+		m.nrptDB = nil
 	}
 	return err
 }
@@ -387,7 +388,7 @@ func (m windowsManager) Close() error {
 // disableDynamicUpdates sets the appropriate registry values to prevent the
 // Windows DHCP client from sending dynamic DNS updates for our interface to
 // AD domain controllers.
-func (m windowsManager) disableDynamicUpdates() error {
+func (m *windowsManager) disableDynamicUpdates() error {
 	if err := m.setSingleDWORD(winutil.IPv4TCPIPInterfacePrefix, "DisableDynamicUpdate", 1); err != nil {
 		return err
 	}
@@ -399,7 +400,7 @@ func (m windowsManager) disableDynamicUpdates() error {
 
 // setSingleDWORD opens the Registry Key in HKLM for the interface associated
 // with the windowsManager and sets the "keyPrefix\value" to data.
-func (m windowsManager) setSingleDWORD(prefix winutil.RegistryPathPrefix, value string, data uint32) error {
+func (m *windowsManager) setSingleDWORD(prefix winutil.RegistryPathPrefix, value string, data uint32) error {
 	k, err := m.openInterfaceKey(prefix)
 	if err != nil {
 		return err
@@ -416,11 +417,11 @@ func (m windowsManager) setSingleDWORD(prefix winutil.RegistryPathPrefix, value
 //
 // Further, LLMNR and NetBIOS are being deprecated anyway in favor of MDNS.
 // https://techcommunity.microsoft.com/t5/networking-blog/aligning-on-mdns-ramping-down-netbios-name-resolution-and-llmnr/ba-p/3290816
-func (m windowsManager) disableNetBIOS() error {
+func (m *windowsManager) disableNetBIOS() error {
 	return m.setSingleDWORD(winutil.NetBTInterfacePrefix, "NetbiosOptions", 2)
 }
 
-func (m windowsManager) GetBaseConfig() (OSConfig, error) {
+func (m *windowsManager) GetBaseConfig() (OSConfig, error) {
 	resolvers, err := m.getBasePrimaryResolver()
 	if err != nil {
 		return OSConfig{}, err
@@ -439,7 +440,7 @@ func (m windowsManager) GetBaseConfig() (OSConfig, error) {
 // It's used on Windows 7 to emulate split DNS by trying to figure out
 // what the "previous" primary resolver was. It might be wrong, or
 // incomplete.
-func (m windowsManager) getBasePrimaryResolver() (resolvers []netip.Addr, err error) {
+func (m *windowsManager) getBasePrimaryResolver() (resolvers []netip.Addr, err error) {
 	tsGUID, err := windows.GUIDFromString(m.guid)
 	if err != nil {
 		return nil, err

net/dns/manager_windows_test.go

@@ -89,7 +89,7 @@ func TestManagerWindowsGPMove(t *testing.T) {
 	if err != nil {
 		t.Fatalf("NewOSConfigurator: %v\n", err)
 	}
-	mgr := cfg.(windowsManager)
+	mgr := cfg.(*windowsManager)
 	defer mgr.Close()
 
 	usingGP := mgr.nrptDB.writeAsGP
@@ -218,7 +218,7 @@ func runTest(t *testing.T, isLocal bool) {
 	if err != nil {
 		t.Fatalf("NewOSConfigurator: %v\n", err)
 	}
-	mgr := cfg.(windowsManager)
+	mgr := cfg.(*windowsManager)
 	defer mgr.Close()
 
 	usingGP := mgr.nrptDB.writeAsGP