From cae4033813428f95fe33d6b6e09b96e0ca48e186 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 21 Oct 2026 18:34:31 +0000 Subject: [PATCH] policy/v2: refuse a port list in a grant ip entry Tailscale SaaS takes one port range per ip entry; a list was accepted and expanded. Write each range as an entry of its own. --- .../policy/v2/tailscale_grants_compat_test.go | 9 +--- hscontrol/policy/v2/types.go | 41 +++++++++---------- hscontrol/policy/v2/types_test.go | 30 ++++++++++++++ hscontrol/servertest/policy_test.go | 2 - 4 files changed, 51 insertions(+), 31 deletions(-) diff --git a/hscontrol/policy/v2/tailscale_grants_compat_test.go b/hscontrol/policy/v2/tailscale_grants_compat_test.go index a8799ef81..760a43585 100644 --- a/hscontrol/policy/v2/tailscale_grants_compat_test.go +++ b/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 //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. diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 9e40e4b47..93438991e 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -77,6 +77,7 @@ var ( // 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 +966,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 diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index f0f19a911..8ed6e09d5 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/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: ` diff --git a/hscontrol/servertest/policy_test.go b/hscontrol/servertest/policy_test.go index 766d6a3f8..7279aee72 100644 --- a/hscontrol/servertest/policy_test.go +++ b/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"