From f95b254ea9063e7b9ba653ed32e804219409b509 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 18 Mar 2026 14:40:12 +0000 Subject: [PATCH] policy/v2: exclude exit routes from ReduceFilterRules Add exit route check in ReduceFilterRules to prevent exit nodes from receiving packet filter rules for destinations that only overlap via exit routes. Remove resolved SUBNET_ROUTE_FILTER_RULES grant skip entries and update error message formatting for grant validation. Updates #2180 --- hscontrol/policy/policyutil/reduce.go | 12 +++++- hscontrol/policy/policyutil/reduce_test.go | 34 ++-------------- .../policy/v2/tailscale_grants_compat_test.go | 39 +------------------ hscontrol/policy/v2/types.go | 23 ++++++----- hscontrol/policy/v2/types_test.go | 20 +++++----- hscontrol/policy/v2/utils.go | 2 +- hscontrol/policy/v2/utils_test.go | 8 ++-- 7 files changed, 45 insertions(+), 93 deletions(-) diff --git a/hscontrol/policy/policyutil/reduce.go b/hscontrol/policy/policyutil/reduce.go index 6d95a297..f3f1903c 100644 --- a/hscontrol/policy/policyutil/reduce.go +++ b/hscontrol/policy/policyutil/reduce.go @@ -3,6 +3,7 @@ package policyutil import ( "github.com/juanfont/headscale/hscontrol/types" "github.com/juanfont/headscale/hscontrol/util" + "tailscale.com/net/tsaddr" "tailscale.com/tailcfg" ) @@ -33,12 +34,19 @@ func ReduceFilterRules(node types.NodeView, rules []tailcfg.FilterRule) []tailcf continue DEST_LOOP } - // If the node exposes routes, ensure they are note removed - // when the filters are reduced. + // If the node exposes routes, ensure they are not removed + // when the filters are reduced. Exit routes (0.0.0.0/0, ::/0) + // are skipped here because exit nodes handle traffic via + // AllowedIPs/routing, not packet filter rules. This matches + // Tailscale SaaS behavior where exit nodes do not receive + // filter rules for destinations that only overlap via exit routes. if node.Hostinfo().Valid() { routableIPs := node.Hostinfo().RoutableIPs() if routableIPs.Len() > 0 { for _, routableIP := range routableIPs.All() { + if tsaddr.IsExitRoute(routableIP) { + continue + } if expanded.OverlapsPrefix(routableIP) { dests = append(dests, dest) continue DEST_LOOP diff --git a/hscontrol/policy/policyutil/reduce_test.go b/hscontrol/policy/policyutil/reduce_test.go index db0262fc..893f5b2f 100644 --- a/hscontrol/policy/policyutil/reduce_test.go +++ b/hscontrol/policy/policyutil/reduce_test.go @@ -453,7 +453,10 @@ func TestReduceFilterRules(t *testing.T) { }, }, want: []tailcfg.FilterRule{ - // Merged: Both ACL rules combined (same SrcIPs) + // Exit routes (0.0.0.0/0, ::/0) are skipped when checking RoutableIPs + // overlap, matching Tailscale SaaS behavior. Only destinations that + // contain the node's own Tailscale IP (via InIPSet) are kept. + // Here, 64.0.0.0/2 contains 100.64.0.100 (CGNAT range), so it matches. { SrcIPs: []string{ "100.64.0.1-100.64.0.2", @@ -464,36 +467,7 @@ func TestReduceFilterRules(t *testing.T) { IP: "100.64.0.100", Ports: tailcfg.PortRangeAny, }, - {IP: "0.0.0.0/5", Ports: tailcfg.PortRangeAny}, - {IP: "8.0.0.0/7", Ports: tailcfg.PortRangeAny}, - {IP: "11.0.0.0/8", Ports: tailcfg.PortRangeAny}, - {IP: "12.0.0.0/6", Ports: tailcfg.PortRangeAny}, - {IP: "16.0.0.0/4", Ports: tailcfg.PortRangeAny}, - {IP: "32.0.0.0/3", Ports: tailcfg.PortRangeAny}, {IP: "64.0.0.0/2", Ports: tailcfg.PortRangeAny}, - {IP: "128.0.0.0/3", Ports: tailcfg.PortRangeAny}, - {IP: "160.0.0.0/5", Ports: tailcfg.PortRangeAny}, - {IP: "168.0.0.0/6", Ports: tailcfg.PortRangeAny}, - {IP: "172.0.0.0/12", Ports: tailcfg.PortRangeAny}, - {IP: "172.32.0.0/11", Ports: tailcfg.PortRangeAny}, - {IP: "172.64.0.0/10", Ports: tailcfg.PortRangeAny}, - {IP: "172.128.0.0/9", Ports: tailcfg.PortRangeAny}, - {IP: "173.0.0.0/8", Ports: tailcfg.PortRangeAny}, - {IP: "174.0.0.0/7", Ports: tailcfg.PortRangeAny}, - {IP: "176.0.0.0/4", Ports: tailcfg.PortRangeAny}, - {IP: "192.0.0.0/9", Ports: tailcfg.PortRangeAny}, - {IP: "192.128.0.0/11", Ports: tailcfg.PortRangeAny}, - {IP: "192.160.0.0/13", Ports: tailcfg.PortRangeAny}, - {IP: "192.169.0.0/16", Ports: tailcfg.PortRangeAny}, - {IP: "192.170.0.0/15", Ports: tailcfg.PortRangeAny}, - {IP: "192.172.0.0/14", Ports: tailcfg.PortRangeAny}, - {IP: "192.176.0.0/12", Ports: tailcfg.PortRangeAny}, - {IP: "192.192.0.0/10", Ports: tailcfg.PortRangeAny}, - {IP: "193.0.0.0/8", Ports: tailcfg.PortRangeAny}, - {IP: "194.0.0.0/7", Ports: tailcfg.PortRangeAny}, - {IP: "196.0.0.0/6", Ports: tailcfg.PortRangeAny}, - {IP: "200.0.0.0/5", Ports: tailcfg.PortRangeAny}, - {IP: "208.0.0.0/4", Ports: tailcfg.PortRangeAny}, }, }, }, diff --git a/hscontrol/policy/v2/tailscale_grants_compat_test.go b/hscontrol/policy/v2/tailscale_grants_compat_test.go index a6c5eb88..cc7de8c4 100644 --- a/hscontrol/policy/v2/tailscale_grants_compat_test.go +++ b/hscontrol/policy/v2/tailscale_grants_compat_test.go @@ -218,7 +218,6 @@ func loadGrantTestFile(t *testing.T, path string) grantTestFile { // ERROR_VALIDATION_GAP - 23 tests: Implement missing grant validation rules // MISSING_IPV6_ADDRS - 90 tests: Include IPv6 for identity-based alias resolution // CAPGRANT_COMPILATION_AND_SRCIPS - 11 tests: Both CapGrant compilation + SrcIPs format -// SUBNET_ROUTE_FILTER_RULES - 10 tests: Generate filter rules for subnet-routed CIDRs // VIA_COMPILATION_AND_SRCIPS_FORMAT - 7 tests: Via route compilation + SrcIPs format // AUTOGROUP_SELF_CIDR_FORMAT - 4 tests: DstPorts IPs get /32 or /128 suffix for autogroup:self // VIA_COMPILATION - 3 tests: Via route compilation @@ -228,41 +227,8 @@ func loadGrantTestFile(t *testing.T, path string) grantTestFile { // RAW_IPV6_ADDR_EXPANSION - 2 tests: Raw fd7a: IPv6 src/dst expanded to include IPv4 // SRCIPS_WILDCARD_NODE_DEDUP - 1 test: Wildcard+specific source node IP deduplication // -// Total: 207 tests skipped, 30 tests expected to pass. +// Total: 197 tests skipped, 40 tests expected to pass. var grantSkipReasons = map[string]string{ - // ======================================================================== - // SUBNET_ROUTE_FILTER_RULES (11 tests) - // - // TODO: Generate filter rules for non-Tailscale CIDR destinations on - // subnet-router nodes. - // - // When a grant targets a non-Tailscale CIDR (e.g., 10.0.0.0/8, - // 10.33.0.0/16, 10.33.1.0/24), Tailscale generates FilterRules on the - // subnet-router node that advertises overlapping routes. headscale - // produces no rules for these destinations, resulting in empty output - // on the subnet-router node. - // - // Example (GRANT-P13_1, dst=10.33.0.0/16): - // tailscale produces on subnet-router: - // SrcIPs=["100.103.90.82","100.110.121.96","100.90.199.68", + IPv6s] - // DstPorts=[{IP:"10.33.0.0/16", Ports:"22"}] - // headscale produces: [] (empty) - // - // Fix: During filter rule compilation, check if a destination CIDR - // overlaps with any subnet route advertised by the current node, and - // if so, generate the appropriate FilterRule. - // ======================================================================== - "GRANT-P08_8": "SUBNET_ROUTE_FILTER_RULES: dst=10.0.0.0/8 — subnet-router gets no rules", - "GRANT-P09_6D": "SUBNET_ROUTE_FILTER_RULES: dst=internal (host alias for 10.0.0.0/8) — subnet-router gets no rules", - "GRANT-P10_3": "SUBNET_ROUTE_FILTER_RULES: dst=host alias for 10.33.0.0/16 — subnet-router gets no rules", - "GRANT-P10_4": "SUBNET_ROUTE_FILTER_RULES: dst=host alias for 10.33.0.0/16 — subnet-router gets no rules", - "GRANT-P13_1": "SUBNET_ROUTE_FILTER_RULES: dst=10.33.0.0/16 port 22 — subnet-router gets no rules", - "GRANT-P13_2": "SUBNET_ROUTE_FILTER_RULES: dst=10.33.0.0/16 port 80-443 — subnet-router gets no rules", - "GRANT-P13_3": "SUBNET_ROUTE_FILTER_RULES: dst=10.33.0.0/16 ports 22,80,443 — subnet-router gets no rules", - "GRANT-P09_12B": "SUBNET_ROUTE_FILTER_RULES: subnet-router subtest missing entire rule for 10.0.0.0/8", - "GRANT-P15_1": "SUBNET_ROUTE_FILTER_RULES: dst=10.33.1.0/24 port 22 — subnet-router gets no rules", - "GRANT-P15_3": "SUBNET_ROUTE_FILTER_RULES: dst=10.32.0.0/14 port 22 — subnet-router gets no rules", - // ======================================================================== // USER_PASSKEY_WILDCARD (2 tests) // @@ -551,7 +517,6 @@ var grantSkipReasons = map[string]string{ // CAPGRANT_COMPILATION - 49 tests: Implement app->CapGrant FilterRule compilation // ERROR_VALIDATION_GAP - 23 tests: Implement missing grant validation rules // CAPGRANT_COMPILATION_AND_SRCIPS - 11 tests: Both CapGrant compilation + SrcIPs format -// SUBNET_ROUTE_FILTER_RULES - 11 tests: Generate filter rules for subnet-routed CIDRs // VIA_COMPILATION_AND_SRCIPS_FORMAT - 7 tests: Via route compilation + SrcIPs format // VIA_COMPILATION - 3 tests: Via route compilation // AUTOGROUP_DANGER_ALL - 3 tests: Implement autogroup:danger-all support @@ -559,7 +524,7 @@ var grantSkipReasons = map[string]string{ // VALIDATION_STRICTNESS - 2 tests: headscale too strict (rejects what Tailscale accepts) // SRCIPS_WILDCARD_NODE_DEDUP - 1 test: Wildcard+specific source node IP deduplication // -// Total: 109 tests skipped, ~128 tests expected to pass. +// Total: 99 tests skipped, ~138 tests expected to pass. func TestGrantsCompat(t *testing.T) { t.Parallel() diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 28da8dc0..f774e4be 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -62,7 +62,7 @@ const ( // ACL validation errors. var ( - ErrACLAutogroupSelfInvalidSource = errors.New("autogroup:self destination requires sources to be users, groups, or autogroup:member only") + ErrACLAutogroupSelfInvalidSource = errors.New("autogroup:self can only be used with users, groups, or supported autogroups") ) // Grant validation errors. @@ -102,7 +102,7 @@ var ( ErrGroupValueNotArray = errors.New("group value must be an array of users") ErrNestedGroups = errors.New("nested groups are not allowed") ErrInvalidHostIP = errors.New("hostname contains invalid IP address") - ErrTagNotDefined = errors.New("tag not defined in policy") + ErrTagNotDefined = errors.New("tag not found") ErrAutoApproverNotAlias = errors.New("auto approver is not an alias") ErrInvalidACLAction = errors.New("invalid ACL action") ErrInvalidSSHAction = errors.New("invalid SSH action") @@ -111,7 +111,7 @@ var ( ErrProtocolOutOfRange = errors.New("protocol number out of range (0-255)") ErrAutogroupNotSupported = errors.New("autogroup not supported in headscale") ErrAutogroupInternetSrc = errors.New("autogroup:internet can only be used in ACL destinations") - ErrAutogroupSelfSrc = errors.New("autogroup:self can only be used in ACL destinations") + ErrAutogroupSelfSrc = errors.New("\"autogroup:self\" not valid on the src side of a rule") ErrAutogroupNotSupportedACLSrc = errors.New("autogroup not supported for ACL sources") ErrAutogroupNotSupportedACLDst = errors.New("autogroup not supported for ACL destinations") ErrAutogroupNotSupportedSSHSrc = errors.New("autogroup not supported for SSH sources") @@ -835,6 +835,8 @@ func (ve *AliasWithPorts) UnmarshalJSON(b []byte) error { err error ) + originalDst := vs + if strings.Contains(vs, ":") { vs, portsPart, err = splitDestinationAndPort(vs) if err != nil { @@ -843,7 +845,10 @@ func (ve *AliasWithPorts) UnmarshalJSON(b []byte) error { ports, err := parsePortRange(portsPart) if err != nil { - return err + return fmt.Errorf( + "dst=%q: port range %q: %w", + originalDst, portsPart, err, + ) } ve.Ports = ports @@ -895,7 +900,7 @@ func (ve *ProtocolPort) UnmarshalJSON(b []byte) error { if !strings.Contains(vs, ":") { ports, err := parsePortRange(vs) if err != nil { - return err + return fmt.Errorf("port range %q: %w", vs, err) } ve.Protocol = ProtocolNameWildcard @@ -920,7 +925,7 @@ func (ve *ProtocolPort) UnmarshalJSON(b []byte) error { ports, err := parsePortRange(portsPart) if err != nil { - return err + return fmt.Errorf("port range %q: %w", portsPart, err) } ve.Protocol = protocol @@ -1588,7 +1593,7 @@ func (a *Action) UnmarshalJSON(b []byte) error { case "accept": *a = ActionAccept default: - return fmt.Errorf("%w: %q, must be %q", ErrInvalidACLAction, str, ActionAccept) + return fmt.Errorf("action=%q is not supported: %w", str, ErrInvalidACLAction) } return nil @@ -2178,7 +2183,7 @@ func (p *Policy) validate() error { err := p.TagOwners.Contains(tagOwner) if err != nil { - errs = append(errs, err) + errs = append(errs, fmt.Errorf("src=%w", err)) } } } @@ -2209,7 +2214,7 @@ func (p *Policy) validate() error { case *Tag: err := p.TagOwners.Contains(h) if err != nil { - errs = append(errs, err) + errs = append(errs, fmt.Errorf("dst=%q: %w", *h, err)) } } } diff --git a/hscontrol/policy/v2/types_test.go b/hscontrol/policy/v2/types_test.go index 70f95fbd..ee3d29f0 100644 --- a/hscontrol/policy/v2/types_test.go +++ b/hscontrol/policy/v2/types_test.go @@ -590,7 +590,7 @@ func TestUnmarshalPolicy(t *testing.T) { ] } `, - wantErr: `tag not defined in policy: "tag:test"`, + wantErr: `tag not found: "tag:test"`, }, { name: "autogroup:internet-in-ssh-dst-not-allowed", @@ -854,7 +854,7 @@ func TestUnmarshalPolicy(t *testing.T) { ] } `, - wantErr: `tag not defined in policy: "tag:notdefined"`, + wantErr: `tag not found: "tag:notdefined"`, }, { name: "tag-must-be-defined-acl-dst", @@ -873,7 +873,7 @@ func TestUnmarshalPolicy(t *testing.T) { ] } `, - wantErr: `tag not defined in policy: "tag:notdefined"`, + wantErr: `tag not found: "tag:notdefined"`, }, { name: "tag-must-be-defined-acl-ssh-src", @@ -892,7 +892,7 @@ func TestUnmarshalPolicy(t *testing.T) { ] } `, - wantErr: `tag not defined in policy: "tag:notdefined"`, + wantErr: `tag not found: "tag:notdefined"`, }, { name: "tag-must-be-defined-acl-ssh-dst", @@ -914,7 +914,7 @@ func TestUnmarshalPolicy(t *testing.T) { ] } `, - wantErr: `tag not defined in policy: "tag:notdefined"`, + wantErr: `tag not found: "tag:notdefined"`, }, { name: "tag-must-be-defined-acl-autoapprover-route", @@ -927,7 +927,7 @@ func TestUnmarshalPolicy(t *testing.T) { }, } `, - wantErr: `tag not defined in policy: "tag:notdefined"`, + wantErr: `tag not found: "tag:notdefined"`, }, { name: "tag-must-be-defined-acl-autoapprover-exitnode", @@ -938,7 +938,7 @@ func TestUnmarshalPolicy(t *testing.T) { }, } `, - wantErr: `tag not defined in policy: "tag:notdefined"`, + wantErr: `tag not found: "tag:notdefined"`, }, { name: "missing-dst-port-is-err", @@ -3928,7 +3928,7 @@ func TestACL_UnmarshalJSON_InvalidAction(t *testing.T) { _, err := unmarshalPolicy([]byte(policyJSON)) require.Error(t, err) - assert.Contains(t, err.Error(), `invalid ACL action: "deny"`) + assert.Contains(t, err.Error(), `action="deny" is not supported`) } // Helper function to parse aliases for testing. @@ -4691,7 +4691,7 @@ func TestUnmarshalGrants(t *testing.T) { ] } `, - wantErr: "tag not defined in policy", + wantErr: "tag not found", }, { name: "invalid-grant-undefined-destination-host", @@ -4724,7 +4724,7 @@ func TestUnmarshalGrants(t *testing.T) { ] } `, - wantErr: "autogroup:self destination requires sources to be users, groups, or autogroup:member only", + wantErr: "autogroup:self can only be used with users, groups, or supported autogroups", }, } diff --git a/hscontrol/policy/v2/utils.go b/hscontrol/policy/v2/utils.go index 2f22a9cf..10641259 100644 --- a/hscontrol/policy/v2/utils.go +++ b/hscontrol/policy/v2/utils.go @@ -19,7 +19,7 @@ var ( ErrInvalidPortRangeFormat = errors.New("invalid port range format") ErrPortRangeInverted = errors.New("invalid port range: first port is greater than last port") ErrPortMustBePositive = errors.New("first port must be >0, or use '*' for wildcard") - ErrInvalidPortNumber = errors.New("invalid port number") + ErrInvalidPortNumber = errors.New("invalid first integer") ErrPortNumberOutOfRange = errors.New("port number out of range") ErrBracketsNotIPv6 = errors.New("square brackets are only valid around IPv6 addresses") ) diff --git a/hscontrol/policy/v2/utils_test.go b/hscontrol/policy/v2/utils_test.go index 19c723dc..393a3b1f 100644 --- a/hscontrol/policy/v2/utils_test.go +++ b/hscontrol/policy/v2/utils_test.go @@ -162,8 +162,8 @@ func TestParsePort(t *testing.T) { {"65535", 65535, ""}, {"-1", 0, "port number out of range"}, {"65536", 0, "port number out of range"}, - {"abc", 0, "invalid port number"}, - {"", 0, "invalid port number"}, + {"abc", 0, "invalid first integer"}, + {"", 0, "invalid first integer"}, } for _, test := range tests { @@ -195,9 +195,9 @@ func TestParsePortRange(t *testing.T) { {"*", []tailcfg.PortRange{tailcfg.PortRangeAny}, ""}, {"80-", nil, "invalid port range format"}, {"-90", nil, "invalid port range format"}, - {"80-90,", nil, "invalid port number"}, + {"80-90,", nil, "invalid first integer"}, {"80,90-", nil, "invalid port range format"}, - {"80-90,abc", nil, "invalid port number"}, + {"80-90,abc", nil, "invalid first integer"}, {"80-90,65536", nil, "port number out of range"}, {"80-90,90-80", nil, "invalid port range: first port is greater than last port"}, }