diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 93438991e..495bc1e8c 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -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 diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index 8ed6e09d5..44d02fca2 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -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) + }) + } +}