mirror of
https://github.com/juanfont/headscale.git
synced 2026-11-04 04:34:07 +09:00
policy/v2: limit the destination and port pairs a policy declares
A grant lists dst and ip apart, so its filter grows with their product. Refuse a policy over 100,000 pairs, on every parse.
This commit is contained in:
1 parent
cae4033813
commit
7fa696a690
2 files changed
+135
No files matched your search
@@ -74,6 +74,19 @@ var (
|
||||
ErrACLAutogroupSelfInvalidSource = errors.New("autogroup:self can only be used with users, groups, or supported autogroups")
|
||||
)
|
||||
|
||||
// ErrTooManyDstPorts is returned for a policy over [maxDstPorts].
|
||||
var ErrTooManyDstPorts = errors.New(
|
||||
"too many combinations of destination and port range",
|
||||
)
|
||||
|
||||
// maxDstPorts bounds the [tailcfg.NetPortRange] entries the ACLs and grants
|
||||
// of one policy compile to. A rule compiles to one entry per destination
|
||||
// prefix and port range, so a grant, which lists the two separately, grows
|
||||
// the filter with the product of its lists and not with the size of the
|
||||
// policy. The limit is about what 1 MiB of policy holds when every
|
||||
// combination is written out as an ACL destination.
|
||||
const maxDstPorts = 100_000
|
||||
|
||||
// Grant validation errors.
|
||||
var (
|
||||
ErrGrantMissingIPOrApp = errors.New("ip and app can not both be empty")
|
||||
@@ -2405,6 +2418,17 @@ func validateGrantSrcDstCombination(sources Aliases, destinations Aliases) error
|
||||
return nil
|
||||
}
|
||||
|
||||
// minDstPrefixes returns how many prefixes dst resolves to as far as the
|
||||
// policy alone tells. Users, groups and tags count as one, their devices are
|
||||
// not known before the policy is compiled.
|
||||
func minDstPrefixes(dst Alias) int64 {
|
||||
if ag, ok := dst.(*AutoGroup); ok && ag.Is(AutoGroupInternet) {
|
||||
return int64(len(util.TheInternet().Prefixes()))
|
||||
}
|
||||
|
||||
return 1
|
||||
}
|
||||
|
||||
// validate reports if there are any errors in a policy after
|
||||
// the unmarshaling process.
|
||||
// It runs through all rules and checks if there are any inconsistencies
|
||||
@@ -2420,6 +2444,10 @@ func (p *Policy) validate() error {
|
||||
// when adding more validation, please add to the list of errors.
|
||||
var errs []error
|
||||
|
||||
// Summed over the policy, many rules just below a limit of their own
|
||||
// would add up again. int64: an int can wrap on 32-bit builds.
|
||||
var dstPorts int64
|
||||
|
||||
for _, acl := range p.ACLs {
|
||||
for _, src := range acl.Sources {
|
||||
switch src := src.(type) {
|
||||
@@ -2460,6 +2488,8 @@ func (p *Policy) validate() error {
|
||||
}
|
||||
|
||||
for _, dst := range acl.Destinations {
|
||||
dstPorts += minDstPrefixes(dst.Alias) * int64(len(dst.Ports))
|
||||
|
||||
switch h := dst.Alias.(type) {
|
||||
case *Host:
|
||||
if !p.Hosts.exist(*h) {
|
||||
@@ -2698,8 +2728,12 @@ func (p *Policy) validate() error {
|
||||
}
|
||||
}
|
||||
|
||||
var dstPrefixes int64
|
||||
|
||||
// Validate destinations (empty arrays are allowed — they produce no rules)
|
||||
for _, dst := range grant.Destinations {
|
||||
dstPrefixes += minDstPrefixes(dst)
|
||||
|
||||
switch h := dst.(type) {
|
||||
case *Host:
|
||||
if !p.Hosts.exist(*h) {
|
||||
@@ -2730,6 +2764,10 @@ func (p *Policy) validate() error {
|
||||
}
|
||||
}
|
||||
|
||||
for _, ipp := range grant.InternetProtocols {
|
||||
dstPorts += dstPrefixes * int64(len(ipp.Ports))
|
||||
}
|
||||
|
||||
// Validate via tags. Wording matches Tailscale SaaS
|
||||
// ("tag %q not found"), which differs from the ACL-src
|
||||
// wording ("src=tag not found: %q").
|
||||
@@ -2751,6 +2789,12 @@ func (p *Policy) validate() error {
|
||||
}
|
||||
}
|
||||
|
||||
if dstPorts > maxDstPorts {
|
||||
errs = append(errs, fmt.Errorf(
|
||||
"%w: %d, limit %d", ErrTooManyDstPorts, dstPorts, maxDstPorts,
|
||||
))
|
||||
}
|
||||
|
||||
for _, na := range p.NodeAttrs {
|
||||
// SaaS accepts entries with neither attr nor ipPool (they
|
||||
// compile to a no-op); headscale follows suit so policies
|
||||
|
||||
@@ -6364,3 +6364,94 @@ func TestPolicyMetadataFields(t *testing.T) {
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
// TestUnmarshalPolicyMaxDstPorts checks that the size limit counts what the
|
||||
// filter holds an entry for, the combinations of destination prefix and port
|
||||
// range, and not the length of either list.
|
||||
func TestUnmarshalPolicyMaxDstPorts(t *testing.T) {
|
||||
list := func(item string, n int) string {
|
||||
return strings.TrimSuffix(strings.Repeat(item+",", n), ",")
|
||||
}
|
||||
grant := func(extra, dst string, dsts int, ip string) string {
|
||||
return fmt.Sprintf(
|
||||
`{"src": ["*"], %s "dst": [%s], "ip": [%s]}`,
|
||||
extra, list(`"`+dst+`"`, dsts), ip,
|
||||
)
|
||||
}
|
||||
// One range per ip entry; repeats still count.
|
||||
ports := func(n int) string {
|
||||
return list(`"443"`, n)
|
||||
}
|
||||
|
||||
tests := []struct {
|
||||
name string
|
||||
policy string
|
||||
wantErr bool
|
||||
}{
|
||||
{
|
||||
name: "grant with many ports",
|
||||
policy: `{"grants": [` + grant("", "*", 1, ports(50_000)) + `]}`,
|
||||
},
|
||||
{
|
||||
name: "grant with many destinations",
|
||||
policy: `{"grants": [` + grant("", "*", 50_000, ports(1)) + `]}`,
|
||||
},
|
||||
{
|
||||
name: "many destinations and many ports in different grants",
|
||||
policy: `{"grants": [` + grant("", "*", 50_000, ports(1)) + `,` +
|
||||
grant("", "*", 1, ports(50_000)) + `]}`,
|
||||
},
|
||||
{
|
||||
name: "grant at the limit",
|
||||
policy: `{"grants": [` + grant("", "*", 100, ports(1000)) + `]}`,
|
||||
},
|
||||
{
|
||||
name: "grant with many destinations and many ports",
|
||||
policy: `{"grants": [` + grant("", "*", 1000, ports(1000)) + `]}`,
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "via grant",
|
||||
policy: `{"tagOwners": {"tag:router": []}, "grants": [` +
|
||||
grant(`"via": ["tag:router"],`, "10.0.0.0/24", 1000, ports(1000)) +
|
||||
`]}`,
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "grants that add up",
|
||||
policy: `{"grants": [` + list(grant("", "*", 100, ports(100)), 11) + `]}`,
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "autogroup:internet counts as its prefixes",
|
||||
policy: `{"grants": [` +
|
||||
grant("", "autogroup:internet", 100, ports(100)) + `]}`,
|
||||
wantErr: true,
|
||||
},
|
||||
{
|
||||
name: "acl with many ports",
|
||||
policy: `{"acls": [{"action": "accept", "src": ["*"], "dst": ["*:` +
|
||||
list("443", 50_000) + `"]}]}`,
|
||||
},
|
||||
{
|
||||
name: "acl with many ports on autogroup:internet",
|
||||
policy: `{"acls": [{"action": "accept", "src": ["*"], ` +
|
||||
`"dst": ["autogroup:internet:` + list("443", 50_000) + `"]}]}`,
|
||||
wantErr: true,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
_, err := unmarshalPolicy([]byte(tt.policy))
|
||||
|
||||
if tt.wantErr {
|
||||
require.ErrorIs(t, err, ErrTooManyDstPorts)
|
||||
|
||||
return
|
||||
}
|
||||
|
||||
require.NoError(t, err)
|
||||
})
|
||||
}
|
||||
}
|
||||
Reference in new issue
Block a user