diff --git a/hscontrol/mapper/mapper.go b/hscontrol/mapper/mapper.go index a499800c..9e98e6e1 100644 --- a/hscontrol/mapper/mapper.go +++ b/hscontrol/mapper/mapper.go @@ -428,8 +428,9 @@ func (m *mapper) buildFromChange( } } - if len(resp.PeerPatches) > 0 { - builder.WithPeerChangedPatch(resp.PeerPatches) + patches := m.filterVisiblePeerPatches(nodeID, resp.PeerPatches) + if len(patches) > 0 { + builder.WithPeerChangedPatch(patches) } if resp.PingRequest != nil { @@ -439,6 +440,49 @@ func (m *mapper) buildFromChange( return builder.Build() } +// filterVisiblePeerPatches drops peer-change patches whose target peer the +// recipient cannot see under the ACL policy, mirroring the [policy.ReduceNodes] +// visibility filter [MapResponseBuilder.buildTailPeers] applies to full peer +// objects. 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( + nodeID types.NodeID, + patches []*tailcfg.PeerChange, +) []*tailcfg.PeerChange { + node, ok := m.state.GetNodeByID(nodeID) + if !ok { + return nil + } + + matchers, err := m.state.MatchersForNode(node) + if err != nil { + // Fail closed: if visibility cannot be resolved, send no patches + // rather than risk leaking peers the node may not access. + return nil + } + + // No matchers means no policy restrictions, so every peer is visible — + // the same default buildTailPeers applies. + if len(matchers) == 0 { + return patches + } + + var filtered []*tailcfg.PeerChange + for _, patch := range patches { + peer, ok := m.state.GetNodeByID(types.NodeID(patch.NodeID)) + if !ok { + continue + } + + if node.CanAccess(matchers, peer) || peer.CanAccess(matchers, node) { + filtered = append(filtered, patch) + } + } + + return filtered +} + func writeDebugMapResponse( resp *tailcfg.MapResponse, t debugType, diff --git a/hscontrol/mapper/mapper_test.go b/hscontrol/mapper/mapper_test.go index 54975e84..99eb00ec 100644 --- a/hscontrol/mapper/mapper_test.go +++ b/hscontrol/mapper/mapper_test.go @@ -7,7 +7,12 @@ import ( "github.com/google/go-cmp/cmp" "github.com/google/go-cmp/cmp/cmpopts" + "github.com/juanfont/headscale/hscontrol/db" + "github.com/juanfont/headscale/hscontrol/state" "github.com/juanfont/headscale/hscontrol/types" + "github.com/juanfont/headscale/hscontrol/types/change" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" "tailscale.com/tailcfg" "tailscale.com/types/dnstype" ) @@ -193,3 +198,86 @@ 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 +// existence, presence, and addresses of peers its policy forbids accessing. +func TestBuildFromChangeFiltersPeerPatchesByVisibility(t *testing.T) { + tmp := t.TempDir() + + p4 := netip.MustParsePrefix("100.64.0.0/10") + p6 := netip.MustParsePrefix("fd7a:115c:a1e0::/48") + + cfg := &types.Config{ + Database: types.DatabaseConfig{ + Type: types.DatabaseSqlite, + Sqlite: types.SqliteConfig{Path: tmp + "/h.db"}, + }, + PrefixV4: &p4, + PrefixV6: &p6, + IPAllocation: types.IPAllocationStrategySequential, + BaseDomain: "headscale.test", + Policy: types.PolicyConfig{Mode: types.PolicyModeDB}, + DERP: types.DERPConfig{ + DERPMap: &tailcfg.DERPMap{ + Regions: map[int]*tailcfg.DERPRegion{999: {RegionID: 999}}, + }, + }, + Tuning: types.Tuning{ + NodeStoreBatchSize: state.TestBatchSize, + NodeStoreBatchTimeout: state.TestBatchTimeout, + }, + } + + database, err := db.NewHeadscaleDatabase(cfg) + require.NoError(t, err) + + user1 := database.CreateUserForTest("u1") + user2 := database.CreateUserForTest("u2") + n1 := database.CreateRegisteredNodeForTest(user1, "n1") + n1b := database.CreateRegisteredNodeForTest(user1, "n1b") + n2 := database.CreateRegisteredNodeForTest(user2, "n2") + require.NoError(t, database.Close()) + + s, err := state.NewState(cfg) + require.NoError(t, err) + t.Cleanup(func() { _ = s.Close() }) + + // Each user may reach only its own devices, so n1 cannot access n2. + policy := `{"acls":[ + {"action":"accept","src":["u1@"],"dst":["u1@:*"]}, + {"action":"accept","src":["u2@"],"dst":["u2@:*"]} + ]}` + _, err = s.SetPolicy([]byte(policy)) + require.NoError(t, err) + + m := &mapper{state: s, cfg: cfg} + + // n2 (user2) comes online; n1 (user1) must NOT receive its patch. + leakChange := change.NodeOnline(n2.ID) + resp, err := m.buildFromChange(n1.ID, tailcfg.CurrentCapabilityVersion, &leakChange) + require.NoError(t, err) + require.NotNil(t, resp) + + for _, p := range resp.PeersChangedPatch { + assert.NotEqual(t, n2.ID.NodeID(), p.NodeID, + "n1 must not receive an online patch for n2, which its policy forbids accessing") + } + + // Control: n1b (same user) coming online IS visible to n1. + okChange := change.NodeOnline(n1b.ID) + resp2, err := m.buildFromChange(n1.ID, tailcfg.CurrentCapabilityVersion, &okChange) + require.NoError(t, err) + require.NotNil(t, resp2) + + var gotVisible bool + for _, p := range resp2.PeersChangedPatch { + if p.NodeID == n1b.ID.NodeID() { + gotVisible = true + } + } + assert.True(t, gotVisible, + "n1 must receive the online patch for visible same-user peer n1b") +}