policy/v2: fix empty grants/acls returning FilterAllowAll
compileFilterRules, compileGrants, and updateLocked guarded the
"no rules so allow all" fallback with len(pol.Grants) == 0, which
matches both an absent grants field and an explicit empty array.
JSON {"grants": []} unmarshals to a non-nil empty slice; it should
compile to zero filter rules (deny all) to match Tailscale SaaS,
but the length check sent it down the FilterAllowAll path.
Distinguish absent (nil) from explicit-empty by switching the guard
to pol.Grants == nil, the same asymmetry already used for ACLs.
{} keeps allowing all; {"acls": []} and {"grants": []} now both
deny all.
Fixes #3211
This commit is contained in:
parent
174e409da6
commit
2e1a716a9a
@ -101,7 +101,7 @@ func (pol *Policy) compileGrants(
|
|||||||
users types.Users,
|
users types.Users,
|
||||||
nodes views.Slice[types.NodeView],
|
nodes views.Slice[types.NodeView],
|
||||||
) []compiledGrant {
|
) []compiledGrant {
|
||||||
if pol == nil || (pol.ACLs == nil && len(pol.Grants) == 0) {
|
if pol == nil || (pol.ACLs == nil && pol.Grants == nil) {
|
||||||
return nil
|
return nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -138,7 +138,7 @@ func (pol *Policy) compileFilterRules(
|
|||||||
users types.Users,
|
users types.Users,
|
||||||
nodes views.Slice[types.NodeView],
|
nodes views.Slice[types.NodeView],
|
||||||
) ([]tailcfg.FilterRule, error) {
|
) ([]tailcfg.FilterRule, error) {
|
||||||
if pol == nil || (pol.ACLs == nil && len(pol.Grants) == 0) {
|
if pol == nil || (pol.ACLs == nil && pol.Grants == nil) {
|
||||||
return tailcfg.FilterAllowAll, nil
|
return tailcfg.FilterAllowAll, nil
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|||||||
@ -3537,11 +3537,26 @@ func TestFilterAllowAllFix(t *testing.T) {
|
|||||||
wantFilterAllow: true,
|
wantFilterAllow: true,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "nil ACLs and empty grants returns FilterAllowAll",
|
name: "nil ACLs and empty grants denies all",
|
||||||
pol: &Policy{
|
pol: &Policy{
|
||||||
Grants: []Grant{},
|
Grants: []Grant{},
|
||||||
},
|
},
|
||||||
wantFilterAllow: true,
|
wantFilterAllow: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "empty ACLs and nil grants denies all",
|
||||||
|
pol: &Policy{
|
||||||
|
ACLs: []ACL{},
|
||||||
|
},
|
||||||
|
wantFilterAllow: false,
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "empty ACLs and empty grants denies all",
|
||||||
|
pol: &Policy{
|
||||||
|
ACLs: []ACL{},
|
||||||
|
Grants: []Grant{},
|
||||||
|
},
|
||||||
|
wantFilterAllow: false,
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: "both ACLs and grants should not return FilterAllowAll",
|
name: "both ACLs and grants should not return FilterAllowAll",
|
||||||
|
|||||||
@ -109,7 +109,7 @@ func (pm *PolicyManager) updateLocked() (bool, error) {
|
|||||||
pm.needsPerNodeFilter = hasPerNodeGrants(pm.compiledGrants)
|
pm.needsPerNodeFilter = hasPerNodeGrants(pm.compiledGrants)
|
||||||
|
|
||||||
var filter []tailcfg.FilterRule
|
var filter []tailcfg.FilterRule
|
||||||
if pm.pol == nil || (pm.pol.ACLs == nil && len(pm.pol.Grants) == 0) {
|
if pm.pol == nil || (pm.pol.ACLs == nil && pm.pol.Grants == nil) {
|
||||||
filter = tailcfg.FilterAllowAll
|
filter = tailcfg.FilterAllowAll
|
||||||
} else {
|
} else {
|
||||||
filter = globalFilterRules(pm.compiledGrants)
|
filter = globalFilterRules(pm.compiledGrants)
|
||||||
|
|||||||
@ -44,6 +44,20 @@ func TestPolicyManager(t *testing.T) {
|
|||||||
wantFilter: tailcfg.FilterAllowAll,
|
wantFilter: tailcfg.FilterAllowAll,
|
||||||
wantMatchers: matcher.MatchesFromFilterRules(tailcfg.FilterAllowAll),
|
wantMatchers: matcher.MatchesFromFilterRules(tailcfg.FilterAllowAll),
|
||||||
},
|
},
|
||||||
|
{
|
||||||
|
name: "empty-acls-denies-all",
|
||||||
|
pol: `{"acls": []}`,
|
||||||
|
nodes: types.Nodes{},
|
||||||
|
wantFilter: nil,
|
||||||
|
wantMatchers: matcher.MatchesFromFilterRules(nil),
|
||||||
|
},
|
||||||
|
{
|
||||||
|
name: "empty-grants-denies-all",
|
||||||
|
pol: `{"grants": []}`,
|
||||||
|
nodes: types.Nodes{},
|
||||||
|
wantFilter: nil,
|
||||||
|
wantMatchers: matcher.MatchesFromFilterRules(nil),
|
||||||
|
},
|
||||||
}
|
}
|
||||||
|
|
||||||
for _, tt := range tests {
|
for _, tt := range tests {
|
||||||
|
|||||||
@ -5212,3 +5212,32 @@ func TestGrantMarshalJSON(t *testing.T) {
|
|||||||
})
|
})
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// TestUnmarshalPolicyEmptyArrays locks in the JSON unmarshal contract
|
||||||
|
// the FilterAllowAll guards depend on: an absent "acls" or "grants"
|
||||||
|
// field produces a nil slice, while an explicit empty array produces
|
||||||
|
// a non-nil empty slice. The compileFilterRules guard distinguishes
|
||||||
|
// these to differentiate "no policy → allow all" from "explicit
|
||||||
|
// empty rule set → deny all" (matching Tailscale SaaS).
|
||||||
|
func TestUnmarshalPolicyEmptyArrays(t *testing.T) {
|
||||||
|
cases := []struct {
|
||||||
|
name string
|
||||||
|
input string
|
||||||
|
aclsIsNil bool
|
||||||
|
grantsIsNil bool
|
||||||
|
}{
|
||||||
|
{name: "empty-object", input: "{}", aclsIsNil: true, grantsIsNil: true},
|
||||||
|
{name: "empty-acls", input: `{"acls": []}`, aclsIsNil: false, grantsIsNil: true},
|
||||||
|
{name: "empty-grants", input: `{"grants": []}`, aclsIsNil: true, grantsIsNil: false},
|
||||||
|
}
|
||||||
|
|
||||||
|
for _, tc := range cases {
|
||||||
|
t.Run(tc.name, func(t *testing.T) {
|
||||||
|
pol, err := unmarshalPolicy([]byte(tc.input))
|
||||||
|
require.NoError(t, err)
|
||||||
|
require.NotNil(t, pol)
|
||||||
|
require.Equal(t, tc.aclsIsNil, pol.ACLs == nil, "ACLs nil-ness")
|
||||||
|
require.Equal(t, tc.grantsIsNil, pol.Grants == nil, "Grants nil-ness")
|
||||||
|
})
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
Loading…
x
Reference in New Issue
Block a user