From e9081c9f2f091d1c067046c2c881eee0a427f976 Mon Sep 17 00:00:00 2001 From: Mark Xue Date: Mon, 27 Jul 2026 12:41:59 -0700 Subject: [PATCH] Gate the A.5 Upd' door on content type, refuse rekey while a bind is owed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Upd' door feeds process_incoming_message on our send-PQ, which applies a commit atomically — so a peer-authored commit smuggled behind the tag would advance our epoch before the kind check refused it, and the refusal wore the fatal Mls disposition. Same gap the A.4 leg doors already closed (#111): read the content type off the plaintext framing first, refuse anything but a proposal with the retriable DecryptionFailed, nothing applied. The door also refuses a rekey while a classical bind is owed — its Commit' would move the epoch the owed bind reserved, failing the discharge with the PQ leaf spent. Honest peers never reach it (owing a bind means the turn is ours); a deviating one is a retriable guard-phase no-op. Both guards mutation-checked. No wire, FFI, or error-variant change; the generated binding is unchanged, so no contract bump (as #111). Co-Authored-By: Claude Fable 5 --- .changeset/gate-the-rekey-upd-door.md | 25 +++++++ rust/two-mls-pq/src/session/pq_ops.rs | 48 +++++++++++++- rust/two-mls-pq/src/session/tests.rs | 93 +++++++++++++++++++++++++++ 3 files changed, 164 insertions(+), 2 deletions(-) create mode 100644 .changeset/gate-the-rekey-upd-door.md diff --git a/.changeset/gate-the-rekey-upd-door.md b/.changeset/gate-the-rekey-upd-door.md new file mode 100644 index 0000000..4c78d2d --- /dev/null +++ b/.changeset/gate-the-rekey-upd-door.md @@ -0,0 +1,25 @@ +--- +"@germ-network/two-mls-pq": minor +--- + +Gate the A.5 `Upd'` door on content type, and refuse a rekey while a bind is owed + +The A.5 `Upd'` door carries a proposal, but the routing tag is the sender's, and +it feeds `process_incoming_message` on our own send-PQ — which validates and +*applies* a commit atomically. The peer is a member of that group, so it could +author a commit there that would apply, moving our send-PQ epoch, before the +door's kind check refused it — and the refusal wore the fatal `Mls` disposition +that asks a host to tear the session down. This closes the same gap the A.4 leg +doors already closed: the door now reads the content type off the plaintext +framing first and refuses anything but a proposal with the retriable +`DecryptionFailed`, nothing applied. + +The door also now refuses a rekey while a classical bind is owed. Its closure +commits our send-PQ, which moves the epoch an owed bind reserved in its +attestation — and discharging against a moved epoch fails with the PQ leaf +already spent. An honest peer never reaches this (owing a bind means the turn is +still ours), so a deviating one is refused in the guard phase as a retriable +no-op, exactly as the bind entry points guard the same reservation. + +No wire, FFI, or error-variant change; a session that saw the old fatal `Mls` +here now sees a retriable `DecryptionFailed`. diff --git a/rust/two-mls-pq/src/session/pq_ops.rs b/rust/two-mls-pq/src/session/pq_ops.rs index 90bf1ca..fe61f1d 100644 --- a/rust/two-mls-pq/src/session/pq_ops.rs +++ b/rust/two-mls-pq/src/session/pq_ops.rs @@ -148,6 +148,22 @@ fn process_a4_leg( Ok(payload.to_vec()) } +/// The content type an MLS protocol message declares in its PLAINTEXT framing — a pure read, +/// no keys and no state, so it can gate a door before `process_incoming_message` (which +/// validates and APPLIES atomically) ever sees the message. `None` for a non-protocol message +/// (Welcome, KeyPackage, GroupInfo), which no door here accepts anyway. Covers both wire +/// formats: our control messages are `PublicMessage` today (`EncryptionOptions::default`), but +/// reading the type off either shape keeps the gate correct if that ever changes. +fn declared_content_type(msg: &MlsMessage) -> Option { + match msg.description() { + mls_rs::MlsMessageDescription::PrivateProtocolMessage { content_type, .. } + | mls_rs::MlsMessageDescription::PublicProtocolMessage { content_type, .. } => { + Some(content_type) + } + _ => None, + } +} + /// Encode an A.4 leg's authenticated CONTENT: the domain tag then the payload /// (`[0x17][ek]` or `[0x19][wire_ct]`). This is the plaintext handed to /// `encrypt_application_message`, so the tag is covered by the MLS signature — binding the @@ -976,11 +992,34 @@ impl TwoMlsPqSession { } let proposal_msg = MlsMessage::from_bytes(proposal_bytes).map_err(|_| TwoMlsPqError::Mls)?; + // Content-type gate, mirroring `process_a4_leg`'s and for the same reason: the + // routing tag is the sender's, and this door feeds `process_incoming_message` on + // our SEND-PQ, which APPLIES a commit atomically. So a Commit smuggled behind the + // `Upd'` tag — the peer is a member of that group and can author a valid one — + // would advance our send-PQ epoch before the kind match in the closure could + // refuse it, and (pre-#111 style) answer the fatal `Mls`. Read the type off the + // plaintext framing HERE, in the guard phase, and refuse anything but a proposal + // as a pure no-op: `DecryptionFailed`, the retriable/discardable disposition, + // nothing consumed. A proposal still enters the closure, where mls-rs caches it + // and the refusal path drops it. + if declared_content_type(&proposal_msg) != Some(mls_rs::group::ContentType::Proposal) { + return Err(TwoMlsPqError::DecryptionFailed); + } // Inflight only (see `pq_ratchet_respond`): the slot may hold our own retained // frame from the previous round. if inner.pq_inflight.is_some() { return Err(TwoMlsPqError::SessionNotReady); } + // No A.5 while a bind is owed. The closure below commits our send-PQ (the Commit'), + // moving `pq_epoch` — but an owed bind has RESERVED the current `pq_epoch` in its + // attestation, and `discharge_owed_bind` refuses a bind whose reserved epoch no + // longer matches, with the PQ leaf already spent and unrebuildable. An honest peer + // never reaches this: owing a bind means the turn is still ours, so it is not the + // peer's to open a round. A deviating one is refused retriably here, before the + // choke point — the bind entry points guard the same reservation the same way. + if inner.owed_bind.is_some() { + return Err(TwoMlsPqError::SessionNotReady); + } proposal_msg }; self.mutate_and_persist(crate::BlobKind::Checkpoint, |inner| { @@ -1028,9 +1067,13 @@ impl TwoMlsPqSession { // drops what it refused. Nothing is lost by that: the peer re-sends its // `Upd'` — which is exactly what makes the credential-lag refusal below // retriable — and the retry re-ingests it. + // The content-type gate in the guard phase already proved this is a proposal, + // so `process_incoming_message` returns `Proposal` here — but decrypt/validate + // failures are still a peer-frame judgement, so map them to the retriable + // `DecryptionFailed`, never the fatal `Mls`. let ingested = match send_pq .process_incoming_message(proposal_msg) - .map_err(|_| TwoMlsPqError::Mls)? + .map_err(map_app_message_err)? { // Only the peer's own-leaf Update is a legitimate A.5 opener. ReceivedMessage::Proposal(desc) => { @@ -1040,7 +1083,8 @@ impl TwoMlsPqSession { }) }) } - _ => Err(TwoMlsPqError::Mls), + // Unreachable past the gate; defense in depth, and never fatal. + _ => Err(TwoMlsPqError::DecryptionFailed), }; rotated = match ingested { Ok(announced) => announced, diff --git a/rust/two-mls-pq/src/session/tests.rs b/rust/two-mls-pq/src/session/tests.rs index e0448c7..ee1cf9f 100644 --- a/rust/two-mls-pq/src/session/tests.rs +++ b/rust/two-mls-pq/src/session/tests.rs @@ -1580,6 +1580,99 @@ fn test_unsolicited_rekey_commit_is_rejected() { assert_err!(bob.pq_rekey_apply(bogus), TwoMlsPqError::SessionNotReady); } +/// A Commit smuggled behind the A.5 `Upd'` tag must be refused BEFORE it is applied. +/// +/// The `Upd'` door feeds `process_incoming_message` on our own send-PQ, which validates and +/// APPLIES a commit atomically. The peer is a member of that group, so it can author a commit +/// there that WOULD apply — moving our send-PQ epoch — and the pre-gate code refused it only +/// AFTER, on the message kind, with the fatal `Mls`. The content-type gate reads the kind off +/// the plaintext framing first and refuses anything but a proposal with the retriable +/// `DecryptionFailed`, nothing applied. +/// +/// The forged commit here is a REAL one for our send-PQ at its current epoch — built in the +/// peer's recv-PQ mirror of it — so absent the gate `process_incoming_message` accepts and +/// applies it. The epoch assertion is what a mutation of the gate trips. +#[test] +fn test_commit_smuggled_behind_the_rekey_upd_tag_is_refused_unapplied() { + let (alice, bob) = establish_full(); + + let send_pq_epoch = |s: &Arc| { + let inner = s.lock(); + assert_some!(inner.send_group.as_ref().and_then(|g| g.pq.as_ref())).current_epoch() + }; + let before = send_pq_epoch(&bob); + + // Alice's recv-PQ mirror IS Bob's send-PQ, so a commit built there is a valid commit for + // Bob's send-PQ at its current epoch — exactly what would apply if it reached the ingest. + let commit = { + let mut inner = alice.lock(); + let mirror = assert_some!(inner.recv_group.as_mut().and_then(|g| g.pq.as_mut())); + assert_ok!(assert_ok!(mirror.commit_builder().build()) + .commit_message + .to_bytes()) + }; + let mut forged = vec![super::PQ_REKEY_UPD_TAG]; + forged.extend_from_slice(&commit); + + assert_err!( + bob.pq_rekey_respond(forged), + TwoMlsPqError::DecryptionFailed + ); + assert_eq!( + send_pq_epoch(&bob), + before, + "a commit refused at the Upd' door must not have applied" + ); + { + let inner = bob.lock(); + let send_pq = assert_some!(inner.send_group.as_ref().and_then(|g| g.pq.as_ref())); + assert!( + send_pq.get_cached_proposals().is_empty() && !send_pq.has_pending_commit(), + "and must have left no residue" + ); + } + + // The session is untouched: a real rotation-driven A.5 still runs to completion. + ratchet_round(&bob, &alice, b"flip"); + let new_id = make_client().client_id(); + rekey_round(&bob, &alice, new_id); +} + +/// The A.5 `Upd'` door refuses a rekey while a bind is owed. The closure commits our send-PQ, +/// moving `pq_epoch` — but an owed bind reserved the current one in its attestation, and +/// discharging against a moved epoch fails with the PQ leaf already spent. An honest peer +/// never sends here (owing a bind means the turn is ours), so a deviating one is refused in +/// the guard as a retriable no-op, the same way the bind entry points guard the reservation. +#[test] +fn test_rekey_upd_refused_while_a_bind_is_owed() { + let (alice, bob) = establish_full(); + // Drive an A.4 to the point where Bob owes his classical bind (bound, not yet discharged). + let ek = open_ratchet(&bob, &alice); + assert_ok!(alice.pq_ratchet_respond(ek)); + let ct = assert_some!(alice.pq_take_pending_outbound()); + assert_ok!(bob.pq_ratchet_bind(ct)); + assert!( + bob.lock().owed_bind.is_some(), + "Bob should owe his classical bind here" + ); + + // A well-formed Upd' proposal for Bob's send-PQ, built in Alice's recv-PQ mirror of it. + let upd = { + let mut inner = alice.lock(); + let mirror = assert_some!(inner.recv_group.as_mut().and_then(|g| g.pq.as_mut())); + assert_ok!(assert_ok!(mirror.propose_update(Vec::new())).to_bytes()) + }; + let mut frame = vec![super::PQ_REKEY_UPD_TAG]; + frame.extend_from_slice(&upd); + + // Refused retriably in the guard — nothing consumed, the owed bind intact. + assert_err!(bob.pq_rekey_respond(frame), TwoMlsPqError::SessionNotReady); + assert!(bob.lock().owed_bind.is_some(), "the owed bind must survive"); + + // And the owed bind still discharges, closing the round normally. + discharge_bind(&bob, &alice, b"after-refused-rekey"); +} + /// The session's own leaf signature public keys in (send-PQ, recv-PQ) — the two /// leaves an A.5 credential handoff must move to the new principal: the recv-mirror /// leaf via the initiator's Upd' (proposal replaces the proposer), the own-send-PQ