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