policy/v2: bound grant destination expansion

Require one port or range per grant IP entry. Reject more than 100,000
declared destination/port-range combinations on every parse.

hscontrol/policy/v2/tailscale_grants_compat_test.go

@@ -171,11 +171,8 @@ var grantSkipReasons = map[string]string{
 	"via-grant-v62": "FILTER_ADDRESS_FORM: same addresses, different wire form",
 
 	// Checks keyed <test_id>/<node>/filter: a listed check that matches
-	// fails, so a fix must delete its entry.
-	"grant-dxp-comma-ports/refused": grantPortList,
-
-	// Contiguous pinned addresses; TestViaGrantHACompat compares these
-	// filters by address.
+	// fails, so a fix must delete its entry. Contiguous pinned addresses;
+	// TestViaGrantHACompat compares these filters by address.
 	"via-grant-v64/client-a/filter":  srcAddressForm,
 	"via-grant-v64/client-b/filter":  srcAddressForm,
 	"via-grant-v64/plain/filter":     srcAddressForm,
@@ -190,8 +187,6 @@ var grantSkipReasons = map[string]string{
 	"via-grant-v65/via-r/filter":     srcAddressForm,
 }
 
-const grantPortList = "GRANT_PORT_LIST: SaaS refuses a comma port list in a grant ip entry (only one port range allowed); headscale accepts it"
-
 // TestGrantsCompat is a data-driven test that loads all GRANT-*.json
 // test files captured from Tailscale SaaS and compares headscale's grants
 // engine output against the real Tailscale behavior.

hscontrol/policy/v2/types.go

@@ -74,9 +74,23 @@ 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")
+	ErrGrantIPPortList                 = errors.New("only one port range allowed")
 	ErrGrantViaNotATag                 = errors.New("via can only be a tag")
 	ErrProtocolPortInvalidFormat       = errors.New("expected only one colon in Internet protocol and port type")
 	ErrCapNameInvalidForm              = errors.New("capability name must have the form {domain}/{path}")
@@ -965,38 +979,34 @@ func (ve *ProtocolPort) UnmarshalJSON(b []byte) error {
 			return nil
 		}
 
-		// Only contains a port, no protocol
-		if !strings.Contains(vs, ":") {
-			ports, err := parsePortRange(vs)
-			if err != nil {
-				return fmt.Errorf("port range %q: %w", vs, err)
+		protocol, portsPart := ProtocolNameWildcard, vs
+
+		// "proto:ports", or ports alone for any protocol.
+		if strings.Contains(vs, ":") {
+			parts := strings.Split(vs, ":")
+			if len(parts) != 2 {
+				return fmt.Errorf("%w, got: %v(%d)", ErrProtocolPortInvalidFormat, parts, len(parts))
 			}
 
-			ve.Protocol = ProtocolNameWildcard
-			ve.Ports = ports
+			protocol, portsPart = Protocol(parts[0]), parts[1]
 
-			return nil
+			err := protocol.validate()
+			if err != nil {
+				return err
+			}
 		}
 
-		parts := strings.Split(vs, ":")
-		if len(parts) != 2 {
-			return fmt.Errorf("%w, got: %v(%d)", ErrProtocolPortInvalidFormat, parts, len(parts))
-		}
-
-		protocol := Protocol(parts[0])
-
-		err := protocol.validate()
-		if err != nil {
-			return err
-		}
-
-		portsPart := parts[1]
-
 		ports, err := parsePortRange(portsPart)
 		if err != nil {
 			return fmt.Errorf("port range %q: %w", portsPart, err)
 		}
 
+		// Tailscale SaaS takes one range per entry; a list goes in
+		// entries of its own.
+		if len(ports) > 1 {
+			return fmt.Errorf("%w: %q", ErrGrantIPPortList, vs)
+		}
+
 		ve.Protocol = protocol
 		ve.Ports = ports
 
@@ -2408,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
@@ -2423,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) {
@@ -2463,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) {
@@ -2701,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) {
@@ -2733,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").
@@ -2754,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

hscontrol/policy/v2/types_test.go

@@ -5260,6 +5260,36 @@ func TestUnmarshalGrants(t *testing.T) {
 `,
 			wantErr: "ip and app can not both be empty",
 		},
+		{
+			name: "invalid-grant-ip-port-list",
+			input: `
+{
+	"grants": [
+		{
+			"src": ["*"],
+			"dst": ["*"],
+			"ip": ["tcp:80,443"]
+		}
+	]
+}
+`,
+			wantErr: `only one port range allowed: "tcp:80,443"`,
+		},
+		{
+			name: "invalid-grant-ip-bare-port-list",
+			input: `
+{
+	"grants": [
+		{
+			"src": ["*"],
+			"dst": ["*"],
+			"ip": ["80-90,443"]
+		}
+	]
+}
+`,
+			wantErr: `only one port range allowed: "80-90,443"`,
+		},
 		{
 			name: "valid-grant-empty-sources",
 			input: `
@@ -6334,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)
+		})
+	}
+}

hscontrol/servertest/policy_test.go

@@ -321,8 +321,6 @@ var rejectedPolicySkipReasons = map[string]string{
 	"acl-err11/refused": "ROUTE_HOST_BITS: headscale accepts an autoApprovers route with host bits",
 
 	"ssh-check-tag-src/refused": "SSH_CHECK_TAG_SRC: headscale accepts a check rule with a tag source",
-
-	"grant-dxp-comma-ports/refused": "GRANT_PORT_LIST: headscale accepts a comma port list in a grant ip entry",
 }
 
 const noTailnetDomains = "NO_TAILNET_DOMAINS: headscale does not check user:*@domain or localpart:*@domain against the tailnet's domains"