diff options
Diffstat (limited to 'packages')
| -rw-r--r-- | packages/meshbay-hub/src/meshbay_hub/static/transport.js | 21 | ||||
| -rw-r--r-- | packages/meshbay-hub/tests/test_spa_ordering.py | 87 |
2 files changed, 100 insertions, 8 deletions
diff --git a/packages/meshbay-hub/src/meshbay_hub/static/transport.js b/packages/meshbay-hub/src/meshbay_hub/static/transport.js index c200674..5ead80e 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/transport.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/transport.js @@ -174,6 +174,18 @@ class MeshBayTransport { throw new Error('Node requires GEK proof but no crypto available'); } + // Recorded the moment the challenge arrives, because everything below may + // need them — joining, in particular, happens before the proof and signs a + // transcript over both. Reading them further down, next to the proof that + // also uses them, meant join_request ran with neither. + // + // nonce_node ties a join to this connection, so one cannot be lifted onto + // another. node_pk is announced here because a first-time member has no + // GEK and so cannot complete the handshake that would prove it; it is + // unverified at this point and checked against the ack below. + this._nonceNode = window.MeshBayCrypto.b64decode(reply.nonce); + this.nodePk = reply.node_pk || null; + // Recover session keys from node if not available locally (P2P keypair bundle) if (!this._sessionKeys && this._bundleKey && window.MeshBayKeys) { const kpResp = await this._sendAndWait({ @@ -258,14 +270,7 @@ class MeshBayTransport { _extractDtlsFingerprint(this._pc.localDescription.sdp), _extractDtlsFingerprint(this._rawAnswerSdp), ); - const nonceNode = C.b64decode(reply.nonce); - // Kept for the life of the connection: a join_request is signed over it, - // which is what stops one being lifted onto another connection. - this._nonceNode = nonceNode; - // Announced in the challenge because joining needs it before the ack: a - // first-time member has no GEK, so they cannot complete the handshake that - // would prove this key. Unverified here; checked against the ack below. - this.nodePk = reply.node_pk || null; + const nonceNode = this._nonceNode; // captured when the challenge arrived const gid = groupId || ''; const proof = await C.handshakeProof( diff --git a/packages/meshbay-hub/tests/test_spa_ordering.py b/packages/meshbay-hub/tests/test_spa_ordering.py new file mode 100644 index 0000000..839698a --- /dev/null +++ b/packages/meshbay-hub/tests/test_spa_ordering.py @@ -0,0 +1,87 @@ +""" +Ordering guards for the SPA's connect() flow. + +These are source-level checks, which is not how one would normally test +behaviour. They exist because a specific class of bug shipped to a live browser +twice and no other test could see it: `connect()` is a long sequence in which +later steps read values earlier steps set, and the Python end-to-end client in +QE/deploy/ cannot catch a mistake there — it is a different implementation, +written in the right order by construction, so it passes while the browser fails. + +Concretely: join_request signs a transcript over the node key and the node nonce, +and runs *before* the GEK proof, because a first-time member has no GEK to prove. +Both values were being read further down, next to the proof that also uses them, +so every invited member hit "Handshake incomplete — reconnect and retry". + +If you restructure connect(), these will fail. Check the invariant still holds — +that nothing reads a value assigned later — and then move the markers. +""" + +from pathlib import Path + +import pytest + +STATIC = (Path(__file__).resolve().parents[1] + / "src" / "meshbay_hub" / "static") +TRANSPORT = STATIC / "transport.js" + +pytestmark = pytest.mark.skipif( + not TRANSPORT.exists(), reason="SPA sources not present") + + +def _positions(*needles: str) -> list[int]: + source = TRANSPORT.read_text() + out = [] + for needle in needles: + idx = source.find(needle) + assert idx != -1, f"{needle!r} is gone from transport.js — update this test" + out.append(idx) + return out + + +def test_challenge_values_are_captured_before_joining(): + """ + joinGroup() signs over node_pk and nonce_node, so both must be recorded when + the challenge arrives — not later, beside the proof. + """ + # Deliberately loose markers: what matters is where the assignment happens, + # not how it is spelled, so a reordering fails on the ordering assertion + # below rather than on a missing string. + node_pk, nonce_node, join_call = _positions( + "this.nodePk = reply.node_pk", + "this._nonceNode = ", + "await this.joinGroup(", + ) + assert node_pk < join_call, ( + "node_pk is read from the challenge after joinGroup() runs — the join " + "would sign a transcript naming nothing") + assert nonce_node < join_call, ( + "nonce_node is captured after joinGroup() runs — the join would not be " + "bound to this connection") + + +def test_join_happens_before_the_gek_proof(): + """ + The whole point of joining in the pre-proof window: someone who has never + held the group key cannot produce a proof, so the key has to arrive first. + """ + join_call, proof = _positions( + "await this.joinGroup(", + "await C.handshakeProof(", + ) + assert join_call < proof, ( + "the join must happen before the GEK proof — a first-time member has no " + "key to prove with") + + +def test_the_ack_still_verifies_the_announced_node_key(): + """ + Taking node_pk from the challenge is only safe because the ack proves it and + the client compares the two. Losing that check would leave the announcement + trusted on its own. + """ + source = TRANSPORT.read_text() + assert "Node identity changed during the handshake" in source, ( + "the challenge's node_pk must be checked against the ack's") + assert "verifyNodeSignature" in source, ( + "the ack's signature over the handshake transcript must still be verified") |