From bb91bb842c189d0ebe7ea87e6744b5484f1f3511 Mon Sep 17 00:00:00 2001 From: Alex Chan Date: Tue, 14 Apr 2026 18:15:27 +0100 Subject: [PATCH] all: remove everything related to non-seamless key renewal Seamless key renewal has been the default in all clients since 1.90. We retained the ability to disable it from the control plane as a precaution, but we haven't seen any issues that require us to disable it. We're now removing all the code for non-seamless key renewal, because we don't expect to turn it on again, and indeed it's been untested in the field for three releases so might contain latent bugs! Updates tailscale/corp#33042 Change-Id: I4b80bf07a3a50298d1c303743484169accc8844b Signed-off-by: Alex Chan --- control/controlknobs/controlknobs.go | 23 ----- ipn/ipnlocal/local.go | 23 ++--- ipn/ipnlocal/state_test.go | 115 ++++------------------- tailcfg/tailcfg.go | 15 --- tstest/integration/integration_test.go | 125 +++++++++++-------------- 5 files changed, 82 insertions(+), 219 deletions(-) diff --git a/control/controlknobs/controlknobs.go b/control/controlknobs/controlknobs.go index 14f30d9ce..0e8051210 100644 --- a/control/controlknobs/controlknobs.go +++ b/control/controlknobs/controlknobs.go @@ -62,12 +62,6 @@ type Knobs struct { // netfiltering, unless overridden by the user. LinuxForceNfTables atomic.Bool - // SeamlessKeyRenewal is whether to renew node keys without breaking connections. - // This is enabled by default in 1.90 and later, but we but we can remotely disable - // it from the control plane if there's a problem. - // http://go/seamless-key-renewal - SeamlessKeyRenewal atomic.Bool - // ProbeUDPLifetime is whether the node should probe UDP path lifetime on // the tail end of an active direct connection in magicsock. ProbeUDPLifetime atomic.Bool @@ -142,8 +136,6 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { silentDisco = has(tailcfg.NodeAttrSilentDisco) forceIPTables = has(tailcfg.NodeAttrLinuxMustUseIPTables) forceNfTables = has(tailcfg.NodeAttrLinuxMustUseNfTables) - seamlessKeyRenewal = has(tailcfg.NodeAttrSeamlessKeyRenewal) - disableSeamlessKeyRenewal = has(tailcfg.NodeAttrDisableSeamlessKeyRenewal) probeUDPLifetime = has(tailcfg.NodeAttrProbeUDPLifetime) appCStoreRoutes = has(tailcfg.NodeAttrStoreAppCRoutes) userDialUseRoutes = has(tailcfg.NodeAttrUserDialUseRoutes) @@ -181,21 +173,6 @@ func (k *Knobs) UpdateFromNodeAttributes(capMap tailcfg.NodeCapMap) { k.DisableSkipStatusQueue.Store(disableSkipStatusQueue) k.DisableHostsFileUpdates.Store(disableHostsFileUpdates) k.ForceRegisterMagicDNSIPv4Only.Store(forceRegisterMagicDNSIPv4Only) - - // If both attributes are present, then "enable" should win. This reflects - // the history of seamless key renewal. - // - // Before 1.90, seamless was a private alpha, opt-in feature. Devices would - // only seamless do if customers opted in using the seamless renewal attr. - // - // In 1.90 and later, seamless is the default behaviour, and devices will use - // seamless unless explicitly told not to by control (e.g. if we discover - // a bug and want clients to use the prior behaviour). - // - // If a customer has opted in to the pre-1.90 seamless implementation, we - // don't want to switch it off for them -- we only want to switch it off for - // devices that haven't opted in. - k.SeamlessKeyRenewal.Store(seamlessKeyRenewal || !disableSeamlessKeyRenewal) } // AsDebugJSON returns k as something that can be marshalled with json.Marshal diff --git a/ipn/ipnlocal/local.go b/ipn/ipnlocal/local.go index 31354aba6..b09ddac86 100644 --- a/ipn/ipnlocal/local.go +++ b/ipn/ipnlocal/local.go @@ -3571,12 +3571,11 @@ func (b *LocalBackend) setAuthURLLocked(url string) { // // b.mu must be held. func (b *LocalBackend) popBrowserAuthNowLocked(url string, keyExpired bool, recipient ipnauth.Actor) { - b.logf("popBrowserAuthNow(%q): url=%v, key-expired=%v, seamless-key-renewal=%v", maybeUsernameOf(recipient), url != "", keyExpired, b.seamlessRenewalEnabled()) + b.logf("popBrowserAuthNow(%q): url=%v, key-expired=%v", maybeUsernameOf(recipient), url != "", keyExpired) - // Deconfigure the local network data plane if: - // - seamless key renewal is not enabled; - // - key is expired (in which case tailnet connectivity is down anyway). - if !b.seamlessRenewalEnabled() || keyExpired { + // Deconfigure the local network data plane if the key is expired + // (in which case tailnet connectivity is down anyway). + if keyExpired { b.blockEngineUpdatesLocked(true) b.stopEngineAndWaitLocked() @@ -5938,9 +5937,9 @@ func (b *LocalBackend) enterStateLocked(newState ipn.State) { switch newState { case ipn.NeedsLogin: feature.SystemdStatus("Needs login: %s", authURL) - // always block updates on NeedsLogin even if seamless renewal is enabled, - // to prevent calls to authReconfigLocked from reconfiguring the engine when our - // key has expired and we're waiting to authenticate to use the new key. + // always block updates on NeedsLogin, to prevent calls to authReconfigLocked + // from reconfiguring the engine when our key has expired and we're waiting + // to authenticate to use the new key. b.blockEngineUpdatesLocked(true) fallthrough case ipn.Stopped, ipn.NoState: @@ -7598,14 +7597,6 @@ func (b *LocalBackend) ReadRouteInfo() (*appctype.RouteInfo, error) { return b.readRouteInfoLocked() } -// seamlessRenewalEnabled reports whether seamless key renewals are enabled. -// -// As of 2025-09-11, this is the default behaviour unless nodes receive -// [tailcfg.NodeAttrDisableSeamlessKeyRenewal] in their netmap. -func (b *LocalBackend) seamlessRenewalEnabled() bool { - return b.ControlKnobs().SeamlessKeyRenewal.Load() -} - var ( disallowedAddrs = []netip.Addr{ netip.MustParseAddr("::1"), diff --git a/ipn/ipnlocal/state_test.go b/ipn/ipnlocal/state_test.go index 7e92647a6..7646ed764 100644 --- a/ipn/ipnlocal/state_test.go +++ b/ipn/ipnlocal/state_test.go @@ -373,14 +373,6 @@ func (b *LocalBackend) nonInteractiveLoginForStateTest() { // predictable, but maybe a bit less thorough. This is more of an overall // state machine test than a test of the wgengine+magicsock integration. func TestStateMachine(t *testing.T) { - runTestStateMachine(t, false) -} - -func TestStateMachineSeamless(t *testing.T) { - runTestStateMachine(t, true) -} - -func runTestStateMachine(t *testing.T, seamless bool) { envknob.Setenv("TAILSCALE_USE_WIP_CODE", "1") defer envknob.Setenv("TAILSCALE_USE_WIP_CODE", "") c := qt.New(t) @@ -590,12 +582,6 @@ func runTestStateMachine(t *testing.T, seamless bool) { cc.persist.UserProfile.LoginName = "user1" cc.persist.NodeID = "node1" - // even if seamless is being enabled by default rather than by policy, this is - // the point where it will first get enabled. - if seamless { - sys.ControlKnobs().SeamlessKeyRenewal.Store(true) - } - cc.send(sendOpt{loginFinished: true, nm: &netmap.NetworkMap{}}) { nn := notifies.drain(3) @@ -1480,11 +1466,23 @@ func TestEngineReconfigOnStateChange(t *testing.T) { lb.StartLoginInteractive(context.Background()) cc().sendAuthURL(node1) }, - // Without seamless renewal, even starting a reauth tears down everything: - wantState: ipn.Starting, - wantCfg: &wgcfg.Config{}, - wantRouterCfg: &router.Config{}, - wantDNSCfg: &dns.Config{}, + // Starting a reauth should leave everything up: + wantState: ipn.Starting, + wantCfg: &wgcfg.Config{ + Peers: []wgcfg.Peer{}, + Addresses: node1.SelfNode.Addresses().AsSlice(), + }, + wantRouterCfg: &router.Config{ + SNATSubnetRoutes: true, + NetfilterMode: preftype.NetfilterOn, + LocalAddrs: node1.SelfNode.Addresses().AsSlice(), + Routes: routesWithQuad100(), + }, + wantDNSCfg: &dns.Config{ + AcceptDNS: true, + Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, + Hosts: hostsFor(node1), + }, }, { name: "Start/Connect/Login/InitReauth/Login", @@ -1518,71 +1516,8 @@ func TestEngineReconfigOnStateChange(t *testing.T) { }, }, { - name: "Seamless/Start/Connect/Login/InitReauth", + name: "Start/Connect/Login/Expire", steps: func(t *testing.T, lb *LocalBackend, cc func() *mockControl) { - lb.ControlKnobs().SeamlessKeyRenewal.Store(true) - mustDo(t)(lb.Start(ipn.Options{})) - mustDo2(t)(lb.EditPrefs(connect)) - cc().authenticated(node1) - - // Start the re-auth process: - lb.StartLoginInteractive(context.Background()) - cc().sendAuthURL(node1) - }, - // With seamless renewal, starting a reauth should leave everything up: - wantState: ipn.Starting, - wantCfg: &wgcfg.Config{ - Peers: []wgcfg.Peer{}, - Addresses: node1.SelfNode.Addresses().AsSlice(), - }, - wantRouterCfg: &router.Config{ - SNATSubnetRoutes: true, - NetfilterMode: preftype.NetfilterOn, - LocalAddrs: node1.SelfNode.Addresses().AsSlice(), - Routes: routesWithQuad100(), - }, - wantDNSCfg: &dns.Config{ - AcceptDNS: true, - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, - Hosts: hostsFor(node1), - }, - }, - { - name: "Seamless/Start/Connect/Login/InitReauth/Login", - steps: func(t *testing.T, lb *LocalBackend, cc func() *mockControl) { - lb.ControlKnobs().SeamlessKeyRenewal.Store(true) - mustDo(t)(lb.Start(ipn.Options{})) - mustDo2(t)(lb.EditPrefs(connect)) - cc().authenticated(node1) - - // Start the re-auth process: - lb.StartLoginInteractive(context.Background()) - cc().sendAuthURL(node1) - - // Complete the re-auth process: - cc().authenticated(node1) - }, - wantState: ipn.Starting, - wantCfg: &wgcfg.Config{ - Peers: []wgcfg.Peer{}, - Addresses: node1.SelfNode.Addresses().AsSlice(), - }, - wantRouterCfg: &router.Config{ - SNATSubnetRoutes: true, - NetfilterMode: preftype.NetfilterOn, - LocalAddrs: node1.SelfNode.Addresses().AsSlice(), - Routes: routesWithQuad100(), - }, - wantDNSCfg: &dns.Config{ - AcceptDNS: true, - Routes: map[dnsname.FQDN][]*dnstype.Resolver{}, - Hosts: hostsFor(node1), - }, - }, - { - name: "Seamless/Start/Connect/Login/Expire", - steps: func(t *testing.T, lb *LocalBackend, cc func() *mockControl) { - lb.ControlKnobs().SeamlessKeyRenewal.Store(true) mustDo(t)(lb.Start(ipn.Options{})) mustDo2(t)(lb.EditPrefs(connect)) cc().authenticated(node1) @@ -1592,7 +1527,7 @@ func TestEngineReconfigOnStateChange(t *testing.T) { }).View(), }}) }, - // Even with seamless, if the key we are using expires, we want to disconnect: + // If the key we are using expires, we want to disconnect: wantState: ipn.NeedsLogin, wantCfg: &wgcfg.Config{}, wantRouterCfg: &router.Config{}, @@ -1641,14 +1576,6 @@ func TestEngineReconfigOnStateChange(t *testing.T) { // TestSendPreservesAuthURL tests that wgengine updates arriving in the middle of // processing an auth URL doesn't result in the auth URL being cleared. func TestSendPreservesAuthURL(t *testing.T) { - runTestSendPreservesAuthURL(t, false) -} - -func TestSendPreservesAuthURLSeamless(t *testing.T) { - runTestSendPreservesAuthURL(t, true) -} - -func runTestSendPreservesAuthURL(t *testing.T, seamless bool) { var cc *mockControl b := newLocalBackendWithTestControl(t, true, func(tb testing.TB, opts controlclient.Options) controlclient.Client { cc = newClient(t, opts) @@ -1667,10 +1594,6 @@ func runTestSendPreservesAuthURL(t *testing.T, seamless bool) { cc.persist.UserProfile.LoginName = "user1" cc.persist.NodeID = "node1" - if seamless { - b.sys.ControlKnobs().SeamlessKeyRenewal.Store(true) - } - cc.send(sendOpt{loginFinished: true, nm: &netmap.NetworkMap{ SelfNode: (&tailcfg.Node{MachineAuthorized: true}).View(), }}) diff --git a/tailcfg/tailcfg.go b/tailcfg/tailcfg.go index da218837a..91d23a5a6 100644 --- a/tailcfg/tailcfg.go +++ b/tailcfg/tailcfg.go @@ -2600,21 +2600,6 @@ const ( // This cannot be set simultaneously with NodeAttrLinuxMustUseIPTables. NodeAttrLinuxMustUseNfTables NodeCapability = "linux-netfilter?v=nftables" - // NodeAttrDisableSeamlessKeyRenewal disables seamless key renewal, which is - // enabled by default in clients as of 2025-09-17 (1.90 and later). - // - // We will use this attribute to manage the rollout, and disable seamless in - // clients with known bugs. - // http://go/seamless-key-renewal - NodeAttrDisableSeamlessKeyRenewal NodeCapability = "disable-seamless-key-renewal" - - // NodeAttrSeamlessKeyRenewal was used to opt-in to seamless key renewal - // during its private alpha. - // - // Deprecated: NodeAttrSeamlessKeyRenewal is deprecated as of CapabilityVersion 126, - // because seamless key renewal is now enabled by default. - NodeAttrSeamlessKeyRenewal NodeCapability = "seamless-key-renewal" - // NodeAttrProbeUDPLifetime makes the client probe UDP path lifetime at the // tail end of an active direct connection in magicsock. NodeAttrProbeUDPLifetime NodeCapability = "probe-udp-lifetime" diff --git a/tstest/integration/integration_test.go b/tstest/integration/integration_test.go index 0d0fdeeef..3064d6a26 100644 --- a/tstest/integration/integration_test.go +++ b/tstest/integration/integration_test.go @@ -467,83 +467,70 @@ func TestOneNodeUpAuth(t *testing.T) { }, } { tstest.Shard(t) + t.Run(tt.name, func(t *testing.T) { + tstest.Parallel(t) - for _, useSeamlessKeyRenewal := range []bool{true, false} { - name := tt.name - if useSeamlessKeyRenewal { - name += "-with-seamless" - } - t.Run(name, func(t *testing.T) { - tstest.Parallel(t) - - env := NewTestEnv(t, ConfigureControl( - func(control *testcontrol.Server) { - if tt.authKey != "" { - control.RequireAuthKey = tt.authKey - } else { - control.RequireAuth = true - } - - if tt.requireDeviceApproval { - control.RequireMachineAuth = true - } - - control.AllNodesSameUser = true - - if useSeamlessKeyRenewal { - control.DefaultNodeCapabilities = &tailcfg.NodeCapMap{ - tailcfg.NodeAttrSeamlessKeyRenewal: []tailcfg.RawMessage{}, - } - } - }, - )) - - n1 := NewTestNode(t, env) - d1 := n1.StartDaemon() - defer d1.MustCleanShutdown(t) - - for i, step := range tt.steps { - t.Logf("Running step %d", i) - cmdArgs := append(step.args, "--login-server="+env.ControlURL()) - - t.Logf("Running command: %s", strings.Join(cmdArgs, " ")) - - var authURLCount atomic.Int32 - var deviceApprovalURLCount atomic.Int32 - - handler := &authURLParserWriter{t: t, - authURLFn: completeLogin(t, env.Control, &authURLCount), - deviceApprovalURLFn: completeDeviceApproval(t, n1, &deviceApprovalURLCount), + env := NewTestEnv(t, ConfigureControl( + func(control *testcontrol.Server) { + if tt.authKey != "" { + control.RequireAuthKey = tt.authKey + } else { + control.RequireAuth = true } - cmd := n1.Tailscale(cmdArgs...) - cmd.Stdout = handler - cmd.Stdout = handler - cmd.Stderr = cmd.Stdout - if err := cmd.Run(); err != nil { - t.Fatalf("up: %v", err) + if tt.requireDeviceApproval { + control.RequireMachineAuth = true } - n1.AwaitRunning() + control.AllNodesSameUser = true + }, + )) - var wantAuthURLCount int32 - if step.wantAuthURL { - wantAuthURLCount = 1 - } - if n := authURLCount.Load(); n != wantAuthURLCount { - t.Errorf("Auth URLs completed = %d; want %d", n, wantAuthURLCount) - } + n1 := NewTestNode(t, env) + d1 := n1.StartDaemon() + defer d1.MustCleanShutdown(t) - var wantDeviceApprovalURLCount int32 - if step.wantDeviceApprovalURL { - wantDeviceApprovalURLCount = 1 - } - if n := deviceApprovalURLCount.Load(); n != wantDeviceApprovalURLCount { - t.Errorf("Device approval URLs completed = %d; want %d", n, wantDeviceApprovalURLCount) - } + for i, step := range tt.steps { + t.Logf("Running step %d", i) + cmdArgs := append(step.args, "--login-server="+env.ControlURL()) + + t.Logf("Running command: %s", strings.Join(cmdArgs, " ")) + + var authURLCount atomic.Int32 + var deviceApprovalURLCount atomic.Int32 + + handler := &authURLParserWriter{t: t, + authURLFn: completeLogin(t, env.Control, &authURLCount), + deviceApprovalURLFn: completeDeviceApproval(t, n1, &deviceApprovalURLCount), } - }) - } + + cmd := n1.Tailscale(cmdArgs...) + cmd.Stdout = handler + cmd.Stdout = handler + cmd.Stderr = cmd.Stdout + if err := cmd.Run(); err != nil { + t.Fatalf("up: %v", err) + } + + n1.AwaitRunning() + + var wantAuthURLCount int32 + if step.wantAuthURL { + wantAuthURLCount = 1 + } + if n := authURLCount.Load(); n != wantAuthURLCount { + t.Errorf("Auth URLs completed = %d; want %d", n, wantAuthURLCount) + } + + var wantDeviceApprovalURLCount int32 + if step.wantDeviceApprovalURL { + wantDeviceApprovalURLCount = 1 + } + if n := deviceApprovalURLCount.Load(); n != wantDeviceApprovalURLCount { + t.Errorf("Device approval URLs completed = %d; want %d", n, wantDeviceApprovalURLCount) + } + } + }) } }