From 41e1d336cc3e2c017322551ad0325f04b5d64b98 Mon Sep 17 00:00:00 2001 From: Aaron Klotz Date: Wed, 3 Aug 2022 16:18:19 -0600 Subject: [PATCH] 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 --- net/dns/manager_windows.go | 27 ++++++++++++++------------- net/dns/manager_windows_test.go | 4 ++-- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/net/dns/manager_windows.go b/net/dns/manager_windows.go index baf349511..c3bf3b58c 100644 --- a/net/dns/manager_windows.go +++ b/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 diff --git a/net/dns/manager_windows_test.go b/net/dns/manager_windows_test.go index 5063f730f..ae0569d5b 100644 --- a/net/dns/manager_windows_test.go +++ b/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