From 4c61ebacf498a65f44d6389560e52fe1ca56ec4c Mon Sep 17 00:00:00 2001 From: David Anderson Date: Fri, 2 Apr 2021 19:24:02 -0700 Subject: [PATCH] wgengine: move DNS configuration out of wgengine/router. Signed-off-by: David Anderson --- cmd/tailscaled/tailscaled.go | 7 +++++++ internal/deepprint/deepprint_test.go | 7 +++---- net/dns/manager.go | 7 ++++--- net/dns/manager_default.go | 4 ++-- net/dns/manager_freebsd.go | 2 +- net/dns/manager_linux.go | 2 +- net/dns/manager_openbsd.go | 2 +- net/dns/manager_windows.go | 2 +- net/dns/noop.go | 4 +--- wgengine/router/router.go | 5 ----- wgengine/router/router_linux.go | 11 ----------- wgengine/router/router_openbsd.go | 11 ----------- wgengine/router/router_userspace_bsd.go | 13 ------------- wgengine/router/router_windows.go | 9 --------- wgengine/userspace.go | 23 +++++++++++++++++------ 15 files changed, 38 insertions(+), 71 deletions(-) diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index db2f37a6a..86669d383 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -350,6 +350,13 @@ func tryEngine(logf logger.Logf, linkMon *monitor.Mon, name string) (e wgengine. return nil, false, err } conf.Router = r + tunname, err := dev.Name() + if err != nil { + r.Close() + dev.Close() + return nil, false, err + } + conf.DNS = dns.NewOSConfigurator(logf, tunname) } e, err = wgengine.NewUserspaceEngine(logf, conf) if err != nil { diff --git a/internal/deepprint/deepprint_test.go b/internal/deepprint/deepprint_test.go index 01ecb43ad..d7576f27a 100644 --- a/internal/deepprint/deepprint_test.go +++ b/internal/deepprint/deepprint_test.go @@ -9,7 +9,6 @@ import ( "testing" "inet.af/netaddr" - "tailscale.com/net/dns" "tailscale.com/wgengine/router" "tailscale.com/wgengine/wgcfg" ) @@ -45,9 +44,9 @@ func getVal() []interface{} { }, }, &router.Config{ - DNS: dns.OSConfig{ - Nameservers: []netaddr.IP{netaddr.IPv4(8, 8, 8, 8)}, - Domains: []string{"tailscale.net"}, + Routes: []netaddr.IPPrefix{ + netaddr.MustParseIPPrefix("1.2.3.0/24"), + netaddr.MustParseIPPrefix("1234::/64"), }, }, map[string]string{ diff --git a/net/dns/manager.go b/net/dns/manager.go index d47c16819..7afc19f4c 100644 --- a/net/dns/manager.go +++ b/net/dns/manager.go @@ -33,11 +33,11 @@ type Manager struct { } // NewManagers created a new manager from the given config. -func NewManager(logf logger.Logf, interfaceName string) *Manager { +func NewManager(logf logger.Logf, oscfg OSConfigurator) *Manager { logf = logger.WithPrefix(logf, "dns: ") m := &Manager{ logf: logf, - impl: newManager(logf, interfaceName), + impl: oscfg, } m.logf("using %T", m.impl) @@ -81,7 +81,8 @@ func (m *Manager) Down() error { // 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) { - dns := NewManager(logf, interfaceName) + oscfg := NewOSConfigurator(logf, interfaceName) + dns := NewManager(logf, oscfg) if err := dns.Down(); err != nil { logf("dns down: %v", err) } diff --git a/net/dns/manager_default.go b/net/dns/manager_default.go index 732e3cb73..f7e7e3e5e 100644 --- a/net/dns/manager_default.go +++ b/net/dns/manager_default.go @@ -8,9 +8,9 @@ package dns import "tailscale.com/types/logger" -func newManager(logger.Logf, string) OSConfigurator { +func NewOSConfigurator(logger.Logf, string) OSConfigurator { // 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() + return NewNoopManager() } diff --git a/net/dns/manager_freebsd.go b/net/dns/manager_freebsd.go index 5b2d12195..1485c6728 100644 --- a/net/dns/manager_freebsd.go +++ b/net/dns/manager_freebsd.go @@ -6,7 +6,7 @@ package dns import "tailscale.com/types/logger" -func newManager(logf logger.Logf, _ string) OSConfigurator { +func NewOSConfigurator(logf logger.Logf, _ string) OSConfigurator { switch { case isResolvconfActive(): return newResolvconfManager(logf) diff --git a/net/dns/manager_linux.go b/net/dns/manager_linux.go index 95ef9b906..727b24e30 100644 --- a/net/dns/manager_linux.go +++ b/net/dns/manager_linux.go @@ -6,7 +6,7 @@ package dns import "tailscale.com/types/logger" -func newManager(logf logger.Logf, interfaceName string) OSConfigurator { +func NewOSConfigurator(logf logger.Logf, interfaceName string) OSConfigurator { switch { // TODO: rework NetworkManager and resolved support. // case isResolvedActive(): diff --git a/net/dns/manager_openbsd.go b/net/dns/manager_openbsd.go index 5289c3f9e..0abddd2f7 100644 --- a/net/dns/manager_openbsd.go +++ b/net/dns/manager_openbsd.go @@ -6,6 +6,6 @@ package dns import "tailscale.com/types/logger" -func newManager(logger.Logf, string) OSConfigurator { +func NewOSConfigurator(logger.Logf, string) OSConfigurator { return newDirectManager() } diff --git a/net/dns/manager_windows.go b/net/dns/manager_windows.go index 4cd12f328..ed11f4604 100644 --- a/net/dns/manager_windows.go +++ b/net/dns/manager_windows.go @@ -25,7 +25,7 @@ type windowsManager struct { guid string } -func newManager(logf logger.Logf, interfaceName string) OSConfigurator { +func NewOSConfigurator(logf logger.Logf, interfaceName string) OSConfigurator { return windowsManager{ logf: logf, guid: interfaceName, diff --git a/net/dns/noop.go b/net/dns/noop.go index 3b102c7fc..98c38d9ed 100644 --- a/net/dns/noop.go +++ b/net/dns/noop.go @@ -2,8 +2,6 @@ // 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 type noopManager struct{} @@ -12,6 +10,6 @@ func (m noopManager) Set(OSConfig) error { return nil } func (m noopManager) RoutingMode() RoutingMode { return RoutingModeNone } func (m noopManager) Close() error { return nil } -func newNoopManager() noopManager { +func NewNoopManager() noopManager { return noopManager{} } diff --git a/wgengine/router/router.go b/wgengine/router/router.go index 6fe63212e..11fce3302 100644 --- a/wgengine/router/router.go +++ b/wgengine/router/router.go @@ -9,7 +9,6 @@ package router import ( "github.com/tailscale/wireguard-go/tun" "inet.af/netaddr" - "tailscale.com/net/dns" "tailscale.com/types/logger" "tailscale.com/types/preftype" ) @@ -58,11 +57,7 @@ type Config struct { // this node has chosen to use. Routes []netaddr.IPPrefix - // Set internally by wgengine, must not be set elsewhere. - DNS dns.OSConfig - // Linux-only things below, ignored on other platforms. - SubnetRoutes []netaddr.IPPrefix // subnets being advertised to other Tailscale nodes SNATSubnetRoutes bool // SNAT traffic to local subnets NetfilterMode preftype.NetfilterMode // how much to manage netfilter rules diff --git a/wgengine/router/router_linux.go b/wgengine/router/router_linux.go index ea4132b65..334f3fd57 100644 --- a/wgengine/router/router_linux.go +++ b/wgengine/router/router_linux.go @@ -18,7 +18,6 @@ import ( "github.com/go-multierror/multierror" "github.com/tailscale/wireguard-go/tun" "inet.af/netaddr" - "tailscale.com/net/dns" "tailscale.com/net/tsaddr" "tailscale.com/types/logger" "tailscale.com/types/preftype" @@ -102,8 +101,6 @@ type linuxRouter struct { v6Available bool v6NATAvailable bool - dns *dns.Manager - ipt4 netfilterRunner ipt6 netfilterRunner cmd commandRunner @@ -158,7 +155,6 @@ func newUserspaceRouterAdvanced(logf logger.Logf, tunname string, netfilter4, ne ipt4: netfilter4, ipt6: netfilter6, cmd: cmd, - dns: dns.NewManager(logf, tunname), }, nil } @@ -180,9 +176,6 @@ func (r *linuxRouter) Up() error { } func (r *linuxRouter) Close() error { - if err := r.dns.Down(); err != nil { - return fmt.Errorf("dns down: %w", err) - } if err := r.downInterface(); err != nil { return err } @@ -206,10 +199,6 @@ func (r *linuxRouter) Set(cfg *Config) error { cfg = &shutdownConfig } - if err := r.dns.Set(cfg.DNS); err != nil { - errs = append(errs, fmt.Errorf("dns set: %w", err)) - } - if err := r.setNetfilterMode(cfg.NetfilterMode); err != nil { errs = append(errs, err) } diff --git a/wgengine/router/router_openbsd.go b/wgengine/router/router_openbsd.go index 68ea13fcf..dc88d08cc 100644 --- a/wgengine/router/router_openbsd.go +++ b/wgengine/router/router_openbsd.go @@ -12,7 +12,6 @@ import ( "github.com/tailscale/wireguard-go/tun" "inet.af/netaddr" - "tailscale.com/net/dns" "tailscale.com/types/logger" ) @@ -26,8 +25,6 @@ type openbsdRouter struct { local4 netaddr.IPPrefix local6 netaddr.IPPrefix routes map[netaddr.IPPrefix]struct{} - - dns *dns.Manager } func newUserspaceRouter(logf logger.Logf, tundev tun.Device) (Router, error) { @@ -39,7 +36,6 @@ func newUserspaceRouter(logf logger.Logf, tundev tun.Device) (Router, error) { return &openbsdRouter{ logf: logf, tunname: tunname, - dns: dns.NewManager(logf, tunname), }, nil } @@ -210,17 +206,10 @@ func (r *openbsdRouter) Set(cfg *Config) error { r.local6 = localAddr6 r.routes = newRoutes - if err := r.dns.Set(cfg.DNS); err != nil { - errq = fmt.Errorf("dns set: %v", err) - } - 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 } diff --git a/wgengine/router/router_userspace_bsd.go b/wgengine/router/router_userspace_bsd.go index 9d69473c4..6e3c35a77 100644 --- a/wgengine/router/router_userspace_bsd.go +++ b/wgengine/router/router_userspace_bsd.go @@ -14,7 +14,6 @@ import ( "github.com/tailscale/wireguard-go/tun" "inet.af/netaddr" - "tailscale.com/net/dns" "tailscale.com/types/logger" "tailscale.com/version" ) @@ -24,8 +23,6 @@ type userspaceBSDRouter struct { tunname string local []netaddr.IPPrefix routes map[netaddr.IPPrefix]struct{} - - dns *dns.Manager } func newUserspaceBSDRouter(logf logger.Logf, tundev tun.Device) (Router, error) { @@ -37,7 +34,6 @@ func newUserspaceBSDRouter(logf logger.Logf, tundev tun.Device) (Router, error) return &userspaceBSDRouter{ logf: logf, tunname: tunname, - dns: dns.NewManager(logf, tunname), }, nil } @@ -183,18 +179,9 @@ func (r *userspaceBSDRouter) Set(cfg *Config) (reterr error) { } r.routes = newRoutes - if err := r.dns.Set(cfg.DNS); err != nil { - r.logf("DNS set: %v", err) - setErr(err) - } - return errq } func (r *userspaceBSDRouter) Close() error { - if err := r.dns.Down(); err != nil { - r.logf("dns down: %v", err) - } - // No interface cleanup is necessary during normal shutdown. return nil } diff --git a/wgengine/router/router_windows.go b/wgengine/router/router_windows.go index 2a1aa2729..42c81590b 100644 --- a/wgengine/router/router_windows.go +++ b/wgengine/router/router_windows.go @@ -29,7 +29,6 @@ type winRouter struct { logf func(fmt string, args ...interface{}) nativeTun *tun.NativeTun routeChangeCallback *winipcfg.RouteChangeCallback - dns *dns.Manager firewall *firewallTweaker // firewallSubproc is a subprocess that runs a tweaked version of @@ -53,7 +52,6 @@ func newUserspaceRouter(logf logger.Logf, tundev tun.Device) (Router, error) { return &winRouter{ logf: logf, nativeTun: nativeTun, - dns: dns.NewManager(logf, guid.String()), firewall: &firewallTweaker{ logf: logger.WithPrefix(logf, "firewall: "), tunGUID: *guid, @@ -92,10 +90,6 @@ func (r *winRouter) Set(cfg *Config) error { return err } - if err := r.dns.Set(cfg.DNS); err != nil { - return fmt.Errorf("dns set: %w", err) - } - // Flush DNS on router config change to clear cached DNS entries (solves #1430) if err := dns.Flush(); err != nil { r.logf("flushdns error: %v", err) @@ -116,9 +110,6 @@ func hasDefaultRoute(routes []netaddr.IPPrefix) bool { func (r *winRouter) Close() error { r.firewall.clear() - if err := r.dns.Down(); err != nil { - return fmt.Errorf("dns down: %w", err) - } if r.routeChangeCallback != nil { r.routeChangeCallback.Unregister() } diff --git a/wgengine/userspace.go b/wgengine/userspace.go index bf85d2d59..987ab04c8 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -84,6 +84,7 @@ type userspaceEngine struct { tundev *tstun.Wrapper wgdev *device.Device router router.Router + dns *dns.Manager resolver *resolver.Resolver magicConn *magicsock.Conn linkMon *monitor.Mon @@ -143,6 +144,10 @@ type Config struct { // If nil, a fake Router that does nothing is used. Router router.Router + // DNS interfaces the Engine to the OS DNS resolver configuration. + // If nil, a fake OSConfigurator that does nothing is used. + DNS dns.OSConfigurator + // LinkMonitor optionally provides an existing link monitor to re-use. // If nil, a new link monitor is created. LinkMonitor *monitor.Mon @@ -193,6 +198,10 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) logf("[v1] using fake (no-op) OS network configurator") conf.Router = router.NewFake(logf) } + if conf.DNS == nil { + logf("[v1] using fake (no-op) DNS configurator") + conf.DNS = dns.NewNoopManager() + } tsTUNDev := tstun.Wrap(logf, conf.Tun) closePool.add(tsTUNDev) @@ -204,6 +213,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) waitCh: make(chan struct{}), tundev: tsTUNDev, router: conf.Router, + dns: dns.NewManager(logf, conf.DNS), pingers: make(map[wgkey.Key]*pinger), } e.isLocalAddr.Store(genLocalAddrFunc(nil)) @@ -990,25 +1000,26 @@ func (e *userspaceEngine) Reconfig(cfg *wgcfg.Config, routerCfg *router.Config, LocalDomains: dnsCfg.AuthoritativeSuffixes, Routes: map[string][]netaddr.IPPort{}, } + osCfg := dns.OSConfig{ + Domains: dnsCfg.SearchDomains, + } // We must proxy through quad-100 if MagicDNS hosts are in // use, or there are any per-domain routes. mustProxy := len(dnsCfg.Hosts) > 0 || len(dnsCfg.Routes) > 0 - routerCfg.DNS = dns.OSConfig{ - Domains: dnsCfg.SearchDomains, - } if mustProxy { - routerCfg.DNS.Nameservers = []netaddr.IP{tsaddr.TailscaleServiceIP()} + osCfg.Nameservers = []netaddr.IP{tsaddr.TailscaleServiceIP()} resolverCfg.Routes["."] = dnsCfg.DefaultResolvers for suffix, resolvers := range dnsCfg.Routes { resolverCfg.Routes[suffix] = resolvers } } else { for _, resolver := range dnsCfg.DefaultResolvers { - routerCfg.DNS.Nameservers = append(routerCfg.DNS.Nameservers, resolver.IP) + osCfg.Nameservers = append(osCfg.Nameservers, resolver.IP) } } - routerCfg.DNS.Domains = dnsCfg.SearchDomains + osCfg.Domains = dnsCfg.SearchDomains e.resolver.SetConfig(resolverCfg) // TODO: check error and propagate to health pkg + e.dns.Set(osCfg) // TODO: check error and propagate to health pkg e.logf("wgengine: Reconfig: configuring router") err := e.router.Set(routerCfg) health.SetRouterHealth(err)