net/dns: make "direct" mode on Linux warn on resolv.conf fights

Run an inotify goroutine and watch if another program takes over
/etc/inotify.conf. Log if so.

For now this only logs. In the future I want to wire it up into the
health system to warn (visible in "tailscale status", etc) about the
situation, with a short URL to more info about how you should really
be using systemd-resolved if you want programs to not fight over your
DNS files on Linux.

Updates #4254 etc etc

Change-Id: I86ad9125717d266d0e3822d4d847d88da6a0daaa
Signed-off-by: Brad Fitzpatrick <bradfitz@tailscale.com>

cmd/tailscaled/depaware.txt

@@ -71,6 +71,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de
         github.com/golang/groupcache/lru                             from tailscale.com/net/dnscache
         github.com/google/btree                                      from gvisor.dev/gvisor/pkg/tcpip/header+
         github.com/hdevalence/ed25519consensus                       from tailscale.com/tka
+   L 💣 github.com/illarion/gonotify                                 from tailscale.com/net/dns
    L    github.com/insomniacslk/dhcp/dhcpv4                          from tailscale.com/net/tstun
    L    github.com/insomniacslk/dhcp/iana                            from github.com/insomniacslk/dhcp/dhcpv4
    L    github.com/insomniacslk/dhcp/interfaces                      from github.com/insomniacslk/dhcp/dhcpv4

go.mod

@@ -30,6 +30,7 @@ require (
 	github.com/goreleaser/nfpm v1.10.3
 	github.com/hdevalence/ed25519consensus v0.0.0-20220222234857-c00d1f31bab3
 	github.com/iancoleman/strcase v0.2.0
+	github.com/illarion/gonotify v1.0.1
 	github.com/insomniacslk/dhcp v0.0.0-20211209223715-7d93572ebe8e
 	github.com/jsimonetti/rtnetlink v1.1.2-0.20220408201609-d380b505068b
 	github.com/kballard/go-shellquote v0.0.0-20180428030007-95032a82bc51

go.sum

@@ -614,6 +614,8 @@ github.com/iancoleman/strcase v0.2.0 h1:05I4QRnGpI0m37iZQRuskXh+w77mr6Z41lwQzuHL
 github.com/iancoleman/strcase v0.2.0/go.mod h1:iwCmte+B7n89clKwxIoIXy/HfoL7AsD47ZCWhYzw7ho=
 github.com/ianlancetaylor/demangle v0.0.0-20181102032728-5e5cf60278f6/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
 github.com/ianlancetaylor/demangle v0.0.0-20200824232613-28f6c0f3b639/go.mod h1:aSSvb/t6k1mPoxDqO4vJh6VOCGPwU4O0C2/Eqndh1Sc=
+github.com/illarion/gonotify v1.0.1 h1:F1d+0Fgbq/sDWjj/r66ekjDG+IDeecQKUFH4wNwsoio=
+github.com/illarion/gonotify v1.0.1/go.mod h1:zt5pmDofZpU1f8aqlK0+95eQhoEAn/d4G4B/FjVW4jE=
 github.com/imdario/mergo v0.3.4/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
 github.com/imdario/mergo v0.3.8/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=
 github.com/imdario/mergo v0.3.9/go.mod h1:2EnlNZ0deacrJVfApfmtdGgDfMuh/nq6Ok1EcJh5FfA=

net/dns/direct.go

@@ -18,6 +18,7 @@ import (
 	"path/filepath"
 	"runtime"
 	"strings"
+	"sync"
 	"time"
 
 	"tailscale.com/net/dns/resolvconffile"
@@ -130,20 +131,29 @@ type directManager struct {
 	// where a reader can see an empty or partial /etc/resolv.conf),
 	// but is better than having non-functioning DNS.
 	renameBroken bool
+
+	ctx      context.Context    // valid until Close
+	ctxClose context.CancelFunc // closes ctx
+
+	mu               sync.Mutex
+	wantResolvConf   []byte // if non-nil, what we expect /etc/resolv.conf to contain
+	lastWarnContents []byte // last resolv.conf contents that we warned about
 }
 
 func newDirectManager(logf logger.Logf) *directManager {
-	return &directManager{
+	return newDirectManagerOnFS(logf, directFS{})
-		logf: logf,
-		fs:   directFS{},
-	}
 }
 
 func newDirectManagerOnFS(logf logger.Logf, fs wholeFileFS) *directManager {
-	return &directManager{
+	ctx, cancel := context.WithCancel(context.Background())
-		logf: logf,
+	m := &directManager{
-		fs:   fs,
+		logf:     logf,
+		fs:       fs,
+		ctx:      ctx,
+		ctxClose: cancel,
 	}
+	go m.runFileWatcher()
+	return m
 }
 
 func (m *directManager) readResolvFile(path string) (OSConfig, error) {
@@ -272,6 +282,63 @@ func (m *directManager) rename(old, new string) error {
 	return nil
 }
 
+// setWant sets the expected contents of /etc/resolv.conf, if any.
+//
+// A value of nil means no particular value is expected.
+//
+// m takes ownership of want.
+func (m *directManager) setWant(want []byte) {
+	m.mu.Lock()
+	defer m.mu.Unlock()
+	m.wantResolvConf = want
+}
+
+// checkForFileTrample checks whether /etc/resolv.conf has been trampled
+// by another program on the system. (e.g. a DHCP client)
+//
+// For now (2022-11-12) this only logs on changes in state.
+func (m *directManager) checkForFileTrample() {
+	m.mu.Lock()
+	want := m.wantResolvConf
+	lastWarn := m.lastWarnContents
+	m.mu.Unlock()
+
+	if want == nil {
+		return
+	}
+
+	cur, err := m.fs.ReadFile(resolvConf)
+	if err != nil {
+		m.logf("trample: read error: %v", err)
+		return
+	}
+	if bytes.Equal(cur, want) {
+		if lastWarn != nil {
+			m.mu.Lock()
+			m.lastWarnContents = nil
+			m.mu.Unlock()
+			m.logf("trample: resolv.conf again matches expected content")
+		}
+		// TODO(bradfitz): register with health package that all is well
+		return
+	}
+	if bytes.Equal(cur, lastWarn) {
+		// We already logged about this, so not worth doing it again.
+		return
+	}
+
+	m.mu.Lock()
+	m.lastWarnContents = cur
+	m.mu.Unlock()
+
+	show := cur
+	if len(show) > 1024 {
+		show = show[:1024]
+	}
+	m.logf("trample: resolv.conf changed from what we expected. did some other program interfere? current contents: %q", show)
+	// TODO(bradfitz): register with health package that something is wrong
+}
+
 func (m *directManager) SetDNS(config OSConfig) (err error) {
 	defer func() {
 		if err != nil && errors.Is(err, fs.ErrPermission) && runtime.GOOS == "linux" &&
@@ -283,6 +350,7 @@ func (m *directManager) SetDNS(config OSConfig) (err error) {
 			err = nil
 		}
 	}()
+	m.setWant(nil) // reset our expectations before any work
 	var changed bool
 	if config.IsZero() {
 		changed, err = m.restoreBackup()
@@ -300,6 +368,11 @@ func (m *directManager) SetDNS(config OSConfig) (err error) {
 		if err := m.atomicWriteFile(m.fs, resolvConf, buf.Bytes(), 0644); err != nil {
 			return err
 		}
+
+		// Now that we've successfully written to the file, lock it in.
+		// If we see /etc/resolv.conf with different contents, we know somebody
+		// else trampled on it.
+		m.setWant(buf.Bytes())
 	}
 
 	// We might have taken over a configuration managed by resolved,

net/dns/direct_linux.go

@@ -0,0 +1,62 @@
+// Copyright (c) 2022 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 (
+	"context"
+
+	"github.com/illarion/gonotify"
+)
+
+func (m *directManager) runFileWatcher() {
+	in, err := gonotify.NewInotify()
+	if err != nil {
+		// Oh well, we tried. This is all best effort for now, to
+		// surface warnings to users.
+		m.logf("dns: inotify new: %v", err)
+		return
+	}
+	ctx, cancel := context.WithCancel(m.ctx)
+	defer cancel()
+	go m.closeInotifyOnDone(ctx, in)
+
+	const events = gonotify.IN_ATTRIB |
+		gonotify.IN_CLOSE_WRITE |
+		gonotify.IN_CREATE |
+		gonotify.IN_DELETE |
+		gonotify.IN_MODIFY |
+		gonotify.IN_MOVE
+
+	if err := in.AddWatch("/etc/", events); err != nil {
+		m.logf("dns: inotify addwatch: %v", err)
+		return
+	}
+	for {
+		events, err := in.Read()
+		if ctx.Err() != nil {
+			return
+		}
+		if err != nil {
+			m.logf("dns: inotify read: %v", err)
+			return
+		}
+		var match bool
+		for _, ev := range events {
+			if ev.Name == resolvConf {
+				match = true
+				break
+			}
+		}
+		if !match {
+			continue
+		}
+		m.checkForFileTrample()
+	}
+}
+
+func (m *directManager) closeInotifyOnDone(ctx context.Context, in *gonotify.Inotify) {
+	<-ctx.Done()
+	in.Close()
+}

net/dns/direct_notlinux.go

@@ -0,0 +1,11 @@
+// Copyright (c) 2022 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.
+
+//go:build !linux
+
+package dns
+
+func (m *directManager) runFileWatcher() {
+	// Not implemented on other platforms. Maybe it could resort to polling.
+}