You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
dictyon has two different identities for the same live peer state. Full PeersChanged records update or append peers by public key, while PeersChangedPatch and numeric removals address peers by server-assigned NodeID.
That split breaks key rotation: a full changed record carrying the same NodeID and a new key is treated as a new peer, leaving the old-key record in the netmap. The patch path also collapses an explicit empty endpoint list into “unchanged,” so the server cannot revoke every previously advertised direct endpoint.
Verified against main9aa9a56223afdd47d7723104cd16a5c07d5fc722.
Evidence
crates/dictyon/src/control/netmap.rs:115-149 handles PeersChanged by searching self.peers for p.key == changed_peer.key; a changed key with the same node ID therefore falls into the append path.
netmap.rs:151-160 applies removals and PeersChangedPatch separately; apply_peer_change at :173-176 finds the target by peer.id == change.node_id.
crates/mitos/src/types/mod.rs:345-429 models Node.id as the server-assigned numeric identity and Node.key as a rotatable public key. PeerChange at :431-479 is explicitly keyed by NodeID and can carry a new Key.
netmap.rs:186-195 applies endpoints only after change.endpoints.filter(|endpoints| !endpoints.is_empty()). None and Some(vec![]) therefore both mean “do nothing,” even though the wire shape can distinguish omitted from explicitly empty.
The netmap property test at crates/dictyon/src/control/tests/netmap_tests.rs:522-629 asserts only that peer keys are unique. It does not assert uniqueness by node ID or generate a same-ID/key-rotation update.
The reference implementation at tailscale/tailscale@33af330c8ed8a1cb02530eaf70d9179f9656db2c, control/controlclient/map.go, stores peers in a map keyed by NodeID; both full changes and patches update that same entry. It applies Endpoints whenever the patch field is non-nil, including an empty list that clears prior endpoints.
NodeID, node key, endpoints, and patch/removal routing feed the eventual WireGuard peer configuration. A key rotation delivered as a full peer change can leave two records for one peer: one stale key and one current key. Different consumers may then select different records, retain obsolete routes, or configure duplicate peers.
Ignoring an explicit empty endpoint list preserves stale direct-connect addresses after the control plane has revoked them. The client can continue dialing an address the server deliberately removed, rather than falling back to current discovery or DERP state.
This is already-diverged ownership, not cosmetic DRY: one update path treats the key as identity, while the other treats the node ID as identity.
Desired correction
Give Netmap one authoritative peer index keyed by NodeID. Full peer lists, PeersChanged, patches, and removals must all mutate that owner. Public-key changes should update the existing peer record rather than create another peer.
Preserve tri-state patch semantics for collection fields: omitted means unchanged, present-empty means clear, and present-non-empty means validate and replace. Apply the same rule to other nullable patch collections as they become modeled, while leaving #67’s capability-policy semantics with that issue.
Done when:
one live peer record exists per non-zero NodeID;
a full PeersChanged entry with an existing ID and rotated key replaces that peer without increasing peer count;
key- and ID-based compatibility inputs resolve through the same canonical record;
Endpoints: [] clears previously stored endpoints, while an omitted field preserves them;
full-list ingestion rejects or deterministically resolves duplicate IDs; and
property/integration tests assert unique node IDs, key rotation, endpoint revocation, and subsequent removal of the rotated peer.
Finding
dictyonhas two different identities for the same live peer state. FullPeersChangedrecords update or append peers by public key, whilePeersChangedPatchand numeric removals address peers by server-assignedNodeID.That split breaks key rotation: a full changed record carrying the same
NodeIDand a new key is treated as a new peer, leaving the old-key record in the netmap. The patch path also collapses an explicit empty endpoint list into “unchanged,” so the server cannot revoke every previously advertised direct endpoint.Verified against
main9aa9a56223afdd47d7723104cd16a5c07d5fc722.Evidence
crates/dictyon/src/control/netmap.rs:115-149handlesPeersChangedby searchingself.peersforp.key == changed_peer.key; a changed key with the same node ID therefore falls into the append path.netmap.rs:151-160applies removals andPeersChangedPatchseparately;apply_peer_changeat:173-176finds the target bypeer.id == change.node_id.crates/mitos/src/types/mod.rs:345-429modelsNode.idas the server-assigned numeric identity andNode.keyas a rotatable public key.PeerChangeat:431-479is explicitly keyed byNodeIDand can carry a newKey.netmap.rs:186-195applies endpoints only afterchange.endpoints.filter(|endpoints| !endpoints.is_empty()).NoneandSome(vec![])therefore both mean “do nothing,” even though the wire shape can distinguish omitted from explicitly empty.crates/dictyon/src/control/tests/netmap_tests.rs:522-629asserts only that peer keys are unique. It does not assert uniqueness by node ID or generate a same-ID/key-rotation update.tailscale/tailscale@33af330c8ed8a1cb02530eaf70d9179f9656db2c,control/controlclient/map.go, stores peers in a map keyed byNodeID; both full changes and patches update that same entry. It appliesEndpointswhenever the patch field is non-nil, including an empty list that clears prior endpoints.CapMapand concerns the lower-level peer identity and endpoint-state contract.Why this matters
NodeID, node key, endpoints, and patch/removal routing feed the eventual WireGuard peer configuration. A key rotation delivered as a full peer change can leave two records for one peer: one stale key and one current key. Different consumers may then select different records, retain obsolete routes, or configure duplicate peers.Ignoring an explicit empty endpoint list preserves stale direct-connect addresses after the control plane has revoked them. The client can continue dialing an address the server deliberately removed, rather than falling back to current discovery or DERP state.
This is already-diverged ownership, not cosmetic DRY: one update path treats the key as identity, while the other treats the node ID as identity.
Desired correction
Give
Netmapone authoritative peer index keyed byNodeID. Full peer lists,PeersChanged, patches, and removals must all mutate that owner. Public-key changes should update the existing peer record rather than create another peer.Preserve tri-state patch semantics for collection fields: omitted means unchanged, present-empty means clear, and present-non-empty means validate and replace. Apply the same rule to other nullable patch collections as they become modeled, while leaving #67’s capability-policy semantics with that issue.
Done when:
NodeID;PeersChangedentry with an existing ID and rotated key replaces that peer without increasing peer count;Endpoints: []clears previously stored endpoints, while an omitted field preserves them;