From a9b4bf1535e849cbfe9ea31cd8723b8961a0e58e Mon Sep 17 00:00:00 2001 From: Brad Fitzpatrick Date: Fri, 3 Jun 2022 12:08:33 -0700 Subject: [PATCH] ipn/ipnserver, cmd/tailscaled: fix peerapi on Windows We weren't wiring up netstack.Impl to the LocalBackend in some cases on Windows. This fixes Windows 7 when run as a service. Updates #4750 (fixes after pull in to corp repo) Change-Id: I9ce51b797710f2bedfa90545776b7628c7528e99 Signed-off-by: Brad Fitzpatrick --- cmd/tailscaled/depaware.txt | 2 +- cmd/tailscaled/tailscaled_windows.go | 33 ++++++++++++++-------------- ipn/ipnserver/server.go | 29 ++++++++++++------------ ipn/ipnserver/server_test.go | 8 ++++++- 4 files changed, 40 insertions(+), 32 deletions(-) diff --git a/cmd/tailscaled/depaware.txt b/cmd/tailscaled/depaware.txt index ba0de4bc2..9b8afbc25 100644 --- a/cmd/tailscaled/depaware.txt +++ b/cmd/tailscaled/depaware.txt @@ -282,7 +282,7 @@ tailscale.com/cmd/tailscaled dependencies: (generated by github.com/tailscale/de tailscale.com/wgengine/filter from tailscale.com/control/controlclient+ tailscale.com/wgengine/magicsock from tailscale.com/ipn/ipnlocal+ tailscale.com/wgengine/monitor from tailscale.com/control/controlclient+ - tailscale.com/wgengine/netstack from tailscale.com/cmd/tailscaled + tailscale.com/wgengine/netstack from tailscale.com/cmd/tailscaled+ tailscale.com/wgengine/router from tailscale.com/ipn/ipnlocal+ tailscale.com/wgengine/wgcfg from tailscale.com/ipn/ipnlocal+ tailscale.com/wgengine/wgcfg/nmcfg from tailscale.com/ipn/ipnlocal diff --git a/cmd/tailscaled/tailscaled_windows.go b/cmd/tailscaled/tailscaled_windows.go index 2e0d7cf2b..27f1c2742 100644 --- a/cmd/tailscaled/tailscaled_windows.go +++ b/cmd/tailscaled/tailscaled_windows.go @@ -264,15 +264,15 @@ func startIPNServer(ctx context.Context, logid string) error { } dialer := new(tsdial.Dialer) - getEngineRaw := func() (wgengine.Engine, error) { + getEngineRaw := func() (wgengine.Engine, *netstack.Impl, error) { dev, devName, err := tstun.New(logf, "Tailscale") if err != nil { - return nil, fmt.Errorf("TUN: %w", err) + return nil, nil, fmt.Errorf("TUN: %w", err) } r, err := router.New(logf, dev, nil) if err != nil { dev.Close() - return nil, fmt.Errorf("router: %w", err) + return nil, nil, fmt.Errorf("router: %w", err) } if wrapNetstack { r = netstack.NewSubnetRouterWrapper(r) @@ -281,7 +281,7 @@ func startIPNServer(ctx context.Context, logid string) error { if err != nil { r.Close() dev.Close() - return nil, fmt.Errorf("DNS: %w", err) + return nil, nil, fmt.Errorf("DNS: %w", err) } eng, err := wgengine.NewUserspaceEngine(logf, wgengine.Config{ Tun: dev, @@ -294,23 +294,24 @@ func startIPNServer(ctx context.Context, logid string) error { if err != nil { r.Close() dev.Close() - return nil, fmt.Errorf("engine: %w", err) + return nil, nil, fmt.Errorf("engine: %w", err) } ns, err := newNetstack(logf, dialer, eng) if err != nil { - return nil, fmt.Errorf("newNetstack: %w", err) + return nil, nil, fmt.Errorf("newNetstack: %w", err) } ns.ProcessLocalIPs = false ns.ProcessSubnets = wrapNetstack if err := ns.Start(); err != nil { - return nil, fmt.Errorf("failed to start netstack: %w", err) + return nil, nil, fmt.Errorf("failed to start netstack: %w", err) } - return wgengine.NewWatchdog(eng), nil + return wgengine.NewWatchdog(eng), ns, nil } type engineOrError struct { - Engine wgengine.Engine - Err error + Engine wgengine.Engine + Netstack *netstack.Impl + Err error } engErrc := make(chan engineOrError) t0 := time.Now() @@ -319,7 +320,7 @@ func startIPNServer(ctx context.Context, logid string) error { for try := 1; ; try++ { logf("tailscaled: getting engine... (try %v)", try) t1 := time.Now() - eng, err := getEngineRaw() + eng, ns, err := getEngineRaw() d, dt := time.Since(t1).Round(ms), time.Since(t1).Round(ms) if err != nil { logf("tailscaled: engine fetch error (try %v) in %v (total %v, sysUptime %v): %v", @@ -332,7 +333,7 @@ func startIPNServer(ctx context.Context, logid string) error { } } timer := time.NewTimer(5 * time.Second) - engErrc <- engineOrError{eng, err} + engErrc <- engineOrError{eng, ns, err} if err == nil { timer.Stop() return @@ -344,14 +345,14 @@ func startIPNServer(ctx context.Context, logid string) error { // getEngine is called by ipnserver to get the engine. It's // not called concurrently and is not called again once it // successfully returns an engine. - getEngine := func() (wgengine.Engine, error) { + getEngine := func() (wgengine.Engine, *netstack.Impl, error) { if msg := envknob.String("TS_DEBUG_WIN_FAIL"); msg != "" { - return nil, fmt.Errorf("pretending to be a service failure: %v", msg) + return nil, nil, fmt.Errorf("pretending to be a service failure: %v", msg) } for { res := <-engErrc if res.Engine != nil { - return res.Engine, nil + return res.Engine, res.Netstack, nil } if time.Since(t0) < time.Minute || windowsUptime() < 10*time.Minute { // Ignore errors during early boot. Windows 10 auto logs in the GUI @@ -362,7 +363,7 @@ func startIPNServer(ctx context.Context, logid string) error { } // Return nicer errors to users, annotated with logids, which helps // when they file bugs. - return nil, fmt.Errorf("%w\n\nlogid: %v", res.Err, logid) + return nil, nil, fmt.Errorf("%w\n\nlogid: %v", res.Err, logid) } } store, err := store.New(logf, statePathOrDefault()) diff --git a/ipn/ipnserver/server.go b/ipn/ipnserver/server.go index 47b088100..cd4094fef 100644 --- a/ipn/ipnserver/server.go +++ b/ipn/ipnserver/server.go @@ -51,6 +51,7 @@ import ( "tailscale.com/version/distro" "tailscale.com/wgengine" "tailscale.com/wgengine/monitor" + "tailscale.com/wgengine/netstack" ) // Options is the configuration of the Tailscale node agent. @@ -659,7 +660,7 @@ func (s *Server) writeToClients(n ipn.Notify) { // The getEngine func is called repeatedly, once per connection, until it returns an engine successfully. // // Deprecated: use New and Server.Run instead. -func Run(ctx context.Context, logf logger.Logf, ln net.Listener, store ipn.StateStore, linkMon *monitor.Mon, dialer *tsdial.Dialer, logid string, getEngine func() (wgengine.Engine, error), opts Options) error { +func Run(ctx context.Context, logf logger.Logf, ln net.Listener, store ipn.StateStore, linkMon *monitor.Mon, dialer *tsdial.Dialer, logid string, getEngine func() (wgengine.Engine, *netstack.Impl, error), opts Options) error { getEngine = getEngineUntilItWorksWrapper(getEngine) runDone := make(chan struct{}) defer close(runDone) @@ -706,7 +707,7 @@ func Run(ctx context.Context, logf logger.Logf, ln net.Listener, store ipn.State bo := backoff.NewBackoff("ipnserver", logf, 30*time.Second) var unservedConn net.Conn // if non-nil, accepted, but hasn't served yet - eng, err := getEngine() + eng, ns, err := getEngine() if err != nil { logf("ipnserver: initial getEngine call: %v", err) for i := 1; ctx.Err() == nil; i++ { @@ -717,7 +718,7 @@ func Run(ctx context.Context, logf logger.Logf, ln net.Listener, store ipn.State continue } logf("ipnserver: try%d: trying getEngine again...", i) - eng, err = getEngine() + eng, ns, err = getEngine() if err == nil { logf("%d: GetEngine worked; exiting failure loop", i) unservedConn = c @@ -747,6 +748,9 @@ func Run(ctx context.Context, logf logger.Logf, ln net.Listener, store ipn.State if err != nil { return err } + if ns != nil { + ns.SetLocalBackend(server.LocalBackend()) + } serverMu.Lock() serverOrNil = server serverMu.Unlock() @@ -996,29 +1000,26 @@ func BabysitProc(ctx context.Context, args []string, logf logger.Logf) { } } -// FixedEngine returns a func that returns eng and a nil error. -func FixedEngine(eng wgengine.Engine) func() (wgengine.Engine, error) { - return func() (wgengine.Engine, error) { return eng, nil } -} - // getEngineUntilItWorksWrapper returns a getEngine wrapper that does // not call getEngine concurrently and stops calling getEngine once // it's returned a working engine. -func getEngineUntilItWorksWrapper(getEngine func() (wgengine.Engine, error)) func() (wgengine.Engine, error) { +func getEngineUntilItWorksWrapper(getEngine func() (wgengine.Engine, *netstack.Impl, error)) func() (wgengine.Engine, *netstack.Impl, error) { var mu sync.Mutex var engGood wgengine.Engine - return func() (wgengine.Engine, error) { + var nsGood *netstack.Impl + return func() (wgengine.Engine, *netstack.Impl, error) { mu.Lock() defer mu.Unlock() if engGood != nil { - return engGood, nil + return engGood, nsGood, nil } - e, err := getEngine() + e, ns, err := getEngine() if err != nil { - return nil, err + return nil, nil, err } engGood = e - return e, nil + nsGood = ns + return e, ns, nil } } diff --git a/ipn/ipnserver/server_test.go b/ipn/ipnserver/server_test.go index 3580164ec..a7fb9cd82 100644 --- a/ipn/ipnserver/server_test.go +++ b/ipn/ipnserver/server_test.go @@ -17,6 +17,7 @@ import ( "tailscale.com/net/tsdial" "tailscale.com/safesocket" "tailscale.com/wgengine" + "tailscale.com/wgengine/netstack" ) func TestRunMultipleAccepts(t *testing.T) { @@ -75,6 +76,11 @@ func TestRunMultipleAccepts(t *testing.T) { } defer ln.Close() - err = ipnserver.Run(ctx, logTriggerTestf, ln, store, nil /* mon */, new(tsdial.Dialer), "dummy_logid", ipnserver.FixedEngine(eng), opts) + err = ipnserver.Run(ctx, logTriggerTestf, ln, store, nil /* mon */, new(tsdial.Dialer), "dummy_logid", FixedEngine(eng), opts) t.Logf("ipnserver.Run = %v", err) } + +// FixedEngine returns a func that returns eng and a nil error. +func FixedEngine(eng wgengine.Engine) func() (wgengine.Engine, *netstack.Impl, error) { + return func() (wgengine.Engine, *netstack.Impl, error) { return eng, nil, nil } +}