fix(libsignal): keep skipped message-key seeds projectable

.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

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 `<Name>Tag`; parsers must dispatch on `<Name>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<T>`, 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.

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<LegacySessionRecordV1, LegacySessionInteropError> {
@@ -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::<Result<Vec<_>, _>>()?;
 
     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 {

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() {