From abdb0d27b62500559f873cb88bccf50678944366 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 20:49:32 +0000 Subject: [PATCH 1/6] feat(libsignal): let a consumer provide the X25519 agreement SignalCryptoProvider already lets a consumer supply AES-256-CBC, AES-256-GCM, HMAC-SHA256 and the transport AEAD, but the X25519 agreement was wired straight to the in-crate implementation, so it could not be routed anywhere. That gap is what blocks a consumer whose requirement is that every primitive run inside one approved module, and it also puts a platform's faster X25519 out of reach while the same consumer can already reach its faster AES. The trait gains x25519_agreement over raw 32-byte keys, with a default that runs the same code the agreement ran before, so existing trait implementations keep compiling and the default build produces the same bytes. Routing happens at PrivateKey::calculate_agreement, the one place the agreement is decided: keeping it there means the ratchet and X3DH call sites stay untouched and there is a single point to keep in sync with the provider. --- wacore/libsignal/src/core/curve.rs | 88 +++- wacore/libsignal/src/crypto/mod.rs | 7 + wacore/libsignal/src/crypto/provider.rs | 106 ++++- .../tests/crypto_provider_x25519_agreement.rs | 409 ++++++++++++++++++ 4 files changed, 605 insertions(+), 5 deletions(-) create mode 100644 wacore/libsignal/tests/crypto_provider_x25519_agreement.rs diff --git a/wacore/libsignal/src/core/curve.rs b/wacore/libsignal/src/core/curve.rs index 981546e6d..7c40c377e 100644 --- a/wacore/libsignal/src/core/curve.rs +++ b/wacore/libsignal/src/core/curve.rs @@ -516,15 +516,28 @@ impl PrivateKey { pub fn calculate_agreement(&self, their_key: &PublicKey) -> Result<[u8; 32], CurveError> { match (&self.key, their_key.key) { + // The single place the agreement is decided, so a consumer that + // supplies the primitive replaces it for every caller at once. (PrivateKeyData::DjbPrivateKey { key, .. }, PublicKeyData::DjbPublicKey(pub_key)) => { - // Use from_bytes_without_cache since agreement doesn't need the Edwards cache - let private_key = curve25519::PrivateKey::from_bytes_without_cache(*key); - Ok(private_key.calculate_agreement(&pub_key)) + Ok(crate::crypto::x25519_agreement(key, &pub_key)) } } } } +/// This crate's own X25519 agreement, and the only copy of that path: it is +/// what [`SignalCryptoProvider::x25519_agreement`] runs by default. +/// +/// [`SignalCryptoProvider::x25519_agreement`]: crate::crypto::SignalCryptoProvider::x25519_agreement +pub(crate) fn x25519_agreement( + private_key: &[u8; curve25519::PRIVATE_KEY_LENGTH], + their_public_key: &[u8; curve25519::PUBLIC_KEY_LENGTH], +) -> [u8; 32] { + // from_bytes_without_cache because agreement never reads the Edwards cache. + curve25519::PrivateKey::from_bytes_without_cache(*private_key) + .calculate_agreement(their_public_key) +} + impl TryFrom<&[u8]> for PrivateKey { type Error = CurveError; @@ -799,6 +812,75 @@ mod tests { } } + fn key_bytes(hex_key: &str) -> [u8; 32] { + let mut out = [0u8; 32]; + hex::decode_to_slice(hex_key, &mut out).expect("32 hex-encoded bytes"); + out + } + + /// RFC 7748 section 6.1. With no provider installed the agreement runs the + /// library's own path, so this pins the bytes that routing must preserve. + #[test] + fn rfc7748_agreement_vector() { + let alice_private = PrivateKey::deserialize(&key_bytes( + "77076d0a7318a57d3c16c17251b26645df4c2f87ebc0992ab177fba51db92c2a", + )) + .expect("alice private key"); + let bob_private = PrivateKey::deserialize(&key_bytes( + "5dab087e624a8a4b79e17f8b83800ee66f3bb1292618b6fd1c2f8b27ff88e0eb", + )) + .expect("bob private key"); + let alice_public = PublicKey::from_djb_public_key_bytes(&key_bytes( + "8520f0098930a754748b7ddcb43ef75a0dbf3a0d26381af4eba4a98eaa9b4e6a", + )) + .expect("alice public key"); + let bob_public = PublicKey::from_djb_public_key_bytes(&key_bytes( + "de9edb7d7b7dc1b4d35b61c2ece435373f8343c85b78674dadfc7e146f882b4f", + )) + .expect("bob public key"); + let expected = + key_bytes("4a5d9d5ba4ce2de1728e3bf480350f25e07e21c947d19e3376f09b3c1e161742"); + + assert_eq!(alice_private.public_key().expect("derive"), alice_public); + assert_eq!(bob_private.public_key().expect("derive"), bob_public); + assert_eq!( + alice_private.calculate_agreement(&bob_public).expect("dh"), + expected + ); + assert_eq!( + bob_private.calculate_agreement(&alice_public).expect("dh"), + expected + ); + } + + /// A key of the wrong length never reaches the agreement at all. + #[test] + fn agreement_rejects_wrong_length_keys() { + assert!(matches!( + PrivateKey::deserialize(&[0x42; 31]), + Err(CurveError::BadKeyLength(KeyType::Djb, 31)) + )); + assert!(matches!( + PublicKey::from_djb_public_key_bytes(&[0x42; 33]), + Err(CurveError::BadKeyLength(KeyType::Djb, 33)) + )); + } + + /// Signing state is expensive and the agreement has no use for it; routing + /// must not start building it behind our back. + #[test] + fn agreement_leaves_the_signing_cache_cold() { + let mut csprng = rng(); + let alice = KeyPair::generate(&mut csprng); + let bob = KeyPair::generate(&mut csprng); + + alice + .calculate_agreement(&bob.public_key) + .expect("agreement"); + + assert!(!alice.private_key.has_warm_signing_cache()); + } + #[test] fn test_key_agreement_works_without_signing() { let mut csprng = rng(); diff --git a/wacore/libsignal/src/crypto/mod.rs b/wacore/libsignal/src/crypto/mod.rs index 5ca546e61..b7a06e52d 100644 --- a/wacore/libsignal/src/crypto/mod.rs +++ b/wacore/libsignal/src/crypto/mod.rs @@ -74,6 +74,13 @@ pub fn hmac_sha256_two_part(key: &[u8], first: &[u8], second: &[u8]) -> [u8; 32] provider::provider().hmac_sha256_two_part(key, first, second) } +/// X25519 key agreement over raw 32-byte keys. Delegates to the active +/// [`SignalCryptoProvider`], whose default is this crate's own implementation. +#[inline] +pub fn x25519_agreement(private_key: &[u8; 32], their_public_key: &[u8; 32]) -> [u8; 32] { + provider::provider().x25519_agreement(private_key, their_public_key) +} + /// In-place AES-256-GCM seal. On entry `buffer` holds the plaintext; on return /// it holds `ciphertext || tag` (length grown by 16). Zero allocations with /// the default [`RustCryptoProvider`]. diff --git a/wacore/libsignal/src/crypto/provider.rs b/wacore/libsignal/src/crypto/provider.rs index 62a2cdf6b..8920a4fec 100644 --- a/wacore/libsignal/src/crypto/provider.rs +++ b/wacore/libsignal/src/crypto/provider.rs @@ -1,8 +1,9 @@ //! Pluggable Signal crypto provider. //! //! Default uses RustCrypto (soft). Override via [`set_crypto_provider`] to -//! delegate hot-path primitives to a faster backend (e.g. `node:crypto` -//! over a WASM bridge). Must be set before any crypto call. +//! delegate the primitives to another backend: AES-256-CBC, AES-256-GCM, +//! HMAC-SHA256, the transport AEAD and the X25519 agreement. Must be set +//! before any crypto call, key agreement included. use std::sync::OnceLock; @@ -220,6 +221,17 @@ pub trait SignalCryptoProvider: Send + Sync + 'static { self.hmac_sha256(key, &input) } + /// X25519 key agreement over the raw 32-byte keys as this crate stores + /// them; an implementation clamps the private key itself, as X25519 + /// requires (clamping is idempotent, so repeating it is safe). Overriding + /// this replaces the primitive for every agreement the crate performs. + /// + /// The default is this crate's own implementation, so a provider that + /// leaves it alone keeps today's result byte for byte. + fn x25519_agreement(&self, private_key: &[u8; 32], their_public_key: &[u8; 32]) -> [u8; 32] { + crate::core::curve::x25519_agreement(private_key, their_public_key) + } + /// In-place AES-256-GCM seal. On entry `buffer` holds the plaintext; on /// return it holds `ciphertext || tag` (length grown by 16). /// @@ -574,6 +586,96 @@ mod tests { ); } + /// A provider written before the agreement joined the trait: it implements + /// only the required methods and must keep agreeing keys correctly. + struct SymmetricOnlyProvider; + + impl SignalCryptoProvider for SymmetricOnlyProvider { + fn aes_256_cbc_encrypt( + &self, + key: &[u8; 32], + iv: &[u8; 16], + plaintext: &[u8], + out: &mut Vec, + ) -> Result<(), CryptoProviderError> { + RustCryptoProvider.aes_256_cbc_encrypt(key, iv, plaintext, out) + } + + fn aes_256_cbc_decrypt( + &self, + key: &[u8; 32], + iv: &[u8; 16], + ciphertext: &[u8], + out: &mut Vec, + ) -> Result<(), CryptoProviderError> { + RustCryptoProvider.aes_256_cbc_decrypt(key, iv, ciphertext, out) + } + + fn aes_256_gcm_encrypt( + &self, + key: &[u8; 32], + nonce: &[u8; 12], + aad: &[u8], + plaintext: &[u8], + out: &mut Vec, + ) -> Result<(), CryptoProviderError> { + RustCryptoProvider.aes_256_gcm_encrypt(key, nonce, aad, plaintext, out) + } + + fn aes_256_gcm_decrypt( + &self, + key: &[u8; 32], + nonce: &[u8; 12], + aad: &[u8], + ciphertext_with_tag: &[u8], + out: &mut Vec, + ) -> Result<(), CryptoProviderError> { + RustCryptoProvider.aes_256_gcm_decrypt(key, nonce, aad, ciphertext_with_tag, out) + } + + fn hmac_sha256(&self, key: &[u8], input: &[u8]) -> [u8; 32] { + RustCryptoProvider.hmac_sha256(key, input) + } + } + + /// RFC 7748 section 6.1, reached through the trait default. + #[test] + fn provider_without_agreement_override_keeps_the_pure_path() { + let alice_private = [ + 0x77, 0x07, 0x6d, 0x0a, 0x73, 0x18, 0xa5, 0x7d, 0x3c, 0x16, 0xc1, 0x72, 0x51, 0xb2, + 0x66, 0x45, 0xdf, 0x4c, 0x2f, 0x87, 0xeb, 0xc0, 0x99, 0x2a, 0xb1, 0x77, 0xfb, 0xa5, + 0x1d, 0xb9, 0x2c, 0x2a, + ]; + let bob_public = [ + 0xde, 0x9e, 0xdb, 0x7d, 0x7b, 0x7d, 0xc1, 0xb4, 0xd3, 0x5b, 0x61, 0xc2, 0xec, 0xe4, + 0x35, 0x37, 0x3f, 0x83, 0x43, 0xc8, 0x5b, 0x78, 0x67, 0x4d, 0xad, 0xfc, 0x7e, 0x14, + 0x6f, 0x88, 0x2b, 0x4f, + ]; + let expected = [ + 0x4a, 0x5d, 0x9d, 0x5b, 0xa4, 0xce, 0x2d, 0xe1, 0x72, 0x8e, 0x3b, 0xf4, 0x80, 0x35, + 0x0f, 0x25, 0xe0, 0x7e, 0x21, 0xc9, 0x47, 0xd1, 0x9e, 0x33, 0x76, 0xf0, 0x9b, 0x3c, + 0x1e, 0x16, 0x17, 0x42, + ]; + + assert_eq!( + SymmetricOnlyProvider.x25519_agreement(&alice_private, &bob_public), + expected + ); + assert_eq!( + RustCryptoProvider.x25519_agreement(&alice_private, &bob_public), + expected + ); + } + + /// A public key of low order agrees to the all-zero secret; the primitive + /// reports it as bytes rather than failing, and callers must not read that + /// as a usable secret. + #[test] + fn provider_agreement_with_low_order_point_is_all_zero() { + let agreement = RustCryptoProvider.x25519_agreement(&[0x42; 32], &[0u8; 32]); + assert_eq!(agreement, [0u8; 32]); + } + /// NIST SP 800-38D Test Case 14: all-zero key/nonce, 128-bit plaintext. #[test] fn rust_provider_gcm_nist_tc14() { diff --git a/wacore/libsignal/tests/crypto_provider_x25519_agreement.rs b/wacore/libsignal/tests/crypto_provider_x25519_agreement.rs new file mode 100644 index 000000000..6dfc2dd58 --- /dev/null +++ b/wacore/libsignal/tests/crypto_provider_x25519_agreement.rs @@ -0,0 +1,409 @@ +//! Proof that the X25519 agreement reaches the installed crypto provider. +//! Its own integration binary because `set_crypto_provider` writes a +//! process-wide global the rest of the suite must not observe. +//! Async I/O uses `futures::executor::block_on` (no tokio in this crate). + +use async_trait::async_trait; +use std::collections::HashMap; +use std::sync::Once; +use std::sync::atomic::{AtomicUsize, Ordering}; +use wacore_libsignal::crypto::{ + CryptoProviderError, RustCryptoProvider, SignalCryptoProvider, set_crypto_provider, +}; +use wacore_libsignal::protocol::{ + CiphertextMessage, Direction, GenericSignedPreKey, IdentityChange, IdentityKey, + IdentityKeyPair, IdentityKeyStore, KeyPair, PreKeyBundle, PreKeyId, PreKeyRecord, PreKeyStore, + ProtocolAddress, SessionRecord, SessionStore, SignalProtocolError, SignedPreKeyId, + SignedPreKeyRecord, SignedPreKeyStore, Timestamp, UsePQRatchet, message_decrypt, + message_encrypt, process_prekey_bundle, +}; + +// ---- the provider under test ------------------------------------------------ + +static AGREEMENTS: AtomicUsize = AtomicUsize::new(0); + +/// Answers agreements with a secret that depends on the argument order, so the +/// two sides of a session derive different roots: the same shape of damage a +/// mismatched external implementation would cause. +fn substituted_agreement(private_key: &[u8; 32], their_public_key: &[u8; 32]) -> [u8; 32] { + RustCryptoProvider.hmac_sha256(private_key, their_public_key) +} + +struct SubstitutedAgreementProvider; + +impl SignalCryptoProvider for SubstitutedAgreementProvider { + fn aes_256_cbc_encrypt( + &self, + key: &[u8; 32], + iv: &[u8; 16], + plaintext: &[u8], + out: &mut Vec, + ) -> Result<(), CryptoProviderError> { + RustCryptoProvider.aes_256_cbc_encrypt(key, iv, plaintext, out) + } + + fn aes_256_cbc_decrypt( + &self, + key: &[u8; 32], + iv: &[u8; 16], + ciphertext: &[u8], + out: &mut Vec, + ) -> Result<(), CryptoProviderError> { + RustCryptoProvider.aes_256_cbc_decrypt(key, iv, ciphertext, out) + } + + fn aes_256_gcm_encrypt( + &self, + key: &[u8; 32], + nonce: &[u8; 12], + aad: &[u8], + plaintext: &[u8], + out: &mut Vec, + ) -> Result<(), CryptoProviderError> { + RustCryptoProvider.aes_256_gcm_encrypt(key, nonce, aad, plaintext, out) + } + + fn aes_256_gcm_decrypt( + &self, + key: &[u8; 32], + nonce: &[u8; 12], + aad: &[u8], + ciphertext_with_tag: &[u8], + out: &mut Vec, + ) -> Result<(), CryptoProviderError> { + RustCryptoProvider.aes_256_gcm_decrypt(key, nonce, aad, ciphertext_with_tag, out) + } + + fn hmac_sha256(&self, key: &[u8], input: &[u8]) -> [u8; 32] { + RustCryptoProvider.hmac_sha256(key, input) + } + + fn x25519_agreement(&self, private_key: &[u8; 32], their_public_key: &[u8; 32]) -> [u8; 32] { + AGREEMENTS.fetch_add(1, Ordering::Relaxed); + substituted_agreement(private_key, their_public_key) + } +} + +/// The global takes one writer, and both tests share this binary when the +/// runner keeps them in a single process. +fn install_provider() { + static ONCE: Once = Once::new(); + ONCE.call_once(|| { + set_crypto_provider(SubstitutedAgreementProvider).expect("provider installs first"); + }); +} + +// ---- in-memory stores (same fixtures the other test binaries keep local) ----- + +#[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, + ) -> wacore_libsignal::protocol::error::Result { + Ok(self.identity_key_pair.clone()) + } + async fn get_local_registration_id(&self) -> wacore_libsignal::protocol::error::Result { + Ok(self.registration_id) + } + async fn save_identity( + &mut self, + address: &ProtocolAddress, + identity: &IdentityKey, + ) -> wacore_libsignal::protocol::error::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, + ) -> wacore_libsignal::protocol::error::Result { + Ok(true) + } + async fn get_identity( + &self, + address: &ProtocolAddress, + ) -> wacore_libsignal::protocol::error::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, + ) -> wacore_libsignal::protocol::error::Result { + self.0 + .get(&id) + .cloned() + .ok_or(SignalProtocolError::InvalidPreKeyId) + } + async fn save_pre_key( + &mut self, + id: PreKeyId, + record: &PreKeyRecord, + ) -> wacore_libsignal::protocol::error::Result<()> { + self.0.insert(id, record.clone()); + Ok(()) + } + async fn remove_pre_key( + &mut self, + id: PreKeyId, + ) -> wacore_libsignal::protocol::error::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, + ) -> wacore_libsignal::protocol::error::Result { + self.0 + .get(&id) + .cloned() + .ok_or(SignalProtocolError::InvalidSignedPreKeyId) + } + async fn save_signed_pre_key( + &mut self, + id: SignedPreKeyId, + record: &SignedPreKeyRecord, + ) -> wacore_libsignal::protocol::error::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, + ) -> wacore_libsignal::protocol::error::Result> { + Ok(self.0.get(address).cloned()) + } + async fn has_session( + &self, + address: &ProtocolAddress, + ) -> wacore_libsignal::protocol::error::Result { + Ok(self.0.contains_key(address)) + } + async fn store_session( + &mut self, + address: &ProtocolAddress, + record: SessionRecord, + ) -> wacore_libsignal::protocol::error::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 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("store prekey"); + signed_prekey_store + .save_signed_pre_key(signed_prekey_id, &signed_prekey_record) + .await + .expect("store signed prekey"); + }); + + Self { + address: ProtocolAddress::new(name, device_id.into()), + identity_store: InMemoryIdentityKeyStore { + identity_key_pair, + registration_id: 1234, + 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 send(from: &mut Peer, to: &ProtocolAddress, plaintext: &[u8]) -> CiphertextMessage { + futures::executor::block_on(async { + message_encrypt( + plaintext, + to, + &mut from.session_store, + &mut from.identity_store, + ) + .await + .expect("encrypt") + }) +} + +fn receive( + to: &mut Peer, + from: &ProtocolAddress, + ct: &CiphertextMessage, +) -> Result, SignalProtocolError> { + let mut rng = rand::make_rng::(); + futures::executor::block_on(async { + message_decrypt( + ct, + from, + &mut to.session_store, + &mut to.identity_store, + &mut to.prekey_store, + &to.signed_prekey_store, + &mut rng, + UsePQRatchet::No, + ) + .await + .map(|d| d.plaintext) + }) +} + +// ---- scenarios -------------------------------------------------------------- + +/// The installed provider answers the agreement, and its answer is what the +/// key types hand back. +#[test] +fn agreement_is_routed_to_the_installed_provider() { + install_provider(); + + let mut rng = rand::make_rng::(); + let alice = KeyPair::generate(&mut rng); + let bob = KeyPair::generate(&mut rng); + + let before = AGREEMENTS.load(Ordering::Relaxed); + let agreement = alice + .calculate_agreement(&bob.public_key) + .expect("agreement"); + + let bob_public: [u8; 32] = bob + .public_key + .public_key_bytes() + .try_into() + .expect("djb public key"); + assert!(AGREEMENTS.load(Ordering::Relaxed) > before); + assert_eq!( + agreement, + substituted_agreement(alice.private_key.serialize(), &bob_public) + ); + assert_ne!( + agreement, + bob.calculate_agreement(&alice.public_key) + .expect("agreement") + ); +} + +/// A provider whose agreement is inconsistent between the two sides breaks the +/// session at the MAC, as a typed error rather than a panic. +#[test] +fn inconsistent_agreement_fails_the_session_with_a_typed_error() { + install_provider(); + + let mut alice = Peer::new("alice-provider", 1); + let mut bob = Peer::new("bob-provider", 1); + + let bundle = bob.bundle(); + let mut rng = rand::make_rng::(); + futures::executor::block_on(async { + process_prekey_bundle( + &bob.address, + &mut alice.session_store, + &mut alice.identity_store, + &bundle, + &mut rng, + UsePQRatchet::No, + ) + .await + .expect("bundle accepted"); + }); + + let ct = send(&mut alice, &bob.address, b"unreadable"); + let err = receive(&mut bob, &alice.address, &ct).expect_err("roots cannot match"); + assert!( + matches!( + err, + SignalProtocolError::InvalidMessage(..) | SignalProtocolError::BadMac(..) + ), + "expected a typed decrypt failure, got {err:?}" + ); +} From a8117b7dba420105325196255406a907bb039fea Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 20:55:18 +0000 Subject: [PATCH 2/6] test(libsignal): build the provider fixture bundle from the peer device id --- wacore/libsignal/tests/crypto_provider_x25519_agreement.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/wacore/libsignal/tests/crypto_provider_x25519_agreement.rs b/wacore/libsignal/tests/crypto_provider_x25519_agreement.rs index 6dfc2dd58..2747a1560 100644 --- a/wacore/libsignal/tests/crypto_provider_x25519_agreement.rs +++ b/wacore/libsignal/tests/crypto_provider_x25519_agreement.rs @@ -293,7 +293,7 @@ impl Peer { fn bundle(&self) -> PreKeyBundle { PreKeyBundle::new( self.identity_store.registration_id, - 1u32.into(), + self.address.device_id(), Some((self.prekey_id, self.prekey_pair.public_key)), self.signed_prekey_id, self.signed_prekey_pair.public_key, From e69916de0716571f4fc909f9038519e2434a61a2 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 22:44:16 +0000 Subject: [PATCH 3/6] feat(libsignal): let the X25519 agreement report a backend failure A substituted agreement can be backed by something that refuses the operation, and an infallible return left such a provider two bad options: panic, or answer with fabricated bytes that a session would then build a root key from, surfacing much later as a MAC failure that points nowhere near the cause. x25519_agreement now returns Result. The trait default is still this crate's own implementation and still cannot fail, so a build without a custom provider is unchanged and existing implementations keep compiling. The error travels the path calculate_agreement already had: a new CurveError variant carrying the provider error, mapped onto a SignalProtocolError variant of its own, so a caller can tell "the backend refused" from "the peer key was malformed". --- wacore/libsignal/src/core/curve.rs | 5 +- wacore/libsignal/src/crypto/mod.rs | 8 +++- wacore/libsignal/src/crypto/provider.rs | 29 +++++++++--- wacore/libsignal/src/protocol/error.rs | 5 ++ .../tests/crypto_provider_x25519_agreement.rs | 47 +++++++++++++++++-- 5 files changed, 80 insertions(+), 14 deletions(-) diff --git a/wacore/libsignal/src/core/curve.rs b/wacore/libsignal/src/core/curve.rs index 7c40c377e..7181701b6 100644 --- a/wacore/libsignal/src/core/curve.rs +++ b/wacore/libsignal/src/core/curve.rs @@ -41,6 +41,9 @@ pub enum CurveError { BadKeyType(u8), #[error("bad key length <{1}> for key with type <{0}>")] BadKeyLength(KeyType, usize), + /// Only a substituted agreement can produce this: the default never fails. + #[error("the active crypto provider failed the key agreement")] + AgreementFailed(#[from] crate::crypto::CryptoProviderError), } impl TryFrom for KeyType { @@ -519,7 +522,7 @@ impl PrivateKey { // The single place the agreement is decided, so a consumer that // supplies the primitive replaces it for every caller at once. (PrivateKeyData::DjbPrivateKey { key, .. }, PublicKeyData::DjbPublicKey(pub_key)) => { - Ok(crate::crypto::x25519_agreement(key, &pub_key)) + Ok(crate::crypto::x25519_agreement(key, &pub_key)?) } } } diff --git a/wacore/libsignal/src/crypto/mod.rs b/wacore/libsignal/src/crypto/mod.rs index b7a06e52d..3f22257ec 100644 --- a/wacore/libsignal/src/crypto/mod.rs +++ b/wacore/libsignal/src/crypto/mod.rs @@ -75,9 +75,13 @@ pub fn hmac_sha256_two_part(key: &[u8], first: &[u8], second: &[u8]) -> [u8; 32] } /// X25519 key agreement over raw 32-byte keys. Delegates to the active -/// [`SignalCryptoProvider`], whose default is this crate's own implementation. +/// [`SignalCryptoProvider`], whose default is this crate's own implementation +/// and never fails; only a substituted backend can return an error. #[inline] -pub fn x25519_agreement(private_key: &[u8; 32], their_public_key: &[u8; 32]) -> [u8; 32] { +pub fn x25519_agreement( + private_key: &[u8; 32], + their_public_key: &[u8; 32], +) -> std::result::Result<[u8; 32], CryptoProviderError> { provider::provider().x25519_agreement(private_key, their_public_key) } diff --git a/wacore/libsignal/src/crypto/provider.rs b/wacore/libsignal/src/crypto/provider.rs index 8920a4fec..e37240a04 100644 --- a/wacore/libsignal/src/crypto/provider.rs +++ b/wacore/libsignal/src/crypto/provider.rs @@ -226,10 +226,19 @@ pub trait SignalCryptoProvider: Send + Sync + 'static { /// requires (clamping is idempotent, so repeating it is safe). Overriding /// this replaces the primitive for every agreement the crate performs. /// - /// The default is this crate's own implementation, so a provider that - /// leaves it alone keeps today's result byte for byte. - fn x25519_agreement(&self, private_key: &[u8; 32], their_public_key: &[u8; 32]) -> [u8; 32] { - crate::core::curve::x25519_agreement(private_key, their_public_key) + /// A backend that can fail reports it here instead of panicking or + /// answering with fabricated bytes, which would let a session advance on + /// key material nobody agreed to. The default is this crate's own + /// implementation: it cannot fail, and keeps today's result byte for byte. + fn x25519_agreement( + &self, + private_key: &[u8; 32], + their_public_key: &[u8; 32], + ) -> Result<[u8; 32], CryptoProviderError> { + Ok(crate::core::curve::x25519_agreement( + private_key, + their_public_key, + )) } /// In-place AES-256-GCM seal. On entry `buffer` holds the plaintext; on @@ -658,11 +667,15 @@ mod tests { ]; assert_eq!( - SymmetricOnlyProvider.x25519_agreement(&alice_private, &bob_public), + SymmetricOnlyProvider + .x25519_agreement(&alice_private, &bob_public) + .expect("the default cannot fail"), expected ); assert_eq!( - RustCryptoProvider.x25519_agreement(&alice_private, &bob_public), + RustCryptoProvider + .x25519_agreement(&alice_private, &bob_public) + .expect("the default cannot fail"), expected ); } @@ -672,7 +685,9 @@ mod tests { /// as a usable secret. #[test] fn provider_agreement_with_low_order_point_is_all_zero() { - let agreement = RustCryptoProvider.x25519_agreement(&[0x42; 32], &[0u8; 32]); + let agreement = RustCryptoProvider + .x25519_agreement(&[0x42; 32], &[0u8; 32]) + .expect("the default cannot fail"); assert_eq!(agreement, [0u8; 32]); } diff --git a/wacore/libsignal/src/protocol/error.rs b/wacore/libsignal/src/protocol/error.rs index d6768c832..d6125b923 100644 --- a/wacore/libsignal/src/protocol/error.rs +++ b/wacore/libsignal/src/protocol/error.rs @@ -10,6 +10,7 @@ use crate::{ ProtocolAddress, curve::{CurveError, KeyType}, }, + crypto::CryptoProviderError, protocol::CiphertextMessageType, }; use thiserror::Error; @@ -56,6 +57,9 @@ pub enum SignalProtocolError { #[error("invalid signature detected")] SignatureValidationFailed, + #[error("the active crypto provider failed the key agreement")] + KeyAgreementFailed(#[source] CryptoProviderError), + #[error("untrusted identity for address {0}")] UntrustedIdentity(ProtocolAddress), @@ -108,6 +112,7 @@ impl From for SignalProtocolError { CurveError::NoKeyTypeIdentifier => Self::NoKeyTypeIdentifier, CurveError::BadKeyType(raw) => Self::BadKeyType(raw), CurveError::BadKeyLength(key_type, len) => Self::BadKeyLength(key_type, len), + CurveError::AgreementFailed(source) => Self::KeyAgreementFailed(source), } } } diff --git a/wacore/libsignal/tests/crypto_provider_x25519_agreement.rs b/wacore/libsignal/tests/crypto_provider_x25519_agreement.rs index 2747a1560..71307cf13 100644 --- a/wacore/libsignal/tests/crypto_provider_x25519_agreement.rs +++ b/wacore/libsignal/tests/crypto_provider_x25519_agreement.rs @@ -11,9 +11,9 @@ use wacore_libsignal::crypto::{ CryptoProviderError, RustCryptoProvider, SignalCryptoProvider, set_crypto_provider, }; use wacore_libsignal::protocol::{ - CiphertextMessage, Direction, GenericSignedPreKey, IdentityChange, IdentityKey, + CiphertextMessage, CurveError, Direction, GenericSignedPreKey, IdentityChange, IdentityKey, IdentityKeyPair, IdentityKeyStore, KeyPair, PreKeyBundle, PreKeyId, PreKeyRecord, PreKeyStore, - ProtocolAddress, SessionRecord, SessionStore, SignalProtocolError, SignedPreKeyId, + ProtocolAddress, PublicKey, SessionRecord, SessionStore, SignalProtocolError, SignedPreKeyId, SignedPreKeyRecord, SignedPreKeyStore, Timestamp, UsePQRatchet, message_decrypt, message_encrypt, process_prekey_bundle, }; @@ -29,6 +29,10 @@ fn substituted_agreement(private_key: &[u8; 32], their_public_key: &[u8; 32]) -> RustCryptoProvider.hmac_sha256(private_key, their_public_key) } +/// Peer key that makes the provider below report a backend failure, standing in +/// for the moment an external module refuses the operation. +const UNAVAILABLE_PEER_KEY: [u8; 32] = [0xee; 32]; + struct SubstitutedAgreementProvider; impl SignalCryptoProvider for SubstitutedAgreementProvider { @@ -78,9 +82,16 @@ impl SignalCryptoProvider for SubstitutedAgreementProvider { RustCryptoProvider.hmac_sha256(key, input) } - fn x25519_agreement(&self, private_key: &[u8; 32], their_public_key: &[u8; 32]) -> [u8; 32] { + fn x25519_agreement( + &self, + private_key: &[u8; 32], + their_public_key: &[u8; 32], + ) -> Result<[u8; 32], CryptoProviderError> { AGREEMENTS.fetch_add(1, Ordering::Relaxed); - substituted_agreement(private_key, their_public_key) + if their_public_key == &UNAVAILABLE_PEER_KEY { + return Err(CryptoProviderError::BackendFailed); + } + Ok(substituted_agreement(private_key, their_public_key)) } } @@ -407,3 +418,31 @@ fn inconsistent_agreement_fails_the_session_with_a_typed_error() { "expected a typed decrypt failure, got {err:?}" ); } + +/// A backend that refuses the operation surfaces as an error the caller can +/// match on, all the way up to the protocol error type, and never as bytes the +/// session would then build keys from. +#[test] +fn provider_backend_failure_surfaces_as_a_typed_error() { + install_provider(); + + let mut rng = rand::make_rng::(); + let ours = KeyPair::generate(&mut rng); + let unavailable = + PublicKey::from_djb_public_key_bytes(&UNAVAILABLE_PEER_KEY).expect("peer key"); + + let err = ours + .calculate_agreement(&unavailable) + .expect_err("the backend refused"); + assert!( + matches!( + err, + CurveError::AgreementFailed(CryptoProviderError::BackendFailed) + ), + "expected a typed agreement failure, got {err:?}" + ); + assert!(matches!( + SignalProtocolError::from(err), + SignalProtocolError::KeyAgreementFailed(CryptoProviderError::BackendFailed) + )); +} From 1240a050aa67e553f25b92e58d8a8de8820ae38a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 22:56:43 +0000 Subject: [PATCH 4/6] fix(libsignal): keep a refused agreement out of the decrypt verdict The candidate-session search collects each state's failure and reports one verdict for the message: duplicate, bad MAC, or "decryption failed". A backend that refuses the agreement mid-ratchet was landing in that pile and coming out as InvalidMessage, so the caller would ask the peer to resend a message that was never corrupt, and the real cause would be invisible. A refused agreement is not evidence about the message: every sibling state would ask the same backend and get the same answer. Return it as itself, the way an already-consumed body already does. Unreachable without a custom provider, since the default agreement cannot fail. --- .../libsignal/src/protocol/session_cipher.rs | 12 + .../crypto_provider_agreement_failure.rs | 404 ++++++++++++++++++ 2 files changed, 416 insertions(+) create mode 100644 wacore/libsignal/tests/crypto_provider_agreement_failure.rs diff --git a/wacore/libsignal/src/protocol/session_cipher.rs b/wacore/libsignal/src/protocol/session_cipher.rs index ef31edccd..e1c364850 100644 --- a/wacore/libsignal/src/protocol/session_cipher.rs +++ b/wacore/libsignal/src/protocol/session_cipher.rs @@ -1091,6 +1091,14 @@ fn decrypt_message_with_record<'a, R: Rng + CryptoRng>( record.set_session_state(current_state); return Err(error); } + Err(e @ SignalProtocolError::KeyAgreementFailed(_)) => { + // A refused agreement says nothing about this message: every + // other session would ask the same backend and get the same + // answer, and collapsing it into the aggregate verdict below + // would have the caller treat a live message as corrupt. + record.set_session_state(current_state); + return Err(e); + } Err(e) if !ciphertext.is_available() => { // Authentication succeeded, but the provider rejected the // caller-owned body after it had been consumed for in-place @@ -1200,6 +1208,10 @@ fn decrypt_message_with_record<'a, R: Rng + CryptoRng>( record.restore_previous_session(idx, previous); return Err(error); } + Err(e @ SignalProtocolError::KeyAgreementFailed(_)) => { + record.restore_previous_session(idx, previous); + return Err(e); + } Err(e) if !ciphertext.is_available() => { record.restore_previous_session(idx, previous); return Err(e); diff --git a/wacore/libsignal/tests/crypto_provider_agreement_failure.rs b/wacore/libsignal/tests/crypto_provider_agreement_failure.rs new file mode 100644 index 000000000..6aa5da228 --- /dev/null +++ b/wacore/libsignal/tests/crypto_provider_agreement_failure.rs @@ -0,0 +1,404 @@ +//! A backend refusal during a DH ratchet must reach the caller as itself. +//! Its own integration binary because `set_crypto_provider` writes a +//! process-wide global the rest of the suite must not observe. +//! Async I/O uses `futures::executor::block_on` (no tokio in this crate). + +use async_trait::async_trait; +use std::collections::HashMap; +use std::sync::atomic::{AtomicBool, Ordering}; +use wacore_libsignal::crypto::{ + CryptoProviderError, RustCryptoProvider, SignalCryptoProvider, set_crypto_provider, +}; +use wacore_libsignal::protocol::{ + CiphertextMessage, Direction, GenericSignedPreKey, IdentityChange, IdentityKey, + IdentityKeyPair, IdentityKeyStore, KeyPair, PreKeyBundle, PreKeyId, PreKeyRecord, PreKeyStore, + ProtocolAddress, SessionRecord, SessionStore, SignalProtocolError, SignedPreKeyId, + SignedPreKeyRecord, SignedPreKeyStore, Timestamp, UsePQRatchet, message_decrypt, + message_encrypt, process_prekey_bundle, +}; + +// ---- the provider under test ------------------------------------------------ + +/// Trips the agreement into a backend failure. Off while the session is built, +/// so everything up to that point is the real primitive. +static REFUSING: AtomicBool = AtomicBool::new(false); + +struct FusedAgreementProvider; + +impl SignalCryptoProvider for FusedAgreementProvider { + fn aes_256_cbc_encrypt( + &self, + key: &[u8; 32], + iv: &[u8; 16], + plaintext: &[u8], + out: &mut Vec, + ) -> Result<(), CryptoProviderError> { + RustCryptoProvider.aes_256_cbc_encrypt(key, iv, plaintext, out) + } + + fn aes_256_cbc_decrypt( + &self, + key: &[u8; 32], + iv: &[u8; 16], + ciphertext: &[u8], + out: &mut Vec, + ) -> Result<(), CryptoProviderError> { + RustCryptoProvider.aes_256_cbc_decrypt(key, iv, ciphertext, out) + } + + fn aes_256_gcm_encrypt( + &self, + key: &[u8; 32], + nonce: &[u8; 12], + aad: &[u8], + plaintext: &[u8], + out: &mut Vec, + ) -> Result<(), CryptoProviderError> { + RustCryptoProvider.aes_256_gcm_encrypt(key, nonce, aad, plaintext, out) + } + + fn aes_256_gcm_decrypt( + &self, + key: &[u8; 32], + nonce: &[u8; 12], + aad: &[u8], + ciphertext_with_tag: &[u8], + out: &mut Vec, + ) -> Result<(), CryptoProviderError> { + RustCryptoProvider.aes_256_gcm_decrypt(key, nonce, aad, ciphertext_with_tag, out) + } + + fn hmac_sha256(&self, key: &[u8], input: &[u8]) -> [u8; 32] { + RustCryptoProvider.hmac_sha256(key, input) + } + + fn x25519_agreement( + &self, + private_key: &[u8; 32], + their_public_key: &[u8; 32], + ) -> Result<[u8; 32], CryptoProviderError> { + if REFUSING.load(Ordering::SeqCst) { + return Err(CryptoProviderError::BackendFailed); + } + RustCryptoProvider.x25519_agreement(private_key, their_public_key) + } +} + +// ---- in-memory stores (same fixtures the other test binaries keep local) ----- + +#[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, + ) -> wacore_libsignal::protocol::error::Result { + Ok(self.identity_key_pair.clone()) + } + async fn get_local_registration_id(&self) -> wacore_libsignal::protocol::error::Result { + Ok(self.registration_id) + } + async fn save_identity( + &mut self, + address: &ProtocolAddress, + identity: &IdentityKey, + ) -> wacore_libsignal::protocol::error::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, + ) -> wacore_libsignal::protocol::error::Result { + Ok(true) + } + async fn get_identity( + &self, + address: &ProtocolAddress, + ) -> wacore_libsignal::protocol::error::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, + ) -> wacore_libsignal::protocol::error::Result { + self.0 + .get(&id) + .cloned() + .ok_or(SignalProtocolError::InvalidPreKeyId) + } + async fn save_pre_key( + &mut self, + id: PreKeyId, + record: &PreKeyRecord, + ) -> wacore_libsignal::protocol::error::Result<()> { + self.0.insert(id, record.clone()); + Ok(()) + } + async fn remove_pre_key( + &mut self, + id: PreKeyId, + ) -> wacore_libsignal::protocol::error::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, + ) -> wacore_libsignal::protocol::error::Result { + self.0 + .get(&id) + .cloned() + .ok_or(SignalProtocolError::InvalidSignedPreKeyId) + } + async fn save_signed_pre_key( + &mut self, + id: SignedPreKeyId, + record: &SignedPreKeyRecord, + ) -> wacore_libsignal::protocol::error::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, + ) -> wacore_libsignal::protocol::error::Result> { + Ok(self.0.get(address).cloned()) + } + async fn has_session( + &self, + address: &ProtocolAddress, + ) -> wacore_libsignal::protocol::error::Result { + Ok(self.0.contains_key(address)) + } + async fn store_session( + &mut self, + address: &ProtocolAddress, + record: SessionRecord, + ) -> wacore_libsignal::protocol::error::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 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("store prekey"); + signed_prekey_store + .save_signed_pre_key(signed_prekey_id, &signed_prekey_record) + .await + .expect("store signed prekey"); + }); + + Self { + address: ProtocolAddress::new(name, device_id.into()), + identity_store: InMemoryIdentityKeyStore { + identity_key_pair, + registration_id: 1234, + 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, + self.address.device_id(), + 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 send(from: &mut Peer, to: &ProtocolAddress, plaintext: &[u8]) -> CiphertextMessage { + futures::executor::block_on(async { + message_encrypt( + plaintext, + to, + &mut from.session_store, + &mut from.identity_store, + ) + .await + .expect("encrypt") + }) +} + +fn receive( + to: &mut Peer, + from: &ProtocolAddress, + ct: &CiphertextMessage, +) -> Result, SignalProtocolError> { + let mut rng = rand::make_rng::(); + futures::executor::block_on(async { + message_decrypt( + ct, + from, + &mut to.session_store, + &mut to.identity_store, + &mut to.prekey_store, + &to.signed_prekey_store, + &mut rng, + UsePQRatchet::No, + ) + .await + .map(|d| d.plaintext) + }) +} + +// ---- scenario --------------------------------------------------------------- + +/// One test per binary: the fuse is process-wide state, and a second test +/// running beside it would see the flipped one. +/// +/// A message that carries a fresh ratchet key makes the receiver agree keys +/// mid-decrypt. With the backend refusing, that has to arrive as +/// `KeyAgreementFailed` and not as the "no session could read it" verdict the +/// candidate search otherwise reports, which would have the caller ask the peer +/// to resend a message that was never corrupt. +#[test] +fn a_refused_agreement_survives_the_decrypt_candidate_search() { + set_crypto_provider(FusedAgreementProvider).expect("provider installs first"); + + let mut alice = Peer::new("alice-fused", 1); + let mut bob = Peer::new("bob-fused", 1); + + let bundle = bob.bundle(); + let mut rng = rand::make_rng::(); + futures::executor::block_on(async { + process_prekey_bundle( + &bob.address, + &mut alice.session_store, + &mut alice.identity_store, + &bundle, + &mut rng, + UsePQRatchet::No, + ) + .await + .expect("bundle accepted"); + }); + + // Ping-pong until both sides hold a live session, all on the real primitive. + let opening = send(&mut alice, &bob.address, b"hello bob"); + assert_eq!( + receive(&mut bob, &alice.address, &opening).expect("pkmsg decrypts"), + b"hello bob" + ); + let reply = send(&mut bob, &alice.address, b"hello alice"); + assert_eq!( + receive(&mut alice, &bob.address, &reply).expect("reply decrypts"), + b"hello alice" + ); + let follow_up = send(&mut alice, &bob.address, b"still here"); + assert_eq!( + receive(&mut bob, &alice.address, &follow_up).expect("follow-up decrypts"), + b"still here" + ); + + // Bob's next send opens a new sending ratchet, so Alice has to agree keys + // to read it. + let ratcheted = send(&mut bob, &alice.address, b"new ratchet"); + + REFUSING.store(true, Ordering::SeqCst); + let err = receive(&mut alice, &bob.address, &ratcheted).expect_err("the backend refused"); + REFUSING.store(false, Ordering::SeqCst); + + assert!( + matches!( + err, + SignalProtocolError::KeyAgreementFailed(CryptoProviderError::BackendFailed) + ), + "expected the backend failure to survive, got {err:?}" + ); + + // With the backend back, the same message reads normally: the failure was + // never a property of the message. + assert_eq!( + receive(&mut alice, &bob.address, &ratcheted).expect("decrypts once the backend returns"), + b"new ratchet" + ); +} From ae73eaca79c7dde0ac6bebe551d359defc90e2a4 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 23:01:24 +0000 Subject: [PATCH 5/6] test(libsignal): cover the archived-session half of the refusal path The first half of the test returns from the current-state arm, so the candidate loop over archived sessions never ran. Archiving the live session leaves no current state, which makes that loop the only path, and the message still decrypts afterwards: the session the loop borrowed has to come back where it was. --- .../crypto_provider_agreement_failure.rs | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/wacore/libsignal/tests/crypto_provider_agreement_failure.rs b/wacore/libsignal/tests/crypto_provider_agreement_failure.rs index 6aa5da228..a867a412f 100644 --- a/wacore/libsignal/tests/crypto_provider_agreement_failure.rs +++ b/wacore/libsignal/tests/crypto_provider_agreement_failure.rs @@ -401,4 +401,52 @@ fn a_refused_agreement_survives_the_decrypt_candidate_search() { receive(&mut alice, &bob.address, &ratcheted).expect("decrypts once the backend returns"), b"new ratchet" ); + + // Same refusal, reached through the archived-session half of the search: + // with no current state the candidate loop is the only path, and the + // session it borrowed has to be back in place afterwards. + let after = send(&mut alice, &bob.address, b"after recovery"); + assert_eq!( + receive(&mut bob, &alice.address, &after).expect("decrypts"), + b"after recovery" + ); + let for_the_archive = send(&mut bob, &alice.address, b"read me from the archive"); + + { + let record = alice + .session_store + .0 + .get_mut(&bob.address) + .expect("alice has a session"); + record.archive_current_state().expect("archive"); + assert!(record.session_state().is_none()); + assert_eq!(record.previous_session_count(), 1); + } + + REFUSING.store(true, Ordering::SeqCst); + let err = receive(&mut alice, &bob.address, &for_the_archive).expect_err("the backend refused"); + REFUSING.store(false, Ordering::SeqCst); + + assert!( + matches!( + err, + SignalProtocolError::KeyAgreementFailed(CryptoProviderError::BackendFailed) + ), + "expected the backend failure to survive the archived-session search, got {err:?}" + ); + assert_eq!( + alice + .session_store + .0 + .get(&bob.address) + .expect("record") + .previous_session_count(), + 1, + "the archived session must still be there after the refusal" + ); + assert_eq!( + receive(&mut alice, &bob.address, &for_the_archive) + .expect("the archived session still reads it"), + b"read me from the archive" + ); } From 892e4a41adf585861b5c0ff7b5dab706aca2fe8a Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 23:13:21 +0000 Subject: [PATCH 6/6] fix(libsignal): let the candidate search outlive a refused agreement Returning the refusal on the spot was too eager. The backend is asked with the candidate's own ratchet key, and a sibling session that already has an open receiver chain agrees nothing at all, so a message that session could read was being reported as a backend failure and lost. The refusal goes back in the pile and the search continues. It then outranks the MAC-based verdicts when nothing could read the message, so a caller still tells "this backend is down" from "this message is corrupt", and a replay still wins over both. --- .../libsignal/src/protocol/session_cipher.rs | 30 +++++++++------- .../crypto_provider_agreement_failure.rs | 34 +++++++++++++++++++ 2 files changed, 52 insertions(+), 12 deletions(-) diff --git a/wacore/libsignal/src/protocol/session_cipher.rs b/wacore/libsignal/src/protocol/session_cipher.rs index e1c364850..09e081446 100644 --- a/wacore/libsignal/src/protocol/session_cipher.rs +++ b/wacore/libsignal/src/protocol/session_cipher.rs @@ -758,6 +758,18 @@ async fn message_decrypt_signal_inner( Ok((decrypt.commit(), identity_change)) } +/// A refused agreement is a local failure, not a verdict about the message, so +/// it outranks the MAC-based classifications: the caller has to be able to tell +/// "this backend is down" from "this message is corrupt". Kept in the pile +/// rather than returned on the spot, since a sibling session with an open +/// receiver chain needs no agreement and may still read the message. +fn take_key_agreement_failure(errs: &mut Vec) -> Option { + let at = errs + .iter() + .position(|e| matches!(e, SignalProtocolError::KeyAgreementFailed(_)))?; + Some(errs.remove(at)) +} + fn create_decryption_failure_log( remote_address: &ProtocolAddress, mut errs: &[SignalProtocolError], @@ -1091,14 +1103,6 @@ fn decrypt_message_with_record<'a, R: Rng + CryptoRng>( record.set_session_state(current_state); return Err(error); } - Err(e @ SignalProtocolError::KeyAgreementFailed(_)) => { - // A refused agreement says nothing about this message: every - // other session would ask the same backend and get the same - // answer, and collapsing it into the aggregate verdict below - // would have the caller treat a live message as corrupt. - record.set_session_state(current_state); - return Err(e); - } Err(e) if !ciphertext.is_available() => { // Authentication succeeded, but the provider rejected the // caller-owned body after it had been consumed for in-place @@ -1126,6 +1130,9 @@ fn decrypt_message_with_record<'a, R: Rng + CryptoRng>( ciphertext.signal_message() )? ); + if let Some(refused) = take_key_agreement_failure(&mut errs) { + return Err(refused); + } // Preserve BadMac so it maps to WA Web error code 7 in retry receipts. if errs .iter() @@ -1208,10 +1215,6 @@ fn decrypt_message_with_record<'a, R: Rng + CryptoRng>( record.restore_previous_session(idx, previous); return Err(error); } - Err(e @ SignalProtocolError::KeyAgreementFailed(_)) => { - record.restore_previous_session(idx, previous); - return Err(e); - } Err(e) if !ciphertext.is_available() => { record.restore_previous_session(idx, previous); return Err(e); @@ -1261,6 +1264,9 @@ fn decrypt_message_with_record<'a, R: Rng + CryptoRng>( }) { return Err(SignalProtocolError::DuplicatedMessage(chain, counter)); } + if let Some(refused) = take_key_agreement_failure(&mut errs) { + return Err(refused); + } // Otherwise, if any session state produced a BadMac error, propagate it rather // than the generic InvalidMessage. BadMac means at least one state derived a // message key and verified the MAC — it specifically failed, which maps to WA diff --git a/wacore/libsignal/tests/crypto_provider_agreement_failure.rs b/wacore/libsignal/tests/crypto_provider_agreement_failure.rs index a867a412f..2e8beebb6 100644 --- a/wacore/libsignal/tests/crypto_provider_agreement_failure.rs +++ b/wacore/libsignal/tests/crypto_provider_agreement_failure.rs @@ -449,4 +449,38 @@ fn a_refused_agreement_survives_the_decrypt_candidate_search() { .expect("the archived session still reads it"), b"read me from the archive" ); + + // A refusal in one candidate must not end the search: the backend is asked + // with that candidate's own ratchet key, and a sibling with an open + // receiver chain reads the message without agreeing anything. + let first = send(&mut bob, &alice.address, b"same chain, first"); + let second = send(&mut bob, &alice.address, b"same chain, second"); + assert_eq!( + receive(&mut alice, &bob.address, &first).expect("opens the chain"), + b"same chain, first" + ); + + // Rebuilding archives that session and installs a fresh one, which knows + // nothing about Bob's ratchet key and has to agree keys to try. + let fresh_bundle = bob.bundle(); + futures::executor::block_on(async { + process_prekey_bundle( + &bob.address, + &mut alice.session_store, + &mut alice.identity_store, + &fresh_bundle, + &mut rng, + UsePQRatchet::No, + ) + .await + .expect("bundle accepted"); + }); + + REFUSING.store(true, Ordering::SeqCst); + let plaintext = receive(&mut alice, &bob.address, &second); + REFUSING.store(false, Ordering::SeqCst); + assert_eq!( + plaintext.expect("the archived chain needs no agreement"), + b"same chain, second" + ); }