Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions src/features/signal.rs
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,15 @@ impl<'a> Signal<'a> {
)
.await?;

// A pkmsg consumed prekey is reported, not deleted by the decrypt; buffer
// it so the flush below removes it atomically with the promoted session.
if let Some(prekey_id) = decrypted.consumed_prekey_id {
adapter
.pre_key_store
.buffer_consumed_prekey(prekey_id, &signal_addr)
.await;
}

drop(_guard);
self.client.flush_signal_cache().await?;

Expand Down
22 changes: 22 additions & 0 deletions src/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1845,6 +1845,16 @@ impl Client {

match decrypt_res {
Ok(decrypted) => {
// Buffer the prekey this pkmsg consumed: message_decrypt promoted
// the session into the (volatile) cache but no longer deletes the
// prekey itself. The post-loop flush deletes it only once that
// session is durable, keeping a crash from orphaning the prekey.
if let Some(prekey_id) = decrypted.consumed_prekey_id {
adapter
.pre_key_store
.buffer_consumed_prekey(prekey_id, &signal_address)
.await;
}
if decrypted.identity_change == IdentityChange::ReplacedExisting
&& !local_identity_reacted
{
Expand Down Expand Up @@ -1950,6 +1960,12 @@ impl Client {
info.id,
address
);
if let Some(prekey_id) = decrypted.consumed_prekey_id {
adapter
.pre_key_store
.buffer_consumed_prekey(prekey_id, &signal_address)
.await;
}
// Normally NewOrUnchanged here (the untrusted
// identity was deleted+flushed before the retry),
// but mirror the main-decode gate so a concurrent
Expand Down Expand Up @@ -2631,6 +2647,12 @@ impl Client {
info.id,
info.source.sender
);
if let Some(prekey_id) = decrypted.consumed_prekey_id {
adapter
.pre_key_store
.buffer_consumed_prekey(prekey_id, signal_address)
.await;
}
let padded_plaintext = decrypted.plaintext;
match self
.clone()
Expand Down
101 changes: 100 additions & 1 deletion src/store/signal_adapter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,13 +223,33 @@ impl PreKeyStore for PreKeyAdapter {
.map_err(signal_err("backend"))
}
async fn remove_pre_key(&mut self, prekey_id: PreKeyId) -> Result<(), SignalProtocolError> {
// Plain immediate-removal primitive. The inbound pkmsg path does NOT route
// through here: message_decrypt reports the consumed prekey and the receive
// path buffers it via buffer_consumed_prekey so the durable delete is
// atomic with the session flush (matching WAWebSignalProtocolStoreUnifiedApi).
let device = self.0.device.read().await;
WacorePreKeyStore::remove_prekey(&*device, prekey_id.into())
device
.backend
.remove_prekey(prekey_id.into())
.await
.map_err(signal_err("backend"))
}
}

impl PreKeyAdapter {
/// Buffer a consumed one-time prekey for deletion on the next cache flush,
/// keyed by the session address whose pkmsg promotion consumed it. Called by
/// the inbound receive path after `message_decrypt` reports the consumed
/// prekey: the promoted session is still volatile in the cache, so the prekey
/// must only be deleted once that session is durably flushed.
pub async fn buffer_consumed_prekey(&self, prekey_id: PreKeyId, address: &ProtocolAddress) {
self.0
.cache
.remove_prekey(prekey_id.into(), address.as_str())
.await;
}
}

#[cfg_attr(target_arch = "wasm32", async_trait(?Send))]
#[cfg_attr(not(target_arch = "wasm32"), async_trait)]
impl SignedPreKeyStore for SignedPreKeyAdapter {
Expand Down Expand Up @@ -290,3 +310,82 @@ impl wacore::libsignal::protocol::SenderKeyStore for SenderKeyAdapter {
self.0.cache.sender_key_lock(sender_key_name).await
}
}

#[cfg(test)]
mod tests {
use super::*;
use crate::store::Device;
use wacore::store::in_memory::InMemoryBackend;

const PREKEY_ID: u32 = 7777;

/// The inbound decrypt path consumes a one-time prekey and buffers it via
/// `buffer_consumed_prekey`. It must NOT delete the prekey from the backend
/// synchronously: the promoted session is still volatile at that point, so an
/// eager backend delete would lose both on a crash. The removal must only be
/// committed during the session-bearing cache flush.
#[tokio::test]
async fn buffer_consumed_prekey_defers_backend_delete_to_flush() {
let backend: Arc<dyn crate::store::Backend> = Arc::new(InMemoryBackend::new());
backend
.store_prekey(PREKEY_ID, b"durable-prekey", false)
.await
.unwrap();

let device = Arc::new(RwLock::new(Device::new(backend.clone())));
let cache = Arc::new(SignalStoreCache::new());
let adapter = SignalProtocolStoreAdapter::new(device, cache.clone());

let addr = ProtocolAddress::new("bob".to_string(), 1.into());
// The real path stores the promoted session before buffering the prekey.
cache
.put_session(
&addr,
wacore::libsignal::protocol::SessionRecord::new_fresh(),
)
.await;
adapter
.pre_key_store
.buffer_consumed_prekey(PREKEY_ID.into(), &addr)
.await;

// Still durable: the removal was only buffered, not written to the backend.
assert!(
backend.load_prekey(PREKEY_ID).await.unwrap().is_some(),
"buffer_consumed_prekey must not delete from the backend before flush"
);

// The flush commits the session AND the buffered prekey removal together.
cache.flush(backend.as_ref()).await.unwrap();
assert!(
backend.load_prekey(PREKEY_ID).await.unwrap().is_none(),
"flush must commit the buffered prekey removal"
);
}

/// The plain `remove_pre_key` primitive (not used by the inbound consume path)
/// removes immediately from the backend.
#[tokio::test]
async fn remove_pre_key_deletes_immediately() {
let backend: Arc<dyn crate::store::Backend> = Arc::new(InMemoryBackend::new());
backend
.store_prekey(PREKEY_ID, b"durable-prekey", false)
.await
.unwrap();

let device = Arc::new(RwLock::new(Device::new(backend.clone())));
let cache = Arc::new(SignalStoreCache::new());
let mut adapter = SignalProtocolStoreAdapter::new(device, cache.clone());

adapter
.pre_key_store
.remove_pre_key(PREKEY_ID.into())
.await
.unwrap();

assert!(
backend.load_prekey(PREKEY_ID).await.unwrap().is_none(),
"remove_pre_key must delete from the backend immediately"
);
}
}
16 changes: 12 additions & 4 deletions wacore/libsignal/src/protocol/session_cipher.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,13 @@ use crate::protocol::{
pub struct DecryptionResult {
pub plaintext: Vec<u8>,
pub identity_change: IdentityChange,
/// The one-time pre-key a pkmsg consumed, if any. The decrypt does NOT delete
/// it: removing the prekey is the caller's responsibility, and only once the
/// promoted session is itself durable. A crash with the prekey already gone
/// but the session still volatile makes a redelivered pkmsg undecryptable, so
/// the caller buffers this id and deletes it alongside the session flush.
/// `None` for a SignalMessage decrypt or a pkmsg that reused an existing session.
pub consumed_prekey_id: Option<PreKeyId>,
}

pub async fn message_encrypt(
Expand Down Expand Up @@ -295,13 +302,13 @@ pub async fn message_decrypt_prekey<R: Rng + CryptoRng>(

let (plaintext, pre_key_used, identity_change) = result?;

if let Some(pre_key_id) = pre_key_used {
pre_key_store.remove_pre_key(pre_key_id).await?;
}

// The consumed prekey is reported up, not deleted here: the promoted session
// is still volatile in the caller's cache, so the prekey must only be removed
// once that session is durable (see DecryptionResult::consumed_prekey_id).
Ok(DecryptionResult {
plaintext,
identity_change,
consumed_prekey_id: pre_key_used,
})
}

Expand Down Expand Up @@ -406,6 +413,7 @@ pub async fn message_decrypt_signal<R: Rng + CryptoRng>(
Ok(DecryptionResult {
plaintext,
identity_change,
consumed_prekey_id: None,
})
}

Expand Down
Loading
Loading