diff --git a/.changeset/session-id-is-the-initiator-group-id.md b/.changeset/session-id-is-the-initiator-group-id.md new file mode 100644 index 0000000..902aa69 --- /dev/null +++ b/.changeset/session-id-is-the-initiator-group-id.md @@ -0,0 +1,30 @@ +--- +"@germ-network/two-mls-pq": minor +--- + +The session id is the initiator's group id; remove the client-id-pair hash + +`active_session_id()` and its `SessionId` type returned a hash of the two client +ids — `SHA-256(min(a,b) ‖ max(a,b))`, no seed. That is a participant-**pair** +fingerprint, not a session id: anyone holding the two public client ids can +compute it, and it is identical across every session the pair ever opens. Both +are removed (finishing what contract 31 began by dropping the free +`derive_session_id`). + +The real session id is the **initiator's randomly-generated group id** — fresh +per session, unpredictable, and already shared, because the initiator's send +group is the acceptor's receive group. Read it with `send_group_id()` on the +initiator and `receive_group_id()` on the acceptor (the classical half, present +from construction); both name the same value, and it survives archive restore +with the group state. The tests now assert this, including that two sessions +between the *same* pair get different ids — the property the old hash could never +satisfy. + +Nothing downstream breaks: the vended Swift wrapper never forwarded the accessor, +AbstractTwoMLS never called it, and the app already keys on group ids +(`receiveGroupId` / `sendGroupId`). The stored field is dropped from the live +session; its slot stays vestigial in the archive (written empty, ignored on +decode) so a released 0.14 archive still decodes under the v3 migration. + +Binding contract 31 → 32 — two FFI symbols removed, re-pair the vendored binding. +No wire change (archive layout stays v3) and no error-variant change. diff --git a/Sources/TwoMLSPQ/PQSession.swift b/Sources/TwoMLSPQ/PQSession.swift index b19d37e..36cf1ba 100644 --- a/Sources/TwoMLSPQ/PQSession.swift +++ b/Sources/TwoMLSPQ/PQSession.swift @@ -225,12 +225,19 @@ import TwoMLSPQBinding // v31 (contract 31): `deriveSessionId` is REMOVED from the crate's FFI surface — a session // pins its id at its FOUNDING pair for life, so re-deriving from current client ids // disagreed with it after a principal rotation (and always, for a born-dedicated -// acceptor). `activeSessionId()` is the session's id; a pre-session pair key is a digest -// the caller computes itself. Nothing to do here: the raw binding is an internal target, -// so this function was never reachable from this product. The bump covers the dropped FFI -// symbol, which requires the vendored binding to be re-paired with the binary. No wire, -// API, or error-variant change. -private let expectedBindingContract: UInt64 = 31 +// acceptor). Nothing to do here: the raw binding is an internal target, so this function +// was never reachable from this product. The bump covers the dropped FFI symbol, which +// requires the vendored binding to be re-paired with the binary. No wire, API, or +// error-variant change. +// v32 (contract 32): `activeSessionId()` and its `SessionId` type are REMOVED, finishing v31. +// A seedless hash of the two client ids is a participant-pair fingerprint, not a session +// id — predictable from the public ids and identical across every session the pair opens. +// The session id is the INITIATOR's randomly-generated group id (fresh per session, +// already shared: the initiator's send group is the acceptor's receive group), read via +// the group-id accessors. Nothing to do here either — the removed function was never +// reachable from this product, and the whole app stack already keys on the group id. The +// bump covers the dropped FFI symbols. No wire, API, or error-variant change. +private let expectedBindingContract: UInt64 = 32 enum TwoMLSPQBindingContract { static let verified: Void = { @@ -752,13 +759,15 @@ public struct PQSession { base.receiveGroupId()?.classical.bytes } - /// This side's OWN send-group classical id — a stable, per-endpoint session - /// identifier present from creation. Distinct from `shouldListenOn()` (which - /// bundles the same value into a routing/rendezvous tuple): this is the identity - /// value on its own, for an adopter that keys local session state by it. It is - /// LOCAL — each endpoint's send group differs (my send group is the peer's - /// receive group), so it is never a shared-across-peers at-rest identifier, and - /// it is NOT `activeSessionId()` (the shared client-id-pair hash). + /// This side's OWN send-group classical id — present from creation. Distinct from + /// `shouldListenOn()` (which bundles the same value into a routing/rendezvous tuple): + /// this is the identity value on its own, for an adopter that keys local session state + /// by it. It is PER-ENDPOINT — each side's send group differs, and my send group is the + /// peer's receive group. The INITIATOR's send-group id doubles as the shared session id + /// (the acceptor names the same value as its receive group); the acceptor's own + /// `localSessionId` is a distinct per-endpoint value. There is no separate derived + /// session id — a hash of the two client ids would be a pair fingerprint, not a session + /// id. public var localSessionId: GroupID? { base.sendGroupId()?.classical.bytes } diff --git a/Sources/TwoMLSPQBinding/two_mls_pq.swift b/Sources/TwoMLSPQBinding/two_mls_pq.swift index 83b79a9..4234e03 100644 --- a/Sources/TwoMLSPQBinding/two_mls_pq.swift +++ b/Sources/TwoMLSPQBinding/two_mls_pq.swift @@ -1791,8 +1791,6 @@ public func FfiConverterTypeTwoMlsPqPrincipal_lower(_ value: TwoMlsPqPrincipal) */ public protocol TwoMlsPqSessionProtocol: AnyObject, Sendable { - func activeSessionId() -> SessionId - /** * The app-state binding this session was created with (`initiate`'s `app_binding`, * or the binding the accepted welcome carried), or `None` for an unbound session. @@ -1899,12 +1897,14 @@ public protocol TwoMlsPqSessionProtocol: AnyObject, Sendable { /** * This session's OWN send-group id — the classical half is present from - * creation, the PQ half empty until its deferred bootstrap (A.3). Unlike - * [`active_session_id`](Self::active_session_id) (a hash of the two client ids, - * shared across the pair) this is a per-endpoint value: each side's send group - * differs, so an adopter keying local state by it never shares an at-rest - * identifier with its peer. The mirror of [`receive_group_id`](Self::receive_group_id) - * (my send group is the peer's receive group). + * creation, the PQ half empty until its deferred bootstrap (A.3). A per-endpoint + * value: each side's send group differs. The mirror of + * [`receive_group_id`](Self::receive_group_id) — my send group is the peer's receive + * group — which is what makes the INITIATOR's send-group id a shared identifier both + * parties can name (the initiator by this accessor, the acceptor by `receive_group_id`). + * That randomly-seeded, per-session group id is the session identifier; there is no + * separate derived id (a hash of the two client ids would be a pair fingerprint, not a + * session id — the same pair's every session would share it). */ func sendGroupId() -> CombinerGroupId? @@ -2522,14 +2522,6 @@ public static func restore(core: Archive?, checkpoint: Archive?)throws -> TwoMl -open func activeSessionId() -> SessionId { - return try! FfiConverterTypeSessionId_lift(try! rustCall() { - uniffi_two_mls_pq_fn_method_twomlspqsession_active_session_id( - self.uniffiCloneHandle(),$0 - ) -}) -} - /** * The app-state binding this session was created with (`initiate`'s `app_binding`, * or the binding the accepted welcome carried), or `None` for an unbound session. @@ -2708,12 +2700,14 @@ open func receiveGroupId() -> CombinerGroupId? { /** * This session's OWN send-group id — the classical half is present from - * creation, the PQ half empty until its deferred bootstrap (A.3). Unlike - * [`active_session_id`](Self::active_session_id) (a hash of the two client ids, - * shared across the pair) this is a per-endpoint value: each side's send group - * differs, so an adopter keying local state by it never shares an at-rest - * identifier with its peer. The mirror of [`receive_group_id`](Self::receive_group_id) - * (my send group is the peer's receive group). + * creation, the PQ half empty until its deferred bootstrap (A.3). A per-endpoint + * value: each side's send group differs. The mirror of + * [`receive_group_id`](Self::receive_group_id) — my send group is the peer's receive + * group — which is what makes the INITIATOR's send-group id a shared identifier both + * parties can name (the initiator by this accessor, the acceptor by `receive_group_id`). + * That randomly-seeded, per-session group id is the session identifier; there is no + * separate derived id (a hash of the two client ids would be a pair fingerprint, not a + * session id — the same pair's every session would share it). */ open func sendGroupId() -> CombinerGroupId? { return try! FfiConverterOptionTypeCombinerGroupId.lift(try! rustCall() { @@ -4782,61 +4776,6 @@ public func FfiConverterTypeRendezvousId_lower(_ value: RendezvousId) -> RustBuf return FfiConverterTypeRendezvousId.lower(value) } - -/** - * Session identifier derived from both parties' client IDs at init time. - * Both sides can derive the same ID independently, preventing identity - * confusion when both parties initiate simultaneously. - */ -public struct SessionId: Equatable, Hashable { - public var bytes: Data - - // Default memberwise initializers are never public by default, so we - // declare one manually. - public init(bytes: Data) { - self.bytes = bytes - } - - - - -} - -#if compiler(>=6) -extension SessionId: Sendable {} -#endif - -#if swift(>=5.8) -@_documentation(visibility: private) -#endif -public struct FfiConverterTypeSessionId: FfiConverterRustBuffer { - public static func read(from buf: inout (data: Data, offset: Data.Index)) throws -> SessionId { - return - try SessionId( - bytes: FfiConverterData.read(from: &buf) - ) - } - - public static func write(_ value: SessionId, into buf: inout [UInt8]) { - FfiConverterData.write(value.bytes, into: &buf) - } -} - - -#if swift(>=5.8) -@_documentation(visibility: private) -#endif -public func FfiConverterTypeSessionId_lift(_ buf: RustBuffer) throws -> SessionId { - return try FfiConverterTypeSessionId.lift(buf) -} - -#if swift(>=5.8) -@_documentation(visibility: private) -#endif -public func FfiConverterTypeSessionId_lower(_ value: SessionId) -> RustBuffer { - return FfiConverterTypeSessionId.lower(value) -} - // Note that we don't yet support `indirect` for enums. // See https://github.com/mozilla/uniffi-rs/issues/396 for further discussion. /** @@ -6420,9 +6359,6 @@ private let initializationResult: InitializationResult = { if (uniffi_two_mls_pq_checksum_method_twomlspqprincipal_generate_key_package() != 11085) { return InitializationResult.apiChecksumMismatch } - if (uniffi_two_mls_pq_checksum_method_twomlspqsession_active_session_id() != 37750) { - return InitializationResult.apiChecksumMismatch - } if (uniffi_two_mls_pq_checksum_method_twomlspqsession_app_binding() != 59144) { return InitializationResult.apiChecksumMismatch } @@ -6459,7 +6395,7 @@ private let initializationResult: InitializationResult = { if (uniffi_two_mls_pq_checksum_method_twomlspqsession_receive_group_id() != 24855) { return InitializationResult.apiChecksumMismatch } - if (uniffi_two_mls_pq_checksum_method_twomlspqsession_send_group_id() != 56157) { + if (uniffi_two_mls_pq_checksum_method_twomlspqsession_send_group_id() != 9016) { return InitializationResult.apiChecksumMismatch } if (uniffi_two_mls_pq_checksum_method_twomlspqsession_set_initial_app_payload() != 22701) { diff --git a/book/src/api-reference.md b/book/src/api-reference.md index 6333530..6c55b69 100644 --- a/book/src/api-reference.md +++ b/book/src/api-reference.md @@ -87,7 +87,8 @@ The receiving side of a published key package — no live client required. initiator sees the dedicated principal from the very first frame, with no founding→dedicated rotation, so nothing can displace the welcome staple. The receive-group join still uses the invitation identity (the welcome was addressed to its key package), and - the session id still derives from the founding pair, so both sides agree on it. + the session id — the initiator's random group id — is unaffected by any of this, + so both sides still name the same one. `expected_remote` is the identity the caller already expects the welcome from (Germ validates it from the decrypted initial frame): a key package naming anyone else is rejected as `RemoteIdentityMismatch` **before any invitation state is @@ -137,20 +138,23 @@ The receiving side of a published key package — no live client required. - `MlsCipherSuite::is_combiner_pq()` / `is_combiner_classical()` — routing signals (true for the PQ `0xFDEA` and classical `0x0003` halves respectively). -`derive_session_id(a, b)` was **removed** in contract 31. A session pins its id at its -**founding** pair — the invitation identity the initiator addressed — and never moves it -again, while the client ids themselves do: a principal rotation replaces them, and a -born-dedicated acceptor never operated under its founding id at all. Re-deriving from the -ids a caller holds later therefore produced a digest the session did not agree with. There -is no single replacement, because the call answered two different questions: +**The session id is the initiator's group id.** `derive_session_id(a, b)` was removed in +contract 31 and its stored replacement `active_session_id()` in contract 32, because both +were a hash of the two client ids — `SHA-256(min(a,b) ‖ max(a,b))` — and that is a +participant-**pair** fingerprint, not a session id: it is computable by anyone holding the +two public `ClientId`s, and it is identical across every session the pair ever opens. -- *"What is this session's id?"* — [`active_session_id()`](#twomlspqsession), the stored - founding value: identical on both sides, available from construction, preserved across - archive restore. -- *"What is a stable key for this pair, before a session exists?"* — compute your own - digest. It was only `SHA-256(min(a,b) ‖ max(a,b))` over two public `ClientId`s, with - nothing secret and nothing protocol-specific in it — but do not call the result a session - id, since it stops matching the session's the moment either party rotates. +A real session id must be fresh and unpredictable per session. The one already in hand is +the **initiator's randomly-generated group id**: seeded at group creation, unique per +session, and shared — the initiator's send group *is* the acceptor's receive group. Read it +with [`send_group_id()`](#twomlspqsession) on the initiator and +[`receive_group_id()`](#twomlspqsession) on the acceptor (the classical half, present from +construction; both sides name the same value). It survives archive restore with the group +state. + +If you genuinely need a stable key for a *pair* before any session exists, compute your own +`SHA-256(min(a,b) ‖ max(a,b))` over the two `ClientId`s — nothing secret, nothing +protocol-specific — but do not call it a session id. ## `TwoMlsPqSession` @@ -175,9 +179,9 @@ rest by higher `state_seq`) and fails closed (`ArchiveInvalid`) on a PQ-epoch ma mismatch. State: `is_established`, `is_fully_established`, `has_receive_group`, -`active_session_id` (the founding-pair id this session was constructed with — since -contract 31 the only way to obtain it, the free `derive_session_id` having been removed), -`receive_group_id`, `my_principal_state`, `their_principal_state`, +`send_group_id` / `receive_group_id` (the initiator's `send_group_id` is the session id — +see above; each is a `CombinerGroupId` whose classical half is present from construction), +`my_principal_state`, `their_principal_state`, `pending_outbound` (the standalone copy of the own welcome — not consumed by `encrypt`; the welcome also rides every pre-commit frame as the staple), `epochs`, `app_binding() -> Result>>` (Swift `try appBinding() -> Data?`; the diff --git a/rust/two-mls-pq/src/lib.rs b/rust/two-mls-pq/src/lib.rs index abfe0e4..aa22645 100644 --- a/rust/two-mls-pq/src/lib.rs +++ b/rust/two-mls-pq/src/lib.rs @@ -448,7 +448,21 @@ pub fn version() -> String { // constructors. Removing an exported function drops its FFI symbol, so the vendored binding // must be re-paired with the binary; nothing else in the surface, the wire, or the error // variants moves. -const BINDING_CONTRACT_VERSION: u64 = 31; +// +// v32: `active_session_id` and its `SessionId` type are REMOVED, finishing what v31 began. +// v31 kept a stored client-id-pair hash as "the session's id", but a seedless +// `SHA-256(clientA ‖ clientB)` is a participant-PAIR fingerprint, not a session id: it is +// computable by anyone holding the two public client ids and is identical across every +// session the pair ever opens. The real session id is the INITIATOR's randomly-generated +// group id — fresh per session, unpredictable, and already shared (the initiator's send group +// is the acceptor's receive group), read via `send_group_id` / `receive_group_id`. The whole +// consuming stack (AbstractTwoMLS, the app) already keys on the group id and never called the +// removed accessor, so nothing downstream breaks. The stored+archived field is dropped from +// the live session; its wire slot stays VESTIGIAL in `SessionArchive` (written empty) so a +// released 0.14 archive still decodes under the v3 migration. `pair_session_id` and the +// `SessionId` uniffi record are gone. Drops two FFI symbols — re-pair the vendored binding — +// no wire (archive layout stays v3) or error-variant change. +const BINDING_CONTRACT_VERSION: u64 = 32; /// See `BINDING_CONTRACT_VERSION`. Exported so the Swift layer can verify the /// binding it was generated with matches the binary it loaded. @@ -484,14 +498,6 @@ pub struct CombinerGroupId { pub pq: MlsGroupId, } -/// Session identifier derived from both parties' client IDs at init time. -/// Both sides can derive the same ID independently, preventing identity -/// confusion when both parties initiate simultaneously. -#[derive(Debug, Clone, PartialEq, Eq, uniffi::Record)] -pub struct SessionId { - pub bytes: Vec, -} - /// Transport rendezvous channel identifier. /// Derived per epoch via `exportSecret(label="rendezvous", context="TwoMLS", len=32)`. #[derive(Debug, Clone, uniffi::Record)] @@ -1019,31 +1025,6 @@ pub(crate) fn sha256(bytes: &[u8]) -> Vec { crate::suite::TwoMlsSuite::CURRENT.digest(bytes) } -/// The symmetric pair digest behind a session's id: both sides compute the same value -/// from the same two client ids regardless of who initiated. -/// -/// Crate-internal, and staying that way. The constructors call it with the **founding** -/// pair — the invitation identity the initiator addressed — which is the only pair for -/// which the result means anything: the session pins that value for life, while the ids -/// themselves move on (a principal rotation replaces them, and a born-dedicated acceptor -/// never operated under its founding id at all). A caller re-deriving from the ids it -/// holds later would get a digest the session does not agree with, which is why this is -/// not exported; `TwoMlsPqSession::active_session_id` reads the stored value instead. -pub(crate) fn pair_session_id(my_id: ClientId, their_id: ClientId) -> SessionId { - let (first, second) = if my_id.bytes <= their_id.bytes { - (my_id.bytes, their_id.bytes) - } else { - (their_id.bytes, my_id.bytes) - }; - - let mut input = first; - input.extend_from_slice(&second); - - SessionId { - bytes: sha256(&input), - } -} - impl From for TwoMlsPqError { fn from(_: mls_rs::error::MlsError) -> Self { TwoMlsPqError::Mls @@ -1070,32 +1051,8 @@ pub type Result = std::result::Result; #[cfg(test)] mod tests { - use super::*; - - fn client_id(bytes: &[u8]) -> ClientId { - ClientId { - bytes: bytes.to_vec(), - } - } - - #[test] - fn test_pair_session_id_is_symmetric() { - let alice = client_id(b"alice"); - let bob = client_id(b"bob"); - assert_eq!( - pair_session_id(alice.clone(), bob.clone()).bytes, - pair_session_id(bob, alice).bytes - ); - } - - #[test] - fn test_pair_session_id_differs_for_different_pairs() { - let alice = client_id(b"alice"); - let bob = client_id(b"bob"); - let carol = client_id(b"carol"); - assert_ne!( - pair_session_id(alice.clone(), bob).bytes, - pair_session_id(alice, carol).bytes - ); - } + // The session-id derivation was removed in contract 32 — the session id is the + // initiator's random group id (see `TwoMlsPqSession::send_group_id` and the + // session-id tests in `session::tests`). Nothing pair-hash-shaped remains to unit-test + // here. } diff --git a/rust/two-mls-pq/src/session/archive.rs b/rust/two-mls-pq/src/session/archive.rs index 8461c48..582855e 100644 --- a/rust/two-mls-pq/src/session/archive.rs +++ b/rust/two-mls-pq/src/session/archive.rs @@ -343,6 +343,12 @@ pub(crate) mod archive_wire { /// PQ op advanced without emitting a checkpoint (forbidden), so restore fails closed. pub(in crate::session) send_pq_epoch: Option, pub(in crate::session) recv_pq_epoch: Option, + /// VESTIGIAL. Once the client-id-pair `SessionId` this session exposed (removed in + /// contract 32 — the session id is the initiator's group id, needs no separate + /// value). The field stays because a released 0.14 (v2) archive carries these bytes + /// in exactly this position, and the v3 migration decodes the same body shape, so + /// removing it would misread every 0.14 blob. Written empty from v3 on, ignored on + /// decode. Drop it only in a version that no longer accepts v2. #[mls_codec(with = "mls_rs_codec::byte_vec")] pub(in crate::session) session_id: Vec, /// The session's current client signing identity, rebuilt byte-exact on restore @@ -858,9 +864,7 @@ fn session_from_wire( deferred_candidate: wire.deferred_candidate, auth_core: auth_core_restored, pq_inflight, - session_id: SessionId { - bytes: wire.session_id, - }, + // `wire.session_id` is vestigial (see the wire field) — read and dropped. state_seq: wire.state_seq, my_state, their_state, @@ -1052,7 +1056,8 @@ fn build_archive_wire( state_seq: inner.state_seq, send_pq_epoch, recv_pq_epoch, - session_id: inner.session_id.bytes.clone(), + // Vestigial (see the wire field): written empty from v3 on. + session_id: Vec::new(), client, my_state: wire_principal_state(&inner.my_state), their_state: wire_principal_state(&inner.their_state), diff --git a/rust/two-mls-pq/src/session/mod.rs b/rust/two-mls-pq/src/session/mod.rs index 1c8ec5b..a1289b4 100644 --- a/rust/two-mls-pq/src/session/mod.rs +++ b/rust/two-mls-pq/src/session/mod.rs @@ -31,7 +31,7 @@ use crate::{ }, Archive, ClientId, CombinerGroupId, CommitResult, DecryptResult, EncryptResult, EpochRendezvous, ListenChannels, MlsGroupId, MlsSenderMessage, PrepareEncryptResult, - PrincipalState, RendezvousId, Result, SessionId, TwoMlsPqError, + PrincipalState, RendezvousId, Result, TwoMlsPqError, }; use zeroize::Zeroizing; @@ -234,7 +234,6 @@ struct SessionInner { /// `AuthView` — the auth analogue of `track_psk_stores`. auth_core: AuthCoreHandle, pq_inflight: Option, - session_id: SessionId, /// Monotonic per-session mutation counter, bumped once per state-advancing FFI call /// (see `mutate_and_persist`). Serialized in the archive so it continues across a /// restore; stamps each pushed blob and feeds `depends_on_seq` on outbound frames. `u64` @@ -1207,7 +1206,6 @@ fn build_session( send_group: Option, recv_group: Option, pending_outbound: Option>, - session_id: SessionId, their_id: ClientId, initiated: bool, joined_welcome_digest: Option>, @@ -1244,7 +1242,6 @@ fn build_session( deferred_candidate: None, auth_core, pq_inflight: None, - session_id, state_seq: 0, my_state: PrincipalState::Sync { client_id: my_id }, their_state: PrincipalState::Sync { @@ -1524,7 +1521,6 @@ impl TwoMlsPqSession { validate_combiner_kp(client.combiner().cipher_suite(), &their_key_package)?; let their_parsed = parse_mls_key_package(their_key_package.classical.clone())?; let their_id = their_parsed.client_id; - let session_id = crate::pair_session_id(client.client_id(), their_id.clone()); let (send_group, apq_welcome) = create_combiner_send_group( &their_key_package.classical, @@ -1542,7 +1538,6 @@ impl TwoMlsPqSession { // idempotently skips), not the sealed envelope. `pending_outbound` is replaced // with the envelope just below. Some(apq_welcome), - session_id, their_id, true, None, @@ -1699,10 +1694,6 @@ impl TwoMlsPqSession { )?; let their_parsed = parse_mls_key_package(their_classical_key_package.clone())?; let their_id = their_parsed.client_id; - // The session id derives from the FOUNDING pair — the invitation identity the - // peer initiated toward — never the dedicated principal, so both sides compute - // the same value (the initiator derives it from the key package it addressed). - let session_id = crate::pair_session_id(client.client_id(), their_id.clone()); // Decode the incoming welcome once; validate its cipher suite(s) before joining, so a // mismatch fails early and clearly rather than deep inside mls-rs — then join the @@ -1800,7 +1791,6 @@ impl TwoMlsPqSession { Some(send_group), Some(recv_group), Some(apq_welcome), - session_id, their_id, false, // Record which welcome the recv group was joined from: welcomes are @@ -2087,10 +2077,6 @@ impl TwoMlsPqSession { self.lock().recv_group.is_some() } - pub fn active_session_id(&self) -> SessionId { - self.lock().session_id.clone() - } - pub fn my_principal_state(&self) -> PrincipalState { self.lock().my_state.clone() } @@ -2133,12 +2119,14 @@ impl TwoMlsPqSession { } /// This session's OWN send-group id — the classical half is present from - /// creation, the PQ half empty until its deferred bootstrap (A.3). Unlike - /// [`active_session_id`](Self::active_session_id) (a hash of the two client ids, - /// shared across the pair) this is a per-endpoint value: each side's send group - /// differs, so an adopter keying local state by it never shares an at-rest - /// identifier with its peer. The mirror of [`receive_group_id`](Self::receive_group_id) - /// (my send group is the peer's receive group). + /// creation, the PQ half empty until its deferred bootstrap (A.3). A per-endpoint + /// value: each side's send group differs. The mirror of + /// [`receive_group_id`](Self::receive_group_id) — my send group is the peer's receive + /// group — which is what makes the INITIATOR's send-group id a shared identifier both + /// parties can name (the initiator by this accessor, the acceptor by `receive_group_id`). + /// That randomly-seeded, per-session group id is the session identifier; there is no + /// separate derived id (a hash of the two client ids would be a pair fingerprint, not a + /// session id — the same pair's every session would share it). pub fn send_group_id(&self) -> Option { let inner = self.lock(); inner.send_group.as_ref().map(|sg| CombinerGroupId { diff --git a/rust/two-mls-pq/src/session/tests.rs b/rust/two-mls-pq/src/session/tests.rs index e0448c7..eee9c04 100644 --- a/rust/two-mls-pq/src/session/tests.rs +++ b/rust/two-mls-pq/src/session/tests.rs @@ -2719,6 +2719,11 @@ fn test_accept_with_invalid_welcome_bytes_returns_error() { ); } +/// The canonical session id is the INITIATOR's randomly-generated group id, and both sides +/// name the same one — the initiator's send group IS the acceptor's receive group. This is the +/// real session identifier: seeded with fresh randomness at group creation, so it is unique +/// per session and unpredictable, unlike a bare hash of the two client ids (which every +/// session this pair ever opens would share — see `test_session_id_is_fresh_per_session`). #[test] fn test_session_id_is_same_from_both_sides() { let alice = make_client(); @@ -2737,11 +2742,13 @@ fn test_session_id_is_same_from_both_sides() { None )); - assert_eq!( - alice_session.active_session_id().bytes, - bob_session.active_session_id().bytes, - "session IDs must match" - ); + // Alice initiates, so the session id is her send group's (classical) id; Bob names the + // same value as his RECEIVE group. The classical half is present from creation, so this + // holds before the A.3 PQ bootstrap. + let alice_id = assert_some!(alice_session.send_group_id()).classical.bytes; + let bob_id = assert_some!(bob_session.receive_group_id()).classical.bytes; + assert!(!alice_id.is_empty(), "a group id is never empty"); + assert_eq!(alice_id, bob_id, "both sides must name the same session id"); } #[test] @@ -3150,9 +3157,14 @@ fn test_archive_round_trips_session_state() { let (alice_session, bob_session) = establish_sessions(); message_round(&alice_session, &bob_session, b"before"); - let session_id = alice_session.active_session_id(); + // The session id (the initiator's group id) rides the archived group state, so it is the + // same across a restore. + let session_id = assert_some!(alice_session.send_group_id()).classical.bytes; let restored = round_trip(&alice_session); - assert_eq!(restored.active_session_id(), session_id); + assert_eq!( + assert_some!(restored.send_group_id()).classical.bytes, + session_id + ); // Both directions keep flowing across the restore. message_round(&restored, &bob_session, b"restored->bob"); @@ -4256,25 +4268,43 @@ fn test_process_incoming_proposal_returns_none_until_queued() { assert!(prep2.did_commit, "must commit after queue_proposal"); } +/// The session id is fresh PER SESSION, not per pair — the crux of why the initiator's random +/// group id is the right identifier and a client-id-pair hash is not. Different peers differ, +/// of course; but so do two sessions between the SAME pair, which a bare +/// `SHA-256(clientA ‖ clientB)` — no seed — could never distinguish. Both are asserted here. #[test] -fn test_session_id_differs_for_different_pairs() { +fn test_session_id_is_fresh_per_session() { let alice = make_client(); let bob = make_client(); let carol = make_client(); - let bob_kp = make_combiner_kp(&bob); - let carol_kp = make_combiner_kp(&carol); - let alice_bob = assert_ok!(TwoMlsPqSession::initiate(Arc::clone(&alice), bob_kp, None)); + let sid = |s: &Arc| assert_some!(s.send_group_id()).classical.bytes; + + // Different peers → different ids. + let alice_bob = assert_ok!(TwoMlsPqSession::initiate( + Arc::clone(&alice), + make_combiner_kp(&bob), + None + )); let alice_carol = assert_ok!(TwoMlsPqSession::initiate( Arc::clone(&alice), - carol_kp, + make_combiner_kp(&carol), None )); + assert_ne!(sid(&alice_bob), sid(&alice_carol), "different pairs differ"); + // The SAME pair, a second session → still a different id. This is the property the old + // client-id-pair hash lacked: with no per-session seed it would have returned the same + // bytes for both, colliding two distinct conversations under one identifier. + let alice_bob_again = assert_ok!(TwoMlsPqSession::initiate( + Arc::clone(&alice), + make_combiner_kp(&bob), + None + )); assert_ne!( - alice_bob.active_session_id().bytes, - alice_carol.active_session_id().bytes, - "different peer pairs must produce different session IDs" + sid(&alice_bob), + sid(&alice_bob_again), + "a fresh session between the same pair must get a fresh id" ); } @@ -5479,12 +5509,13 @@ fn test_receive_under_dedicated_principal() { )); // The acceptor's principal is the dedicated agent from birth — no Pending state, - // nothing staged. The session id still derives from the FOUNDING pair (the - // invitation identity Alice initiated toward), so both sides agree on it. + // nothing staged. Both sides still agree on the session id, and now trivially so: it is + // the initiator's random group id (Alice's send group = Bob's receive group), which does + // not depend on either party's client id — so a born-dedicated acceptor changes nothing. assert_eq!(bob_s.my_principal_state().client_id().bytes, dedicated); assert_eq!( - alice_s.active_session_id().bytes, - bob_s.active_session_id().bytes + assert_some!(alice_s.send_group_id()).classical.bytes, + assert_some!(bob_s.receive_group_id()).classical.bytes ); // Pre-join, Alice still knows the peer as the invitation identity. assert_eq!(