rsp2k/headscale
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity

policyutil: fix reduceCapGrantRule and add route reduction

Browse source 
reduceCapGrantRule was dropping rules whose CapGrant IPs overlap a
subnet route; treat subnet routes as part of node identity so those rules
survive reduction. ReduceFilterRules now also reduces route-reachable
destinations.

Updates #3157
This commit is contained in:
Kristoffer Dalby 2026-04-15 08:27:42 +00:00
parent b051e7b2bc
commit ded51a4d30
3 changed files with 553 additions and 63 deletions
Download patch file Download diff file Expand all files Collapse all files

503
hscontrol/policy/policyutil/reduce_test.go
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{
@ -759,7 +765,9 @@ func TestReduceFilterRules(t *testing.T) {
}
for _, tt := range tests {
for idx, pmf := range policy.PolicyManagerFuncsForTest([]byte(tt.pol)) {
for idx, pmf := range policy.PolicyManagerFuncsForTest(
[]byte(tt.pol),
) {
t.Run(fmt.Sprintf("%s-index%d", tt.name, idx), func(t *testing.T) {
var (
pm policy.PolicyManager
@ -781,3 +789,494 @@ func TestReduceFilterRules(t *testing.T) {
}
}
}
// 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
node *types.Node
rules []tailcfg.FilterRule
wantCount int
wantRoutes []string
}{
{
name: "approved-route-included",
node: &types.Node{
IPv4: ap("100.64.0.1"),
IPv6: ap("fd7a:115c:a1e0::1"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.33.0.0/16"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.33.0.0/16"),
},
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"*"},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.33.0.0/16", Ports: tailcfg.PortRangeAny},
},
},
},
wantCount: 1,
wantRoutes: []string{"10.33.0.0/16"},
},
{
name: "unapproved-route-excluded",
node: &types.Node{
IPv4: ap("100.64.0.1"),
IPv6: ap("fd7a:115c:a1e0::1"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.33.0.0/16"),
// Advertised but NOT approved:
netip.MustParsePrefix("172.16.0.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
// Only 10.33.0.0/16 approved
netip.MustParsePrefix("10.33.0.0/16"),
},
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"*"},
DstPorts: []tailcfg.NetPortRange{
// Targets the unapproved route
{IP: "172.16.0.0/24", Ports: tailcfg.PortRangeAny},
},
},
},
// 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",
node: &types.Node{
IPv4: ap("100.64.0.1"),
IPv6: ap("fd7a:115c:a1e0::1"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.33.0.0/16"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.33.0.0/16"),
},
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"*"},
DstPorts: []tailcfg.NetPortRange{
// Not advertised, not approved
{IP: "192.168.0.0/16", Ports: tailcfg.PortRangeAny},
},
},
},
wantCount: 0,
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := policyutil.ReduceFilterRules(
tt.node.View(), tt.rules,
)
require.Len(t, got, tt.wantCount,
"rule count mismatch")
if tt.wantCount > 0 {
var gotRoutes []string
for _, dp := range got[0].DstPorts {
gotRoutes = append(gotRoutes, dp.IP)
}
require.Equal(t, tt.wantRoutes, gotRoutes)
}
})
}
}
// TestReduceFilterRulesCapGrant tests the CapGrant branch of
// ReduceFilterRules, which was previously untested. All existing
// test cases use ACL-only policies with DstPorts rules.
func TestReduceFilterRulesCapGrant(t *testing.T) {
tests := []struct {
name string
node *types.Node
rules []tailcfg.FilterRule
want []tailcfg.FilterRule
}{
{
name: "capgrant-matches-node-ip-narrowed",
node: &types.Node{
IPv4: ap("100.64.0.1"),
IPv6: ap("fd7a:115c:a1e0::1"),
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.0.0.0/8"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
// Broad IPv4 prefix containing node IP
netip.MustParsePrefix("100.64.0.0/10"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/drive-sharer": nil,
},
},
},
},
},
want: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.0.0.0/8"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
// Only IPv4 narrowed (IPv6 not in /10)
netip.MustParsePrefix("100.64.0.1/32"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/drive-sharer": nil,
},
},
},
},
},
},
{
name: "capgrant-no-match-filtered-out",
node: &types.Node{
IPv4: ap("100.64.0.1"),
IPv6: ap("fd7a:115c:a1e0::1"),
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.0.0.0/8"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
// Different IP — doesn't match this node
netip.MustParsePrefix("100.64.0.99/32"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/drive-sharer": nil,
},
},
},
},
},
want: []tailcfg.FilterRule{},
},
{
name: "capgrant-with-subnet-route-overlap",
node: &types.Node{
IPv4: ap("100.64.0.1"),
IPv6: ap("fd7a:115c:a1e0::1"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.33.0.0/16"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.33.0.0/16"),
},
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"*"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
// Subnet route overlap
netip.MustParsePrefix("10.0.0.0/8"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/relay-target": nil,
},
},
},
},
},
want: []tailcfg.FilterRule{
{
SrcIPs: []string{"*"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
// 10.0.0.0/8 doesn't contain node's
// CGNAT IP (100.64.0.1), so no IP
// narrowing. Only route overlap kept
// as original prefix.
netip.MustParsePrefix("10.0.0.0/8"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/relay-target": nil,
},
},
},
},
},
},
{
name: "capgrant-exit-route-skipped",
node: &types.Node{
IPv4: ap("100.64.0.1"),
IPv6: ap("fd7a:115c:a1e0::1"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: tsaddr.ExitRoutes(),
},
ApprovedRoutes: tsaddr.ExitRoutes(),
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"*"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
// 0.0.0.0/0 overlaps the exit route
// but exit routes should be skipped
netip.MustParsePrefix("0.0.0.0/0"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/drive-sharer": nil,
},
},
},
},
},
want: []tailcfg.FilterRule{
{
SrcIPs: []string{"*"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
// Node IP narrowed (0.0.0.0/0
// contains the node's IP)
netip.MustParsePrefix("100.64.0.1/32"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/drive-sharer": nil,
},
},
},
},
},
},
{
name: "mixed-dstports-and-capgrant-rules",
node: &types.Node{
IPv4: ap("100.64.0.1"),
IPv6: ap("fd7a:115c:a1e0::1"),
},
rules: []tailcfg.FilterRule{
{
// DstPorts rule that matches
SrcIPs: []string{"10.0.0.0/8"},
DstPorts: []tailcfg.NetPortRange{
{IP: "100.64.0.1", Ports: tailcfg.PortRangeAny},
},
},
{
// CapGrant rule that matches
SrcIPs: []string{"10.0.0.0/8"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
netip.MustParsePrefix("100.64.0.1/32"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/drive-sharer": nil,
},
},
},
},
{
// CapGrant rule that doesn't match
SrcIPs: []string{"10.0.0.0/8"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
netip.MustParsePrefix("100.64.0.99/32"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/drive-sharer": nil,
},
},
},
},
},
want: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.0.0.0/8"},
DstPorts: []tailcfg.NetPortRange{
{IP: "100.64.0.1", Ports: tailcfg.PortRangeAny},
},
},
{
SrcIPs: []string{"10.0.0.0/8"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
netip.MustParsePrefix("100.64.0.1/32"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/drive-sharer": nil,
},
},
},
},
// Third rule filtered out — doesn't match node
},
},
{
name: "capgrant-ipv4-only-node",
node: &types.Node{
IPv4: ap("100.64.0.1"),
// IPv6 is nil, single-IP node
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.0.0.0/8"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
netip.MustParsePrefix("100.64.0.1/32"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/drive-sharer": nil,
},
},
},
},
},
want: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.0.0.0/8"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
netip.MustParsePrefix("100.64.0.1/32"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/drive-sharer": nil,
},
},
},
},
},
},
{
name: "capgrant-zero-ip-node-no-panic",
node: &types.Node{
// Both IPv4 and IPv6 are nil, zero IPs.
// Must not panic.
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.0.0.0/8"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
netip.MustParsePrefix("100.64.0.1/32"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/drive-sharer": nil,
},
},
},
},
},
want: []tailcfg.FilterRule{},
},
{
// A broad prefix that both contains the node IP (/32 narrow)
// and overlaps one of its approved subnet routes (route
// preserved) would otherwise be emitted twice.
name: "capgrant-ip-and-route-overlap-dedup",
node: &types.Node{
IPv4: ap("100.64.0.1"),
IPv6: ap("fd7a:115c:a1e0::1"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("100.64.0.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("100.64.0.0/24"),
},
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"*"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
netip.MustParsePrefix("100.64.0.0/24"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/relay-target": nil,
},
},
},
},
},
want: []tailcfg.FilterRule{
{
SrcIPs: []string{"*"},
CapGrant: []tailcfg.CapGrant{
{
Dsts: []netip.Prefix{
netip.MustParsePrefix("100.64.0.1/32"),
netip.MustParsePrefix("100.64.0.0/24"),
},
CapMap: tailcfg.PeerCapMap{
"tailscale.com/cap/relay-target": nil,
},
},
},
},
},
},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
got := policyutil.ReduceFilterRules(tt.node.View(), tt.rules)
require.Len(t, got, len(tt.want),
"rule count mismatch")
for i := range tt.want {
require.Equal(t, tt.want[i].SrcIPs, got[i].SrcIPs,
"rule[%d] SrcIPs", i)
require.Len(t, got[i].DstPorts, len(tt.want[i].DstPorts),
"rule[%d] DstPorts count", i)
require.Len(t, got[i].CapGrant, len(tt.want[i].CapGrant),
"rule[%d] CapGrant count", i)
for j := range tt.want[i].CapGrant {
require.ElementsMatch(t,
tt.want[i].CapGrant[j].Dsts,
got[i].CapGrant[j].Dsts,
"rule[%d].CapGrant[%d] Dsts", i, j,
)
}
}
})
}
}