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 <[email protected]>

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

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())

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
 	}
 }
 

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 }
+}