From e05f45cfb12b71a66660b5809f4624b2a5c9c561 Mon Sep 17 00:00:00 2001 From: Kristoffer Dalby Date: Wed, 18 Mar 2026 13:40:36 +0000 Subject: [PATCH] policy/v2: use approved node routes in wildcard SrcIPs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Tailscale documentation, the wildcard (*) source includes "any approved subnets" — the actually-advertised-and-approved routes from nodes, not the autoApprover policy prefixes. Change Asterix.resolve() to return just the base CGNAT+ULA set, and add approved subnet routes as separate SrcIPs entries in the filter compilation path. This preserves individual route prefixes that would otherwise be merged by IPSet (e.g., 10.0.0.0/8 absorbing 10.33.0.0/16). Also swap rule ordering in compileGrantWithAutogroupSelf() to emit non-self destination rules before autogroup:self rules, matching the Tailscale FilterRule wire format ordering. Remove the unused AutoApproverPolicy.prefixes() method. Updates #2180 --- hscontrol/policy/v2/filter.go | 92 +++++++++++++------ .../v2/tailscale_acl_data_compat_test.go | 18 ++-- .../policy/v2/tailscale_grants_compat_test.go | 6 +- hscontrol/policy/v2/types.go | 48 +++++----- 4 files changed, 103 insertions(+), 61 deletions(-) diff --git a/hscontrol/policy/v2/filter.go b/hscontrol/policy/v2/filter.go index 30e7acbc..6ad7fa91 100644 --- a/hscontrol/policy/v2/filter.go +++ b/hscontrol/policy/v2/filter.go @@ -21,6 +21,34 @@ var ( errSelfInSources = errors.New("autogroup:self cannot be used in sources") ) +// sourcesHaveWildcard returns true if any of the source aliases is +// a wildcard (*). Used to determine whether approved subnet routes +// should be appended to SrcIPs. +func sourcesHaveWildcard(srcs Aliases) bool { + for _, src := range srcs { + if _, ok := src.(Asterix); ok { + return true + } + } + + return false +} + +// srcIPsWithRoutes returns the SrcIPs string slice, appending +// approved subnet routes when the sources include a wildcard. +func srcIPsWithRoutes( + resolved ResolvedAddresses, + hasWildcard bool, + nodes views.Slice[types.NodeView], +) []string { + ips := resolved.Strings() + if hasWildcard { + ips = append(ips, approvedSubnetRoutes(nodes)...) + } + + return ips +} + // compileFilterRules takes a set of nodes and an ACLPolicy and generates a // set of Tailscale compatible FilterRules used to allow traffic on clients. func (pol *Policy) compileFilterRules( @@ -48,12 +76,14 @@ func (pol *Policy) compileFilterRules( continue } + hasWildcard := sourcesHaveWildcard(grant.Sources) + for _, ipp := range grant.InternetProtocols { destPorts := pol.destinationsToNetPortRange(users, nodes, grant.Destinations, ipp.Ports) if len(destPorts) > 0 { rules = append(rules, tailcfg.FilterRule{ - SrcIPs: srcIPs.Strings(), + SrcIPs: srcIPsWithRoutes(srcIPs, hasWildcard, nodes), DstPorts: destPorts, IPProto: ipp.Protocol.toIANAProtocolNumbers(), }) @@ -76,7 +106,7 @@ func (pol *Policy) compileFilterRules( } rules = append(rules, tailcfg.FilterRule{ - SrcIPs: srcIPs.Strings(), + SrcIPs: srcIPsWithRoutes(srcIPs, hasWildcard, nodes), CapGrant: capGrants, }) } @@ -222,7 +252,38 @@ func (pol *Policy) compileGrantWithAutogroupSelf( return rules, nil } + hasWildcard := sourcesHaveWildcard(grant.Sources) + for _, ipp := range grant.InternetProtocols { + // Handle non-self destinations first to match Tailscale's + // rule ordering in the FilterRule wire format. + if len(otherDests) > 0 { + var srcIPs netipx.IPSetBuilder + + for _, ips := range resolvedSrcs { + for _, pref := range ips.Prefixes() { + srcIPs.AddPrefix(pref) + } + } + + srcResolved, err := newResolved(&srcIPs) + if err != nil { + return nil, err + } + + if !srcResolved.Empty() { + destPorts := pol.destinationsToNetPortRange(users, nodes, otherDests, ipp.Ports) + + if len(destPorts) > 0 { + rules = append(rules, tailcfg.FilterRule{ + SrcIPs: srcIPsWithRoutes(srcResolved, hasWildcard, nodes), + DstPorts: destPorts, + IPProto: ipp.Protocol.toIANAProtocolNumbers(), + }) + } + } + } + // Handle autogroup:self destinations (if any) // Tagged nodes don't participate in autogroup:self (identity is tag-based, not user-based) if len(autogroupSelfDests) > 0 && !node.IsTagged() { @@ -277,33 +338,6 @@ func (pol *Policy) compileGrantWithAutogroupSelf( } } } - - if len(otherDests) > 0 { - var srcIPs netipx.IPSetBuilder - - for _, ips := range resolvedSrcs { - for _, pref := range ips.Prefixes() { - srcIPs.AddPrefix(pref) - } - } - - srcResolved, err := newResolved(&srcIPs) - if err != nil { - return nil, err - } - - if !srcResolved.Empty() { - destPorts := pol.destinationsToNetPortRange(users, nodes, otherDests, ipp.Ports) - - if len(destPorts) > 0 { - rules = append(rules, tailcfg.FilterRule{ - SrcIPs: srcResolved.Strings(), - DstPorts: destPorts, - IPProto: ipp.Protocol.toIANAProtocolNumbers(), - }) - } - } - } } return rules, nil diff --git a/hscontrol/policy/v2/tailscale_acl_data_compat_test.go b/hscontrol/policy/v2/tailscale_acl_data_compat_test.go index 833e3233..7c371b36 100644 --- a/hscontrol/policy/v2/tailscale_acl_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_acl_data_compat_test.go @@ -89,7 +89,11 @@ func setupACLCompatNodes(users types.Users) types.Nodes { { ID: 7, GivenName: "subnet-router", IPv4: ptrAddr("100.92.142.61"), IPv6: ptrAddr("fd7a:115c:a1e0::3e37:8e3d"), - Tags: []string{"tag:router"}, Hostinfo: &tailcfg.Hostinfo{}, + Tags: []string{"tag:router"}, + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.33.0.0/16")}, + }, + ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.33.0.0/16")}, }, { ID: 8, GivenName: "exit-node", @@ -146,11 +150,13 @@ type aclTestFile struct { } `json:"input"` Topology struct { Nodes map[string]struct { - Hostname string `json:"hostname"` - Tags []string `json:"tags"` - IPv4 string `json:"ipv4"` - IPv6 string `json:"ipv6"` - User string `json:"user"` + Hostname string `json:"hostname"` + Tags []string `json:"tags"` + IPv4 string `json:"ipv4"` + IPv6 string `json:"ipv6"` + User string `json:"user"` + RoutableIPs []string `json:"routable_ips"` + ApprovedRoutes []string `json:"approved_routes"` } `json:"nodes"` } `json:"topology"` Captures map[string]struct { diff --git a/hscontrol/policy/v2/tailscale_grants_compat_test.go b/hscontrol/policy/v2/tailscale_grants_compat_test.go index fa7aa5ea..a6c5eb88 100644 --- a/hscontrol/policy/v2/tailscale_grants_compat_test.go +++ b/hscontrol/policy/v2/tailscale_grants_compat_test.go @@ -20,6 +20,7 @@ package v2 import ( "encoding/json" + "net/netip" "os" "path/filepath" "strings" @@ -136,7 +137,10 @@ func setupGrantsCompatNodes(users types.Users) types.Nodes { IPv4: ptrAddr("100.92.142.61"), IPv6: ptrAddr("fd7a:115c:a1e0::3e37:8e3d"), Tags: []string{"tag:router"}, - Hostinfo: &tailcfg.Hostinfo{}, + Hostinfo: &tailcfg.Hostinfo{ + RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.33.0.0/16")}, + }, + ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.33.0.0/16")}, } nodeExitNode := &types.Node{ diff --git a/hscontrol/policy/v2/types.go b/hscontrol/policy/v2/types.go index 00870741..28da8dc0 100644 --- a/hscontrol/policy/v2/types.go +++ b/hscontrol/policy/v2/types.go @@ -296,19 +296,33 @@ func (a Asterix) Resolve(p *Policy, u types.Users, n views.Slice[types.NodeView] return newResolvedAddresses(a.resolve(p, u, n)) } -func (a Asterix) resolve(p *Policy, _ types.Users, _ views.Slice[types.NodeView]) (*netipx.IPSet, error) { - if p != nil && len(p.AutoApprovers.prefixes()) > 0 { - var ipb netipx.IPSetBuilder - ipb.AddSet(asterixResolved()) +func (a Asterix) resolve(_ *Policy, _ types.Users, _ views.Slice[types.NodeView]) (*netipx.IPSet, error) { + return asterixResolved(), nil +} - for _, pfx := range p.AutoApprovers.prefixes() { - ipb.AddPrefix(pfx) +// approvedSubnetRoutes collects all approved non-exit subnet routes +// advertised across all nodes. Per Tailscale documentation, wildcard +// (*) SrcIPs include "any approved subnets". +// +// These are collected separately from the Asterix IPSet because +// IPSet merges overlapping ranges (e.g. 10.0.0.0/8 absorbs +// 10.33.0.0/16), but Tailscale preserves individual route entries. +func approvedSubnetRoutes(nodes views.Slice[types.NodeView]) []string { + seen := make(map[string]bool) + + var routes []string + + for _, node := range nodes.All() { + for _, route := range node.SubnetRoutes() { + s := route.String() + if !seen[s] { + seen[s] = true + routes = append(routes, s) + } } - - return ipb.IPSet() } - return asterixResolved(), nil + return routes } // Username is a string that represents a username, it must contain an @. @@ -1481,22 +1495,6 @@ func (ap AutoApproverPolicy) MarshalJSON() ([]byte, error) { return json.Marshal(&obj) } -// prefixes returns the prefixes that have auto-approvers defined in the policy. -// It filters out exit routes since they are not associated with a specific prefix and are handled separately. -func (ap AutoApproverPolicy) prefixes() []netip.Prefix { - prefixes := make([]netip.Prefix, 0, len(ap.Routes)) - - for prefix := range ap.Routes { - if tsaddr.IsExitRoute(prefix) { - continue - } - - prefixes = append(prefixes, prefix) - } - - return prefixes -} - // resolveAutoApprovers resolves the AutoApprovers to a map of netip.Prefix to netipx.IPSet. // The resulting map can be used to quickly look up if a node can self-approve a route. // It is intended for internal use in a PolicyManager.