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