From d5feb45118a38c4e21cbc19d0f20b4e2880f4c4d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jo=C3=A3o=20Lucas?= <55464917+jlucaso1@users.noreply.github.com> Date: Wed, 5 Aug 2026 13:12:27 -0300 Subject: [PATCH] fix(libsignal): keep skipped message-key seeds projectable A session that skipped a message key could not be exported to the v1 model: v1 expresses a message key as its 32-byte seed, the native format persisted only the derived cipher/mac/iv, and that derivation has no inverse. The round trip was lossy from the first cycle, so importing a v1 record and exporting it back already failed with ChainNotRepresentable. The seed now rides along with the keys it derives, in a new local field on SessionStructure.Chain.MessageKey. It is written unconditionally: the persisted format must not vary with build flags, or a binary without legacy-session-interop would erase what one with it wrote. The derived triple stays, so a build that ignores the seed still loads the record and decrypts; only the export path reads it. The field is declared in build.rs and spliced into the descriptor rather than added to whatsapp.proto, which is regenerated wholesale from whatspec and would lose it on the next sync. Field number 100, because upstream appends low numbers without warning - kyberPreKeyId = 4 and kyberCiphertext = 5 landed on SessionStructure.PendingPreKey that way - and a collision would silently reinterpret the seed in every record already on disk. The splice now fails the build on such a collision instead of resolving it. SessionMessageKeyMaterial carries fixed-width arrays instead of Vec. The lengths were already invariants enforced at every conversion; in the type they cost no allocation and no runtime check. Writing the four persisted fields out of one buffer keeps the skipped-key path at a single allocation, down from three before this change. --- .github/workflows/main.yml | 4 + AGENTS.md | 1 + .../libsignal/src/protocol/legacy_session.rs | 132 ++++--- wacore/libsignal/src/protocol/ratchet/keys.rs | 108 +++++- .../src/protocol/record_components.rs | 217 +++++++----- .../libsignal/src/protocol/state/session.rs | 43 ++- .../tests/legacy_session_skipped_keys.rs | 327 ++++++++++++++++++ waproto/build.rs | 114 +++++- 8 files changed, 813 insertions(+), 133 deletions(-) create mode 100644 wacore/libsignal/tests/legacy_session_skipped_keys.rs diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index d429f3ffa..963b7c8df 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -157,6 +157,10 @@ jobs: run: | cargo nextest run --profile ci -p wacore --features voip --lib cargo nextest run --profile ci -p whatsapp-rust --features "voip tokio-native tokio-transport" --lib + # legacy-session-interop is off by default, so every other test job + # compiles its module away and never runs a single one of its tests. + - name: Test (legacy-session-interop) + run: cargo nextest run --profile ci -p wacore-libsignal --features legacy-session-interop rustdoc: name: Rustdoc diff --git a/AGENTS.md b/AGENTS.md index d6ca458c3..2df5f43e2 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -32,6 +32,7 @@ Things that look correct and are not: - **Locks.** `session_locks` serializes Signal encrypt/decrypt per protocol address; `chat_lanes` (`ChatLane::enqueue_lock` in `src/client.rs`) serializes *incoming* processing per chat. Outgoing sends are deliberately not per-chat locked — WA Web doesn't lock them either. - **Wire-tagged enums.** Every protocol enum derives `WireEnum`, and its `#[wire = ...]` attribute is the single source of truth for the wire value. Do not also derive `serde::Serialize`/`Deserialize` or add `#[serde(rename_all)]` — the derive owns both. In tagged mode it generates a sibling `Tag`; parsers must dispatch on `Tag::try_from(node.tag.as_ref())` rather than string literals, so renaming a tag stays a one-attribute change. Modes and attributes: `agent_docs/protocol_architecture.md`. - **Event payloads are a frozen API.** Sealed with `#[non_exhaustive]` + `#[derive(bon::Builder)]` and constructed via `Type::builder()…build()`; a maybe-absent field is `Option`, never an empty-string or zero sentinel. The full stability policy is the `Event` doc comment in `wacore/src/types/events.rs`. +- **`whatsapp.proto` is not the whole persisted schema.** It comes from whatspec and is regenerated wholesale, so fields we persist but upstream does not declare live in `LOCAL_FIELDS` in `waproto/build.rs`, spliced into the descriptor at build time. Never hand-edit the `.proto` or `.desc` to add one — the next sync would drop it. - **Blocking work** — `ureq`, heavy CPU — belongs in `tokio::task::spawn_blocking`; it shares a runtime with the read loop. - **let-chains**, never nested `if let`. Clippy's `collapsible_if` is denied in CI. - **No real PII in tests**, including vectors derived from production captures. Regenerate them from fictitious JIDs and numbers. diff --git a/wacore/libsignal/src/protocol/legacy_session.rs b/wacore/libsignal/src/protocol/legacy_session.rs index d71285c18..956323bdb 100644 --- a/wacore/libsignal/src/protocol/legacy_session.rs +++ b/wacore/libsignal/src/protocol/legacy_session.rs @@ -569,8 +569,10 @@ impl SessionRecord { /// is an inert placeholder. Local identity and registration values remain /// external because v1 does not persist them. /// - /// Derived skipped-message keys have no inverse to their v1 seed, so their - /// presence returns [`LegacySessionInteropError::NotRepresentable`]. + /// Skipped message keys carry the seed v1 expects. A record persisted + /// before that seed was retained has only the derived keys, which have no + /// inverse, so it returns + /// [`LegacySessionInteropError::ChainNotRepresentable`]. pub fn into_legacy_session_v1_operational( self, ) -> Result { @@ -961,7 +963,10 @@ fn chain_into_components( .into_iter() .map(|key| SessionMessageKeyComponents { index: key.index, - material: SessionMessageKeyMaterial::Seed(key.seed.into()), + material: SessionMessageKeyMaterial::Seed( + <[u8; LEGACY_KEY_MATERIAL_LEN]>::try_from(key.seed.as_ref()) + .expect("validated skipped message-key seed"), + ), }) .collect(), } @@ -1208,28 +1213,24 @@ fn project_chain_parts( chain: chain_index, field: LegacySessionFieldV1::ChainKeyIndex, })?; - if message_keys - .iter() - .any(|key| matches!(key.material, SessionMessageKeyMaterial::Derived { .. })) - { - return Err(LegacySessionInteropError::ChainNotRepresentable { - session, - chain: chain_index, - field: LegacySessionUnrepresentableFieldV1::DerivedMessageKey, - }); - } + // Exhaustive so a new material variant has to state its v1 form here + // instead of compiling into a silent projection. let message_keys = message_keys .into_iter() - .map(|key| { - let SessionMessageKeyMaterial::Seed(seed) = key.material else { - unreachable!("derived message-key material was rejected before allocation") - }; - LegacySessionMessageKeyV1 { + .map(|key| match key.material { + SessionMessageKeyMaterial::Seed(seed) => Ok(LegacySessionMessageKeyV1 { index: key.index, - seed: seed.into(), + seed: Bytes::copy_from_slice(&seed), + }), + SessionMessageKeyMaterial::Derived { .. } => { + Err(LegacySessionInteropError::ChainNotRepresentable { + session, + chain: chain_index, + field: LegacySessionUnrepresentableFieldV1::DerivedMessageKey, + }) } }) - .collect(); + .collect::, _>>()?; Ok(LegacySessionChainV1 { ratchet_key, @@ -1659,27 +1660,58 @@ mod tests { seed: Bytes::copy_from_slice(&seed), }]; - let components = record(vec![session]) + let native = record(vec![session]) .into_session_record(local_context()) - .expect("import") + .expect("import"); + let persisted = crate::protocol::stores::SessionStructure::from( + native.session_state().expect("current state"), + ); + let stored = &persisted.receiver_chains[0].message_keys[0]; + let expected = MessageKeyGenerator::new_from_seed(&seed, 0).generate_keys(); + assert_eq!( + stored.cipher_key.as_deref(), + Some(&expected.cipher_key()[..]) + ); + assert_eq!(stored.mac_key.as_deref(), Some(&expected.mac_key()[..])); + assert_eq!(stored.iv.as_deref(), Some(&expected.iv()[..])); + assert_eq!(stored.seed.as_deref(), Some(&seed[..])); + + let material = &native .into_components() - .expect("components"); - let material = &components.current_session.expect("current").receiver_chains[0] + .expect("components") + .current_session + .expect("current") + .receiver_chains[0] .message_keys[0] .material; - let expected = MessageKeyGenerator::new_from_seed(&seed, 0).generate_keys(); - match material { - SessionMessageKeyMaterial::Derived { - cipher_key, - mac_key, - iv, - } => { - assert_eq!(cipher_key, expected.cipher_key()); - assert_eq!(mac_key, expected.mac_key()); - assert_eq!(iv, expected.iv()); - } - SessionMessageKeyMaterial::Seed(_) => panic!("seed must be derived on import"), - } + assert_eq!(material, &SessionMessageKeyMaterial::Seed(seed)); + } + + /// Regression: a session holding a skipped key used to be unprojectable + /// from the first cycle, because import kept only the derived keys. + #[test] + fn a_retained_skipped_key_projects_back_to_its_seed() { + let seed = vec![0x77; 32]; + let mut session = reference_session(63, LegacySessionDispositionV1::Current); + session.chains[1].message_keys = vec![LegacySessionMessageKeyV1 { + index: 0, + seed: seed.clone().into(), + }]; + + let projected = record(vec![session]) + .into_session_record(local_context()) + .expect("import") + .into_legacy_session_v1_operational() + .expect("skipped key stays projectable"); + + let chains = &projected.sessions[0].session.chains; + let receiving = chains + .iter() + .find(|chain| chain.role == LegacySessionChainRoleV1::Receiving) + .expect("receiving chain"); + assert_eq!(receiving.message_keys.len(), 1); + assert_eq!(receiving.message_keys[0].index, 0); + assert_eq!(receiving.message_keys[0].seed, seed); } #[test] @@ -1813,9 +1845,31 @@ mod tests { index: 0, seed: vec![0x44; 32].into(), }]; - let native = record(vec![session]) + // Skipped keys persisted before the seed was retained come back as + // `Derived`; strip the seed to reproduce one of those records. + let mut components = record(vec![session]) .into_session_record(local_context()) - .expect("import"); + .expect("import") + .into_components() + .expect("components"); + let key = &mut components + .current_session + .as_mut() + .expect("current") + .receiver_chains[0] + .message_keys[0]; + let SessionMessageKeyMaterial::Seed(seed) = &key.material else { + panic!("imported skipped key must retain its seed") + }; + let seed = <[u8; 32]>::try_from(seed.as_slice()).expect("32-byte seed"); + let keys = MessageKeyGenerator::new_from_seed(&seed, key.index).generate_keys(); + key.material = SessionMessageKeyMaterial::Derived { + cipher_key: *keys.cipher_key(), + mac_key: *keys.mac_key(), + iv: *keys.iv(), + }; + + let native = SessionRecord::from_components(components).expect("seedless record"); assert!(matches!( native.into_legacy_session_v1_operational(), Err(LegacySessionInteropError::ChainNotRepresentable { diff --git a/wacore/libsignal/src/protocol/ratchet/keys.rs b/wacore/libsignal/src/protocol/ratchet/keys.rs index 405f9e872..b072816f2 100644 --- a/wacore/libsignal/src/protocol/ratchet/keys.rs +++ b/wacore/libsignal/src/protocol/ratchet/keys.rs @@ -18,8 +18,6 @@ use crate::protocol::{PrivateKey, PublicKey, Result, crypto, stores::session_str /// 2. **Zero-cost round-trip**: Keys loaded from protobuf are kept in serialized form and /// returned as-is when saving, avoiding unnecessary deserialization and re-serialization pub enum MessageKeyGenerator { - /// Native computed keys - from encryption operations - Keys(MessageKeys), /// Seed for lazy derivation - keys derived on demand Seed(([u8; 32], u32)), /// Original protobuf - zero-cost pass-through on save @@ -37,7 +35,6 @@ impl MessageKeyGenerator { pub fn generate_keys(self) -> MessageKeys { match self { Self::Seed((seed, counter)) => MessageKeys::derive_keys(&seed, None, counter), - Self::Keys(k) => k, Self::Serialized(pb) => { // Parse on demand - only when keys are actually needed. // Note: from_pb() validates field lengths before creating Serialized, @@ -71,19 +68,39 @@ impl MessageKeyGenerator { /// Convert to protobuf format for storage. /// Zero-cost for Serialized variant (pass-through), allocates for others. + /// + /// The seed is persisted next to the keys it derives: the derivation is + /// one-way, so a record that kept only the derived keys could never be + /// projected back into a seed-based external format. pub fn into_pb(self) -> session_structure::chain::MessageKey { match self { // Zero-cost pass-through: return original protobuf unchanged Self::Serialized(pb) => pb, // Need to serialize: derive keys and convert - Self::Seed(_) | Self::Keys(_) => { - use bytes::Bytes; - let keys = self.generate_keys(); + Self::Seed((seed, counter)) => { + use bytes::BytesMut; + let keys = MessageKeys::derive_keys(&seed, None, counter); + // The four fields are written, stored and dropped together, so + // they share one buffer: `split_to` hands out refcounted views + // instead of copying each field into its own allocation. + let mut material = BytesMut::with_capacity( + keys.cipher_key().len() + keys.mac_key().len() + keys.iv().len() + seed.len(), + ); + material.extend_from_slice(keys.cipher_key()); + material.extend_from_slice(keys.mac_key()); + material.extend_from_slice(keys.iv()); + material.extend_from_slice(&seed); + + let mut material = material.freeze(); + let cipher_key = material.split_to(keys.cipher_key().len()); + let mac_key = material.split_to(keys.mac_key().len()); + let iv = material.split_to(keys.iv().len()); session_structure::chain::MessageKey { - cipher_key: Some(Bytes::copy_from_slice(keys.cipher_key())), - mac_key: Some(Bytes::copy_from_slice(keys.mac_key())), - iv: Some(Bytes::copy_from_slice(keys.iv())), + cipher_key: Some(cipher_key), + mac_key: Some(mac_key), + iv: Some(iv), index: Some(keys.counter()), + seed: Some(material), } } } @@ -109,7 +126,6 @@ impl MessageKeyGenerator { #[inline] pub fn counter(&self) -> u32 { match self { - Self::Keys(k) => k.counter(), Self::Seed((_, counter)) => *counter, Self::Serialized(pb) => pb.index.unwrap_or(0), } @@ -463,6 +479,78 @@ mod tests { assert_eq!(keys.cipher_key(), keys2.cipher_key()); } + /// The seed is one-way, so persisting it alongside the keys it derives is + /// the only thing that keeps a skipped key exportable. + #[test] + fn into_pb_persists_the_seed_next_to_the_derived_keys() { + let seed = [0x3Cu8; 32]; + let pb = MessageKeyGenerator::new_from_seed(&seed, 11).into_pb(); + let expected = MessageKeys::derive_keys(&seed, None, 11); + + assert_eq!(pb.index, Some(11)); + assert_eq!(pb.seed.as_deref(), Some(&seed[..])); + assert_eq!(pb.cipher_key.as_deref(), Some(&expected.cipher_key()[..])); + assert_eq!(pb.mac_key.as_deref(), Some(&expected.mac_key()[..])); + assert_eq!(pb.iv.as_deref(), Some(&expected.iv()[..])); + } + + /// Reloading a persisted key must keep using the stored derived material, + /// not re-derive from the seed: a decrypt that changed keys here would + /// silently fail the MAC. Only material the seed does *not* produce can + /// tell the two apart, so the fixture stores a deliberately unrelated + /// triple next to it. + #[test] + fn reloaded_keys_come_from_the_persisted_derived_material() { + use bytes::Bytes; + + let seed = [0x9Eu8; 32]; + let mut pb = MessageKeyGenerator::new_from_seed(&seed, 4).into_pb(); + pb.cipher_key = Some(Bytes::from_static(&[0x11; 32])); + pb.mac_key = Some(Bytes::from_static(&[0x22; 32])); + pb.iv = Some(Bytes::from_static(&[0x33; 16])); + let from_seed = MessageKeys::derive_keys(&seed, None, 4); + + let reloaded = MessageKeyGenerator::from_pb(pb) + .expect("key stays loadable") + .generate_keys(); + + assert_eq!(reloaded.cipher_key(), &[0x11; 32]); + assert_eq!(reloaded.mac_key(), &[0x22; 32]); + assert_eq!(reloaded.iv(), &[0x33; 16]); + assert_eq!(reloaded.counter(), 4); + assert_ne!(reloaded.cipher_key(), from_seed.cipher_key()); + } + + /// Keys persisted before the seed was retained must still load and produce + /// exactly what was stored. + #[test] + fn seedless_persisted_keys_still_load() { + let seed = [0x9Eu8; 32]; + let mut pb = MessageKeyGenerator::new_from_seed(&seed, 4).into_pb(); + let expected = MessageKeys::derive_keys(&seed, None, 4); + pb.seed = None; + + let reloaded = MessageKeyGenerator::from_pb(pb) + .expect("seedless key stays loadable") + .generate_keys(); + + assert_eq!(reloaded.cipher_key(), expected.cipher_key()); + assert_eq!(reloaded.mac_key(), expected.mac_key()); + assert_eq!(reloaded.iv(), expected.iv()); + assert_eq!(reloaded.counter(), 4); + } + + /// A seed alone is not a loadable key: `from_pb` still requires the three + /// derived fields, so a downgrade that drops the seed cannot fail the + /// whole record. + #[test] + fn from_pb_still_rejects_a_key_without_derived_material() { + let mut pb = MessageKeyGenerator::new_from_seed(&[0x2Bu8; 32], 0).into_pb(); + pb.cipher_key = None; + + assert!(MessageKeyGenerator::from_pb(pb).is_err()); + } + /// Test MessageKeys derive_keys with known inputs #[test] fn test_message_keys_derive_with_salt() { diff --git a/wacore/libsignal/src/protocol/record_components.rs b/wacore/libsignal/src/protocol/record_components.rs index d331505d4..37c16b354 100644 --- a/wacore/libsignal/src/protocol/record_components.rs +++ b/wacore/libsignal/src/protocol/record_components.rs @@ -14,6 +14,7 @@ use bytes::Bytes; use crate::core::curve::PublicKey; use crate::protocol::error::{Result, SignalProtocolError}; use crate::protocol::ratchet::MessageKeyGenerator; +use crate::protocol::ratchet::keys::MessageKeys; use crate::protocol::stores::{ SenderKeyStateStructure, SessionStructure, sender_key_state_structure, session_structure, }; @@ -76,15 +77,17 @@ pub struct SessionMessageKeyComponents { /// Secret material used by a skipped session message key. /// -/// `Seed` is accepted as a compact import form and is expanded with the -/// protocol's canonical derivation. Exported records always use `Derived`. -#[derive(Clone, PartialEq, Eq)] +/// `Seed` is the compact form: importing it expands the derived keys with the +/// protocol's canonical derivation, and exporting a record that retained the +/// seed returns it. `Derived` is what remains when there is no seed to return, +/// which is the case for keys persisted before the seed was retained. +#[derive(Clone, Copy, PartialEq, Eq)] pub enum SessionMessageKeyMaterial { - Seed(Vec), + Seed([u8; SYMMETRIC_KEY_BYTES]), Derived { - cipher_key: Vec, - mac_key: Vec, - iv: Vec, + cipher_key: [u8; SYMMETRIC_KEY_BYTES], + mac_key: [u8; SYMMETRIC_KEY_BYTES], + iv: [u8; MESSAGE_IV_BYTES], }, } @@ -297,70 +300,72 @@ fn required_exact_bytes( ) } +/// Fixed-width key material, copied out of the persisted `Bytes` without +/// allocating. +fn key_material(value: Bytes, field: &'static str) -> Result<[u8; N]> { + <[u8; N]>::try_from(value.as_ref()) + .map_err(|_| SignalProtocolError::InvalidArgument(format!("{field} must be {N} bytes"))) +} + +fn required_key_material( + value: Option, + field: &'static str, +) -> Result<[u8; N]> { + key_material(value.ok_or_else(|| invalid(field, "present"))?, field) +} + impl SessionMessageKeyComponents { - fn into_structure(self) -> Result { + fn into_structure(self) -> session_structure::chain::MessageKey { match self.material { SessionMessageKeyMaterial::Seed(seed) => { - let seed: [u8; SYMMETRIC_KEY_BYTES] = seed - .try_into() - .map_err(|_| invalid("session message-key seed", "32 bytes"))?; - Ok(MessageKeyGenerator::new_from_seed(&seed, self.index).into_pb()) + MessageKeyGenerator::new_from_seed(&seed, self.index).into_pb() } SessionMessageKeyMaterial::Derived { cipher_key, mac_key, iv, - } => Ok(session_structure::chain::MessageKey { + } => session_structure::chain::MessageKey { index: Some(self.index), - cipher_key: Some(Bytes::from(exact_bytes( - cipher_key, - SYMMETRIC_KEY_BYTES, - "session message cipher key", - )?)), - mac_key: Some(Bytes::from(exact_bytes( - mac_key, - SYMMETRIC_KEY_BYTES, - "session message MAC key", - )?)), - iv: Some(Bytes::from(exact_bytes( - iv, - MESSAGE_IV_BYTES, - "session message IV", - )?)), - }), + cipher_key: Some(Bytes::copy_from_slice(&cipher_key)), + mac_key: Some(Bytes::copy_from_slice(&mac_key)), + iv: Some(Bytes::copy_from_slice(&iv)), + seed: None, + }, } } - fn from_structure(value: session_structure::chain::MessageKey) -> Result { + fn from_structure(mut value: session_structure::chain::MessageKey) -> Result { + let index = value + .index + .ok_or_else(|| invalid("session message-key index", "present"))?; + // A persisted seed supersedes the derived triple on export rather than + // complementing it, so one that does not reproduce that triple would + // hand a consumer a key decrypting nothing, undetectably. Both are + // written from the same material; disagreeing means the record is + // corrupt. + if let Some(seed) = value.seed.take() { + let seed = key_material(seed, "session message-key seed")?; + let derived = MessageKeys::derive_keys(&seed, None, index); + if value.cipher_key.as_deref() != Some(derived.cipher_key()) + || value.mac_key.as_deref() != Some(derived.mac_key()) + || value.iv.as_deref() != Some(derived.iv()) + { + return Err(invalid( + "session message-key seed", + "consistent with the derived keys stored beside it", + )); + } + return Ok(Self { + index, + material: SessionMessageKeyMaterial::Seed(seed), + }); + } Ok(Self { - index: value - .index - .ok_or_else(|| invalid("session message-key index", "present"))?, + index, material: SessionMessageKeyMaterial::Derived { - cipher_key: exact_bytes( - value - .cipher_key - .ok_or_else(|| invalid("session message cipher key", "present"))? - .to_vec(), - SYMMETRIC_KEY_BYTES, - "session message cipher key", - )?, - mac_key: exact_bytes( - value - .mac_key - .ok_or_else(|| invalid("session message MAC key", "present"))? - .to_vec(), - SYMMETRIC_KEY_BYTES, - "session message MAC key", - )?, - iv: exact_bytes( - value - .iv - .ok_or_else(|| invalid("session message IV", "present"))? - .to_vec(), - MESSAGE_IV_BYTES, - "session message IV", - )?, + cipher_key: required_key_material(value.cipher_key, "session message cipher key")?, + mac_key: required_key_material(value.mac_key, "session message MAC key")?, + iv: required_key_material(value.iv, "session message IV")?, }, }) } @@ -437,7 +442,7 @@ impl SessionChainComponents { .message_keys .into_iter() .map(SessionMessageKeyComponents::into_structure) - .collect::>()?, + .collect(), }) } @@ -464,7 +469,7 @@ impl SessionChainComponents { .message_keys .into_iter() .map(SessionMessageKeyComponents::into_structure) - .collect::>()?, + .collect(), }) } @@ -931,9 +936,9 @@ mod tests { message_keys: vec![SessionMessageKeyComponents { index: 1, material: SessionMessageKeyMaterial::Derived { - cipher_key: vec![16; 32], - mac_key: vec![17; 32], - iv: vec![18; 16], + cipher_key: [16; 32], + mac_key: [17; 32], + iv: [18; 16], }, }], }), @@ -947,9 +952,9 @@ mod tests { message_keys: vec![SessionMessageKeyComponents { index: 3, material: SessionMessageKeyMaterial::Derived { - cipher_key: vec![19; 32], - mac_key: vec![20; 32], - iv: vec![21; 16], + cipher_key: [19; 32], + mac_key: [20; 32], + iv: [21; 16], }, }], }], @@ -1037,9 +1042,9 @@ mod tests { key: Some(vec![42; 32]), }; let material = SessionMessageKeyMaterial::Derived { - cipher_key: vec![1; 32], - mac_key: vec![2; 32], - iv: vec![3; 16], + cipher_key: [1; 32], + mac_key: [2; 32], + iv: [3; 16], }; assert_eq!( @@ -1058,23 +1063,81 @@ mod tests { let expected = MessageKeyGenerator::new_from_seed(&seed, 17).into_pb(); let actual = SessionMessageKeyComponents { index: 17, - material: SessionMessageKeyMaterial::Seed(seed.to_vec()), + material: SessionMessageKeyMaterial::Seed(seed), } - .into_structure() - .expect("valid seed"); + .into_structure(); assert_eq!(actual, expected); } + /// The symptom this field exists for: a record imported with a seed used + /// to come back as `Derived`, which no seed-based format can express. + #[test] + fn seed_material_survives_the_record_round_trip() { + let expected = SessionMessageKeyComponents { + index: 17, + material: SessionMessageKeyMaterial::Seed([9; SYMMETRIC_KEY_BYTES]), + }; + let actual = SessionMessageKeyComponents::from_structure(expected.clone().into_structure()) + .expect("valid persisted key"); + + assert_eq!(actual, expected); + } + + /// Keys persisted before the seed was retained carry only the derived + /// triple and must keep projecting as `Derived`. + #[test] + fn a_persisted_key_without_a_seed_projects_as_derived() { + let mut persisted = SessionMessageKeyComponents { + index: 3, + material: SessionMessageKeyMaterial::Seed([9; SYMMETRIC_KEY_BYTES]), + } + .into_structure(); + persisted.seed = None; + + let actual = + SessionMessageKeyComponents::from_structure(persisted).expect("seedless key projects"); + + assert!(matches!( + actual.material, + SessionMessageKeyMaterial::Derived { .. } + )); + } + + /// The seed supersedes the derived triple, so a corrupt one must fail the + /// projection instead of exporting key material that derives nothing. + #[test] + fn a_malformed_persisted_seed_is_rejected() { + for length in [0, SYMMETRIC_KEY_BYTES - 1, SYMMETRIC_KEY_BYTES + 1] { + let mut persisted = SessionMessageKeyComponents { + index: 3, + material: SessionMessageKeyMaterial::Seed([9; SYMMETRIC_KEY_BYTES]), + } + .into_structure(); + persisted.seed = Some(Bytes::from(vec![0; length])); + + let error = SessionMessageKeyComponents::from_structure(persisted) + .expect_err("malformed seed must fail"); + assert!( + matches!(error, SignalProtocolError::InvalidArgument(_)), + "{length}-byte seed: {error}" + ); + } + } + + /// A well-formed seed that derives something else is the dangerous case: + /// the export would look fine and the exported key would decrypt nothing. #[test] - fn invalid_seed_length_is_rejected() { - let error = SessionMessageKeyComponents { - index: 1, - material: SessionMessageKeyMaterial::Seed(vec![0; 31]), + fn a_persisted_seed_that_derives_other_keys_is_rejected() { + let mut persisted = SessionMessageKeyComponents { + index: 3, + material: SessionMessageKeyMaterial::Seed([9; SYMMETRIC_KEY_BYTES]), } - .into_structure() - .expect_err("short seed must fail"); + .into_structure(); + persisted.seed = Some(Bytes::copy_from_slice(&[10; SYMMETRIC_KEY_BYTES])); + let error = SessionMessageKeyComponents::from_structure(persisted) + .expect_err("inconsistent seed must fail"); assert!(matches!(error, SignalProtocolError::InvalidArgument(_))); } diff --git a/wacore/libsignal/src/protocol/state/session.rs b/wacore/libsignal/src/protocol/state/session.rs index 84d7ee092..18dc28b12 100644 --- a/wacore/libsignal/src/protocol/state/session.rs +++ b/wacore/libsignal/src/protocol/state/session.rs @@ -732,9 +732,11 @@ impl From<&SessionState> for SessionStructure { /// Record-level field number carrying the sender-chain counter reservation in /// the serialized `RecordStructure`. The upstream (whatspec) proto cannot be -/// edited to add local fields, so the record encoder — already hand-rolled in -/// [`SessionRecord::serialize_into`] — writes it directly; the number sits far -/// above RecordStructure's fields (1, 2) so a future upstream addition cannot +/// edited to add local fields — `waproto/build.rs` splices those into the +/// descriptor instead, via `LOCAL_FIELDS` — but this one is written directly +/// by the record encoder, already hand-rolled in +/// [`SessionRecord::serialize_into`]; the number sits far above +/// RecordStructure's fields (1, 2) so a future upstream addition cannot /// collide. Standard unknown-field skipping keeps old readers compatible. /// /// That compatibility is one-way: a build without lease support silently @@ -1651,6 +1653,7 @@ mod tests { cipher_key: Some(vec![seed.wrapping_add(idx); 32].into()), mac_key: Some(vec![seed.wrapping_add(idx).wrapping_add(1); 32].into()), iv: Some(vec![seed.wrapping_add(idx).wrapping_add(2); 16].into()), + seed: Some(vec![seed.wrapping_add(idx).wrapping_add(3); 32].into()), } }) .collect(); @@ -1886,6 +1889,40 @@ mod tests { MessageKeyGenerator::new_from_seed(&seed, counter) } + /// The seed is additive: a record written before it existed must still + /// deserialize and hand back exactly the keys it stored. + #[test] + fn seedless_persisted_message_keys_still_load_and_decrypt() { + let base_key = KeyPair::generate(&mut rng()).public_key; + let mut state = create_test_session_state(3, &base_key); + let sender_key = KeyPair::generate(&mut rng()).public_key; + state.add_receiver_chain(&sender_key, &ChainKey::new([7u8; 32], 0)); + state + .set_message_keys(&sender_key, create_test_message_key_generator(5)) + .expect("skipped key stored"); + let expected = create_test_message_key_generator(5).generate_keys(); + + let mut structure = SessionStructure::from(&state); + assert!(structure.receiver_chains[0].message_keys[0].seed.is_some()); + structure.receiver_chains[0].message_keys[0].seed = None; + let bytes = SessionRecord::new(SessionState::from(structure)) + .serialize() + .expect("serialize"); + + let mut record = SessionRecord::deserialize(&bytes).expect("deserialize"); + let keys = record + .session_state_mut() + .expect("current state") + .get_message_keys(&sender_key, 5) + .expect("valid session") + .expect("skipped key survives") + .generate_keys(); + + assert_eq!(keys.cipher_key(), expected.cipher_key()); + assert_eq!(keys.mac_key(), expected.mac_key()); + assert_eq!(keys.iv(), expected.iv()); + } + #[test] fn test_receiver_chain_lookup_by_bytes() { let base_key = KeyPair::generate(&mut rng()).public_key; diff --git a/wacore/libsignal/tests/legacy_session_skipped_keys.rs b/wacore/libsignal/tests/legacy_session_skipped_keys.rs new file mode 100644 index 000000000..539dcfbf1 --- /dev/null +++ b/wacore/libsignal/tests/legacy_session_skipped_keys.rs @@ -0,0 +1,327 @@ +//! A session that skipped a message key must still round-trip through the v1 +//! model. The skipped key is produced the only way it can be produced: an +//! out-of-order delivery over the real encrypt/decrypt APIs. +//! Async I/O uses `futures::executor::block_on` (no tokio in this crate). +#![cfg(feature = "legacy-session-interop")] + +use async_trait::async_trait; +use std::collections::HashMap; +use wacore_libsignal::protocol::{ + CiphertextMessage, Direction, GenericSignedPreKey, IdentityChange, IdentityKey, + IdentityKeyPair, IdentityKeyStore, KeyPair, LegacySessionLocalContext, PreKeyBundle, PreKeyId, + PreKeyRecord, PreKeyStore, ProtocolAddress, SessionRecord, SessionStore, SignalProtocolError, + SignedPreKeyId, SignedPreKeyRecord, SignedPreKeyStore, Timestamp, UsePQRatchet, + message_decrypt, message_encrypt, process_prekey_bundle, +}; + +type Result = wacore_libsignal::protocol::error::Result; + +// ---- in-memory stores, kept local so this test file is self-contained ------ + +#[derive(Clone)] +struct InMemoryIdentityKeyStore { + identity_key_pair: IdentityKeyPair, + registration_id: u32, + identities: HashMap, +} + +#[async_trait] +impl IdentityKeyStore for InMemoryIdentityKeyStore { + async fn get_identity_key_pair(&self) -> Result { + Ok(self.identity_key_pair.clone()) + } + async fn get_local_registration_id(&self) -> Result { + Ok(self.registration_id) + } + async fn save_identity( + &mut self, + address: &ProtocolAddress, + identity: &IdentityKey, + ) -> Result { + let changed = self + .identities + .get(address) + .is_some_and(|prev| prev != identity); + self.identities.insert(address.clone(), *identity); + Ok(IdentityChange::from_changed(changed)) + } + async fn is_trusted_identity( + &self, + _: &ProtocolAddress, + _: &IdentityKey, + _: Direction, + ) -> Result { + Ok(true) + } + async fn get_identity(&self, address: &ProtocolAddress) -> Result> { + Ok(self.identities.get(address).cloned()) + } +} + +#[derive(Default, Clone)] +struct InMemoryPreKeyStore(HashMap); + +#[async_trait] +impl PreKeyStore for InMemoryPreKeyStore { + async fn get_pre_key(&self, id: PreKeyId) -> Result { + self.0 + .get(&id) + .cloned() + .ok_or(SignalProtocolError::InvalidPreKeyId) + } + async fn save_pre_key(&mut self, id: PreKeyId, record: &PreKeyRecord) -> Result<()> { + self.0.insert(id, record.clone()); + Ok(()) + } + async fn remove_pre_key(&mut self, id: PreKeyId) -> Result<()> { + self.0.remove(&id); + Ok(()) + } +} + +#[derive(Default, Clone)] +struct InMemorySignedPreKeyStore(HashMap); + +#[async_trait] +impl SignedPreKeyStore for InMemorySignedPreKeyStore { + async fn get_signed_pre_key(&self, id: SignedPreKeyId) -> Result { + self.0 + .get(&id) + .cloned() + .ok_or(SignalProtocolError::InvalidSignedPreKeyId) + } + async fn save_signed_pre_key( + &mut self, + id: SignedPreKeyId, + record: &SignedPreKeyRecord, + ) -> Result<()> { + self.0.insert(id, record.clone()); + Ok(()) + } +} + +#[derive(Default, Clone)] +struct InMemorySessionStore(HashMap); + +#[async_trait] +impl SessionStore for InMemorySessionStore { + async fn load_session(&self, address: &ProtocolAddress) -> Result> { + Ok(self.0.get(address).cloned()) + } + async fn has_session(&self, address: &ProtocolAddress) -> Result { + Ok(self.0.contains_key(address)) + } + async fn store_session( + &mut self, + address: &ProtocolAddress, + record: SessionRecord, + ) -> Result<()> { + self.0.insert(address.clone(), record); + Ok(()) + } +} + +// ---- peer fixture ---------------------------------------------------------- + +struct Peer { + address: ProtocolAddress, + identity_store: InMemoryIdentityKeyStore, + prekey_store: InMemoryPreKeyStore, + signed_prekey_store: InMemorySignedPreKeyStore, + session_store: InMemorySessionStore, + prekey_id: PreKeyId, + prekey_pair: KeyPair, + signed_prekey_id: SignedPreKeyId, + signed_prekey_pair: KeyPair, + signed_prekey_signature: Vec, +} + +impl Peer { + fn new(name: &str, device_id: u32) -> Self { + let mut rng = rand::make_rng::(); + + let identity_key_pair = IdentityKeyPair::generate(&mut rng); + let registration_id = rand::random::() & 0x3FFF; + + let prekey_id: PreKeyId = 1u32.into(); + let prekey_pair = KeyPair::generate(&mut rng); + let prekey_record = PreKeyRecord::new(prekey_id, &prekey_pair); + + let signed_prekey_id: SignedPreKeyId = 1u32.into(); + let signed_prekey_pair = KeyPair::generate(&mut rng); + let signed_prekey_signature = identity_key_pair + .private_key() + .calculate_signature(&signed_prekey_pair.public_key.serialize(), &mut rng) + .expect("sign"); + let signed_prekey_record = SignedPreKeyRecord::new( + signed_prekey_id, + Timestamp::from_epoch_millis(0), + &signed_prekey_pair, + &signed_prekey_signature, + ); + + let mut prekey_store = InMemoryPreKeyStore::default(); + let mut signed_prekey_store = InMemorySignedPreKeyStore::default(); + futures::executor::block_on(async { + prekey_store + .save_pre_key(prekey_id, &prekey_record) + .await + .expect("save prekey"); + signed_prekey_store + .save_signed_pre_key(signed_prekey_id, &signed_prekey_record) + .await + .expect("save signed prekey"); + }); + + Self { + address: ProtocolAddress::new(name, device_id.into()), + identity_store: InMemoryIdentityKeyStore { + identity_key_pair, + registration_id, + identities: HashMap::new(), + }, + prekey_store, + signed_prekey_store, + session_store: InMemorySessionStore::default(), + prekey_id, + prekey_pair, + signed_prekey_id, + signed_prekey_pair, + signed_prekey_signature: signed_prekey_signature.to_vec(), + } + } + + fn bundle(&self) -> PreKeyBundle { + PreKeyBundle::new( + self.identity_store.registration_id, + 1u32.into(), + Some((self.prekey_id, self.prekey_pair.public_key)), + self.signed_prekey_id, + self.signed_prekey_pair.public_key, + self.signed_prekey_signature.clone(), + *self.identity_store.identity_key_pair.identity_key(), + ) + .expect("valid bundle") + } + + fn legacy_context(&self) -> LegacySessionLocalContext { + LegacySessionLocalContext { + identity_key: *self.identity_store.identity_key_pair.identity_key(), + registration_id: self.identity_store.registration_id, + } + } +} + +// ---- helpers --------------------------------------------------------------- + +fn send(from: &mut Peer, to: &ProtocolAddress, plaintext: &[u8]) -> CiphertextMessage { + futures::executor::block_on(message_encrypt( + plaintext, + to, + &mut from.session_store, + &mut from.identity_store, + )) + .expect("encrypt") +} + +fn receive( + to: &mut Peer, + from: &ProtocolAddress, + ct: &CiphertextMessage, +) -> std::result::Result, SignalProtocolError> { + let mut rng = rand::make_rng::(); + futures::executor::block_on(message_decrypt( + ct, + from, + &mut to.session_store, + &mut to.identity_store, + &mut to.prekey_store, + &to.signed_prekey_store, + &mut rng, + UsePQRatchet::No, + )) + .map(|decrypted| decrypted.plaintext) +} + +fn establish(alice: &mut Peer, bob: &mut Peer) { + let bundle = bob.bundle(); + let mut rng = rand::make_rng::(); + futures::executor::block_on(process_prekey_bundle( + &bob.address, + &mut alice.session_store, + &mut alice.identity_store, + &bundle, + &mut rng, + UsePQRatchet::No, + )) + .expect("prekey bundle accepted"); + + let ct = send(alice, &bob.address, b"hello bob"); + assert_eq!( + receive(bob, &alice.address, &ct).expect("first pkmsg decrypts"), + b"hello bob" + ); +} + +/// Export `peer`'s session with `remote` to the v1 model and put the reimported +/// record back in its place, so everything after this runs on state that made +/// the full trip. +fn cycle_through_v1(peer: &mut Peer, remote: &ProtocolAddress) { + let context = peer.legacy_context(); + futures::executor::block_on(async { + let record = peer + .session_store + .load_session(remote) + .await + .expect("load") + .expect("session exists"); + let reimported = record + .into_legacy_session_v1_operational() + .expect("v1 projection") + .into_session_record(context) + .expect("v1 import"); + peer.session_store + .store_session(remote, reimported) + .await + .expect("store"); + }); +} + +// ---- scenario -------------------------------------------------------------- + +/// The full cycle. Before the seed was persisted, the second projection failed +/// with `ChainNotRepresentable`: the skipped key had only its derived material +/// left, and the v1 model has nowhere to put that. +#[test] +fn a_skipped_key_survives_a_full_v1_projection_cycle() { + let mut alice = Peer::new("alice", 1); + let mut bob = Peer::new("bob", 1); + establish(&mut alice, &mut bob); + + // Bob answers, so Alice's session leaves its pending-prekey state and gains + // a receiver chain to skip a key on. + let opener = send(&mut bob, &alice.address, b"hi alice"); + assert_eq!( + receive(&mut alice, &bob.address, &opener).expect("decrypt"), + b"hi alice" + ); + cycle_through_v1(&mut alice, &bob.address); + + // Out-of-order delivery: the second message arrives first, so decrypting it + // derives and retains the key for the first one. + let skipped = send(&mut bob, &alice.address, b"m0"); + let delivered = send(&mut bob, &alice.address, b"m1"); + assert_eq!( + receive(&mut alice, &bob.address, &delivered).expect("decrypt out of order"), + b"m1" + ); + + cycle_through_v1(&mut alice, &bob.address); + + // The seed made the round trip only if it still derives the key the skipped + // message was encrypted with. + assert_eq!( + receive(&mut alice, &bob.address, &skipped).expect("skipped message decrypts"), + b"m0" + ); +} diff --git a/waproto/build.rs b/waproto/build.rs index 2a1769222..a6bd07c21 100644 --- a/waproto/build.rs +++ b/waproto/build.rs @@ -14,9 +14,43 @@ //! codegen time (word boundaries match heck/prost), so the Rust API keeps the //! prost-style names. Attribute/override paths below therefore use the //! *proto* (camelCase) field names. +//! +//! Fields this crate persists but upstream does not declare go in +//! [`LOCAL_FIELDS`], never in the `.proto`. use buffa::Message as _; -use buffa_descriptor::generated::descriptor::{DescriptorProto, FileDescriptorSet}; +use buffa_descriptor::generated::descriptor::{ + DescriptorProto, FieldDescriptorProto, FileDescriptorSet, field_descriptor_proto, +}; + +/// A field this crate persists that the upstream proto does not declare. +struct LocalField { + /// Message path inside the `whatsapp` package, e.g. `Outer.Inner`. + message: &'static str, + name: &'static str, + number: i32, + kind: field_descriptor_proto::Type, +} + +/// Local additions to the upstream schema, spliced into the descriptor at +/// build time. `src/whatsapp.proto` and `src/whatsapp.desc` are regenerated +/// wholesale from whatspec, so a field declared there would be lost on the +/// next sync; declared here it survives, and a sync that lands on one of these +/// numbers fails the build instead of silently reinterpreting records already +/// written. +/// +/// Numbers stay far above what upstream uses — it appends low ones without +/// notice, the way `kyberPreKeyId = 4` and `kyberCiphertext = 5` arrived on +/// `SessionStructure.PendingPreKey`. +const LOCAL_FIELDS: &[LocalField] = &[LocalField { + // Deriving the stored cipher/mac/iv from this seed is one-way, so a + // skipped message key that kept only the derived material could never be + // projected back into a seed-based external format. + message: "SessionStructure.Chain.MessageKey", + name: "seed", + number: 100, + kind: field_descriptor_proto::Type::TYPE_BYTES, +}]; fn main() -> std::io::Result<()> { // Rerun on desc change (new codegen) and proto change (so the staleness @@ -31,15 +65,28 @@ fn main() -> std::io::Result<()> { let out_dir = std::env::var("OUT_DIR").expect("OUT_DIR must be set by cargo"); let out_path = std::path::PathBuf::from(&out_dir); - // Emit the wire-tag consts (field numbers) for hand-written partial decoders. // Build-time descriptor decode — nothing to pin in waproto::codec. #[allow(clippy::disallowed_methods)] - let fds = FileDescriptorSet::decode_from_slice(&std::fs::read("src/whatsapp.desc")?) + let mut fds = FileDescriptorSet::decode_from_slice(&std::fs::read("src/whatsapp.desc")?) .map_err(std::io::Error::other)?; + apply_local_fields(&mut fds)?; + + // Emit the wire-tag consts (field numbers) for hand-written partial decoders. generate_tags(&fds, &out_path.join("tags.rs"))?; + // Codegen reads the spliced descriptor, so local fields reach the Rust API + // with the same guarantees as upstream ones. `buffa_build` registers this + // path as rerun-if-changed, so rewriting it unconditionally would bump its + // mtime on every build and leave the crate permanently dirty. + let descriptor = out_path.join("whatsapp.local.desc"); + #[allow(clippy::disallowed_methods)] + let encoded = fds.encode_to_vec(); + if !std::fs::read(&descriptor).is_ok_and(|current| current == encoded) { + std::fs::write(&descriptor, &encoded)?; + } + buffa_build::Config::new() - .descriptor_set("src/whatsapp.desc") + .descriptor_set(&descriptor) .files(&["whatsapp.proto"]) // snake_case Rust idents from the upstream camelCase proto (see module // docs); wire format and descriptor names are untouched. @@ -136,6 +183,10 @@ fn main() -> std::io::Result<()> { ".whatsapp.SessionStructure.Chain.MessageKey.iv", "#[serde(skip)]", ) + .field_attribute( + ".whatsapp.SessionStructure.Chain.MessageKey.seed", + "#[serde(skip)]", + ) .field_attribute( ".whatsapp.SenderKeyStateStructure.SenderChainKey.seed", "#[serde(skip)]", @@ -166,6 +217,61 @@ fn main() -> std::io::Result<()> { Ok(()) } +/// Splice [`LOCAL_FIELDS`] into the descriptor decoded from `whatsapp.desc`. +/// +/// Fails rather than resolves when upstream already occupies the name or the +/// number: silently winning that race would change what an existing field +/// decodes as, and every record already on disk with it. +fn apply_local_fields(fds: &mut FileDescriptorSet) -> std::io::Result<()> { + for local in LOCAL_FIELDS { + let mut messages = fds + .file + .iter_mut() + .filter(|file| file.package.as_deref() == Some("whatsapp")) + .flat_map(|file| file.message_type.iter_mut()); + let mut message = None; + for segment in local.message.split('.') { + message = match message { + None => messages.find(|msg| msg.name.as_deref() == Some(segment)), + Some(parent) => { + let parent: &mut DescriptorProto = parent; + parent + .nested_type + .iter_mut() + .find(|msg| msg.name.as_deref() == Some(segment)) + } + }; + if message.is_none() { + break; + } + } + let message = message.ok_or_else(|| { + std::io::Error::other(format!("local field target {} not found", local.message)) + })?; + + if let Some(existing) = message + .field + .iter() + .find(|f| f.number == Some(local.number) || f.name.as_deref() == Some(local.name)) + { + return Err(std::io::Error::other(format!( + "local field {}.{} = {} collides with upstream {:?} = {:?}", + local.message, local.name, local.number, existing.name, existing.number + ))); + } + + message.field.push(FieldDescriptorProto { + name: Some(local.name.to_owned()), + number: Some(local.number), + label: Some(field_descriptor_proto::Label::LABEL_OPTIONAL), + r#type: Some(local.kind), + json_name: Some(local.name.to_owned()), + ..Default::default() + }); + } + Ok(()) +} + /// Emit `tags.rs`: a nested module tree mirroring the proto's message /// hierarchy, with one `pub const : u32 = ;` per field. Reads /// the original (camelCase) descriptor; const/module names go through