mirror of
https://github.com/juanfont/headscale.git
synced 2026-07-15 04:29:09 +09:00
mapper: filter incremental UserProfiles by ACL visibility
buildFromChange's PeersChanged path passed an unfiltered changed-node slice to WithUserProfiles, while the full-map path uses the BuildPeerMap-filtered ListPeers. A node thus received the identities (login name, display name, avatar) of users owning peers its ACL forbids accessing. Filter the UserProfiles peer set via the same ReduceNodes visibility check buildTailPeers applies.
This commit is contained in:
committed by
Kristoffer Dalby
parent
cd1c208980
commit
8237ac662a
@@ -13,6 +13,7 @@ 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"
|
||||
@@ -419,7 +420,7 @@ func (m *mapper) buildFromChange(
|
||||
} else {
|
||||
if len(resp.PeersChanged) > 0 {
|
||||
peers := m.state.ListPeers(nodeID, resp.PeersChanged...)
|
||||
builder.WithUserProfiles(peers)
|
||||
builder.WithUserProfiles(m.filterVisibleNodes(nodeID, peers))
|
||||
builder.WithPeerChanges(peers)
|
||||
}
|
||||
|
||||
@@ -483,6 +484,37 @@ func (m *mapper) filterVisiblePeerPatches(
|
||||
return filtered
|
||||
}
|
||||
|
||||
// filterVisibleNodes restricts a peer slice to the nodes the recipient can see
|
||||
// under the ACL policy, mirroring the policy.ReduceNodes filter that
|
||||
// buildTailPeers applies to full peer objects. It guards UserProfiles on the
|
||||
// incremental PeersChanged path: unlike the full-map path (whose ListPeers
|
||||
// returns the BuildPeerMap-filtered set), that path 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] {
|
||||
node, ok := m.state.GetNodeByID(nodeID)
|
||||
if !ok {
|
||||
return views.SliceOf([]types.NodeView{})
|
||||
}
|
||||
|
||||
matchers, err := m.state.MatchersForNode(node)
|
||||
if err != nil {
|
||||
// Fail closed: emit no peer user profiles rather than risk a leak.
|
||||
return views.SliceOf([]types.NodeView{})
|
||||
}
|
||||
|
||||
// No matchers means no policy restrictions, so every peer is visible —
|
||||
// the same default buildTailPeers applies.
|
||||
if len(matchers) == 0 {
|
||||
return peers
|
||||
}
|
||||
|
||||
return policy.ReduceNodes(node, peers, matchers)
|
||||
}
|
||||
|
||||
func writeDebugMapResponse(
|
||||
resp *tailcfg.MapResponse,
|
||||
t debugType,
|
||||
|
||||
@@ -281,3 +281,72 @@ func TestBuildFromChangeFiltersPeerPatchesByVisibility(t *testing.T) {
|
||||
assert.True(t, gotVisible,
|
||||
"n1 must receive the online patch for visible same-user peer n1b")
|
||||
}
|
||||
|
||||
// TestBuildFromChangeFiltersUserProfilesByVisibility proves the incremental
|
||||
// PeersChanged path restricts UserProfiles to the recipient's ACL-visible
|
||||
// peers, like the full-map path (whose ListPeers returns the
|
||||
// BuildPeerMap-filtered set). Without it, a changed node broadcast to all
|
||||
// nodes leaks its owner's identity (login name, display name, avatar) to
|
||||
// recipients whose policy forbids accessing that node.
|
||||
func TestBuildFromChangeFiltersUserProfilesByVisibility(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")
|
||||
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) is added and broadcast. n1 (user1) cannot access it, so n1
|
||||
// must NOT receive user2's profile.
|
||||
c := change.NodeAdded(n2.ID)
|
||||
resp, err := m.buildFromChange(n1.ID, tailcfg.CurrentCapabilityVersion, &c)
|
||||
require.NoError(t, err)
|
||||
require.NotNil(t, resp)
|
||||
|
||||
for _, up := range resp.UserProfiles {
|
||||
assert.NotEqual(t, tailcfg.UserID(user2.ID), up.ID,
|
||||
"n1 must not receive user2's profile; n2 is not ACL-visible to n1")
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user