diff --git a/cmd/derper/mesh.go b/cmd/derper/mesh.go index 60ee42aca..4e5392732 100644 --- a/cmd/derper/mesh.go +++ b/cmd/derper/mesh.go @@ -15,6 +15,7 @@ "tailscale.com/derp" "tailscale.com/derp/derphttp" + "tailscale.com/net/netmon" "tailscale.com/types/key" "tailscale.com/types/logger" ) @@ -36,7 +37,8 @@ func startMesh(s *derp.Server) error { func startMeshWithHost(s *derp.Server, host string) error { logf := logger.WithPrefix(log.Printf, fmt.Sprintf("mesh(%q): ", host)) - c, err := derphttp.NewClient(s.PrivateKey(), "https://"+host+"/derp", logf) + netMon := netmon.NewStatic() // good enough for cmd/derper; no need for netns fanciness + c, err := derphttp.NewClient(s.PrivateKey(), "https://"+host+"/derp", logf, netMon) if err != nil { return err } diff --git a/cmd/tailscaled/tailscaled.go b/cmd/tailscaled/tailscaled.go index 8e28bb50d..17faeaab4 100644 --- a/cmd/tailscaled/tailscaled.go +++ b/cmd/tailscaled/tailscaled.go @@ -393,8 +393,8 @@ func run() (err error) { // Always clean up, even if we're going to run the server. This covers cases // such as when a system was rebooted without shutting down, or tailscaled // crashed, and would for example restore system DNS configuration. - dns.CleanUp(logf, args.tunname) - router.CleanUp(logf, args.tunname) + dns.CleanUp(logf, netMon, args.tunname) + router.CleanUp(logf, netMon, args.tunname) // If the cleanUp flag was passed, then exit. if args.cleanUp { return nil diff --git a/control/controlclient/direct.go b/control/controlclient/direct.go index db2475e66..636df148c 100644 --- a/control/controlclient/direct.go +++ b/control/controlclient/direct.go @@ -56,6 +56,7 @@ "tailscale.com/util/singleflight" "tailscale.com/util/syspolicy" "tailscale.com/util/systemd" + "tailscale.com/util/testenv" "tailscale.com/util/zstdframe" ) @@ -68,7 +69,7 @@ type Direct struct { serverURL string // URL of the tailcontrol server clock tstime.Clock logf logger.Logf - netMon *netmon.Monitor // or nil + netMon *netmon.Monitor // non-nil health *health.Tracker discoPubKey key.DiscoPublic getMachinePrivKey func() (key.MachinePrivate, error) @@ -120,10 +121,9 @@ type Options struct { Hostinfo *tailcfg.Hostinfo // non-nil passes ownership, nil means to use default using os.Hostname, etc DiscoPublicKey key.DiscoPublic Logf logger.Logf - HTTPTestClient *http.Client // optional HTTP client to use (for tests only) - NoiseTestClient *http.Client // optional HTTP client to use for noise RPCs (tests only) - DebugFlags []string // debug settings to send to control - NetMon *netmon.Monitor // optional network monitor + HTTPTestClient *http.Client // optional HTTP client to use (for tests only) + NoiseTestClient *http.Client // optional HTTP client to use for noise RPCs (tests only) + DebugFlags []string // debug settings to send to control HealthTracker *health.Tracker PopBrowserURL func(url string) // optional func to open browser OnClientVersion func(*tailcfg.ClientVersion) // optional func to inform GUI of client version status @@ -213,6 +213,19 @@ func NewDirect(opts Options) (*Direct, error) { if opts.GetMachinePrivateKey == nil { return nil, errors.New("controlclient.New: no GetMachinePrivateKey specified") } + if opts.Dialer == nil { + if testenv.InTest() { + panic("no Dialer") + } + return nil, errors.New("controlclient.New: no Dialer specified") + } + netMon := opts.Dialer.NetMon() + if netMon == nil { + if testenv.InTest() { + panic("no NetMon in Dialer") + } + return nil, errors.New("controlclient.New: Dialer has nil NetMon") + } if opts.ControlKnobs == nil { opts.ControlKnobs = &controlknobs.Knobs{} } @@ -233,9 +246,8 @@ func NewDirect(opts Options) (*Direct, error) { dnsCache := &dnscache.Resolver{ Forward: dnscache.Get().Forward, // use default cache's forwarder UseLastGood: true, - LookupIPFallback: dnsfallback.MakeLookupFunc(opts.Logf, opts.NetMon), + LookupIPFallback: dnsfallback.MakeLookupFunc(opts.Logf, netMon), Logf: opts.Logf, - NetMon: opts.NetMon, } httpc := opts.HTTPTestClient @@ -272,7 +284,7 @@ func NewDirect(opts Options) (*Direct, error) { authKey: opts.AuthKey, discoPubKey: opts.DiscoPublicKey, debugFlags: opts.DebugFlags, - netMon: opts.NetMon, + netMon: netMon, health: opts.HealthTracker, skipIPForwardingCheck: opts.SkipIPForwardingCheck, pinger: opts.Pinger, diff --git a/control/controlclient/direct_test.go b/control/controlclient/direct_test.go index 169fcc471..48f9617db 100644 --- a/control/controlclient/direct_test.go +++ b/control/controlclient/direct_test.go @@ -14,6 +14,7 @@ "tailscale.com/hostinfo" "tailscale.com/ipn/ipnstate" + "tailscale.com/net/netmon" "tailscale.com/net/tsdial" "tailscale.com/tailcfg" "tailscale.com/types/key" @@ -31,7 +32,7 @@ func TestNewDirect(t *testing.T) { GetMachinePrivateKey: func() (key.MachinePrivate, error) { return k, nil }, - Dialer: new(tsdial.Dialer), + Dialer: tsdial.NewDialer(netmon.NewStatic()), } c, err := NewDirect(opts) if err != nil { @@ -107,7 +108,7 @@ func TestTsmpPing(t *testing.T) { GetMachinePrivateKey: func() (key.MachinePrivate, error) { return k, nil }, - Dialer: new(tsdial.Dialer), + Dialer: tsdial.NewDialer(netmon.NewStatic()), } c, err := NewDirect(opts) diff --git a/control/controlclient/noise_test.go b/control/controlclient/noise_test.go index 3ae1fc0e6..78c1effb4 100644 --- a/control/controlclient/noise_test.go +++ b/control/controlclient/noise_test.go @@ -16,6 +16,7 @@ "golang.org/x/net/http2" "tailscale.com/control/controlhttp" + "tailscale.com/net/netmon" "tailscale.com/net/tsdial" "tailscale.com/tailcfg" "tailscale.com/types/key" @@ -73,7 +74,7 @@ func (tt noiseClientTest) run(t *testing.T) { }) defer hs.Close() - dialer := new(tsdial.Dialer) + dialer := tsdial.NewDialer(netmon.NewStatic()) nc, err := NewNoiseClient(NoiseOpts{ PrivKey: clientPrivate, ServerPubKey: serverPrivate.Public(), diff --git a/control/controlhttp/client.go b/control/controlhttp/client.go index f517ad91e..fe9ccccac 100644 --- a/control/controlhttp/client.go +++ b/control/controlhttp/client.go @@ -393,7 +393,6 @@ func (a *Dialer) resolver() *dnscache.Resolver { LookupIPFallback: dnsfallback.MakeLookupFunc(a.logf, a.NetMon), UseLastGood: true, Logf: a.Logf, // not a.logf method; we want to propagate nil-ness - NetMon: a.NetMon, } } @@ -412,7 +411,6 @@ func (a *Dialer) tryURLUpgrade(ctx context.Context, u *url.URL, addr netip.Addr, SingleHostStaticResult: []netip.Addr{addr}, SingleHost: u.Hostname(), Logf: a.Logf, // not a.logf method; we want to propagate nil-ness - NetMon: a.NetMon, } } else { dns = a.resolver() diff --git a/control/controlhttp/http_test.go b/control/controlhttp/http_test.go index 879305ead..ba2767289 100644 --- a/control/controlhttp/http_test.go +++ b/control/controlhttp/http_test.go @@ -22,6 +22,7 @@ "tailscale.com/control/controlbase" "tailscale.com/net/dnscache" + "tailscale.com/net/netmon" "tailscale.com/net/socks5" "tailscale.com/net/tsdial" "tailscale.com/tailcfg" @@ -199,14 +200,17 @@ func testControlHTTP(t *testing.T, param httpTestParam) { defer cancel() } + netMon := netmon.NewStatic() + dialer := tsdial.NewDialer(netMon) a := &Dialer{ Hostname: "localhost", HTTPPort: strconv.Itoa(httpLn.Addr().(*net.TCPAddr).Port), HTTPSPort: strconv.Itoa(httpsLn.Addr().(*net.TCPAddr).Port), MachineKey: client, ControlKey: server.Public(), + NetMon: netMon, ProtocolVersion: testProtocolVersion, - Dialer: new(tsdial.Dialer).SystemDial, + Dialer: dialer.SystemDial, Logf: t.Logf, omitCertErrorLogging: true, testFallbackDelay: fallbackDelay, @@ -643,7 +647,7 @@ func TestDialPlan(t *testing.T) { dialer := closeTrackDialer{ t: t, - inner: new(tsdial.Dialer).SystemDial, + inner: tsdial.NewDialer(netmon.NewStatic()).SystemDial, conns: make(map[*closeTrackConn]bool), } defer dialer.Done() diff --git a/derp/derphttp/derphttp_client.go b/derp/derphttp/derphttp_client.go index 4b3bc1495..d5ec17b4f 100644 --- a/derp/derphttp/derphttp_client.go +++ b/derp/derphttp/derphttp_client.go @@ -71,7 +71,7 @@ type Client struct { privateKey key.NodePrivate logf logger.Logf - netMon *netmon.Monitor // optional; nil means interfaces will be looked up on-demand + netMon *netmon.Monitor // always non-nil dialer func(ctx context.Context, network, addr string) (net.Conn, error) // Either url or getRegion is non-nil: @@ -116,9 +116,11 @@ func (c *Client) String() string { // NewRegionClient returns a new DERP-over-HTTP client. It connects lazily. // To trigger a connection, use Connect. -// The netMon parameter is optional; if non-nil it's used to do faster interface lookups. // The healthTracker parameter is also optional. func NewRegionClient(privateKey key.NodePrivate, logf logger.Logf, netMon *netmon.Monitor, getRegion func() *tailcfg.DERPRegion) *Client { + if netMon == nil { + panic("nil netMon") + } ctx, cancel := context.WithCancel(context.Background()) c := &Client{ privateKey: privateKey, @@ -140,7 +142,10 @@ func NewNetcheckClient(logf logger.Logf) *Client { // NewClient returns a new DERP-over-HTTP client. It connects lazily. // To trigger a connection, use Connect. -func NewClient(privateKey key.NodePrivate, serverURL string, logf logger.Logf) (*Client, error) { +func NewClient(privateKey key.NodePrivate, serverURL string, logf logger.Logf, netMon *netmon.Monitor) (*Client, error) { + if netMon == nil { + panic("nil netMon") + } u, err := url.Parse(serverURL) if err != nil { return nil, fmt.Errorf("derphttp.NewClient: %v", err) @@ -157,6 +162,7 @@ func NewClient(privateKey key.NodePrivate, serverURL string, logf logger.Logf) ( ctx: ctx, cancelCtx: cancel, clock: tstime.StdClock{}, + netMon: netMon, } return c, nil } diff --git a/derp/derphttp/derphttp_test.go b/derp/derphttp/derphttp_test.go index 8f2b5ee92..28890f151 100644 --- a/derp/derphttp/derphttp_test.go +++ b/derp/derphttp/derphttp_test.go @@ -16,12 +16,15 @@ "time" "tailscale.com/derp" + "tailscale.com/net/netmon" "tailscale.com/types/key" ) func TestSendRecv(t *testing.T) { serverPrivateKey := key.NewNode() + netMon := netmon.NewStatic() + const numClients = 3 var clientPrivateKeys []key.NodePrivate var clientKeys []key.NodePublic @@ -68,7 +71,7 @@ func TestSendRecv(t *testing.T) { }() for i := range numClients { key := clientPrivateKeys[i] - c, err := NewClient(key, serverURL, t.Logf) + c, err := NewClient(key, serverURL, t.Logf, netMon) if err != nil { t.Fatalf("client %d: %v", i, err) } @@ -183,7 +186,7 @@ func TestPing(t *testing.T) { } }() - c, err := NewClient(key.NewNode(), serverURL, t.Logf) + c, err := NewClient(key.NewNode(), serverURL, t.Logf, netmon.NewStatic()) if err != nil { t.Fatalf("NewClient: %v", err) } @@ -236,7 +239,7 @@ func newTestServer(t *testing.T, k key.NodePrivate) (serverURL string, s *derp.S } func newWatcherClient(t *testing.T, watcherPrivateKey key.NodePrivate, serverToWatchURL string) (c *Client) { - c, err := NewClient(watcherPrivateKey, serverToWatchURL, t.Logf) + c, err := NewClient(watcherPrivateKey, serverToWatchURL, t.Logf, netmon.NewStatic()) if err != nil { t.Fatal(err) } diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index a113e3625..aeb180d98 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -353,6 +353,12 @@ func NewLocalBackend(logf logger.Logf, logID logid.PublicID, sys *tsd.System, lo e := sys.Engine.Get() store := sys.StateStore.Get() dialer := sys.Dialer.Get() + if dialer == nil { + return nil, errors.New("dialer to NewLocalBackend must be set") + } + if dialer.NetMon() == nil { + return nil, errors.New("dialer to NewLocalBackend must have a NetMon") + } _ = sys.MagicSock.Get() // or panic goos := envknob.GOOS() @@ -1762,7 +1768,6 @@ func (b *LocalBackend) Start(opts ipn.Options) error { HTTPTestClient: httpTestClient, DiscoPublicKey: discoPublic, DebugFlags: debugFlags, - NetMon: b.sys.NetMon.Get(), HealthTracker: b.health, Pinger: b, PopBrowserURL: b.tellClientToBrowseToURL, diff --git a/ipn/ipnlocal/local_test.go b/ipn/ipnlocal/local_test.go index 6faaa131f..741eeb6ac 100644 --- a/ipn/ipnlocal/local_test.go +++ b/ipn/ipnlocal/local_test.go @@ -2273,7 +2273,6 @@ func TestOnTailnetDefaultAutoUpdate(t *testing.T) { t.Skip("test broken on macOS; see https://github.com/tailscale/tailscale/issues/11894") } tests := []struct { - desc string before, after opt.Bool tailnetDefault bool }{ @@ -2309,7 +2308,7 @@ func TestOnTailnetDefaultAutoUpdate(t *testing.T) { }, } for _, tt := range tests { - t.Run(fmt.Sprintf("before=%s after=%s", tt.before, tt.after), func(t *testing.T) { + t.Run(fmt.Sprintf("before=%s,after=%s", tt.before, tt.after), func(t *testing.T) { b := newTestBackend(t) p := ipn.NewPrefs() p.AutoUpdate.Apply = tt.before diff --git a/ipn/ipnlocal/network-lock_test.go b/ipn/ipnlocal/network-lock_test.go index da317f343..85da13d8b 100644 --- a/ipn/ipnlocal/network-lock_test.go +++ b/ipn/ipnlocal/network-lock_test.go @@ -20,6 +20,8 @@ "tailscale.com/hostinfo" "tailscale.com/ipn" "tailscale.com/ipn/store/mem" + "tailscale.com/net/netmon" + "tailscale.com/net/tsdial" "tailscale.com/tailcfg" "tailscale.com/tka" "tailscale.com/types/key" @@ -50,6 +52,7 @@ func fakeControlClient(t *testing.T, c *http.Client) *controlclient.Auto { HTTPTestClient: c, NoiseTestClient: c, Observer: observerFunc(func(controlclient.Status) {}), + Dialer: tsdial.NewDialer(netmon.NewStatic()), } cc, err := controlclient.NewNoStart(opts) diff --git a/ipn/ipnlocal/serve_test.go b/ipn/ipnlocal/serve_test.go index c294502f1..800e5a89e 100644 --- a/ipn/ipnlocal/serve_test.go +++ b/ipn/ipnlocal/serve_test.go @@ -28,6 +28,7 @@ "tailscale.com/ipn/store/mem" "tailscale.com/tailcfg" "tailscale.com/tsd" + "tailscale.com/types/logger" "tailscale.com/types/logid" "tailscale.com/types/netmap" "tailscale.com/util/mak" @@ -663,14 +664,21 @@ func mustCreateURL(t *testing.T, u string) url.URL { } func newTestBackend(t *testing.T) *LocalBackend { + var logf logger.Logf = logger.Discard + const debug = true + if debug { + logf = logger.WithPrefix(t.Logf, "... ") + } + sys := &tsd.System{} - e, err := wgengine.NewUserspaceEngine(t.Logf, wgengine.Config{SetSubsystem: sys.Set}) + e, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{SetSubsystem: sys.Set}) if err != nil { t.Fatal(err) } sys.Set(e) sys.Set(new(mem.Store)) - b, err := NewLocalBackend(t.Logf, logid.PublicID{}, sys, 0) + + b, err := NewLocalBackend(logf, logid.PublicID{}, sys, 0) if err != nil { t.Fatal(err) } @@ -678,7 +686,7 @@ func newTestBackend(t *testing.T) *LocalBackend { dir := t.TempDir() b.SetVarRoot(dir) - pm := must.Get(newProfileManager(new(mem.Store), t.Logf)) + pm := must.Get(newProfileManager(new(mem.Store), logf)) pm.currentProfile = &ipn.LoginProfile{ID: "id0"} b.pm = pm diff --git a/log/sockstatlog/logger.go b/log/sockstatlog/logger.go index e34dee483..2897b907b 100644 --- a/log/sockstatlog/logger.go +++ b/log/sockstatlog/logger.go @@ -93,11 +93,16 @@ func SockstatLogID(logID logid.PublicID) logid.PrivateID { // On platforms that do not support sockstat logging, a nil Logger will be returned. // The returned Logger is not yet enabled, and must be shut down with Shutdown when it is no longer needed. // Logs will be uploaded to the log server using a new log ID derived from the provided backend logID. -// The netMon parameter is optional; if non-nil it's used to do faster interface lookups. +// +// The netMon parameter is optional. It should be specified in environments where +// Tailscaled is manipulating the routing table. func NewLogger(logdir string, logf logger.Logf, logID logid.PublicID, netMon *netmon.Monitor, health *health.Tracker) (*Logger, error) { if !sockstats.IsAvailable { return nil, nil } + if netMon == nil { + netMon = netmon.NewStatic() + } if err := os.MkdirAll(logdir, 0755); err != nil && !os.IsExist(err) { return nil, err diff --git a/logpolicy/logpolicy.go b/logpolicy/logpolicy.go index 4097ce898..2595a7da1 100644 --- a/logpolicy/logpolicy.go +++ b/logpolicy/logpolicy.go @@ -447,8 +447,8 @@ func tryFixLogStateLocation(dir, cmdname string, logf logger.Logf) { // New returns a new log policy (a logger and its instance ID) for a given // collection name. // -// The netMon parameter is optional; if non-nil it's used to do faster -// interface lookups. +// The netMon parameter is optional. It should be specified in environments where +// Tailscaled is manipulating the routing table. // // The logf parameter is optional; if non-nil, information logs (e.g. when // migrating state) are sent to that logger, and global changes to the log @@ -459,6 +459,9 @@ func New(collection string, netMon *netmon.Monitor, health *health.Tracker, logf // NewWithConfigPath is identical to New, but uses the specified directory and // command name. If either is empty, it derives them automatically. +// +// The netMon parameter is optional. It should be specified in environments where +// Tailscaled is manipulating the routing table. func NewWithConfigPath(collection, dir, cmdName string, netMon *netmon.Monitor, health *health.Tracker, logf logger.Logf) *Policy { var lflags int if term.IsTerminal(2) || runtime.GOOS == "windows" { @@ -681,8 +684,12 @@ func (p *Policy) Shutdown(ctx context.Context) error { // - If TLS connection fails, try again using LetsEncrypt's built-in root certificate, // for the benefit of older OS platforms which might not include it. // -// The netMon parameter is optional; if non-nil it's used to do faster interface lookups. +// The netMon parameter is optional. It should be specified in environments where +// Tailscaled is manipulating the routing table. func MakeDialFunc(netMon *netmon.Monitor, logf logger.Logf) func(ctx context.Context, netw, addr string) (net.Conn, error) { + if netMon == nil { + netMon = netmon.NewStatic() + } return func(ctx context.Context, netw, addr string) (net.Conn, error) { return dialContext(ctx, netw, addr, netMon, logf) } @@ -725,7 +732,6 @@ func dialContext(ctx context.Context, netw, addr string, netMon *netmon.Monitor, Forward: dnscache.Get().Forward, // use default cache's forwarder UseLastGood: true, LookupIPFallback: dnsfallback.MakeLookupFunc(logf, netMon), - NetMon: netMon, } dialer := dnscache.Dialer(nd.DialContext, dnsCache) c, err = dialer(ctx, netw, addr) @@ -738,7 +744,8 @@ func dialContext(ctx context.Context, netw, addr string, netMon *netmon.Monitor, // NewLogtailTransport returns an HTTP Transport particularly suited to uploading // logs to the given host name. See DialContext for details on how it works. // -// The netMon parameter is optional; if non-nil it's used to do faster interface lookups. +// The netMon parameter is optional. It should be specified in environments where +// Tailscaled is manipulating the routing table. // // The logf parameter is optional; if non-nil, logs are printed using the // provided function; if nil, log.Printf will be used instead. @@ -746,6 +753,9 @@ func NewLogtailTransport(host string, netMon *netmon.Monitor, health *health.Tra if testenv.InTest() { return noopPretendSuccessTransport{} } + if netMon == nil { + netMon = netmon.NewStatic() + } // Start with a copy of http.DefaultTransport and tweak it a bit. tr := http.DefaultTransport.(*http.Transport).Clone() diff --git a/logtail/logtail.go b/logtail/logtail.go index a3fad3d3a..14cded52a 100644 --- a/logtail/logtail.go +++ b/logtail/logtail.go @@ -232,7 +232,7 @@ func (l *Logger) SetVerbosityLevel(level int) { atomic.StoreInt64(&l.stderrLevel, int64(level)) } -// SetNetMon sets the optional the network monitor. +// SetNetMon sets the network monitor. // // It should not be changed concurrently with log writes and should // only be set once. diff --git a/net/dns/manager.go b/net/dns/manager.go index bea078a2b..6810d5a6b 100644 --- a/net/dns/manager.go +++ b/net/dns/manager.go @@ -55,15 +55,17 @@ type Manager struct { } // NewManagers created a new manager from the given config. -// The netMon parameter is optional; if non-nil it's used to do faster interface lookups. -func NewManager(logf logger.Logf, oscfg OSConfigurator, netMon *netmon.Monitor, health *health.Tracker, dialer *tsdial.Dialer, linkSel resolver.ForwardLinkSelector, knobs *controlknobs.Knobs) *Manager { +func NewManager(logf logger.Logf, oscfg OSConfigurator, health *health.Tracker, dialer *tsdial.Dialer, linkSel resolver.ForwardLinkSelector, knobs *controlknobs.Knobs) *Manager { if dialer == nil { panic("nil Dialer") } + if dialer.NetMon() == nil { + panic("Dialer has nil NetMon") + } logf = logger.WithPrefix(logf, "dns: ") m := &Manager{ logf: logf, - resolver: resolver.New(logf, netMon, linkSel, dialer, knobs), + resolver: resolver.New(logf, linkSel, dialer, knobs), os: oscfg, health: health, } @@ -454,13 +456,15 @@ func (m *Manager) FlushCaches() error { // CleanUp restores the system DNS configuration to its original state // 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) { +func CleanUp(logf logger.Logf, netMon *netmon.Monitor, interfaceName string) { oscfg, err := NewOSConfigurator(logf, nil, interfaceName) if err != nil { logf("creating dns cleanup: %v", err) return } - dns := NewManager(logf, oscfg, nil, nil, &tsdial.Dialer{Logf: logf}, nil, nil) + d := &tsdial.Dialer{Logf: logf} + d.SetNetMon(netMon) + dns := NewManager(logf, oscfg, nil, d, nil, nil) if err := dns.Down(); err != nil { logf("dns down: %v", err) } diff --git a/net/dns/manager_tcp_test.go b/net/dns/manager_tcp_test.go index 77fab0179..8c61035d2 100644 --- a/net/dns/manager_tcp_test.go +++ b/net/dns/manager_tcp_test.go @@ -15,6 +15,7 @@ "github.com/google/go-cmp/cmp" dns "golang.org/x/net/dns/dnsmessage" + "tailscale.com/net/netmon" "tailscale.com/net/tsdial" "tailscale.com/tstest" "tailscale.com/util/dnsname" @@ -87,7 +88,7 @@ func TestDNSOverTCP(t *testing.T) { SearchDomains: fqdns("coffee.shop"), }, } - m := NewManager(t.Logf, &f, nil, nil, new(tsdial.Dialer), nil, nil) + m := NewManager(t.Logf, &f, nil, tsdial.NewDialer(netmon.NewStatic()), nil, nil) m.resolver.TestOnlySetHook(f.SetResolver) m.Set(Config{ Hosts: hosts( @@ -172,7 +173,7 @@ func TestDNSOverTCP_TooLarge(t *testing.T) { SearchDomains: fqdns("coffee.shop"), }, } - m := NewManager(log, &f, nil, nil, new(tsdial.Dialer), nil, nil) + m := NewManager(log, &f, nil, tsdial.NewDialer(netmon.NewStatic()), nil, nil) 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 46a1e288c..1e7adc943 100644 --- a/net/dns/manager_test.go +++ b/net/dns/manager_test.go @@ -12,6 +12,7 @@ "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" "tailscale.com/net/dns/resolver" + "tailscale.com/net/netmon" "tailscale.com/net/tsdial" "tailscale.com/types/dnstype" "tailscale.com/util/dnsname" @@ -613,7 +614,7 @@ func TestManager(t *testing.T) { SplitDNS: test.split, BaseConfig: test.bs, } - m := NewManager(t.Logf, &f, nil, nil, new(tsdial.Dialer), nil, nil) + m := NewManager(t.Logf, &f, nil, tsdial.NewDialer(netmon.NewStatic()), nil, nil) m.resolver.TestOnlySetHook(f.SetResolver) if err := m.Set(test.in); err != nil { diff --git a/net/dns/resolver/forwarder.go b/net/dns/resolver/forwarder.go index 884e53321..7484fe558 100644 --- a/net/dns/resolver/forwarder.go +++ b/net/dns/resolver/forwarder.go @@ -10,7 +10,6 @@ "errors" "fmt" "io" - "math/rand" "net" "net/http" "net/netip" @@ -186,7 +185,7 @@ type resolverAndDelay struct { // forwarder forwards DNS packets to a number of upstream nameservers. type forwarder struct { logf logger.Logf - netMon *netmon.Monitor + netMon *netmon.Monitor // always non-nil linkSel ForwardLinkSelector // TODO(bradfitz): remove this when tsdial.Dialer absorbs it dialer *tsdial.Dialer @@ -214,11 +213,10 @@ type forwarder struct { cloudHostFallback []resolverAndDelay } -func init() { - rand.Seed(time.Now().UnixNano()) -} - func newForwarder(logf logger.Logf, netMon *netmon.Monitor, linkSel ForwardLinkSelector, dialer *tsdial.Dialer, knobs *controlknobs.Knobs) *forwarder { + if netMon == nil { + panic("nil netMon") + } f := &forwarder{ logf: logger.WithPrefix(logf, "forward: "), netMon: netMon, @@ -410,7 +408,6 @@ func (f *forwarder) getKnownDoHClientForProvider(urlBase string) (c *http.Client SingleHost: dohURL.Hostname(), SingleHostStaticResult: allIPs, Logf: f.logf, - NetMon: f.netMon, }) c = &http.Client{ Transport: &http.Transport{ @@ -584,7 +581,7 @@ func (f *forwarder) send(ctx context.Context, fq *forwardQuery, rr resolverAndDe } // Kick off the race between the UDP and TCP queries. - rh := race.New[[]byte](timeout, firstUDP, thenTCP) + rh := race.New(timeout, firstUDP, thenTCP) resp, err := rh.Start(ctx) if err == nil { return resp, nil diff --git a/net/dns/resolver/tsdns.go b/net/dns/resolver/tsdns.go index a73add0c9..c44565ee1 100644 --- a/net/dns/resolver/tsdns.go +++ b/net/dns/resolver/tsdns.go @@ -181,7 +181,7 @@ func WriteRoutes(w *bufio.Writer, routes map[dnsname.FQDN][]*dnstype.Resolver) { // it delegates to upstream nameservers if any are set. type Resolver struct { logf logger.Logf - netMon *netmon.Monitor // or nil + netMon *netmon.Monitor // non-nil dialer *tsdial.Dialer // non-nil saveConfigForTests func(cfg Config) // used in tests to capture resolver config // forwarder forwards requests to upstream nameservers. @@ -205,11 +205,14 @@ type ForwardLinkSelector interface { } // New returns a new resolver. -// netMon optionally specifies a network monitor to use for socket rebinding. -func New(logf logger.Logf, netMon *netmon.Monitor, linkSel ForwardLinkSelector, dialer *tsdial.Dialer, knobs *controlknobs.Knobs) *Resolver { +func New(logf logger.Logf, linkSel ForwardLinkSelector, dialer *tsdial.Dialer, knobs *controlknobs.Knobs) *Resolver { if dialer == nil { panic("nil Dialer") } + netMon := dialer.NetMon() + if netMon == nil { + logf("nil netMon") + } r := &Resolver{ logf: logger.WithPrefix(logf, "resolver: "), netMon: netMon, diff --git a/net/dns/resolver/tsdns_test.go b/net/dns/resolver/tsdns_test.go index 47fecd0cf..a0f625b20 100644 --- a/net/dns/resolver/tsdns_test.go +++ b/net/dns/resolver/tsdns_test.go @@ -28,6 +28,7 @@ "tailscale.com/net/tsdial" "tailscale.com/tstest" "tailscale.com/types/dnstype" + "tailscale.com/types/logger" "tailscale.com/util/dnsname" ) @@ -313,7 +314,11 @@ func TestRDNSNameToIPv6(t *testing.T) { } func newResolver(t testing.TB) *Resolver { - return New(t.Logf, nil /* no network monitor */, nil /* no link selector */, new(tsdial.Dialer), nil /* no control knobs */) + return New(t.Logf, + nil, // no link selector + tsdial.NewDialer(netmon.NewStatic()), + nil, // no control knobs + ) } func TestResolveLocal(t *testing.T) { @@ -1009,7 +1014,13 @@ func TestForwardLinkSelection(t *testing.T) { // routes differently. specialIP := netaddr.IPv4(1, 2, 3, 4) - fwd := newForwarder(t.Logf, nil, linkSelFunc(func(ip netip.Addr) string { + netMon, err := netmon.New(logger.WithPrefix(t.Logf, ".... netmon: ")) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { netMon.Close() }) + + fwd := newForwarder(t.Logf, netMon, linkSelFunc(func(ip netip.Addr) string { if ip == netaddr.IPv4(1, 2, 3, 4) { return "special" } diff --git a/net/dnscache/dnscache.go b/net/dnscache/dnscache.go index fc3191b34..b23c7d516 100644 --- a/net/dnscache/dnscache.go +++ b/net/dnscache/dnscache.go @@ -19,7 +19,6 @@ "time" "tailscale.com/envknob" - "tailscale.com/net/netmon" "tailscale.com/types/logger" "tailscale.com/util/cloudenv" "tailscale.com/util/singleflight" @@ -90,11 +89,6 @@ type Resolver struct { // be added to all log messages printed with this logger. Logf logger.Logf - // NetMon optionally provides a netmon.Monitor to use to get the current - // (cached) network interface. - // If nil, the interface will be looked up dynamically. - NetMon *netmon.Monitor - sf singleflight.Group[string, ipRes] mu sync.Mutex diff --git a/net/netcheck/netcheck.go b/net/netcheck/netcheck.go index c3dfb5be8..ded2f1efe 100644 --- a/net/netcheck/netcheck.go +++ b/net/netcheck/netcheck.go @@ -1657,7 +1657,6 @@ func (c *Client) nodeAddr(ctx context.Context, n *tailcfg.DERPNode, proto probeP Forward: net.DefaultResolver, UseLastGood: true, Logf: c.logf, - NetMon: c.NetMon, } } resolver := c.resolver diff --git a/net/netcheck/netcheck_test.go b/net/netcheck/netcheck_test.go index 41c55cff5..ef0fe24eb 100644 --- a/net/netcheck/netcheck_test.go +++ b/net/netcheck/netcheck_test.go @@ -24,7 +24,6 @@ "tailscale.com/net/stun/stuntest" "tailscale.com/tailcfg" "tailscale.com/tstest" - "tailscale.com/types/logger" ) func TestHairpinSTUN(t *testing.T) { @@ -157,14 +156,8 @@ func TestHairpinWait(t *testing.T) { } func newTestClient(t testing.TB) *Client { - netMon, err := netmon.New(logger.WithPrefix(t.Logf, "... netmon: ")) - if err != nil { - t.Fatalf("netmon.New: %v", err) - } - t.Cleanup(func() { netMon.Close() }) - c := &Client{ - NetMon: netMon, + NetMon: netmon.NewStatic(), Logf: t.Logf, } return c diff --git a/net/netcheck/standalone.go b/net/netcheck/standalone.go index 87fbc211e..c72d7005f 100644 --- a/net/netcheck/standalone.go +++ b/net/netcheck/standalone.go @@ -24,12 +24,15 @@ // to bind, errors will be returned, if one or both protocols can bind no error // is returned. func (c *Client) Standalone(ctx context.Context, bindAddr string) error { + if c.NetMon == nil { + panic("netcheck.Client.NetMon must be set") + } if bindAddr == "" { bindAddr = ":0" } var errs []error - u4, err := nettype.MakePacketListenerWithNetIP(netns.Listener(c.logf, nil)).ListenPacket(ctx, "udp4", bindAddr) + u4, err := nettype.MakePacketListenerWithNetIP(netns.Listener(c.logf, c.NetMon)).ListenPacket(ctx, "udp4", bindAddr) if err != nil { c.logf("udp4: %v", err) errs = append(errs, err) @@ -37,7 +40,7 @@ func (c *Client) Standalone(ctx context.Context, bindAddr string) error { go readPackets(ctx, c.logf, u4, c.ReceiveSTUNPacket) } - u6, err := nettype.MakePacketListenerWithNetIP(netns.Listener(c.logf, nil)).ListenPacket(ctx, "udp6", bindAddr) + u6, err := nettype.MakePacketListenerWithNetIP(netns.Listener(c.logf, c.NetMon)).ListenPacket(ctx, "udp6", bindAddr) if err != nil { c.logf("udp6: %v", err) errs = append(errs, err) diff --git a/net/netmon/netmon.go b/net/netmon/netmon.go index 39cde33ba..f65e3fa7c 100644 --- a/net/netmon/netmon.go +++ b/net/netmon/netmon.go @@ -55,6 +55,7 @@ type Monitor struct { om osMon // nil means not supported on this platform change chan bool // send false to wake poller, true to also force ChangeDeltas be sent stop chan struct{} // closed on Stop + static bool // static Monitor that doesn't actually monitor // Things that must be set early, before use, // and not change at runtime. @@ -139,6 +140,17 @@ func New(logf logger.Logf) (*Monitor, error) { return m, nil } +// NewStatic returns a Monitor that's a one-time snapshot of the network state +// but doesn't actually monitor for changes. It should only be used in tests +// and situations like cleanups or short-lived CLI programs. +func NewStatic() *Monitor { + m := &Monitor{static: true} + if st, err := m.interfaceStateUncached(); err == nil { + m.ifState = st + } + return m +} + // InterfaceState returns the latest snapshot of the machine's network // interfaces. // @@ -168,6 +180,10 @@ func (m *Monitor) SetTailscaleInterfaceName(ifName string) { // It's the same as interfaces.LikelyHomeRouterIP, but it caches the // result until the monitor detects a network change. func (m *Monitor) GatewayAndSelfIP() (gw, myIP netip.Addr, ok bool) { + if m.static { + return + } + m.mu.Lock() defer m.mu.Unlock() if m.gwValid { @@ -190,6 +206,9 @@ func (m *Monitor) GatewayAndSelfIP() (gw, myIP netip.Addr, ok bool) { // notified (in their own goroutine) when the network state changes. // To remove this callback, call unregister (or close the monitor). func (m *Monitor) RegisterChangeCallback(callback ChangeFunc) (unregister func()) { + if m.static { + return func() {} + } m.mu.Lock() defer m.mu.Unlock() handle := m.cbs.Add(callback) @@ -210,6 +229,9 @@ func (m *Monitor) RegisterChangeCallback(callback ChangeFunc) (unregister func() // notified (in their own goroutine) when a Linux ip rule is deleted. // To remove this callback, call unregister (or close the monitor). func (m *Monitor) RegisterRuleDeleteCallback(callback RuleDeleteCallback) (unregister func()) { + if m.static { + return func() {} + } m.mu.Lock() defer m.mu.Unlock() handle := m.ruleDelCB.Add(callback) @@ -223,6 +245,9 @@ func (m *Monitor) RegisterRuleDeleteCallback(callback RuleDeleteCallback) (unreg // Start starts the monitor. // A monitor can only be started & closed once. func (m *Monitor) Start() { + if m.static { + return + } m.mu.Lock() defer m.mu.Unlock() if m.started || m.closed { @@ -244,6 +269,9 @@ func (m *Monitor) Start() { // Close closes the monitor. func (m *Monitor) Close() error { + if m.static { + return nil + } m.mu.Lock() if m.closed { m.mu.Unlock() @@ -275,6 +303,9 @@ func (m *Monitor) Close() error { // ChangeFunc callbacks will be called within the event coalescing // period (under a fraction of a second). func (m *Monitor) InjectEvent() { + if m.static { + return + } select { case m.change <- true: default: @@ -290,6 +321,9 @@ func (m *Monitor) InjectEvent() { // This is like InjectEvent but only fires ChangeFunc callbacks // if the network state differed at all. func (m *Monitor) Poll() { + if m.static { + return + } select { case m.change <- false: default: diff --git a/net/netns/netns.go b/net/netns/netns.go index 2acb15129..efae1ba52 100644 --- a/net/netns/netns.go +++ b/net/netns/netns.go @@ -56,8 +56,10 @@ func SetDisableBindConnToInterface(v bool) { // Listener returns a new net.Listener with its Control hook func // initialized as necessary to run in logical network namespace that // doesn't route back into Tailscale. -// The netMon parameter is optional; if non-nil it's used to do faster interface lookups. func Listener(logf logger.Logf, netMon *netmon.Monitor) *net.ListenConfig { + if netMon == nil { + panic("netns.Listener called with nil netMon") + } if disabled.Load() { return new(net.ListenConfig) } @@ -68,8 +70,10 @@ func Listener(logf logger.Logf, netMon *netmon.Monitor) *net.ListenConfig { // hook func initialized as necessary to run in a logical network // namespace that doesn't route back into Tailscale. It also handles // using a SOCKS if configured in the environment with ALL_PROXY. -// The netMon parameter is optional; if non-nil it's used to do faster interface lookups. func NewDialer(logf logger.Logf, netMon *netmon.Monitor) Dialer { + if netMon == nil { + panic("netns.NewDialer called with nil netMon") + } return FromDialer(logf, netMon, &net.Dialer{ KeepAlive: netknob.PlatformTCPKeepAlive(), }) @@ -79,8 +83,10 @@ func NewDialer(logf logger.Logf, netMon *netmon.Monitor) Dialer { // network namespace that doesn't route back into Tailscale. It also // handles using a SOCKS if configured in the environment with // ALL_PROXY. -// The netMon parameter is optional; if non-nil it's used to do faster interface lookups. func FromDialer(logf logger.Logf, netMon *netmon.Monitor, d *net.Dialer) Dialer { + if netMon == nil { + panic("netns.FromDialer called with nil netMon") + } if disabled.Load() { return d } diff --git a/net/portmapper/igd_test.go b/net/portmapper/igd_test.go index 0b9d47664..5c24d03aa 100644 --- a/net/portmapper/igd_test.go +++ b/net/portmapper/igd_test.go @@ -16,6 +16,7 @@ "tailscale.com/control/controlknobs" "tailscale.com/net/netaddr" + "tailscale.com/net/netmon" "tailscale.com/syncs" "tailscale.com/types/logger" ) @@ -259,12 +260,13 @@ func (d *TestIGD) handlePCPQuery(pkt []byte, src netip.AddrPort) { func newTestClient(t *testing.T, igd *TestIGD) *Client { var c *Client - c = NewClient(t.Logf, nil, nil, new(controlknobs.Knobs), func() { + c = NewClient(t.Logf, netmon.NewStatic(), nil, new(controlknobs.Knobs), func() { t.Logf("port map changed") t.Logf("have mapping: %v", c.HaveMapping()) }) c.testPxPPort = igd.TestPxPPort() c.testUPnPPort = igd.TestUPnPPort() + c.netMon = netmon.NewStatic() c.SetGatewayLookupFunc(testIPAndGateway) return c } diff --git a/net/portmapper/portmapper.go b/net/portmapper/portmapper.go index 01ed150ca..8c42b17fe 100644 --- a/net/portmapper/portmapper.go +++ b/net/portmapper/portmapper.go @@ -198,8 +198,7 @@ func (m *pmpMapping) Release(ctx context.Context) { // NewClient returns a new portmapping client. // -// The netMon parameter is optional; if non-nil it's used to do faster interface -// lookups. +// The netMon parameter is required. // // The debug argument allows configuring the behaviour of the portmapper for // debugging; if nil, a sensible set of defaults will be used. @@ -211,10 +210,13 @@ func (m *pmpMapping) Release(ctx context.Context) { // whenever the port mapping status has changed. If nil, it doesn't make a // callback. func NewClient(logf logger.Logf, netMon *netmon.Monitor, debug *DebugKnobs, controlKnobs *controlknobs.Knobs, onChange func()) *Client { + if netMon == nil { + panic("nil netMon") + } ret := &Client{ logf: logf, netMon: netMon, - ipAndGateway: interfaces.LikelyHomeRouterIP, + ipAndGateway: interfaces.LikelyHomeRouterIP, // TODO(bradfitz): move this to netMon onChange: onChange, controlKnobs: controlKnobs, } diff --git a/net/tsdial/tsdial.go b/net/tsdial/tsdial.go index 1a5ca7052..80d208f4f 100644 --- a/net/tsdial/tsdial.go +++ b/net/tsdial/tsdial.go @@ -26,13 +26,27 @@ "tailscale.com/types/netmap" "tailscale.com/util/clientmetric" "tailscale.com/util/mak" + "tailscale.com/util/testenv" "tailscale.com/version" ) +// NewDialer returns a new Dialer that can dial out of tailscaled. +// Its exported fields should be set before use, if any. +func NewDialer(netMon *netmon.Monitor) *Dialer { + if netMon == nil { + panic("NewDialer: netMon is nil") + } + d := &Dialer{} + d.SetNetMon(netMon) + return d +} + // Dialer dials out of tailscaled, while taking care of details while // handling the dozens of edge cases depending on the server mode // (TUN, netstack), the OS network sandboxing style (macOS/iOS // Extension, none), user-selected route acceptance prefs, etc. +// +// Before use, SetNetMon should be called with a netmon.Monitor. type Dialer struct { Logf logger.Logf // UseNetstackForIP if non-nil is whether NetstackDialTCP (if @@ -130,9 +144,14 @@ func (d *Dialer) Close() error { return nil } +// SetNetMon sets d's network monitor to netMon. +// It is a no-op to call SetNetMon with the same netMon as the current one. func (d *Dialer) SetNetMon(netMon *netmon.Monitor) { d.mu.Lock() defer d.mu.Unlock() + if d.netMon == netMon { + return + } if d.netMonUnregister != nil { go d.netMonUnregister() d.netMonUnregister = nil @@ -141,6 +160,14 @@ func (d *Dialer) SetNetMon(netMon *netmon.Monitor) { d.netMonUnregister = d.netMon.RegisterChangeCallback(d.linkChanged) } +// NetMon returns the Dialer's network monitor. +// It returns nil if SetNetMon has not been called. +func (d *Dialer) NetMon() *netmon.Monitor { + d.mu.Lock() + defer d.mu.Unlock() + return d.netMon +} + var ( metricLinkChangeConnClosed = clientmetric.NewCounter("tsdial_linkchange_closes") metricChangeDeltaNoDefaultRoute = clientmetric.NewCounter("tsdial_changedelta_no_default_route") @@ -314,6 +341,13 @@ func (d *Dialer) logf(format string, args ...any) { // Control and (in the future, as of 2022-04-27) DERPs.. func (d *Dialer) SystemDial(ctx context.Context, network, addr string) (net.Conn, error) { d.mu.Lock() + if d.netMon == nil { + d.mu.Unlock() + if testenv.InTest() { + panic("SystemDial requires a netmon.Monitor; call SetNetMon first") + } + return nil, errors.New("SystemDial requires a netmon.Monitor; call SetNetMon first") + } closed := d.closed d.mu.Unlock() if closed { diff --git a/prober/derp_test.go b/prober/derp_test.go index 8afbffac7..a34292a23 100644 --- a/prober/derp_test.go +++ b/prober/derp_test.go @@ -16,6 +16,7 @@ "tailscale.com/derp" "tailscale.com/derp/derphttp" + "tailscale.com/net/netmon" "tailscale.com/tailcfg" "tailscale.com/types/key" ) @@ -140,7 +141,7 @@ func TestRunDerpProbeNodePair(t *testing.T) { } }() newClient := func() *derphttp.Client { - c, err := derphttp.NewClient(key.NewNode(), serverURL, t.Logf) + c, err := derphttp.NewClient(key.NewNode(), serverURL, t.Logf, netmon.NewStatic()) if err != nil { t.Fatalf("NewClient: %v", err) } diff --git a/wgengine/router/router.go b/wgengine/router/router.go index 9a81771a1..5200343cc 100644 --- a/wgengine/router/router.go +++ b/wgengine/router/router.go @@ -53,7 +53,7 @@ func New(logf logger.Logf, tundev tun.Device, netMon *netmon.Monitor, health *he // CleanUp restores the system network configuration to its original state // 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) { +func CleanUp(logf logger.Logf, netMon *netmon.Monitor, interfaceName string) { cleanUp(logf, interfaceName) } diff --git a/wgengine/userspace.go b/wgengine/userspace.go index 34eec9c16..3981a53ef 100644 --- a/wgengine/userspace.go +++ b/wgengine/userspace.go @@ -193,7 +193,7 @@ type Config struct { HealthTracker *health.Tracker // Dialer is the dialer to use for outbound connections. - // If nil, a new Dialer is created + // If nil, a new Dialer is created. Dialer *tsdial.Dialer // ControlKnobs is the set of control plane-provied knobs @@ -341,7 +341,7 @@ func NewUserspaceEngine(logf logger.Logf, conf Config) (_ Engine, reterr error) tunName, _ := conf.Tun.Name() conf.Dialer.SetTUNName(tunName) conf.Dialer.SetNetMon(e.netMon) - e.dns = dns.NewManager(logf, conf.DNS, e.netMon, e.health, conf.Dialer, fwdDNSLinkSelector{e, tunName}, conf.ControlKnobs) + e.dns = dns.NewManager(logf, conf.DNS, e.health, conf.Dialer, fwdDNSLinkSelector{e, tunName}, conf.ControlKnobs) // TODO: there's probably a better place for this sockstats.SetNetMon(e.netMon)