wgengine/router: split off windows dnsdshynkev/dns-refactor

Signed-off-by: Dmytro Shynkevych <dmytro@tailscale.com>

7 files changed, 112 insertions, 45 deletions

diff --git a/ipn/local.go b/ipn/local.go
index 9acd988b7..c1116b085 100644
--- a/ipn/local.go
+++ b/ipn/local.go
@@ -864,7 +864,10 @@ func (b *LocalBackend) authReconfig() {
 	}
 	// Per-domain DNS is enabled only when proxying with no nameservers in the netmap:
 	// finding fallback servers is unreliable in this case, so we want to avoid it if possible.
-	perDomain := proxied && (len(nm.DNS) == 0)
+	perDomain := proxied && len(nm.DNS) == 0
+	if proxied {
+		nm.DNS = []wgcfg.IP{wgcfg.IPv4(100, 100, 100, 100)}
+	}
 	rcfg.DNS = dns.Config{
 		Disabled:  !uc.CorpDNS,
 		PerDomain: perDomain,
diff --git a/wgengine/router/dns/manager_default.go b/wgengine/router/dns/manager_default.go
index 361bcbb17..65d79302f 100644
--- a/wgengine/router/dns/manager_default.go
+++ b/wgengine/router/dns/manager_default.go
@@ -2,7 +2,7 @@
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
 
-// +build !linux,!freebsd,!openbsd
+// +build !linux,!freebsd,!openbsd,!windows
 
 package dns
 
diff --git a/wgengine/router/dns/manager_windows.go b/wgengine/router/dns/manager_windows.go
new file mode 100644
index 000000000..9af7adea2
--- /dev/null
+++ b/wgengine/router/dns/manager_windows.go
@@ -0,0 +1,72 @@
+// 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.
+
+package dns
+
+import (
+	"fmt"
+	"strings"
+
+	"golang.org/x/sys/windows/registry"
+)
+
+type windowsManager struct {
+	interfaceName string
+}
+
+func newManager(mconfig ManagerConfig) managerImpl {
+	return &windowsManager{
+		interfaceName: mconfig.InterfaceName,
+	}
+}
+
+func setRegistry(path, nameservers, domains string) error {
+	key, err := registry.OpenKey(registry.LOCAL_MACHINE, path, registry.READ|registry.SET_VALUE)
+	if err != nil {
+		return fmt.Errorf("opening %s: %w", path, err)
+	}
+	defer key.Close()
+
+	err = key.SetStringValue("NameServer", nameservers)
+	if err != nil {
+		return fmt.Errorf("setting %s/NameServer: %w", path, err)
+	}
+
+	err = key.SetStringValue("Domain", domains)
+	if err != nil {
+		return fmt.Errorf("setting %s/Domain: %w", path, err)
+	}
+
+	return nil
+}
+
+func (m *windowsManager) Up(config Config) error {
+	var ipsv4 []string
+	var ipsv6 []string
+	for _, ip := range config.Nameservers {
+		if ip.Is4() {
+			ipsv4 = append(ipsv4, ip.String())
+		} else {
+			ipsv6 = append(ipsv6, ip.String())
+		}
+	}
+	nsv4 := strings.Join(ipsv4, ",")
+	nsv6 := strings.Join(ipsv6, ",")
+	domains := strings.Join(config.Domains, ";")
+
+	v4Path := `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\` + m.interfaceName
+	if err := setRegistry(v4Path, nsv4, domains); err != nil {
+		return err
+	}
+	v6Path := `SYSTEM\CurrentControlSet\Services\Tcpip6\Parameters\Interfaces\` + m.interfaceName
+	if err := setRegistry(v6Path, nsv6, domains); err != nil {
+		return err
+	}
+
+	return nil
+}
+
+func (m *windowsManager) Down() error {
+	return m.Up(Config{Nameservers: nil, Domains: nil})
+}
diff --git a/wgengine/router/ifconfig_windows.go b/wgengine/router/ifconfig_windows.go
index 01dfc2196..410e4facb 100644
--- a/wgengine/router/ifconfig_windows.go
+++ b/wgengine/router/ifconfig_windows.go
@@ -21,7 +21,6 @@ import (
 	"github.com/tailscale/wireguard-go/device"
 	"github.com/tailscale/wireguard-go/tun"
 	"golang.org/x/sys/windows"
-	"golang.org/x/sys/windows/registry"
 	"tailscale.com/wgengine/winnet"
 )
 
@@ -157,28 +156,6 @@ func monitorDefaultRoutes(device *device.Device, autoMTU bool, tun *tun.NativeTu
 	return cb, nil
 }
 
-func setDNSDomains(g windows.GUID, dnsDomains []string) {
-	gs := g.String()
-	log.Printf("setDNSDomains(%v) guid=%v\n", dnsDomains, gs)
-	p := `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\` + gs
-	key, err := registry.OpenKey(registry.LOCAL_MACHINE, p, registry.READ|registry.SET_VALUE)
-	if err != nil {
-		log.Printf("setDNSDomains(%v): open: %v\n", p, err)
-		return
-	}
-	defer key.Close()
-
-	// Windows only supports a single per-interface DNS domain.
-	dom := ""
-	if len(dnsDomains) > 0 {
-		dom = dnsDomains[0]
-	}
-	err = key.SetStringValue("Domain", dom)
-	if err != nil {
-		log.Printf("setDNSDomains(%v): SetStringValue: %v\n", p, err)
-	}
-}
-
 func setFirewall(ifcGUID *windows.GUID) (bool, error) {
 	c := ole.Connection{}
 	err := c.Initialize()
@@ -262,8 +239,6 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) error {
 		}
 	}()
 
-	setDNSDomains(guid, cfg.DNS.Domains)
-
 	routes := []winipcfg.RouteData{}
 	var firstGateway4 *net.IP
 	var firstGateway6 *net.IP
@@ -358,16 +333,6 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) error {
 		errAcc = err
 	}
 
-	var dnsIPs []net.IP
-	for _, ip := range cfg.DNS.Nameservers {
-		dnsIPs = append(dnsIPs, ip.IPAddr().IP)
-	}
-	err = iface.SetDNS(dnsIPs)
-	if err != nil && errAcc == nil {
-		log.Printf("setdns: %v\n", err)
-		errAcc = err
-	}
-
 	ipif, err := iface.GetIpInterface(winipcfg.AF_INET)
 	if err != nil {
 		log.Printf("getipif: %v\n", err)
diff --git a/wgengine/router/router_userspace_bsd.go b/wgengine/router/router_userspace_bsd.go
index 16876716d..19a0d4430 100644
--- a/wgengine/router/router_userspace_bsd.go
+++ b/wgengine/router/router_userspace_bsd.go
@@ -160,13 +160,20 @@ func (r *userspaceBSDRouter) Close() error {
 	if err := r.dns.Down(); err != nil {
 		r.logf("dns down: %v", err)
 	}
-	cleanup(r.logf, r.tunname)
+	ifup := []string{"ifconfig", r.tunname, "down"}
+	if out, err := cmd(ifup...).CombinedOutput(); err != nil {
+		r.logf("ifconfig down: %v\n%s", err, out)
+	}
 	return nil
 }
 
 func cleanup(logf logger.Logf, interfaceName string) {
-	ifup := []string{"ifconfig", interfaceName, "down"}
+	// If the interface was left behind, ifconfig down will not remove it.
+	// In fact, this will leave a system in a tainted state where starting tailscaled
+	// will result in "interface tailscale0 already exists"
+	// until the defunct interface is ifconfig-destroyed.
+	ifup := []string{"ifconfig", interfaceName, "destroy"}
 	if out, err := cmd(ifup...).CombinedOutput(); err != nil {
-		logf("ifconfig down: %v\n%s", err, out)
+		logf("ifconfig destroy: %v\n%s", err, out)
 	}
 }
diff --git a/wgengine/router/router_windows.go b/wgengine/router/router_windows.go
index 00f3e281c..f329bf0d5 100644
--- a/wgengine/router/router_windows.go
+++ b/wgengine/router/router_windows.go
@@ -5,12 +5,13 @@
 package router
 
 import (
-	"log"
+	"fmt"
 
 	winipcfg "github.com/tailscale/winipcfg-go"
 	"github.com/tailscale/wireguard-go/device"
 	"github.com/tailscale/wireguard-go/tun"
 	"tailscale.com/types/logger"
+	"tailscale.com/wgengine/router/dns"
 )
 
 type winRouter struct {
@@ -19,6 +20,7 @@ type winRouter struct {
 	nativeTun           *tun.NativeTun
 	wgdev               *device.Device
 	routeChangeCallback *winipcfg.RouteChangeCallback
+	dns                 *dns.Manager
 }
 
 func newUserspaceRouter(logf logger.Logf, wgdev *device.Device, tundev tun.Device) (Router, error) {
@@ -26,11 +28,20 @@ func newUserspaceRouter(logf logger.Logf, wgdev *device.Device, tundev tun.Devic
 	if err != nil {
 		return nil, err
 	}
+
+	nativeTun := tundev.(*tun.NativeTun)
+	guid := nativeTun.GUID().String()
+	mconfig := dns.ManagerConfig{
+		Logf:          logf,
+		InterfaceName: guid,
+	}
+
 	return &winRouter{
 		logf:      logf,
 		wgdev:     wgdev,
 		tunname:   tunname,
-		nativeTun: tundev.(*tun.NativeTun),
+		nativeTun: nativeTun,
+		dns:       dns.NewManager(mconfig),
 	}, nil
 }
 
@@ -40,7 +51,7 @@ func (r *winRouter) Up() error {
 	var err error
 	r.routeChangeCallback, err = monitorDefaultRoutes(r.wgdev, true, r.nativeTun)
 	if err != nil {
-		log.Fatalf("MonitorDefaultRoutes: %v\n", err)
+		return fmt.Errorf("MonitorDefaultRoutes: %w", err)
 	}
 	return nil
 }
@@ -52,13 +63,20 @@ func (r *winRouter) Set(cfg *Config) error {
 
 	err := configureInterface(cfg, r.nativeTun)
 	if err != nil {
-		r.logf("ConfigureInterface: %v\n", err)
-		return err
+		return fmt.Errorf("ConfigureInterface: %w", err)
 	}
+
+	if err := r.dns.Set(cfg.DNS); err != nil {
+		return fmt.Errorf("dns set: %w", err)
+	}
+
 	return nil
 }
 
 func (r *winRouter) Close() error {
+	if err := r.dns.Down(); err != nil {
+		return fmt.Errorf("dns down: %w", err)
+	}
 	if r.routeChangeCallback != nil {
 		r.routeChangeCallback.Unregister()
 	}
diff --git a/wgengine/userspace.go b/wgengine/userspace.go
index d460a3527..228660c65 100644
--- a/wgengine/userspace.go
+++ b/wgengine/userspace.go
@@ -581,6 +581,8 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 	}
 	e.mu.Unlock()
 
+	e.resolver.SetNameservers([]string{"9.9.9.9:53"})
+
 	engineChanged := updateSig(&e.lastEngineSig, cfg)
 	routerChanged := updateSig(&e.lastRouterSig, routerCfg)
 	if !engineChanged && !routerChanged {