net/dns: fall back to copy+delete/truncate if moving to/from /etc/resolv.conf fails.

In some containers, /etc/resolv.conf is a bind-mount from outside the container.
This prevents renaming to or from /etc/resolv.conf, because it's on a different
filesystem from linux's perspective. It also prevents removing /etc/resolv.conf,
because doing so would break the bind-mount.

If we find ourselves within this environment, fall back to using copy+delete when
renaming to /etc/resolv.conf, and copy+truncate when renaming from /etc/resolv.conf.

Fixes #3000

Co-authored-by: Denton Gentry <dgentry@tailscale.com>
Signed-off-by: David Anderson <danderson@tailscale.com>

net/dns/direct.go

@@ -18,6 +18,7 @@ import (
 	"strings"
 
 	"inet.af/netaddr"
+	"tailscale.com/types/logger"
 	"tailscale.com/util/dnsname"
 )
 
@@ -133,18 +134,36 @@ func isResolvedRunning() bool {
 // The caller must call Down before program shutdown
 // or as cleanup if the program terminates unexpectedly.
 type directManager struct {
-	fs wholeFileFS
+	logf logger.Logf
+	fs   wholeFileFS
+	// renameBroken is set if fs.Rename to or from /etc/resolv.conf
+	// fails. This can happen in some container runtimes, where
+	// /etc/resolv.conf is bind-mounted from outside the container,
+	// and therefore /etc and /etc/resolv.conf are different
+	// filesystems as far as rename(2) is concerned.
+	//
+	// In those situations, we fall back to emulating rename with file
+	// copies and truncations, which is not as good (opens up a race
+	// where a reader can see an empty or partial /etc/resolv.conf),
+	// but is better than having non-functioning DNS.
+	renameBroken bool
 }
 
-func newDirectManager() directManager {
-	return directManager{fs: directFS{}}
+func newDirectManager(logf logger.Logf) *directManager {
+	return &directManager{
+		logf: logf,
+		fs:   directFS{},
+	}
 }
 
-func newDirectManagerOnFS(fs wholeFileFS) directManager {
-	return directManager{fs: fs}
+func newDirectManagerOnFS(logf logger.Logf, fs wholeFileFS) *directManager {
+	return &directManager{
+		logf: logf,
+		fs:   fs,
+	}
 }
 
-func (m directManager) readResolvFile(path string) (OSConfig, error) {
+func (m *directManager) readResolvFile(path string) (OSConfig, error) {
 	b, err := m.fs.ReadFile(path)
 	if err != nil {
 		return OSConfig{}, err
@@ -154,7 +173,7 @@ func (m directManager) readResolvFile(path string) (OSConfig, error) {
 
 // ownedByTailscale reports whether /etc/resolv.conf seems to be a
 // tailscale-managed file.
-func (m directManager) ownedByTailscale() (bool, error) {
+func (m *directManager) ownedByTailscale() (bool, error) {
 	isRegular, err := m.fs.Stat(resolvConf)
 	if err != nil {
 		if os.IsNotExist(err) {
@@ -177,7 +196,7 @@ func (m directManager) ownedByTailscale() (bool, error) {
 
 // backupConfig creates or updates a backup of /etc/resolv.conf, if
 // resolv.conf does not currently contain a Tailscale-managed config.
-func (m directManager) backupConfig() error {
+func (m *directManager) backupConfig() error {
 	if _, err := m.fs.Stat(resolvConf); err != nil {
 		if os.IsNotExist(err) {
 			// No resolv.conf, nothing to back up. Also get rid of any
@@ -196,10 +215,10 @@ func (m directManager) backupConfig() error {
 		return nil
 	}
 
-	return m.fs.Rename(resolvConf, backupConf)
+	return m.rename(resolvConf, backupConf)
 }
 
-func (m directManager) restoreBackup() (restored bool, err error) {
+func (m *directManager) restoreBackup() (restored bool, err error) {
 	if _, err := m.fs.Stat(backupConf); err != nil {
 		if os.IsNotExist(err) {
 			// No backup, nothing we can do.
@@ -225,14 +244,48 @@ func (m directManager) restoreBackup() (restored bool, err error) {
 	}
 
 	// We own resolv.conf, and a backup exists.
-	if err := m.fs.Rename(backupConf, resolvConf); err != nil {
+	if err := m.rename(backupConf, resolvConf); err != nil {
 		return false, err
 	}
 
 	return true, nil
 }
 
-func (m directManager) SetDNS(config OSConfig) (err error) {
+// rename tries to rename old to new using m.fs.Rename, and falls back
+// to hand-copying bytes and truncating old if that fails.
+//
+// This is a workaround to /etc/resolv.conf being a bind-mounted file
+// some container environments, which cannot be moved elsewhere in
+// /etc (because that would be a cross-filesystem move) or deleted
+// (because that would break the bind in surprising ways).
+func (m *directManager) rename(old, new string) error {
+	if !m.renameBroken {
+		err := m.fs.Rename(old, new)
+		if err == nil {
+			return nil
+		}
+		m.logf("rename of %q to %q failed (%v), falling back to copy+delete", old, new, err)
+		m.renameBroken = true
+	}
+
+	bs, err := m.fs.ReadFile(old)
+	if err != nil {
+		return fmt.Errorf("reading %q to rename: %v", old, err)
+	}
+	if err := m.fs.WriteFile(new, bs, 0644); err != nil {
+		return fmt.Errorf("writing to %q in rename of %q: %v", new, old, err)
+	}
+
+	if err := m.fs.Remove(old); err != nil {
+		err2 := m.fs.Truncate(old)
+		if err2 != nil {
+			return fmt.Errorf("remove of %q failed (%v) and so did truncate: %v", old, err, err2)
+		}
+	}
+	return nil
+}
+
+func (m *directManager) SetDNS(config OSConfig) (err error) {
 	var changed bool
 	if config.IsZero() {
 		changed, err = m.restoreBackup()
@@ -247,7 +300,7 @@ func (m directManager) SetDNS(config OSConfig) (err error) {
 
 		buf := new(bytes.Buffer)
 		writeResolvConf(buf, config.Nameservers, config.SearchDomains)
-		if err := atomicWriteFile(m.fs, resolvConf, buf.Bytes(), 0644); err != nil {
+		if err := m.atomicWriteFile(m.fs, resolvConf, buf.Bytes(), 0644); err != nil {
 			return err
 		}
 	}
@@ -275,11 +328,11 @@ func (m directManager) SetDNS(config OSConfig) (err error) {
 	return nil
 }
 
-func (m directManager) SupportsSplitDNS() bool {
+func (m *directManager) SupportsSplitDNS() bool {
 	return false
 }
 
-func (m directManager) GetBaseConfig() (OSConfig, error) {
+func (m *directManager) GetBaseConfig() (OSConfig, error) {
 	owned, err := m.ownedByTailscale()
 	if err != nil {
 		return OSConfig{}, err
@@ -292,7 +345,7 @@ func (m directManager) GetBaseConfig() (OSConfig, error) {
 	return m.readResolvFile(fileToRead)
 }
 
-func (m directManager) Close() error {
+func (m *directManager) Close() error {
 	// We used to keep a file for the tailscale config and symlinked
 	// to it, but then we stopped because /etc/resolv.conf being a
 	// symlink to surprising places breaks snaps and other sandboxing
@@ -324,7 +377,7 @@ func (m directManager) Close() error {
 	}
 
 	// We own resolv.conf, and a backup exists.
-	if err := m.fs.Rename(backupConf, resolvConf); err != nil {
+	if err := m.rename(backupConf, resolvConf); err != nil {
 		return err
 	}
 
@@ -335,7 +388,7 @@ func (m directManager) Close() error {
 	return nil
 }
 
-func atomicWriteFile(fs wholeFileFS, filename string, data []byte, perm os.FileMode) error {
+func (m *directManager) atomicWriteFile(fs wholeFileFS, filename string, data []byte, perm os.FileMode) error {
 	var randBytes [12]byte
 	if _, err := rand.Read(randBytes[:]); err != nil {
 		return fmt.Errorf("atomicWriteFile: %w", err)
@@ -347,7 +400,7 @@ func atomicWriteFile(fs wholeFileFS, filename string, data []byte, perm os.FileM
 	if err := fs.WriteFile(tmpName, data, perm); err != nil {
 		return fmt.Errorf("atomicWriteFile: %w", err)
 	}
-	return fs.Rename(tmpName, filename)
+	return m.rename(tmpName, filename)
 }
 
 // wholeFileFS is a high-level file system abstraction designed just for use
@@ -359,6 +412,7 @@ type wholeFileFS interface {
 	Rename(oldName, newName string) error
 	Remove(name string) error
 	ReadFile(name string) ([]byte, error)
+	Truncate(name string) error
 	WriteFile(name string, contents []byte, perm os.FileMode) error
 }
 
@@ -391,6 +445,10 @@ func (fs directFS) ReadFile(name string) ([]byte, error) {
 	return ioutil.ReadFile(fs.path(name))
 }
 
+func (fs directFS) Truncate(name string) error {
+	return os.Truncate(fs.path(name), 0)
+}
+
 func (fs directFS) WriteFile(name string, contents []byte, perm os.FileMode) error {
 	return ioutil.WriteFile(fs.path(name), contents, perm)
 }

net/dns/direct_test.go

@@ -5,31 +5,65 @@
 package dns
 
 import (
-	"io/ioutil"
+	"errors"
+	"fmt"
+	"io/fs"
 	"os"
 	"path/filepath"
+	"strings"
+	"syscall"
 	"testing"
 
 	"inet.af/netaddr"
 	"tailscale.com/util/dnsname"
 )
 
-func TestSetDNS(t *testing.T) {
-	const orig = "nameserver 9.9.9.9 # orig"
+func TestDirectManager(t *testing.T) {
 	tmp := t.TempDir()
-	resolvPath := filepath.Join(tmp, "etc", "resolv.conf")
-	backupPath := filepath.Join(tmp, "etc", "resolv.pre-tailscale-backup.conf")
+	if err := os.MkdirAll(filepath.Join(tmp, "etc"), 0700); err != nil {
+		t.Fatal(err)
+	}
+	testDirect(t, directFS{prefix: tmp})
+}
 
-	if err := os.MkdirAll(filepath.Dir(resolvPath), 0777); err != nil {
+type boundResolvConfFS struct {
+	directFS
+}
+
+func (fs boundResolvConfFS) Rename(old, new string) error {
+	if old == "/etc/resolv.conf" || new == "/etc/resolv.conf" {
+		return errors.New("cannot move to/from /etc/resolv.conf")
+	}
+	return fs.directFS.Rename(old, new)
+}
+
+func (fs boundResolvConfFS) Remove(name string) error {
+	if name == "/etc/resolv.conf" {
+		return errors.New("cannot remove /etc/resolv.conf")
+	}
+	return fs.directFS.Remove(name)
+}
+
+func TestDirectBrokenRename(t *testing.T) {
+	tmp := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(tmp, "etc"), 0700); err != nil {
 		t.Fatal(err)
 	}
-	if err := ioutil.WriteFile(resolvPath, []byte(orig), 0644); err != nil {
+	testDirect(t, boundResolvConfFS{directFS{prefix: tmp}})
+}
+
+func testDirect(t *testing.T, fs wholeFileFS) {
+	const orig = "nameserver 9.9.9.9 # orig"
+	resolvPath := "/etc/resolv.conf"
+	backupPath := "/etc/resolv.pre-tailscale-backup.conf"
+
+	if err := fs.WriteFile(resolvPath, []byte(orig), 0644); err != nil {
 		t.Fatal(err)
 	}
 
 	readFile := func(t *testing.T, path string) string {
 		t.Helper()
-		b, err := ioutil.ReadFile(path)
+		b, err := fs.ReadFile(path)
 		if err != nil {
 			t.Fatal(err)
 		}
@@ -39,12 +73,12 @@ func TestSetDNS(t *testing.T) {
 		if got := readFile(t, resolvPath); got != orig {
 			t.Fatalf("resolv.conf:\n%s, want:\n%s", got, orig)
 		}
-		if _, err := os.Stat(backupPath); !os.IsNotExist(err) {
+		if _, err := fs.Stat(backupPath); !os.IsNotExist(err) {
 			t.Fatalf("resolv.conf backup: want it to be gone but: %v", err)
 		}
 	}
 
-	m := directManager{fs: directFS{prefix: tmp}}
+	m := directManager{logf: t.Logf, fs: fs}
 	if err := m.SetDNS(OSConfig{
 		Nameservers:   []netaddr.IP{netaddr.MustParseIP("8.8.8.8"), netaddr.MustParseIP("8.8.4.4")},
 		SearchDomains: []dnsname.FQDN{"ts.net.", "ts-dns.test."},
@@ -81,3 +115,26 @@ search ts.net ts-dns.test
 	}
 	assertBaseState(t)
 }
+
+type brokenRemoveFS struct {
+	directFS
+}
+
+func (b brokenRemoveFS) Rename(old, new string) error {
+	return errors.New("nyaaah I'm a silly container!")
+}
+
+func (b brokenRemoveFS) Remove(name string) error {
+	if strings.Contains(name, "/etc/resolv.conf") {
+		return fmt.Errorf("Faking remove failure: %q", &fs.PathError{Err: syscall.EBUSY})
+	}
+	return b.directFS.Remove(name)
+}
+
+func TestDirectBrokenRemove(t *testing.T) {
+	tmp := t.TempDir()
+	if err := os.MkdirAll(filepath.Join(tmp, "etc"), 0700); err != nil {
+		t.Fatal(err)
+	}
+	testDirect(t, brokenRemoveFS{directFS{prefix: tmp}})
+}

net/dns/manager_freebsd.go

@@ -15,7 +15,7 @@ import (
 func NewOSConfigurator(logf logger.Logf, _ string) (OSConfigurator, error) {
 	bs, err := ioutil.ReadFile("/etc/resolv.conf")
 	if os.IsNotExist(err) {
-		return newDirectManager(), nil
+		return newDirectManager(logf), nil
 	}
 	if err != nil {
 		return nil, fmt.Errorf("reading /etc/resolv.conf: %w", err)
@@ -25,16 +25,16 @@ func NewOSConfigurator(logf logger.Logf, _ string) (OSConfigurator, error) {
 	case "resolvconf":
 		switch resolvconfStyle() {
 		case "":
-			return newDirectManager(), nil
+			return newDirectManager(logf), nil
 		case "debian":
 			return newDebianResolvconfManager(logf)
 		case "openresolv":
 			return newOpenresolvManager()
 		default:
 			logf("[unexpected] got unknown flavor of resolvconf %q, falling back to direct manager", resolvconfStyle())
-			return newDirectManager(), nil
+			return newDirectManager(logf), nil
 		}
 	default:
-		return newDirectManager(), nil
+		return newDirectManager(logf), nil
 	}
 }

net/dns/manager_linux.go

@@ -40,7 +40,7 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (ret OSConfigurat
 	}
 	switch mode {
 	case "direct":
-		return newDirectManagerOnFS(env.fs), nil
+		return newDirectManagerOnFS(logf, env.fs), nil
 	case "systemd-resolved":
 		return newResolvedManager(logf, interfaceName)
 	case "network-manager":
@@ -51,7 +51,7 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (ret OSConfigurat
 		return newOpenresolvManager()
 	default:
 		logf("[unexpected] detected unknown DNS mode %q, using direct manager as last resort", mode)
-		return newDirectManagerOnFS(env.fs), nil
+		return newDirectManagerOnFS(logf, env.fs), nil
 	}
 }
 

net/dns/manager_linux_test.go

@@ -186,8 +186,20 @@ func (m memFS) ReadFile(name string) ([]byte, error) {
 	panic("TODO")
 }
 
-func (fs memFS) WriteFile(name string, contents []byte, perm os.FileMode) error {
-	fs[name] = string(contents)
+func (m memFS) Truncate(name string) error {
+	v, ok := m[name]
+	if !ok {
+		return fs.ErrNotExist
+	}
+	if s, ok := v.(string); ok {
+		m[name] = s[:0]
+	}
+
+	return nil
+}
+
+func (m memFS) WriteFile(name string, contents []byte, perm os.FileMode) error {
+	m[name] = string(contents)
 	return nil
 }
 

net/dns/manager_openbsd.go

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

net/dns/resolved.go

@@ -68,7 +68,7 @@ func isResolvedActive() bool {
 		return false
 	}
 
-	config, err := newDirectManager().readResolvFile(resolvConf)
+	config, err := newDirectManager(logger.Discard).readResolvFile(resolvConf)
 	if err != nil {
 		return false
 	}

net/dns/wsl_windows.go

@@ -99,9 +99,9 @@ func (wm *wslManager) SetDNS(cfg OSConfig) error {
 	} else if len(distros) == 0 {
 		return nil
 	}
-	managers := make(map[string]directManager)
+	managers := make(map[string]*directManager)
 	for _, distro := range distros {
-		managers[distro] = newDirectManagerOnFS(wslFS{
+		managers[distro] = newDirectManagerOnFS(wm.logf, wslFS{
 			user:   "root",
 			distro: distro,
 		})
@@ -141,7 +141,7 @@ generateResolvConf = false
 
 // setWSLConf attempts to disable generateResolvConf in each WSL2 linux.
 // If any are changed, it reports true.
-func (wm *wslManager) setWSLConf(managers map[string]directManager) (changed bool) {
+func (wm *wslManager) setWSLConf(managers map[string]*directManager) (changed bool) {
 	for distro, m := range managers {
 		b, err := m.fs.ReadFile(wslConf)
 		if err != nil && !os.IsNotExist(err) {
@@ -189,6 +189,8 @@ func (fs wslFS) Rename(oldName, newName string) error {
 }
 func (fs wslFS) Remove(name string) error { return wslRun(fs.cmd("rm", "--", name)) }
 
+func (fs wslFS) Truncate(name string) error { return fs.WriteFile(name, nil, 0644) }
+
 func (fs wslFS) ReadFile(name string) ([]byte, error) {
 	b, err := wslCombinedOutput(fs.cmd("cat", "--", name))
 	if ee, _ := err.(*exec.ExitError); ee != nil && ee.ExitCode() == 1 {