all: dns refactor, add Proxied and PerDomain flags from control (#615)

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

control/controlclient/direct.go

@@ -30,6 +30,7 @@ import (
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"golang.org/x/crypto/nacl/box"
 	"golang.org/x/oauth2"
+	"inet.af/netaddr"
 	"tailscale.com/log/logheap"
 	"tailscale.com/net/netns"
 	"tailscale.com/net/tlsdial"
@@ -638,8 +639,7 @@ func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkM
 			UserProfiles: make(map[tailcfg.UserID]tailcfg.UserProfile),
 			Domain:       resp.Domain,
 			Roles:        resp.Roles,
-			DNS:          resp.DNS,
+			DNS:          resp.DNSConfig,
-			DNSDomains:   resp.SearchPaths,
 			Hostinfo:     resp.Node.Hostinfo,
 			PacketFilter: c.parsePacketFilter(resp.PacketFilter),
 			DERPMap:      lastDERPMap,
@@ -653,6 +653,15 @@ func (c *Direct) PollNetMap(ctx context.Context, maxPolls int, cb func(*NetworkM
 		} else {
 			nm.MachineStatus = tailcfg.MachineUnauthorized
 		}
+		if len(resp.DNS) > 0 {
+			nm.DNS.Nameservers = wgIPToNetaddr(resp.DNS)
+		}
+		if len(resp.SearchPaths) > 0 {
+			nm.DNS.Domains = resp.SearchPaths
+		}
+		if Debug.ProxyDNS {
+			nm.DNS.Proxied = true
+		}
 
 		// Printing the netmap can be extremely verbose, but is very
 		// handy for debugging. Let's limit how often we do it.
@@ -792,12 +801,24 @@ func loadServerKey(ctx context.Context, httpc *http.Client, serverURL string) (w
 	return key, nil
 }
 
+func wgIPToNetaddr(ips []wgcfg.IP) (ret []netaddr.IP) {
+	for _, ip := range ips {
+		nip, ok := netaddr.FromStdIP(ip.IP())
+		if !ok {
+			panic(fmt.Sprintf("conversion of %s from wgcfg to netaddr IP failed", ip))
+		}
+		ret = append(ret, nip.Unmap())
+	}
+	return ret
+}
+
 // Debug contains temporary internal-only debug knobs.
 // They're unexported to not draw attention to them.
 var Debug = initDebug()
 
 type debug struct {
 	NetMap     bool
+	ProxyDNS   bool
 	OnlyDisco  bool
 	Disco      bool
 	ForceDisco bool // ask control server to not filter out our disco key
@@ -806,6 +827,7 @@ type debug struct {
 func initDebug() debug {
 	d := debug{
 		NetMap:     envBool("TS_DEBUG_NETMAP"),
+		ProxyDNS:   envBool("TS_DEBUG_PROXY_DNS"),
 		OnlyDisco:  os.Getenv("TS_DEBUG_USE_DISCO") == "only",
 		ForceDisco: os.Getenv("TS_DEBUG_USE_DISCO") == "only" || envBool("TS_DEBUG_USE_DISCO"),
 	}

control/controlclient/netmap.go

@@ -32,8 +32,7 @@ type NetworkMap struct {
 	LocalPort     uint16 // used for debugging
 	MachineStatus tailcfg.MachineStatus
 	Peers         []*tailcfg.Node // sorted by Node.ID
-	DNS           []wgcfg.IP
+	DNS           tailcfg.DNSConfig
-	DNSDomains    []string
 	Hostinfo      tailcfg.Hostinfo
 	PacketFilter  filter.Matches
 
@@ -219,8 +218,8 @@ const (
 
 // 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(flags WGConfigFlags, dnsOverride []wgcfg.IP) string {
+func (nm *NetworkMap) UAPI(flags WGConfigFlags) string {
-	wgcfg, err := nm.WGCfg(log.Printf, flags, dnsOverride)
+	wgcfg, err := nm.WGCfg(log.Printf, flags)
 	if err != nil {
 		log.Fatalf("WGCfg() failed unexpectedly: %v", err)
 	}
@@ -237,13 +236,12 @@ func (nm *NetworkMap) UAPI(flags WGConfigFlags, dnsOverride []wgcfg.IP) string {
 const EndpointDiscoSuffix = ".disco.tailscale:12345"
 
 // WGCfg returns the NetworkMaps's Wireguard configuration.
-func (nm *NetworkMap) WGCfg(logf logger.Logf, flags WGConfigFlags, dnsOverride []wgcfg.IP) (*wgcfg.Config, error) {
+func (nm *NetworkMap) WGCfg(logf logger.Logf, flags WGConfigFlags) (*wgcfg.Config, error) {
 	cfg := &wgcfg.Config{
 		Name:       "tailscale",
 		PrivateKey: nm.PrivateKey,
 		Addresses:  nm.Addresses,
 		ListenPort: nm.LocalPort,
-		DNS:        append([]wgcfg.IP(nil), dnsOverride...),
 		Peers:      make([]wgcfg.Peer, 0, len(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

@@ -19,6 +19,7 @@ import (
 	"tailscale.com/internal/deepprint"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/ipn/policy"
+	"tailscale.com/net/tsaddr"
 	"tailscale.com/portlist"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/empty"
@@ -28,6 +29,7 @@ import (
 	"tailscale.com/wgengine"
 	"tailscale.com/wgengine/filter"
 	"tailscale.com/wgengine/router"
+	"tailscale.com/wgengine/router/dns"
 	"tailscale.com/wgengine/tsdns"
 )
 
@@ -911,28 +913,71 @@ func (b *LocalBackend) authReconfig() {
 		flags |= controlclient.AllowSingleHosts
 	}
 
-	dns := nm.DNS
+	cfg, err := nm.WGCfg(b.logf, flags)
-	dom := nm.DNSDomains
-	if !uc.CorpDNS {
-		dns = []wgcfg.IP{}
-		dom = []string{}
-	}
-	cfg, err := nm.WGCfg(b.logf, flags, dns)
 	if err != nil {
 		b.logf("wgcfg: %v", err)
 		return
 	}
 
-	err = b.e.Reconfig(cfg, routerConfig(cfg, uc, dom))
+	rcfg := routerConfig(cfg, uc)
+
+	// If CorpDNS is false, rcfg.DNS remains the zero value.
+	if uc.CorpDNS {
+		domains := nm.DNS.Domains
+		proxied := nm.DNS.Proxied
+		if proxied {
+			if len(nm.DNS.Nameservers) == 0 {
+				b.logf("[unexpected] dns proxied but no nameservers")
+				proxied = false
+			} else {
+				domains = append(domains, domainsForProxying(nm)...)
+			}
+		}
+		rcfg.DNS = dns.Config{
+			Nameservers: nm.DNS.Nameservers,
+			Domains:     domains,
+			PerDomain:   nm.DNS.PerDomain,
+			Proxied:     proxied,
+		}
+	}
+
+	err = b.e.Reconfig(cfg, rcfg)
 	if err == wgengine.ErrNoChanges {
 		return
 	}
 	b.logf("authReconfig: ra=%v dns=%v 0x%02x: %v", uc.RouteAll, uc.CorpDNS, flags, err)
 }
 
-// routerConfig produces a router.Config from a wireguard config,
+// domainsForProxying produces a list of search domains for proxied DNS.
-// IPN prefs, and the dnsDomains pulled from control's network map.
+func domainsForProxying(nm *controlclient.NetworkMap) []string {
-func routerConfig(cfg *wgcfg.Config, prefs *Prefs, dnsDomains []string) *router.Config {
+	var domains []string
+	if idx := strings.IndexByte(nm.Name, '.'); idx != -1 {
+		domains = append(domains, nm.Name[idx+1:])
+	}
+	for _, peer := range nm.Peers {
+		idx := strings.IndexByte(peer.Name, '.')
+		if idx == -1 {
+			continue
+		}
+		domain := peer.Name[idx+1:]
+		seen := false
+		// In theory this makes the function O(n^2) worst case,
+		// but in practice we expect domains to contain very few elements
+		// (only one until invitations are introduced).
+		for _, seenDomain := range domains {
+			if domain == seenDomain {
+				seen = true
+			}
+		}
+		if !seen {
+			domains = append(domains, domain)
+		}
+	}
+	return domains
+}
+
+// routerConfig produces a router.Config from a wireguard config and IPN prefs.
+func routerConfig(cfg *wgcfg.Config, prefs *Prefs) *router.Config {
 	var addrs []wgcfg.CIDR
 	for _, addr := range cfg.Addresses {
 		addrs = append(addrs, wgcfg.CIDR{
@@ -946,20 +991,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.
-	// TODO(dmytro): make this configurable.
 	rs.Routes = append(rs.Routes, netaddr.IPPrefix{
-		IP:   netaddr.IPv4(100, 100, 100, 100),
+		IP:   tsaddr.TailscaleServiceIP(),
 		Bits: 32,
 	})
 
@@ -983,17 +1022,6 @@ func wgCIDRsToFilter(cidrLists ...[]wgcfg.CIDR) (ret []filter.Net) {
 	return ret
 }
 
-func wgIPToNetaddr(ips []wgcfg.IP) (ret []netaddr.IP) {
-	for _, ip := range ips {
-		nip, ok := netaddr.FromStdIP(ip.IP())
-		if !ok {
-			panic(fmt.Sprintf("conversion of %s from wgcfg to netaddr IP failed", ip))
-		}
-		ret = append(ret, nip.Unmap())
-	}
-	return ret
-}
-
 func wgCIDRToNetaddr(cidrs []wgcfg.CIDR) (ret []netaddr.IPPrefix) {
 	for _, cidr := range cidrs {
 		ncidr, ok := netaddr.FromStdIPNet(cidr.IPNet())

net/tsaddr/tsaddr.go

@@ -32,6 +32,15 @@ func CGNATRange() netaddr.IPPrefix {
 
 var cgnatRange oncePrefix
 
+// TailscaleServiceIP returns the listen address of services
+// provided by Tailscale itself such as the Magic DNS proxy.
+func TailscaleServiceIP() netaddr.IP {
+	serviceIP.Do(func() { mustIP(&serviceIP.v, "100.100.100.100") })
+	return serviceIP.v
+}
+
+var serviceIP onceIP
+
 // IsTailscaleIP reports whether ip is an IP address in a range that
 // Tailscale assigns from.
 func IsTailscaleIP(ip netaddr.IP) bool {
@@ -50,3 +59,16 @@ type oncePrefix struct {
 	sync.Once
 	v netaddr.IPPrefix
 }
+
+func mustIP(v *netaddr.IP, ip string) {
+	var err error
+	*v, err = netaddr.ParseIP(ip)
+	if err != nil {
+		panic(err)
+	}
+}
+
+type onceIP struct {
+	sync.Once
+	v netaddr.IP
+}

tailcfg/tailcfg.go

@@ -17,6 +17,7 @@ import (
 	"github.com/tailscale/wireguard-go/wgcfg"
 	"go4.org/mem"
 	"golang.org/x/oauth2"
+	"inet.af/netaddr"
 	"tailscale.com/types/key"
 	"tailscale.com/types/opt"
 	"tailscale.com/types/structs"
@@ -492,15 +493,31 @@ var FilterAllowAll = []FilterRule{
 	},
 }
 
+// DNSConfig is the DNS configuration.
+type DNSConfig struct {
+	Nameservers []netaddr.IP `json:",omitempty"`
+	Domains     []string     `json:",omitempty"`
+	PerDomain   bool
+	Proxied     bool
+}
+
 type MapResponse struct {
 	KeepAlive bool // if set, all other fields are ignored
 
 	// Networking
-	Node        *Node
+	Node    *Node
-	Peers       []*Node
+	Peers   []*Node
-	DNS         []wgcfg.IP
+	DERPMap *DERPMap
+
+	// DNS is the same as DNSConfig.Nameservers.
+	//
+	// TODO(dmytro): should be sent in DNSConfig.Nameservers once clients have updated.
+	DNS []wgcfg.IP
+	// SearchPaths are the same as DNSConfig.Domains.
+	//
+	// TODO(dmytro): should be sent in DNSConfig.Domains once clients have updated.
 	SearchPaths []string
-	DERPMap     *DERPMap
+	DNSConfig   DNSConfig
 
 	// ACLs
 	Domain       string

wgengine/magicsock/magicsock_test.go

@@ -277,7 +277,7 @@ func meshStacks(logf logger.Logf, ms []*magicStack) (cleanup func()) {
 				peerSet[key.Public(peer.Key)] = struct{}{}
 			}
 			m.conn.UpdatePeers(peerSet)
-			wg, err := netmap.WGCfg(logf, controlclient.AllowSingleHosts, nil)
+			wg, err := netmap.WGCfg(logf, controlclient.AllowSingleHosts)
 			if err != nil {
 				// We're too far from the *testing.T to be graceful,
 				// blow up. Shouldn't happen anyway.

wgengine/router/dns.go

@@ -1,74 +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.
-
-package router
-
-import (
-	"inet.af/netaddr"
-)
-
-// DNSConfig is the subset of Config that contains DNS parameters.
-type DNSConfig struct {
-	// Nameservers are the IP addresses of the nameservers to use.
-	Nameservers []netaddr.IP
-	// Domains are the search domains to use.
-	Domains []string
-}
-
-// EquivalentTo determines whether its argument and receiver
-// represent equivalent DNS configurations (then DNS reconfig is a no-op).
-func (lhs DNSConfig) EquivalentTo(rhs DNSConfig) bool {
-	if len(lhs.Nameservers) != len(rhs.Nameservers) {
-		return false
-	}
-
-	if len(lhs.Domains) != len(rhs.Domains) {
-		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 {
-		if rhs.Nameservers[i] != server {
-			return false
-		}
-	}
-
-	for i, domain := range lhs.Domains {
-		if rhs.Domains[i] != domain {
-			return false
-		}
-	}
-
-	return true
-}
-
-// dnsMode determines how DNS settings are managed.
-type dnsMode uint8
-
-const (
-	// dnsDirect indicates that /etc/resolv.conf is edited directly.
-	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 {
-	switch m {
-	case dnsDirect:
-		return "direct"
-	case dnsResolvconf:
-		return "resolvconf"
-	case dnsNetworkManager:
-		return "networkmanager"
-	case dnsResolved:
-		return "resolved"
-	default:
-		return "???"
-	}
-}

wgengine/router/dns/config.go

@@ -0,0 +1,75 @@
+// 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 (
+	"inet.af/netaddr"
+
+	"tailscale.com/types/logger"
+)
+
+// Config is the set of parameters that uniquely determine
+// the state to which a manager should bring system DNS settings.
+type Config struct {
+	// 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 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
+	// Proxied indicates whether DNS requests are proxied through a tsdns.Resolver.
+	Proxied bool
+}
+
+// Equal determines whether its argument and receiver
+// represent equivalent DNS configurations (then DNS reconfig is a no-op).
+func (lhs Config) Equal(rhs Config) bool {
+	if lhs.Proxied != rhs.Proxied || lhs.PerDomain != rhs.PerDomain {
+		return false
+	}
+
+	if len(lhs.Nameservers) != len(rhs.Nameservers) {
+		return false
+	}
+
+	if len(lhs.Domains) != len(rhs.Domains) {
+		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 {
+		if rhs.Nameservers[i] != server {
+			return false
+		}
+	}
+
+	// The order of domains, on the other hand, is significant.
+	for i, domain := range lhs.Domains {
+		if rhs.Domains[i] != domain {
+			return false
+		}
+	}
+
+	return true
+}
+
+// ManagerConfig is the set of parameters from which
+// a manager implementation is chosen and initialized.
+type ManagerConfig struct {
+	// logf is the logger for the manager to use.
+	Logf logger.Logf
+	// InterfaceNAme is the name of the interface with which DNS settings should be associated.
+	InterfaceName string
+	// Cleanup indicates that the manager is created for cleanup only.
+	// A no-op manager will be instantiated if the system needs no cleanup.
+	Cleanup bool
+	// PerDomain indicates that a manager capable of per-domain configuration is preferred.
+	// Certain managers are per-domain only; they will not be considered if this is false.
+	PerDomain bool
+}

wgengine/router/dns/direct.go

@@ -4,7 +4,7 @@
 
 // +build linux freebsd openbsd
 
-package router
+package dns
 
 import (
 	"bufio"
@@ -27,8 +27,8 @@ const (
 	resolvConf = "/etc/resolv.conf"
 )
 
-// dnsWriteConfig writes DNS configuration in resolv.conf format to the given writer.
+// writeResolvConf writes DNS configuration in resolv.conf format to the given writer.
-func dnsWriteConfig(w io.Writer, servers []netaddr.IP, domains []string) {
+func writeResolvConf(w io.Writer, servers []netaddr.IP, domains []string) {
 	io.WriteString(w, "# resolv.conf(5) file generated by tailscale\n")
 	io.WriteString(w, "# DO NOT EDIT THIS FILE BY HAND -- CHANGES WILL BE OVERWRITTEN\n\n")
 	for _, ns := range servers {
@@ -46,9 +46,9 @@ func dnsWriteConfig(w io.Writer, servers []netaddr.IP, domains []string) {
 	}
 }
 
-// dnsReadConfig reads DNS configuration from /etc/resolv.conf.
+// readResolvConf reads DNS configuration from /etc/resolv.conf.
-func dnsReadConfig() (DNSConfig, error) {
+func readResolvConf() (Config, error) {
-	var config DNSConfig
+	var config Config
 
 	f, err := os.Open("/etc/resolv.conf")
 	if err != nil {
@@ -100,17 +100,24 @@ func isResolvedRunning() bool {
 	return err == 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 directManager{}
+}
+
+// Up implements managerImpl.
+func (m directManager) Up(config Config) error {
 	// Write the tsConf file.
 	buf := new(bytes.Buffer)
-	dnsWriteConfig(buf, config.Nameservers, config.Domains)
+	writeResolvConf(buf, config.Nameservers, config.Domains)
 	if err := atomicfile.WriteFile(tsConf, buf.Bytes(), 0644); err != nil {
 		return err
 	}
@@ -152,9 +159,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

@@ -0,0 +1,94 @@
+// 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 (
+	"time"
+
+	"tailscale.com/types/logger"
+)
+
+// 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 reconfigTimeout = time.Second
+
+type managerImpl interface {
+	// Up updates system DNS settings to match the given configuration.
+	Up(Config) 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 {
+	logf logger.Logf
+
+	impl managerImpl
+
+	config  Config
+	mconfig ManagerConfig
+}
+
+// NewManagers created a new manager from the given config.
+func NewManager(mconfig ManagerConfig) *Manager {
+	mconfig.Logf = logger.WithPrefix(mconfig.Logf, "dns: ")
+	m := &Manager{
+		logf: mconfig.Logf,
+		impl: newManager(mconfig),
+
+		config:  Config{PerDomain: mconfig.PerDomain},
+		mconfig: mconfig,
+	}
+
+	m.logf("using %T", m.impl)
+	return m
+}
+
+func (m *Manager) Set(config Config) error {
+	if config.Equal(m.config) {
+		return nil
+	}
+
+	m.logf("Set: %+v", config)
+
+	if 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("switched to %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.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,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.
+
+// +build !linux,!freebsd,!openbsd,!windows
+
+package dns
+
+func newManager(mconfig ManagerConfig) managerImpl {
+	// TODO(dmytro): on darwin, we should use a macOS-specific method such as scutil.
+	// This is currently not implemented. Editing /etc/resolv.conf does not work,
+	// as most applications use the system resolver, which disregards it.
+	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 isResolvconfActive():
+		return newResolvconfManager(mconfig)
+	default:
+		return newDirectManager(mconfig)
+	}
+}

wgengine/router/dns/manager_linux.go

@@ -0,0 +1,27 @@
+// 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 {
+	// systemd-resolved should only activate per-domain.
+	case isResolvedActive() && mconfig.PerDomain:
+		if mconfig.Cleanup {
+			return newNoopManager(mconfig)
+		} else {
+			return newResolvedManager(mconfig)
+		}
+	case isNMActive():
+		if mconfig.Cleanup {
+			return newNoopManager(mconfig)
+		} else {
+			return newNMManager(mconfig)
+		}
+	case isResolvconfActive():
+		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/manager_windows.go

@@ -0,0 +1,83 @@
+// 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"
+
+	"github.com/tailscale/wireguard-go/tun"
+	"golang.org/x/sys/windows/registry"
+	"tailscale.com/types/logger"
+)
+
+type windowsManager struct {
+	logf logger.Logf
+	guid string
+}
+
+func newManager(mconfig ManagerConfig) managerImpl {
+	return windowsManager{
+		logf: mconfig.Logf,
+		guid: tun.WintunGUID,
+	}
+}
+
+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, ",")
+
+	var domains string
+	if len(config.Domains) > 0 {
+		if len(config.Domains) > 1 {
+			m.logf("only a single search domain is supported")
+		}
+		domains = config.Domains[0]
+	}
+
+	v4Path := `SYSTEM\CurrentControlSet\Services\Tcpip\Parameters\Interfaces\` + m.guid
+	if err := setRegistry(v4Path, nsv4, domains); err != nil {
+		return err
+	}
+	v6Path := `SYSTEM\CurrentControlSet\Services\Tcpip6\Parameters\Interfaces\` + m.guid
+	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})
+}

wgengine/router/dns/nm.go

@@ -4,7 +4,7 @@
 
 // +build linux
 
-package router
+package dns
 
 import (
 	"bufio"
@@ -20,8 +20,8 @@ import (
 
 type nmConnectionSettings map[string]map[string]dbus.Variant
 
-// nmIsActive determines if NetworkManager is currently managing system DNS settings.
+// isNMActive determines if NetworkManager is currently managing system DNS settings.
-func nmIsActive() bool {
+func isNMActive() bool {
 	// This is somewhat tricky because NetworkManager supports a number
 	// of DNS configuration modes. In all cases, we expect it to be installed
 	// and /etc/resolv.conf to contain a mention of NetworkManager in the comments.
@@ -50,10 +50,20 @@ func nmIsActive() bool {
 	return false
 }
 
-// dnsNetworkManagerUp updates the DNS config for the Tailscale interface
+// nmManager uses the NetworkManager DBus API.
-// through the NetworkManager DBus API.
+type nmManager struct {
-func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error {
+	interfaceName string
-	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 is a shared connection whose lifecycle is managed by the dbus package.
@@ -90,7 +100,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)
@@ -189,7 +199,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 noopManager{}
+}

wgengine/router/dns/resolvconf.go

@@ -4,7 +4,7 @@
 
 // +build linux freebsd
 
-package router
+package dns
 
 import (
 	"bufio"
@@ -14,10 +14,10 @@ import (
 	"os/exec"
 )
 
-// resolvconfIsActive indicates whether the system appears to be using resolvconf.
+// isResolvconfActive 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 {
+func isResolvconfActive() bool {
 	// Sanity-check first: if there is no resolvconf binary, then this is fruitless.
 	//
 	// However, this binary may be a shim like the one systemd-resolved provides.
@@ -57,21 +57,31 @@ func resolvconfIsActive() bool {
 	return false
 }
 
-// resolvconfImplementation enumerates supported implementations of the resolvconf CLI.
+// resolvconfImpl enumerates supported implementations of the resolvconf CLI.
-type resolvconfImplementation uint8
+type resolvconfImpl uint8
 
 const (
 	// resolvconfOpenresolv is the implementation packaged as "openresolv" on Ubuntu.
 	// It supports exclusive mode and interface metrics.
-	resolvconfOpenresolv resolvconfImplementation = iota
+	resolvconfOpenresolv resolvconfImpl = iota
 	// resolvconfLegacy is the implementation by Thomas Hood packaged as "resolvconf" on Ubuntu.
 	// It does not support exclusive mode or interface metrics.
 	resolvconfLegacy
 )
 
-// getResolvconfImplementation returns the implementation of resolvconf
+func (impl resolvconfImpl) String() string {
-// that appears to be in use.
+	switch impl {
-func getResolvconfImplementation() resolvconfImplementation {
+	case resolvconfOpenresolv:
+		return "openresolv"
+	case resolvconfLegacy:
+		return "legacy"
+	default:
+		return "unknown"
+	}
+}
+
+// getResolvconfImpl returns the implementation of resolvconf that appears to be in use.
+func getResolvconfImpl() resolvconfImpl {
 	err := exec.Command("resolvconf", "-v").Run()
 	if err != nil {
 		if exitErr, ok := err.(*exec.ExitError); ok {
@@ -85,21 +95,31 @@ func getResolvconfImplementation() resolvconfImplementation {
 	return resolvconfOpenresolv
 }
 
+type resolvconfManager struct {
+	impl resolvconfImpl
+}
+
+func newResolvconfManager(mconfig ManagerConfig) managerImpl {
+	impl := getResolvconfImpl()
+	mconfig.Logf("resolvconf implementation is %s", impl)
+
+	return resolvconfManager{
+		impl: impl,
+	}
+}
+
 // resolvconfConfigName is the name of the config submitted to resolvconf.
 // It has this form to match the "tun*" rule in interface-order
 // when running resolvconfLegacy, hopefully placing our config first.
 const resolvconfConfigName = "tun-tailscale.inet"
 
-// dnsResolvconfUp invokes the resolvconf binary to associate
+// Up implements managerImpl.
-// the given DNS configuration the Tailscale interface.
+func (m resolvconfManager) Up(config Config) error {
-func dnsResolvconfUp(config DNSConfig, interfaceName string) error {
-	implementation := getResolvconfImplementation()
-
 	stdin := new(bytes.Buffer)
-	dnsWriteConfig(stdin, config.Nameservers, config.Domains) // dns_direct.go
+	writeResolvConf(stdin, config.Nameservers, config.Domains) // dns_direct.go
 
 	var cmd *exec.Cmd
-	switch implementation {
+	switch m.impl {
 	case resolvconfOpenresolv:
 		// Request maximal priority (metric 0) and exclusive mode.
 		cmd = exec.Command("resolvconf", "-m", "0", "-x", "-a", resolvconfConfigName)
@@ -117,12 +137,10 @@ 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 {
-	implementation := getResolvconfImplementation()
-
 	var cmd *exec.Cmd
-	switch implementation {
+	switch m.impl {
 	case resolvconfOpenresolv:
 		cmd = exec.Command("resolvconf", "-f", "-d", resolvconfConfigName)
 	case resolvconfLegacy:

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 {
@@ -55,8 +47,8 @@ type resolvedLinkDomain struct {
 	RoutingOnly bool
 }
 
-// resolvedIsActive determines if resolved is currently managing system DNS settings.
+// isResolvedActive determines if resolved is currently managing system DNS settings.
-func resolvedIsActive() bool {
+func isResolvedActive() bool {
 	// systemd-resolved is never installed without systemd.
 	_, err := exec.LookPath("systemctl")
 	if err != nil {
@@ -69,7 +61,7 @@ func resolvedIsActive() bool {
 		return false
 	}
 
-	config, err := dnsReadConfig()
+	config, err := readResolvConf()
 	if err != nil {
 		return false
 	}
@@ -82,10 +74,16 @@ func resolvedIsActive() bool {
 	return false
 }
 
-// dnsResolvedUp sets the DNS parameters for the Tailscale interface
+// resolvedManager uses the systemd-resolved DBus API.
-// to given nameservers and search domains using the resolved DBus API.
+type resolvedManager struct{}
-func dnsResolvedUp(config DNSConfig) error {
+
-	ctx, cancel := context.WithTimeout(context.Background(), dnsReconfigTimeout)
+func newResolvedManager(mconfig ManagerConfig) managerImpl {
+	return resolvedManager{}
+}
+
+// Up implements managerImpl.
+func (m resolvedManager) Up(config Config) error {
+	ctx, cancel := context.WithTimeout(context.Background(), reconfigTimeout)
 	defer cancel()
 
 	// conn is a shared connection whose lifecycle is managed by the dbus package.
@@ -100,6 +98,8 @@ func dnsResolvedUp(config DNSConfig) error {
 		dbus.ObjectPath("/org/freedesktop/resolve1"),
 	)
 
+	// In principle, we could persist this in the manager struct
+	// if we knew that interface indices are persistent. This does not seem to be the case.
 	_, iface, err := interfaces.Tailscale()
 	if err != nil {
 		return fmt.Errorf("getting interface index: %w", err)
@@ -129,7 +129,7 @@ func dnsResolvedUp(config DNSConfig) error {
 		iface.Index, linkNameservers,
 	).Store()
 	if err != nil {
-		return fmt.Errorf("SetLinkDNS: %w", err)
+		return fmt.Errorf("setLinkDNS: %w", err)
 	}
 
 	var linkDomains = make([]resolvedLinkDomain, len(config.Domains))
@@ -145,15 +145,15 @@ func dnsResolvedUp(config DNSConfig) error {
 		iface.Index, linkDomains,
 	).Store()
 	if err != nil {
-		return fmt.Errorf("SetLinkDomains: %w", err)
+		return fmt.Errorf("setLinkDomains: %w", err)
 	}
 
 	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 is a shared connection whose lifecycle is managed by the dbus package.

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.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.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)

wgengine/router/router.go

@@ -11,6 +11,7 @@ import (
 	"github.com/tailscale/wireguard-go/tun"
 	"inet.af/netaddr"
 	"tailscale.com/types/logger"
+	"tailscale.com/wgengine/router/dns"
 )
 
 // Router is responsible for managing the system network stack.
@@ -40,6 +41,15 @@ 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,
+	}
+	dns := dns.NewManager(mconfig)
+	if err := dns.Down(); err != nil {
+		logf("dns down: %v", err)
+	}
 	cleanup(logf, interfaceName)
 }
 
@@ -72,7 +82,7 @@ type Config struct {
 	LocalAddrs []netaddr.IPPrefix
 	Routes     []netaddr.IPPrefix // routes to point into the Tailscale interface
 
-	DNSConfig
+	DNS dns.Config
 
 	// Linux-only things below, ignored on other platforms.
 

wgengine/router/router_darwin.go

@@ -14,10 +14,6 @@ func newUserspaceRouter(logf logger.Logf, wgdev *device.Device, tundev tun.Devic
 	return newUserspaceBSDRouter(logf, wgdev, tundev)
 }
 
-// TODO(dmytro): the following should use a macOS-specific method such as scutil.
+func cleanup(logger.Logf, string) {
-// This is currently not implemented. Editing /etc/resolv.conf does not work,
+	// Nothing to do.
-// as most applications use the system resolver, which disregards it.
+}
-
-func upDNS(DNSConfig, string) error { return nil }
-func downDNS(string) error          { return nil }
-func cleanup(logger.Logf, string)   {}

wgengine/router/router_default.go

@@ -16,6 +16,6 @@ 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
+	// Nothing to do here.
 }

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"
@@ -21,42 +19,7 @@ func newUserspaceRouter(logf logger.Logf, _ *device.Device, tundev tun.Device) (
 	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
-}
-
 func cleanup(logf logger.Logf, interfaceName string) {
-	if err := downDNS(interfaceName); err != nil {
-		logf("dns down: %v", err)
-	}
 	// 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"

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.
@@ -84,8 +85,7 @@ type linuxRouter struct {
 	snatSubnetRoutes bool
 	netfilterMode    NetfilterMode
 
-	dnsMode   dnsMode
+	dns *dns.Manager
-	dnsConfig DNSConfig
 
 	ipt4 netfilterRunner
 	cmd  commandRunner
@@ -109,6 +109,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,
@@ -116,6 +121,7 @@ func newUserspaceRouterAdvanced(logf logger.Logf, tunname string, netfilter netf
 		netfilterMode:   NetfilterOff,
 		ipt4:            netfilter,
 		cmd:             cmd,
+		dns:             dns.NewManager(mconfig),
 	}, nil
 }
 
@@ -133,26 +139,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
@@ -206,12 +198,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
@@ -855,68 +843,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 iptables.
-	// 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
 }
 
@@ -62,7 +70,10 @@ func (r *openbsdRouter) Set(cfg *Config) error {
 	}
 
 	// TODO: support configuring multiple local addrs on interface.
-	if len(cfg.LocalAddrs) != 1 {
+	if len(cfg.LocalAddrs) == 0 {
+		return nil
+	}
+	if len(cfg.LocalAddrs) > 1 {
 		return errors.New("freebsd doesn't support setting multiple local addrs yet")
 	}
 	localAddr := cfg.LocalAddrs[0]
@@ -155,26 +166,22 @@ 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 {
+	if err := r.dns.Down(); err != nil {
+		return fmt.Errorf("dns down: %v", err)
+	}
 	cleanup(r.logf, r.tunname)
 	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,19 +149,15 @@ 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 := downDNS(r.tunname); err != nil {
+	if err := r.dns.Down(); err != nil {
 		r.logf("dns down: %v", err)
 	}
 	// No interface cleanup is necessary during normal shutdown.

wgengine/router/router_windows.go

@@ -5,12 +5,14 @@
 package router
 
 import (
+	"fmt"
 	"log"
 
 	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 +21,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 +29,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
 }
 
@@ -55,10 +67,18 @@ func (r *winRouter) Set(cfg *Config) error {
 		r.logf("ConfigureInterface: %v\n", err)
 		return 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()
 	}
@@ -66,5 +86,5 @@ func (r *winRouter) Close() error {
 }
 
 func cleanup(logf logger.Logf, interfaceName string) {
-	// DNS is interface-bound, so nothing to do here.
+	// Nothing to do here.
 }

wgengine/tsdns/tsdns.go

@@ -40,6 +40,7 @@ var ErrClosed = errors.New("closed")
 var (
 	errAllFailed      = errors.New("all upstream nameservers failed")
 	errFullQueue      = errors.New("request queue full")
+	errNoNameservers  = errors.New("no upstream nameservers set")
 	errMapNotSet      = errors.New("domain map not set")
 	errNotImplemented = errors.New("query type not implemented")
 	errNotQuery       = errors.New("not a DNS query")
@@ -257,7 +258,7 @@ func (r *Resolver) delegate(query []byte) ([]byte, error) {
 	r.mu.RUnlock()
 
 	if len(nameservers) == 0 {
-		return nil, errAllFailed
+		return nil, errNoNameservers
 	}
 
 	ctx, cancel := context.WithTimeout(context.Background(), delegateTimeout)

wgengine/userspace.go

@@ -13,6 +13,7 @@ import (
 	"fmt"
 	"io"
 	"log"
+	"net"
 	"os"
 	"os/exec"
 	"runtime"
@@ -31,6 +32,7 @@ import (
 	"tailscale.com/internal/deepprint"
 	"tailscale.com/ipn/ipnstate"
 	"tailscale.com/net/interfaces"
+	"tailscale.com/net/tsaddr"
 	"tailscale.com/tailcfg"
 	"tailscale.com/types/key"
 	"tailscale.com/types/logger"
@@ -82,16 +84,15 @@ const (
 )
 
 type userspaceEngine struct {
-	logf            logger.Logf
+	logf      logger.Logf
-	reqCh           chan struct{}
+	reqCh     chan struct{}
-	waitCh          chan struct{} // chan is closed when first Close call completes; contrast with closing bool
+	waitCh    chan struct{} // chan is closed when first Close call completes; contrast with closing bool
-	tundev          *tstun.TUN
+	tundev    *tstun.TUN
-	wgdev           *device.Device
+	wgdev     *device.Device
-	router          router.Router
+	router    router.Router
-	resolver        *tsdns.Resolver
+	resolver  *tsdns.Resolver
-	useTailscaleDNS bool
+	magicConn *magicsock.Conn
-	magicConn       *magicsock.Conn
+	linkMon   *monitor.Mon
-	linkMon         *monitor.Mon
 
 	// localAddrs is the set of IP addresses assigned to the local
 	// tunnel interface. It's used to reflect local packets
@@ -131,13 +132,9 @@ type EngineConfig struct {
 	RouterGen RouterGen
 	// ListenPort is the port on which the engine will listen.
 	ListenPort uint16
-	// EchoRespondToAll determines whether ICMP Echo requests incoming from Tailscale peers
+	// Fake determines whether this engine is running in fake mode,
-	// will be intercepted and responded to, regardless of the source host.
+	// which disables such features as DNS configuration and unrestricted ICMP Echo responses.
-	EchoRespondToAll bool
+	Fake bool
-	// UseTailscaleDNS determines whether DNS requests for names of the form <mynode>.<mydomain>.<root>
-	// directed to the designated Taislcale DNS address (see wgengine/tsdns)
-	// will be intercepted and resolved by a tsdns.Resolver.
-	UseTailscaleDNS bool
 }
 
 type Loggify struct {
@@ -152,11 +149,11 @@ func (l *Loggify) Write(b []byte) (int, error) {
 func NewFakeUserspaceEngine(logf logger.Logf, listenPort uint16) (Engine, error) {
 	logf("Starting userspace wireguard engine (FAKE tuntap device).")
 	conf := EngineConfig{
-		Logf:             logf,
+		Logf:       logf,
-		TUN:              tstun.NewFakeTUN(),
+		TUN:        tstun.NewFakeTUN(),
-		RouterGen:        router.NewFake,
+		RouterGen:  router.NewFake,
-		ListenPort:       listenPort,
+		ListenPort: listenPort,
-		EchoRespondToAll: true,
+		Fake:       true,
 	}
 	return NewUserspaceEngineAdvanced(conf)
 }
@@ -183,8 +180,6 @@ func NewUserspaceEngine(logf logger.Logf, tunname string, listenPort uint16) (En
 		TUN:        tun,
 		RouterGen:  router.New,
 		ListenPort: listenPort,
-		// TODO(dmytro): plumb this down.
-		UseTailscaleDNS: true,
 	}
 
 	e, err := NewUserspaceEngineAdvanced(conf)
@@ -204,19 +199,18 @@ func newUserspaceEngineAdvanced(conf EngineConfig) (_ Engine, reterr error) {
 	logf := conf.Logf
 
 	e := &userspaceEngine{
-		logf:            logf,
+		logf:     logf,
-		reqCh:           make(chan struct{}, 1),
+		reqCh:    make(chan struct{}, 1),
-		waitCh:          make(chan struct{}),
+		waitCh:   make(chan struct{}),
-		tundev:          tstun.WrapTUN(logf, conf.TUN),
+		tundev:   tstun.WrapTUN(logf, conf.TUN),
-		resolver:        tsdns.NewResolver(logf, magicDNSDomain),
+		resolver: tsdns.NewResolver(logf, magicDNSDomain),
-		useTailscaleDNS: conf.UseTailscaleDNS,
+		pingers:  make(map[wgcfg.Key]*pinger),
-		pingers:         make(map[wgcfg.Key]*pinger),
 	}
 	e.localAddrs.Store(map[packet.IP]bool{})
 	e.linkState, _ = getLinkState()
 
 	// Respond to all pings only in fake mode.
-	if conf.EchoRespondToAll {
+	if conf.Fake {
 		e.tundev.PostFilterIn = echoRespondToAll
 	}
 	e.tundev.PreFilterOut = e.handleLocalPackets
@@ -376,11 +370,9 @@ func echoRespondToAll(p *packet.ParsedPacket, t *tstun.TUN) filter.Response {
 // tailscaled directly. Other packets are allowed to proceed into the
 // main ACL filter.
 func (e *userspaceEngine) handleLocalPackets(p *packet.ParsedPacket, t *tstun.TUN) filter.Response {
-	if e.useTailscaleDNS {
+	if verdict := e.handleDNS(p, t); verdict == filter.Drop {
-		if verdict := e.handleDNS(p, t); verdict == filter.Drop {
+		// local DNS handled the packet.
-			// local DNS handled the packet.
+		return filter.Drop
-			return filter.Drop
-		}
 	}
 
 	if runtime.GOOS == "darwin" && e.isLocalAddr(p.DstIP) {
@@ -824,13 +816,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 := deepprint.UpdateHash(&e.lastEngineSigFull, cfg)
 	routerChanged := deepprint.UpdateHash(&e.lastRouterSig, routerCfg)
 	if !engineChanged && !routerChanged {
@@ -852,6 +837,15 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config)
 	}
 
 	if routerChanged {
+		if routerCfg.DNS.Proxied {
+			ips := routerCfg.DNS.Nameservers
+			nameservers := make([]string, len(ips))
+			for i, ip := range ips {
+				nameservers[i] = net.JoinHostPort(ip.String(), "53")
+			}
+			e.resolver.SetNameservers(nameservers)
+			routerCfg.DNS.Nameservers = []netaddr.IP{tsaddr.TailscaleServiceIP()}
+		}
 		e.logf("wgengine: Reconfig: configuring router")
 		if err := e.router.Set(routerCfg); err != nil {
 			return err