net/dns: return error from NewOSManager, use it to initialize NM.

Signed-off-by: David Anderson <danderson@tailscale.com>

cmd/tailscaled/tailscaled.go

@@ -365,7 +365,11 @@ func tryEngine(logf logger.Logf, linkMon *monitor.Mon, name string) (e wgengine.
 			dev.Close()
 			return nil, false, err
 		}
-		conf.DNS = dns.NewOSConfigurator(logf, devName)
+		d, err := dns.NewOSConfigurator(logf, devName)
+		if err != nil {
+			return nil, false, err
+		}
+		conf.DNS = d
 		conf.Router = r
 		if wrapNetstack {
 			conf.Router = netstack.NewSubnetRouterWrapper(conf.Router)

cmd/tailscaled/tailscaled_windows.go

@@ -175,10 +175,16 @@ func startIPNServer(ctx context.Context, logid string) error {
 		if wrapNetstack {
 			r = netstack.NewSubnetRouterWrapper(r)
 		}
+		d, err := dns.NewOSConfigurator(logf, devName)
+		if err != nil {
+			r.Close()
+			dev.Close()
+			return nil, err
+		}
 		eng, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{
 			Tun:        dev,
 			Router:     r,
-			DNS:        dns.NewOSConfigurator(logf, devName),
+			DNS:        d,
 			ListenPort: 41641,
 		})
 		if err != nil {

net/dns/debian_resolvconf.go

@@ -53,7 +53,7 @@ type resolvconfManager struct {
 	scriptInstalled bool // libc update script has been installed
 }
 
-func newDebianResolvconfManager(logf logger.Logf) *resolvconfManager {
+func newDebianResolvconfManager(logf logger.Logf) (*resolvconfManager, error) {
 	ret := &resolvconfManager{
 		logf:            logf,
 		listRecordsPath: "/lib/resolvconf/list-records",
@@ -86,7 +86,7 @@ func newDebianResolvconfManager(logf logger.Logf) *resolvconfManager {
 		ret.interfacesDir = "/etc/resolvconf/run/interfaces"
 	}
 
-	return ret
+	return ret, nil
 }
 
 func (m *resolvconfManager) SetDNS(config OSConfig) error {

net/dns/direct.go

@@ -120,8 +120,8 @@ func isResolvedRunning() bool {
 // or as cleanup if the program terminates unexpectedly.
 type directManager struct{}
 
-func newDirectManager() directManager {
+func newDirectManager() (directManager, error) {
-	return directManager{}
+	return directManager{}, nil
 }
 
 // ownedByTailscale reports whether /etc/resolv.conf seems to be a

net/dns/manager.go

@@ -240,7 +240,11 @@ 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) {
-	oscfg := NewOSConfigurator(logf, interfaceName)
+	oscfg, err := NewOSConfigurator(logf, interfaceName)
+	if err != nil {
+		logf("creating dns cleanup: %v", err)
+		return
+	}
 	dns := NewManager(logf, oscfg, nil)
 	if err := dns.Down(); err != nil {
 		logf("dns down: %v", err)

net/dns/manager_default.go

@@ -8,7 +8,7 @@ package dns
 
 import "tailscale.com/types/logger"
 
-func NewOSConfigurator(logger.Logf, string) OSConfigurator {
+func NewOSConfigurator(logger.Logf, string) (OSConfigurator, error) {
 	// 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.

net/dns/manager_freebsd.go

@@ -6,7 +6,7 @@ package dns
 
 import "tailscale.com/types/logger"
 
-func NewOSConfigurator(logf logger.Logf, _ string) OSConfigurator {
+func NewOSConfigurator(logf logger.Logf, _ string) (OSConfigurator, error) {
 	switch {
 	case isResolvconfActive():
 		return newResolvconfManager(logf)

net/dns/manager_linux.go

@@ -6,7 +6,7 @@ package dns
 
 import "tailscale.com/types/logger"
 
-func NewOSConfigurator(logf logger.Logf, interfaceName string) OSConfigurator {
+func NewOSConfigurator(logf logger.Logf, interfaceName string) (OSConfigurator, error) {
 	switch {
 	case isResolvedActive():
 		return newResolvedManager(logf)

net/dns/manager_openbsd.go

@@ -6,6 +6,6 @@ package dns
 
 import "tailscale.com/types/logger"
 
-func NewOSConfigurator(logger.Logf, string) OSConfigurator {
+func NewOSConfigurator(logger.Logf, string) (OSConfigurator, error) {
 	return newDirectManager()
 }

net/dns/manager_windows.go

@@ -39,7 +39,7 @@ type windowsManager struct {
 	nrptWorks bool
 }
 
-func NewOSConfigurator(logf logger.Logf, interfaceName string) OSConfigurator {
+func NewOSConfigurator(logf logger.Logf, interfaceName string) (OSConfigurator, error) {
 	ret := windowsManager{
 		logf:      logf,
 		guid:      interfaceName,
@@ -56,7 +56,7 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) OSConfigurator {
 		ret.delKey(nrptBase)
 	}
 
-	return ret
+	return ret, nil
 }
 
 // keyOpenTimeout is how long we wait for a registry key to

net/dns/nm.go

@@ -71,42 +71,26 @@ func isNMActive() bool {
 // nmManager uses the NetworkManager DBus API.
 type nmManager struct {
 	interfaceName string
-	canSplit      bool
+	manager       dbus.BusObject
+	dnsManager    dbus.BusObject
 }
 
-func nmCanSplitDNS() bool {
+func newNMManager(interfaceName string) (*nmManager, error) {
 	conn, err := dbus.SystemBus()
 	if err != nil {
-		return false
+		return nil, err
 	}
 
-	var mode string
+	return &nmManager{
-	nm := conn.Object("org.freedesktop.NetworkManager", dbus.ObjectPath("/org/freedesktop/NetworkManager/DnsManager"))
-	v, err := nm.GetProperty("org.freedesktop.NetworkManager.DnsManager.Mode")
-	if err != nil {
-		return false
-	}
-	mode, ok := v.Value().(string)
-	if !ok {
-		return false
-	}
-
-	// Per NM's documentation, it only does split-DNS when it's
-	// programming dnsmasq or systemd-resolved. All other modes are
-	// primary-only.
-	return mode == "dnsmasq" || mode == "systemd-resolved"
-}
-
-func newNMManager(interfaceName string) nmManager {
-	return nmManager{
 		interfaceName: interfaceName,
-		canSplit:      nmCanSplitDNS(),
+		manager:       conn.Object("org.freedesktop.NetworkManager", dbus.ObjectPath("/org/freedesktop/NetworkManager")),
-	}
+		dnsManager:    conn.Object("org.freedesktop.NetworkManager", dbus.ObjectPath("/org/freedesktop/NetworkManager/DnsManager")),
+	}, nil
 }
 
 type nmConnectionSettings map[string]map[string]dbus.Variant
 
-func (m nmManager) SetDNS(config OSConfig) error {
+func (m *nmManager) SetDNS(config OSConfig) error {
 	ctx, cancel := context.WithTimeout(context.Background(), reconfigTimeout)
 	defer cancel()
 
@@ -127,7 +111,7 @@ func (m nmManager) SetDNS(config OSConfig) error {
 	return err
 }
 
-func (m nmManager) trySet(ctx context.Context, config OSConfig) error {
+func (m *nmManager) trySet(ctx context.Context, config OSConfig) error {
 	conn, err := dbus.SystemBus()
 	if err != nil {
 		return fmt.Errorf("connecting to system bus: %w", err)
@@ -262,9 +246,24 @@ func (m nmManager) trySet(ctx context.Context, config OSConfig) error {
 	return nil
 }
 
-func (m nmManager) SupportsSplitDNS() bool { return m.canSplit }
+func (m *nmManager) SupportsSplitDNS() bool {
+	var mode string
+	v, err := m.dnsManager.GetProperty("org.freedesktop.NetworkManager.DnsManager.Mode")
+	if err != nil {
+		return false
+	}
+	mode, ok := v.Value().(string)
+	if !ok {
+		return false
+	}
 
-func (m nmManager) GetBaseConfig() (OSConfig, error) {
+	// Per NM's documentation, it only does split-DNS when it's
+	// programming dnsmasq or systemd-resolved. All other modes are
+	// primary-only.
+	return mode == "dnsmasq" || mode == "systemd-resolved"
+}
+
+func (m *nmManager) GetBaseConfig() (OSConfig, error) {
 	conn, err := dbus.SystemBus()
 	if err != nil {
 		return OSConfig{}, err
@@ -362,7 +361,7 @@ func (m nmManager) GetBaseConfig() (OSConfig, error) {
 	return ret, nil
 }
 
-func (m nmManager) Close() error {
+func (m *nmManager) Close() error {
 	// No need to do anything on close, NetworkManager will delete our
 	// settings when the tailscale interface goes away.
 	return nil

net/dns/noop.go

@@ -13,6 +13,6 @@ func (m noopManager) GetBaseConfig() (OSConfig, error) {
 	return OSConfig{}, ErrGetBaseConfigNotSupported
 }
 
-func NewNoopManager() noopManager {
+func NewNoopManager() (noopManager, error) {
-	return noopManager{}
+	return noopManager{}, nil
 }

net/dns/openresolv.go

@@ -15,8 +15,8 @@ import (
 // implementation of the `resolvconf` program.
 type openresolvManager struct{}
 
-func newOpenresolvManager() openresolvManager {
+func newOpenresolvManager() (openresolvManager, error) {
-	return openresolvManager{}
+	return openresolvManager{}, nil
 }
 
 func (m openresolvManager) SetDNS(config OSConfig) error {

net/dns/resolvconf.go

@@ -42,7 +42,7 @@ func isResolvconfActive() bool {
 	return false
 }
 
-func newResolvconfManager(logf logger.Logf) OSConfigurator {
+func newResolvconfManager(logf logger.Logf) (OSConfigurator, error) {
 	_, err := exec.Command("resolvconf", "--version").CombinedOutput()
 	if err != nil {
 		if exitErr, ok := err.(*exec.ExitError); ok && exitErr.ExitCode() == 99 {

net/dns/resolved.go

@@ -87,10 +87,10 @@ type resolvedManager struct {
 	logf logger.Logf
 }
 
-func newResolvedManager(logf logger.Logf) resolvedManager {
+func newResolvedManager(logf logger.Logf) (resolvedManager, error) {
 	return resolvedManager{
 		logf: logf,
-	}
+	}, nil
 }
 
 // Up implements managerImpl.

wgengine/userspace.go

@@ -219,7 +219,11 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error)
 	}
 	if conf.DNS == nil {
 		logf("[v1] using fake (no-op) DNS configurator")
-		conf.DNS = dns.NewNoopManager()
+		d, err := dns.NewNoopManager()
+		if err != nil {
+			return nil, err
+		}
+		conf.DNS = d
 	}
 
 	tsTUNDev := tstun.Wrap(logf, conf.Tun)