diff --git a/hscontrol/mapper/mapper.go b/hscontrol/mapper/mapper.go index b6cf1a7d..313861bd 100644 --- a/hscontrol/mapper/mapper.go +++ b/hscontrol/mapper/mapper.go @@ -438,9 +438,9 @@ func (m *mapper) buildFromChange( // 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 snapshot peer map ([NodeStore.ListPeers]) is used -// only as the candidate set, matching buildTailPeers; the live policy decides -// visibility because the snapshot is not rebuilt on policy changes. +// 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. diff --git a/hscontrol/mapper/mapper_test.go b/hscontrol/mapper/mapper_test.go index f19f4657..25134002 100644 --- a/hscontrol/mapper/mapper_test.go +++ b/hscontrol/mapper/mapper_test.go @@ -485,12 +485,12 @@ func TestBuildFromChangeVisibilityMatchesFullMap(t *testing.T) { return false } - // wantFull pins the actual peer-visibility semantics so the invariant below - // cannot pass vacuously (e.g. if every path broke to zero identically). - // Note deny_all: an empty ACL set compiles to zero matchers, which headscale - // treats as "no visibility restriction" — all peers are visible on every - // path (the packet filter denies traffic separately). user_isolation and - // autogroup_self are the discriminating cases that prove filtering works. + // wantFull pins the actual peer-visibility semantics so the cross-path + // check below cannot pass vacuously (e.g. if every path broke to zero + // identically). + // Note deny_all: an empty ACL set yields no peer adjacency, so nothing is + // visible on any path. user_isolation and autogroup_self remain the + // discriminating cases that prove filtering works. tests := []struct { name string policy string @@ -505,7 +505,7 @@ func TestBuildFromChangeVisibilityMatchesFullMap(t *testing.T) { ]}`, 1, }, - {"deny_all", `{"acls":[]}`, 2}, + {"deny_all", `{"acls":[]}`, 0}, { "autogroup_self", `{"acls":[{"action":"accept","src":["autogroup:member"],"dst":["autogroup:self:*"]}]}`, diff --git a/hscontrol/state/state.go b/hscontrol/state/state.go index 2c29ffa5..ddb6af25 100644 --- a/hscontrol/state/state.go +++ b/hscontrol/state/state.go @@ -865,14 +865,10 @@ func (s *State) ListPeers(nodeID types.NodeID, peerIDs ...types.NodeID) views.Sl return s.nodeStore.ListPeers(nodeID) } - // For specific peerIDs, filter from all nodes. - // This path is used for incremental updates (NodeAdded, NodeChanged) - // where the caller already knows which peer IDs are involved. - // Peer visibility filtering happens in the mapper against the live - // policy (buildTailPeers and the shared visiblePeerIDs filter), because - // the snapshot peer map is not rebuilt on policy changes. - allNodes := s.nodeStore.ListNodes() - + // Incremental updates (NodeAdded, NodeChanged) name the peers involved. + // Resolve them through the recipient's adjacency so a changed node the + // policy hides from this recipient is never delivered; the mapper still + // applies the live matchers on top. nodeIDSet := make(map[types.NodeID]struct{}, len(peerIDs)) for _, id := range peerIDs { nodeIDSet[id] = struct{}{} @@ -880,20 +876,11 @@ func (s *State) ListPeers(nodeID types.NodeID, peerIDs ...types.NodeID) views.Sl var filteredNodes []types.NodeView - for _, node := range allNodes.All() { - // A node is never its own peer. [db.ListPeers] enforces this with - // `id <> nodeID`; the caller may name the recipient in peerIDs - // (a change batch that includes it), and the mapper's only other - // self filter is [policy.ReduceNodes], which is skipped when the - // node has no matchers. Self would then reach the client in - // [tailcfg.MapResponse.PeersChanged], where it is merged into the - // peer map and listed alongside the self node. - if node.ID() == nodeID { - continue - } - - if _, exists := nodeIDSet[node.ID()]; exists { - filteredNodes = append(filteredNodes, node) + // Adjacency is built from node pairs, so it never contains the + // recipient: a change batch naming it cannot return it as its own peer. + for _, peer := range s.nodeStore.ListPeers(nodeID).All() { + if _, exists := nodeIDSet[peer.ID()]; exists { + filteredNodes = append(filteredNodes, peer) } }