mirror of
https://github.com/juanfont/headscale.git
synced 2026-11-04 12:39:25 +09:00
policy: offer exit routes only to viewers whose rules reach the internet
Every destination overlaps /0, so any rule offered a peer's exit routes. They now need a rule to the internet, the test exit-node visibility uses; via steering defers them to a regular grant only on that test.
This commit is contained in:
1 parent
701138104c
commit
a9ad49f9a6
9 files changed
+115
-50
No files matched your search
@@ -7,7 +7,6 @@ import (
|
||||
|
||||
"github.com/juanfont/headscale/hscontrol/util"
|
||||
"go4.org/netipx"
|
||||
"tailscale.com/net/tsaddr"
|
||||
"tailscale.com/tailcfg"
|
||||
)
|
||||
|
||||
@@ -154,17 +153,7 @@ func overlaps(ranges []netipx.IPRange, r netipx.IPRange) bool {
|
||||
|
||||
// DestsIsTheInternet reports whether the destination covers "the
|
||||
// internet" — the set represented by autogroup:internet, special-cased
|
||||
// for exit nodes. Returns true if either family's /0 is contained
|
||||
// (0.0.0.0/0 or ::/0), or if dests is a superset of [util.TheInternet]. A
|
||||
// single-family /0 counts because operators may write it directly and
|
||||
// it still denotes the whole internet for that family.
|
||||
// for exit nodes. See [util.CoversTheInternet].
|
||||
func (m *Match) DestsIsTheInternet() bool {
|
||||
if m.dests.ContainsPrefix(tsaddr.AllIPv4()) ||
|
||||
m.dests.ContainsPrefix(tsaddr.AllIPv6()) {
|
||||
return true
|
||||
}
|
||||
|
||||
// Superset-of-[util.TheInternet] check handles merged filter rules
|
||||
// where the internet prefixes are combined with other dests.
|
||||
return util.IPSetSubsetOf(util.TheInternet(), m.dests)
|
||||
return util.CoversTheInternet(m.dests)
|
||||
}
|
||||
@@ -171,6 +171,16 @@ func TestViaInternetExitSteeringSurvivesUnrelatedRules(t *testing.T) {
|
||||
extra: `"acls": [{"action": "accept", "src": ["autogroup:member"], "dst": ["*:*"]}],`,
|
||||
wantExcluded: false,
|
||||
},
|
||||
{
|
||||
name: "acl-ipv4-default",
|
||||
extra: `"acls": [{"action": "accept", "src": ["autogroup:member"], "dst": ["0.0.0.0/0:*"]}],`,
|
||||
wantExcluded: false,
|
||||
},
|
||||
{
|
||||
name: "acl-ipv6-default",
|
||||
extra: `"acls": [{"action": "accept", "src": ["autogroup:member"], "dst": ["::/0:*"]}],`,
|
||||
wantExcluded: false,
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
|
||||
@@ -16,6 +16,7 @@ import (
|
||||
"github.com/juanfont/headscale/hscontrol/policy/matcher"
|
||||
"github.com/juanfont/headscale/hscontrol/policy/policyutil"
|
||||
"github.com/juanfont/headscale/hscontrol/types"
|
||||
"github.com/juanfont/headscale/hscontrol/util"
|
||||
"github.com/puzpuzpuz/xsync/v4"
|
||||
"github.com/rs/zerolog/log"
|
||||
"go4.org/netipx"
|
||||
@@ -1734,14 +1735,19 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
|
||||
continue
|
||||
}
|
||||
|
||||
reachesInternet := grantReachesInternet(grant, resolvedDstPrefixes[i])
|
||||
|
||||
// A non-via grant covering routes that a via grant included
|
||||
// defers to global HA primary election. Match by overlap so
|
||||
// a broader or narrower regular dst still catches the
|
||||
// routes the via grant added to Include.
|
||||
for _, dstPrefix := range resolvedDstPrefixes[i] {
|
||||
for _, p := range result.Include {
|
||||
if dstPrefix.Overlaps(p) &&
|
||||
!slices.Contains(result.UsePrimary, p) {
|
||||
// Only a grant to the internet covers an exit route,
|
||||
// as [types.Node.CanAccessRoutes] decides.
|
||||
covers := dstPrefix.Overlaps(p) &&
|
||||
(reachesInternet || !tsaddr.IsExitRoute(p))
|
||||
if covers && !slices.Contains(result.UsePrimary, p) {
|
||||
result.UsePrimary = append(result.UsePrimary, p)
|
||||
}
|
||||
}
|
||||
@@ -1755,7 +1761,7 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
|
||||
|
||||
// A regular grant to the internet lets the viewer use any
|
||||
// exit node, not just the via-tagged ones.
|
||||
if grantReachesInternet(grant) {
|
||||
if reachesInternet {
|
||||
result.Exclude = slices.DeleteFunc(result.Exclude, tsaddr.IsExitRoute)
|
||||
}
|
||||
}
|
||||
@@ -1764,11 +1770,13 @@ func (pm *PolicyManager) ViaRoutesForPeer(viewer, peer types.NodeView) types.Via
|
||||
return result
|
||||
}
|
||||
|
||||
// grantReachesInternet reports whether a grant's destinations include
|
||||
// the internet. Neither the wildcard nor autogroup:internet resolves
|
||||
// to 0.0.0.0/0, so check the aliases themselves.
|
||||
func grantReachesInternet(grant Grant) bool {
|
||||
return slices.ContainsFunc(grant.Destinations, func(d Alias) bool {
|
||||
// grantReachesInternet reports whether a grant's destinations, resolved
|
||||
// to dsts, reach the internet by the test the grant's filter rule gets
|
||||
// ([matcher.Match.DestsIsTheInternet]). Neither the wildcard nor
|
||||
// autogroup:internet resolves to the internet here, so check those
|
||||
// aliases themselves.
|
||||
func grantReachesInternet(grant Grant, dsts []netip.Prefix) bool {
|
||||
internetAlias := slices.ContainsFunc(grant.Destinations, func(d Alias) bool {
|
||||
if ag, ok := d.(*AutoGroup); ok {
|
||||
return ag.Is(AutoGroupInternet)
|
||||
}
|
||||
@@ -1777,6 +1785,19 @@ func grantReachesInternet(grant Grant) bool {
|
||||
|
||||
return ok
|
||||
})
|
||||
if internetAlias {
|
||||
return true
|
||||
}
|
||||
|
||||
var b netipx.IPSetBuilder
|
||||
for _, p := range dsts {
|
||||
b.AddPrefix(p)
|
||||
}
|
||||
|
||||
// Resolved prefixes are valid, so the builder cannot fail.
|
||||
set, _ := b.IPSet()
|
||||
|
||||
return util.CoversTheInternet(set)
|
||||
}
|
||||
|
||||
func (pm *PolicyManager) Version() int {
|
||||
|
||||
@@ -183,9 +183,6 @@ var routesSkipReasons = map[string]string{
|
||||
"routes-m4-member-self/m-odin2/filter": filterAddressForm,
|
||||
"routes-m4-member-self/r-exit/filter": "FILTER_ADDRESS_FORM: same addresses, different wire form and rule order",
|
||||
|
||||
"routes-m1-wildcard-src-multi-range/srv/peer-allowed-ips": exitRoutesOnOverlap,
|
||||
"routes-m6-subnet-to-subnet-multi-route/r-multi/peer-allowed-ips": exitRoutesOnOverlap,
|
||||
|
||||
"routes-m1-wildcard-src-multi-range/m-odin1/peers": sameUserPeers,
|
||||
"routes-m1-wildcard-src-multi-range/m-odin2/peers": sameUserPeers,
|
||||
"routes-m2-member-partial-subnet/m-odin1/peers": sameUserPeers,
|
||||
@@ -272,8 +269,6 @@ func notCompared(prefixes []string, key string) bool {
|
||||
|
||||
const filterAddressForm = "FILTER_ADDRESS_FORM: same addresses, different wire form"
|
||||
|
||||
const exitRoutesOnOverlap = "EXIT_ROUTES_ON_OVERLAP: headscale offers a peer's exit routes to a viewer whose rules reach only nodes or subnets; SaaS leaves them out"
|
||||
|
||||
const sameUserPeers = "SAME_USER_PEERS: SaaS lists a user's own devices as each other's peers with no rule linking them; headscale peers only what the policy links"
|
||||
|
||||
// routesSetByAPI reports whether the capture's routes were approved through
|
||||
|
||||
@@ -107,9 +107,6 @@ var routesMapCompatTests = []struct {
|
||||
var viaCompatSkipReasons = map[string]string{
|
||||
"tagged-self": "TAGGED_SELF_PROFILE: SaaS gives a tagged node a profile of its own, headscale the shared tagged-devices one",
|
||||
|
||||
"routes-m1-wildcard-src-multi-range/srv/peer-allowed-ips": exitRoutesOnOverlap,
|
||||
"routes-m6-subnet-to-subnet-multi-route/r-multi/peer-allowed-ips": exitRoutesOnOverlap,
|
||||
|
||||
"routes-m1-wildcard-src-multi-range/m-odin1/peers": sameUserPeers,
|
||||
"routes-m1-wildcard-src-multi-range/m-odin2/peers": sameUserPeers,
|
||||
"routes-m2-member-partial-subnet/m-odin1/peers": sameUserPeers,
|
||||
@@ -120,8 +117,6 @@ var viaCompatSkipReasons = map[string]string{
|
||||
"routes-m6-subnet-to-subnet-multi-route/m-odin2/peers": sameUserPeers,
|
||||
}
|
||||
|
||||
const exitRoutesOnOverlap = "EXIT_ROUTES_ON_OVERLAP: headscale offers a peer's exit routes to a viewer whose rules reach only nodes or subnets; SaaS leaves them out"
|
||||
|
||||
const sameUserPeers = "SAME_USER_PEERS: SaaS lists a user's own devices as each other's peers with no rule linking them; headscale peers only what the policy links"
|
||||
|
||||
// TestRoutesMapCompat replays routesMapCompatTests through
|
||||
|
||||
@@ -1500,9 +1500,8 @@ func (s *State) GetNodePrimaryRoutes(nodeID types.NodeID) []netip.Prefix {
|
||||
// AllowedIPs, combining primary routes (from HA election), approved exit
|
||||
// routes, and via grant steering.
|
||||
//
|
||||
// Approved exit routes (0.0.0.0/0, ::/0) are included alongside subnet
|
||||
// routes — they appear in every peer's AllowedIPs, while unapproved
|
||||
// ones do not.
|
||||
// Approved exit routes (0.0.0.0/0, ::/0) are reduced alongside subnet
|
||||
// routes: a viewer gets them when its rules reach the internet.
|
||||
func (s *State) RoutesForPeer(
|
||||
viewer, peer types.NodeView,
|
||||
matchers []matcher.Match,
|
||||
|
||||
+18
-16
@@ -338,9 +338,8 @@ func (node *Node) Prefixes() []netip.Prefix {
|
||||
}
|
||||
|
||||
// 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 [Node.CanAccessRoute] which only
|
||||
// handles non-exit routing.
|
||||
// and/or ::/0). [Node.CanAccessRoutes] offers them only to a viewer whose
|
||||
// rules reach the internet.
|
||||
func (node *Node) ExitRoutes() []netip.Prefix {
|
||||
return node.approvedRoutes(true)
|
||||
}
|
||||
@@ -464,19 +463,10 @@ func (node *Node) CanAccessRoute(matchers []matcher.Match, route netip.Prefix) b
|
||||
// CanAccessRoutes returns the authorized routes in input order, evaluating the
|
||||
// node's matcher membership once for the whole list.
|
||||
//
|
||||
// Unlike [Node.CanAccess], this function intentionally does NOT check
|
||||
// [matcher.Match.DestsIsTheInternet]. Exit routes (0.0.0.0/0, ::/0) are handled by
|
||||
// RoutesForPeer (state.go) which adds them unconditionally from
|
||||
// [Node.ExitRoutes], not through ACL-based route filtering. The
|
||||
// [matcher.Match.DestsIsTheInternet] check in [Node.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 [matcher.Match.DestsOverlapsPrefixes](0.0.0.0/0) already returns
|
||||
// true, so the check would be redundant for that case.
|
||||
// Exit routes (0.0.0.0/0, ::/0) are kept only for a rule from the node to the
|
||||
// internet ([matcher.Match.DestsIsTheInternet]), the test [Node.CanAccess]
|
||||
// applies to exit nodes: every destination overlaps /0, so overlap would offer
|
||||
// them for any rule.
|
||||
func (node *Node) CanAccessRoutes(
|
||||
matchers []matcher.Match,
|
||||
routes []netip.Prefix,
|
||||
@@ -507,7 +497,19 @@ func (node *Node) CanAccessRoutes(
|
||||
var result []netip.Prefix
|
||||
|
||||
for _, route := range routes {
|
||||
exit := tsaddr.IsExitRoute(route)
|
||||
|
||||
for i, m := range matchers {
|
||||
if exit {
|
||||
if sides[i].isSrc && m.DestsIsTheInternet() {
|
||||
result = append(result, route)
|
||||
|
||||
break
|
||||
}
|
||||
|
||||
continue
|
||||
}
|
||||
|
||||
if (sides[i].isSrc && m.DestsOverlapsPrefixes(route)) ||
|
||||
(sides[i].isDst && m.SrcsOverlapsPrefixes(route)) {
|
||||
result = append(result, route)
|
||||
|
||||
@@ -12,8 +12,10 @@ import (
|
||||
"github.com/google/go-cmp/cmp/cmpopts"
|
||||
"github.com/juanfont/headscale/hscontrol/policy/matcher"
|
||||
"github.com/juanfont/headscale/hscontrol/util"
|
||||
"github.com/stretchr/testify/assert"
|
||||
"github.com/stretchr/testify/require"
|
||||
"pgregory.net/rapid"
|
||||
"tailscale.com/net/tsaddr"
|
||||
"tailscale.com/tailcfg"
|
||||
"tailscale.com/types/key"
|
||||
)
|
||||
@@ -1224,6 +1226,37 @@ func TestIsExitNodeSubnetRouterAllocs(t *testing.T) {
|
||||
require.Zero(t, allocs)
|
||||
}
|
||||
|
||||
// TestCanAccessRoutesExitNeedsInternet checks that a viewer is offered a
|
||||
// peer's exit routes only when one of its rules reaches the internet, not for
|
||||
// any rule whose destination overlaps /0.
|
||||
func TestCanAccessRoutesExitNeedsInternet(t *testing.T) {
|
||||
viewer := Node{IPv4: new(netip.MustParseAddr("100.64.0.1"))}
|
||||
routes := []netip.Prefix{tsaddr.AllIPv4(), tsaddr.AllIPv6(), netip.MustParsePrefix("10.40.7.0/24")}
|
||||
rule := func(src string, dsts ...string) []matcher.Match {
|
||||
r := tailcfg.FilterRule{SrcIPs: []string{src}}
|
||||
for _, dst := range dsts {
|
||||
r.DstPorts = append(r.DstPorts, tailcfg.NetPortRange{IP: dst, Ports: tailcfg.PortRangeAny})
|
||||
}
|
||||
|
||||
return matcher.MatchesFromFilterRules([]tailcfg.FilterRule{r})
|
||||
}
|
||||
|
||||
internet := util.PrefixesToString(util.TheInternet().Prefixes())
|
||||
|
||||
for name, tt := range map[string]struct {
|
||||
matchers []matcher.Match
|
||||
want []netip.Prefix
|
||||
}{
|
||||
"to a subnet": {rule("100.64.0.1", "10.40.7.0/24"), routes[2:]},
|
||||
"from anywhere": {rule("*", "100.64.0.1"), routes[2:]},
|
||||
"to everything": {rule("100.64.0.1", "*"), routes},
|
||||
"to the internet": {rule("100.64.0.1", internet...), routes[:2]},
|
||||
"to another viewer": {rule("100.64.0.1", "100.64.0.2"), nil},
|
||||
} {
|
||||
assert.Equalf(t, tt.want, viewer.CanAccessRoutes(tt.matchers, routes), "%s", name)
|
||||
}
|
||||
}
|
||||
|
||||
func TestCanAccessRoutesMatchesReference(t *testing.T) {
|
||||
prefixStrings := []string{
|
||||
"100.64.0.1/32", "100.64.0.2/32", "100.64.0.0/24",
|
||||
@@ -1313,6 +1346,18 @@ func canAccessRouteReference(
|
||||
src := node.IPs()
|
||||
subnetRoutes := node.SubnetRoutes()
|
||||
|
||||
if tsaddr.IsExitRoute(route) {
|
||||
for _, matcher := range matchers {
|
||||
fromNode := matcher.SrcsContainsIPs(src...) ||
|
||||
(len(subnetRoutes) > 0 && matcher.SrcsOverlapsPrefixes(subnetRoutes...))
|
||||
if fromNode && matcher.DestsIsTheInternet() {
|
||||
return true
|
||||
}
|
||||
}
|
||||
|
||||
return false
|
||||
}
|
||||
|
||||
for _, matcher := range matchers {
|
||||
if matcher.SrcsContainsIPs(src...) && matcher.DestsOverlapsPrefixes(route) {
|
||||
return true
|
||||
|
||||
@@ -63,6 +63,15 @@ var TheInternet = sync.OnceValue(func() *netipx.IPSet {
|
||||
return theInternetSet
|
||||
})
|
||||
|
||||
// CoversTheInternet reports whether set covers [TheInternet]. Either
|
||||
// family's /0 counts alone, because operators write it directly and it
|
||||
// still denotes the whole internet for that family.
|
||||
func CoversTheInternet(set *netipx.IPSet) bool {
|
||||
return set.ContainsPrefix(tsaddr.AllIPv4()) ||
|
||||
set.ContainsPrefix(tsaddr.AllIPv6()) ||
|
||||
IPSetSubsetOf(TheInternet(), set)
|
||||
}
|
||||
|
||||
// IPSetSubsetOf reports whether every prefix of candidate is contained in
|
||||
// container. A nil candidate or container returns false.
|
||||
func IPSetSubsetOf(candidate, container *netipx.IPSet) bool {
|
||||
|
||||
Reference in new issue
Block a user