From 1d9900273e206cc4546d8587b84b38d88284b6a1 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Fri, 9 Jan 2026 10:28:50 +0000 Subject: [PATCH] state: disable key expiry for tagged nodes Nodes registered with tagged PreAuthKeys now have key expiry disabled, matching Tailscale's behavior. User-owned nodes continue to use the client-requested expiry. On re-authentication, tagged nodes preserve their disabled expiry while user-owned nodes can update their expiry from the client request. Fixes #2971 --- hscontrol/auth_tags_test.go | 154 ++++++++++++++++++++++++++++++++++++ hscontrol/state/state.go | 23 ++++-- 2 files changed, 170 insertions(+), 7 deletions(-) diff --git a/hscontrol/auth_tags_test.go b/hscontrol/auth_tags_test.go index 4bc32a6a..bbaa834b 100644 --- a/hscontrol/auth_tags_test.go +++ b/hscontrol/auth_tags_test.go @@ -471,6 +471,160 @@ func TestSingleVsMultipleTags(t *testing.T) { assert.False(t, node2.HasTag("tag:other")) } +// TestTaggedPreAuthKeyDisablesKeyExpiry tests that nodes registered with +// a tagged PreAuthKey have key expiry disabled (expiry is nil). +func TestTaggedPreAuthKeyDisablesKeyExpiry(t *testing.T) { + app := createTestApp(t) + + user := app.state.CreateUserForTest("tag-creator") + tags := []string{"tag:server", "tag:prod"} + + // Create a tagged PreAuthKey + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, tags) + require.NoError(t, err) + require.ElementsMatch(t, tags, pak.Tags) + + // Register a node using the tagged key + machineKey := key.NewMachine() + nodeKey := key.NewNode() + + // Client requests an expiry time, but for tagged nodes it should be ignored + clientRequestedExpiry := time.Now().Add(24 * time.Hour) + + regReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{ + AuthKey: pak.Key, + }, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "tagged-expiry-test", + }, + Expiry: clientRequestedExpiry, + } + + resp, err := app.handleRegisterWithAuthKey(regReq, machineKey.Public()) + require.NoError(t, err) + require.True(t, resp.MachineAuthorized) + + // Verify the node has key expiry DISABLED (expiry is nil/zero) + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + + // Critical assertion: Tagged nodes should have expiry disabled + assert.True(t, node.IsTagged(), "Node should be tagged") + assert.False(t, node.Expiry().Valid(), "Tagged node should have expiry disabled (nil)") +} + +// TestUntaggedPreAuthKeyPreservesKeyExpiry tests that nodes registered with +// an untagged PreAuthKey preserve the client's requested key expiry. +func TestUntaggedPreAuthKeyPreservesKeyExpiry(t *testing.T) { + app := createTestApp(t) + + user := app.state.CreateUserForTest("node-owner") + + // Create an untagged PreAuthKey + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, nil) + require.NoError(t, err) + require.Empty(t, pak.Tags, "PreAuthKey should not be tagged") + + // Register a node + machineKey := key.NewMachine() + nodeKey := key.NewNode() + + // Client requests an expiry time + clientRequestedExpiry := time.Now().Add(24 * time.Hour) + + regReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{ + AuthKey: pak.Key, + }, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "untagged-expiry-test", + }, + Expiry: clientRequestedExpiry, + } + + resp, err := app.handleRegisterWithAuthKey(regReq, machineKey.Public()) + require.NoError(t, err) + require.True(t, resp.MachineAuthorized) + + // Verify the node has the client's requested expiry + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + + // Critical assertion: User-owned nodes should preserve client expiry + assert.False(t, node.IsTagged(), "Node should not be tagged") + assert.True(t, node.Expiry().Valid(), "User-owned node should have expiry set") + // Allow some tolerance for test execution time + assert.WithinDuration(t, clientRequestedExpiry, node.Expiry().Get(), 5*time.Second, + "User-owned node should have the client's requested expiry") +} + +// TestTaggedNodeReauthPreservesDisabledExpiry tests that when a tagged node +// re-authenticates, the disabled expiry is preserved (not updated from client request). +func TestTaggedNodeReauthPreservesDisabledExpiry(t *testing.T) { + app := createTestApp(t) + + user := app.state.CreateUserForTest("tag-creator") + tags := []string{"tag:server"} + + // Create a reusable tagged PreAuthKey + pak, err := app.state.CreatePreAuthKey(user.TypedID(), true, false, nil, tags) + require.NoError(t, err) + + // Initial registration + machineKey := key.NewMachine() + nodeKey := key.NewNode() + + regReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{ + AuthKey: pak.Key, + }, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "tagged-reauth-test", + }, + Expiry: time.Now().Add(24 * time.Hour), + } + + resp, err := app.handleRegisterWithAuthKey(regReq, machineKey.Public()) + require.NoError(t, err) + require.True(t, resp.MachineAuthorized) + + // Verify initial registration has expiry disabled + node, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + require.True(t, node.IsTagged()) + require.False(t, node.Expiry().Valid(), "Initial registration should have expiry disabled") + + // Re-authenticate with a NEW expiry request (should be ignored for tagged nodes) + newRequestedExpiry := time.Now().Add(48 * time.Hour) + reAuthReq := tailcfg.RegisterRequest{ + Auth: &tailcfg.RegisterResponseAuth{ + AuthKey: pak.Key, + }, + NodeKey: nodeKey.Public(), + Hostinfo: &tailcfg.Hostinfo{ + Hostname: "tagged-reauth-test", + }, + Expiry: newRequestedExpiry, // Client requests new expiry + } + + reAuthResp, err := app.handleRegisterWithAuthKey(reAuthReq, machineKey.Public()) + require.NoError(t, err) + require.True(t, reAuthResp.MachineAuthorized) + + // Verify expiry is STILL disabled after re-auth + nodeAfterReauth, found := app.state.GetNodeByNodeKey(nodeKey.Public()) + require.True(t, found) + + // Critical assertion: Tagged node should preserve disabled expiry on re-auth + assert.True(t, nodeAfterReauth.IsTagged(), "Node should still be tagged") + assert.False(t, nodeAfterReauth.Expiry().Valid(), + "Tagged node should have expiry PRESERVED as disabled after re-auth") +} + // TestReAuthWithDifferentMachineKey tests the edge case where a node attempts // to re-authenticate with the same NodeKey but a DIFFERENT MachineKey. // This scenario should be handled gracefully (currently creates a new node). diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index b365269c..5ddb9c97 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -1142,6 +1142,9 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro nodeToRegister.User = params.PreAuthKey.User } // If PreAuthKey.UserID is nil, the node is "orphaned" (system-created) + + // Tagged nodes have key expiry disabled. + nodeToRegister.Expiry = nil } else { // USER-OWNED NODE nodeToRegister.UserID = ¶ms.PreAuthKey.User.ID @@ -1311,10 +1314,14 @@ func (s *State) HandleNodeFromAuthPath( node.IsOnline = ptr.To(false) node.LastSeen = ptr.To(time.Now()) - if expiry != nil { - node.Expiry = expiry - } else { - node.Expiry = regEntry.Node.Expiry + // Tagged nodes keep their existing expiry (disabled). + // User-owned nodes update expiry from the provided value or registration entry. + if !node.IsTagged() { + if expiry != nil { + node.Expiry = expiry + } else { + node.Expiry = regEntry.Node.Expiry + } } }) @@ -1542,9 +1549,11 @@ func (s *State) HandleNodeFromPreAuthKey( node.IsOnline = ptr.To(false) node.LastSeen = ptr.To(time.Now()) - // Update expiry, if it is zero, it means that the node will - // not have an expiry anymore. If it is non-zero, we set that. - node.Expiry = ®Req.Expiry + // Tagged nodes keep their existing expiry (disabled). + // User-owned nodes update expiry from the client request. + if !node.IsTagged() { + node.Expiry = ®Req.Expiry + } }) if !ok {