fix(server): place rooms on adopted members, so a node cannot mint an id into a replica set (C25) - #369
Open
vieiralucas wants to merge 17 commits into
Open
fix(server): place rooms on adopted members, so a node cannot mint an id into a replica set (C25)#369vieiralucas wants to merge 17 commits into
vieiralucas wants to merge 17 commits into
Conversation
vieiralucas
force-pushed
the
server/c25-node-id-placement
branch
from
July 29, 2026 00:38
ec7f12a to
c385026
Compare
… id into a replica set (C25) C13 gated replication, leadership and durability on membership in `replicas_for(room)`, and placement is HRW over the member set — a pure, publicly computable function — so a member could grind a node id that placed it on the room it wanted, introduce itself over gossip's join path, and reach all three gates from inside the replica set. Learning a member and placing rooms on it are now two admissions. The roster is what a node dials, probes and gossips about; the ring is built from the adopted members alone, and a gossip-learned member is pending until the cluster adopts it. Adoption cannot be a local predicate — placement must be identical on every node — so the evidence is disseminated instead: a node records only that it completed an identity-checked peer link to a member, that claim rides gossip attributed to the member the receiving link is bound to, and two already-adopted verifiers place a member. Verifier sets are grow-only, so adoption is a convergent fixpoint over them.
…his node's own dial verifies Cold review broke the first cut four ways, each of them the premise underneath a rule rather than the rule itself. - Sticky adoption made the ring a function of history: a member adopted before its vouchers were reaped stayed placed on the node that saw that order and was never placed on one that did not. The adopted set is now derived from the configured members plus the evidence, to a fixpoint, on every change. - Counting distinct verifier node ids was the mint one level up: a certificate names a host and a host mints unlimited ids, so one machine holding two of them raised the whole bar. The bar counts distinct verifier hosts and excludes the candidate's own. - An inbound link is no verification, however well its certificate names the member: the member chooses when to dial in, so the vouch is one it caused, and it could dial in under each ground id in turn. - A member's recorded advertise address was first-write-wins over a field any peer may set, and the ring turned on it. A member is dialed at its own node id; the advertised address is dropped. The bar is also a constant now rather than clamped to the adopted count, with one exception keyed on configuration: a node configured with no peers has no cluster to be outvoted by. On the reply half of a round a claim counts only where the dial established the sender. The residual is pinned as a passing test and filed as C27: a member that owns a host owns every id under it, and honest nodes verify one truthfully — closing that needs the ring to weigh trust units rather than node ids.
…address rules The mutation sweep found three assertions that held for a reason other than the rule they name: two same-host vouchers were already one unit by the host dedup, two pending vouchers were already excluded by sharing the candidate's host, and the address rule was only exercised on the additive path.
…l one, and the bootstrap bar is fixed at construction Cold review pass 2 broke the trust-unit count two ways. - The unit was the host as raw text while the certificate binding compares it semantically — lowercased, root label dropped, IP literals parsed as addresses. So `evil.example` beside `evil.example.`, or an IPv6 literal beside its expanded form, were one certificate and two vouchers: one machine held two units and could adopt any id anywhere, which is not the bounded residual C27 records. The unit is now reduced by that same relation. - The bootstrap exception read `configured.len() <= 1` off a set that shrinks, so a node given one seed peer fell to the single-node rule the moment that seed was reaped and began placing on one vouch while every peer still held the constant. It is decided once, at construction. Also drops `add_member`'s ignored address argument, so no caller can believe the advertised address is honored, and pins the fixpoint, the returned member's ring status and the host-aliasing relation, three rules whose tests held for another reason. Files C28 for two gossip-plane inputs review found unbounded: an unrefutable incarnation that evicts a member for good, and an unbounded roster that starves adoption.
Cold review pass 3 found two bypasses with one root: an address had many spellings and nothing reduced them. - The dial URL was built from the authority verbatim while the host was read as everything before the first ':'. So 'wss://a.example:1@b.example:9000' read as host 'a.example' and connected to 'b.example': a member certified for 'a.example' alone could grind such an id onto any room, bind its own certificate to it, and let every honest node verify it by dialing the honest node it embedded. A full bypass under mTLS, and not the bounded own-host residual C27 records. - Node ids were never canonicalized, so 'x:9000', 'ws://x:9000', 'WS://x:9000' and 'x:9000/anything' were four members that one honest endpoint answers for. Every peer that dialed any of them verified it truthfully, all were adopted, and none of them ever speaks — so a room whose replica set filled with them waited on acks that never came, targeted per room because HRW is public. An advertise address is now an authority and nothing else — userinfo, a path, a query and a fragment are refused — reduced to one canonical spelling, and canonicalized again at every door an id arrives through: config, a gossip tuple, and a link's claim. Also refuses a present-but-empty CRDTSYNC_CLUSTER_PEERS, which produced a clustered node running the single-node bar of one, and pins the rule the second pass introduced but never tested: an adopted member leaves the ring when a voucher is reaped, because the set is derived and not accumulated. Files C29 for two shapes in which the bar is unsatisfiable — a cluster of two trust units can never re-admit a member it reaped, and a cluster whose members share a host can never admit one.
…pells Cold review pass 4, after pass 3 had approved the address story. All three are in how an id is derived from *configuration* rather than in the evidence machinery. - The bootstrap exception read `configured.len() <= 1` — a set, after canonicalization and de-duplication — so a peer list whose every entry spelled this node's own address collapsed to one member and dropped the bar to a single vouch. That is the state the empty-list refusal had just been added to catch, reached without the variable being empty, and a templated peer list on a one-replica deployment reaches it. The bar now reads the list for a peer other than self, and such a list is refused. - `CRDTSYNC_NODE_ID` was the one door that did not canonicalize, a regression from the plain trim it replaced. A node written in a non-canonical spelling kept it while its peers derived the canonical one, so it carried a doppelganger of itself: adopted on two honest vouchers, placed on rooms it never answers for, unable to refute a suspicion of itself, and dropping every follower-head report because the link was bound to the other spelling. - The canonical form reduced the host but not the port, so `:9000`, `:09000` and `:009000` were three node ids on one listener — one certificate, one trust unit, unbounded ring positions. That is the targeted write-stall the previous pass was meant to have closed, and an operator reaches it with no attacker by writing a leading zero. An address with no canonical form is now refused where it is written rather than joined under, a frame on a link claiming this node's own id makes it vouch for nobody, and an authority may carry only the characters a host and a port are written with — so a malformed address is a permanent failure rather than one redialed forever. Two tests held for the wrong reason and are replaced: the two-spellings vouch test could not fail (config had already canonicalized them), and the one-endpoint-one-id test had no port spellings, which is what hid the port.
…ip that skipped the config door C25 refuses an address with no canonical form where it is written, so a config-built membership can no longer carry one as far as the serve path. The startup refusal is still the right gate for a membership assembled programmatically, so the test asserts both: the config door refuses it first, and the dialer's validation refuses it for a view that did not pass through that door.
Cold review pass 5, after pass 4 had approved the address story. `normalized` stripped one trailing dot, so `a.example..` reduced to `a.example.` — a *different* node id, which DNS resolves to the same host and which rustls accepts against the same certificate. A compromised member had only to answer one gossip round with that spelling: the victim learned a second id for an *honest* member, dialed it, verified it truthfully, and adopted it on the attacker's single vouch plus its own. Measured on a five-member view: 47 of 90 sample rooms took the ghost into their replica set and 13 believed it their leader, none of which the honest node ever speaks as — so those rooms' majority-ack never releases. Not the own-host residual: the minted id is on a host the attacker does not own, and it works under the strongest posture. Every trailing dot now folds, a host with an empty label is refused, and a bracketed host must be followed by its port and nothing else — a *wrong* canonical form is worse than none, because it is accepted. A certificate name carrying a run of root labels binds nothing, refused as malformed rather than folded, which keeps C13's narrow binding and the fixpoint at once. An underscore is allowed: container runtimes hand those names out and they carry no second reading. Two rules the sweep proved untested are now pinned — that a node advertises only the verifications it made itself, and that the peer-list door refuses an undialable address — plus the fixpoint itself and the ghost-id shape end to end. The wildcard SAN is recorded on C27: the inbound binding refuses one, the outbound dial is plain TLS and honours it, so it is an unbounded trust-unit generator.
…rs the same bar as its subject Cold review pass 6. `PeerEndpoint::parse` required a bracketed host to be followed by a port and nothing else, but never that the brackets *contain* an IPv6 literal. `host_of` then returned everything between them with its colons intact and the canonical form emitted that verbatim with the port appended, so `[10.0.0.1:9000]:9443` became the node id `10.0.0.1:9000:9443`. That id is not refused, it is accepted: it parses, so startup validation and the peer-identity member loop both pass, and written in configuration it is adopted from birth and placed on every room HRW gives it — while the URL the dialer builds from it is rejected as an invalid authority at send time, which reads as unreachable rather than as a permanent address error and is redialed forever. Those rooms reach no quorum. Written as `CRDTSYNC_NODE_ID` it has no canonical form at all, so peers drop it from gossip and refuse its `PeerAuth`, and the node joins nothing while believing it has. An operator typo reaches both. The root cause is that pass 5's fixpoint property was asserted over a list of spellings rather than established by construction, and the bracketed family was not on the list. The list now carries it, and brackets carry an IPv6 literal. Separately, `merge_liveness` gated the node a claim was *about* but stored it under a `sender` it never gated — the hazard `note_verified`'s own contract states, applied to only one side. Peer admission takes the id a link claims, so one certified host opens a link per port and banks a permanent verifier entry per link against every member, keyed on ids no reap ever strikes because reaping strikes members. They can never count, only grow. Both sides of a claim now clear one predicate. Also: `gossip::exchange` let an inbound push introduce a third member, which production's `apply_gossip` refuses, so the convergence tests were measuring a push the cluster rejects. Tests: the `can_be_verified` gate had no test (the one mutation that survived pass 6's sweep) and now has one; a tautological `matches!(_, None | Some(_))` conjunct is replaced by the bracketed spellings it should have covered. C30 (the canonical id is finer-grained than what the resolver dials) and C31 (a member adopted after startup has no replication link) filed.
Round 2 had A dial B and expected B to learn C from the push. An inbound frame introduces only its own sender, so that is a join the cluster has no path for — it converged here only because the in-process `exchange` helper was more permissive than `apply_gossip`. B dials A and learns C from the reply, which is the direction that actually carries a newcomer.
A direction sweep over every comparison this unit adds found two that no test pinned. One was real: nothing failed when `gossip::exchange`'s inbound filter was replaced by `true`, so the rule that an inbound frame introduces only its own sender rested on the helper's implementation and not on a test. A newcomer admitted from a push is a join with no dial behind it — the unchecked join the adoption rules exist to close, one level down at the roster. The other was dead code rather than an untested rule: the claim guard also tested `!is_self(node)` and `!is_self(sender)`, both of which `can_be_verified` already decides where the claim is recorded, so dropping them changed nothing and no test could have failed. Both ends are now decided in one place.
…lves of a round ask one question Cold review pass 7. `canonical_member_addr` canonicalized the port as a number but never as a default, while the dial resolves an absent port from the scheme. So `wss://h` and `wss://h:443` were two node ids for one socket — and this one needs no malformed input: advertising a member portlessly is supported. One admitted member injects a single gossip tuple naming the other spelling of an *honest* member's address. Every node then dials it on the ordinary cadence, reaches that member's real listener, is answered by its real certificate, and records a truthful vouch; two hosts vouch and the phantom is adopted. It never speaks — the honest node's `PeerAuth` claims its configured spelling, so links bind to the other id, `is_self` is false for the phantom so that node cannot refute a suspicion of it, and its probe always succeeds because it is that node's own listener. Measured over 1000 rooms: 764 replicas held, effective primary of 245 that no node believes it leads, so those rooms accept no writes. Separately, the reply half computed `dial_establishes_identity(sender)` while the inbound half passed `true`. `verifiers` has to converge and this is what decides what enters it, so a plaintext member under `require_peer_identity` had its claims kept by every node it dialed and dropped by every node that dialed it: two nodes with identical evidence placed 197 of 256 rooms differently. Also: the gossip `verified` byte decoded leniently, so two byte strings encoded one message. Pass 7's third finding — a certified host self-introduces once per port per scheme, so C25's roster bound on claims does not bound that path — is C28's class and is folded into it rather than fixed here.
…vidence actually converges to Cold review pass 8. `dial_establishes_identity` decides what enters `verifiers`, and `verifiers` is what the ring is derived from, so it has to be the same answer everywhere. It read `advertises_tls`, a roster lookup, which answers `false` for a member not yet learned — so the frame that *introduces* a sender was judged differently from every later one, and the claims it carried were kept by nodes that already knew it and dropped by nodes meeting it. One delivery, two states. A node id is an advertise address, so the transport is a property of the id and needs no state to read. The module docs claimed the evidence "converges however gossip interleaves". It does not. Liveness is relayed and anti-entropies; a verification claim is first-hand and is never passed on, deliberately, because with no signature on the wire "A and B verified X" is free for any node to write and relaying it would hand a compromised member a mint rather than a split. The consequence is that a compromised adopted member is a swing vote for any candidate exactly one other trust unit has reached: it sets the flag toward some peers and clears it toward others, and the groups place rooms differently — measured at 721/1000 rooms, 149 of them with two nodes each believing itself primary, surviving 30 further anti-entropy sweeps. The docs now state what holds, and the residual is pinned as a passing test and filed as C39. Board: C15 (#368/#373) filed its own C27-C33 first, so this branch's units are renumbered C34-C38 and the stale duplicate C26 line is dropped.
vieiralucas
force-pushed
the
server/c25-node-id-placement
branch
from
July 29, 2026 04:33
8efc160 to
c425670
Compare
…t dropped Cold review pass 9. Two seams asked "is this node on the roster?" before retaining a verification claim, and both dropped rather than deferred. The frame that vouches for a member and the frame that introduces it are two frames, and which lands first is ordinary network ordering — so the identical three frames delivered as [joiner, s1, s2] adopted the joiner and as [s1, s2, joiner] left it pending, placing 959 of 2000 sample rooms differently. No compromised member and no selective speech: `adopted` is derived from `verifiers`, so making retention a function of arrival order made the ring one too, a level further down. It healed as soon as a voucher gossiped again, so it was a window and not a split. The part worth fixing is that the documentation asserted the opposite — that within one node the order claims arrive in cannot change the result. A claim is now held for a member the view has not met, and counts for nothing until that member exists, because the fixpoint only ever scans the roster. The tombstone rule survives: a reaped member refuses a held claim exactly as it refuses a carried one, so it cannot return pre-adopted. Pass 9's other findings stay on the board: the 64 MiB default frame cap and the roster x senders cross-product are C35's class, with measured costs added there; `require_peer_identity` is a cluster-wide protocol parameter that must be uniform, recorded in DECISIONS rather than filed. No HIGH found by pass 9.
…t is re-sent Cold review pass 10, on my own pass-9 fix. Pass 9 measured a real window: a claim naming a member the receiver had not met was dropped with its tuple, so retention — and the ring, since `adopted` is derived from `verifiers` — depended on frame arrival order (959/2000 rooms). Holding the claim closed that and opened something worse. Held ids are on no roster. `reap_dead` strikes `verifiers` only for ids it reaps off the roster, and `rebuild_placement` only ever scans the roster, so a held claim is state nothing reclaims and nothing can see. One inbound 24.9 MB frame retained 376.5 MB — 15x the wire — and stalled the single-threaded registry actor for 591 ms; at tungstenite's default 64 MiB cap that is 3.3 million claims, about 1 GB, from one frame. It also made the tombstone refusal temporal, because `reaped` is pruned after TOMBSTONE_RETENTION_TICKS: past about five minutes a reaped id was neither on the roster nor tombstoned, so claims banked again and a returning member was adopted pre-verified (980/2000 rooms). The comment above the sibling site still stated the invariant the code had stopped holding. The window was the cheaper defect, and it is not even a loss: a verifier re-advertises what it verified every round, so a claim that outruns its subject is re-sent within one gossip interval and the view converges then. That healing property is now pinned; the order-independence it replaced was never true and is no longer claimed. The regression test I wrote for the held-claim version was vacuous. It drove `merge_liveness`, whose `None` branch admits the unmet member from the same tuple, so the claim was retained either way — it passes on the commit it was written to catch. The replacement drives the seam that actually drops.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
C13 (#365) bound a peer link to a member and gated replication, leadership and durability on it. All three gates ask the same question — is the sender in
replicas_for(room)— and placement is HRW over the member set, a pure and publicly computable function, so which rooms a node replicates follows from its node id. Gossip's join path let an unknown node introduce itself straight into the set the ring is built from.The defect
Acceptedfor a write two nodes held.The split
Membership::membersis the roster — dialed, probed, gossiped about, advertised back;adopted_membersis the ring. Every gate that readsreplicas_forinherited the change without a line of its own.Adoption is the cluster's decision, and it is derived
Message::Gossipas one flag per tuple, recorded against the member the receiving link is bound to and against nobody the payload names.configured+verifiersto a fixpoint on every change, never accumulated. Accumulating makes the ring a function of history: a member adopted before its vouchers were reaped stays placed on the node that saw that order and is never placed on one that did not.ADOPTION_VERIFIERS = 2already-adopted trust units have verified it.A trust unit is a machine, not a node id
evil.examplebesideevil.example., or an IPv6 literal beside its expanded form — and two units adopt any id anywhere.What a link means
CRDTSYNC_CLUSTER_REQUIRE_PEER_IDENTITY=1a plaintext member's vouches are dropped. That reads the member's advertised transport — sound outbound, where the dial runs over exactly that transport, and exactly what C13's pass 3 rejected inbound.The bar
Residuals, each pinned as a passing test or filed
u64::MAXincarnation evicts a member for good, and an unbounded roster from one peer starves adoption. Both pre-existing; both worse now.Verification
git archivecopy of the committed tree,--no-fail-fast, baseline 0. 26 mutations over every rule in pass 1 and 7 re-measured on the current head: highest are the ring built from the roster 7, the roster reduced to the ring 15/17, configured members starting pending 11, a claim attributed to whoever the payload names 11, every member advertised verified 10. The bar itself is refused by the compiler (const _: () = assert!(ADOPTION_VERIFIERS > 1)). Four survivors were audited: three were tests that named a rule they did not isolate — each now does, and each mutant is killed on re-measure — and one is an equivalent mutant (the bar is hoisted out of the fixpoint, where the running set equals the configured one).Suites
crates/server/tests/adoption.rs(32);gossip.rsconvergence tests drive rounds until the ring converges as well as the roster;reaping.rssays what a returned member's ring status is; C13's residual test is removed frompeer_identity.rs. Wire:Message::Gossip's tuple gains a trailing flag byte (MemberAdvert).See DECISIONS + ARCHITECTURE §Member Adoption (2026-07-28).