From 4f040dead2badfae47ccaf41a8d62072e19fc540 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 18 Mar 2026 21:15:58 +0000 Subject: [PATCH] policy/v2: implement grant validation rules matching Tailscale SaaS Implement comprehensive grant validation including: accept empty sources/destinations (they produce no rules), validate grant ip/app field requirements, capability name format, autogroup constraints, via tag existence, and default route CIDR restrictions. Updates #2180 --- hscontrol/policy/v2/filter.go | 8 + .../policy/v2/tailscale_grants_compat_test.go | 89 +--------- hscontrol/policy/v2/types.go | 157 +++++++++++++++--- hscontrol/policy/v2/types_test.go | 28 +++- 4 files changed, 167 insertions(+), 115 deletions(-) diff --git a/hscontrol/policy/v2/filter.go b/hscontrol/policy/v2/filter.go index 3f5dcc28..52cd7267 100644 --- a/hscontrol/policy/v2/filter.go +++ b/hscontrol/policy/v2/filter.go @@ -478,6 +478,14 @@ func (pol *Policy) compileGrantWithAutogroupSelf( } } + // When the grant has literally empty src=[] or dst=[], produce no rules + // at all — Tailscale returns null for these. This is distinct from sources + // that resolve to empty (e.g., group:empty) where Tailscale still produces + // CapGrant rules with empty SrcIPs. + if len(grant.Sources) == 0 || len(grant.Destinations) == 0 { + return rules, nil + } + if len(resolvedSrcs) == 0 && grant.App == nil { return rules, nil } diff --git a/hscontrol/policy/v2/tailscale_grants_compat_test.go b/hscontrol/policy/v2/tailscale_grants_compat_test.go index 024753ff..6384d9be 100644 --- a/hscontrol/policy/v2/tailscale_grants_compat_test.go +++ b/hscontrol/policy/v2/tailscale_grants_compat_test.go @@ -214,12 +214,10 @@ func loadGrantTestFile(t *testing.T, path string) grantTestFile { // // Impact summary (highest first): // -// ERROR_VALIDATION_GAP - 23 tests: Implement missing grant validation rules // AUTOGROUP_DANGER_ALL - 3 tests: Implement autogroup:danger-all support // USER_PASSKEY_WILDCARD - 2 tests: user:*@passkey wildcard pattern unresolvable -// VALIDATION_STRICTNESS - 2 tests: headscale too strict (rejects what Tailscale accepts) // -// Total: 30 tests skipped, ~207 tests expected to pass. +// Total: 5 tests skipped, ~232 tests expected to pass. var grantSkipReasons = map[string]string{ // ======================================================================== // USER_PASSKEY_WILDCARD (2 tests) @@ -238,11 +236,6 @@ var grantSkipReasons = map[string]string{ "GRANT-K20": "USER_PASSKEY_WILDCARD: src=user:*@passkey, dst=tag:server — source can't be resolved, no rules produced", "GRANT-K21": "USER_PASSKEY_WILDCARD: src=*, dst=user:*@passkey — destination can't be resolved, no rules produced", - // (VIA_COMPILATION tests removed — via route compilation now implemented) - - // (VIA_COMPILATION_AND_SRCIPS_FORMAT tests removed — via route compilation - // and SrcIPs format are both now implemented), - // ======================================================================== // AUTOGROUP_DANGER_ALL (3 tests) // @@ -259,79 +252,6 @@ var grantSkipReasons = map[string]string{ "GRANT-K6": "AUTOGROUP_DANGER_ALL", "GRANT-K7": "AUTOGROUP_DANGER_ALL", "GRANT-K8": "AUTOGROUP_DANGER_ALL", - - // ======================================================================== - // ERROR_VALIDATION_GAP (23 tests) - // - // TODO: Implement grant validation rules that Tailscale enforces but - // headscale does not yet. - // - // These are policies that Tailscale rejects (api_response_code=400) but - // headscale currently accepts without error. Each test documents the - // specific validation that needs to be added. - // ======================================================================== - - // Capability name format validation: - // Tailscale requires cap names to be {domain}/{path} without https:// prefix - // and rejects caps in the tailscale.com domain. - "GRANT-A2": "ERROR_VALIDATION_GAP: capability name must have the form {domain}/{path} — headscale should reject https:// prefix in cap names", - "GRANT-A5": "ERROR_VALIDATION_GAP: capability name must not be in the tailscale.com domain — headscale should reject tailscale.com/cap/relay-target", - "GRANT-K9": "ERROR_VALIDATION_GAP: capability name must not be in the tailscale.com domain — headscale should reject tailscale.com/cap/ingress", - "GRANT-K10": "ERROR_VALIDATION_GAP: capability name must not be in the tailscale.com domain — headscale should reject tailscale.com/cap/funnel", - - // autogroup:self validation: - // Tailscale only allows autogroup:self as dst when src is a user, group, - // or supported autogroup (like autogroup:member). It rejects autogroup:self - // when src is "*" (which includes tags) or when src is a tag. - "GRANT-E3": "ERROR_VALIDATION_GAP: autogroup:self can only be used with users, groups, or supported autogroups — src=[*] includes tags", - "GRANT-H9": "ERROR_VALIDATION_GAP: autogroup:self can only be used with users, groups, or supported autogroups — src=[*] includes tags", - "GRANT-P04_3": "ERROR_VALIDATION_GAP: autogroup:self can only be used with users, groups, or supported autogroups — src=[*] with ip grant", - "GRANT-P09_13A": "ERROR_VALIDATION_GAP: autogroup:self can only be used with users, groups, or supported autogroups — src=[*] with ip:[*]", - "GRANT-P09_13B": "ERROR_VALIDATION_GAP: autogroup:self can only be used with users, groups, or supported autogroups — src=[*] with ip:[22]", - "GRANT-P09_13C": "ERROR_VALIDATION_GAP: autogroup:self can only be used with users, groups, or supported autogroups — src=[*] with ip:[22,80,443]", - "GRANT-P09_13D": "ERROR_VALIDATION_GAP: autogroup:self can only be used with users, groups, or supported autogroups — src=[*] with ip:[80-443]", - "GRANT-P09_13H_CORRECT": "ERROR_VALIDATION_GAP: autogroup:self can only be used with users, groups, or supported autogroups — multi-grant with self", - "GRANT-P09_13H_NAIVE": "ERROR_VALIDATION_GAP: autogroup:self can only be used with users, groups, or supported autogroups — naive multi-dst with self", - - // Via route validation: - // Tailscale requires "via" to be a tag, rejects other values. - "GRANT-I4": "ERROR_VALIDATION_GAP: via can only be a tag — headscale should reject non-tag via values", - - // autogroup:internet + app grants validation: - // Tailscale rejects app grants when dst includes autogroup:internet. - "GRANT-V01": "ERROR_VALIDATION_GAP: cannot use app grants with autogroup:internet — headscale does not reject", - "GRANT-V22": "ERROR_VALIDATION_GAP: cannot use app grants with autogroup:internet — headscale returns different error (rejects mixed ip+app instead)", - - // Raw default route CIDR validation: - // Tailscale rejects 0.0.0.0/0 and ::/0 as grant dst, requiring "*" or - // "autogroup:internet" instead. This applies with or without via. - "GRANT-V04": "ERROR_VALIDATION_GAP: dst 0.0.0.0/0 rejected — headscale should reject raw default route CIDRs in grant dst", - "GRANT-V05": "ERROR_VALIDATION_GAP: dst ::/0 rejected — headscale should reject raw default route CIDRs in grant dst", - "GRANT-V08": "ERROR_VALIDATION_GAP: dst 0.0.0.0/0 with ip grant — same rejection as V04", - "GRANT-V14": "ERROR_VALIDATION_GAP: dst 0.0.0.0/0 with via — rejected even with via field", - "GRANT-V15": "ERROR_VALIDATION_GAP: dst ::/0 with via — rejected even with via field", - "GRANT-V16": "ERROR_VALIDATION_GAP: dst [0.0.0.0/0, ::/0] with via — both rejected", - "GRANT-V18": "ERROR_VALIDATION_GAP: dst 0.0.0.0/0 with via + app — rejected regardless of via or app", - - // Empty src/dst validation difference: - // Tailscale ACCEPTS empty src/dst arrays (producing no filter rules), - // but headscale rejects them with "grant sources/destinations cannot be empty". - // headscale is stricter here — should match Tailscale and accept empty arrays. - "GRANT-H4": "VALIDATION_STRICTNESS: headscale rejects empty src=[] but Tailscale accepts it (producing no rules)", - "GRANT-H5": "VALIDATION_STRICTNESS: headscale rejects empty dst=[] but Tailscale accepts it (producing no rules)", - - // ======================================================================== - // NIL_VS_EMPTY_RULES (varies) - // - // TODO: headscale returns empty []FilterRule{} where Tailscale returns null. - // - // Some success tests have null packet_filter_rules for online nodes, - // meaning Tailscale determined no rules apply. headscale may still produce - // empty-but-non-nil results due to how filter compilation works. - // These are handled by cmpopts.EquateEmpty() in the comparison, so they - // should no longer fail. If they still fail, the specific test needs - // investigation. - // ======================================================================== } // TestGrantsCompat is a data-driven test that loads all 237 GRANT-*.json @@ -349,12 +269,10 @@ var grantSkipReasons = map[string]string{ // // Skip category impact summary (highest first): // -// ERROR_VALIDATION_GAP - 23 tests: Implement missing grant validation rules // AUTOGROUP_DANGER_ALL - 3 tests: Implement autogroup:danger-all support // USER_PASSKEY_WILDCARD - 2 tests: user:*@passkey wildcard pattern unresolvable -// VALIDATION_STRICTNESS - 2 tests: headscale too strict (rejects what Tailscale accepts) // -// Total: 30 tests skipped, ~207 tests expected to pass. +// Total: 5 tests skipped, ~232 tests expected to pass. func TestGrantsCompat(t *testing.T) { t.Parallel() @@ -431,6 +349,9 @@ var grantErrorMessageMap = map[string]string{ // Tailscale says "ip and app can not both be empty", // headscale says "grants must specify either 'ip' or 'app' field" "ip and app can not both be empty": "grants must specify either", + // Tailscale says "via can only be a tag", + // headscale rejects at unmarshal time via Tag.UnmarshalJSON: "tag must start with 'tag:'" + "via can only be a tag": "tag must start with", } // assertGrantErrorContains checks that an error message contains the expected diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index fdfbf3e1..775dd905 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -67,12 +67,18 @@ var ( // Grant validation errors. var ( - ErrGrantMissingIPOrApp = errors.New("grants must specify either 'ip' or 'app' field") - ErrGrantInvalidViaTag = errors.New("grant 'via' tag is not defined in policy") - ErrGrantViaNotSupported = errors.New("grant 'via' routing is not yet supported in headscale") - ErrGrantEmptySources = errors.New("grant sources cannot be empty") - ErrGrantEmptyDestinations = errors.New("grant destinations cannot be empty") - ErrProtocolPortInvalidFormat = errors.New("expected only one colon in Internet protocol and port type") + ErrGrantMissingIPOrApp = errors.New("grants must specify either 'ip' or 'app' field") + ErrGrantInvalidViaTag = errors.New("grant 'via' tag is not defined in policy") + ErrGrantViaNotSupported = errors.New("grant 'via' routing is not yet supported in headscale") + ErrGrantEmptySources = errors.New("grant sources cannot be empty") + ErrGrantEmptyDestinations = errors.New("grant destinations cannot be empty") + ErrProtocolPortInvalidFormat = errors.New("expected only one colon in Internet protocol and port type") + ErrCapNameInvalidForm = errors.New("capability name must have the form {domain}/{path}") + ErrCapNameTailscaleDomain = errors.New("capability name must not be in the tailscale.com domain") + ErrGrantAutogroupSelfInvalidSource = errors.New("autogroup:self can only be used with users, groups, or supported autogroups") + ErrGrantViaOnlyTag = errors.New("via can only be a tag") + ErrGrantAppWithAutogroupInternet = errors.New("cannot use app grants with autogroup:internet") + ErrGrantDefaultRouteCIDR = errors.New("to allow all IP addresses, use \"*\" or \"autogroup:internet\"") ) // Policy validation errors. @@ -2180,6 +2186,81 @@ func validateACLSrcDstCombination(sources Aliases, destinations []AliasWithPorts return nil } +// validateCapabilityName validates that a capability name has the form +// {domain}/{path} (no URL scheme) and is not in the tailscale.com domain +// (unless it's on the allowlist of user-grantable capabilities). +// Tailscale SaaS enforces these rules to prevent confusion with built-in +// capabilities and URL-formatted names. +func validateCapabilityName(name string) error { + // Reject URL schemes (e.g., "https://tailscale.com/cap/ingress") + if strings.Contains(name, "://") { + return ErrCapNameInvalidForm + } + + // Reject caps in the tailscale.com domain unless allowlisted. + if strings.HasPrefix(name, "tailscale.com/") { + if !tailscaleCapAllowlist[tailcfg.PeerCapability(name)] { + return ErrCapNameTailscaleDomain + } + } + + return nil +} + +// tailscaleCapAllowlist contains the tailscale.com/cap/* capability names +// that users are allowed to specify in grant app fields. Companion caps +// (drive-sharer, relay-target) and internal caps (ingress, funnel) are +// generated by the server and cannot be specified by users. +var tailscaleCapAllowlist = map[tailcfg.PeerCapability]bool{ + tailcfg.PeerCapabilityTaildrive: true, // tailscale.com/cap/drive + tailcfg.PeerCapabilityRelay: true, // tailscale.com/cap/relay + tailcfg.PeerCapabilityWebUI: true, // tailscale.com/cap/webui + tailcfg.PeerCapabilityKubernetes: true, // tailscale.com/cap/kubernetes + tailcfg.PeerCapabilityTsIDP: true, // tailscale.com/cap/tsidp +} + +// validateGrantSrcDstCombination validates grant-specific source/destination +// combinations. Grants are stricter than ACLs: wildcard (*) sources are NOT +// allowed with autogroup:self destinations because * includes tags, and tags +// cannot use autogroup:self. ACLs allow this combination because ACL +// autogroup:self evaluation narrows it per-node, but grants reject it at +// validation time. +func validateGrantSrcDstCombination(sources Aliases, destinations Aliases) error { + hasAutogroupSelf := false + + for _, dst := range destinations { + if ag, ok := dst.(*AutoGroup); ok && ag.Is(AutoGroupSelf) { + hasAutogroupSelf = true + + break + } + } + + if !hasAutogroupSelf { + return nil + } + + for _, src := range sources { + switch v := src.(type) { + case *Username, *Group: + continue + case *AutoGroup: + if v.Is(AutoGroupMember) { + continue + } + + return ErrGrantAutogroupSelfInvalidSource + case Asterix: + // Grants reject wildcard with autogroup:self (unlike ACLs) + return ErrGrantAutogroupSelfInvalidSource + default: + return ErrGrantAutogroupSelfInvalidSource + } + } + + return nil +} + // 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 @@ -2386,11 +2467,45 @@ func (p *Policy) validate() error { errs = append(errs, ErrGrantMissingIPOrApp) } - // Validate sources - if len(grant.Sources) == 0 { - errs = append(errs, ErrGrantEmptySources) + // Validate capability name format in app grants. + // Tailscale requires cap names to be {domain}/{path} (no URL scheme) + // and rejects caps in the tailscale.com domain. + for capName := range grant.App { + err := validateCapabilityName(string(capName)) + if err != nil { + errs = append(errs, err) + } } + // Validate that app grants are not used with autogroup:internet. + if hasApp { + for _, dst := range grant.Destinations { + if ag, ok := dst.(*AutoGroup); ok && ag.Is(AutoGroupInternet) { + errs = append(errs, ErrGrantAppWithAutogroupInternet) + + break + } + } + } + + // Validate destinations do not contain raw default route CIDRs. + // Tailscale rejects 0.0.0.0/0 and ::/0 as grant dst, requiring + // "*" or "autogroup:internet" instead. + for _, dst := range grant.Destinations { + if p, ok := dst.(*Prefix); ok { + prefix := netip.Prefix(*p) + if prefix.Bits() == 0 { + errs = append(errs, fmt.Errorf( + "dst %q: %w", + prefix.String(), ErrGrantDefaultRouteCIDR, + )) + + break + } + } + } + + // Validate sources (empty arrays are allowed — they produce no rules) for _, src := range grant.Sources { switch src := src.(type) { case *Host: @@ -2429,11 +2544,7 @@ func (p *Policy) validate() error { } } - // Validate destinations - if len(grant.Destinations) == 0 { - errs = append(errs, ErrGrantEmptyDestinations) - } - + // Validate destinations (empty arrays are allowed — they produce no rules) for _, dst := range grant.Destinations { switch h := dst.(type) { case *Host: @@ -2473,19 +2584,11 @@ func (p *Policy) validate() error { } } - // Validate ACL source/destination combinations follow Tailscale's security model - // (Grants use same rules as ACLs for autogroup:self and other constraints) - // Convert grant destinations to AliasWithPorts format for validation - var dstWithPorts []AliasWithPorts - for _, dst := range grant.Destinations { - // For grants, we don't have per-destination ports, so use wildcard - dstWithPorts = append(dstWithPorts, AliasWithPorts{ - Alias: dst, - Ports: []tailcfg.PortRange{tailcfg.PortRangeAny}, - }) - } - - err := validateACLSrcDstCombination(grant.Sources, dstWithPorts) + // Validate grant-specific source/destination combinations. + // Grants are stricter than ACLs: wildcard (*) src with autogroup:self + // dst is rejected because * includes tags, and tags cannot use + // autogroup:self. + err := validateGrantSrcDstCombination(grant.Sources, grant.Destinations) if err != nil { errs = append(errs, err) } diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index bb5e597a..1e31c7f2 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -4632,7 +4632,7 @@ func TestUnmarshalGrants(t *testing.T) { wantErr: "grants must specify either 'ip' or 'app' field", }, { - name: "invalid-grant-empty-sources", + name: "valid-grant-empty-sources", input: ` { "grants": [ @@ -4644,10 +4644,20 @@ func TestUnmarshalGrants(t *testing.T) { ] } `, - wantErr: "grant sources cannot be empty", + want: &Policy{ + Grants: []Grant{ + { + Sources: Aliases{}, + Destinations: Aliases{Wildcard}, + InternetProtocols: []ProtocolPort{ + {Protocol: "*", Ports: []tailcfg.PortRange{tailcfg.PortRangeAny}}, + }, + }, + }, + }, }, { - name: "invalid-grant-empty-destinations", + name: "valid-grant-empty-destinations", input: ` { "grants": [ @@ -4659,7 +4669,17 @@ func TestUnmarshalGrants(t *testing.T) { ] } `, - wantErr: "grant destinations cannot be empty", + want: &Policy{ + Grants: []Grant{ + { + Sources: Aliases{Wildcard}, + Destinations: Aliases{}, + InternetProtocols: []ProtocolPort{ + {Protocol: "*", Ports: []tailcfg.PortRange{tailcfg.PortRangeAny}}, + }, + }, + }, + }, }, { name: "invalid-grant-undefined-via-tag",