From 552b08096b71ba7440e6c9c4bd83728c4c1faa0b Mon Sep 17 00:00:00 2001 From: Alex Brainman Date: Sun, 17 Jan 2021 15:23:52 +1100 Subject: [PATCH] wgengine/router/dns: use wireguard winipcfg package MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This change replaces existing DNS setting code with golang.zx2c4.com/wireguard/windows/tunnel/winipcfg.LUID.SetDNS and LUID.SetDNSDomain As suggested by Jason at https://github.com/tailscale/tailscale/issues/1050#issuecomment-749196854 Updates #1050 Signed-off-by: Alex Brainman --- cmd/tailscale/depaware.txt | 7 +- cmd/tailscaled/depaware.txt | 12 ++- wgengine/router/dns/manager_windows.go | 112 ++++++------------------ wgengine/router/dns/registry_windows.go | 76 ---------------- 4 files changed, 38 insertions(+), 169 deletions(-) delete mode 100644 wgengine/router/dns/registry_windows.go diff --git a/cmd/tailscale/depaware.txt b/cmd/tailscale/depaware.txt index 2af1dd217..67012ed9b 100644 --- a/cmd/tailscale/depaware.txt +++ b/cmd/tailscale/depaware.txt @@ -63,7 +63,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep tailscale.com/paths from tailscale.com/cmd/tailscale/cli tailscale.com/portlist from tailscale.com/ipn tailscale.com/safesocket from tailscale.com/cmd/tailscale/cli - 💣 tailscale.com/syncs from tailscale.com/net/interfaces+ + tailscale.com/syncs from tailscale.com/net/interfaces+ tailscale.com/tailcfg from tailscale.com/cmd/tailscale/cli+ W tailscale.com/tsconst from tailscale.com/net/interfaces tailscale.com/types/empty from tailscale.com/control/controlclient+ @@ -119,7 +119,7 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep golang.org/x/sys/cpu from golang.org/x/crypto/blake2b+ LD golang.org/x/sys/unix from github.com/jsimonetti/rtnetlink/internal/unix+ W golang.org/x/sys/windows from github.com/apenwarr/fixconsole+ - W golang.org/x/sys/windows/registry from golang.zx2c4.com/wireguard/windows/tunnel/winipcfg+ + W golang.org/x/sys/windows/registry from golang.zx2c4.com/wireguard/windows/tunnel/winipcfg golang.org/x/text/secure/bidirule from golang.org/x/net/idna golang.org/x/text/transform from golang.org/x/text/secure/bidirule+ golang.org/x/text/unicode/bidi from golang.org/x/net/idna+ @@ -174,7 +174,8 @@ tailscale.com/cmd/tailscale dependencies: (generated by github.com/tailscale/dep hash/maphash from go4.org/mem html from tailscale.com/ipn/ipnstate io from bufio+ - io/ioutil from crypto/tls+ + io/fs from crypto/rand+ + io/ioutil from github.com/godbus/dbus/v5+ log from expvar+ math from compress/flate+ math/big from crypto/dsa+ diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index 64c949cdd..d61c9016c 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -100,7 +100,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/portlist from tailscale.com/ipn tailscale.com/safesocket from tailscale.com/ipn/ipnserver tailscale.com/smallzstd from tailscale.com/ipn/ipnserver+ - 💣 tailscale.com/syncs from tailscale.com/net/interfaces+ + tailscale.com/syncs from tailscale.com/net/interfaces+ tailscale.com/tailcfg from tailscale.com/control/controlclient+ W tailscale.com/tsconst from tailscale.com/net/interfaces tailscale.com/types/empty from tailscale.com/control/controlclient+ @@ -160,7 +160,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de golang.org/x/sys/cpu from golang.org/x/crypto/blake2b+ LD golang.org/x/sys/unix from github.com/jsimonetti/rtnetlink/internal/unix+ W golang.org/x/sys/windows from github.com/apenwarr/fixconsole+ - W golang.org/x/sys/windows/registry from golang.zx2c4.com/wireguard/windows/tunnel/winipcfg+ + W golang.org/x/sys/windows/registry from golang.zx2c4.com/wireguard/windows/tunnel/winipcfg golang.org/x/term from tailscale.com/logpolicy golang.org/x/text/secure/bidirule from golang.org/x/net/idna golang.org/x/text/transform from golang.org/x/text/secure/bidirule+ @@ -215,10 +215,10 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de hash/crc32 from compress/gzip+ hash/fnv from tailscale.com/wgengine/magicsock hash/maphash from go4.org/mem - html from html/template+ - html/template from net/http/pprof + html from net/http/pprof+ io from bufio+ - io/ioutil from crypto/tls+ + io/fs from crypto/rand+ + io/ioutil from github.com/godbus/dbus/v5+ log from expvar+ math from compress/flate+ math/big from crypto/dsa+ @@ -253,8 +253,6 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de sync/atomic from context+ syscall from crypto/rand+ text/tabwriter from runtime/pprof - text/template from html/template - text/template/parse from html/template+ time from compress/gzip+ unicode from bytes+ unicode/utf16 from encoding/asn1+ diff --git a/wgengine/router/dns/manager_windows.go b/wgengine/router/dns/manager_windows.go index 5940404e7..640fea5cc 100644 --- a/wgengine/router/dns/manager_windows.go +++ b/wgengine/router/dns/manager_windows.go @@ -5,111 +5,57 @@ package dns import ( - "fmt" - "os/exec" - "strings" - "syscall" - "time" + "net" - "golang.org/x/sys/windows/registry" + "golang.org/x/sys/windows" + "golang.zx2c4.com/wireguard/windows/tunnel/winipcfg" "tailscale.com/types/logger" ) -const ( - ipv4RegBase = `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters` - ipv6RegBase = `SYSTEM\CurrentControlSet\Services\Tcpip6\Parameters` -) - type windowsManager struct { - logf logger.Logf - guid string + logf logger.Logf + luid winipcfg.LUID + initerr error } func newManager(mconfig ManagerConfig) managerImpl { - return windowsManager{ + m := windowsManager{ logf: mconfig.Logf, - guid: mconfig.InterfaceName, } -} - -// keyOpenTimeout is how long we wait for a registry key to -// appear. For some reason, registry keys tied to ephemeral interfaces -// can take a long while to appear after interface creation, and we -// can end up racing with that. -const keyOpenTimeout = 20 * time.Second - -func setRegistryString(path, name, value string) error { - key, err := openKeyWait(registry.LOCAL_MACHINE, path, registry.SET_VALUE, keyOpenTimeout) - if err != nil { - return fmt.Errorf("opening %s: %w", path, err) + var guid windows.GUID + guid, m.initerr = windows.GUIDFromString(mconfig.InterfaceName) + if m.initerr != nil { + return m } - defer key.Close() - - err = key.SetStringValue(name, value) - if err != nil { - return fmt.Errorf("setting %s[%s]: %w", path, name, err) - } - return nil -} - -func (m windowsManager) setNameservers(basePath string, nameservers []string) error { - path := fmt.Sprintf(`%s\Interfaces\%s`, basePath, m.guid) - value := strings.Join(nameservers, ",") - return setRegistryString(path, "NameServer", value) -} - -func (m windowsManager) setDomains(basePath string, domains []string) error { - path := fmt.Sprintf(`%s\Interfaces\%s`, basePath, m.guid) - value := strings.Join(domains, ",") - return setRegistryString(path, "SearchList", value) + m.luid, m.initerr = winipcfg.LUIDFromGUID(&guid) + return m } func (m windowsManager) Up(config Config) error { - var ipsv4 []string - var ipsv6 []string + if m.initerr != nil { + return m.initerr + } + var ips []net.IP for _, ip := range config.Nameservers { - if ip.Is4() { - ipsv4 = append(ipsv4, ip.String()) - } else { - ipsv6 = append(ipsv6, ip.String()) - } + ips = append(ips, ip.IPAddr().IP) } - - if err := m.setNameservers(ipv4RegBase, ipsv4); err != nil { - return err - } - if err := m.setDomains(ipv4RegBase, config.Domains); err != nil { + err := m.luid.SetDNS(ips) + if err != nil { return err } - if err := m.setNameservers(ipv6RegBase, ipsv6); err != nil { - return err + dnsSearch := "" + if len(config.Domains) > 0 { + dnsSearch = config.Domains[0] } - if err := m.setDomains(ipv6RegBase, config.Domains); err != nil { - return err + err = m.luid.SetDNSDomain(dnsSearch) + if err != nil { + return nil + } + if len(config.Domains) > 1 { + m.logf("%d DNS search domains were specified, but only one is supported, so the first one (%s) was used.", len(config.Domains), dnsSearch) } - - // Force DNS re-registration in Active Directory. What we actually - // care about is that this command invokes the undocumented hidden - // function that forces Windows to notice that adapter settings - // have changed, which makes the DNS settings actually take - // effect. - // - // This command can take a few seconds to run, so run it async, best effort. - go func() { - t0 := time.Now() - m.logf("running ipconfig /registerdns ...") - cmd := exec.Command("ipconfig", "/registerdns") - cmd.SysProcAttr = &syscall.SysProcAttr{HideWindow: true} - d := time.Since(t0).Round(time.Millisecond) - if err := cmd.Run(); err != nil { - m.logf("error running ipconfig /registerdns after %v: %v", d, err) - } else { - m.logf("ran ipconfig /registerdns in %v", d) - } - }() - return nil } diff --git a/wgengine/router/dns/registry_windows.go b/wgengine/router/dns/registry_windows.go deleted file mode 100644 index f8e1f514a..000000000 --- a/wgengine/router/dns/registry_windows.go +++ /dev/null @@ -1,76 +0,0 @@ -// Copyright (c) 2020 Tailscale Inc & AUTHORS All rights reserved. -// Use of this source code is governed by a BSD-style -// license that can be found in the LICENSE file. -// -// The code in this file originates from https://git.zx2c4.com/wireguard-go: -// Copyright (C) 2017-2020 WireGuard LLC. All Rights Reserved. -// Copying license: https://git.zx2c4.com/wireguard-go/tree/COPYING - -package dns - -import ( - "fmt" - "runtime" - "strings" - "time" - - "golang.org/x/sys/windows" - "golang.org/x/sys/windows/registry" -) - -func openKeyWait(k registry.Key, path string, access uint32, timeout time.Duration) (registry.Key, error) { - runtime.LockOSThread() - defer runtime.UnlockOSThread() - - deadline := time.Now().Add(timeout) - pathSpl := strings.Split(path, "\\") - for i := 0; ; i++ { - keyName := pathSpl[i] - isLast := i+1 == len(pathSpl) - - event, err := windows.CreateEvent(nil, 0, 0, nil) - if err != nil { - return 0, fmt.Errorf("windows.CreateEvent: %v", err) - } - defer windows.CloseHandle(event) - - var key registry.Key - for { - err = windows.RegNotifyChangeKeyValue(windows.Handle(k), false, windows.REG_NOTIFY_CHANGE_NAME, event, true) - if err != nil { - return 0, fmt.Errorf("windows.RegNotifyChangeKeyValue: %v", err) - } - - var accessFlags uint32 - if isLast { - accessFlags = access - } else { - accessFlags = registry.NOTIFY - } - key, err = registry.OpenKey(k, keyName, accessFlags) - if err == windows.ERROR_FILE_NOT_FOUND || err == windows.ERROR_PATH_NOT_FOUND { - timeout := time.Until(deadline) / time.Millisecond - if timeout < 0 { - timeout = 0 - } - s, err := windows.WaitForSingleObject(event, uint32(timeout)) - if err != nil { - return 0, fmt.Errorf("windows.WaitForSingleObject: %v", err) - } - if s == uint32(windows.WAIT_TIMEOUT) { // windows.WAIT_TIMEOUT status const is misclassified as error in golang.org/x/sys/windows - return 0, fmt.Errorf("timeout waiting for registry key") - } - } else if err != nil { - return 0, fmt.Errorf("registry.OpenKey(%v): %v", path, err) - } else { - if isLast { - return key, nil - } - defer key.Close() - break - } - } - - k = key - } -}