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"}, }