From 12a34f3895c4d66979d09c1ac44274b0b1564154 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 18 Mar 2026 22:17:38 +0000 Subject: [PATCH] policy/v2: implement grant validation rules matching Tailscale SaaS Add five categories of grant validation that Tailscale enforces: 1. Capability name format: reject URL schemes (://) and restrict tailscale.com domain to an allowlist of user-grantable caps. 2. Grant-specific autogroup:self: reject wildcard (*) sources with autogroup:self destinations (stricter than ACL rules since * includes tags which cannot use autogroup:self). 3. App + autogroup:internet: reject app grants targeting autogroup:internet. 4. Raw default route CIDRs: reject 0.0.0.0/0 and ::/0 as grant destinations, requiring "*" or "autogroup:internet" instead. 5. Via field: non-tag values (e.g. autogroup:tagged) are caught at unmarshal time by Tag.UnmarshalJSON validation. This resolves 23 ERROR_VALIDATION_GAP + 1 via validation test, reducing the grant compat skip list from 28 to 5 remaining tests. Updates #2180 --- .../policy/v2/tailscale_grants_compat_test.go | 82 +--------- hscontrol/policy/v2/types.go | 149 +++++++++++++++--- 2 files changed, 135 insertions(+), 96 deletions(-) diff --git a/hscontrol/policy/v2/tailscale_grants_compat_test.go b/hscontrol/policy/v2/tailscale_grants_compat_test.go index 9aff6713..6384d9be 100644 --- a/hscontrol/policy/v2/tailscale_grants_compat_test.go +++ b/hscontrol/policy/v2/tailscale_grants_compat_test.go @@ -214,11 +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 // -// Total: 28 tests skipped, ~209 tests expected to pass. +// Total: 5 tests skipped, ~232 tests expected to pass. var grantSkipReasons = map[string]string{ // ======================================================================== // USER_PASSKEY_WILDCARD (2 tests) @@ -237,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) // @@ -258,74 +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", - - // (VALIDATION_STRICTNESS tests H4/H5 removed — empty src/dst now accepted) - - // ======================================================================== - // 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 @@ -343,11 +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 // -// Total: 28 tests skipped, ~209 tests expected to pass. +// Total: 5 tests skipped, ~232 tests expected to pass. func TestGrantsCompat(t *testing.T) { t.Parallel() @@ -424,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 cc01d77c..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,6 +2467,44 @@ func (p *Policy) validate() error { errs = append(errs, ErrGrantMissingIPOrApp) } + // 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) { @@ -2465,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) }