ipn: !WantRunning + !LoggedOut should not be idle on startup.

There was logic that would make a "down" tailscale backend (ie.
!WantRunning) refuse to do any network activity. Unfortunately, this
makes the macOS and iOS UI unable to render correctly if they start
while !WantRunning.

Now that we have Prefs.LoggedOut, use that instead. So `tailscale down`
will still allow the controlclient to connect its authroutine, but
pause the maproutine. `tailscale logout` will entirely stop all
activity.

This new behaviour is not obviously correct; it's a bit annoying that
`tailsale down` doesn't terminate all activity like you might expect.
Maybe we should redesign the UI code to render differently when
disconnected, and then revert this change.

Signed-off-by: Avery Pennarun <apenwarr@tailscale.com>

ipn/ipnlocal/local.go

@@ -809,17 +809,13 @@ func (b *LocalBackend) Start(opts ipn.Options) error {
 	b.send(ipn.Notify{BackendLogID: &blid})
 	b.send(ipn.Notify{Prefs: prefs})
 
-	if wantRunning && !loggedOut {
+	if !loggedOut && b.hasNodeKey() {
+		// Even if !WantRunning, we should verify our key, if there
+		// is one. If you want tailscaled to be completely idle,
+		// use logout instead.
 		cc.Login(nil, controlclient.LoginDefault)
-
-		b.mu.Lock()
-		b.state = ipn.Starting
-		b.mu.Unlock()
-
-		b.send(ipn.Notify{State: &b.state})
-	} else {
-		b.stateMachine()
 	}
+	b.stateMachine()
 	return nil
 }
 
@@ -2125,6 +2121,15 @@ func (b *LocalBackend) enterState(newState ipn.State) {
 
 }
 
+func (b *LocalBackend) hasNodeKey() bool {
+	// we can't use b.Prefs(), because it strips the keys, oops!
+	b.mu.Lock()
+	p := b.prefs
+	b.mu.Unlock()
+
+	return p.Persist != nil && !p.Persist.PrivateNodeKey.IsZero()
+}
+
 // nextState returns the state the backend seems to be in, based on
 // its internal state.
 func (b *LocalBackend) nextState() ipn.State {
@@ -2136,13 +2141,11 @@ func (b *LocalBackend) nextState() ipn.State {
 		state       = b.state
 		wantRunning = b.prefs.WantRunning
 		loggedOut   = b.prefs.LoggedOut
-		hasNodeKey  = b.prefs.Persist != nil &&
-			!b.prefs.Persist.PrivateNodeKey.IsZero()
 	)
 	b.mu.Unlock()
 
 	switch {
-	case !wantRunning && !loggedOut && hasNodeKey:
+	case !wantRunning && !loggedOut && b.hasNodeKey():
 		return ipn.Stopped
 	case netMap == nil:
 		if cc.AuthCantContinue() || loggedOut {

ipn/ipnlocal/state_test.go

@@ -283,8 +283,10 @@ func TestStateMachine(t *testing.T) {
 		cc.opts = opts
 		cc.logf = opts.Logf
 		cc.authBlocked = true
+		cc.persist = cc.opts.Persist
 		cc.mu.Unlock()
 
+		cc.logf("ccGen: new mockControl.")
 		cc.called("New")
 		return cc, nil
 	}
@@ -749,7 +751,7 @@ func TestStateMachine(t *testing.T) {
 		// NOTE: cc.Shutdown() is correct here, since we didn't call
 		// b.Shutdown() explicitly ourselves.
 		// BUG: UpdateEndpoints should be called here since we're not WantRunning.
-		assert.Equal([]string{"Shutdown", "New", "UpdateEndpoints", "pause"}, cc.getCalls())
+		assert.Equal([]string{"Shutdown", "New", "UpdateEndpoints", "Login", "pause"}, cc.getCalls())
 
 		nn := notifies.drain(2)
 		assert.Equal([]string{}, cc.getCalls())
@@ -780,7 +782,7 @@ func TestStateMachine(t *testing.T) {
 	// The last test case is the most common one: restarting when both
 	// logged in and WantRunning.
 	t.Logf("\n\nStart5")
-	notifies.expect(2)
+	notifies.expect(1)
 	assert.Nil(b.Start(ipn.Options{
 		StateKey: ipn.GlobalDaemonStateKey,
 	}))
@@ -789,27 +791,27 @@ func TestStateMachine(t *testing.T) {
 		// b.Shutdown() ourselves.
 		assert.Equal([]string{"Shutdown", "New", "UpdateEndpoints", "Login"}, cc.getCalls())
 
-		nn := notifies.drain(2)
+		nn := notifies.drain(1)
 		assert.Equal([]string{}, cc.getCalls())
 		assert.NotNil(nn[0].Prefs)
 		assert.Equal(false, nn[0].Prefs.LoggedOut)
 		assert.Equal(true, nn[0].Prefs.WantRunning)
-		assert.NotNil(nn[1].State)
-		assert.Equal(ipn.Starting, *nn[1].State)
-		assert.Equal(ipn.Starting, b.State())
+		assert.Equal(ipn.NoState, b.State())
 	}
 
 	// Control server accepts our valid key from before.
 	t.Logf("\n\nLoginFinished5")
-	notifies.expect(1)
+	notifies.expect(2)
 	cc.setAuthBlocked(false)
 	cc.send(nil, "", true, &netmap.NetworkMap{
 		MachineStatus: tailcfg.MachineAuthorized,
 	})
 	{
-		nn := notifies.drain(1)
-		assert.Equal([]string{}, cc.getCalls())
+		nn := notifies.drain(2)
+		assert.Equal([]string{"unpause"}, cc.getCalls())
 		assert.NotNil(nn[0].LoginFinished)
+		assert.NotNil(nn[1].State)
+		assert.Equal(ipn.Starting, *nn[1].State)
 		// NOTE: No prefs change this time. WantRunning stays true.
 		// We were in Starting in the first place, so that doesn't
 		// change either.