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
This commit is contained in:
parent
18e13f6ffa
commit
1d9900273e
@ -471,6 +471,160 @@ func TestSingleVsMultipleTags(t *testing.T) {
|
|||||||
assert.False(t, node2.HasTag("tag:other"))
|
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
|
// TestReAuthWithDifferentMachineKey tests the edge case where a node attempts
|
||||||
// to re-authenticate with the same NodeKey but a DIFFERENT MachineKey.
|
// to re-authenticate with the same NodeKey but a DIFFERENT MachineKey.
|
||||||
// This scenario should be handled gracefully (currently creates a new node).
|
// This scenario should be handled gracefully (currently creates a new node).
|
||||||
|
|||||||
@ -1142,6 +1142,9 @@ func (s *State) createAndSaveNewNode(params newNodeParams) (types.NodeView, erro
|
|||||||
nodeToRegister.User = params.PreAuthKey.User
|
nodeToRegister.User = params.PreAuthKey.User
|
||||||
}
|
}
|
||||||
// If PreAuthKey.UserID is nil, the node is "orphaned" (system-created)
|
// If PreAuthKey.UserID is nil, the node is "orphaned" (system-created)
|
||||||
|
|
||||||
|
// Tagged nodes have key expiry disabled.
|
||||||
|
nodeToRegister.Expiry = nil
|
||||||
} else {
|
} else {
|
||||||
// USER-OWNED NODE
|
// USER-OWNED NODE
|
||||||
nodeToRegister.UserID = ¶ms.PreAuthKey.User.ID
|
nodeToRegister.UserID = ¶ms.PreAuthKey.User.ID
|
||||||
@ -1311,10 +1314,14 @@ func (s *State) HandleNodeFromAuthPath(
|
|||||||
node.IsOnline = ptr.To(false)
|
node.IsOnline = ptr.To(false)
|
||||||
node.LastSeen = ptr.To(time.Now())
|
node.LastSeen = ptr.To(time.Now())
|
||||||
|
|
||||||
if expiry != nil {
|
// Tagged nodes keep their existing expiry (disabled).
|
||||||
node.Expiry = expiry
|
// User-owned nodes update expiry from the provided value or registration entry.
|
||||||
} else {
|
if !node.IsTagged() {
|
||||||
node.Expiry = regEntry.Node.Expiry
|
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.IsOnline = ptr.To(false)
|
||||||
node.LastSeen = ptr.To(time.Now())
|
node.LastSeen = ptr.To(time.Now())
|
||||||
|
|
||||||
// Update expiry, if it is zero, it means that the node will
|
// Tagged nodes keep their existing expiry (disabled).
|
||||||
// not have an expiry anymore. If it is non-zero, we set that.
|
// User-owned nodes update expiry from the client request.
|
||||||
node.Expiry = ®Req.Expiry
|
if !node.IsTagged() {
|
||||||
|
node.Expiry = ®Req.Expiry
|
||||||
|
}
|
||||||
})
|
})
|
||||||
|
|
||||||
if !ok {
|
if !ok {
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user