eliott/headscale
1
0
Fork 0
mirror of https://github.com/juanfont/headscale synced 2026-04-25 17:15:33 +02:00
Code Issues Packages Projects Releases Wiki Activity

types: consider subnet routes as source identity in ACL matching

Browse Source 
CanAccess now treats a node's advertised subnet routes as part of
its source identity, so an ACL granting the subnet-owner as source lets
traffic from the subnet through. Matches SaaS semantics.

Updates #3157
This commit is contained in:
Kristoffer Dalby
2026-04-15 08:27:07 +00:00
parent f49c42e716
commit b01e67e8e5
3 changed files with 508 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

191
hscontrol/policy/policy_test.go
View File

@@ -4,7 +4,6 @@ import (
"fmt"
"net/netip"
"testing"
"time"
"github.com/google/go-cmp/cmp"
"github.com/juanfont/headscale/hscontrol/policy/matcher"
@@ -768,6 +767,65 @@ func TestReduceNodes(t *testing.T) {
},
},
},
// Subnet-to-subnet: routers must see each other when ACL
// uses only subnet CIDRs. Issue #3157.
{
name: "subnet-to-subnet-routers-see-each-other-3157",
args: args{
nodes: []*types.Node{
{
ID: 1,
IPv4: ap("100.64.0.1"),
Hostname: "router-a",
User: &types.User{Name: "router-a"},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.88.8.0/24")},
},
ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.88.8.0/24")},
},
{
ID: 2,
IPv4: ap("100.64.0.2"),
Hostname: "router-b",
User: &types.User{Name: "router-b"},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.99.9.0/24")},
},
ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.99.9.0/24")},
},
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.88.8.0/24"},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny},
},
},
},
node: &types.Node{
ID: 1,
IPv4: ap("100.64.0.1"),
Hostname: "router-a",
User: &types.User{Name: "router-a"},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.88.8.0/24")},
},
ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.88.8.0/24")},
},
},
want: []*types.Node{
{
ID: 2,
IPv4: ap("100.64.0.2"),
Hostname: "router-b",
User: &types.User{Name: "router-b"},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.99.9.0/24")},
},
ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.99.9.0/24")},
},
},
},
}
for _, tt := range tests {
@@ -1205,11 +1263,11 @@ func TestSSHPolicyRules(t *testing.T) {
},
Action: &tailcfg.SSHAction{
Accept: false,
SessionDuration: 24 * time.Hour,
HoldAndDelegate: "unused-url/machine/ssh/action/from/$SRC_NODE_ID/to/$DST_NODE_ID?ssh_user=$SSH_USER&local_user=$LOCAL_USER",
AllowAgentForwarding: true,
AllowLocalPortForwarding: true,
AllowRemotePortForwarding: true,
SessionDuration: 0,
HoldAndDelegate: "unused-url/machine/ssh/action/$SRC_NODE_ID/to/$DST_NODE_ID?local_user=$LOCAL_USER",
AllowAgentForwarding: false,
AllowLocalPortForwarding: false,
AllowRemotePortForwarding: false,
},
},
}},
@@ -2230,6 +2288,127 @@ func TestReduceRoutes(t *testing.T) {
netip.MustParsePrefix("192.168.1.0/14"),
},
},
// Subnet-to-subnet tests for issue #3157.
// When an ACL references subnet CIDRs as both source and destination,
// the subnet routers for those subnets must receive routes to each
// other's subnets.
{
name: "subnet-to-subnet-src-router-gets-dst-route-3157",
args: args{
node: &types.Node{
ID: 1,
IPv4: ap("100.64.0.1"),
User: &types.User{Name: "router-a"},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.88.8.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.88.8.0/24"),
},
},
routes: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.88.8.0/24"},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny},
},
},
},
},
want: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
{
name: "subnet-to-subnet-dst-router-gets-src-route-3157",
args: args{
node: &types.Node{
ID: 2,
IPv4: ap("100.64.0.2"),
User: &types.User{Name: "router-b"},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
routes: []netip.Prefix{
netip.MustParsePrefix("10.88.8.0/24"),
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.88.8.0/24"},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny},
},
},
},
},
want: []netip.Prefix{
netip.MustParsePrefix("10.88.8.0/24"),
},
},
{
name: "subnet-to-subnet-regular-node-no-route-leak-3157",
args: args{
node: &types.Node{
ID: 3,
IPv4: ap("100.64.0.3"),
User: &types.User{Name: "regular-node"},
},
routes: []netip.Prefix{
netip.MustParsePrefix("10.88.8.0/24"),
netip.MustParsePrefix("10.99.9.0/24"),
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.88.8.0/24"},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny},
},
},
},
},
want: nil,
},
{
name: "subnet-to-subnet-unrelated-router-no-route-leak-3157",
args: args{
node: &types.Node{
ID: 4,
IPv4: ap("100.64.0.4"),
User: &types.User{Name: "router-c"},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("172.16.0.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("172.16.0.0/24"),
},
},
routes: []netip.Prefix{
netip.MustParsePrefix("10.88.8.0/24"),
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.88.8.0/24"},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny},
},
},
},
},
want: nil,
},
}
for _, tt := range tests {

137
hscontrol/types/node.go
View File

@@ -40,11 +40,19 @@ var (
type RouteFunc func(id NodeID) []netip.Prefix
// ViaRouteResult describes via grant effects for a viewer-peer pair.
// UsePrimary is always a subset of Include: it marks which included
// prefixes must additionally defer to HA primary election.
type ViaRouteResult struct {
// Include contains prefixes this peer should serve to this viewer (via-designated).
Include []netip.Prefix
// Exclude contains prefixes steered to OTHER peers (suppress from global primary).
Exclude []netip.Prefix
// UsePrimary contains prefixes from Include where a regular
// (non-via) grant also covers the prefix. In these cases HA
// primary election wins — only the primary router should get
// the route in AllowedIPs. When a prefix is NOT in UsePrimary,
// per-viewer via steering applies.
UsePrimary []netip.Prefix
}
type (
@@ -190,6 +198,9 @@ func (node *Node) IsEphemeral() bool {
return node.AuthKey != nil && node.AuthKey.Ephemeral
}
// IPs returns the node's allocated Tailscale addresses. Order is
// deterministic: IPv4 (if allocated) first, IPv6 second. At most one
// of each family.
func (node *Node) IPs() []netip.Addr {
var ret []netip.Addr
@@ -267,9 +278,10 @@ func (node *Node) Prefixes() []netip.Prefix {
return addrs
}
// ExitRoutes returns a list of both exit routes if the
// node has any exit routes enabled.
// If none are enabled, it will return nil.
// ExitRoutes returns the node's approved exit routes (0.0.0.0/0
// and/or ::/0). Consumed unconditionally by RoutesForPeer when the
// viewer uses an exit node; excluded from CanAccessRoute which only
// handles non-exit routing.
func (node *Node) ExitRoutes() []netip.Prefix {
var routes []netip.Prefix
@@ -282,6 +294,9 @@ func (node *Node) ExitRoutes() []netip.Prefix {
return routes
}
// IsExitNode reports whether the node has any approved exit routes.
// Approval is required: an advertised-but-unapproved exit route does
// not make the node an exit node (fix for #3169).
func (node *Node) IsExitNode() bool {
return len(node.ExitRoutes()) > 0
}
@@ -319,27 +334,39 @@ func (node *Node) AppendToIPSet(build *netipx.IPSetBuilder) {
}
}
// CanAccess reports whether node may reach node2 under the given
// matchers. A node owns two source identities for ACL purposes:
// - its own IPs (regular peer membership)
// - any approved subnet routes it advertises (subnet-router-as-src,
// used for subnet-to-subnet ACLs — issue #3157)
//
// Either identity matching a rule's src — combined with the dst
// matching node2's IPs, node2's approved subnet routes, or "the
// internet" when node2 is an exit node — grants access.
func (node *Node) CanAccess(matchers []matcher.Match, node2 *Node) bool {
src := node.IPs()
allowedIPs := node2.IPs()
srcRoutes := node.SubnetRoutes()
dstRoutes := node2.SubnetRoutes()
dstIsExit := node2.IsExitNode()
for _, matcher := range matchers {
if !matcher.SrcsContainsIPs(src...) {
for _, m := range matchers {
srcMatchesIP := m.SrcsContainsIPs(src...)
srcMatchesRoutes := len(srcRoutes) > 0 && m.SrcsOverlapsPrefixes(srcRoutes...)
if !srcMatchesIP && !srcMatchesRoutes {
continue
}
if matcher.DestsContainsIP(allowedIPs...) {
if m.DestsContainsIP(allowedIPs...) {
return true
}
// Check if the node has access to routes that might be part of a
// smaller subnet that is served from node2 as a subnet router.
if matcher.DestsOverlapsPrefixes(node2.SubnetRoutes()...) {
if len(dstRoutes) > 0 && m.DestsOverlapsPrefixes(dstRoutes...) {
return true
}
// If the dst is "the internet" and node2 is an exit node, allow access.
if matcher.DestsIsTheInternet() && node2.IsExitNode() {
if dstIsExit && m.DestsIsTheInternet() {
return true
}
}
@@ -347,8 +374,25 @@ func (node *Node) CanAccess(matchers []matcher.Match, node2 *Node) bool {
return false
}
// CanAccessRoute determines whether a specific route prefix should be
// visible to this node based on the given matchers.
//
// Unlike CanAccess, this function intentionally does NOT check
// DestsIsTheInternet(). Exit routes (0.0.0.0/0, ::/0) are handled by
// RoutesForPeer (state.go) which adds them unconditionally from
// ExitRoutes(), not through ACL-based route filtering. The
// DestsIsTheInternet check in CanAccess exists solely for peer
// visibility determination (should two nodes see each other), which
// is a separate concern from route prefix authorization.
//
// Additionally, autogroup:internet is explicitly skipped during filter
// rule compilation (filter.go), so no matchers ever contain "the
// internet" from internet-targeted ACLs. Wildcard "*" dests produce
// matchers where DestsOverlapsPrefixes(0.0.0.0/0) already returns
// true, so the check would be redundant for that case.
func (node *Node) CanAccessRoute(matchers []matcher.Match, route netip.Prefix) bool {
src := node.IPs()
subnetRoutes := node.SubnetRoutes()
for _, matcher := range matchers {
if matcher.SrcsContainsIPs(src...) && matcher.DestsOverlapsPrefixes(route) {
@@ -358,6 +402,25 @@ func (node *Node) CanAccessRoute(matchers []matcher.Match, route netip.Prefix) b
if matcher.SrcsOverlapsPrefixes(route) && matcher.DestsContainsIP(src...) {
return true
}
// A subnet router acts on behalf of its advertised subnets.
// If the node's approved subnet routes overlap the source set
// and the route overlaps the destination set, the router needs
// this route to forward traffic from its local subnet.
if len(subnetRoutes) > 0 {
if matcher.SrcsOverlapsPrefixes(subnetRoutes...) &&
matcher.DestsOverlapsPrefixes(route) {
return true
}
// Reverse: traffic from the route's subnet is destined for
// this node's subnets; the router needs the route for return
// traffic.
if matcher.SrcsOverlapsPrefixes(route) &&
matcher.DestsOverlapsPrefixes(subnetRoutes...) {
return true
}
}
}
return false
@@ -465,8 +528,10 @@ func (node *Node) GetFQDN(baseDomain string) (string, error) {
return hostname, nil
}
// AnnouncedRoutes returns the list of routes that the node announces.
// It should be used instead of checking Hostinfo.RoutableIPs directly.
// AnnouncedRoutes returns the list of routes the node announces, as
// reported by the client in Hostinfo.RoutableIPs. Announcement alone
// does not grant visibility — see SubnetRoutes for approval-gated
// access.
func (node *Node) AnnouncedRoutes() []netip.Prefix {
if node.Hostinfo == nil {
return nil
@@ -476,7 +541,8 @@ func (node *Node) AnnouncedRoutes() []netip.Prefix {
}
// SubnetRoutes returns the list of routes (excluding exit routes) that the node
// announces and are approved.
// announces and are approved. Also used by CanAccess and CanAccessRoute as part
// of the subnet-router-as-source identity (issue #3157).
//
// IMPORTANT: This method is used for internal data structures and should NOT be
// used for the gRPC Proto conversion. For Proto, SubnetRoutes must be populated
@@ -610,12 +676,8 @@ func EndpointsChanged(oldEndpoints, newEndpoints []netip.AddrPort) bool {
newCopy := slices.Clone(newEndpoints)
// Sort both slices to enable order-independent comparison
slices.SortFunc(oldCopy, func(a, b netip.AddrPort) int {
return a.Compare(b)
})
slices.SortFunc(newCopy, func(a, b netip.AddrPort) int {
return a.Compare(b)
})
slices.SortFunc(oldCopy, netip.AddrPort.Compare)
slices.SortFunc(newCopy, netip.AddrPort.Compare)
return !slices.Equal(oldCopy, newCopy)
}
@@ -1041,28 +1103,49 @@ func (nv NodeView) IPsAsString() []string {
// HasNetworkChanges checks if the node has network-related changes.
// Returns true if IPs, announced routes, or approved routes changed.
// This is primarily used for policy cache invalidation.
// This is primarily used for policy cache invalidation. Route slices
// are compared order-insensitively since clients may re-advertise the
// same routes in a different order.
func (nv NodeView) HasNetworkChanges(other NodeView) bool {
if !slices.Equal(nv.IPs(), other.IPs()) {
return true
}
if !slices.Equal(nv.AnnouncedRoutes(), other.AnnouncedRoutes()) {
if !equalPrefixesUnordered(nv.AnnouncedRoutes(), other.AnnouncedRoutes()) {
return true
}
if !slices.Equal(nv.SubnetRoutes(), other.SubnetRoutes()) {
if !equalPrefixesUnordered(nv.SubnetRoutes(), other.SubnetRoutes()) {
return true
}
if !slices.Equal(nv.ExitRoutes(), other.ExitRoutes()) {
if !equalPrefixesUnordered(nv.ExitRoutes(), other.ExitRoutes()) {
return true
}
return false
}
// HasPolicyChange reports whether the node has changes that affect policy evaluation.
// equalPrefixesUnordered reports whether a and b contain the same
// prefixes, order-independent. Inputs are cloned before sorting so
// callers' slices are not mutated.
func equalPrefixesUnordered(a, b []netip.Prefix) bool {
if len(a) != len(b) {
return false
}
ac := slices.Clone(a)
bc := slices.Clone(b)
slices.SortFunc(ac, netip.Prefix.Compare)
slices.SortFunc(bc, netip.Prefix.Compare)
return slices.Equal(ac, bc)
}
// HasPolicyChange reports whether the node has changes that affect
// policy evaluation. Includes approved subnet routes because they act
// as source identity in CanAccess for subnet-to-subnet ACLs (#3157).
func (nv NodeView) HasPolicyChange(other NodeView) bool {
if nv.UserID() != other.UserID() {
return true
@@ -1076,6 +1159,10 @@ func (nv NodeView) HasPolicyChange(other NodeView) bool {
return true
}
if !equalPrefixesUnordered(nv.SubnetRoutes(), other.SubnetRoutes()) {
return true
}
return false
}

211
hscontrol/types/node_test.go
View File

@@ -113,6 +113,183 @@ func Test_NodeCanAccess(t *testing.T) {
},
want: true,
},
// Subnet-to-subnet tests for issue #3157.
// When ACL src and dst are both subnet CIDRs, subnet
// routers advertising those subnets must see each other.
{
name: "subnet-to-subnet-src-router-sees-dst-router-3157",
node1: Node{
IPv4: iap("100.64.0.1"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.88.8.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.88.8.0/24"),
},
},
node2: Node{
IPv4: iap("100.64.0.2"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.88.8.0/24"},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny},
},
},
},
want: true,
},
{
// With a unidirectional ACL (src=A→dst=B), the dst
// router cannot access the src router. Bidirectional
// peer visibility comes from ReduceNodes checking
// both A.CanAccess(B) || B.CanAccess(A).
name: "subnet-to-subnet-unidirectional-dst-cannot-access-src-3157",
node1: Node{
IPv4: iap("100.64.0.2"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
node2: Node{
IPv4: iap("100.64.0.1"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.88.8.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.88.8.0/24"),
},
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.88.8.0/24"},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny},
},
},
},
want: false,
},
{
// With a bidirectional ACL, both routers can access
// each other.
name: "subnet-to-subnet-bidirectional-3157",
node1: Node{
IPv4: iap("100.64.0.2"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
node2: Node{
IPv4: iap("100.64.0.1"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.88.8.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.88.8.0/24"),
},
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.88.8.0/24"},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny},
},
},
{
SrcIPs: []string{"10.99.9.0/24"},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.88.8.0/24", Ports: tailcfg.PortRangeAny},
},
},
},
want: true,
},
{
name: "subnet-to-subnet-regular-node-excluded-3157",
node1: Node{
IPv4: iap("100.64.0.3"),
},
node2: Node{
IPv4: iap("100.64.0.2"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.88.8.0/24"},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny},
},
},
},
want: false,
},
{
name: "subnet-to-subnet-unrelated-router-excluded-3157",
node1: Node{
IPv4: iap("100.64.0.3"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("172.16.0.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("172.16.0.0/24"),
},
},
node2: Node{
IPv4: iap("100.64.0.2"),
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
ApprovedRoutes: []netip.Prefix{
netip.MustParsePrefix("10.99.9.0/24"),
},
},
rules: []tailcfg.FilterRule{
{
SrcIPs: []string{"10.88.8.0/24"},
DstPorts: []tailcfg.NetPortRange{
{IP: "10.99.9.0/24", Ports: tailcfg.PortRangeAny},
},
},
},
want: false,
},
}
for _, tt := range tests {
@@ -127,6 +304,40 @@ func Test_NodeCanAccess(t *testing.T) {
}
}
// Test_NodeCanAccess_Unidirectional asserts that a one-way rule grants
// access in one direction only. A unidirectional ACL is a valid and
// intentional pattern; the "OR" aggregation in the v1 compat harness
// loses this asymmetry, which motivated the directional split in
// TestRoutesCompatPeerVisibility.
func Test_NodeCanAccess_Unidirectional(t *testing.T) {
iap := func(ipStr string) *netip.Addr {
ip := netip.MustParseAddr(ipStr)
return &ip
}
nodeA := Node{IPv4: iap("100.64.0.1")}
nodeB := Node{IPv4: iap("100.64.0.2")}
rules := []tailcfg.FilterRule{
{
SrcIPs: []string{"100.64.0.1/32"},
DstPorts: []tailcfg.NetPortRange{
{IP: "100.64.0.2/32", Ports: tailcfg.PortRangeAny},
},
},
}
matchers := matcher.MatchesFromFilterRules(rules)
if !nodeA.CanAccess(matchers, &nodeB) {
t.Errorf("A→B: want true, got false")
}
if nodeB.CanAccess(matchers, &nodeA) {
t.Errorf("B→A: want false, got true (unidirectional rule leaked reverse access)")
}
}
func TestNodeFQDN(t *testing.T) {
tests := []struct {
name string