diff --git a/control/controlclient/auto.go b/control/controlclient/auto.go index 421bc0def..542426f38 100644 --- a/control/controlclient/auto.go +++ b/control/controlclient/auto.go @@ -472,15 +472,16 @@ func (mrs mapRoutineState) UpdateFullNetmap(nm *netmap.NetworkMap) { c.mu.Lock() c.inMapPoll = true c.expiry = nm.SelfKeyExpiry() - c.logf("[v1] mapRoutine: netmap received: loggedIn=%v inMapPoll=true", c.loggedIn) + stillAuthed := c.loggedIn + c.logf("[v1] mapRoutine: netmap received: loggedIn=%v inMapPoll=true", stillAuthed) // Reset the backoff timer if we got a netmap. mrs.bo.Reset() c.mu.Unlock() - // Always send status - sendStatus will check if we should forward the netmap - // based on loggedIn, hasNodeKey, and inMapPoll. - c.sendStatus("mapRoutine-got-netmap", nil, "", nm) + if stillAuthed { + c.sendStatus("mapRoutine-got-netmap", nil, "", nm) + } } func (mrs mapRoutineState) UpdateNetmapDelta(muts []netmap.NodeMutation) bool { @@ -613,16 +614,10 @@ func (c *Auto) mapRoutine() { c.mu.Lock() loggedIn := c.loggedIn + c.logf("[v1] mapRoutine: loggedIn=%v", loggedIn) ctx := c.mapCtx c.mu.Unlock() - // Check if we have a valid node key that could receive updates. - // Even if !loggedIn (e.g., key expired, waiting for interactive auth), - // we should still poll if we have credentials, because the server - // might send us a key extension notification. - _, hasNodeKey := c.direct.GetPersist().PublicNodeKeyOK() - c.logf("[v1] mapRoutine: loggedIn=%v hasNodeKey=%v", loggedIn, hasNodeKey) - report := func(err error, msg string) { c.logf("[v1] %s: %v", msg, err) err = fmt.Errorf("%s: %w", msg, err) @@ -633,8 +628,8 @@ func (c *Auto) mapRoutine() { } } - if !loggedIn && !hasNodeKey { - // No credentials at all, wait for auth to complete. + if !loggedIn { + // Wait for something interesting to happen c.mu.Lock() c.inMapPoll = false c.mu.Unlock() @@ -725,17 +720,14 @@ func (c *Auto) sendStatus(who string, err error, url string, nm *netmap.NetworkM loginGoal := c.loginGoal c.mu.Unlock() - // Check if we have a valid node key - if so, we should forward the netmap - // even if !loggedIn, to allow the backend to see key expiry changes. - _, hasNodeKey := c.direct.GetPersist().PublicNodeKeyOK() - c.logf("[v1] sendStatus: %s: loggedIn=%v inMapPoll=%v hasNodeKey=%v", who, loggedIn, inMapPoll, hasNodeKey) + c.logf("[v1] sendStatus: %s: loggedIn=%v inMapPoll=%v", who, loggedIn, inMapPoll) var p persist.PersistView - if nm != nil && (loggedIn || hasNodeKey) && inMapPoll { + if nm != nil && loggedIn && inMapPoll { p = c.direct.GetPersist() } else { // don't send netmap status, as it's misleading when we're - // not logged in and have no credentials. + // not logged in. nm = nil } newSt := &Status{ @@ -850,29 +842,7 @@ func (c *Auto) Login(flags LoginFlags) { c.loginGoal = &LoginGoal{ flags: flags, } - // If we have valid credentials (loggedIn=true) or a valid node key, - // don't cancel the map poll. This allows the client to continue receiving - // key extension notifications from the server while the auth flow proceeds - // in parallel. - // - // This is important for the "Extend key" feature: if the admin extends a - // key while the user has clicked "Login", we want the map poll to receive - // that notification and recover without requiring the user to complete the - // auth flow. - // - // The hasNodeKey check handles the case where a tsnet server restarts with - // an expired key: loggedIn is false (server returned AuthURL), but we have - // a valid node key that can still receive map updates including key extensions. - // - // "First successful flow wins": if a key extension arrives via map poll, - // the client recovers. If the auth flow completes first, that works too. - var hasNodeKey bool - if c.direct != nil { - _, hasNodeKey = c.direct.GetPersist().PublicNodeKeyOK() - } - if !c.loggedIn && !hasNodeKey { - c.cancelMapCtxLocked() - } + c.cancelMapCtxLocked() c.cancelAuthCtxLocked() } diff --git a/control/controlclient/key_expiry_test.go b/control/controlclient/key_expiry_test.go deleted file mode 100644 index 296ed3bad..000000000 --- a/control/controlclient/key_expiry_test.go +++ /dev/null @@ -1,152 +0,0 @@ -// Copyright (c) Tailscale Inc & contributors -// SPDX-License-Identifier: BSD-3-Clause - -package controlclient - -import ( - "context" - "testing" - - "tailscale.com/types/key" - "tailscale.com/types/persist" -) - -// TestLoginPreservesMapPollWhenLoggedIn tests the fix for the key extension bug. -// -// When a client has valid credentials (loggedIn=true) but needs to re-authenticate -// due to key expiry, calling Login() should NOT cancel the map poll. This allows -// the client to continue receiving key extension notifications from the server -// while the auth flow proceeds in parallel. -func TestLoginPreservesMapPollWhenLoggedIn(t *testing.T) { - // Create an Auto client that is already logged in - // This simulates a client with valid credentials but expired key - auto := &Auto{ - logf: t.Logf, - loggedIn: true, // Already authenticated (key expired, but creds valid) - closed: false, - } - auto.mapCtx, auto.mapCancel = context.WithCancel(context.Background()) - t.Cleanup(auto.mapCancel) - auto.authCtx, auto.authCancel = context.WithCancel(context.Background()) - t.Cleanup(auto.authCancel) - - originalMapCtx := auto.mapCtx - - // Call Login() - this is what happens when user clicks "Login" after key expiry - auto.Login(LoginInteractive) - - // The fix: when loggedIn=true, mapCtx should NOT be cancelled - // This allows the map poll to continue receiving key extension notifications - select { - case <-originalMapCtx.Done(): - t.Error("Login() cancelled mapCtx even though loggedIn=true; key extension notifications would be lost") - default: - // Good - map context still active - } - - // Verify loginGoal was set (auth flow can proceed in parallel) - auto.mu.Lock() - hasLoginGoal := auto.loginGoal != nil - auto.mu.Unlock() - - if !hasLoginGoal { - t.Error("loginGoal should be set even though mapCtx wasn't cancelled") - } -} - -// TestLoginPreservesMapPollWithNodeKey tests the tsnet restart scenario. -// -// When a tsnet server restarts with an expired key: -// 1. The server has a valid node key stored in persist -// 2. Control returns an AuthURL (for interactive login) -// 3. loggedIn is false (because TryLogin returned a URL, not success) -// 4. But we should NOT cancel the map poll, because the server might send -// a key extension notification via the existing node key -// -// This test verifies that Login() preserves the map poll when we have a -// valid node key, even if loggedIn=false. -func TestLoginPreservesMapPollWithNodeKey(t *testing.T) { - // Create persist data with a valid node key (simulating stored credentials) - nodeKey := key.NewNode() - p := &persist.Persist{ - PrivateNodeKey: nodeKey, - } - - // Create a Direct client with the persist data - direct := &Direct{ - persist: p.View(), - } - - // Create an Auto client that is NOT logged in but HAS a valid node key - // This simulates a tsnet server restart with expired key: - // - loggedIn=false because control returned an AuthURL - // - but we have a valid node key that can receive map updates - auto := &Auto{ - logf: t.Logf, - loggedIn: false, // Control returned AuthURL, so not "logged in" yet - closed: false, - direct: direct, - } - auto.mapCtx, auto.mapCancel = context.WithCancel(context.Background()) - t.Cleanup(auto.mapCancel) - auto.authCtx, auto.authCancel = context.WithCancel(context.Background()) - t.Cleanup(auto.authCancel) - - originalMapCtx := auto.mapCtx - - // Call Login() - this is what tsnet's StartLoginInteractive does - auto.Login(LoginInteractive) - - // The fix: even though loggedIn=false, we have a valid node key, - // so mapCtx should NOT be cancelled. This allows us to receive - // key extension notifications from the server. - select { - case <-originalMapCtx.Done(): - t.Error("Login() cancelled mapCtx even though we have a valid node key; " + - "key extension notifications would be lost in tsnet restart scenario") - default: - // Good - map context still active, can receive key extensions - } - - // Verify loginGoal was set (auth flow can proceed in parallel) - auto.mu.Lock() - hasLoginGoal := auto.loginGoal != nil - auto.mu.Unlock() - - if !hasLoginGoal { - t.Error("loginGoal should be set for the auth flow to proceed") - } -} - -// TestLoginCancelsMapPollWhenNoNodeKey verifies that when there's no node key -// at all (fresh install, never authenticated), Login() should cancel the map poll. -func TestLoginCancelsMapPollWhenNoNodeKey(t *testing.T) { - // Create a Direct client with empty persist (no node key) - direct := &Direct{ - persist: new(persist.Persist).View(), - } - - auto := &Auto{ - logf: t.Logf, - loggedIn: false, - closed: false, - direct: direct, - } - auto.mapCtx, auto.mapCancel = context.WithCancel(context.Background()) - t.Cleanup(auto.mapCancel) - auto.authCtx, auto.authCancel = context.WithCancel(context.Background()) - t.Cleanup(auto.authCancel) - - originalMapCtx := auto.mapCtx - - // Call Login() - auto.Login(LoginInteractive) - - // When loggedIn=false AND no node key, mapCtx SHOULD be cancelled - select { - case <-originalMapCtx.Done(): - // Good - cancelled as expected for fresh login with no credentials - default: - t.Error("mapCtx should be cancelled when loggedIn=false and no node key") - } -} diff --git a/ipn/ipnlocal/key_extension_test.go b/ipn/ipnlocal/key_extension_test.go deleted file mode 100644 index 389178751..000000000 --- a/ipn/ipnlocal/key_extension_test.go +++ /dev/null @@ -1,449 +0,0 @@ -// Copyright (c) Tailscale Inc & contributors -// SPDX-License-Identifier: BSD-3-Clause - -package ipnlocal - -import ( - "testing" - "time" - - qt "github.com/frankban/quicktest" - "tailscale.com/control/controlclient" - "tailscale.com/ipn" - "tailscale.com/ipn/store/mem" - "tailscale.com/tailcfg" - "tailscale.com/tsd" - "tailscale.com/tstest" - "tailscale.com/types/key" - "tailscale.com/types/logid" - "tailscale.com/types/netmap" - "tailscale.com/wgengine" -) - -// TestKeyExtensionWakesUpExpiredClient verifies that when a client is in NeedsLogin -// state due to key expiry, receiving a netmap with an extended (future) KeyExpiry -// correctly transitions the client back to a working state. -// -// This tests the key recovery path: client has expired key -> admin extends key -// -> server sends updated netmap -> client should recover. -func TestKeyExtensionWakesUpExpiredClient(t *testing.T) { - c := qt.New(t) - logf := tstest.WhileTestRunningLogger(t) - - // Setup test infrastructure - sys := tsd.NewSystem() - store := new(mem.Store) - sys.Set(store) - e, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set, sys.HealthTracker.Get(), sys.UserMetricsRegistry(), sys.Bus.Get()) - c.Assert(err, qt.IsNil) - t.Cleanup(e.Close) - sys.Set(e) - - b, err := NewLocalBackend(logf, logid.PublicID{}, sys, 0) - c.Assert(err, qt.IsNil) - t.Cleanup(b.Shutdown) - - var cc *mockControl - b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) { - cc = newClient(t, opts) - return cc, nil - }) - - // Start the backend - c.Assert(b.Start(ipn.Options{}), qt.IsNil) - - // Simulate successful login and authenticated state - cc.populateKeys() - nodeKey := key.NewNode().Public() - now := time.Now() - - // First, get to a Running state with a valid key - futureExpiry := now.Add(1 * time.Hour) - cc.send(sendOpt{ - loginFinished: true, - nm: &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - ID: 1, - Key: nodeKey, - MachineAuthorized: true, - KeyExpiry: futureExpiry, - }).View(), - }, - }) - - // Enable WantRunning - required for keyExpired to trigger NeedsLogin state - b.EditPrefs(&ipn.MaskedPrefs{ - WantRunningSet: true, - Prefs: ipn.Prefs{WantRunning: true}, - }) - - // Verify we're in a good state initially - b.mu.Lock() - c.Assert(b.keyExpired, qt.IsFalse, qt.Commentf("key should not be expired initially")) - b.mu.Unlock() - - // Now simulate key expiry by sending a netmap with past KeyExpiry - pastExpiry := now.Add(-1 * time.Hour) - cc.send(sendOpt{ - nm: &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - ID: 1, - Key: nodeKey, - MachineAuthorized: true, - KeyExpiry: pastExpiry, - }).View(), - }, - }) - - // Verify the client detects key expiry - b.mu.Lock() - c.Assert(b.keyExpired, qt.IsTrue, qt.Commentf("key should be detected as expired")) - b.mu.Unlock() - - // Verify state is NeedsLogin (requires WantRunning=true) - state := b.State() - c.Assert(state, qt.Equals, ipn.NeedsLogin, qt.Commentf("state should be NeedsLogin when key is expired and WantRunning=true")) - - // Set blocked to true to simulate the engine being blocked (as would happen - // when entering NeedsLogin due to key expiry in real flow) - b.mu.Lock() - b.blocked = true - b.mu.Unlock() - - // Now simulate admin extending the key - server sends new netmap with extended expiry - extendedExpiry := now.Add(30 * time.Minute) - cc.send(sendOpt{ - nm: &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - ID: 1, - Key: nodeKey, - MachineAuthorized: true, - KeyExpiry: extendedExpiry, - }).View(), - }, - }) - - // Verify the client recovers: - // 1. keyExpired should be false - b.mu.Lock() - c.Assert(b.keyExpired, qt.IsFalse, qt.Commentf("key should no longer be expired after extension")) - - // 2. blocked should be false (unblocked when key extended) - c.Assert(b.blocked, qt.IsFalse, qt.Commentf("engine should be unblocked after key extension")) - b.mu.Unlock() - - // 3. state should transition away from NeedsLogin - // Note: exact state depends on other factors (MachineAuthorized, etc.) - // but it should NOT be NeedsLogin anymore - state = b.State() - if state == ipn.NeedsLogin { - // Check if it's still NeedsLogin for a reason OTHER than key expiry - b.mu.Lock() - keyExp := b.keyExpired - b.mu.Unlock() - if !keyExp { - // Key is not expired, so NeedsLogin must be for another reason - // (which is acceptable in this test's context) - t.Logf("state is NeedsLogin but keyExpired=false, which is acceptable") - } else { - t.Errorf("state is still NeedsLogin with keyExpired=true after key extension") - } - } -} - -// TestKeyExpiredStateMachine verifies that when a key expires, the state machine -// correctly transitions to NeedsLogin and sets keyExpired=true. -func TestKeyExpiredStateMachine(t *testing.T) { - c := qt.New(t) - logf := tstest.WhileTestRunningLogger(t) - - sys := tsd.NewSystem() - store := new(mem.Store) - sys.Set(store) - e, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set, sys.HealthTracker.Get(), sys.UserMetricsRegistry(), sys.Bus.Get()) - c.Assert(err, qt.IsNil) - t.Cleanup(e.Close) - sys.Set(e) - - b, err := NewLocalBackend(logf, logid.PublicID{}, sys, 0) - c.Assert(err, qt.IsNil) - t.Cleanup(b.Shutdown) - - var cc *mockControl - b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) { - cc = newClient(t, opts) - return cc, nil - }) - - c.Assert(b.Start(ipn.Options{}), qt.IsNil) - - cc.populateKeys() - nodeKey := key.NewNode().Public() - now := time.Now() - - // Get to Running state with valid key - futureExpiry := now.Add(1 * time.Hour) - cc.send(sendOpt{ - loginFinished: true, - nm: &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - ID: 1, - Key: nodeKey, - MachineAuthorized: true, - KeyExpiry: futureExpiry, - }).View(), - }, - }) - - // Enable WantRunning - b.EditPrefs(&ipn.MaskedPrefs{ - WantRunningSet: true, - Prefs: ipn.Prefs{WantRunning: true}, - }) - - // Verify initial state - b.mu.Lock() - c.Assert(b.keyExpired, qt.IsFalse) - b.mu.Unlock() - - // Now expire the key - pastExpiry := now.Add(-1 * time.Hour) - cc.send(sendOpt{ - nm: &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - ID: 1, - Key: nodeKey, - MachineAuthorized: true, - KeyExpiry: pastExpiry, - }).View(), - }, - }) - - // Verify keyExpired is set - b.mu.Lock() - c.Assert(b.keyExpired, qt.IsTrue, qt.Commentf("keyExpired should be true after receiving expired KeyExpiry")) - b.mu.Unlock() - - // Verify state is NeedsLogin - c.Assert(b.State(), qt.Equals, ipn.NeedsLogin) -} - -// TestKeyExpiryExtendedUnblocksEngine verifies that when a key is extended, -// the engine is unblocked even if it was blocked due to key expiry. -func TestKeyExpiryExtendedUnblocksEngine(t *testing.T) { - c := qt.New(t) - logf := tstest.WhileTestRunningLogger(t) - - sys := tsd.NewSystem() - store := new(mem.Store) - sys.Set(store) - e, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set, sys.HealthTracker.Get(), sys.UserMetricsRegistry(), sys.Bus.Get()) - c.Assert(err, qt.IsNil) - t.Cleanup(e.Close) - sys.Set(e) - - b, err := NewLocalBackend(logf, logid.PublicID{}, sys, 0) - c.Assert(err, qt.IsNil) - t.Cleanup(b.Shutdown) - - var cc *mockControl - b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) { - cc = newClient(t, opts) - return cc, nil - }) - - c.Assert(b.Start(ipn.Options{}), qt.IsNil) - - cc.populateKeys() - nodeKey := key.NewNode().Public() - now := time.Now() - - // Get to authenticated state - cc.send(sendOpt{ - loginFinished: true, - nm: &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - ID: 1, - Key: nodeKey, - MachineAuthorized: true, - KeyExpiry: now.Add(1 * time.Hour), - }).View(), - }, - }) - - // Simulate key expiry - pastExpiry := now.Add(-1 * time.Hour) - cc.send(sendOpt{ - nm: &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - ID: 1, - Key: nodeKey, - MachineAuthorized: true, - KeyExpiry: pastExpiry, - }).View(), - }, - }) - - // Manually set blocked=true to simulate blocked state - b.mu.Lock() - b.blocked = true - wasBlocked := b.blocked - b.mu.Unlock() - c.Assert(wasBlocked, qt.IsTrue) - - // Extend the key - extendedExpiry := now.Add(30 * time.Minute) - cc.send(sendOpt{ - nm: &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - ID: 1, - Key: nodeKey, - MachineAuthorized: true, - KeyExpiry: extendedExpiry, - }).View(), - }, - }) - - // Verify engine is unblocked - b.mu.Lock() - c.Assert(b.blocked, qt.IsFalse, qt.Commentf("engine should be unblocked after key extension")) - c.Assert(b.keyExpired, qt.IsFalse, qt.Commentf("keyExpired should be false after extension")) - b.mu.Unlock() -} - -// TestKeyExpiryZeroMeansNoExpiry verifies that a zero KeyExpiry (used for -// tagged nodes or nodes with expiry disabled) is not treated as expired. -func TestKeyExpiryZeroMeansNoExpiry(t *testing.T) { - c := qt.New(t) - logf := tstest.WhileTestRunningLogger(t) - - sys := tsd.NewSystem() - store := new(mem.Store) - sys.Set(store) - e, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set, sys.HealthTracker.Get(), sys.UserMetricsRegistry(), sys.Bus.Get()) - c.Assert(err, qt.IsNil) - t.Cleanup(e.Close) - sys.Set(e) - - b, err := NewLocalBackend(logf, logid.PublicID{}, sys, 0) - c.Assert(err, qt.IsNil) - t.Cleanup(b.Shutdown) - - var cc *mockControl - b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) { - cc = newClient(t, opts) - return cc, nil - }) - - c.Assert(b.Start(ipn.Options{}), qt.IsNil) - - cc.populateKeys() - nodeKey := key.NewNode().Public() - - // Send netmap with zero KeyExpiry (like a tagged node) - cc.send(sendOpt{ - loginFinished: true, - nm: &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - ID: 1, - Key: nodeKey, - MachineAuthorized: true, - KeyExpiry: time.Time{}, // zero = no expiry - }).View(), - }, - }) - - // Verify key is NOT considered expired - b.mu.Lock() - c.Assert(b.keyExpired, qt.IsFalse, qt.Commentf("zero KeyExpiry should not be treated as expired")) - b.mu.Unlock() - - // State should not be NeedsLogin due to expiry - state := b.State() - c.Assert(state, qt.Not(qt.Equals), ipn.NeedsLogin, qt.Commentf("should not be in NeedsLogin with zero KeyExpiry")) -} - -// TestKeyExpiryWithNetMapUpdate verifies that key expiry detection works -// correctly across multiple netmap updates with varying expiry times. -func TestKeyExpiryWithNetMapUpdate(t *testing.T) { - c := qt.New(t) - logf := tstest.WhileTestRunningLogger(t) - - sys := tsd.NewSystem() - store := new(mem.Store) - sys.Set(store) - e, err := wgengine.NewFakeUserspaceEngine(logf, sys.Set, sys.HealthTracker.Get(), sys.UserMetricsRegistry(), sys.Bus.Get()) - c.Assert(err, qt.IsNil) - t.Cleanup(e.Close) - sys.Set(e) - - b, err := NewLocalBackend(logf, logid.PublicID{}, sys, 0) - c.Assert(err, qt.IsNil) - t.Cleanup(b.Shutdown) - - var cc *mockControl - b.SetControlClientGetterForTesting(func(opts controlclient.Options) (controlclient.Client, error) { - cc = newClient(t, opts) - return cc, nil - }) - - c.Assert(b.Start(ipn.Options{}), qt.IsNil) - - cc.populateKeys() - nodeKey := key.NewNode().Public() - now := time.Now() - - // Initial login with future expiry - futureExpiry := now.Add(24 * time.Hour) - cc.send(sendOpt{ - loginFinished: true, - nm: &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - ID: 1, - Key: nodeKey, - MachineAuthorized: true, - KeyExpiry: futureExpiry, - }).View(), - }, - }) - - b.mu.Lock() - c.Assert(b.keyExpired, qt.IsFalse) - b.mu.Unlock() - - // Simulate multiple netmap updates, tracking keyExpired state - testCases := []struct { - name string - expiry time.Time - wantExpired bool - }{ - {"still valid", now.Add(12 * time.Hour), false}, - {"expires soon", now.Add(5 * time.Minute), false}, - {"just expired", now.Add(-1 * time.Second), true}, - {"expired long ago", now.Add(-24 * time.Hour), true}, - {"extended again", now.Add(1 * time.Hour), false}, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - cc.send(sendOpt{ - nm: &netmap.NetworkMap{ - SelfNode: (&tailcfg.Node{ - ID: 1, - Key: nodeKey, - MachineAuthorized: true, - KeyExpiry: tc.expiry, - }).View(), - }, - }) - - b.mu.Lock() - gotExpired := b.keyExpired - b.mu.Unlock() - - c.Assert(gotExpired, qt.Equals, tc.wantExpired, - qt.Commentf("%s: expiry=%v, keyExpired=%v, want=%v", - tc.name, tc.expiry, gotExpired, tc.wantExpired)) - }) - } -} diff --git a/tsnet/tsnet_test.go b/tsnet/tsnet_test.go index 69b8531e4..2226b37d2 100644 --- a/tsnet/tsnet_test.go +++ b/tsnet/tsnet_test.go @@ -3884,132 +3884,3 @@ func TestListenMultipleEphemeralPorts(t *testing.T) { testMultipleEphemeral(t, lt) }) } - -// TestKeyExtensionAfterRestart verifies that a tsnet client with an expired node key -// that has launched into an interactive login after a restart recovers when the old -// key gets extended. -// -// See https://github.com/tailscale/tailscale/issues/19326. -func TestKeyExtensionAfterRestart(t *testing.T) { - ctx, cancel := context.WithTimeout(context.Background(), 120*time.Second) - defer cancel() - - controlURL, control := startControl(t) - control.RequireAuth = true - - tmp := filepath.Join(t.TempDir(), "s1") - os.MkdirAll(tmp, 0755) - - newServer := func() *Server { - s := &Server{ - Dir: tmp, - ControlURL: controlURL, - Hostname: "s1", - Logf: tstest.WhileTestRunningLogger(t), - } - t.Cleanup(func() { s.Close() }) - return s - } - - // Start a node as tsnet instance s1. - s1 := newServer() - if err := s1.Start(); err != nil { - t.Fatalf("s1.Start: %v", err) - } - upErrCh := make(chan error, 1) - go func() { _, err := s1.Up(ctx); upErrCh <- err }() - - var initialAuthURL string - if err := tstest.WaitFor(20*time.Second, func() error { - url := s1.lb.StatusWithoutPeers().AuthURL - if url == "" { - return errors.New("no AuthURL yet") - } - initialAuthURL = url - return nil - }); err != nil { - t.Fatalf("waiting for initial AuthURL: %v", err) - } - if !control.CompleteAuth(initialAuthURL) { - t.Fatal("failed to complete initial AuthURL") - } - select { - case err := <-upErrCh: - if err != nil { - t.Fatalf("s1.Up: %v", err) - } - case <-time.After(20 * time.Second): - t.Fatalf("timed out waiting for s1.Up to return, s1.lb.State()=%v", s1.lb.State()) - } - - nodePub := s1.lb.StatusWithoutPeers().Self.PublicKey - - // Expire s1's node key. - serverNode := control.Node(nodePub) - if serverNode == nil { - t.Fatalf("node %v not in control", nodePub) - } - serverNode.KeyExpiry = time.Now().Add(-time.Minute) - control.UpdateNode(serverNode) - - // Wait for s1 to transition away from the Running state. - if err := tstest.WaitFor(20*time.Second, func() error { - if got := s1.lb.State(); got == ipn.Running { - return errors.New("still Running") - } - return nil - }); err != nil { - t.Fatalf("waiting to transition away from Running: %v", err) - } - - if err := s1.Close(); err != nil { - t.Fatalf("s1.Close: %v", err) - } - - // Restart the node as tsnet instance s2. - s2 := newServer() - if err := s2.Start(); err != nil { - t.Fatalf("s2.Start: %v", err) - } - s2UpErrCh := make(chan error, 1) - go func() { _, err := s2.Up(ctx); s2UpErrCh <- err }() - - // Wait for s2 to transition into the NeedsLogin state. - var secondAuthURL string - if err := tstest.WaitFor(20*time.Second, func() error { - u := s2.lb.StatusWithoutPeers().AuthURL - if u == "" { - return errors.New("no AuthURL yet") - } - secondAuthURL = u - return nil - }); err != nil { - t.Fatalf("waiting for s2 AuthURL: %v", err) - } - // We deliberately do not complete the auth. - _ = secondAuthURL - - // Extend the old node key. - serverNode.KeyExpiry = time.Now().Add(24 * time.Hour) - control.UpdateNode(serverNode) - - // Wait for s2 to receive the netmap with the key extension info - // and transition to Running. - if err := tstest.WaitFor(20*time.Second, func() error { - if got := s2.lb.State(); got != ipn.Running { - return fmt.Errorf("in state %v; want Running", got) - } - return nil - }); err != nil { - t.Fatalf("waiting to return to Running after key extension: %v", err) - } - - select { - case err := <-s2UpErrCh: - if err != nil { - t.Fatalf("s2.Up: %v", err) - } - case <-time.After(20 * time.Second): - t.Fatalf("timed out waiting for s2.Up to return, s2.lb.State()=%v", s2.lb.State()) - } -}