From 9b063b86c3390a29dd1abbd820e432fa2b043159 Mon Sep 17 00:00:00 2001 From: David Crawshaw Date: Fri, 25 Jun 2021 09:43:13 -0700 Subject: [PATCH] net/dns: factor directManager out over an FS interface This is preliminary work for using the directManager as part of a wslManager on windows, where in addition to configuring windows we'll use wsl.exe to edit the linux file system and modify the system resolv.conf. The pinholeFS is a little funky, but it's designed to work through simple unix tools via wsl.exe without invoking bash. I would not have thought it would stand on its own like this, but it turns out it's useful for writing a test for the directManager. Signed-off-by: David Crawshaw --- net/dns/direct.go | 118 ++++++++++++++++++++++++++----------- net/dns/direct_test.go | 83 ++++++++++++++++++++++++++ net/dns/manager_freebsd.go | 4 +- net/dns/manager_linux.go | 14 ++--- net/dns/manager_openbsd.go | 2 +- net/dns/resolved.go | 2 +- 6 files changed, 179 insertions(+), 44 deletions(-) create mode 100644 net/dns/direct_test.go diff --git a/net/dns/direct.go b/net/dns/direct.go index bc218bb7d..6fb18347b 100644 --- a/net/dns/direct.go +++ b/net/dns/direct.go @@ -2,13 +2,12 @@ // 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 import ( "bufio" "bytes" + "crypto/rand" "fmt" "io" "io/ioutil" @@ -18,7 +17,6 @@ import ( "strings" "inet.af/netaddr" - "tailscale.com/atomicfile" "tailscale.com/util/dnsname" ) @@ -77,21 +75,17 @@ func readResolv(r io.Reader) (config OSConfig, err error) { return config, nil } -func readResolvFile(path string) (OSConfig, error) { - var config OSConfig - - f, err := os.Open(path) +func (m directManager) readResolvFile(path string) (OSConfig, error) { + b, err := m.fs.ReadFile(path) if err != nil { - return config, err + return OSConfig{}, err } - defer f.Close() - - return readResolv(f) + return readResolv(bytes.NewReader(b)) } // readResolvConf reads DNS configuration from /etc/resolv.conf. -func readResolvConf() (OSConfig, error) { - return readResolvFile(resolvConf) +func (m directManager) readResolvConf() (OSConfig, error) { + return m.readResolvFile(resolvConf) } // resolvOwner returns the apparent owner of the resolv.conf @@ -150,26 +144,32 @@ func isResolvedRunning() bool { // to the disappearance of the Tailscale interface. // The caller must call Down before program shutdown // or as cleanup if the program terminates unexpectedly. -type directManager struct{} +type directManager struct { + fs pinholeFS +} -func newDirectManager() (directManager, error) { - return directManager{}, nil +func newDirectManager() directManager { + return directManager{fs: directFS{}} +} + +func newDirectManagerOnFS(fs pinholeFS) directManager { + return directManager{fs: fs} } // ownedByTailscale reports whether /etc/resolv.conf seems to be a // tailscale-managed file. func (m directManager) ownedByTailscale() (bool, error) { - st, err := os.Stat(resolvConf) + isRegular, err := m.fs.Stat(resolvConf) if err != nil { if os.IsNotExist(err) { return false, nil } return false, err } - if !st.Mode().IsRegular() { + if !isRegular { return false, nil } - bs, err := ioutil.ReadFile(resolvConf) + bs, err := m.fs.ReadFile(resolvConf) if err != nil { return false, err } @@ -182,11 +182,11 @@ 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 { - if _, err := os.Stat(resolvConf); err != nil { + if _, err := m.fs.Stat(resolvConf); err != nil { if os.IsNotExist(err) { // No resolv.conf, nothing to back up. Also get rid of any // existing backup file, to avoid restoring something old. - os.Remove(backupConf) + m.fs.Remove(backupConf) return nil } return err @@ -200,11 +200,11 @@ func (m directManager) backupConfig() error { return nil } - return os.Rename(resolvConf, backupConf) + return m.fs.Rename(resolvConf, backupConf) } func (m directManager) restoreBackup() error { - if _, err := os.Stat(backupConf); err != nil { + if _, err := m.fs.Stat(backupConf); err != nil { if os.IsNotExist(err) { // No backup, nothing we can do. return nil @@ -215,7 +215,7 @@ func (m directManager) restoreBackup() error { if err != nil { return err } - if _, err := os.Stat(resolvConf); err != nil && !os.IsNotExist(err) { + if _, err := m.fs.Stat(resolvConf); err != nil && !os.IsNotExist(err) { return err } resolvConfExists := !os.IsNotExist(err) @@ -223,12 +223,12 @@ func (m directManager) restoreBackup() error { if resolvConfExists && !owned { // There's already a non-tailscale config in place, get rid of // our backup. - os.Remove(backupConf) + m.fs.Remove(backupConf) return nil } // We own resolv.conf, and a backup exists. - if err := os.Rename(backupConf, resolvConf); err != nil { + if err := m.fs.Rename(backupConf, resolvConf); err != nil { return err } @@ -247,7 +247,7 @@ func (m directManager) SetDNS(config OSConfig) error { buf := new(bytes.Buffer) writeResolvConf(buf, config.Nameservers, config.SearchDomains) - if err := atomicfile.WriteFile(resolvConf, buf.Bytes(), 0644); err != nil { + if err := atomicWriteFile(m.fs, resolvConf, buf.Bytes(), 0644); err != nil { return err } } @@ -279,7 +279,7 @@ func (m directManager) GetBaseConfig() (OSConfig, error) { fileToRead = backupConf } - return readResolvFile(fileToRead) + return m.readResolvFile(fileToRead) } func (m directManager) Close() error { @@ -287,9 +287,9 @@ func (m directManager) Close() error { // to it, but then we stopped because /etc/resolv.conf being a // symlink to surprising places breaks snaps and other sandboxing // things. Clean it up if it's still there. - os.Remove("/etc/resolv.tailscale.conf") + m.fs.Remove("/etc/resolv.tailscale.conf") - if _, err := os.Stat(backupConf); err != nil { + if _, err := m.fs.Stat(backupConf); err != nil { if os.IsNotExist(err) { // No backup, nothing we can do. return nil @@ -300,7 +300,7 @@ func (m directManager) Close() error { if err != nil { return err } - _, err = os.Stat(resolvConf) + _, err = m.fs.Stat(resolvConf) if err != nil && !os.IsNotExist(err) { return err } @@ -309,12 +309,12 @@ func (m directManager) Close() error { if resolvConfExists && !owned { // There's already a non-tailscale config in place, get rid of // our backup. - os.Remove(backupConf) + m.fs.Remove(backupConf) return nil } // We own resolv.conf, and a backup exists. - if err := os.Rename(backupConf, resolvConf); err != nil { + if err := m.fs.Rename(backupConf, resolvConf); err != nil { return err } @@ -324,3 +324,55 @@ func (m directManager) Close() error { return nil } + +func atomicWriteFile(fs pinholeFS, 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) + } + + tmpName := fmt.Sprintf("%s.%x.tmp", filename, randBytes[:]) + defer fs.Remove(tmpName) + + if err := fs.WriteFile(tmpName, data, perm); err != nil { + return fmt.Errorf("atomicWriteFile: %w", err) + } + return fs.Rename(tmpName, filename) +} + +// pinholeFS is a high-level file system abstraction designed just for use +// by directManager, with the goal that it is easy to implement over wsl.exe. +type pinholeFS interface { + Stat(name string) (isRegular bool, err error) + Rename(oldName, newName string) error + Remove(name string) error + ReadFile(name string) ([]byte, error) + WriteFile(name string, contents []byte, perm os.FileMode) error +} + +// directFS is a pinholeFS implemented directly on the OS. +type directFS struct { + prefix string // file path prefix; used for testing +} + +func (fs directFS) Stat(name string) (isRegular bool, err error) { + fi, err := os.Stat(fs.prefix + name) + if err != nil { + return false, err + } + return fi.Mode().IsRegular(), nil +} + +func (fs directFS) Rename(oldName, newName string) error { + return os.Rename(fs.prefix+oldName, fs.prefix+newName) +} + +func (fs directFS) Remove(name string) error { return os.Remove(fs.prefix + name) } + +func (fs directFS) ReadFile(name string) ([]byte, error) { + return ioutil.ReadFile(fs.prefix + name) +} + +func (fs directFS) WriteFile(name string, contents []byte, perm os.FileMode) error { + return ioutil.WriteFile(fs.prefix+name, contents, perm) +} diff --git a/net/dns/direct_test.go b/net/dns/direct_test.go new file mode 100644 index 000000000..dc090f8fc --- /dev/null +++ b/net/dns/direct_test.go @@ -0,0 +1,83 @@ +// Copyright (c) 2021 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 ( + "io/ioutil" + "os" + "path/filepath" + "testing" + + "inet.af/netaddr" + "tailscale.com/util/dnsname" +) + +func TestSetDNS(t *testing.T) { + const orig = "nameserver 9.9.9.9 # orig" + 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.Dir(resolvPath), 0777); err != nil { + t.Fatal(err) + } + if err := ioutil.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) + if err != nil { + t.Fatal(err) + } + return string(b) + } + assertBaseState := func(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) { + t.Fatalf("resolv.conf backup: want it to be gone but: %v", err) + } + } + + m := directManager{fs: directFS{prefix: tmp}} + 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."}, + MatchDomains: []dnsname.FQDN{"ignored."}, + }); err != nil { + t.Fatal(err) + } + want := `# resolv.conf(5) file generated by tailscale +# DO NOT EDIT THIS FILE BY HAND -- CHANGES WILL BE OVERWRITTEN + +nameserver 8.8.8.8 +nameserver 8.8.4.4 +search ts.net ts-dns.test +` + if got := readFile(t, resolvPath); got != want { + t.Fatalf("resolv.conf:\n%s, want:\n%s", got, want) + } + if got := readFile(t, backupPath); got != orig { + t.Fatalf("resolv.conf backup:\n%s, want:\n%s", got, orig) + } + + // Test that a nil OSConfig cleans up resolv.conf. + if err := m.SetDNS(OSConfig{}); err != nil { + t.Fatal(err) + } + assertBaseState(t) + + // Test that Close cleans up resolv.conf. + if err := m.SetDNS(OSConfig{Nameservers: []netaddr.IP{netaddr.MustParseIP("8.8.8.8")}}); err != nil { + t.Fatal(err) + } + if err := m.Close(); err != nil { + t.Fatal(err) + } + assertBaseState(t) +} diff --git a/net/dns/manager_freebsd.go b/net/dns/manager_freebsd.go index 64eb5d4b8..8a106dd97 100644 --- a/net/dns/manager_freebsd.go +++ b/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() + return newDirectManager(), nil } if err != nil { return nil, fmt.Errorf("reading /etc/resolv.conf: %w", err) @@ -25,6 +25,6 @@ func NewOSConfigurator(logf logger.Logf, _ string) (OSConfigurator, error) { case "resolvconf": return newResolvconfManager(logf) default: - return newDirectManager() + return newDirectManager(), nil } } diff --git a/net/dns/manager_linux.go b/net/dns/manager_linux.go index 05f62a8d6..071203979 100644 --- a/net/dns/manager_linux.go +++ b/net/dns/manager_linux.go @@ -42,7 +42,7 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (ret OSConfigurat bs, err := ioutil.ReadFile("/etc/resolv.conf") if os.IsNotExist(err) { dbg("rc", "missing") - return newDirectManager() + return newDirectManager(), nil } if err != nil { return nil, fmt.Errorf("reading /etc/resolv.conf: %w", err) @@ -58,11 +58,11 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (ret OSConfigurat // https://github.com/tailscale/tailscale/issues/2136 if err := resolvedIsActuallyResolver(); err != nil { dbg("resolved", "not-in-use") - return newDirectManager() + return newDirectManager(), nil } if err := dbusPing("org.freedesktop.resolve1", "/org/freedesktop/resolve1"); err != nil { dbg("resolved", "no") - return newDirectManager() + return newDirectManager(), nil } if err := dbusPing("org.freedesktop.NetworkManager", "/org/freedesktop/NetworkManager/DnsManager"); err != nil { dbg("nm", "no") @@ -125,7 +125,7 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (ret OSConfigurat dbg("rc", "resolvconf") if _, err := exec.LookPath("resolvconf"); err != nil { dbg("resolvconf", "no") - return newDirectManager() + return newDirectManager(), nil } dbg("resolvconf", "yes") return newResolvconfManager(logf) @@ -143,10 +143,10 @@ func NewOSConfigurator(logf logger.Logf, interfaceName string) (ret OSConfigurat // anyway, so you still need a fallback path that uses // directManager. dbg("rc", "nm") - return newDirectManager() + return newDirectManager(), nil default: dbg("rc", "unknown") - return newDirectManager() + return newDirectManager(), nil } } @@ -195,7 +195,7 @@ func nmIsUsingResolved() error { } func resolvedIsActuallyResolver() error { - cfg, err := readResolvConf() + cfg, err := newDirectManager().readResolvConf() if err != nil { return err } diff --git a/net/dns/manager_openbsd.go b/net/dns/manager_openbsd.go index ed0a08d45..9d655d305 100644 --- a/net/dns/manager_openbsd.go +++ b/net/dns/manager_openbsd.go @@ -7,5 +7,5 @@ package dns import "tailscale.com/types/logger" func NewOSConfigurator(logger.Logf, string) (OSConfigurator, error) { - return newDirectManager() + return newDirectManager(), nil } diff --git a/net/dns/resolved.go b/net/dns/resolved.go index f81c18822..9719e825a 100644 --- a/net/dns/resolved.go +++ b/net/dns/resolved.go @@ -67,7 +67,7 @@ func isResolvedActive() bool { return false } - config, err := readResolvConf() + config, err := newDirectManager().readResolvConf() if err != nil { return false }