diff --git a/control/controlclient/netmap.go b/control/controlclient/netmap.go index 917f87463..d6abaed72 100644 --- a/control/controlclient/netmap.go +++ b/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 { - wgcfg, err := nm.WGCfg(log.Printf, uflags, dnsOverride) +func (nm *NetworkMap) UAPI(uflags int) string { + 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) { - s := nm._WireGuardConfig(logf, uflags, dnsOverride, true) +func (nm *NetworkMap) WGCfg(logf logger.Logf, uflags int) (*wgcfg.Config, error) { + 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 { diff --git a/internal/deepprint/deepprint_test.go b/internal/deepprint/deepprint_test.go index d13aff8e5..d88541f67 100644 --- a/internal/deepprint/deepprint_test.go +++ b/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"}, }, diff --git a/ipn/local.go b/ipn/local.go index 3b9bef3d1..9acd988b7 100644 --- a/ipn/local.go +++ b/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 - dom := nm.DNSDomains - if !uc.CorpDNS { - dns = []wgcfg.IP{} - dom = []string{} - } - cfg, err := nm.WGCfg(b.logf, uflags, dns) + cfg, err := nm.WGCfg(b.logf, uflags) 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. - // TODO(dmytro): make this configurable. + // The Magic DNS IP. + // 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, diff --git a/wgengine/router/dns/config.go b/wgengine/router/dns/config.go index 271d4fc6d..84c798a29 100644 --- a/wgengine/router/dns/config.go +++ b/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. -type DNSConfig struct { +// Config is the set of parameters which uniquely determine +// 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. -type dnsMode uint8 +// 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 -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 -) + // InterfaceName is the name of the interface whose DNS settings should be managed. + InterfaceName string -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 "???" - } + // Cleanup indicates that this manager is created for cleanup only. + // A no-op manager will be instantiated if no cleanup is needed. + 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 } diff --git a/wgengine/router/dns/direct.go b/wgengine/router/dns/direct.go index ffcc06295..3cc012bce 100644 --- a/wgengine/router/dns/direct.go +++ b/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) { - var config DNSConfig +func readResolvConf() (Config, error) { + 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 -// from the given configuration, creating a backup of its old state. +// directManager is a managerImpl which replaces /etc/resolv.conf with a file +// 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 -// and ensure that router.Cleanup is run if the program terminates unexpectedly. -func dnsDirectUp(config DNSConfig) error { +// The caller must call Down before program shutdown +// or as cleanup if the program terminates unexpectedly. +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. -// It is idempotent and behaves correctly even if dnsDirectUp has never been run. -func dnsDirectDown() error { +// Down implements managerImpl. +func (m *directManager) Down() error { if _, err := os.Stat(backupConf); err != nil { // If the backup file does not exist, then Up never ran successfully. if os.IsNotExist(err) { diff --git a/wgengine/router/dns/manager.go b/wgengine/router/dns/manager.go index f57494378..8c5d4b4d5 100644 --- a/wgengine/router/dns/manager.go +++ b/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 - Get() Config - Reset() error + // 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 { - impl managerImpl - oldConfig Config + logf logger.Logf + + impl managerImpl + + config Config + mconfig ManagerConfig } -func NewManager(logf logger.Logf, interfaceName string) *Manager { - return &Manager{ - impl: newManager(logf, interfaceName), +// NewManagers created a new manager from the given config. +func NewManager(mconfig ManagerConfig) *Manager { + 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 { - if len(config.Nameservers) == 0 { - return m.impl.Down() - } - - if config.EquivalentTo(m.oldConfig) { +func (m *Manager) Set(config Config) error { + if config.EquivalentTo(m.config) { 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() } diff --git a/wgengine/router/dns/manager_default.go b/wgengine/router/dns/manager_default.go new file mode 100644 index 000000000..361bcbb17 --- /dev/null +++ b/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) +} diff --git a/wgengine/router/dns/manager_freebsd.go b/wgengine/router/dns/manager_freebsd.go new file mode 100644 index 000000000..4017af058 --- /dev/null +++ b/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) + } +} diff --git a/wgengine/router/dns/manager_linux.go b/wgengine/router/dns/manager_linux.go new file mode 100644 index 000000000..051205fad --- /dev/null +++ b/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) + } +} diff --git a/wgengine/router/dns/manager_openbsd.go b/wgengine/router/dns/manager_openbsd.go new file mode 100644 index 000000000..228e3cca5 --- /dev/null +++ b/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) +} diff --git a/wgengine/router/dns/nm.go b/wgengine/router/dns/nm.go index 77a91b842..eb2245aa9 100644 --- a/wgengine/router/dns/nm.go +++ b/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 -// through the NetworkManager DBus API. -func dnsNetworkManagerUp(config DNSConfig, interfaceName string) error { - ctx, cancel := context.WithTimeout(context.Background(), dnsReconfigTimeout) +type nmManager struct { + interfaceName string +} + +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 - // over other interfaces, except those exploiting this same trick. - // Ref: https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1211110/comments/92. - ipv4Map["dns-priority"] = dbus.MakeVariant(-1) - // In principle, we should not need set this to true, - // as our interface does not configure any automatic DNS settings (presumably via DHCP). - // All the same, better to be safe. - ipv4Map["ignore-auto-dns"] = dbus.MakeVariant(true) + if !config.PerDomain { + // dns-priority = -1 ensures that we have priority + // over other interfaces, except those exploiting this same trick. + // Ref: https://bugs.launchpad.net/ubuntu/+source/network-manager/+bug/1211110/comments/92. + ipv4Map["dns-priority"] = dbus.MakeVariant(-1) + // In principle, we should not need set this to true, + // as our interface does not configure any automatic DNS settings (presumably via DHCP). + // 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) - ipv6Map["ignore-auto-dns"] = dbus.MakeVariant(true) + if !config.PerDomain { + 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. -func dnsNetworkManagerDown(interfaceName string) error { - return dnsNetworkManagerUp(DNSConfig{Nameservers: nil, Domains: nil}, interfaceName) +// Down implements managerImpl. +func (m *nmManager) Down() error { + return m.Up(Config{Nameservers: nil, Domains: nil}) } diff --git a/wgengine/router/dns/noop.go b/wgengine/router/dns/noop.go new file mode 100644 index 000000000..e41621e0f --- /dev/null +++ b/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) +} diff --git a/wgengine/router/dns/resolvconf.go b/wgengine/router/dns/resolvconf.go index 204147ce9..dec517e6a 100644 --- a/wgengine/router/dns/resolvconf.go +++ b/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 -// the given DNS configuration the Tailscale interface. -func dnsResolvconfUp(config DNSConfig, interfaceName string) error { - stdin := new(bytes.Buffer) - dnsWriteConfig(stdin, config.Nameservers, config.Domains) // dns_direct.go +type resolvconfManager struct { + interfaceName string +} - 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. -func dnsResolvconfDown(interfaceName string) error { - cmd := exec.Command("resolvconf", "-f", "-d", interfaceName+".inet") +// Down implements managerImpl. +func (m *resolvconfManager) Down() error { + cmd := exec.Command("resolvconf", "-f", "-d", m.interfaceName+".inet") out, err := cmd.CombinedOutput() if err != nil { return fmt.Errorf("running %s: %s", cmd, out) diff --git a/wgengine/router/dns/resolved.go b/wgengine/router/dns/resolved.go index 51339e079..41be1dfb5 100644 --- a/wgengine/router/dns/resolved.go +++ b/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 -// to given nameservers and search domains using the resolved DBus API. -func dnsResolvedUp(config DNSConfig) error { - ctx, cancel := context.WithTimeout(context.Background(), dnsReconfigTimeout) +type resolvedManager struct{} + +func newResolvedManager(mconfig ManagerConfig) managerImpl { + 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. -func dnsResolvedDown() error { - ctx, cancel := context.WithTimeout(context.Background(), dnsReconfigTimeout) +// Down implements managerImpl. +func (m *resolvedManager) Down() error { + ctx, cancel := context.WithTimeout(context.Background(), reconfigTimeout) defer cancel() conn, err := dbus.SystemBus() diff --git a/wgengine/router/ifconfig_windows.go b/wgengine/router/ifconfig_windows.go index 1c79cd8f2..01dfc2196 100644 --- a/wgengine/router/ifconfig_windows.go +++ b/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) diff --git a/wgengine/router/router.go b/wgengine/router/router.go index 79ea365cf..0aca97a1b 100644 --- a/wgengine/router/router.go +++ b/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) } diff --git a/wgengine/router/router_darwin.go b/wgengine/router/router_darwin.go index 816cae53a..7dab1cf91 100644 --- a/wgengine/router/router_darwin.go +++ b/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 -} diff --git a/wgengine/router/router_default.go b/wgengine/router/router_default.go index db170fba0..697bea51d 100644 --- a/wgengine/router/router_default.go +++ b/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 { - return nil -} +func cleanup(logf logger.Logf, interfaceName string) {} diff --git a/wgengine/router/router_freebsd.go b/wgengine/router/router_freebsd.go index 9fd8e1f41..e7113223f 100644 --- a/wgengine/router/router_freebsd.go +++ b/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 -} diff --git a/wgengine/router/router_linux.go b/wgengine/router/router_linux.go index 2a6154fd7..d1d319eca 100644 --- a/wgengine/router/router_linux.go +++ b/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 { - return err + if err := r.dns.Down(); err != nil { + 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.upDNS(cfg.DNSConfig); err != nil { - r.logf("dns up: %v", err) - } else { - r.dnsConfig = cfg.DNSConfig - } + if err := r.dns.Set(cfg.DNS); err != nil { + return fmt.Errorf("dns set: %v", err) } 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, - // 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) - } - } + // TODO(dmytro): clean up iptable chains. } diff --git a/wgengine/router/router_openbsd.go b/wgengine/router/router_openbsd.go index 574b2804a..2fb6a4189 100644 --- a/wgengine/router/router_openbsd.go +++ b/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 := dnsDirectUp(cfg.DNSConfig); err != nil { - errq = fmt.Errorf("dns up: direct: %v", err) - } else { - r.dnsConfig = cfg.DNSConfig - } + if err := r.dns.Set(cfg.DNS); err != nil { + errq = fmt.Errorf("dns set: %v", err) } 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) diff --git a/wgengine/router/router_userspace_bsd.go b/wgengine/router/router_userspace_bsd.go index 5a8655f12..16876716d 100644 --- a/wgengine/router/router_userspace_bsd.go +++ b/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 := upDNS(cfg.DNSConfig, r.tunname); err != nil { - errq = fmt.Errorf("dns up: %v", err) - } else { - r.dnsConfig = cfg.DNSConfig - } + if err := r.dns.Set(cfg.DNS); err != nil { + errq = fmt.Errorf("dns set: %v", err) } 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) diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 25a0797e9..d460a3527 100644 --- a/wgengine/userspace.go +++ b/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 {