mirror of
https://github.com/juanfont/headscale.git
synced 2026-09-17 22:12:03 +09:00
mapper: filter peer-change patches by ACL visibility
buildFromChange added PeersChangedPatch (online/offline, endpoint, key-expiry) directly, skipping the policy.ReduceNodes visibility filter that buildTailPeers applies to full peers. A node thus received the existence, presence, and addresses of peers its ACL forbids accessing. Restrict patches to the recipient's visible peer set.
This commit is contained in:
committed by
Kristoffer Dalby
parent
0fdff0c79b
commit
cd1c208980
@@ -428,8 +428,9 @@ func (m *mapper) buildFromChange(
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if len(resp.PeerPatches) > 0 {
|
patches := m.filterVisiblePeerPatches(nodeID, resp.PeerPatches)
|
||||||
builder.WithPeerChangedPatch(resp.PeerPatches)
|
if len(patches) > 0 {
|
||||||
|
builder.WithPeerChangedPatch(patches)
|
||||||
}
|
}
|
||||||
|
|
||||||
if resp.PingRequest != nil {
|
if resp.PingRequest != nil {
|
||||||
@@ -439,6 +440,49 @@ func (m *mapper) buildFromChange(
|
|||||||
return builder.Build()
|
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(
|
func writeDebugMapResponse(
|
||||||
resp *tailcfg.MapResponse,
|
resp *tailcfg.MapResponse,
|
||||||
t debugType,
|
t debugType,
|
||||||
|
|||||||
@@ -7,7 +7,12 @@ import (
|
|||||||
|
|
||||||
"github.com/google/go-cmp/cmp"
|
"github.com/google/go-cmp/cmp"
|
||||||
"github.com/google/go-cmp/cmp/cmpopts"
|
"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"
|
||||||
|
"github.com/juanfont/headscale/hscontrol/types/change"
|
||||||
|
"github.com/stretchr/testify/assert"
|
||||||
|
"github.com/stretchr/testify/require"
|
||||||
"tailscale.com/tailcfg"
|
"tailscale.com/tailcfg"
|
||||||
"tailscale.com/types/dnstype"
|
"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")
|
||||||
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user