policyutil: reduce filter rules by approved routes

Updates #3157
This commit is contained in:
Kristoffer Dalby
2026-04-09 15:14:50 +00:00
parent 387dcd679e
commit f60f89647b
2 changed files with 42 additions and 62 deletions
+22 -45
View File
@@ -6,7 +6,6 @@ import (
"github.com/juanfont/headscale/hscontrol/types"
"github.com/juanfont/headscale/hscontrol/util"
"tailscale.com/net/tsaddr"
"tailscale.com/tailcfg"
)
@@ -48,33 +47,18 @@ func ReduceFilterRules(node types.NodeView, rules []tailcfg.FilterRule) []tailcf
continue DEST_LOOP
}
// If the node advertises routes, ensure filter rules
// targeting those routes are preserved. Exit routes
// (0.0.0.0/0, ::/0) are skipped because exit nodes
// handle traffic via AllowedIPs/routing, not packet
// filter rules. This matches Tailscale SaaS behavior.
//
// NOTE: This uses RoutableIPs (advertised routes)
// rather than SubnetRoutes (approved routes). The two
// sets are identical in all 98 golden file captures
// from Tailscale SaaS, so we cannot determine from
// captured data which set Tailscale actually checks.
// RoutableIPs is a superset of SubnetRoutes (which
// further filters by approval), so this is the more
// permissive choice. See MISSING_SAAS_DATA.md for
// the capture needed to resolve this ambiguity.
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
}
}
// If the node has approved subnet routes, preserve
// filter rules targeting those routes. SubnetRoutes()
// returns only approved, non-exit routes — matching
// Tailscale SaaS behavior, which does not generate
// filter rules for advertised-but-unapproved routes.
// Exit routes (0.0.0.0/0, ::/0) are excluded by
// SubnetRoutes() and handled separately via
// AllowedIPs/routing.
for _, subnetRoute := range node.SubnetRoutes() {
if expanded.OverlapsPrefix(subnetRoute) {
dests = append(dests, dest)
continue DEST_LOOP
}
}
}
@@ -127,23 +111,16 @@ func reduceCapGrantRule(
}
}
// Also check routable IPs (subnet routes) — nodes that
// advertise routes should receive CapGrant rules for
// destinations that overlap their routes.
if node.Hostinfo().Valid() {
routableIPs := node.Hostinfo().RoutableIPs()
if routableIPs.Len() > 0 {
for _, dst := range cg.Dsts {
for _, routableIP := range routableIPs.All() {
if tsaddr.IsExitRoute(routableIP) {
continue
}
if dst.Overlaps(routableIP) {
// For route overlaps, keep the original prefix.
matchingDsts = append(matchingDsts, dst)
}
}
// Also check approved subnet routes — nodes serving
// approved routes should receive CapGrant rules for
// destinations that overlap those routes. SubnetRoutes()
// excludes both unapproved and exit routes, matching
// Tailscale SaaS behavior.
for _, dst := range cg.Dsts {
for _, subnetRoute := range node.SubnetRoutes() {
if dst.Overlaps(subnetRoute) {
// For route overlaps, keep the original prefix.
matchingDsts = append(matchingDsts, dst)
}
}
}
+20 -17
View File
@@ -196,6 +196,9 @@ func TestReduceFilterRules(t *testing.T) {
netip.MustParsePrefix("10.33.0.0/16"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.33.0.0/16"),
},
},
peers: types.Nodes{
&types.Node{
@@ -518,6 +521,7 @@ func TestReduceFilterRules(t *testing.T) {
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{netip.MustParsePrefix("8.0.0.0/16"), netip.MustParsePrefix("16.0.0.0/16")},
},
ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("8.0.0.0/16"), netip.MustParsePrefix("16.0.0.0/16")},
},
peers: types.Nodes{
&types.Node{
@@ -601,6 +605,7 @@ func TestReduceFilterRules(t *testing.T) {
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{netip.MustParsePrefix("8.0.0.0/8"), netip.MustParsePrefix("16.0.0.0/8")},
},
ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("8.0.0.0/8"), netip.MustParsePrefix("16.0.0.0/8")},
},
peers: types.Nodes{
&types.Node{
@@ -676,7 +681,8 @@ func TestReduceFilterRules(t *testing.T) {
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{netip.MustParsePrefix("172.16.0.0/24")},
},
Tags: []string{"tag:access-servers"},
ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("172.16.0.0/24")},
Tags: []string{"tag:access-servers"},
},
peers: types.Nodes{
&types.Node{
@@ -784,16 +790,13 @@ func TestReduceFilterRules(t *testing.T) {
}
}
// TestReduceFilterRulesPartialApproval documents the behavior of
// ReduceFilterRules when RoutableIPs (advertised) and SubnetRoutes
// (approved) differ. In production, SubnetRoutes is always a subset
// of RoutableIPs, but the two code paths in ReduceFilterRules handle
// them independently:
// - Lines 56-68: Check RoutableIPs (advertised, may be unapproved)
// - Lines 73-79: Check SubnetRoutes (approved only)
//
// This test documents that ReduceFilterRules includes filter rules
// for advertised-but-unapproved routes via the RoutableIPs check.
// TestReduceFilterRulesPartialApproval verifies that ReduceFilterRules
// only preserves filter rules for routes that are both advertised
// (RoutableIPs) AND approved (ApprovedRoutes), matching Tailscale
// SaaS behavior. Advertised-but-unapproved routes do not cause rule
// preservation: SaaS never generates filter rules for unapproved
// routes, and headscale consults node.SubnetRoutes() (which filters
// by approval) rather than Hostinfo.RoutableIPs() (which does not).
func TestReduceFilterRulesPartialApproval(t *testing.T) {
tests := []struct {
name string
@@ -828,7 +831,7 @@ func TestReduceFilterRulesPartialApproval(t *testing.T) {
wantRoutes: []string{"10.33.0.0/16"},
},
{
name: "unapproved-route-still-included-via-routableips",
name: "unapproved-route-excluded",
node: &types.Node{
IPv4: ap("100.64.0.1"),
IPv6: ap("fd7a:115c:a1e0::1"),
@@ -853,11 +856,11 @@ func TestReduceFilterRulesPartialApproval(t *testing.T) {
},
},
},
// The RoutableIPs check (line 63) matches because
// 172.16.0.0/24 IS in RoutableIPs. The rule is
// kept even though the route is not approved.
wantCount: 1,
wantRoutes: []string{"172.16.0.0/24"},
// SubnetRoutes() does NOT contain 172.16.0.0/24
// (only approved routes), and the ACL dst does not
// overlap the node's own IPs, so the rule is
// dropped. This matches Tailscale SaaS behavior.
wantCount: 0,
},
{
name: "neither-advertised-nor-approved-excluded",