mirror of
https://github.com/juanfont/headscale.git
synced 2026-07-08 17:10:21 +09:00
policy: key autogroup:self invalidation on UserID not User view
invalidateAutogroupSelfCache derived the owning user from node.User().ID(), dereferencing the User association view. The NodeStore stores nodes by value with User as a *User pointer, and not every write path hydrates that association, so a non-tagged node could reach SetNodes with UserID set but User nil. UserView.ID() then dereferenced a nil *User and panicked on /machine/map whenever an autogroup:self policy was active and a node restarted tailscaled. Group affected nodes by node.TypedUserID(), which reads the UserID field directly. UserID is the authoritative ownership field for non-tagged nodes, so this needs no hydrated association and fixes every .User().ID() site in the function.
This commit is contained in:
@@ -1299,13 +1299,18 @@ func (pm *PolicyManager) invalidateAutogroupSelfCache(oldNodes, newNodes views.S
|
|||||||
// Tagged nodes don't participate in autogroup:self (identity is tag-based),
|
// Tagged nodes don't participate in autogroup:self (identity is tag-based),
|
||||||
// so we skip them when collecting affected users, except when tag status changes
|
// so we skip them when collecting affected users, except when tag status changes
|
||||||
// (which affects the user's device set).
|
// (which affects the user's device set).
|
||||||
affectedUsers := make(map[uint]struct{})
|
//
|
||||||
|
// Ownership is keyed on TypedUserID (the UserID field), not the User
|
||||||
|
// association view: the NodeStore holds nodes by value with User as a
|
||||||
|
// *User pointer, and not every write path hydrates that association. A
|
||||||
|
// non-tagged node always has UserID set, so it is the reliable owner key.
|
||||||
|
affectedUsers := make(map[types.UserID]struct{})
|
||||||
|
|
||||||
// Check for removed nodes (only non-tagged nodes affect autogroup:self)
|
// Check for removed nodes (only non-tagged nodes affect autogroup:self)
|
||||||
for nodeID, oldNode := range oldNodeMap {
|
for nodeID, oldNode := range oldNodeMap {
|
||||||
if _, exists := newNodeMap[nodeID]; !exists {
|
if _, exists := newNodeMap[nodeID]; !exists {
|
||||||
if !oldNode.IsTagged() {
|
if !oldNode.IsTagged() {
|
||||||
affectedUsers[oldNode.User().ID()] = struct{}{}
|
affectedUsers[oldNode.TypedUserID()] = struct{}{}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1314,7 +1319,7 @@ func (pm *PolicyManager) invalidateAutogroupSelfCache(oldNodes, newNodes views.S
|
|||||||
for nodeID, newNode := range newNodeMap {
|
for nodeID, newNode := range newNodeMap {
|
||||||
if _, exists := oldNodeMap[nodeID]; !exists {
|
if _, exists := oldNodeMap[nodeID]; !exists {
|
||||||
if !newNode.IsTagged() {
|
if !newNode.IsTagged() {
|
||||||
affectedUsers[newNode.User().ID()] = struct{}{}
|
affectedUsers[newNode.TypedUserID()] = struct{}{}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1327,10 +1332,10 @@ func (pm *PolicyManager) invalidateAutogroupSelfCache(oldNodes, newNodes views.S
|
|||||||
if oldNode.IsTagged() != newNode.IsTagged() {
|
if oldNode.IsTagged() != newNode.IsTagged() {
|
||||||
if !oldNode.IsTagged() {
|
if !oldNode.IsTagged() {
|
||||||
// Was untagged, now tagged: user lost a device
|
// Was untagged, now tagged: user lost a device
|
||||||
affectedUsers[oldNode.User().ID()] = struct{}{}
|
affectedUsers[oldNode.TypedUserID()] = struct{}{}
|
||||||
} else {
|
} else {
|
||||||
// Was tagged, now untagged: user gained a device
|
// Was tagged, now untagged: user gained a device
|
||||||
affectedUsers[newNode.User().ID()] = struct{}{}
|
affectedUsers[newNode.TypedUserID()] = struct{}{}
|
||||||
}
|
}
|
||||||
|
|
||||||
continue
|
continue
|
||||||
@@ -1342,9 +1347,9 @@ func (pm *PolicyManager) invalidateAutogroupSelfCache(oldNodes, newNodes views.S
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Check if user changed (both versions are non-tagged here)
|
// Check if user changed (both versions are non-tagged here)
|
||||||
if oldNode.User().ID() != newNode.User().ID() {
|
if oldNode.TypedUserID() != newNode.TypedUserID() {
|
||||||
affectedUsers[oldNode.User().ID()] = struct{}{}
|
affectedUsers[oldNode.TypedUserID()] = struct{}{}
|
||||||
affectedUsers[newNode.User().ID()] = struct{}{}
|
affectedUsers[newNode.TypedUserID()] = struct{}{}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Check if IPs changed (simple check - could be more sophisticated)
|
// Check if IPs changed (simple check - could be more sophisticated)
|
||||||
@@ -1352,12 +1357,12 @@ func (pm *PolicyManager) invalidateAutogroupSelfCache(oldNodes, newNodes views.S
|
|||||||
|
|
||||||
newIPs := newNode.IPs()
|
newIPs := newNode.IPs()
|
||||||
if len(oldIPs) != len(newIPs) {
|
if len(oldIPs) != len(newIPs) {
|
||||||
affectedUsers[newNode.User().ID()] = struct{}{}
|
affectedUsers[newNode.TypedUserID()] = struct{}{}
|
||||||
} else {
|
} else {
|
||||||
// Check if any IPs are different
|
// Check if any IPs are different
|
||||||
for i, oldIP := range oldIPs {
|
for i, oldIP := range oldIPs {
|
||||||
if i >= len(newIPs) || oldIP != newIPs[i] {
|
if i >= len(newIPs) || oldIP != newIPs[i] {
|
||||||
affectedUsers[newNode.User().ID()] = struct{}{}
|
affectedUsers[newNode.TypedUserID()] = struct{}{}
|
||||||
break
|
break
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -1370,7 +1375,7 @@ func (pm *PolicyManager) invalidateAutogroupSelfCache(oldNodes, newNodes views.S
|
|||||||
// because autogroup:self rules depend on the entire user's device set.
|
// because autogroup:self rules depend on the entire user's device set.
|
||||||
for nodeID := range pm.filterRulesMap {
|
for nodeID := range pm.filterRulesMap {
|
||||||
// Find the user for this cached node
|
// Find the user for this cached node
|
||||||
var nodeUserID uint
|
var nodeUserID types.UserID
|
||||||
|
|
||||||
found := false
|
found := false
|
||||||
|
|
||||||
@@ -1384,7 +1389,7 @@ func (pm *PolicyManager) invalidateAutogroupSelfCache(oldNodes, newNodes views.S
|
|||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
|
||||||
nodeUserID = node.User().ID()
|
nodeUserID = node.TypedUserID()
|
||||||
found = true
|
found = true
|
||||||
|
|
||||||
break
|
break
|
||||||
@@ -1400,7 +1405,7 @@ func (pm *PolicyManager) invalidateAutogroupSelfCache(oldNodes, newNodes views.S
|
|||||||
break
|
break
|
||||||
}
|
}
|
||||||
|
|
||||||
nodeUserID = node.User().ID()
|
nodeUserID = node.TypedUserID()
|
||||||
found = true
|
found = true
|
||||||
|
|
||||||
break
|
break
|
||||||
|
|||||||
@@ -226,6 +226,72 @@ func TestInvalidateAutogroupSelfCache(t *testing.T) {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestSetNodesAutogroupSelfUnhydratedUser reproduces the panic seen on
|
||||||
|
// /machine/map when an autogroup:self policy is active and a non-tagged
|
||||||
|
// node reaches the policy manager with its UserID set but the User
|
||||||
|
// association left unhydrated (User pointer nil). The NodeStore stores
|
||||||
|
// nodes by value with User as a *User; not every write path hydrates the
|
||||||
|
// association, so the autogroup:self cache invalidation must derive the
|
||||||
|
// owning user from UserID, not from the User view.
|
||||||
|
func TestSetNodesAutogroupSelfUnhydratedUser(t *testing.T) {
|
||||||
|
users := types.Users{
|
||||||
|
{Model: gorm.Model{ID: 1}, Name: "user1", Email: "user1@headscale.net"},
|
||||||
|
{Model: gorm.Model{ID: 2}, Name: "user2", Email: "user2@headscale.net"},
|
||||||
|
}
|
||||||
|
|
||||||
|
policy := `{
|
||||||
|
"acls": [
|
||||||
|
{
|
||||||
|
"action": "accept",
|
||||||
|
"src": ["autogroup:member"],
|
||||||
|
"dst": ["autogroup:self:*"]
|
||||||
|
}
|
||||||
|
]
|
||||||
|
}`
|
||||||
|
|
||||||
|
// unhydratedNode mirrors a NodeStore snapshot entry whose UserID is
|
||||||
|
// set (so it is unambiguously user-owned, not tagged) but whose User
|
||||||
|
// association was never loaded.
|
||||||
|
unhydratedNode := func(name, ipv4, ipv6 string, userID uint) *types.Node {
|
||||||
|
return &types.Node{
|
||||||
|
Hostname: name,
|
||||||
|
IPv4: ap(ipv4),
|
||||||
|
IPv6: ap(ipv6),
|
||||||
|
UserID: new(userID),
|
||||||
|
User: nil,
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
initialNodes := types.Nodes{
|
||||||
|
node("user1-node1", "100.64.0.1", "fd7a:115c:a1e0::1", users[0]),
|
||||||
|
node("user2-node1", "100.64.0.2", "fd7a:115c:a1e0::2", users[1]),
|
||||||
|
}
|
||||||
|
for i, n := range initialNodes {
|
||||||
|
n.ID = types.NodeID(i + 1) //nolint:gosec // safe conversion in test
|
||||||
|
}
|
||||||
|
|
||||||
|
pm, err := NewPolicyManager([]byte(policy), users, initialNodes.ViewSlice())
|
||||||
|
require.NoError(t, err)
|
||||||
|
|
||||||
|
require.False(t, initialNodes[0].IsTagged(), "node must be user-owned for autogroup:self")
|
||||||
|
|
||||||
|
// Simulate a node restarting tailscaled: the same node is pushed back
|
||||||
|
// into the policy manager, but the snapshot version has no hydrated
|
||||||
|
// User association. This is the exact shape that crashed beta.1.
|
||||||
|
updatedNodes := types.Nodes{
|
||||||
|
unhydratedNode("user1-node1", "100.64.0.1", "fd7a:115c:a1e0::1", users[0].ID),
|
||||||
|
node("user2-node1", "100.64.0.2", "fd7a:115c:a1e0::2", users[1]),
|
||||||
|
}
|
||||||
|
for i, n := range updatedNodes {
|
||||||
|
n.ID = types.NodeID(i + 1) //nolint:gosec // safe conversion in test
|
||||||
|
}
|
||||||
|
|
||||||
|
require.NotPanics(t, func() {
|
||||||
|
_, err = pm.SetNodes(updatedNodes.ViewSlice())
|
||||||
|
}, "SetNodes must not panic when a non-tagged node has an unhydrated User")
|
||||||
|
require.NoError(t, err)
|
||||||
|
}
|
||||||
|
|
||||||
// TestInvalidateGlobalPolicyCache tests the cache invalidation logic for global policies.
|
// TestInvalidateGlobalPolicyCache tests the cache invalidation logic for global policies.
|
||||||
func TestInvalidateGlobalPolicyCache(t *testing.T) {
|
func TestInvalidateGlobalPolicyCache(t *testing.T) {
|
||||||
mustIPPtr := func(s string) *netip.Addr {
|
mustIPPtr := func(s string) *netip.Addr {
|
||||||
|
|||||||
Reference in New Issue
Block a user