From 8085324449c133adf6214f07845df62cac970b30 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Claus=20Lensb=C3=B8l?= Date: Tue, 18 Nov 2025 15:57:29 -0500 Subject: [PATCH] net/dns: retrample resolve.conf when another process has trampled it MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When using the resolve.conf file for setting DNS, it is possible that some other services will trample the file and overwrite our set DNS server. Experiments has shown this to be a racy error depending on how quickly processes start. Make an attempt to trample back the file a limited number of times if the file is changed. Updates #16635 Signed-off-by: Claus Lensbøl --- cmd/tailscaled/tailscaled.go | 4 +- net/dns/direct.go | 38 ++++++++++- net/dns/direct_linux_test.go | 109 ++++++++++++++++++++++++++++++++ net/dns/manager.go | 51 ++++++++++++--- net/dns/manager_darwin.go | 5 +- net/dns/manager_default.go | 3 +- net/dns/manager_freebsd.go | 11 ++-- net/dns/manager_linux.go | 7 +- net/dns/manager_openbsd.go | 11 ++-- net/dns/manager_plan9.go | 3 +- net/dns/manager_solaris.go | 5 +- net/dns/manager_tcp_test.go | 4 +- net/dns/manager_test.go | 42 +++++++++++- net/dns/manager_windows.go | 7 +- net/dns/manager_windows_test.go | 4 +- net/dns/wsl_windows.go | 2 +- wgengine/userspace.go | 2 +- 17 files changed, 263 insertions(+), 45 deletions(-) create mode 100644 net/dns/direct_linux_test.go diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index d923ca1ed..09c68a3ca 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -771,7 +771,7 @@ func tryEngine(logf logger.Logf, sys *tsd.System, name string) (onlyNetstack boo // configuration being unavailable (from the noop // manager). More in Issue 4017. // TODO(bradfitz): add a Synology-specific DNS manager. - conf.DNS, err = dns.NewOSConfigurator(logf, sys.HealthTracker.Get(), sys.PolicyClientOrDefault(), sys.ControlKnobs(), "") // empty interface name + conf.DNS, err = dns.NewOSConfigurator(logf, sys.HealthTracker.Get(), sys.Bus.Get(), sys.PolicyClientOrDefault(), sys.ControlKnobs(), "") // empty interface name if err != nil { return false, fmt.Errorf("dns.NewOSConfigurator: %w", err) } @@ -805,7 +805,7 @@ func tryEngine(logf logger.Logf, sys *tsd.System, name string) (onlyNetstack boo return false, fmt.Errorf("creating router: %w", err) } - d, err := dns.NewOSConfigurator(logf, sys.HealthTracker.Get(), sys.PolicyClientOrDefault(), sys.ControlKnobs(), devName) + d, err := dns.NewOSConfigurator(logf, sys.HealthTracker.Get(), sys.Bus.Get(), sys.PolicyClientOrDefault(), sys.ControlKnobs(), devName) if err != nil { dev.Close() r.Close() diff --git a/net/dns/direct.go b/net/dns/direct.go index 59eb06964..926dc18a3 100644 --- a/net/dns/direct.go +++ b/net/dns/direct.go @@ -21,6 +21,7 @@ import ( "slices" "strings" "sync" + "sync/atomic" "time" "tailscale.com/feature" @@ -29,6 +30,7 @@ import ( "tailscale.com/net/tsaddr" "tailscale.com/types/logger" "tailscale.com/util/dnsname" + "tailscale.com/util/eventbus" "tailscale.com/version/distro" ) @@ -135,6 +137,11 @@ type directManager struct { // but is better than having non-functioning DNS. renameBroken bool + trampleCount atomic.Int64 + trampleTimer *time.Timer + eventClient *eventbus.Client + trampleDNSPub *eventbus.Publisher[TrampleDNS] + ctx context.Context // valid until Close ctxClose context.CancelFunc // closes ctx @@ -145,11 +152,13 @@ type directManager struct { } //lint:ignore U1000 used in manager_{freebsd,openbsd}.go -func newDirectManager(logf logger.Logf, health *health.Tracker) *directManager { - return newDirectManagerOnFS(logf, health, directFS{}) +func newDirectManager(logf logger.Logf, health *health.Tracker, bus *eventbus.Bus) *directManager { + return newDirectManagerOnFS(logf, health, bus, directFS{}) } -func newDirectManagerOnFS(logf logger.Logf, health *health.Tracker, fs wholeFileFS) *directManager { +var trampleWatchDuration = 5 * time.Second + +func newDirectManagerOnFS(logf logger.Logf, health *health.Tracker, bus *eventbus.Bus, fs wholeFileFS) *directManager { ctx, cancel := context.WithCancel(context.Background()) m := &directManager{ logf: logf, @@ -158,6 +167,13 @@ func newDirectManagerOnFS(logf logger.Logf, health *health.Tracker, fs wholeFile ctx: ctx, ctxClose: cancel, } + if bus != nil { + m.eventClient = bus.Client("dns.directManager") + m.trampleDNSPub = eventbus.Publish[TrampleDNS](m.eventClient) + } + m.trampleTimer = time.AfterFunc(trampleWatchDuration, func() { + m.trampleCount.Store(0) + }) go m.runFileWatcher() return m } @@ -481,10 +497,26 @@ func (m *directManager) checkForFileTrample() { } m.logf("trample: resolv.conf changed from what we expected. did some other program interfere? current contents: %q", show) m.health.SetUnhealthy(resolvTrampleWarnable, nil) + if m.trampleDNSPub != nil { + n := m.trampleCount.Add(1) + + if n < 10 { + m.trampleDNSPub.Publish(TrampleDNS{ + LastTrample: time.Now(), + TramplesInTimeout: n, + }) + m.trampleTimer.Reset(trampleWatchDuration) + } else { + m.logf("trample: resolv.conf was trampled too many times (%d). Abandoning trying to fix it.", n) + } + } } func (m *directManager) Close() error { m.ctxClose() + if m.eventClient != nil { + m.eventClient.Close() + } // We used to keep a file for the tailscale config and symlinked // to it, but then we stopped because /etc/resolv.conf being a diff --git a/net/dns/direct_linux_test.go b/net/dns/direct_linux_test.go new file mode 100644 index 000000000..f4f1c0d49 --- /dev/null +++ b/net/dns/direct_linux_test.go @@ -0,0 +1,109 @@ +// Copyright (c) Tailscale Inc & AUTHORS +// SPDX-License-Identifier: BSD-3-Clause + +//go:build linux + +package dns + +import ( + "context" + "fmt" + "net/netip" + "os" + "path/filepath" + "testing" + "testing/synctest" + + "github.com/illarion/gonotify/v3" + + "tailscale.com/util/dnsname" + "tailscale.com/util/eventbus/eventbustest" +) + +func TestDNSTrampleRecovery(t *testing.T) { + HookWatchFile.Set(watchFile) + synctest.Test(t, func(t *testing.T) { + tmp := t.TempDir() + if err := os.MkdirAll(filepath.Join(tmp, "etc"), 0700); err != nil { + t.Fatal(err) + } + resolvPath := "/etc/resolv.conf" + fs := directFS{prefix: tmp} + readFile := func(t *testing.T, path string) string { + t.Helper() + b, err := fs.ReadFile(path) + if err != nil { + t.Fatal(err) + } + return string(b) + } + + bus := eventbustest.NewBus(t) + eventbustest.LogAllEvents(t, bus) + m := newDirectManagerOnFS(t.Logf, nil, bus, fs) + defer m.Close() + + if err := m.SetDNS(OSConfig{ + Nameservers: []netip.Addr{netip.MustParseAddr("8.8.8.8"), netip.MustParseAddr("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 +# For more info, see https://tailscale.com/s/resolvconf-overwrite +# 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) + } + + tw := eventbustest.NewWatcher(t, bus) + + trample := "Hvem er det som tramper på min bro?" + if err := fs.WriteFile(resolvPath, []byte(trample), 0644); err != nil { + t.Fatal(err) + } + synctest.Wait() + + if err := eventbustest.Expect(tw, eventbustest.Type[TrampleDNS]()); err != nil { + t.Errorf("did not see trample event: %s", err) + } + }) +} + +// watchFile is generally copied from linuxtrample, but cancels the context +// after one trample to end the test. +func watchFile(ctx context.Context, dir, filename string, cb func()) error { + ctx, cancel := context.WithCancel(ctx) + defer cancel() + + const events = gonotify.IN_ATTRIB | + gonotify.IN_CLOSE_WRITE | + gonotify.IN_CREATE | + gonotify.IN_DELETE | + gonotify.IN_MODIFY | + gonotify.IN_MOVE + + watcher, err := gonotify.NewDirWatcher(ctx, events, dir) + if err != nil { + return fmt.Errorf("NewDirWatcher: %w", err) + } + + for { + select { + case event := <-watcher.C: + if event.Name == filename { + cb() + cancel() + } + case <-ctx.Done(): + return ctx.Err() + } + } +} diff --git a/net/dns/manager.go b/net/dns/manager.go index de99fe646..5a3de1c7e 100644 --- a/net/dns/manager.go +++ b/net/dns/manager.go @@ -55,6 +55,8 @@ type Manager struct { logf logger.Logf health *health.Tracker + eventClient *eventbus.Client + activeQueriesAtomic int32 ctx context.Context // good until Down @@ -69,10 +71,10 @@ type Manager struct { config *Config // Tracks the last viable DNS configuration set by Set. nil on failures other than compilation failures or if set has never been called. } -// NewManagers created a new manager from the given config. +// NewManager created a new manager from the given config. // // knobs may be nil. -func NewManager(logf logger.Logf, oscfg OSConfigurator, health *health.Tracker, dialer *tsdial.Dialer, linkSel resolver.ForwardLinkSelector, knobs *controlknobs.Knobs, goos string) *Manager { +func NewManager(logf logger.Logf, oscfg OSConfigurator, health *health.Tracker, dialer *tsdial.Dialer, linkSel resolver.ForwardLinkSelector, knobs *controlknobs.Knobs, goos string, bus *eventbus.Bus) *Manager { if !buildfeatures.HasDNS { return nil } @@ -96,6 +98,22 @@ func NewManager(logf logger.Logf, oscfg OSConfigurator, health *health.Tracker, goos: goos, } + if bus != nil { + m.eventClient = bus.Client("dns.Manager") + eventbus.SubscribeFunc(m.eventClient, func(trample TrampleDNS) { + m.mu.Lock() + defer m.mu.Unlock() + if m.config == nil { + m.logf("resolve.conf was trampled, but there is no DNS config") + return + } + m.logf("resolve.conf was trampled, setting existing config again") + if err := m.setLocked(*m.config); err != nil { + m.logf("error setting DNS config: %s", err) + } + }) + } + m.ctx, m.ctxCancel = context.WithCancel(context.Background()) m.logf("using %T", m.os) return m @@ -178,9 +196,7 @@ func (m *Manager) setLocked(cfg Config) error { m.config = nil return err } - if err := m.os.SetDNS(ocfg); err != nil { - m.config = nil - m.health.SetUnhealthy(osConfigurationSetWarnable, health.Args{health.ArgError: err.Error()}) + if err := m.setDNSLocked(ocfg); err != nil { return err } @@ -190,6 +206,15 @@ func (m *Manager) setLocked(cfg Config) error { return nil } +func (m *Manager) setDNSLocked(ocfg OSConfig) error { + if err := m.os.SetDNS(ocfg); err != nil { + m.config = nil + m.health.SetUnhealthy(osConfigurationSetWarnable, health.Args{health.ArgError: err.Error()}) + return err + } + return nil +} + // compileHostEntries creates a list of single-label resolutions possible // from the configured hosts and search domains. // The entries are compiled in the order of the search domains, then the hosts. @@ -457,6 +482,11 @@ const ( maxReqSizeTCP = 4096 ) +type TrampleDNS struct { + LastTrample time.Time + TramplesInTimeout int64 +} + // dnsTCPSession services DNS requests sent over TCP. type dnsTCPSession struct { m *Manager @@ -585,6 +615,9 @@ func (m *Manager) Down() error { if err := m.os.Close(); err != nil { return err } + if m.eventClient != nil { + m.eventClient.Close() + } m.resolver.Close() return nil } @@ -605,7 +638,7 @@ func CleanUp(logf logger.Logf, netMon *netmon.Monitor, bus *eventbus.Bus, health if !buildfeatures.HasDNS { return } - oscfg, err := NewOSConfigurator(logf, health, policyclient.Get(), nil, interfaceName) + oscfg, err := NewOSConfigurator(logf, health, bus, policyclient.Get(), nil, interfaceName) if err != nil { logf("creating dns cleanup: %v", err) return @@ -613,12 +646,10 @@ func CleanUp(logf logger.Logf, netMon *netmon.Monitor, bus *eventbus.Bus, health d := &tsdial.Dialer{Logf: logf} d.SetNetMon(netMon) d.SetBus(bus) - dns := NewManager(logf, oscfg, health, d, nil, nil, runtime.GOOS) + dns := NewManager(logf, oscfg, health, d, nil, nil, runtime.GOOS, bus) if err := dns.Down(); err != nil { logf("dns down: %v", err) } } -var ( - metricDNSQueryErrorQueue = clientmetric.NewCounter("dns_query_local_error_queue") -) +var metricDNSQueryErrorQueue = clientmetric.NewCounter("dns_query_local_error_queue") diff --git a/net/dns/manager_darwin.go b/net/dns/manager_darwin.go index d73ad71a8..01c920626 100644 --- a/net/dns/manager_darwin.go +++ b/net/dns/manager_darwin.go @@ -13,14 +13,15 @@ import ( "tailscale.com/net/dns/resolvconffile" "tailscale.com/net/tsaddr" "tailscale.com/types/logger" + "tailscale.com/util/eventbus" "tailscale.com/util/mak" "tailscale.com/util/syspolicy/policyclient" ) // NewOSConfigurator creates a new OS configurator. // -// The health tracker and the knobs may be nil and are ignored on this platform. -func NewOSConfigurator(logf logger.Logf, _ *health.Tracker, _ policyclient.Client, _ *controlknobs.Knobs, ifName string) (OSConfigurator, error) { +// The health tracker, bus and the knobs may be nil and are ignored on this platform. +func NewOSConfigurator(logf logger.Logf, _ *health.Tracker, _ *eventbus.Bus, _ policyclient.Client, _ *controlknobs.Knobs, ifName string) (OSConfigurator, error) { return &darwinConfigurator{logf: logf, ifName: ifName}, nil } diff --git a/net/dns/manager_default.go b/net/dns/manager_default.go index 1a86690c5..42e7d295d 100644 --- a/net/dns/manager_default.go +++ b/net/dns/manager_default.go @@ -9,12 +9,13 @@ import ( "tailscale.com/control/controlknobs" "tailscale.com/health" "tailscale.com/types/logger" + "tailscale.com/util/eventbus" "tailscale.com/util/syspolicy/policyclient" ) // NewOSConfigurator creates a new OS configurator. // // The health tracker and the knobs may be nil and are ignored on this platform. -func NewOSConfigurator(logger.Logf, *health.Tracker, policyclient.Client, *controlknobs.Knobs, string) (OSConfigurator, error) { +func NewOSConfigurator(logger.Logf, *health.Tracker, *eventbus.Bus, policyclient.Client, *controlknobs.Knobs, string) (OSConfigurator, error) { return NewNoopManager() } diff --git a/net/dns/manager_freebsd.go b/net/dns/manager_freebsd.go index 3237fb382..da3a821ce 100644 --- a/net/dns/manager_freebsd.go +++ b/net/dns/manager_freebsd.go @@ -10,16 +10,17 @@ import ( "tailscale.com/control/controlknobs" "tailscale.com/health" "tailscale.com/types/logger" + "tailscale.com/util/eventbus" "tailscale.com/util/syspolicy/policyclient" ) // NewOSConfigurator creates a new OS configurator. // // The health tracker may be nil; the knobs may be nil and are ignored on this platform. -func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ policyclient.Client, _ *controlknobs.Knobs, _ string) (OSConfigurator, error) { +func NewOSConfigurator(logf logger.Logf, health *health.Tracker, bus *eventbus.Bus, _ policyclient.Client, _ *controlknobs.Knobs, _ string) (OSConfigurator, error) { bs, err := os.ReadFile("/etc/resolv.conf") if os.IsNotExist(err) { - return newDirectManager(logf, health), nil + return newDirectManager(logf, health, bus), nil } if err != nil { return nil, fmt.Errorf("reading /etc/resolv.conf: %w", err) @@ -29,16 +30,16 @@ func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ policyclient. case "resolvconf": switch resolvconfStyle() { case "": - return newDirectManager(logf, health), nil + return newDirectManager(logf, health, bus), nil case "debian": return newDebianResolvconfManager(logf) case "openresolv": return newOpenresolvManager(logf) default: logf("[unexpected] got unknown flavor of resolvconf %q, falling back to direct manager", resolvconfStyle()) - return newDirectManager(logf, health), nil + return newDirectManager(logf, health, bus), nil } default: - return newDirectManager(logf, health), nil + return newDirectManager(logf, health, bus), nil } } diff --git a/net/dns/manager_linux.go b/net/dns/manager_linux.go index 4304df261..4fbf6a8db 100644 --- a/net/dns/manager_linux.go +++ b/net/dns/manager_linux.go @@ -21,6 +21,7 @@ import ( "tailscale.com/net/netaddr" "tailscale.com/types/logger" "tailscale.com/util/clientmetric" + "tailscale.com/util/eventbus" "tailscale.com/util/syspolicy/policyclient" "tailscale.com/version/distro" ) @@ -63,7 +64,7 @@ var ( // NewOSConfigurator created a new OS configurator. // // The health tracker may be nil; the knobs may be nil and are ignored on this platform. -func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ policyclient.Client, _ *controlknobs.Knobs, interfaceName string) (ret OSConfigurator, err error) { +func NewOSConfigurator(logf logger.Logf, health *health.Tracker, bus *eventbus.Bus, _ policyclient.Client, _ *controlknobs.Knobs, interfaceName string) (ret OSConfigurator, err error) { if !buildfeatures.HasDNS || distro.Get() == distro.JetKVM { return NewNoopManager() } @@ -100,7 +101,7 @@ func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ policyclient. logf("dns: using %q mode", mode) switch mode { case "direct": - return newDirectManagerOnFS(logf, health, env.fs), nil + return newDirectManagerOnFS(logf, health, bus, env.fs), nil case "systemd-resolved": if f, ok := optNewResolvedManager.GetOk(); ok { return f(logf, health, interfaceName) @@ -119,7 +120,7 @@ func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ policyclient. logf("[unexpected] detected unknown DNS mode %q, using direct manager as last resort", mode) } - return newDirectManagerOnFS(logf, health, env.fs), nil + return newDirectManagerOnFS(logf, health, bus, env.fs), nil } // newOSConfigEnv are the funcs newOSConfigurator needs, pulled out for testing. diff --git a/net/dns/manager_openbsd.go b/net/dns/manager_openbsd.go index 6168a9e08..766c82f98 100644 --- a/net/dns/manager_openbsd.go +++ b/net/dns/manager_openbsd.go @@ -11,6 +11,7 @@ import ( "tailscale.com/control/controlknobs" "tailscale.com/health" "tailscale.com/types/logger" + "tailscale.com/util/eventbus" "tailscale.com/util/syspolicy/policyclient" ) @@ -25,8 +26,8 @@ func (kv kv) String() string { // NewOSConfigurator created a new OS configurator. // // The health tracker may be nil; the knobs may be nil and are ignored on this platform. -func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ policyclient.Client, _ *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) { - return newOSConfigurator(logf, health, interfaceName, +func NewOSConfigurator(logf logger.Logf, health *health.Tracker, bus *eventbus.Bus, _ policyclient.Client, _ *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) { + return newOSConfigurator(logf, health, bus, interfaceName, newOSConfigEnv{ rcIsResolvd: rcIsResolvd, fs: directFS{}, @@ -39,7 +40,7 @@ type newOSConfigEnv struct { rcIsResolvd func(resolvConfContents []byte) bool } -func newOSConfigurator(logf logger.Logf, health *health.Tracker, interfaceName string, env newOSConfigEnv) (ret OSConfigurator, err error) { +func newOSConfigurator(logf logger.Logf, health *health.Tracker, bus *eventbus.Bus, interfaceName string, env newOSConfigEnv) (ret OSConfigurator, err error) { var debug []kv dbg := func(k, v string) { debug = append(debug, kv{k, v}) @@ -54,7 +55,7 @@ func newOSConfigurator(logf logger.Logf, health *health.Tracker, interfaceName s bs, err := env.fs.ReadFile(resolvConf) if os.IsNotExist(err) { dbg("rc", "missing") - return newDirectManager(logf, health), nil + return newDirectManager(logf, health, bus), nil } if err != nil { return nil, fmt.Errorf("reading /etc/resolv.conf: %w", err) @@ -66,7 +67,7 @@ func newOSConfigurator(logf logger.Logf, health *health.Tracker, interfaceName s } dbg("resolvd", "missing") - return newDirectManager(logf, health), nil + return newDirectManager(logf, health, bus), nil } func rcIsResolvd(resolvConfContents []byte) bool { diff --git a/net/dns/manager_plan9.go b/net/dns/manager_plan9.go index ef1ceea17..47c996dad 100644 --- a/net/dns/manager_plan9.go +++ b/net/dns/manager_plan9.go @@ -20,11 +20,12 @@ import ( "tailscale.com/control/controlknobs" "tailscale.com/health" "tailscale.com/types/logger" + "tailscale.com/util/eventbus" "tailscale.com/util/set" "tailscale.com/util/syspolicy/policyclient" ) -func NewOSConfigurator(logf logger.Logf, ht *health.Tracker, _ policyclient.Client, knobs *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) { +func NewOSConfigurator(logf logger.Logf, ht *health.Tracker, _ *eventbus.Bus, _ policyclient.Client, knobs *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) { return &plan9DNSManager{ logf: logf, ht: ht, diff --git a/net/dns/manager_solaris.go b/net/dns/manager_solaris.go index de7e72bb5..dcd8b1fd3 100644 --- a/net/dns/manager_solaris.go +++ b/net/dns/manager_solaris.go @@ -7,9 +7,10 @@ import ( "tailscale.com/control/controlknobs" "tailscale.com/health" "tailscale.com/types/logger" + "tailscale.com/util/eventbus" "tailscale.com/util/syspolicy/policyclient" ) -func NewOSConfigurator(logf logger.Logf, health *health.Tracker, _ policyclient.Client, _ *controlknobs.Knobs, iface string) (OSConfigurator, error) { - return newDirectManager(logf, health), nil +func NewOSConfigurator(logf logger.Logf, health *health.Tracker, bus *eventbus.Bus, _ policyclient.Client, _ *controlknobs.Knobs, iface string) (OSConfigurator, error) { + return newDirectManager(logf, health, bus), nil } diff --git a/net/dns/manager_tcp_test.go b/net/dns/manager_tcp_test.go index dcdc88c7a..420efe404 100644 --- a/net/dns/manager_tcp_test.go +++ b/net/dns/manager_tcp_test.go @@ -93,7 +93,7 @@ func TestDNSOverTCP(t *testing.T) { bus := eventbustest.NewBus(t) dialer := tsdial.NewDialer(netmon.NewStatic()) dialer.SetBus(bus) - m := NewManager(t.Logf, &f, health.NewTracker(bus), dialer, nil, nil, "") + m := NewManager(t.Logf, &f, health.NewTracker(bus), dialer, nil, nil, "", bus) m.resolver.TestOnlySetHook(f.SetResolver) m.Set(Config{ Hosts: hosts( @@ -181,7 +181,7 @@ func TestDNSOverTCP_TooLarge(t *testing.T) { bus := eventbustest.NewBus(t) dialer := tsdial.NewDialer(netmon.NewStatic()) dialer.SetBus(bus) - m := NewManager(log, &f, health.NewTracker(bus), dialer, nil, nil, "") + m := NewManager(log, &f, health.NewTracker(bus), dialer, nil, nil, "", bus) m.resolver.TestOnlySetHook(f.SetResolver) m.Set(Config{ Hosts: hosts("andrew.ts.com.", "1.2.3.4"), diff --git a/net/dns/manager_test.go b/net/dns/manager_test.go index 92b660007..8a8950986 100644 --- a/net/dns/manager_test.go +++ b/net/dns/manager_test.go @@ -6,9 +6,11 @@ package dns import ( "errors" "net/netip" + "reflect" "runtime" "strings" "testing" + "testing/synctest" "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" @@ -936,7 +938,7 @@ func TestManager(t *testing.T) { bus := eventbustest.NewBus(t) dialer := tsdial.NewDialer(netmon.NewStatic()) dialer.SetBus(bus) - m := NewManager(t.Logf, &f, health.NewTracker(bus), dialer, nil, knobs, goos) + m := NewManager(t.Logf, &f, health.NewTracker(bus), dialer, nil, knobs, goos, bus) m.resolver.TestOnlySetHook(f.SetResolver) if err := m.Set(test.in); err != nil { @@ -1045,7 +1047,7 @@ func TestConfigRecompilation(t *testing.T) { bus := eventbustest.NewBus(t) dialer := tsdial.NewDialer(netmon.NewStatic()) dialer.SetBus(bus) - m := NewManager(t.Logf, f, health.NewTracker(bus), dialer, nil, nil, "darwin") + m := NewManager(t.Logf, f, health.NewTracker(bus), dialer, nil, nil, "darwin", bus) var managerConfig *resolver.Config m.resolver.TestOnlySetHook(func(cfg resolver.Config) { @@ -1078,3 +1080,39 @@ func TestConfigRecompilation(t *testing.T) { t.Fatalf("Want non nil managerConfig. Got nil") } } + +func TestTrampleRetrample(t *testing.T) { + synctest.Test(t, func(t *testing.T) { + f := &fakeOSConfigurator{} + f.BaseConfig = OSConfig{ + Nameservers: mustIPs("1.1.1.1"), + } + + config := Config{ + Routes: upstreams("ts.net", "69.4.2.0", "foo.ts.net", ""), + SearchDomains: fqdns("foo.ts.net"), + } + + bus := eventbustest.NewBus(t) + dialer := tsdial.NewDialer(netmon.NewStatic()) + dialer.SetBus(bus) + m := NewManager(t.Logf, f, health.NewTracker(bus), dialer, nil, nil, "linux", bus) + + // Initial set should error out and store the config + if err := m.Set(config); err != nil { + t.Fatalf("Want nil error. Got non-nil") + } + + // Set no config + f.OSConfig = OSConfig{} + + inj := eventbustest.NewInjector(t, bus) + eventbustest.Inject(inj, TrampleDNS{}) + synctest.Wait() + + t.Logf("OSConfig: %+v", f.OSConfig) + if reflect.DeepEqual(f.OSConfig, OSConfig{}) { + t.Errorf("Expected config to be set, got nil config") + } + }) +} diff --git a/net/dns/manager_windows.go b/net/dns/manager_windows.go index 5ccadbab2..1eccb9a16 100644 --- a/net/dns/manager_windows.go +++ b/net/dns/manager_windows.go @@ -29,6 +29,7 @@ import ( "tailscale.com/syncs" "tailscale.com/types/logger" "tailscale.com/util/dnsname" + "tailscale.com/util/eventbus" "tailscale.com/util/syspolicy/pkey" "tailscale.com/util/syspolicy/policyclient" "tailscale.com/util/syspolicy/ptype" @@ -57,8 +58,8 @@ type windowsManager struct { // NewOSConfigurator created a new OS configurator. // -// The health tracker and the knobs may be nil. -func NewOSConfigurator(logf logger.Logf, health *health.Tracker, polc policyclient.Client, knobs *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) { +// The health tracker, eventbus and the knobs may be nil. +func NewOSConfigurator(logf logger.Logf, health *health.Tracker, bus *eventbus.Bus, polc policyclient.Client, knobs *controlknobs.Knobs, interfaceName string) (OSConfigurator, error) { if polc == nil { panic("nil policyclient.Client") } @@ -163,7 +164,7 @@ func setTailscaleHosts(logf logger.Logf, prevHostsFile []byte, hosts []*HostEntr header = "# TailscaleHostsSectionStart" footer = "# TailscaleHostsSectionEnd" ) - var comments = []string{ + comments := []string{ "# This section contains MagicDNS entries for Tailscale.", "# Do not edit this section manually.", } diff --git a/net/dns/manager_windows_test.go b/net/dns/manager_windows_test.go index aa538a0f6..5525096b3 100644 --- a/net/dns/manager_windows_test.go +++ b/net/dns/manager_windows_test.go @@ -134,7 +134,7 @@ func TestManagerWindowsGPCopy(t *testing.T) { } defer delIfKey() - cfg, err := NewOSConfigurator(logf, nil, policyclient.NoPolicyClient{}, nil, fakeInterface.String()) + cfg, err := NewOSConfigurator(logf, nil, nil, policyclient.NoPolicyClient{}, nil, fakeInterface.String()) if err != nil { t.Fatalf("NewOSConfigurator: %v\n", err) } @@ -263,7 +263,7 @@ func runTest(t *testing.T, isLocal bool) { } defer delIfKey() - cfg, err := NewOSConfigurator(logf, nil, policyclient.NoPolicyClient{}, nil, fakeInterface.String()) + cfg, err := NewOSConfigurator(logf, nil, nil, policyclient.NoPolicyClient{}, nil, fakeInterface.String()) if err != nil { t.Fatalf("NewOSConfigurator: %v\n", err) } diff --git a/net/dns/wsl_windows.go b/net/dns/wsl_windows.go index 8b0780f55..81e859316 100644 --- a/net/dns/wsl_windows.go +++ b/net/dns/wsl_windows.go @@ -76,7 +76,7 @@ func (wm *wslManager) SetDNS(cfg OSConfig) error { } managers := make(map[string]*directManager) for _, distro := range distros { - managers[distro] = newDirectManagerOnFS(wm.logf, wm.health, wslFS{ + managers[distro] = newDirectManagerOnFS(wm.logf, wm.health, nil, wslFS{ user: "root", distro: distro, }) diff --git a/wgengine/userspace.go b/wgengine/userspace.go index a369fa343..d72111b1f 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -387,7 +387,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) conf.Dialer.SetTUNName(tunName) conf.Dialer.SetNetMon(e.netMon) conf.Dialer.SetBus(e.eventBus) - e.dns = dns.NewManager(logf, conf.DNS, e.health, conf.Dialer, fwdDNSLinkSelector{e, tunName}, conf.ControlKnobs, runtime.GOOS) + e.dns = dns.NewManager(logf, conf.DNS, e.health, conf.Dialer, fwdDNSLinkSelector{e, tunName}, conf.ControlKnobs, runtime.GOOS, e.eventBus) // TODO: there's probably a better place for this sockstats.SetNetMon(e.netMon)