Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: move decrypt_message() code from CentralContext to ConversationGuard (1/2) [WPB-15583] #951

Merged
merged 14 commits into from
Mar 6, 2025
Merged

refactor: move decrypt_message() code from CentralContext to ConversationGuard (1/2) [WPB-15583] #951

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
e2f74f3
chore: remove leftover from recent refactoring
SimonThormeyer Mar 4, 2025
596def8
refactor: rename `handle_when_group_is_pending()` to `try_process_own…
SimonThormeyer Mar 4, 2025
4e9f751
chore: add new `PendingConversation` struct
SimonThormeyer Mar 4, 2025
42b605a
refactor: implement `try_process_own_join_commit()` on `PendingConver…
SimonThormeyer Mar 4, 2025
6b2b0e6
refactor: move `get_parent_conversation()` to `ConversationGuard`
SimonThormeyer Mar 5, 2025
d2b4077
refactor: move `UnmergedPendingGroup` error to `conversation` module
SimonThormeyer Mar 4, 2025
2d4a445
refactor: move `decrypt_message()` to `ConversationGuard`
SimonThormeyer Mar 4, 2025
7f809ae
refactor: rename internal `UnmergedPendingGroup` error
SimonThormeyer Mar 5, 2025
20157c8
refactor: add error that wraps `PendingConversation`
SimonThormeyer Mar 5, 2025
2622b31
refactor: decouple `decrypt_message()` and `process_own_join_commit()`
SimonThormeyer Mar 5, 2025
a58fb15
refactor: use `decrypt_message()` on `ConversationGuard`
SimonThormeyer Mar 5, 2025
97a7ce5
chore: update test to decoupled buffering for `PendingConversation`
SimonThormeyer Mar 5, 2025
6d16dab
refactor: remove `decrypt_message()` on `CentralContext`
SimonThormeyer Mar 5, 2025
bf626f0
refactor: move code from `external_commit` module to `PendingConversa…
SimonThormeyer Mar 6, 2025
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
19 changes: 14 additions & 5 deletions crypto-ffi/src/generic/context/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ use super::{
use crate::NewCrlDistributionPoints;
use async_lock::{Mutex, OnceCell};
use core_crypto::mls::conversation::Conversation as _;
use core_crypto::mls::conversation::Error as ConversationError;
use core_crypto::{
RecursiveError,
context::CentralContext,
Expand Down Expand Up @@ -481,17 +482,25 @@ impl CoreCryptoContext {
Ok(())
}

/// See [core_crypto::context::CentralContext::decrypt_message]
/// See [core_crypto::mls::conversation::conversation_guard::ConversationGuard::decrypt_message]
pub async fn decrypt_message(
&self,
conversation_id: Vec<u8>,
payload: Vec<u8>,
) -> CoreCryptoResult<DecryptedMessage> {
let raw_decrypted_message = self.context.decrypt_message(&conversation_id, payload).await?;

let decrypted_message: DecryptedMessage = raw_decrypted_message.try_into()?;
let result = self
.context
.conversation_guard(&conversation_id)
.await?
.decrypt_message(&payload)
.await;
let decrypted_message = if let Err(ConversationError::PendingConversation(mut pending)) = result {
pending.try_process_own_join_commit(&payload).await
} else {
result
}?;

Ok(decrypted_message)
decrypted_message.try_into()
}

/// See [core_crypto::mls::conversation::conversation_guard::ConversationGuard::encrypt_message]
Expand Down
5 changes: 4 additions & 1 deletion crypto-ffi/src/generic/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,10 @@ impl From<RecursiveError> for CoreCryptoError {
core_crypto::mls::conversation::Error::BufferedCommit => MlsError::BufferedCommit.into(),
core_crypto::mls::conversation::Error::MessageRejected { reason } => MlsError::MessageRejected { reason: reason.clone() }.into(),
core_crypto::mls::conversation::Error::OrphanWelcome => MlsError::OrphanWelcome.into(),
core_crypto::mls::Error::UnmergedPendingGroup => MlsError::UnmergedPendingGroup.into(),
// The internal name is what we want, but renaming the external variant is a breaking change.
// Since we're re-designing the `BufferedMessage` errors soon, it's not worth producing
// an additional breaking change until then, so the names are inconsistent.
core_crypto::mls::conversation::Error::BufferedForPendingConversation => MlsError::UnmergedPendingGroup.into(),
||=> MlsError::Other(error.innermost_error_message()).into(),
})
}
Expand Down
19 changes: 13 additions & 6 deletions crypto-ffi/src/wasm/context/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ use crate::{
CustomConfiguration, DecryptedMessage, FfiClientId, WasmCryptoResult, WelcomeBundle,
};
use core_crypto::mls::conversation::Conversation as _;
use core_crypto::mls::conversation::Error as ConversationError;
use core_crypto::{
RecursiveError,
context::CentralContext,
Expand Down Expand Up @@ -518,17 +519,23 @@ impl CoreCryptoContext {

/// Returns: [`WasmCryptoResult<DecryptedMessage>`]
///
/// see [core_crypto::mls::context::CentralContext::decrypt_message]
/// see [core_crypto::mls::conversation::conversation_guard::ConversationGuard::decrypt_message]
pub fn decrypt_message(&self, conversation_id: ConversationId, payload: Box<[u8]>) -> Promise {
let context = self.inner.clone();
future_to_promise(
async move {
let raw_decrypted_message = context
.decrypt_message(&conversation_id.to_vec(), payload)
.await
.map_err(CoreCryptoError::from)?;
let result = context
.conversation_guard(&conversation_id)
.await?
.decrypt_message(&payload)
.await;
let decrypted_message = if let Err(ConversationError::PendingConversation(mut pending)) = result {
pending.try_process_own_join_commit(&payload).await
} else {
result
}?;

let decrypted_message = DecryptedMessage::try_from(raw_decrypted_message)?;
let decrypted_message = DecryptedMessage::try_from(decrypted_message)?;
WasmCryptoResult::Ok(serde_wasm_bindgen::to_value(&decrypted_message)?)
}
.err_into(),
Expand Down
5 changes: 4 additions & 1 deletion crypto-ffi/src/wasm/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,7 +302,10 @@ impl From<RecursiveError> for InternalError {
core_crypto::mls::conversation::Error::BufferedCommit => MlsError::BufferedCommit.into(),
core_crypto::mls::conversation::Error::MessageRejected { reason } => MlsError::MessageRejected { reason: reason.clone() }.into(),
core_crypto::mls::conversation::Error::OrphanWelcome => MlsError::OrphanWelcome.into(),
core_crypto::mls::Error::UnmergedPendingGroup => MlsError::UnmergedPendingGroup.into(),
// The internal name is what we want, but renaming the external variant is a breaking change.
// Since we're re-designing the `BufferedMessage` errors soon, it's not worth producing
// an additional breaking change until then, so the names are inconsistent.
core_crypto::mls::conversation::Error::BufferedForPendingConversation => MlsError::UnmergedPendingGroup.into(),
||=> MlsError::Other(error.innermost_error_message()).into(),
})
}
Expand Down
10 changes: 9 additions & 1 deletion crypto/benches/encryption.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,15 @@ fn decryption_bench_var_msg_size(c: &mut Criterion) {
},
|(central, id, encrypted)| async move {
let context = central.new_transaction().await.unwrap();
black_box(context.decrypt_message(&id, encrypted).await.unwrap());
black_box(
context
.conversation_guard(&id)
.await
.unwrap()
.decrypt_message(encrypted)
.await
.unwrap(),
);
context.finish().await.unwrap();
},
BatchSize::SmallInput,
Expand Down
16 changes: 14 additions & 2 deletions crypto/benches/transaction.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,25 @@ fn decrypt_transaction(c: &mut Criterion) {
if *transactional {
let context = bob_central.new_transaction().await.unwrap();
for message in encrypted_messages.into_iter() {
context.decrypt_message(&id, message).await.unwrap();
context
.conversation_guard(&id)
.await
.unwrap()
.decrypt_message(message)
.await
.unwrap();
}
context.finish().await.unwrap();
} else {
for message in encrypted_messages.into_iter() {
let context = bob_central.new_transaction().await.unwrap();
context.decrypt_message(&id, message).await.unwrap();
context
.conversation_guard(&id)
.await
.unwrap()
.decrypt_message(message)
.await
.unwrap();
context.finish().await.unwrap();
}
}
Expand Down
5 changes: 4 additions & 1 deletion crypto/src/e2e_identity/conversation_state.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,10 @@ mod tests {
let commit = alice_central.mls_transport.latest_commit().await;
bob_central
.context
.decrypt_message(&id, commit.to_bytes().unwrap())
.conversation_guard(&id)
.await
.unwrap()
.decrypt_message(commit.to_bytes().unwrap())
.await
.unwrap();

Expand Down
40 changes: 32 additions & 8 deletions crypto/src/e2e_identity/rotate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -368,7 +368,10 @@ pub(crate) mod tests {
for (id, commit) in result.conversation_ids_and_commits.into_iter() {
let decrypted = bob_central
.context
.decrypt_message(&id, commit.commit.to_bytes().unwrap())
.conversation_guard(&id)
.await
.unwrap()
.decrypt_message(commit.commit.to_bytes().unwrap())
.await
.unwrap();
alice_central.verify_sender_identity(&case, &decrypted).await;
Expand Down Expand Up @@ -696,7 +699,10 @@ pub(crate) mod tests {

let decrypted = bob_central
.context
.decrypt_message(&id, commit.to_bytes().unwrap())
.conversation_guard(&id)
.await
.unwrap()
.decrypt_message(commit.to_bytes().unwrap())
.await
.unwrap();
alice_central.verify_sender_identity(&case, &decrypted).await;
Expand Down Expand Up @@ -770,7 +776,10 @@ pub(crate) mod tests {

let decrypted = alice_central
.context
.decrypt_message(&id, commit.to_bytes().unwrap())
.conversation_guard(&id)
.await
.unwrap()
.decrypt_message(commit.to_bytes().unwrap())
.await
.unwrap();
bob_central.verify_sender_identity(&case, &decrypted).await;
Expand Down Expand Up @@ -855,7 +864,10 @@ pub(crate) mod tests {
// Bob decrypts the commit...
let decrypted = bob_central
.context
.decrypt_message(&id, commit.to_bytes().unwrap())
.conversation_guard(&id)
.await
.unwrap()
.decrypt_message(commit.to_bytes().unwrap())
.await
.unwrap();
// ...and verifies that now Alice is represented with her new identity
Expand Down Expand Up @@ -935,7 +947,10 @@ pub(crate) mod tests {
// Alice decrypts the commit...
let decrypted = alice_central
.context
.decrypt_message(&id, bob_commit.to_bytes().unwrap())
.conversation_guard(&id)
.await
.unwrap()
.decrypt_message(bob_commit.to_bytes().unwrap())
.await
.unwrap();

Expand All @@ -944,7 +959,10 @@ pub(crate) mod tests {
let renewed_proposal = decrypted.proposals.first().unwrap();
bob_central
.context
.decrypt_message(&id, renewed_proposal.proposal.to_bytes().unwrap())
.conversation_guard(&id)
.await
.unwrap()
.decrypt_message(renewed_proposal.proposal.to_bytes().unwrap())
.await
.unwrap();

Expand All @@ -966,7 +984,10 @@ pub(crate) mod tests {
// Bob verifies that now Alice is represented with her new identity
let decrypted = bob_central
.context
.decrypt_message(&id, rotate_commit.to_bytes().unwrap())
.conversation_guard(&id)
.await
.unwrap()
.decrypt_message(rotate_commit.to_bytes().unwrap())
.await
.unwrap();
alice_central.verify_sender_identity(&case, &decrypted).await;
Expand Down Expand Up @@ -1041,7 +1062,10 @@ pub(crate) mod tests {
// Bob decrypts the commit...
let decrypted = bob_central
.context
.decrypt_message(&id, commit.to_bytes().unwrap())
.conversation_guard(&id)
.await
.unwrap()
.decrypt_message(commit.to_bytes().unwrap())
.await
.unwrap();
// ...and verifies that now Alice is represented with her new identity
Expand Down
Loading
Loading