wgengine/router: decouple dns from router

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

control/controlclient/netmap.go

@@ -187,8 +187,8 @@ func UFlagsHelper(uroutes, rroutes, droutes bool) int {
 
 // TODO(bradfitz): UAPI seems to only be used by the old confnode and
 // pingnode; delete this when those are deleted/rewritten?
-func (nm *NetworkMap) UAPI(uflags int, dnsOverride []wgcfg.IP) string {
+func (nm *NetworkMap) UAPI(uflags int) string {
-	wgcfg, err := nm.WGCfg(log.Printf, uflags, dnsOverride)
+	wgcfg, err := nm.WGCfg(log.Printf, uflags)
 	if err != nil {
 		log.Fatalf("WGCfg() failed unexpectedly: %v\n", err)
 	}
@@ -199,8 +199,8 @@ func (nm *NetworkMap) UAPI(uflags int, dnsOverride []wgcfg.IP) string {
 	return s
 }
 
-func (nm *NetworkMap) WGCfg(logf logger.Logf, uflags int, dnsOverride []wgcfg.IP) (*wgcfg.Config, error) {
+func (nm *NetworkMap) WGCfg(logf logger.Logf, uflags int) (*wgcfg.Config, error) {
-	s := nm._WireGuardConfig(logf, uflags, dnsOverride, true)
+	s := nm._WireGuardConfig(logf, uflags, true)
 	return wgcfg.FromWgQuick(s, "tailscale")
 }
 
@@ -209,7 +209,7 @@ func (nm *NetworkMap) WGCfg(logf logger.Logf, uflags int, dnsOverride []wgcfg.IP
 // This form is then recognize by magicsock's CreateEndpoint.
 const EndpointDiscoSuffix = ".disco.tailscale:12345"
 
-func (nm *NetworkMap) _WireGuardConfig(logf logger.Logf, uflags int, dnsOverride []wgcfg.IP, allEndpoints bool) string {
+func (nm *NetworkMap) _WireGuardConfig(logf logger.Logf, uflags int, allEndpoints bool) string {
 	buf := new(strings.Builder)
 	fmt.Fprintf(buf, "[Interface]\n")
 	fmt.Fprintf(buf, "PrivateKey = %s\n", base64.StdEncoding.EncodeToString(nm.PrivateKey[:]))
@@ -224,13 +224,6 @@ func (nm *NetworkMap) _WireGuardConfig(logf logger.Logf, uflags int, dnsOverride
 		fmt.Fprintf(buf, "\n")
 	}
 	fmt.Fprintf(buf, "ListenPort = %d\n", nm.LocalPort)
-	if len(dnsOverride) > 0 {
-		dnss := []string{}
-		for _, ip := range dnsOverride {
-			dnss = append(dnss, ip.String())
-		}
-		fmt.Fprintf(buf, "DNS = %s\n", strings.Join(dnss, ","))
-	}
 	fmt.Fprintf(buf, "\n")
 
 	for i, peer := range nm.Peers {

internal/deepprint/deepprint_test.go

@@ -11,6 +11,7 @@ import (
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"inet.af/netaddr"
 	"tailscale.com/wgengine/router"
+	"tailscale.com/wgengine/router/dns"
 )
 
 func TestDeepPrint(t *testing.T) {
@@ -50,7 +51,7 @@ func getVal() []interface{} {
 			},
 		},
 		&router.Config{
-			DNSConfig: router.DNSConfig{
+			DNS: dns.Config{
 				Nameservers: []netaddr.IP{netaddr.IPv4(8, 8, 8, 8)},
 				Domains:     []string{"tailscale.net"},
 			},

ipn/local.go

@@ -27,6 +27,7 @@ import (
 	"tailscale.com/wgengine"
 	"tailscale.com/wgengine/filter"
 	"tailscale.com/wgengine/router"
+	"tailscale.com/wgengine/router/dns"
 	"tailscale.com/wgengine/tsdns"
 )
 
@@ -845,19 +846,34 @@ func (b *LocalBackend) authReconfig() {
 		uflags |= controlclient.UAllowSingleHosts
 	}
 
-	dns := nm.DNS
+	cfg, err := nm.WGCfg(b.logf, uflags)
-	dom := nm.DNSDomains
-	if !uc.CorpDNS {
-		dns = []wgcfg.IP{}
-		dom = []string{}
-	}
-	cfg, err := nm.WGCfg(b.logf, uflags, dns)
 	if err != nil {
 		b.logf("wgcfg: %v", err)
 		return
 	}
 
-	err = b.e.Reconfig(cfg, routerConfig(cfg, uc, dom))
+	rcfg := routerConfig(cfg, uc)
+
+	// Proxied ("Magic") DNS is enabled by adding a magic domain (b.tailscale.net)
+	// to search domains in the admin panel.
+	proxied := false
+	for _, dom := range nm.DNSDomains {
+		if dom == "b.tailscale.net" {
+			proxied = true
+		}
+	}
+	// 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)
+	rcfg.DNS = dns.Config{
+		Disabled:  !uc.CorpDNS,
+		PerDomain: perDomain,
+
+		Nameservers: wgIPToNetaddr(nm.DNS),
+		Domains:     nm.DNSDomains,
+	}
+
+	err = b.e.Reconfig(cfg, rcfg)
 	if err == wgengine.ErrNoChanges {
 		return
 	}
@@ -866,7 +882,7 @@ func (b *LocalBackend) authReconfig() {
 
 // routerConfig produces a router.Config from a wireguard config,
 // IPN prefs, and the dnsDomains pulled from control's network map.
-func routerConfig(cfg *wgcfg.Config, prefs *Prefs, dnsDomains []string) *router.Config {
+func routerConfig(cfg *wgcfg.Config, prefs *Prefs) *router.Config {
 	var addrs []wgcfg.CIDR
 	for _, addr := range cfg.Addresses {
 		addrs = append(addrs, wgcfg.CIDR{
@@ -880,18 +896,14 @@ func routerConfig(cfg *wgcfg.Config, prefs *Prefs, dnsDomains []string) *router.
 		SubnetRoutes:     wgCIDRToNetaddr(prefs.AdvertiseRoutes),
 		SNATSubnetRoutes: !prefs.NoSNAT,
 		NetfilterMode:    prefs.NetfilterMode,
-		DNSConfig: router.DNSConfig{
-			Nameservers: wgIPToNetaddr(cfg.DNS),
-			Domains:     dnsDomains,
-		},
 	}
 
 	for _, peer := range cfg.Peers {
 		rs.Routes = append(rs.Routes, wgCIDRToNetaddr(peer.AllowedIPs)...)
 	}
 
-	// The Tailscale DNS IP.
+	// The Magic DNS IP.
-	// TODO(dmytro): make this configurable.
+	// TODO(dmytro): deduplicate this constant (also in wgengine/userspace.go)
 	rs.Routes = append(rs.Routes, netaddr.IPPrefix{
 		IP:   netaddr.IPv4(100, 100, 100, 100),
 		Bits: 32,

wgengine/router/dns/config.go

@@ -6,25 +6,36 @@ package dns
 
 import (
 	"inet.af/netaddr"
+
+	"tailscale.com/types/logger"
 )
 
-// Config is the subset of router.Config that contains DNS parameters.
+// Config is the set of parameters which uniquely determine
-type DNSConfig struct {
+// the state to which a manager should bring system DNS settings.
+type Config struct {
+	// Disabled indicates whether DNS settings should be managed at all.
+	// If true, active changes are rolled back and all other fields of Config are ignored.
+	Disabled bool
+	// PerDomain indicates whether it is preferred to use Nameservers
+	// only for DNS queries for subdomains of Domains.
+	//
+	// Note that Nameservers may still be applied to all queries
+	// if the manager does not support per-domain settings.
+	PerDomain bool
+
 	// Nameservers are the IP addresses of the nameservers to use.
 	Nameservers []netaddr.IP
 	// Domains are the search domains to use.
 	Domains []string
-	// PerDomain indicates whether it is preferred to use Nameservers
-	// only for queries for subdomains of Domains.
-	//
-	// Note that Nameservers may still be applied to all queries
-	// if the selected configuration mode does not support per-domain settings.
-	PerDomain bool
 }
 
 // EquivalentTo determines whether its argument and receiver
 // represent equivalent DNS configurations (then DNS reconfig is a no-op).
 func (lhs Config) EquivalentTo(rhs Config) bool {
+	if lhs.Disabled != rhs.Disabled || lhs.PerDomain != rhs.PerDomain {
+		return false
+	}
+
 	if len(lhs.Nameservers) != len(rhs.Nameservers) {
 		return false
 	}
@@ -33,10 +44,6 @@ func (lhs Config) EquivalentTo(rhs Config) bool {
 		return false
 	}
 
-	if lhs.PerDomain != rhs.PerDomain {
-		return false
-	}
-
 	// With how we perform resolution order shouldn't matter,
 	// but it is unlikely that we will encounter different orders.
 	for i, server := range lhs.Nameservers {
@@ -55,31 +62,19 @@ func (lhs Config) EquivalentTo(rhs Config) bool {
 	return true
 }
 
-// dnsMode determines how DNS settings are managed.
+// ManagerConfig is the set of parameters from which
-type dnsMode uint8
+// a manager implementation is chosen and initialized.
+type ManagerConfig struct {
+	// Logf is the logger for the manager to use.
+	Logf logger.Logf
 
-const (
+	// InterfaceName is the name of the interface whose DNS settings should be managed.
-	// dnsDirect indicates that /etc/resolv.conf is edited directly.
+	InterfaceName string
-	dnsDirect dnsMode = iota
-	// dnsResolvconf indicates that a resolvconf binary is used.
-	dnsResolvconf
-	// dnsNetworkManager indicates that the NetworkManaer DBus API is used.
-	dnsNetworkManager
-	// dnsResolved indicates that the systemd-resolved DBus API is used.
-	dnsResolved
-)
 
-func (m dnsMode) String() string {
+	// Cleanup indicates that this manager is created for cleanup only.
-	switch m {
+	// A no-op manager will be instantiated if no cleanup is needed.
-	case dnsDirect:
+	Cleanup bool
-		return "direct"
+	// PerDomain indicates that a manager capable of per-domain configuration is preferred.
-	case dnsResolvconf:
+	// Certain managers are per-domain only; they will not be considered if this is false.
-		return "resolvconf"
+	PerDomain bool
-	case dnsNetworkManager:
-		return "networkmanager"
-	case dnsResolved:
-		return "resolved"
-	default:
-		return "???"
-	}
 }

wgengine/router/dns/direct.go

@@ -4,7 +4,7 @@
 
 // +build linux freebsd openbsd
 
-package router
+package dns
 
 import (
 	"bufio"
@@ -45,8 +45,8 @@ func writeResolvConf(w io.Writer, servers []netaddr.IP, domains []string) {
 }
 
 // readResolvConf reads DNS configuration from /etc/resolv.conf.
-func readResolvConf() (DNSConfig, error) {
+func readResolvConf() (Config, error) {
-	var config DNSConfig
+	var config Config
 
 	f, err := os.Open("/etc/resolv.conf")
 	if err != nil {
@@ -79,14 +79,21 @@ func readResolvConf() (DNSConfig, error) {
 	return config, nil
 }
 
-// dnsDirectUp replaces /etc/resolv.conf with a file generated
+// directManager is a managerImpl which replaces /etc/resolv.conf with a file
-// from the given configuration, creating a backup of its old state.
+// generated from the given configuration, creating a backup of its old state.
 //
 // This way of configuring DNS is precarious, since it does not react
 // to the disappearance of the Tailscale interface.
-// The caller must call dnsDirectDown before program shutdown
+// The caller must call Down before program shutdown
-// and ensure that router.Cleanup is run if the program terminates unexpectedly.
+// or as cleanup if the program terminates unexpectedly.
-func dnsDirectUp(config DNSConfig) error {
+type directManager struct{}
+
+func newDirectManager(mconfig ManagerConfig) managerImpl {
+	return new(directManager)
+}
+
+// Up implements managerImpl.
+func (m *directManager) Up(config Config) error {
 	// Write the tsConf file.
 	buf := new(bytes.Buffer)
 	writeResolvConf(buf, config.Nameservers, config.Domains)
@@ -127,9 +134,8 @@ func dnsDirectUp(config DNSConfig) error {
 	return nil
 }
 
-// dnsDirectDown restores /etc/resolv.conf to its state before dnsDirectUp.
+// Down implements managerImpl.
-// It is idempotent and behaves correctly even if dnsDirectUp has never been run.
+func (m *directManager) Down() error {
-func dnsDirectDown() error {
 	if _, err := os.Stat(backupConf); err != nil {
 		// If the backup file does not exist, then Up never ran successfully.
 		if os.IsNotExist(err) {

wgengine/router/dns/manager.go

@@ -10,47 +10,81 @@ import (
 	"tailscale.com/types/logger"
 )
 
-// setTimeout is the time interval within which Manager.Set should complete.
+// reconfigTimeout is the time interval within which Manager.{Up,Down} should complete.
 //
 // This is particularly useful because certain conditions can cause indefinite hangs
 // (such as improper dbus auth followed by contextless dbus.Object.Call).
 // Such operations should be wrapped in a timeout context.
-const setTimeout = time.Second
+const reconfigTimeout = time.Second
 
 type managerImpl interface {
-	Set(Config) error
+	// Up updates system DNS settings to match the given configuration.
-	Get() Config
+	Up(Config) error
-	Reset() error
+	// Down undoes the effects of Up.
+	// It is idempotent and performs no action if Up has never been called.
+	Down() error
 }
 
+// Manager manages system DNS settings.
 type Manager struct {
-	impl      managerImpl
+	logf logger.Logf
-	oldConfig Config
+
+	impl managerImpl
+
+	config  Config
+	mconfig ManagerConfig
 }
 
-func NewManager(logf logger.Logf, interfaceName string) *Manager {
+// NewManagers created a new manager from the given config.
-	return &Manager{
+func NewManager(mconfig ManagerConfig) *Manager {
-		impl: newManager(logf, interfaceName),
+	m := &Manager{
+		logf: logger.WithPrefix(mconfig.Logf, "router/dns: "),
+		impl: newManager(mconfig),
+
+		config:  Config{PerDomain: mconfig.PerDomain},
+		mconfig: mconfig,
 	}
+	m.logf("using %T", m.impl)
+	return m
 }
 
-func (m *Manager) Up(config Config) error {
+func (m *Manager) Set(config Config) error {
-	if len(config.Nameservers) == 0 {
+	if config.EquivalentTo(m.config) {
-		return m.impl.Down()
-	}
-
-	if config.EquivalentTo(m.oldConfig) {
 		return nil
 	}
 
+	if config.Disabled || len(config.Nameservers) == 0 {
+		err := m.impl.Down()
+		// If we save the config, we will not retry next time. Only do this on success.
+		if err == nil {
+			m.config = config
+		}
+		return err
+	}
+
+	// Switching to and from per-domain mode may require a change of manager.
+	if config.PerDomain != m.config.PerDomain {
+		if err := m.impl.Down(); err != nil {
+			return err
+		}
+		m.mconfig.PerDomain = config.PerDomain
+		m.impl = newManager(m.mconfig)
+		m.logf("using %T", m.impl)
+	}
+
 	err := m.impl.Up(config)
+	// If we save the config, we will not retry next time. Only do this on success.
 	if err == nil {
-		m.oldConfig = config
+		m.config = config
 	}
 
 	return err
 }
 
+func (m *Manager) Up() error {
+	return m.impl.Up(m.config)
+}
+
 func (m *Manager) Down() error {
 	return m.impl.Down()
 }

wgengine/router/dns/manager_default.go

@@ -0,0 +1,11 @@
+// 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.
+
+// +build !linux,!freebsd,!openbsd
+
+package dns
+
+func newManager(mconfig ManagerConfig) managerImpl {
+	return newNoopManager(mconfig)
+}

wgengine/router/dns/manager_freebsd.go

@@ -0,0 +1,14 @@
+// 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
+
+func newManager(mconfig ManagerConfig) managerImpl {
+	switch {
+	case resolvconfIsActive():
+		return newResolvconfManager(mconfig)
+	default:
+		return newDirectManager(mconfig)
+	}
+}

wgengine/router/dns/manager_linux.go

@@ -0,0 +1,26 @@
+// 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
+
+func newManager(mconfig ManagerConfig) managerImpl {
+	switch {
+	case resolvedIsActive() && mconfig.PerDomain:
+		if mconfig.Cleanup {
+			return newNoopManager(mconfig)
+		} else {
+			return newResolvedManager(mconfig)
+		}
+	case nmIsActive():
+		if mconfig.Cleanup {
+			return newNoopManager(mconfig)
+		} else {
+			return newNMManager(mconfig)
+		}
+	case resolvconfIsActive():
+		return newResolvconfManager(mconfig)
+	default:
+		return newDirectManager(mconfig)
+	}
+}

wgengine/router/dns/manager_openbsd.go

@@ -0,0 +1,9 @@
+// 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
+
+func newManager(mconfig ManagerConfig) managerImpl {
+	return newDirectManager(mconfig)
+}

wgengine/router/dns/nm.go

@@ -4,7 +4,7 @@
 
 // +build linux
 
-package router
+package dns
 
 import (
 	"bufio"
@@ -18,7 +18,7 @@ import (
 	"github.com/godbus/dbus/v5"
 )
 
-type nmSettings map[string]map[string]dbus.Variant
+type nmConnectionSettings map[string]map[string]dbus.Variant
 
 // nmIsActive determines if NetworkManager is currently managing system DNS settings.
 func nmIsActive() bool {
@@ -50,10 +50,19 @@ func nmIsActive() bool {
 	return false
 }
 
-// dnsNetworkManagerUp updates the DNS config for the Tailscale interface
+type nmManager struct {
-// through the NetworkManager DBus API.
+	interfaceName string
-func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
+}
-	ctx, cancel := context.WithTimeout(context.Background(), dnsReconfigTimeout)
+
+func newNMManager(mconfig ManagerConfig) managerImpl {
+	return &nmManager{
+		interfaceName: mconfig.InterfaceName,
+	}
+}
+
+// Up implements managerImpl.
+func (m *nmManager) Up(config Config) error {
+	ctx, cancel := context.WithTimeout(context.Background(), reconfigTimeout)
 	defer cancel()
 
 	conn, err := dbus.SystemBus()
@@ -85,7 +94,7 @@ func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
 	var devicePath dbus.ObjectPath
 	err = nm.CallWithContext(
 		ctx, "org.freedesktop.NetworkManager.GetDeviceByIpIface", 0,
-		interfaceName,
+		m.interfaceName,
 	).Store(&devicePath)
 	if err != nil {
 		return fmt.Errorf("GetDeviceByIpIface: %w", err)
@@ -125,7 +134,7 @@ func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
 	// do not seem to show up in NetworkManager at all,
 	// so they are presumably immune from being tampered with.
 
-	var settings nmSettings
+	var settings nmConnectionSettings
 	err = connection.CallWithContext(
 		ctx, "org.freedesktop.NetworkManager.Settings.Connection.GetSettings", 0,
 	).Store(&settings)
@@ -152,14 +161,16 @@ func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
 	ipv4Map := settings["ipv4"]
 	ipv4Map["dns"] = dbus.MakeVariant(dnsv4)
 	ipv4Map["dns-search"] = dbus.MakeVariant(config.Domains)
-	// dns-priority = -1 ensures that we have priority
+	if !config.PerDomain {
-	// over other interfaces, except those exploiting this same trick.
+		// dns-priority = -1 ensures that we have priority
-	// Ref: https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1211110/comments/92.
+		// over other interfaces, except those exploiting this same trick.
-	ipv4Map["dns-priority"] = dbus.MakeVariant(-1)
+		// Ref: https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1211110/comments/92.
-	// In principle, we should not need set this to true,
+		ipv4Map["dns-priority"] = dbus.MakeVariant(-1)
-	// as our interface does not configure any automatic DNS settings (presumably via DHCP).
+		// In principle, we should not need set this to true,
-	// All the same, better to be safe.
+		// as our interface does not configure any automatic DNS settings (presumably via DHCP).
-	ipv4Map["ignore-auto-dns"] = dbus.MakeVariant(true)
+		// All the same, better to be safe.
+		ipv4Map["ignore-auto-dns"] = dbus.MakeVariant(true)
+	}
 
 	ipv6Map := settings["ipv6"]
 	// This is a hack.
@@ -175,8 +186,10 @@ func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
 	// Finally, set the actual DNS config.
 	ipv6Map["dns"] = dbus.MakeVariant(dnsv6)
 	ipv6Map["dns-search"] = dbus.MakeVariant(config.Domains)
-	ipv6Map["dns-priority"] = dbus.MakeVariant(-1)
+	if !config.PerDomain {
-	ipv6Map["ignore-auto-dns"] = dbus.MakeVariant(true)
+		ipv6Map["dns-priority"] = dbus.MakeVariant(-1)
+		ipv6Map["ignore-auto-dns"] = dbus.MakeVariant(true)
+	}
 
 	// deprecatedProperties are the properties in interface settings
 	// that are deprecated by NetworkManager.
@@ -203,7 +216,7 @@ func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
 	return nil
 }
 
-// dnsNetworkManagerDown undoes the changes made by dnsNetworkManagerUp.
+// Down implements managerImpl.
-func dnsNetworkManagerDown(interfaceName string) error {
+func (m *nmManager) Down() error {
-	return dnsNetworkManagerUp(DNSConfig{Nameservers: nil, Domains: nil}, interfaceName)
+	return m.Up(Config{Nameservers: nil, Domains: nil})
 }

wgengine/router/dns/noop.go

@@ -0,0 +1,17 @@
+// 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
+
+type noopManager struct{}
+
+// Up implements managerImpl.
+func (m *noopManager) Up(Config) error { return nil }
+
+// Down implements managerImpl.
+func (m *noopManager) Down() error { return nil }
+
+func newNoopManager(mconfig ManagerConfig) managerImpl {
+	return new(noopManager)
+}

wgengine/router/dns/resolvconf.go

@@ -4,7 +4,7 @@
 
 // +build linux freebsd
 
-package router
+package dns
 
 import (
 	"bufio"
@@ -15,7 +15,7 @@ import (
 )
 
 // resolvconfIsActive indicates whether the system appears to be using resolvconf.
-// If this is true, then dnsManualUp should be avoided:
+// If this is true, then directManager should be avoided:
 // resolvconf has exclusive ownership of /etc/resolv.conf.
 func resolvconfIsActive() bool {
 	// Sanity-check first: if there is no resolvconf binary, then this is fruitless.
@@ -57,13 +57,22 @@ func resolvconfIsActive() bool {
 	return false
 }
 
-// dnsResolvconfUp invokes the resolvconf binary to associate
+type resolvconfManager struct {
-// the given DNS configuration the Tailscale interface.
+	interfaceName string
-func dnsResolvconfUp(config DNSConfig, interfaceName string) error {
+}
-	stdin := new(bytes.Buffer)
-	dnsWriteConfig(stdin, config.Nameservers, config.Domains) // dns_direct.go
 
-	cmd := exec.Command("resolvconf", "-m", "0", "-x", "-a", interfaceName+".inet")
+func newResolvconfManager(mconfig ManagerConfig) managerImpl {
+	return &resolvconfManager{
+		interfaceName: mconfig.InterfaceName,
+	}
+}
+
+// Up implements managerImpl.
+func (m *resolvconfManager) Up(config Config) error {
+	stdin := new(bytes.Buffer)
+	writeResolvConf(stdin, config.Nameservers, config.Domains) // direct.go
+
+	cmd := exec.Command("resolvconf", "-m", "0", "-x", "-a", m.interfaceName+".inet")
 	cmd.Stdin = stdin
 	out, err := cmd.CombinedOutput()
 	if err != nil {
@@ -73,9 +82,9 @@ func dnsResolvconfUp(config DNSConfig, interfaceName string) error {
 	return nil
 }
 
-// dnsResolvconfDown undoes the action of dnsResolvconfUp.
+// Down implements managerImpl.
-func dnsResolvconfDown(interfaceName string) error {
+func (m *resolvconfManager) Down() error {
-	cmd := exec.Command("resolvconf", "-f", "-d", interfaceName+".inet")
+	cmd := exec.Command("resolvconf", "-f", "-d", m.interfaceName+".inet")
 	out, err := cmd.CombinedOutput()
 	if err != nil {
 		return fmt.Errorf("running %s: %s", cmd, out)

wgengine/router/dns/resolved.go

@@ -4,14 +4,13 @@
 
 // +build linux
 
-package router
+package dns
 
 import (
 	"context"
 	"errors"
 	"fmt"
 	"os/exec"
-	"time"
 
 	"github.com/godbus/dbus/v5"
 	"golang.org/x/sys/unix"
@@ -23,7 +22,7 @@ import (
 //
 // We only consider resolved to be the system resolver if the stub resolver is;
 // that is, if this address is the sole nameserver in /etc/resolved.conf.
-// In other cases, resolved may still be managing the system DNS configuration directly.
+// In other cases, resolved may be managing the system DNS configuration directly.
 // Then the nameserver list will be a concatenation of those for all
 // the interfaces that register their interest in being a default resolver with
 //   SetLinkDomains([]{{"~.", true}, ...})
@@ -36,13 +35,6 @@ import (
 // this address is, in fact, hard-coded into resolved.
 var resolvedListenAddr = netaddr.IPv4(127, 0, 0, 53)
 
-// dnsReconfigTimeout is the timeout for DNS reconfiguration.
-//
-// This is useful because certain conditions can cause indefinite hangs
-// (such as improper dbus auth followed by contextless dbus.Object.Call).
-// Such operations should be wrapped in a timeout context.
-const dnsReconfigTimeout = time.Second
-
 var errNotReady = errors.New("interface not ready")
 
 type resolvedLinkNameserver struct {
@@ -69,7 +61,7 @@ func resolvedIsActive() bool {
 		return false
 	}
 
-	config, err := dnsReadConfig()
+	config, err := readResolvConf()
 	if err != nil {
 		return false
 	}
@@ -82,10 +74,15 @@ func resolvedIsActive() bool {
 	return false
 }
 
-// dnsResolvedUp sets the DNS parameters for the Tailscale interface
+type resolvedManager struct{}
-// to given nameservers and search domains using the resolved DBus API.
+
-func dnsResolvedUp(config DNSConfig) error {
+func newResolvedManager(mconfig ManagerConfig) managerImpl {
-	ctx, cancel := context.WithTimeout(context.Background(), dnsReconfigTimeout)
+	return new(resolvedManager)
+}
+
+// Up implements managerImpl.
+func (m *resolvedManager) Up(config Config) error {
+	ctx, cancel := context.WithTimeout(context.Background(), reconfigTimeout)
 	defer cancel()
 
 	conn, err := dbus.SystemBus()
@@ -150,9 +147,9 @@ func dnsResolvedUp(config DNSConfig) error {
 	return nil
 }
 
-// dnsResolvedDown undoes the changes made by dnsResolvedUp.
+// Down implements managerImpl.
-func dnsResolvedDown() error {
+func (m *resolvedManager) Down() error {
-	ctx, cancel := context.WithTimeout(context.Background(), dnsReconfigTimeout)
+	ctx, cancel := context.WithTimeout(context.Background(), reconfigTimeout)
 	defer cancel()
 
 	conn, err := dbus.SystemBus()

wgengine/router/ifconfig_windows.go

@@ -262,7 +262,7 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) error {
 		}
 	}()
 
-	setDNSDomains(guid, cfg.Domains)
+	setDNSDomains(guid, cfg.DNS.Domains)
 
 	routes := []winipcfg.RouteData{}
 	var firstGateway4 *net.IP
@@ -359,7 +359,7 @@ func configureInterface(cfg *Config, tun *tun.NativeTun) error {
 	}
 
 	var dnsIPs []net.IP
-	for _, ip := range cfg.Nameservers {
+	for _, ip := range cfg.DNS.Nameservers {
 		dnsIPs = append(dnsIPs, ip.IPAddr().IP)
 	}
 	err = iface.SetDNS(dnsIPs)

wgengine/router/router.go

@@ -41,6 +41,16 @@ func New(logf logger.Logf, wgdev *device.Device, tundev tun.Device) (Router, err
 // in case the Tailscale daemon terminated without closing the router.
 // No other state needs to be instantiated before this runs.
 func Cleanup(logf logger.Logf, interfaceName string) {
+	mconfig := dns.ManagerConfig{
+		Logf:          logf,
+		InterfaceName: interfaceName,
+		Cleanup:       true,
+	}
+	mgr := dns.NewManager(mconfig)
+	if err := mgr.Down(); err != nil {
+		logf("dns down: %v", err)
+	}
+
 	cleanup(logf, interfaceName)
 }
 

wgengine/router/router_darwin.go

@@ -52,13 +52,3 @@ func (r *darwinRouter) Up() error {
 	}
 	return r.Router.Up()
 }
-
-func upDNS(config DNSConfig, interfaceName string) error {
-	// Handled by IPNExtension
-	return nil
-}
-
-func downDNS(interfaceName string) error {
-	// Handled by IPNExtension
-	return nil
-}

wgengine/router/router_default.go

@@ -16,6 +16,4 @@ func newUserspaceRouter(logf logger.Logf, tunname string, dev *device.Device, tu
 	return NewFakeRouter(logf, tunname, dev, tuntap, netChanged)
 }
 
-func cleanup() error {
+func cleanup(logf logger.Logf, interfaceName string) {}
-	return nil
-}

wgengine/router/router_freebsd.go

@@ -5,8 +5,6 @@
 package router
 
 import (
-	"fmt"
-
 	"github.com/tailscale/wireguard-go/device"
 	"github.com/tailscale/wireguard-go/tun"
 	"tailscale.com/types/logger"
@@ -20,35 +18,3 @@ import (
 func newUserspaceRouter(logf logger.Logf, _ *device.Device, tundev tun.Device) (Router, error) {
 	return newUserspaceBSDRouter(logf, nil, tundev)
 }
-
-func upDNS(config DNSConfig, interfaceName string) error {
-	if len(config.Nameservers) == 0 {
-		return downDNS(interfaceName)
-	}
-
-	if resolvconfIsActive() {
-		if err := dnsResolvconfUp(config, interfaceName); err != nil {
-			return fmt.Errorf("resolvconf: %w")
-		}
-		return nil
-	}
-
-	if err := dnsDirectUp(config); err != nil {
-		return fmt.Errorf("direct: %w")
-	}
-	return nil
-}
-
-func downDNS(interfaceName string) error {
-	if resolvconfIsActive() {
-		if err := dnsResolvconfDown(interfaceName); err != nil {
-			return fmt.Errorf("resolvconf: %w")
-		}
-		return nil
-	}
-
-	if err := dnsDirectDown(); err != nil {
-		return fmt.Errorf("direct: %w")
-	}
-	return nil
-}

wgengine/router/router_linux.go

@@ -15,6 +15,7 @@ import (
 	"inet.af/netaddr"
 	"tailscale.com/net/tsaddr"
 	"tailscale.com/types/logger"
+	"tailscale.com/wgengine/router/dns"
 )
 
 // The following bits are added to packet marks for Tailscale use.
@@ -68,11 +69,10 @@ type linuxRouter struct {
 	snatSubnetRoutes bool
 	netfilterMode    NetfilterMode
 
-	dnsMode   dnsMode
-	dnsConfig DNSConfig
-
 	ipt4 netfilterRunner
 	cmd  commandRunner
+
+	dns *dns.Manager
 }
 
 func newUserspaceRouter(logf logger.Logf, _ *device.Device, tunDev tun.Device) (Router, error) {
@@ -93,6 +93,11 @@ func newUserspaceRouterAdvanced(logf logger.Logf, tunname string, netfilter netf
 	_, err := exec.Command("ip", "rule").Output()
 	ipRuleAvailable := (err == nil)
 
+	mconfig := dns.ManagerConfig{
+		Logf:          logf,
+		InterfaceName: tunname,
+	}
+
 	return &linuxRouter{
 		logf:            logf,
 		ipRuleAvailable: ipRuleAvailable,
@@ -100,6 +105,7 @@ func newUserspaceRouterAdvanced(logf logger.Logf, tunname string, netfilter netf
 		netfilterMode:   NetfilterOff,
 		ipt4:            netfilter,
 		cmd:             cmd,
+		dns:             dns.NewManager(mconfig),
 	}, nil
 }
 
@@ -117,26 +123,12 @@ func (r *linuxRouter) Up() error {
 		return err
 	}
 
-	switch {
-	// TODO(dmytro): enable resolved when per-domain resolvers are desired.
-	case resolvedIsActive():
-		r.dnsMode = dnsDirect
-	//	r.dnsMode = dnsResolved
-	case nmIsActive():
-		r.dnsMode = dnsNetworkManager
-	case resolvconfIsActive():
-		r.dnsMode = dnsResolvconf
-	default:
-		r.dnsMode = dnsDirect
-	}
-	r.logf("dns mode: %v", r.dnsMode)
-
 	return nil
 }
 
 func (r *linuxRouter) Close() error {
-	if err := r.downDNS(); err != nil {
+	if err := r.dns.Down(); err != nil {
-		return err
+		return fmt.Errorf("dns down: %v", err)
 	}
 	if err := r.downInterface(); err != nil {
 		return err
@@ -190,12 +182,8 @@ func (r *linuxRouter) Set(cfg *Config) error {
 	}
 	r.snatSubnetRoutes = cfg.SNATSubnetRoutes
 
-	if !r.dnsConfig.EquivalentTo(cfg.DNSConfig) {
+	if err := r.dns.Set(cfg.DNS); err != nil {
-		if err := r.upDNS(cfg.DNSConfig); err != nil {
+		return fmt.Errorf("dns set: %v", err)
-			r.logf("dns up: %v", err)
-		} else {
-			r.dnsConfig = cfg.DNSConfig
-		}
 	}
 
 	return nil
@@ -840,68 +828,6 @@ func normalizeCIDR(cidr netaddr.IPPrefix) string {
 	return fmt.Sprintf("%s/%d", nip, cidr.Bits)
 }
 
-// upDNS updates the system DNS configuration to the given one.
-func (r *linuxRouter) upDNS(config DNSConfig) error {
-	if len(config.Nameservers) == 0 {
-		return r.downDNS()
-	}
-
-	switch r.dnsMode {
-	case dnsResolved:
-		if err := dnsResolvedUp(config); err != nil {
-			return fmt.Errorf("resolved: %w", err)
-		}
-	case dnsResolvconf:
-		if err := dnsResolvconfUp(config, r.tunname); err != nil {
-			return fmt.Errorf("resolvconf: %w", err)
-		}
-	case dnsNetworkManager:
-		if err := dnsNetworkManagerUp(config, r.tunname); err != nil {
-			return fmt.Errorf("network manager: %w", err)
-		}
-	case dnsDirect:
-		if err := dnsDirectUp(config); err != nil {
-			return fmt.Errorf("direct: %w", err)
-		}
-	}
-	return nil
-}
-
-// downDNS restores system DNS configuration to its state before upDNS.
-// It is idempotent (in particular, it does nothing if upDNS was never run).
-func (r *linuxRouter) downDNS() error {
-	switch r.dnsMode {
-	case dnsResolved:
-		if err := dnsResolvedDown(); err != nil {
-			return fmt.Errorf("resolved: %w", err)
-		}
-	case dnsResolvconf:
-		if err := dnsResolvconfDown(r.tunname); err != nil {
-			return fmt.Errorf("resolvconf: %w", err)
-		}
-	case dnsNetworkManager:
-		if err := dnsNetworkManagerDown(r.tunname); err != nil {
-			return fmt.Errorf("network manager: %w", err)
-		}
-	case dnsDirect:
-		if err := dnsDirectDown(); err != nil {
-			return fmt.Errorf("direct: %w", err)
-		}
-	}
-	return nil
-}
-
 func cleanup(logf logger.Logf, interfaceName string) {
-	// Note: we need not do anything for dnsResolved,
+	// TODO(dmytro): clean up iptable chains.
-	// as its settings are interface-bound and get cleaned up for us.
-	switch {
-	case resolvconfIsActive():
-		if err := dnsResolvconfDown(interfaceName); err != nil {
-			logf("down down: resolvconf: %v", err)
-		}
-	default:
-		if err := dnsDirectDown(); err != nil {
-			logf("dns down: direct: %v", err)
-		}
-	}
 }

wgengine/router/router_openbsd.go

@@ -14,6 +14,7 @@ import (
 	"github.com/tailscale/wireguard-go/tun"
 	"inet.af/netaddr"
 	"tailscale.com/types/logger"
+	"tailscale.com/wgengine/router/dns"
 )
 
 // For now this router only supports the WireGuard userspace implementation.
@@ -26,7 +27,7 @@ type openbsdRouter struct {
 	local   netaddr.IPPrefix
 	routes  map[netaddr.IPPrefix]struct{}
 
-	dnsConfig DNSConfig
+	dns *dns.Manager
 }
 
 func newUserspaceRouter(logf logger.Logf, _ *device.Device, tundev tun.Device) (Router, error) {
@@ -34,9 +35,16 @@ func newUserspaceRouter(logf logger.Logf, _ *device.Device, tundev tun.Device) (
 	if err != nil {
 		return nil, err
 	}
+
+	mconfig := dns.ManagerConfig{
+		Logf:          logf,
+		InterfaceName: tunname,
+	}
+
 	return &openbsdRouter{
 		logf:    logf,
 		tunname: tunname,
+		dns:     dns.NewManager(mconfig),
 	}, nil
 }
 
@@ -155,26 +163,25 @@ func (r *openbsdRouter) Set(cfg *Config) error {
 	r.local = localAddr
 	r.routes = newRoutes
 
-	if !r.dnsConfig.EquivalentTo(cfg.DNSConfig) {
+	if err := r.dns.Set(cfg.DNS); err != nil {
-		if err := dnsDirectUp(cfg.DNSConfig); err != nil {
+		errq = fmt.Errorf("dns set: %v", err)
-			errq = fmt.Errorf("dns up: direct: %v", err)
-		} else {
-			r.dnsConfig = cfg.DNSConfig
-		}
 	}
 
 	return errq
 }
 
 func (r *openbsdRouter) Close() error {
-	cleanup(r.logf, r.tunname)
+	if err := r.dns.Down(); err != nil {
+		return fmt.Errorf("dns down: %v", err)
+	}
+	out, err := cmd("ifconfig", r.tunname, "down").CombinedOutput()
+	if err != nil {
+		return fmt.Errorf("ifconfig down: %v\n%s", err, out)
+	}
 	return nil
 }
 
 func cleanup(logf logger.Logf, interfaceName string) {
-	if err := dnsDirectDown(); err != nil {
-		logf("dns down: direct: %v", err)
-	}
 	out, err := cmd("ifconfig", interfaceName, "down").CombinedOutput()
 	if err != nil {
 		logf("ifconfig down: %v\n%s", err, out)

wgengine/router/router_userspace_bsd.go

@@ -16,6 +16,7 @@ import (
 	"github.com/tailscale/wireguard-go/tun"
 	"inet.af/netaddr"
 	"tailscale.com/types/logger"
+	"tailscale.com/wgengine/router/dns"
 )
 
 type userspaceBSDRouter struct {
@@ -24,7 +25,7 @@ type userspaceBSDRouter struct {
 	local   netaddr.IPPrefix
 	routes  map[netaddr.IPPrefix]struct{}
 
-	dnsConfig DNSConfig
+	dns *dns.Manager
 }
 
 func newUserspaceBSDRouter(logf logger.Logf, _ *device.Device, tundev tun.Device) (Router, error) {
@@ -32,9 +33,16 @@ func newUserspaceBSDRouter(logf logger.Logf, _ *device.Device, tundev tun.Device
 	if err != nil {
 		return nil, err
 	}
+
+	mconfig := dns.ManagerConfig{
+		Logf:          logf,
+		InterfaceName: tunname,
+	}
+
 	return &userspaceBSDRouter{
 		logf:    logf,
 		tunname: tunname,
+		dns:     dns.NewManager(mconfig),
 	}, nil
 }
 
@@ -141,27 +149,22 @@ func (r *userspaceBSDRouter) Set(cfg *Config) error {
 	r.local = localAddr
 	r.routes = newRoutes
 
-	if !r.dnsConfig.EquivalentTo(cfg.DNSConfig) {
+	if err := r.dns.Set(cfg.DNS); err != nil {
-		if err := upDNS(cfg.DNSConfig, r.tunname); err != nil {
+		errq = fmt.Errorf("dns set: %v", err)
-			errq = fmt.Errorf("dns up: %v", err)
-		} else {
-			r.dnsConfig = cfg.DNSConfig
-		}
 	}
 
 	return errq
 }
 
 func (r *userspaceBSDRouter) Close() error {
+	if err := r.dns.Down(); err != nil {
+		r.logf("dns down: %v", err)
+	}
 	cleanup(r.logf, r.tunname)
 	return nil
 }
 
 func cleanup(logf logger.Logf, interfaceName string) {
-	if err := downDNS(interfaceName); err != nil {
-		logf("dns down: %v", err)
-	}
-
 	ifup := []string{"ifconfig", interfaceName, "down"}
 	if out, err := cmd(ifup...).CombinedOutput(); err != nil {
 		logf("ifconfig down: %v\n%s", err, out)

wgengine/userspace.go

@@ -581,13 +581,6 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 	}
 	e.mu.Unlock()
 
-	// If the only nameserver is quad 100 (Magic DNS), set up the resolver appropriately.
-	if len(routerCfg.Nameservers) == 1 && routerCfg.Nameservers[0] == packet.IP(magicDNSIP).Netaddr() {
-		// TODO(dmytro): plumb dnsReadConfig here instead of hardcoding this.
-		e.resolver.SetNameservers([]string{"8.8.8.8:53"})
-		routerCfg.Domains = append([]string{magicDNSDomain}, routerCfg.Domains...)
-	}
-
 	engineChanged := updateSig(&e.lastEngineSig, cfg)
 	routerChanged := updateSig(&e.lastRouterSig, routerCfg)
 	if !engineChanged && !routerChanged {