MyFavMirrors/headscale
1
0
Fork 0
mirror of https://github.com/juanfont/headscale.git synced 2025-11-20 01:40:21 -05:00
Code Issues Packages Projects Releases Wiki Activity

Fix exit node visibility issue - filter based on autogroup:internet permission

Browse Source 
- Modified tailNode/tailNodes functions to accept exitRouteFilterFunc parameter
- Added canUseExitRoutes helper to check for broad internet access permission
- Added DestsContainsPrefixes method to matcher for checking prefix containment
- Exit routes now only included in peer AllowedIPs when requesting node has internet access
- Added comprehensive unit tests for both scenarios (with and without autogroup:internet)

Fixes #2788

Co-authored-by: kradalby <98431+kradalby@users.noreply.github.com>
This commit is contained in:
copilot-swe-agent[bot]
2025-11-01 08:53:35 +00:00
parent 309437fa14
commit 31bf3a6637
5 changed files with 422 additions and 1 deletions
Download Patch File Download Diff File Expand all files Collapse all files

62
hscontrol/mapper/builder.go
View File

@@ -7,12 +7,50 @@ import (
"time"
"github.com/juanfont/headscale/hscontrol/policy"
"github.com/juanfont/headscale/hscontrol/policy/matcher"
"github.com/juanfont/headscale/hscontrol/types"
"tailscale.com/tailcfg"
"tailscale.com/types/views"
"tailscale.com/util/multierr"
)
// canUseExitRoutes checks if a node can access exit routes (0.0.0.0/0 and ::/0)
// based on ACL matchers. This specifically checks if the node has permission to
// access the internet broadly, which is required to use exit nodes.
//
// Exit routes should only be visible when the ACL explicitly grants broad internet
// access (e.g., via autogroup:internet), not just access to specific services.
func canUseExitRoutes(node types.NodeView, matchers []matcher.Match) bool {
src := node.IPs()
// Sample public internet IPs to test for broad internet access.
// If the ACL grants access to these well-known public IPs, it's granting
// internet access (e.g., via autogroup:internet).
// Use popular public DNS servers as representatives of internet access.
samplePublicIPs := []netip.Addr{
netip.MustParseAddr("1.1.1.1"), // Cloudflare DNS
netip.MustParseAddr("8.8.8.8"), // Google DNS
netip.MustParseAddr("208.67.222.222"), // OpenDNS
}
// Check if any matcher grants access to sample public IPs
for _, matcher := range matchers {
// Check if this node is in the source
if !matcher.SrcsContainsIPs(src...) {
continue
}
// Check if the destination includes public internet IPs.
// This will be true for autogroup:internet (which resolves to the public internet)
// but false for rules that only allow access to specific private IPs or services.
if matcher.DestsContainsIP(samplePublicIPs...) {
return true
}
}
return false
}
// MapResponseBuilder provides a fluent interface for building tailcfg.MapResponse.
type MapResponseBuilder struct {
resp *tailcfg.MapResponse
@@ -81,6 +119,14 @@ func (b *MapResponseBuilder) WithSelfNode() *MapResponseBuilder {
func(id types.NodeID) []netip.Prefix {
return policy.ReduceRoutes(nv, b.mapper.state.GetNodePrimaryRoutes(id), matchers)
},
func(id types.NodeID) []netip.Prefix {
// For self node, always include its own exit routes
peerNode, ok := b.mapper.state.GetNodeByID(id)
if !ok {
return nil
}
return peerNode.ExitRoutes()
},
b.mapper.cfg)
if err != nil {
b.addError(err)
@@ -256,6 +302,22 @@ func (b *MapResponseBuilder) buildTailPeers(peers views.Slice[types.NodeView]) (
func(id types.NodeID) []netip.Prefix {
return policy.ReduceRoutes(node, b.mapper.state.GetNodePrimaryRoutes(id), matchers)
},
func(id types.NodeID) []netip.Prefix {
// For peer nodes, only include exit routes if the requesting node can use exit nodes
peerNode, ok := b.mapper.state.GetNodeByID(id)
if !ok {
return nil
}
exitRoutes := peerNode.ExitRoutes()
if len(exitRoutes) == 0 {
return nil
}
// Check if the requesting node has permission to use exit nodes
if canUseExitRoutes(node, matchers) {
return exitRoutes
}
return nil
},
b.mapper.cfg)
if err != nil {
return nil, err

336
hscontrol/mapper/exit_node_visibility_test.go Normal file
View File

@@ -0,0 +1,336 @@
package mapper
import (
"net/netip"
"testing"
"github.com/juanfont/headscale/hscontrol/policy"
"github.com/juanfont/headscale/hscontrol/types"
"github.com/stretchr/testify/require"
"tailscale.com/net/tsaddr"
"tailscale.com/tailcfg"
"tailscale.com/types/key"
)
// TestExitNodeVisibilityWithoutAutogroupInternet tests that exit nodes are not visible
// to nodes that don't have autogroup:internet permission in their ACL.
// This is a regression test for https://github.com/juanfont/headscale/issues/2788
func TestExitNodeVisibilityWithoutAutogroupInternet(t *testing.T) {
mustNK := func(str string) key.NodePublic {
var k key.NodePublic
_ = k.UnmarshalText([]byte(str))
return k
}
mustDK := func(str string) key.DiscoPublic {
var k key.DiscoPublic
_ = k.UnmarshalText([]byte(str))
return k
}
mustMK := func(str string) key.MachinePublic {
var k key.MachinePublic
_ = k.UnmarshalText([]byte(str))
return k
}
// Create three nodes: mobile, server, exit
mobile := &types.Node{
ID: 1,
MachineKey: mustMK(
"mkey:f08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422507",
),
NodeKey: mustNK(
"nodekey:9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",
),
DiscoKey: mustDK(
"discokey:cf7b0fd05da556fdc3bab365787b506fd82d64a70745db70e00e86c1b1c03084",
),
IPv4: iap("100.64.0.1"),
Hostname: "mobile",
GivenName: "mobile",
UserID: 1,
User: types.User{
Name: "alice",
},
}
server := &types.Node{
ID: 2,
MachineKey: mustMK(
"mkey:e08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422508",
),
NodeKey: mustNK(
"nodekey:8b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306ff",
),
DiscoKey: mustDK(
"discokey:df7b0fd05da556fdc3bab365787b506fd82d64a70745db70e00e86c1b1c03085",
),
IPv4: iap("100.64.0.2"),
Hostname: "server",
GivenName: "server",
UserID: 1,
User: types.User{
Name: "alice",
},
}
exitNode := &types.Node{
ID: 3,
MachineKey: mustMK(
"mkey:d08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422509",
),
NodeKey: mustNK(
"nodekey:7b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fd",
),
DiscoKey: mustDK(
"discokey:ef7b0fd05da556fdc3bab365787b506fd82d64a70745db70e00e86c1b1c03086",
),
IPv4: iap("100.64.0.3"),
Hostname: "exit",
GivenName: "exit",
UserID: 1,
User: types.User{
Name: "alice",
},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
tsaddr.AllIPv4(),
tsaddr.AllIPv6(),
},
},
// Exit node has approved exit routes
ApprovedRoutes: []netip.Prefix{tsaddr.AllIPv4(), tsaddr.AllIPv6()},
}
// ACL that only allows mobile -> server:80, no autogroup:internet
pol := []byte(`{
"hosts": {
"mobile": "100.64.0.1/32",
"server": "100.64.0.2/32",
"exit": "100.64.0.3/32"
},
"acls": [
{
"action": "accept",
"src": ["mobile"],
"dst": ["server:80"]
}
]
}`)
polMan, err := policy.NewPolicyManager(pol, []types.User{mobile.User}, types.Nodes{mobile, server, exitNode}.ViewSlice())
require.NoError(t, err)
matchers, err := polMan.MatchersForNode(mobile.View())
require.NoError(t, err)
cfg := &types.Config{
BaseDomain: "",
RandomizeClientPort: false,
}
// Build the exit node as a peer from mobile's perspective
exitTailNode, err := tailNode(
exitNode.View(),
0,
polMan,
func(id types.NodeID) []netip.Prefix {
// No primary routes for this test
return nil
},
func(id types.NodeID) []netip.Prefix {
// For peer nodes, only include exit routes if the requesting node can use exit nodes
peerNode := exitNode
if id != peerNode.ID {
return nil
}
exitRoutes := peerNode.ExitRoutes()
if len(exitRoutes) == 0 {
return nil
}
// Check if the requesting node has permission to use exit nodes
if canUseExitRoutes(mobile.View(), matchers) {
return exitRoutes
}
return nil
},
cfg,
)
require.NoError(t, err)
// Verify that exit routes are NOT included in AllowedIPs
// since mobile doesn't have autogroup:internet permission
hasExitRoutes := false
for _, prefix := range exitTailNode.AllowedIPs {
if tsaddr.IsExitRoute(prefix) {
hasExitRoutes = true
break
}
}
if hasExitRoutes {
t.Errorf("Exit node should NOT have exit routes in AllowedIPs when requesting node lacks autogroup:internet permission.\nAllowedIPs: %v", exitTailNode.AllowedIPs)
}
// The AllowedIPs should only contain the exit node's own IP, not the exit routes
// Check the count and that no exit routes are present
if len(exitTailNode.AllowedIPs) != 1 {
t.Errorf("Expected exactly 1 IP in AllowedIPs (node's own IP), got %d: %v", len(exitTailNode.AllowedIPs), exitTailNode.AllowedIPs)
}
// Verify the one IP is the node's own IP
expectedIP := netip.MustParsePrefix("100.64.0.3/32")
found := false
for _, ip := range exitTailNode.AllowedIPs {
if ip == expectedIP {
found = true
break
}
}
if !found {
t.Errorf("Expected to find node's own IP %s in AllowedIPs, got: %v", expectedIP, exitTailNode.AllowedIPs)
}
}
// TestExitNodeVisibilityWithAutogroupInternet tests that exit nodes ARE visible
// to nodes that have autogroup:internet permission in their ACL.
func TestExitNodeVisibilityWithAutogroupInternet(t *testing.T) {
mustNK := func(str string) key.NodePublic {
var k key.NodePublic
_ = k.UnmarshalText([]byte(str))
return k
}
mustDK := func(str string) key.DiscoPublic {
var k key.DiscoPublic
_ = k.UnmarshalText([]byte(str))
return k
}
mustMK := func(str string) key.MachinePublic {
var k key.MachinePublic
_ = k.UnmarshalText([]byte(str))
return k
}
mobile := &types.Node{
ID: 1,
MachineKey: mustMK(
"mkey:f08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422507",
),
NodeKey: mustNK(
"nodekey:9b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fe",
),
DiscoKey: mustDK(
"discokey:cf7b0fd05da556fdc3bab365787b506fd82d64a70745db70e00e86c1b1c03084",
),
IPv4: iap("100.64.0.1"),
Hostname: "mobile",
GivenName: "mobile",
UserID: 1,
User: types.User{
Name: "alice",
},
}
exitNode := &types.Node{
ID: 3,
MachineKey: mustMK(
"mkey:d08305b4ee4250b95a70f3b7504d048d75d899993c624a26d422c67af0422509",
),
NodeKey: mustNK(
"nodekey:7b2ffa7e08cc421a3d2cca9012280f6a236fd0de0b4ce005b30a98ad930306fd",
),
DiscoKey: mustDK(
"discokey:ef7b0fd05da556fdc3bab365787b506fd82d64a70745db70e00e86c1b1c03086",
),
IPv4: iap("100.64.0.3"),
Hostname: "exit",
GivenName: "exit",
UserID: 1,
User: types.User{
Name: "alice",
},
Hostinfo: &tailcfg.Hostinfo{
RoutableIPs: []netip.Prefix{
tsaddr.AllIPv4(),
tsaddr.AllIPv6(),
},
},
ApprovedRoutes: []netip.Prefix{tsaddr.AllIPv4(), tsaddr.AllIPv6()},
}
// ACL that allows mobile to use autogroup:internet
pol := []byte(`{
"hosts": {
"mobile": "100.64.0.1/32",
"exit": "100.64.0.3/32"
},
"acls": [
{
"action": "accept",
"src": ["mobile"],
"dst": ["autogroup:internet:*"]
}
]
}`)
polMan, err := policy.NewPolicyManager(pol, []types.User{mobile.User}, types.Nodes{mobile, exitNode}.ViewSlice())
require.NoError(t, err)
matchers, err := polMan.MatchersForNode(mobile.View())
require.NoError(t, err)
cfg := &types.Config{
BaseDomain: "",
RandomizeClientPort: false,
}
// Build the exit node as a peer from mobile's perspective
exitTailNode, err := tailNode(
exitNode.View(),
0,
polMan,
func(id types.NodeID) []netip.Prefix {
return nil
},
func(id types.NodeID) []netip.Prefix {
peerNode := exitNode
if id != peerNode.ID {
return nil
}
exitRoutes := peerNode.ExitRoutes()
if len(exitRoutes) == 0 {
return nil
}
// Check if the requesting node has permission to use exit nodes - mobile has autogroup:internet permission
if canUseExitRoutes(mobile.View(), matchers) {
return exitRoutes
}
return nil
},
cfg,
)
require.NoError(t, err)
// Verify that exit routes ARE included in AllowedIPs
hasIPv4ExitRoute := false
hasIPv6ExitRoute := false
for _, prefix := range exitTailNode.AllowedIPs {
if prefix == tsaddr.AllIPv4() {
hasIPv4ExitRoute = true
}
if prefix == tsaddr.AllIPv6() {
hasIPv6ExitRoute = true
}
}
if !hasIPv4ExitRoute {
t.Errorf("Exit node should have IPv4 exit route (0.0.0.0/0) in AllowedIPs when requesting node has autogroup:internet permission.\nAllowedIPs: %v", exitTailNode.AllowedIPs)
}
if !hasIPv6ExitRoute {
t.Errorf("Exit node should have IPv6 exit route (::/0) in AllowedIPs when requesting node has autogroup:internet permission.\nAllowedIPs: %v", exitTailNode.AllowedIPs)
}
}

8
hscontrol/mapper/tail.go
View File

@@ -21,6 +21,7 @@ func tailNodes(
capVer tailcfg.CapabilityVersion,
checker NodeCanHaveTagChecker,
primaryRouteFunc routeFilterFunc,
exitRouteFunc routeFilterFunc,
cfg *types.Config,
) ([]*tailcfg.Node, error) {
tNodes := make([]*tailcfg.Node, 0, nodes.Len())
@@ -31,6 +32,7 @@ func tailNodes(
capVer,
checker,
primaryRouteFunc,
exitRouteFunc,
cfg,
)
if err != nil {
@@ -49,6 +51,7 @@ func tailNode(
capVer tailcfg.CapabilityVersion,
checker NodeCanHaveTagChecker,
primaryRouteFunc routeFilterFunc,
exitRouteFunc routeFilterFunc,
cfg *types.Config,
) (*tailcfg.Node, error) {
addrs := node.Prefixes()
@@ -90,7 +93,10 @@ func tailNode(
routes := primaryRouteFunc(node.ID())
allowed := append(addrs, routes...)
allowed = append(allowed, node.ExitRoutes()...)
// Only include exit routes if the exitRouteFunc allows them
exitRoutes := exitRouteFunc(node.ID())
allowed = append(allowed, exitRoutes...)
tsaddr.SortPrefixes(allowed)
tNode := tailcfg.Node{

10
hscontrol/mapper/tail_test.go
View File

@@ -221,6 +221,13 @@ func TestTailNode(t *testing.T) {
func(id types.NodeID) []netip.Prefix {
return primary.PrimaryRoutes(id)
},
func(id types.NodeID) []netip.Prefix {
// For tests, include exit routes if node has them
if id == tt.node.ID {
return tt.node.ExitRoutes()
}
return nil
},
cfg,
)
@@ -281,6 +288,9 @@ func TestNodeExpiry(t *testing.T) {
func(id types.NodeID) []netip.Prefix {
return []netip.Prefix{}
},
func(id types.NodeID) []netip.Prefix {
return []netip.Prefix{}
},
&types.Config{},
)
if err != nil {

7
hscontrol/policy/matcher/matcher.go
View File

@@ -91,3 +91,10 @@ func (m *Match) SrcsOverlapsPrefixes(prefixes ...netip.Prefix) bool {
func (m *Match) DestsOverlapsPrefixes(prefixes ...netip.Prefix) bool {
return slices.ContainsFunc(prefixes, m.dests.OverlapsPrefix)
}
// DestsContainsPrefixes checks if the destination IPSet contains all the given prefixes.
// This is more strict than DestsOverlapsPrefixes - it requires the entire prefix to be
// contained in the destination, not just overlapping.
func (m *Match) DestsContainsPrefixes(prefixes ...netip.Prefix) bool {
return slices.ContainsFunc(prefixes, m.dests.ContainsPrefix)
}