mirror of
https://github.com/juanfont/headscale.git
synced 2026-11-04 12:39:25 +09:00
policy/v2: refresh clients when a user change drops the last via grant
SetUsers read needsPerNodeFilter after the recompile cleared it, so the change was reported as empty and clients kept the via rules.
This commit is contained in:
1 parent
200c7b799f
commit
4dcb59d453
3 files changed
+45
-10
No files matched your search
@@ -981,6 +981,10 @@ func (pm *PolicyManager) SetUsers(users []types.User) (bool, bool, error) {
|
||||
prev := pm.users
|
||||
pm.users = users
|
||||
|
||||
// A via grant left without sources is not compiled, so the recompile
|
||||
// can clear the flag while clients still hold its rules.
|
||||
hadPerNodeFilter := pm.needsPerNodeFilter
|
||||
|
||||
// SSH policies and autogroup:self sources resolve users by name, and
|
||||
// the self sources are outside the filter hash, so updateLocked can
|
||||
// report no change while per-node results moved.
|
||||
@@ -1000,7 +1004,8 @@ func (pm *PolicyManager) SetUsers(users []types.User) (bool, bool, error) {
|
||||
|
||||
// SSH rules and per-node filters embed user identity outside the filter
|
||||
// hash, so a user change needs a client refresh even when it did not move.
|
||||
if pm.needsPerNodeFilter || (pm.pol != nil && len(pm.pol.SSHs) > 0) {
|
||||
if hadPerNodeFilter || pm.needsPerNodeFilter ||
|
||||
(pm.pol != nil && len(pm.pol.SSHs) > 0) {
|
||||
policyChanged = true
|
||||
}
|
||||
|
||||
|
||||
@@ -556,6 +556,45 @@ func TestSetUsersDropsStaleSelfFilters(t *testing.T) {
|
||||
}
|
||||
}
|
||||
|
||||
// TestSetUsersReportsLostViaSources: a user change that leaves the only via
|
||||
// grant without sources must be reported, or clients keep its rules.
|
||||
func TestSetUsersReportsLostViaSources(t *testing.T) {
|
||||
pol := `{
|
||||
"tagOwners": {"tag:router": ["u1@"]},
|
||||
"grants": [{"src": ["u3@"], "dst": ["10.0.0.0/16"], "via": ["tag:router"], "ip": ["*"]}]}`
|
||||
|
||||
u1, u3, x3 := types.User{Name: "u1"}, types.User{Name: "u3"}, types.User{Name: "x3"}
|
||||
u1.ID, u3.ID, x3.ID = 1, 3, 3
|
||||
|
||||
router := node("router", "100.64.0.1", "fd7a:115c:a1e0::1", types.User{})
|
||||
router.User, router.UserID = nil, nil
|
||||
router.Tags = []string{"tag:router"}
|
||||
router.Hostinfo = &tailcfg.Hostinfo{
|
||||
RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.0.0.0/16")},
|
||||
}
|
||||
router.ApprovedRoutes = router.Hostinfo.RoutableIPs
|
||||
|
||||
nodes := types.Nodes{router, node("u3-a", "100.64.0.3", "fd7a:115c:a1e0::3", u3)}
|
||||
for i, n := range nodes {
|
||||
n.ID = types.NodeID(i + 1) //nolint:gosec // safe conversion in test
|
||||
}
|
||||
|
||||
pm, err := NewPolicyManager([]byte(pol), types.Users{u1, u3}, nodes.ViewSlice())
|
||||
require.NoError(t, err)
|
||||
|
||||
rules, err := pm.FilterForNode(router.View())
|
||||
require.NoError(t, err)
|
||||
require.NotEmpty(t, rules, "the router admits u3 before the rename")
|
||||
|
||||
policyChanged, _, err := pm.SetUsers(types.Users{u1, x3})
|
||||
require.NoError(t, err)
|
||||
|
||||
rules, err = pm.FilterForNode(router.View())
|
||||
require.NoError(t, err)
|
||||
require.Empty(t, rules, "nobody is named u3 any more")
|
||||
require.True(t, policyChanged, "the withdrawn rule must reach clients")
|
||||
}
|
||||
|
||||
// TestInvalidateGlobalPolicyCache tests the cache invalidation logic for global policies.
|
||||
func TestInvalidateGlobalPolicyCache(t *testing.T) {
|
||||
mustIPPtr := func(s string) *netip.Addr {
|
||||
|
||||
@@ -107,13 +107,6 @@ var routesMapCompatTests = []struct {
|
||||
var viaCompatSkipReasons = map[string]string{
|
||||
"tagged-self": "TAGGED_SELF_PROFILE: SaaS gives a tagged node a profile of its own, headscale the shared tagged-devices one",
|
||||
|
||||
// TestViaGrantSourceLossCompat, keyed <test_id>/rename/<node>/<check>.
|
||||
"via-grant-v67/rename/squirtle/peers": viaSourceLossNotSent,
|
||||
"via-grant-v67/rename/squirtle/packet-filter": viaSourceLossNotSent,
|
||||
"via-grant-v67/rename/blastoise/peers": viaSourceLossNotSent,
|
||||
"via-grant-v67/rename/blastoise/packet-filter": viaSourceLossNotSent,
|
||||
"via-grant-v67/rename/ivysaur/peers": viaSourceLossNotSent,
|
||||
|
||||
"routes-f2-subnet-as-acl-source/ivysaur/packet-filter": filterDstMerged,
|
||||
"routes-f2-subnet-as-acl-source/venusaur/packet-filter": filterDstMerged,
|
||||
|
||||
@@ -263,8 +256,6 @@ var viaCompatSkipReasons = map[string]string{
|
||||
"routes-m6-subnet-to-subnet-multi-route/m-odin2/peers": sameUserPeers,
|
||||
}
|
||||
|
||||
const viaSourceLossNotSent = "VIA_SOURCE_LOSS_NOT_SENT: a user change that leaves a via grant without sources does not reach open streams"
|
||||
|
||||
const exitRoutesOnOverlap = "EXIT_ROUTES_ON_OVERLAP: headscale offers a peer's exit routes to a viewer whose rules reach only nodes or subnets; SaaS leaves them out"
|
||||
|
||||
// Adjacent member addresses merge into one prefix, and the reduced filter
|
||||
|
||||
Reference in new issue
Block a user