diff --git a/CHANGELOG.md b/CHANGELOG.md index 407d7003..14909e81 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ keys remain all-access. - Improve systemd service file hardening [#3341](https://github.com/juanfont/headscale/pull/3341) - The peer map is keyed by node ID and reused for writes that cannot change peer visibility, so a routine map request no longer rebuilds it. Adds `headscale_nodestore_snapshot_builds_total` [#3417](https://github.com/juanfont/headscale/issues/3417) [#3450](https://github.com/juanfont/headscale/pull/3450) - Headscale now requires Go 1.27 to build +- Fix exit node not offered to viewers whose only matching rule is a `via` grant; peer visibility now comes from the peer map alone [#3409](https://github.com/juanfont/headscale/pull/3409) ## 0.29.4 (unreleased) diff --git a/hscontrol/mapper/builder.go b/hscontrol/mapper/builder.go index eaf4dde4..8a71f3ff 100644 --- a/hscontrol/mapper/builder.go +++ b/hscontrol/mapper/builder.go @@ -249,42 +249,32 @@ func (b *MapResponseBuilder) WithPeerChanges(peers views.Slice[types.NodeView]) return b } -// buildTailPeers converts [views.Slice] of [types.NodeView] to a slice of [tailcfg.Node] -// with policy filtering and sorting. +// buildTailPeers converts [views.Slice] of [types.NodeView] to a sorted slice of +// [tailcfg.Node]. The peers come from the NodeStore peer map, which already +// decided visibility; only each peer's routes are filtered by policy here. func (b *MapResponseBuilder) buildTailPeers(peers views.Slice[types.NodeView]) ([]*tailcfg.Node, error) { node, ok := b.mapper.state.GetNodeByID(b.nodeID) if !ok { return nil, ErrNodeNotFoundMapper } - // Get unreduced matchers for peer relationship determination. - // [State.MatchersForNode] returns unreduced matchers that include all rules where the - // node could be either source or destination. This is different from - // [State.FilterForNode] which returns reduced rules for packet filtering (only rules - // where node is destination). + // [State.RoutesForPeer] needs the unreduced matchers: every rule where the + // node is source or destination, not only the [State.FilterForNode] rules + // where it is destination. matchers, err := b.mapper.state.MatchersForNode(node) if err != nil { return nil, err } - // If there are filter rules present, see if there are any nodes that cannot - // access each-other at all and remove them from the peers. - var changedViews views.Slice[types.NodeView] - if len(matchers) > 0 { - changedViews = policy.ReduceNodes(node, peers, matchers) - } else { - changedViews = peers - } - // Snapshot the per-node policy CapMap once per peer-list build // instead of locking the policy manager per peer. The per-call // path used to take pm.mu N times for an N-peer response. allCapMaps := b.mapper.state.NodeCapMaps() // Build tail nodes with per-peer via-aware route function. - tailPeers := make([]*tailcfg.Node, 0, changedViews.Len()) + tailPeers := make([]*tailcfg.Node, 0, peers.Len()) - for _, peer := range changedViews.All() { + for _, peer := range peers.All() { // Pass the peer's policy CapMap as selfPolicyCaps so per-peer // address-shape rules (today: disable-ipv4) apply consistently // in the viewer's netmap. The CapMap merge into tn.CapMap is diff --git a/hscontrol/mapper/mapper.go b/hscontrol/mapper/mapper.go index 313861bd..f6446424 100644 --- a/hscontrol/mapper/mapper.go +++ b/hscontrol/mapper/mapper.go @@ -14,7 +14,6 @@ import ( "strings" "time" - "github.com/juanfont/headscale/hscontrol/policy" "github.com/juanfont/headscale/hscontrol/state" "github.com/juanfont/headscale/hscontrol/types" "github.com/juanfont/headscale/hscontrol/types/change" @@ -412,7 +411,7 @@ func (m *mapper) buildFromChange( } else { if len(resp.PeersChanged) > 0 { peers := m.state.ListPeers(nodeID, resp.PeersChanged...) - builder.WithUserProfiles(m.filterVisibleNodes(nodeID, peers)) + builder.WithUserProfiles(peers) builder.WithPeerChanges(peers) } @@ -433,48 +432,9 @@ func (m *mapper) buildFromChange( return builder.Build() } -// visiblePeerIDs returns the set of peer node IDs the recipient may see under -// the current policy. It is the single visibility decision shared by the -// incremental peer-change and user-profile paths, computed from the same live -// per-node matchers and [policy.ReduceNodes] filter that -// [MapResponseBuilder.buildTailPeers] applies to full peer objects, so the -// paths cannot drift. The recipient's adjacency from the NodeStore -// ([NodeStore.ListPeers]) is the authority for which peers exist for it; the -// live matchers only narrow that set further, matching buildTailPeers. -// -// ok is false when the node or its matchers cannot be resolved; callers must -// then fail closed (emit nothing) rather than risk leaking forbidden peers. -func (m *mapper) visiblePeerIDs(nodeID types.NodeID) (map[tailcfg.NodeID]struct{}, bool) { - node, ok := m.state.GetNodeByID(nodeID) - if !ok { - return nil, false - } - - matchers, err := m.state.MatchersForNode(node) - if err != nil { - return nil, false - } - - peers := m.state.ListPeers(nodeID) - - // No matchers means no policy restrictions, so every peer is visible — - // the same default buildTailPeers applies. - if len(matchers) > 0 { - peers = policy.ReduceNodes(node, peers, matchers) - } - - // Key by tailcfg.NodeID so the peer-patch path can look up by patch.NodeID - // directly, avoiding an unchecked int64->uint64 conversion. - visible := make(map[tailcfg.NodeID]struct{}, peers.Len()) - for _, peer := range peers.All() { - visible[peer.ID().NodeID()] = struct{}{} - } - - return visible, true -} - -// filterVisiblePeerPatches drops peer-change patches whose target peer the -// recipient cannot see under the ACL policy. Without it, online/offline, +// filterVisiblePeerPatches drops peer-change patches whose target is not in +// the recipient's NodeStore peer map, the same set +// [MapResponseBuilder.buildTailPeers] is fed from. Without it, online/offline, // endpoint, and key-expiry patches disclose the existence, presence, and // addresses of peers the recipient's policy forbids it from accessing. func (m *mapper) filterVisiblePeerPatches( @@ -485,10 +445,13 @@ func (m *mapper) filterVisiblePeerPatches( return patches } - visible, ok := m.visiblePeerIDs(nodeID) - if !ok { - // Fail closed: if visibility cannot be resolved, send no patches. - return nil + // Key by tailcfg.NodeID so patches are looked up by patch.NodeID + // directly, avoiding an unchecked int64->uint64 conversion. + peers := m.state.ListPeers(nodeID) + + visible := make(map[tailcfg.NodeID]struct{}, peers.Len()) + for _, peer := range peers.All() { + visible[peer.ID().NodeID()] = struct{}{} } return filterByVisible(visible, patches, func(p *tailcfg.PeerChange) tailcfg.NodeID { @@ -496,25 +459,6 @@ func (m *mapper) filterVisiblePeerPatches( }) } -// filterVisibleNodes restricts a peer slice to the nodes the recipient can see -// under the ACL policy. It guards UserProfiles on the incremental PeersChanged -// path, which receives an unfiltered node slice and would otherwise leak the -// identities of users whose nodes the recipient cannot access. -func (m *mapper) filterVisibleNodes( - nodeID types.NodeID, - peers views.Slice[types.NodeView], -) views.Slice[types.NodeView] { - visible, ok := m.visiblePeerIDs(nodeID) - if !ok { - // Fail closed: emit no peer user profiles rather than risk a leak. - return views.SliceOf([]types.NodeView{}) - } - - return views.SliceOf(filterByVisible(visible, peers.AsSlice(), func(p types.NodeView) tailcfg.NodeID { - return p.ID().NodeID() - })) -} - // filterByVisible keeps only the items whose key resolves to a NodeID present // in the visible set, preserving input order. func filterByVisible[T any]( diff --git a/hscontrol/mapper/mapper_test.go b/hscontrol/mapper/mapper_test.go index 25134002..6425e59f 100644 --- a/hscontrol/mapper/mapper_test.go +++ b/hscontrol/mapper/mapper_test.go @@ -203,8 +203,8 @@ func TestNextDNSCapMapRendering(t *testing.T) { // TestBuildFromChangeFiltersPeerPatchesByVisibility proves that incremental // peer-change patches (online/offline, endpoint, key-expiry) are restricted to -// the recipient's ACL-visible peer set, the same way buildTailPeers filters -// full peer objects via policy.ReduceNodes. Without it, a node receives the +// the recipient's ACL-visible peer set, the same NodeStore peer map +// buildTailPeers is fed from. Without it, a node receives the // existence, presence, and addresses of peers its policy forbids accessing. func TestBuildFromChangeFiltersPeerPatchesByVisibility(t *testing.T) { tmp := t.TempDir() @@ -361,11 +361,10 @@ func TestBuildFromChangeFiltersUserProfilesByVisibility(t *testing.T) { // full-map path under every policy shape, and a cross-user UserProfile must not // leak. If a future refactor lets one path drift from another, this fails. // -// It pins two behaviours the scattered per-path filters get wrong today and the -// consolidation onto the snapshot peer map must fix: deny-all (empty matchers) -// must hide every peer on the incremental path rather than fall open to "no -// matchers => all visible", and per-node policies (autogroup:self) must agree -// across paths. +// It pins two behaviours of the snapshot peer map every path reads: deny-all +// (empty matchers) must hide every peer on the incremental path rather than +// fall open to "no matchers => all visible", and per-node policies +// (autogroup:self) must agree across paths. func TestBuildFromChangeVisibilityMatchesFullMap(t *testing.T) { tmp := t.TempDir() p4 := netip.MustParsePrefix("100.64.0.0/10") @@ -650,12 +649,11 @@ func TestGenerateDNSConfigNilHostinfoNoPanic(t *testing.T) { }, "generateDNSConfig must not panic when a node has nil Hostinfo") } -// policyShapes covers the paths that decide how the mapper filters peers: a +// policyShapes covers the paths that decide which peers the mapper sends: a // global filter with matchers, a per-node (autogroup:self) filter, a policy // that leaves every node with zero matchers, and no rules at all. The -// zero-matcher shape is the interesting one, because -// [MapResponseBuilder.buildTailPeers] skips [policy.ReduceNodes] there and -// emits its input as given. +// zero-matcher shape is the interesting one: it must hide every peer, not +// fall open to "no matchers => all visible". var policyShapes = []struct { name string policy string diff --git a/hscontrol/policy/policy.go b/hscontrol/policy/policy.go index 94316eb7..260b0e46 100644 --- a/hscontrol/policy/policy.go +++ b/hscontrol/policy/policy.go @@ -8,30 +8,8 @@ import ( "github.com/juanfont/headscale/hscontrol/types" "github.com/juanfont/headscale/hscontrol/util" "github.com/rs/zerolog/log" - "tailscale.com/types/views" ) -// ReduceNodes returns the list of peers authorized to be accessed from a given node. -func ReduceNodes( - node types.NodeView, - nodes views.Slice[types.NodeView], - matchers []matcher.Match, -) views.Slice[types.NodeView] { - var result []types.NodeView - - for _, peer := range nodes.All() { - if peer.ID() == node.ID() { - continue - } - - if node.CanAccess(matchers, peer) || peer.CanAccess(matchers, node) { - result = append(result, peer) - } - } - - return views.SliceOf(result) -} - // ReduceRoutes returns a reduced list of routes for a given node that it can access. func ReduceRoutes( node types.NodeView, diff --git a/hscontrol/policy/policy_test.go b/hscontrol/policy/policy_test.go index 1a490f67..7132c0d2 100644 --- a/hscontrol/policy/policy_test.go +++ b/hscontrol/policy/policy_test.go @@ -24,836 +24,7 @@ var p = func(prefStr string) netip.Prefix { return ip } -func TestReduceNodes(t *testing.T) { - type args struct { - nodes types.Nodes - rules []tailcfg.FilterRule - node *types.Node - } - - tests := []struct { - name string - args args - want types.Nodes - }{ - { - name: "all hosts can talk to each other", - args: args{ - nodes: types.Nodes{ // list of all nodes in the database - &types.Node{ - ID: 1, - IPv4: ap("100.64.0.1"), - User: &types.User{Name: "joe"}, - }, - &types.Node{ - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - &types.Node{ - ID: 3, - IPv4: ap("100.64.0.3"), - User: &types.User{Name: "mickael"}, - }, - }, - rules: []tailcfg.FilterRule{ - { - SrcIPs: []string{"100.64.0.1", "100.64.0.2", "100.64.0.3"}, - DstPorts: []tailcfg.NetPortRange{ - {IP: "*"}, - }, - }, - }, - node: &types.Node{ // current nodes - ID: 1, - IPv4: ap("100.64.0.1"), - User: &types.User{Name: "joe"}, - }, - }, - want: types.Nodes{ - &types.Node{ - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - &types.Node{ - ID: 3, - IPv4: ap("100.64.0.3"), - User: &types.User{Name: "mickael"}, - }, - }, - }, - { - name: "One host can talk to another, but not all hosts", - args: args{ - nodes: types.Nodes{ // list of all nodes in the database - &types.Node{ - ID: 1, - IPv4: ap("100.64.0.1"), - User: &types.User{Name: "joe"}, - }, - &types.Node{ - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - &types.Node{ - ID: 3, - IPv4: ap("100.64.0.3"), - User: &types.User{Name: "mickael"}, - }, - }, - rules: []tailcfg.FilterRule{ // list of all ACLRules registered - { - SrcIPs: []string{"100.64.0.1", "100.64.0.2", "100.64.0.3"}, - DstPorts: []tailcfg.NetPortRange{ - {IP: "100.64.0.2"}, - }, - }, - }, - node: &types.Node{ // current nodes - ID: 1, - IPv4: ap("100.64.0.1"), - User: &types.User{Name: "joe"}, - }, - }, - want: types.Nodes{ - &types.Node{ - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - }, - }, - { - name: "host cannot directly talk to destination, but return path is authorized", - args: args{ - nodes: types.Nodes{ // list of all nodes in the database - &types.Node{ - ID: 1, - IPv4: ap("100.64.0.1"), - User: &types.User{Name: "joe"}, - }, - &types.Node{ - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - &types.Node{ - ID: 3, - IPv4: ap("100.64.0.3"), - User: &types.User{Name: "mickael"}, - }, - }, - rules: []tailcfg.FilterRule{ // list of all ACLRules registered - { - SrcIPs: []string{"100.64.0.3"}, - DstPorts: []tailcfg.NetPortRange{ - {IP: "100.64.0.2"}, - }, - }, - }, - node: &types.Node{ // current nodes - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - }, - want: types.Nodes{ - &types.Node{ - ID: 3, - IPv4: ap("100.64.0.3"), - User: &types.User{Name: "mickael"}, - }, - }, - }, - { - name: "rules allows all hosts to reach one destination", - args: args{ - nodes: types.Nodes{ // list of all nodes in the database - &types.Node{ - ID: 1, - IPv4: ap("100.64.0.1"), - User: &types.User{Name: "joe"}, - }, - &types.Node{ - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - &types.Node{ - ID: 3, - IPv4: ap("100.64.0.3"), - User: &types.User{Name: "mickael"}, - }, - }, - rules: []tailcfg.FilterRule{ // list of all ACLRules registered - { - SrcIPs: []string{"*"}, - DstPorts: []tailcfg.NetPortRange{ - {IP: "100.64.0.2"}, - }, - }, - }, - node: &types.Node{ // current nodes - ID: 1, - IPv4: ap("100.64.0.1"), - User: &types.User{Name: "joe"}, - }, - }, - want: types.Nodes{ - &types.Node{ - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - }, - }, - { - name: "rules allows all hosts to reach one destination, destination can reach all hosts", - args: args{ - nodes: types.Nodes{ // list of all nodes in the database - &types.Node{ - ID: 1, - IPv4: ap("100.64.0.1"), - User: &types.User{Name: "joe"}, - }, - &types.Node{ - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - &types.Node{ - ID: 3, - IPv4: ap("100.64.0.3"), - User: &types.User{Name: "mickael"}, - }, - }, - rules: []tailcfg.FilterRule{ // list of all ACLRules registered - { - SrcIPs: []string{"*"}, - DstPorts: []tailcfg.NetPortRange{ - {IP: "100.64.0.2"}, - }, - }, - }, - node: &types.Node{ // current nodes - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - }, - want: types.Nodes{ - &types.Node{ - ID: 1, - IPv4: ap("100.64.0.1"), - User: &types.User{Name: "joe"}, - }, - &types.Node{ - ID: 3, - IPv4: ap("100.64.0.3"), - User: &types.User{Name: "mickael"}, - }, - }, - }, - { - name: "rule allows all hosts to reach all destinations", - args: args{ - nodes: types.Nodes{ // list of all nodes in the database - &types.Node{ - ID: 1, - IPv4: ap("100.64.0.1"), - User: &types.User{Name: "joe"}, - }, - &types.Node{ - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - &types.Node{ - ID: 3, - IPv4: ap("100.64.0.3"), - User: &types.User{Name: "mickael"}, - }, - }, - rules: []tailcfg.FilterRule{ // list of all ACLRules registered - { - SrcIPs: []string{"*"}, - DstPorts: []tailcfg.NetPortRange{ - {IP: "*"}, - }, - }, - }, - node: &types.Node{ // current nodes - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - }, - want: types.Nodes{ - &types.Node{ - ID: 1, - IPv4: ap("100.64.0.1"), - User: &types.User{Name: "joe"}, - }, - &types.Node{ - ID: 3, - IPv4: ap("100.64.0.3"), - User: &types.User{Name: "mickael"}, - }, - }, - }, - { - name: "without rule all communications are forbidden", - args: args{ - nodes: types.Nodes{ // list of all nodes in the database - &types.Node{ - ID: 1, - IPv4: ap("100.64.0.1"), - User: &types.User{Name: "joe"}, - }, - &types.Node{ - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - &types.Node{ - ID: 3, - IPv4: ap("100.64.0.3"), - User: &types.User{Name: "mickael"}, - }, - }, - rules: []tailcfg.FilterRule{ // list of all ACLRules registered - }, - node: &types.Node{ // current nodes - ID: 2, - IPv4: ap("100.64.0.2"), - User: &types.User{Name: "marc"}, - }, - }, - want: nil, - }, - { - // Investigating 699 - // Found some nodes: [ts-head-8w6paa ts-unstable-lys2ib ts-head-upcrmb ts-unstable-rlwpvr] nodes=ts-head-8w6paa - // ACL rules generated ACL=[{"DstPorts":[{"Bits":null,"IP":"*","Ports":{"First":0,"Last":65535}}],"SrcIPs":["fd7a:115c:a1e0::3","100.64.0.3","fd7a:115c:a1e0::4","100.64.0.4"]}] - // ACL Cache Map={"100.64.0.3":{"*":{}},"100.64.0.4":{"*":{}},"fd7a:115c:a1e0::3":{"*":{}},"fd7a:115c:a1e0::4":{"*":{}}} - name: "issue-699-broken-star", - args: args{ - nodes: types.Nodes{ // - &types.Node{ - ID: 1, - Hostname: "ts-head-upcrmb", - IPv4: ap("100.64.0.3"), - IPv6: ap("fd7a:115c:a1e0::3"), - User: &types.User{Name: "user1"}, - }, - &types.Node{ - ID: 2, - Hostname: "ts-unstable-rlwpvr", - IPv4: ap("100.64.0.4"), - IPv6: ap("fd7a:115c:a1e0::4"), - User: &types.User{Name: "user1"}, - }, - &types.Node{ - ID: 3, - Hostname: "ts-head-8w6paa", - IPv4: ap("100.64.0.1"), - IPv6: ap("fd7a:115c:a1e0::1"), - User: &types.User{Name: "user2"}, - }, - &types.Node{ - ID: 4, - Hostname: "ts-unstable-lys2ib", - IPv4: ap("100.64.0.2"), - IPv6: ap("fd7a:115c:a1e0::2"), - User: &types.User{Name: "user2"}, - }, - }, - rules: []tailcfg.FilterRule{ // list of all ACLRules registered - { - DstPorts: []tailcfg.NetPortRange{ - { - IP: "*", - Ports: tailcfg.PortRange{First: 0, Last: 65535}, - }, - }, - SrcIPs: []string{ - "fd7a:115c:a1e0::3", "100.64.0.3", - "fd7a:115c:a1e0::4", "100.64.0.4", - }, - }, - }, - node: &types.Node{ // current nodes - ID: 3, - Hostname: "ts-head-8w6paa", - IPv4: ap("100.64.0.1"), - IPv6: ap("fd7a:115c:a1e0::1"), - User: &types.User{Name: "user2"}, - }, - }, - want: types.Nodes{ - &types.Node{ - ID: 1, - Hostname: "ts-head-upcrmb", - IPv4: ap("100.64.0.3"), - IPv6: ap("fd7a:115c:a1e0::3"), - User: &types.User{Name: "user1"}, - }, - &types.Node{ - ID: 2, - Hostname: "ts-unstable-rlwpvr", - IPv4: ap("100.64.0.4"), - IPv6: ap("fd7a:115c:a1e0::4"), - User: &types.User{Name: "user1"}, - }, - }, - }, - { - name: "failing-edge-case-during-p3-refactor", - args: args{ - nodes: []*types.Node{ - { - ID: 1, - IPv4: ap("100.64.0.2"), - Hostname: "peer1", - User: &types.User{Name: "mini"}, - }, - { - ID: 2, - IPv4: ap("100.64.0.3"), - Hostname: "peer2", - User: &types.User{Name: "peer2"}, - }, - }, - rules: []tailcfg.FilterRule{ - { - SrcIPs: []string{"100.64.0.1/32"}, - DstPorts: []tailcfg.NetPortRange{ - {IP: "100.64.0.3/32", Ports: tailcfg.PortRangeAny}, - {IP: "::/0", Ports: tailcfg.PortRangeAny}, - }, - }, - }, - node: &types.Node{ - ID: 0, - IPv4: ap("100.64.0.1"), - Hostname: "mini", - User: &types.User{Name: "mini"}, - }, - }, - want: []*types.Node{ - { - ID: 2, - IPv4: ap("100.64.0.3"), - Hostname: "peer2", - User: &types.User{Name: "peer2"}, - }, - }, - }, - { - name: "p4-host-in-netmap-user2-dest-bug", - args: args{ - nodes: []*types.Node{ - { - ID: 1, - IPv4: ap("100.64.0.2"), - Hostname: "user1-2", - User: &types.User{Name: "user1"}, - }, - { - ID: 0, - IPv4: ap("100.64.0.1"), - Hostname: "user1-1", - User: &types.User{Name: "user1"}, - }, - { - ID: 3, - IPv4: ap("100.64.0.4"), - Hostname: "user2-2", - User: &types.User{Name: "user2"}, - }, - }, - rules: []tailcfg.FilterRule{ - { - SrcIPs: []string{ - "100.64.0.3/32", - "100.64.0.4/32", - "fd7a:115c:a1e0::3/128", - "fd7a:115c:a1e0::4/128", - }, - DstPorts: []tailcfg.NetPortRange{ - {IP: "100.64.0.3/32", Ports: tailcfg.PortRangeAny}, - {IP: "100.64.0.4/32", Ports: tailcfg.PortRangeAny}, - {IP: "fd7a:115c:a1e0::3/128", Ports: tailcfg.PortRangeAny}, - {IP: "fd7a:115c:a1e0::4/128", Ports: tailcfg.PortRangeAny}, - }, - }, - { - SrcIPs: []string{ - "100.64.0.1/32", - "100.64.0.2/32", - "fd7a:115c:a1e0::1/128", - "fd7a:115c:a1e0::2/128", - }, - DstPorts: []tailcfg.NetPortRange{ - {IP: "100.64.0.3/32", Ports: tailcfg.PortRangeAny}, - {IP: "100.64.0.4/32", Ports: tailcfg.PortRangeAny}, - {IP: "fd7a:115c:a1e0::3/128", Ports: tailcfg.PortRangeAny}, - {IP: "fd7a:115c:a1e0::4/128", Ports: tailcfg.PortRangeAny}, - }, - }, - }, - node: &types.Node{ - ID: 2, - IPv4: ap("100.64.0.3"), - Hostname: "user-2-1", - User: &types.User{Name: "user2"}, - }, - }, - want: []*types.Node{ - { - ID: 1, - IPv4: ap("100.64.0.2"), - Hostname: "user1-2", - User: &types.User{Name: "user1"}, - }, - { - ID: 0, - IPv4: ap("100.64.0.1"), - Hostname: "user1-1", - User: &types.User{Name: "user1"}, - }, - { - ID: 3, - IPv4: ap("100.64.0.4"), - Hostname: "user2-2", - User: &types.User{Name: "user2"}, - }, - }, - }, - { - name: "p4-host-in-netmap-user1-dest-bug", - args: args{ - nodes: []*types.Node{ - { - ID: 1, - IPv4: ap("100.64.0.2"), - Hostname: "user1-2", - User: &types.User{Name: "user1"}, - }, - { - ID: 2, - IPv4: ap("100.64.0.3"), - Hostname: "user-2-1", - User: &types.User{Name: "user2"}, - }, - { - ID: 3, - IPv4: ap("100.64.0.4"), - Hostname: "user2-2", - User: &types.User{Name: "user2"}, - }, - }, - rules: []tailcfg.FilterRule{ - { - SrcIPs: []string{ - "100.64.0.1/32", - "100.64.0.2/32", - "fd7a:115c:a1e0::1/128", - "fd7a:115c:a1e0::2/128", - }, - DstPorts: []tailcfg.NetPortRange{ - {IP: "100.64.0.1/32", Ports: tailcfg.PortRangeAny}, - {IP: "100.64.0.2/32", Ports: tailcfg.PortRangeAny}, - {IP: "fd7a:115c:a1e0::1/128", Ports: tailcfg.PortRangeAny}, - {IP: "fd7a:115c:a1e0::2/128", Ports: tailcfg.PortRangeAny}, - }, - }, - { - SrcIPs: []string{ - "100.64.0.1/32", - "100.64.0.2/32", - "fd7a:115c:a1e0::1/128", - "fd7a:115c:a1e0::2/128", - }, - DstPorts: []tailcfg.NetPortRange{ - {IP: "100.64.0.3/32", Ports: tailcfg.PortRangeAny}, - {IP: "100.64.0.4/32", Ports: tailcfg.PortRangeAny}, - {IP: "fd7a:115c:a1e0::3/128", Ports: tailcfg.PortRangeAny}, - {IP: "fd7a:115c:a1e0::4/128", Ports: tailcfg.PortRangeAny}, - }, - }, - }, - node: &types.Node{ - ID: 0, - IPv4: ap("100.64.0.1"), - Hostname: "user1-1", - User: &types.User{Name: "user1"}, - }, - }, - want: []*types.Node{ - { - ID: 1, - IPv4: ap("100.64.0.2"), - Hostname: "user1-2", - User: &types.User{Name: "user1"}, - }, - { - ID: 2, - IPv4: ap("100.64.0.3"), - Hostname: "user-2-1", - User: &types.User{Name: "user2"}, - }, - { - ID: 3, - IPv4: ap("100.64.0.4"), - Hostname: "user2-2", - User: &types.User{Name: "user2"}, - }, - }, - }, - { - name: "subnet-router-with-only-route", - args: args{ - nodes: []*types.Node{ - { - ID: 1, - IPv4: ap("100.64.0.1"), - Hostname: "user1", - User: &types.User{Name: "user1"}, - }, - { - ID: 2, - IPv4: ap("100.64.0.2"), - Hostname: "router", - User: &types.User{Name: "router"}, - 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{ - "100.64.0.1/32", - }, - DstPorts: []tailcfg.NetPortRange{ - {IP: "10.33.0.0/16", Ports: tailcfg.PortRangeAny}, - }, - }, - }, - node: &types.Node{ - ID: 1, - IPv4: ap("100.64.0.1"), - Hostname: "user1", - User: &types.User{Name: "user1"}, - }, - }, - want: []*types.Node{ - { - ID: 2, - IPv4: ap("100.64.0.2"), - Hostname: "router", - User: &types.User{Name: "router"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.33.0.0/16")}, - }, - ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.33.0.0/16")}, - }, - }, - }, - { - name: "subnet-router-with-only-route-smaller-mask-2181", - args: args{ - nodes: []*types.Node{ - { - ID: 1, - IPv4: ap("100.64.0.1"), - Hostname: "router", - User: &types.User{Name: "router"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")}, - }, - ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")}, - }, - { - ID: 2, - IPv4: ap("100.64.0.2"), - Hostname: "node", - User: &types.User{Name: "node"}, - }, - }, - rules: []tailcfg.FilterRule{ - { - SrcIPs: []string{ - "100.64.0.2/32", - }, - DstPorts: []tailcfg.NetPortRange{ - {IP: "10.99.0.2/32", Ports: tailcfg.PortRangeAny}, - }, - }, - }, - node: &types.Node{ - ID: 1, - IPv4: ap("100.64.0.1"), - Hostname: "router", - User: &types.User{Name: "router"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")}, - }, - ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")}, - }, - }, - want: []*types.Node{ - { - ID: 2, - IPv4: ap("100.64.0.2"), - Hostname: "node", - User: &types.User{Name: "node"}, - }, - }, - }, - { - name: "node-to-subnet-router-with-only-route-smaller-mask-2181", - args: args{ - nodes: []*types.Node{ - { - ID: 1, - IPv4: ap("100.64.0.1"), - Hostname: "router", - User: &types.User{Name: "router"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")}, - }, - ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")}, - }, - { - ID: 2, - IPv4: ap("100.64.0.2"), - Hostname: "node", - User: &types.User{Name: "node"}, - }, - }, - rules: []tailcfg.FilterRule{ - { - SrcIPs: []string{ - "100.64.0.2/32", - }, - DstPorts: []tailcfg.NetPortRange{ - {IP: "10.99.0.2/32", Ports: tailcfg.PortRangeAny}, - }, - }, - }, - node: &types.Node{ - ID: 2, - IPv4: ap("100.64.0.2"), - Hostname: "node", - User: &types.User{Name: "node"}, - }, - }, - want: []*types.Node{ - { - ID: 1, - IPv4: ap("100.64.0.1"), - Hostname: "router", - User: &types.User{Name: "router"}, - Hostinfo: &tailcfg.Hostinfo{ - RoutableIPs: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")}, - }, - ApprovedRoutes: []netip.Prefix{netip.MustParsePrefix("10.99.0.0/16")}, - }, - }, - }, - // 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 { - t.Run(tt.name, func(t *testing.T) { - matchers := matcher.MatchesFromFilterRules(tt.args.rules) - gotViews := ReduceNodes( - tt.args.node.View(), - tt.args.nodes.ViewSlice(), - matchers, - ) - // Convert views back to nodes for comparison in tests - var got types.Nodes - for _, v := range gotViews.All() { - got = append(got, v.AsStruct()) - } - - if diff := cmp.Diff(tt.want, got, util.Comparers...); diff != "" { - t.Errorf("ReduceNodes() unexpected result (-want +got):\n%s", diff) - t.Log("Matchers: ") - - for _, m := range matchers { - t.Log("\t+", m.DebugString()) - } - } - }) - } -} - -func TestReduceNodesFromPolicy(t *testing.T) { +func TestBuildPeerMapFromPolicy(t *testing.T) { n := func(id types.NodeID, ip, hostname, username string, routess ...string) *types.Node { routes := make([]netip.Prefix, 0, len(routess)) for _, route := range routess { @@ -1107,19 +278,13 @@ func TestReduceNodesFromPolicy(t *testing.T) { require.NoError(t, err) assert.Len(t, matchers, tt.wantMatchers) - gotViews := ReduceNodes( - tt.node.View(), - tt.nodes.ViewSlice(), - matchers, - ) - // Convert views back to nodes for comparison in tests - var got types.Nodes - for _, v := range gotViews.All() { - got = append(got, v.AsStruct()) + var want []types.NodeID + for _, n := range tt.want { + want = append(want, n.ID) } - if diff := cmp.Diff(tt.want, got, util.Comparers...); diff != "" { - t.Errorf("TestReduceNodesFromPolicy() unexpected result (-want +got):\n%s", diff) + got := pm.BuildPeerMap(tt.nodes.ViewSlice())[tt.node.ID] + if !assert.ElementsMatch(t, want, got) { t.Log("Matchers: ") for _, m := range matchers { diff --git a/hscontrol/policy/v2/tailscale_routes_data_compat_test.go b/hscontrol/policy/v2/tailscale_routes_data_compat_test.go index cb68aed4..4538e65c 100644 --- a/hscontrol/policy/v2/tailscale_routes_data_compat_test.go +++ b/hscontrol/policy/v2/tailscale_routes_data_compat_test.go @@ -13,8 +13,8 @@ // - TestRoutesCompat: validates filter rule compilation (compileFilterRulesForNode // + ReduceFilterRules) against golden file captures. // -// - TestRoutesCompatPeerVisibility: validates peer visibility (CanAccess / -// ReduceNodes) for the subnet-to-subnet scenarios (f10–f15). These tests +// - TestRoutesCompatPeerVisibility: validates peer visibility (CanAccess) +// for the subnet-to-subnet scenarios (f10–f15). These tests // derive expected peer relationships from the golden file captures: if // Tailscale SaaS delivers filter rules to a node, then the subnet routers // referenced in those rules must be visible as peers. This exercises the diff --git a/hscontrol/servertest/issues_test.go b/hscontrol/servertest/issues_test.go index 4b06f080..329466f5 100644 --- a/hscontrol/servertest/issues_test.go +++ b/hscontrol/servertest/issues_test.go @@ -12,6 +12,7 @@ import ( "github.com/juanfont/headscale/hscontrol/types/change" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + "tailscale.com/net/tsaddr" "tailscale.com/tailcfg" "tailscale.com/types/netmap" ) @@ -300,6 +301,100 @@ func TestIssuesRoutes(t *testing.T) { assert.Contains(t, announced, route, "server should store the advertised route as announced") }) + + // A grant steering autogroup:internet via a tagged exit node must + // offer the exit node to viewers whose only matching rule is the via + // grant. The via rule is compiled onto the via-tagged node, so only + // the exit node's matchers authorise the pair. + t.Run("via_exit_node_offered_to_members", func(t *testing.T) { + t.Parallel() + + srv := servertest.NewServer(t) + adminUser := srv.CreateUser(t, "via3408-admin") + memberUser := srv.CreateUser(t, "via3408-member") + + changed, err := srv.State().SetPolicy([]byte(`{ + "tagOwners": {"tag:exit": ["via3408-admin@"]}, + "autoApprovers": {"exitNode": ["tag:exit"]}, + "groups": {"group:admins": ["via3408-admin@"]}, + "grants": [ + { + "src": ["group:admins"], + "dst": ["*"], + "ip": ["*"] + }, + { + "src": ["autogroup:member"], + "dst": ["autogroup:internet"], + "via": ["tag:exit"], + "ip": ["*"] + } + ] + }`)) + require.NoError(t, err) + + if changed { + changes, err := srv.State().ReloadPolicy() + require.NoError(t, err) + srv.App.Change(changes...) + } + + adminNode := servertest.NewClient(t, srv, "via3408-admin-node", + servertest.WithUser(adminUser)) + memberNode := servertest.NewClient(t, srv, "via3408-member-node", + servertest.WithUser(memberUser)) + exitNode := servertest.NewClient(t, srv, "via3408-exit", + servertest.WithUser(adminUser), + servertest.WithTags("tag:exit")) + + exitNode.Direct().SetHostinfo(&tailcfg.Hostinfo{ + BackendLogID: "servertest-via3408-exit", + Hostname: "via3408-exit", + RoutableIPs: tsaddr.ExitRoutes(), + }) + + ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) + defer cancel() + + require.NoError(t, exitNode.Direct().SendUpdate(ctx)) + + exitID := findNodeID(t, srv, "via3408-exit") + _, routeChange, err := srv.State().SetApprovedRoutes( + exitID, tsaddr.ExitRoutes()) + require.NoError(t, err) + srv.App.Change(routeChange) + + seesExitNode := func(nm *netmap.NetworkMap) bool { + for _, p := range nm.Peers { + hi := p.Hostinfo() + if !hi.Valid() || hi.Hostname() != "via3408-exit" { + continue + } + + var v4, v6 bool + + for i := range p.AllowedIPs().Len() { + switch p.AllowedIPs().At(i) { + case netip.MustParsePrefix("0.0.0.0/0"): + v4 = true + case netip.MustParsePrefix("::/0"): + v6 = true + } + } + + return v4 && v6 + } + + return false + } + + memberNode.WaitForCondition(t, + "member sees the tag:exit exit node with exit routes in AllowedIPs", + 15*time.Second, seesExitNode) + adminNode.WaitForCondition(t, + "admin sees the tag:exit exit node with exit routes in AllowedIPs", + 15*time.Second, seesExitNode) + }) } // TestIssuesIPAllocation tests IP address allocation correctness. diff --git a/hscontrol/servertest/via_compat_test.go b/hscontrol/servertest/via_compat_test.go index 6cbe54fd..ab3fb3e0 100644 --- a/hscontrol/servertest/via_compat_test.go +++ b/hscontrol/servertest/via_compat_test.go @@ -36,6 +36,7 @@ var viaCompatTests = []struct { {"via-grant-v33", "single via grant + HA primary election"}, {"via-grant-v35", "via grant with unadvertised destination"}, {"via-grant-v36", "full complex: peer connectivity + crossed subnet + crossed exit"}, + {"via-grant-v52", "members reach the internet only via tag:exit, admins reach everything"}, } // TestViaGrantMapCompat loads golden captures from Tailscale SaaS and diff --git a/hscontrol/types/node_test.go b/hscontrol/types/node_test.go index c723e5dc..07af8427 100644 --- a/hscontrol/types/node_test.go +++ b/hscontrol/types/node_test.go @@ -154,7 +154,7 @@ func Test_NodeCanAccess(t *testing.T) { { // With a unidirectional ACL (src=A→dst=B), the dst // router cannot access the src router. Bidirectional - // peer visibility comes from [policy.ReduceNodes] checking + // peer visibility comes from BuildPeerMap checking // both A.CanAccess(B) || B.CanAccess(A). name: "subnet-to-subnet-unidirectional-dst-cannot-access-src-3157", node1: Node{