diff --git a/Cargo.lock b/Cargo.lock index 1fe7eafff83..fc21156e891 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1090,6 +1090,7 @@ dependencies = [ "derive_more", "ecdsa", "ed25519-dalek", + "enum-as-inner", "env_logger", "futures-lite", "futures-util", @@ -1700,6 +1701,18 @@ version = "0.6.1" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "edd0f118536f44f5ccd48bcb8b111bdc3de888b58c74639dfb034a357d0f206d" +[[package]] +name = "enum-as-inner" +version = "0.7.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0359ee92f81184d7985519e474bda2a5738476334edd3746c9b1265c067afe70" +dependencies = [ + "heck", + "proc-macro2", + "quote", + "syn 2.0.119", +] + [[package]] name = "env_filter" version = "2.0.0" diff --git a/Cargo.toml b/Cargo.toml index fc1744601bf..c3a992f0b97 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -48,6 +48,7 @@ derive_more = { version = "2.1", features = [ "deref_mut", "into_iterator", ] } +enum-as-inner = "0.7" futures-util = "0.3" hex = "0.4" # temporary, until https://github.com/devashishdxt/idb/pull/42 is merged / released diff --git a/cc-book/src/release_notes.md b/cc-book/src/release_notes.md index 3789bc5e4d3..09d6c9dec82 100644 --- a/cc-book/src/release_notes.md +++ b/cc-book/src/release_notes.md @@ -2,6 +2,9 @@ ## Unreleased +- `DecryptedMessage` is now an enum with `Text`, `Commit`, and `Proposal` variants. For migration, see the migration + guide. + ## CoreCrypto 10 ### v10.2.0 - 2026-07-28 diff --git a/cc-book/src/unreleased/migration_guide.md b/cc-book/src/unreleased/migration_guide.md index fa81f220453..3ba4022f7be 100644 --- a/cc-book/src/unreleased/migration_guide.md +++ b/cc-book/src/unreleased/migration_guide.md @@ -8,3 +8,53 @@ 1. Deprecated the `key` parameter from in-memory Database constructor. In-memory databases are never encrypted. 1. `Database.getLocation` now returns the absolute path to the database file for non-web platforms. + +## DecryptedMessage + +`DecryptedMessage` is now an enum with `Text`, `Commit`, and `Proposal` variants. Data that was previously exposed +through optional properties is now carried by the corresponding variant. Match on the variant before accessing its data: + + + +```typescript +if (DecryptedMessage.Text.instanceOf(decryptedMessage)) { + const { plaintext, senderClientId, identity } = decryptedMessage.inner; + // Handle the application message. +} else if (DecryptedMessage.Commit.instanceOf(decryptedMessage)) { + // Handle the commit. + const { isActive, bufferedMessages, identity } = decryptedMessage.inner; +} else if (DecryptedMessage.Proposal.instanceOf(decryptedMessage)) { + const { delay, identity } = decryptedMessage.inner; + // Handle the proposal. +} +``` + +```swift +switch decryptedMessage { +case let .text(plaintext, senderClientId, identity): + // Handle the application message. +case let .commit(isActive, bufferedMessages, identity): + // Handle the commit. +case let .proposal(delay, identity): + // Handle the proposal. +} +``` + +```kotlin +when (decryptedMessage) { + is DecryptedMessage.Text -> { + val (plaintext, senderClientId, identity) = decryptedMessage + // Handle the application message. + } + is DecryptedMessage.Commit -> { + val (isActive, bufferedMessages, identity) = decryptedMessage + // Handle the commit. + } + is DecryptedMessage.Proposal -> { + val (delay, identity) = decryptedMessage + // Handle the proposal. + } +} +``` + + diff --git a/crypto-ffi/bindings/js/shared/shared/utils.ts b/crypto-ffi/bindings/js/shared/shared/utils.ts index 08bff9980bc..b8744e30c10 100644 --- a/crypto-ffi/bindings/js/shared/shared/utils.ts +++ b/crypto-ffi/bindings/js/shared/shared/utils.ts @@ -1,5 +1,5 @@ import { - DatabaseKey, + type DatabaseKey, type CipherSuite, type ClientId, type CommitBundle, @@ -422,11 +422,24 @@ async function setHelpers() { } ); - const decoder = new TextDecoder(); - const result1 = decoder.decode(decryptedByClient1.message); - const result2 = decoder.decode(decryptedByClient2.message); - - return [result1, result2]; + if ( + ccModule.DecryptedMessage.Text.instanceOf( + decryptedByClient1 + ) && + ccModule.DecryptedMessage.Text.instanceOf( + decryptedByClient2 + ) + ) { + const decoder = new TextDecoder(); + const result1 = decoder.decode( + decryptedByClient1.inner.plaintext + ); + const result2 = decoder.decode( + decryptedByClient2.inner.plaintext + ); + return [result1, result2]; + } + return ["", ""]; } /** diff --git a/crypto-ffi/bindings/js/shared/src/CoreCrypto.ts b/crypto-ffi/bindings/js/shared/src/CoreCrypto.ts index 35170065381..bb45279a92d 100644 --- a/crypto-ffi/bindings/js/shared/src/CoreCrypto.ts +++ b/crypto-ffi/bindings/js/shared/src/CoreCrypto.ts @@ -70,7 +70,7 @@ export { type GroupInfoBundle, type HistorySecret, CredentialRef, - type DecryptedMessage, + DecryptedMessage, CoreCryptoLogLevel, ProteusAutoPrekeyBundle, BufferedDecryptedMessage, diff --git a/crypto-ffi/bindings/shared/src/commonTest/kotlin/com/wire/crypto/MLSTest.kt b/crypto-ffi/bindings/shared/src/commonTest/kotlin/com/wire/crypto/MLSTest.kt index 81c28d96736..a3aad2b8294 100644 --- a/crypto-ffi/bindings/shared/src/commonTest/kotlin/com/wire/crypto/MLSTest.kt +++ b/crypto-ffi/bindings/shared/src/commonTest/kotlin/com/wire/crypto/MLSTest.kt @@ -194,9 +194,7 @@ class MLSTest { val commit = MockMlsTransportSuccessProvider.getInstance().getLatestCommit() val decrypted = alice.transaction { ctx -> ctx.decryptMessage(groupId, commit) } - assertThat(decrypted.message).isNull() - assertThat(decrypted.commitDelay).isNull() - assertThat(decrypted.senderClientId).isNull() + assertIs(decrypted) } @Test @@ -222,7 +220,10 @@ class MLSTest { val ciphertextMsg = alice.transaction { ctx -> ctx.encryptMessage(groupId, msg) } assertThat(ciphertextMsg).isNotEqualTo(msg) - val plaintextMsg = bob.transaction { ctx -> ctx.decryptMessage(groupId, ciphertextMsg).message!! } + val plaintextMsg = + bob.transaction { ctx -> + assertIs(ctx.decryptMessage(groupId, ciphertextMsg)).plaintext + } assertThat(plaintextMsg).isNotEmpty().isEqualTo(msg) val expectedException = @@ -248,7 +249,7 @@ class MLSTest { val commit = MockMlsTransportSuccessProvider.getInstance().getLatestCommit() val decrypted = alice.transaction { ctx -> ctx.decryptMessage(conversationId, commit) } - assertThat(decrypted.message).isNull() + assertIs(decrypted) val members = alice.transaction { ctx -> ctx.getClientIds(conversationId) } assertThat(members).containsAll(listOf(aliceId, bobId, carolId)) @@ -283,7 +284,7 @@ class MLSTest { val commit = MockMlsTransportSuccessProvider.getInstance().getLatestCommit() val decrypted = alice.transaction { ctx -> ctx.decryptMessage(conversationId, commit) } - assertThat(decrypted.message).isNull() + assertIs(decrypted) } @Test diff --git a/crypto-ffi/bindings/swift/WireCoreCrypto/WireCoreCryptoTests/WireCoreCryptoTests.swift b/crypto-ffi/bindings/swift/WireCoreCrypto/WireCoreCryptoTests/WireCoreCryptoTests.swift index b5029941a01..5b361ad6522 100644 --- a/crypto-ffi/bindings/swift/WireCoreCrypto/WireCoreCryptoTests/WireCoreCryptoTests.swift +++ b/crypto-ffi/bindings/swift/WireCoreCrypto/WireCoreCryptoTests/WireCoreCryptoTests.swift @@ -556,9 +556,10 @@ final class WireCoreCryptoTests: XCTestCase { let decrypted = try await alice.transaction { ctx in try await ctx.decryptMessage(conversationId: conversationId, payload: commit!) } - XCTAssertNil(decrypted.message) - XCTAssertNil(decrypted.commitDelay) - XCTAssertNil(decrypted.senderClientId) + guard case .commit = decrypted else { + XCTFail("Expected a decrypted commit") + return + } } func testEncryptMessageCanBeDecryptedByReceiver() async throws { @@ -577,9 +578,12 @@ final class WireCoreCryptoTests: XCTestCase { } XCTAssertNotEqual(ciphertext, message) - let plaintext = try await bob.transaction { ctx in + let decrypted = try await bob.transaction { ctx in try await ctx.decryptMessage(conversationId: conversationId, payload: ciphertext) - .message + } + guard case .text(let plaintext, _, _) = decrypted else { + XCTFail("Expected decrypted text") + return } XCTAssertEqual(plaintext, message) diff --git a/crypto-ffi/src/decrypted_message.rs b/crypto-ffi/src/decrypted_message.rs index 04ea335e927..08fe9dcf5e0 100644 --- a/crypto-ffi/src/decrypted_message.rs +++ b/crypto-ffi/src/decrypted_message.rs @@ -4,77 +4,113 @@ use core_crypto::{BufferedDecryptedMessage as CcBufferedDecryptedMessage, Decryp use crate::{ClientId, WireIdentity}; -/// A decrypted message and various associated metadata. -#[derive(Debug, uniffi::Record)] -pub struct DecryptedMessage { - /// Decrypted plaintext - pub message: Option>, - /// False if processing this message caused the client to be removed from the group, i.e. due to a Remove commit. - pub is_active: bool, - /// Commit delay in seconds. - /// - /// When set, clients must delay this long before processing a commit. - /// This reduces load on the backend, which otherwise would receive epoch change notifications from all clients - /// simultaneously. - pub commit_delay: Option, - /// `ClientId` of the sender of the message being decrypted. Only present for application messages. - pub sender_client_id: Option>, - /// Identity claims present in the sender credential. - pub identity: WireIdentity, - /// Only set when the decrypted message is a commit. - /// - /// Contains buffered messages for next epoch which were received before the commit creating the epoch - /// because the DS did not fan them out in order. - pub buffered_messages: Option>, +/// Represents the items a consumer might require after decrypting a message. +#[derive(Debug, uniffi::Enum)] +pub enum DecryptedMessage { + /// The decrypted message is a text message. + Text { + /// Decrypted text message. + plaintext: Vec, + /// The sender's `ClientId`. + sender_client_id: Arc, + /// Identity claims present in the sender credential. + identity: WireIdentity, + }, + /// The decrypted message is a commit. + Commit { + /// False if processing this message caused the client to be removed from the group, i.e. due to a Remove + /// commit. + is_active: bool, + /// Contains buffered messages for next epoch which were received before the commit creating the epoch + /// because the DS did not fan them out in order. + buffered_messages: Option>, + /// Identity claims present in the sender credential. + identity: WireIdentity, + }, + /// The decrypted message is a proposal. + Proposal { + /// Commit delay in seconds. + /// + /// When set, clients must delay by this time interval before processing a commit. This reduces load on the + /// backend, which otherwise would receive epoch change notifications from all clients simultaneously. + delay: Option, + /// Identity claims present in the sender credential. + identity: WireIdentity, + }, } impl From for DecryptedMessage { fn from(from: CcDecryptedMessage) -> Self { - let buffered_messages = from - .buffered_messages - .map(|bm| bm.into_iter().map(Into::into).collect::>()); - - Self { - message: from.app_msg, - is_active: from.is_active, - commit_delay: from.delay, - sender_client_id: from.sender_client_id.map(Into::into).map(Arc::new), - identity: from.identity.into(), - buffered_messages, + match from { + CcDecryptedMessage::Text(text) => Self::Text { + plaintext: text.plaintext, + sender_client_id: Arc::new(text.sender_client_id.into()), + identity: text.identity.into(), + }, + CcDecryptedMessage::Commit(commit) => Self::Commit { + is_active: commit.is_active, + buffered_messages: commit + .buffered_messages + .map(|messages| messages.into_iter().map(Into::into).collect()), + identity: commit.identity.into(), + }, + CcDecryptedMessage::Proposal(proposal) => Self::Proposal { + delay: proposal.delay, + identity: proposal.identity.into(), + }, } } } -/// A decrypted message that was buffered due to out-of-order delivery by the distribution service. -/// -/// These are returned in the `buffered_messages` field of a `DecryptedMessage` when a commit is -/// processed. They represent messages for the new epoch that arrived before the commit that created it. -#[derive(Debug, Clone, uniffi::Record)] -pub struct BufferedDecryptedMessage { - /// Decrypted plaintext - pub message: Option>, - /// False if processing this message caused the client to be removed from the group, i.e. due to a Remove commit. - pub is_active: bool, - /// Commit delay in seconds. - /// - /// When set, clients must delay this long before processing a commit. - /// This reduces load on the backend, which otherwise would receive epoch change notifications from all clients - /// simultaneously. - pub commit_delay: Option, - /// `ClientId` of the sender of the message being decrypted. Only present for application messages. - pub sender_client_id: Option>, - /// Identity claims present in the sender credential. - pub identity: WireIdentity, +/// A decrypted message that was buffered due to out-of-order delivery by the delivery service. +/// It represents messages for the new epoch that arrived before the commit that created it. +#[derive(Debug, uniffi::Enum)] +pub enum BufferedDecryptedMessage { + /// The decrypted message is a text message. + Text { + /// Decrypted text message. + plaintext: Vec, + /// The sender's `ClientId`. + sender_client_id: Arc, + /// Identity claims present in the sender credential. + identity: WireIdentity, + }, + /// The decrypted message is a commit. + Commit { + /// False if processing this message caused the client to be removed from the group, i.e. due to a Remove + /// commit. + is_active: bool, + /// Identity claims present in the sender credential. + identity: WireIdentity, + }, + /// The decrypted message is a proposal. + Proposal { + ///Commit delay in seconds. + /// + /// When set, clients must delay by this time interval before processing a commit. This reduces load on the + /// backend, which otherwise would receive epoch change notifications from all clients simultaneously. + delay: Option, + /// Identity claims present in the sender credential. + identity: WireIdentity, + }, } impl From for BufferedDecryptedMessage { fn from(from: CcBufferedDecryptedMessage) -> Self { - Self { - message: from.app_msg, - is_active: from.is_active, - commit_delay: from.delay, - sender_client_id: from.sender_client_id.map(Into::into).map(Arc::new), - identity: from.identity.into(), + match from { + CcBufferedDecryptedMessage::Text(text) => Self::Text { + plaintext: text.plaintext, + sender_client_id: Arc::new(text.sender_client_id.into()), + identity: text.identity.into(), + }, + CcBufferedDecryptedMessage::Commit(commit) => Self::Commit { + is_active: commit.is_active, + identity: commit.identity.into(), + }, + CcBufferedDecryptedMessage::Proposal(proposal) => Self::Proposal { + delay: proposal.delay, + identity: proposal.identity.into(), + }, } } } diff --git a/crypto/Cargo.toml b/crypto/Cargo.toml index f554fcf40d5..8cdd30b85f5 100644 --- a/crypto/Cargo.toml +++ b/crypto/Cargo.toml @@ -22,6 +22,7 @@ test-all-cipher = [] [dependencies] derive_more.workspace = true +enum-as-inner.workspace = true thiserror.workspace = true strum.workspace = true hex.workspace = true diff --git a/crypto/src/lib.rs b/crypto/src/lib.rs index 0ea4dd62fbf..a5af153df6e 100644 --- a/crypto/src/lib.rs +++ b/crypto/src/lib.rs @@ -58,8 +58,9 @@ pub use crate::{ ExternalSender, cipher_suite::CipherSuite, conversation::{ - BufferedDecryptedMessage, CommitBundle, ConversationConfiguration, ConversationId, CustomConfiguration, - DecryptedMessage, GroupInfoBundle, GroupInfoEncryptionType, GroupInfoPayload, RatchetTreeType, WirePolicy, + BufferedCommit, BufferedDecryptedMessage, Commit, CommitBundle, ConversationConfiguration, ConversationId, + CustomConfiguration, DecryptedMessage, GroupInfoBundle, GroupInfoEncryptionType, GroupInfoPayload, + Proposal, RatchetTreeType, Text, WirePolicy, }, credential::{ Credential, CredentialRef, CredentialType, FindFilters as CredentialFindFilters, x509::CertificateBundle, diff --git a/crypto/src/mls/conversation/immutable/commit_delay.rs b/crypto/src/mls/conversation/immutable/commit_delay.rs index 7dc9734eba1..fe966373c5e 100644 --- a/crypto/src/mls/conversation/immutable/commit_delay.rs +++ b/crypto/src/mls/conversation/immutable/commit_delay.rs @@ -169,12 +169,12 @@ mod tests { let charlie_hypothetical_position = 1; assert_eq!( - bob_decrypted_message.delay, + bob_decrypted_message.as_proposal().unwrap().delay, Some(DELAY_POS_LINEAR_INCR * bob_hypothetical_position) ); assert_eq!( - charlie_decrypted_message.delay, + charlie_decrypted_message.as_proposal().unwrap().delay, Some(DELAY_POS_LINEAR_INCR * charlie_hypothetical_position) ); }) diff --git a/crypto/src/mls/conversation/mod.rs b/crypto/src/mls/conversation/mod.rs index 67055b11ddf..eda45f607ec 100644 --- a/crypto/src/mls/conversation/mod.rs +++ b/crypto/src/mls/conversation/mod.rs @@ -33,7 +33,7 @@ pub use self::{ immutable::Conversation, mutable::{ ConversationMut, - decrypt::{BufferedDecryptedMessage, DecryptedMessage}, + decrypt::{BufferedCommit, BufferedDecryptedMessage, Commit, DecryptedMessage, Proposal, Text}, }, welcome::WelcomeMessage, }; diff --git a/crypto/src/mls/conversation/mutable/decrypt/buffer_messages.rs b/crypto/src/mls/conversation/mutable/decrypt/buffer_messages.rs index 056159713af..08926efad76 100644 --- a/crypto/src/mls/conversation/mutable/decrypt/buffer_messages.rs +++ b/crypto/src/mls/conversation/mutable/decrypt/buffer_messages.rs @@ -191,25 +191,24 @@ mod tests { .unwrap(); // Finally, Bob receives the green light from the DS and he can merge the external commit - let DecryptedMessage { - buffered_messages: Some(restored_messages), - .. - } = conversation + let restored_messages = conversation .guard_of(&bob) .await .decrypt_message(unmerged_commit.to_bytes().unwrap()) .await .unwrap() - else { - panic!("Alice's messages should have been restored at this point"); - }; + .into_commit() + .unwrap() + .buffered_messages + .expect("Alice's messages should have been restored at this point"); - for (idx, msg) in restored_messages.iter().enumerate() { + for (idx, buffered_message) in restored_messages.into_iter().enumerate() { + let text = DecryptedMessage::from(buffered_message).into_text(); if idx == 0 { // this is the application message - assert_eq!(msg.app_msg.as_deref(), Some("Hello Bob !".as_bytes())); + assert_eq!(text.unwrap().plaintext, b"Hello Bob !"); } else { - assert!(msg.app_msg.is_none()); + assert!(text.is_err()); } } @@ -289,23 +288,22 @@ mod tests { // Finally, Alice receives the original commit for this epoch let original_commit = ext_commit.to_bytes().unwrap(); - let DecryptedMessage { - buffered_messages: Some(restored_messages), - .. - } = conversation + let restored_messages = conversation .guard() .await .decrypt_message(original_commit) .await .unwrap() - else { - panic!("Alice's messages should have been restored at this point"); - }; - for (idx, msg) in restored_messages.into_iter().enumerate() { + .into_commit() + .unwrap() + .buffered_messages + .expect("Alice's messages should have been restored at this point"); + for (idx, buffered_message) in restored_messages.into_iter().enumerate() { + let text = DecryptedMessage::from(buffered_message).into_text(); if idx == 0 { - assert_eq!(msg.app_msg.as_deref(), Some(b"Hello Alice !" as _)); + assert_eq!(text.unwrap().plaintext, b"Hello Alice !"); } else { - assert!(msg.app_msg.is_none()); + assert!(text.is_err()); } } diff --git a/crypto/src/mls/conversation/mutable/decrypt/mod.rs b/crypto/src/mls/conversation/mutable/decrypt/mod.rs index 047777fc09e..1c0b751edfb 100644 --- a/crypto/src/mls/conversation/mutable/decrypt/mod.rs +++ b/crypto/src/mls/conversation/mutable/decrypt/mod.rs @@ -33,52 +33,107 @@ use crate::{ mls::{conversation::Error, credential::ext::CredentialExt as _}, }; -/// Represents the potential items a consumer might require after passing us an encrypted message we -/// have decrypted for him +/// A decrypted MLS application message #[derive(Debug)] -pub struct DecryptedMessage { +pub struct Text { /// Decrypted text message - pub app_msg: Option>, - /// Is the conversation still active after receiving this commit aka has the user been removed from the group - pub is_active: bool, + pub plaintext: Vec, + /// The sender's [ClientId]. + pub sender_client_id: ClientId, + /// Identity claims present in the sender credential + pub identity: WireIdentity, +} + +/// The data relevant after a Proposal was processed. +#[derive(Debug)] +pub struct Proposal { /// Delay time in seconds to feed caller timer for committing pub delay: Option, - /// [ClientId] of the sender of the message being decrypted. Only present for application messages. - pub sender_client_id: Option, /// Identity claims present in the sender credential - /// - /// Present for all messages pub identity: WireIdentity, - /// Only set when the decrypted message is a commit. - /// +} + +/// The data relevant after a Commit was processed. +#[derive(Debug)] +pub struct Commit { + /// Is the conversation still active after receiving this commit, i.e., has the user been removed from the group + pub is_active: bool, /// Contains buffered messages for next epoch which were received before the commit creating the epoch /// because the DS did not fan them out in order. pub buffered_messages: Option>, + /// Identity claims present in the sender credential + pub identity: WireIdentity, } -/// Type safe recursion of [DecryptedMessage] +/// The data relevant after a buffered Commit was processed. #[derive(Debug)] -pub struct BufferedDecryptedMessage { - /// see [DecryptedMessage::app_msg] - pub app_msg: Option>, - /// see [DecryptedMessage::is_active] +pub struct BufferedCommit { + /// Is the conversation still active after receiving this commit, i.e., has the user been removed from the group pub is_active: bool, - /// see [DecryptedMessage::delay] - pub delay: Option, - /// see [DecryptedMessage::sender_client_id] - pub sender_client_id: Option, - /// see [DecryptedMessage::identity] + /// Identity claims present in the sender credential pub identity: WireIdentity, } +/// Represents the potential items a consumer might require after passing us an encrypted message we +/// have decrypted for him +#[derive(Debug, enum_as_inner::EnumAsInner)] +pub enum DecryptedMessage { + /// The decrypted message is a text message. + Text(Text), + /// The decrypted message is a commit. + Commit(Commit), + /// The decrypted message is a proposal. + Proposal(Proposal), +} + +impl DecryptedMessage { + /// Identity claims present in the sender credential + pub fn identity(&self) -> &WireIdentity { + match self { + DecryptedMessage::Text(text) => &text.identity, + DecryptedMessage::Commit(commit) => &commit.identity, + DecryptedMessage::Proposal(proposal) => &proposal.identity, + } + } +} + +/// A decrypted message that was buffered due to out-of-order delivery by the delivery service. +/// It represents messages for the new epoch that arrived before the commit that created it. +#[derive(Debug)] +pub enum BufferedDecryptedMessage { + /// The decrypted message is a text message. + Text(Text), + /// The decrypted message is a commit. + Commit(BufferedCommit), + /// The decrypted message is a proposal. + Proposal(Proposal), +} + impl From for BufferedDecryptedMessage { - fn from(from: DecryptedMessage) -> Self { - Self { - app_msg: from.app_msg, - is_active: from.is_active, - delay: from.delay, - sender_client_id: from.sender_client_id, - identity: from.identity, + fn from(value: DecryptedMessage) -> Self { + match value { + DecryptedMessage::Text(text) => Self::Text(text), + DecryptedMessage::Commit(commit) => Self::Commit({ + BufferedCommit { + is_active: commit.is_active, + identity: commit.identity, + } + }), + DecryptedMessage::Proposal(proposal) => Self::Proposal(proposal), + } + } +} + +impl From for DecryptedMessage { + fn from(value: BufferedDecryptedMessage) -> Self { + match value { + BufferedDecryptedMessage::Text(text) => Self::Text(text), + BufferedDecryptedMessage::Commit(buffered_commit) => Self::Commit(Commit { + is_active: buffered_commit.is_active, + buffered_messages: None, + identity: buffered_commit.identity, + }), + BufferedDecryptedMessage::Proposal(proposal) => Self::Proposal(proposal), } } } @@ -128,7 +183,9 @@ impl ConversationMut { let decrypt_message = decrypt_message_result?; - if !decrypt_message.is_active { + if let Some(decrypted_message) = decrypt_message.as_commit() + && !decrypted_message.is_active + { self.wipe().await?; } Ok(decrypt_message) @@ -155,12 +212,14 @@ impl ConversationMut { let ct = self.extract_confirmation_tag_from_own_commit(&message).await?; let mut decrypted_message = self.handle_own_commit(ct).await?; - debug_assert!( - decrypted_message.buffered_messages.is_none(), - "decrypted message should be constructed with empty buffer" - ); - if recursion_policy == RecursionPolicy::AsNecessary { - decrypted_message.buffered_messages = self.restore_and_clear_pending_messages().await?; + if let Some(decrypted_message) = decrypted_message.as_commit_mut() { + debug_assert!( + decrypted_message.buffered_messages.is_none(), + "decrypted message should be constructed with empty buffer" + ); + if recursion_policy == RecursionPolicy::AsNecessary { + decrypted_message.buffered_messages = self.restore_and_clear_pending_messages().await?; + } } return Ok(decrypted_message); @@ -191,16 +250,18 @@ impl ConversationMut { .await .map_err(RecursiveError::mls_credential("extracting identity"))?; - let sender_client_id = credential + // We only need this in the ProcessedMessageContent::ApplicationMessage match arm below, however, at that point + // we cannot borrow from `message` anymore, because it is moved by `message.into_content()`. + let sender_client_id_result = credential .credential .identity() .try_into() - .inspect_err(|e| log::info!("sender client id couldn't be parsed into the expected format: {e:?}")) - .ok(); + .map_err(RecursiveError::mls_client("client id from credential")); let decrypted = match message.into_content() { ProcessedMessageContent::ApplicationMessage(app_msg) => { let conversation = self; + let sender_client_id = sender_client_id_result?; debug!( group_id = conversation.id().to_owned(), epoch = epoch.as_u64(), @@ -208,14 +269,11 @@ impl ConversationMut { "Application message" ); - DecryptedMessage { - app_msg: Some(app_msg.into_bytes()), - is_active: true, - delay: None, + DecryptedMessage::Text(Text { + plaintext: app_msg.into_bytes(), sender_client_id, identity, - buffered_messages: None, - } + }) } ProcessedMessageContent::ProposalMessage(proposal) => { self.mutate_group(async |_, group, id| { @@ -262,14 +320,7 @@ impl ConversationMut { let delay = self.compute_next_commit_delay().await; - DecryptedMessage { - app_msg: None, - is_active: true, - delay, - sender_client_id: None, - identity, - buffered_messages: None, - } + DecryptedMessage::Proposal(Proposal { delay, identity }) } ProcessedMessageContent::StagedCommitMessage(staged_commit) => { self.validate_commit(&staged_commit).await?; @@ -305,8 +356,6 @@ impl ConversationMut { }) .await?; - let delay = self.compute_next_commit_delay().await; - // can't use `.then` because async let mut buffered_messages = None; if recursion_policy == RecursionPolicy::AsNecessary { @@ -327,14 +376,11 @@ impl ConversationMut { .await .map_err(RecursiveError::transaction("queueing epoch changed notification"))?; - DecryptedMessage { - app_msg: None, + DecryptedMessage::Commit(Commit { is_active, - delay, - sender_client_id: None, - identity, buffered_messages, - } + identity, + }) } ProcessedMessageContent::ExternalJoinProposalMessage(proposal) => { self.mutate_group(async |_, group, id| { @@ -350,14 +396,7 @@ impl ConversationMut { let delay = self.compute_next_commit_delay().await; - DecryptedMessage { - app_msg: None, - is_active: true, - delay, - sender_client_id: None, - identity, - buffered_messages: None, - } + DecryptedMessage::Proposal(Proposal { delay, identity }) } }; @@ -509,7 +548,7 @@ mod tests { .notify_member_fallible(&alice) .await; let decrypted = decrypted.unwrap(); - assert!(decrypted.is_active) + assert!(decrypted.as_commit().is_some_and(|commit| commit.is_active)) }) .await } @@ -528,7 +567,7 @@ mod tests { .notify_member_fallible(&alice) .await; let decrypted = decrypted.unwrap(); - assert!(!decrypted.is_active) + assert!(decrypted.as_commit().is_some_and(|commit| !commit.is_active)) }) .await } @@ -557,8 +596,7 @@ mod tests { let epoch_after = commit.finish().guard_of(&bob).await.epoch().await; assert_eq!(epoch_after, epoch_before + 1); - assert!(decrypted.delay.is_none()); - assert!(decrypted.app_msg.is_none()); + assert!(decrypted.as_commit().is_some()); alice .verify_sender_identity(&case, &alice.initial_credential, &decrypted) @@ -570,15 +608,14 @@ mod tests { } #[apply(all_cred_cipher)] - async fn should_not_return_sender_client_id(case: TestContext) { + async fn should_be_commit_variant(case: TestContext) { let [alice, bob] = case.sessions().await; Box::pin(async move { let conversation = case.create_conversation([&alice, &bob]).await; let (_commit, decrypted) = conversation.update().await.notify_member_fallible(&bob).await; - let sender_client_id = decrypted.unwrap().sender_client_id; - assert!(sender_client_id.is_none()); + assert!(decrypted.unwrap().as_commit().is_some()); }) .await } @@ -588,15 +625,18 @@ mod tests { use super::*; #[apply(all_cred_cipher)] - async fn should_not_return_sender_client_id(case: TestContext) { + async fn should_be_proposal_variant(case: TestContext) { let [alice, bob] = case.sessions().await; Box::pin(async move { let conversation = case.create_conversation([&alice, &bob]).await; - let (_commit, decrypted) = conversation.update_unmerged().await.notify_member_fallible(&bob).await; + let (_commit, decrypted) = conversation + .remove_proposal(&bob) + .await + .notify_member_fallible(&bob) + .await; - let sender_client_id = decrypted.unwrap().sender_client_id; - assert!(sender_client_id.is_none()); + assert!(decrypted.unwrap().as_proposal().is_some()); }) .await } @@ -634,8 +674,8 @@ mod tests { .decrypt_message(encrypted) .await .unwrap(); - let dec_msg = decrypted.app_msg.as_ref().unwrap().as_slice(); - assert_eq!(dec_msg, &msg[..]); + let dec_msg = &decrypted.as_text().unwrap().plaintext; + assert_eq!(dec_msg, msg); assert!(!bob_observer.has_changed().await); alice .verify_sender_identity(&case, &alice.initial_credential, &decrypted) @@ -645,7 +685,7 @@ mod tests { let encrypted = conversation.guard_of(&bob).await.encrypt_message(msg).await.unwrap(); assert_ne!(&msg[..], &encrypted[..]); let decrypted = conversation.guard().await.decrypt_message(encrypted).await.unwrap(); - let dec_msg = decrypted.app_msg.as_ref().unwrap().as_slice(); + let dec_msg = &decrypted.as_text().unwrap().plaintext; assert_eq!(dec_msg, &msg[..]); assert!(!alice_observer.has_changed().await); bob.verify_sender_identity(&case, &bob.initial_credential, &decrypted) @@ -694,10 +734,11 @@ mod tests { assert!(matches!(decrypt.unwrap_err(), Error::BufferedFutureMessage { .. })); let (_, decrypted_commit) = commit_guard.notify_member_fallible(&bob).await; - let decrypted_commit = decrypted_commit.unwrap(); - let buffered_msg = decrypted_commit.buffered_messages.unwrap(); - let decrypted_msg = buffered_msg.first().unwrap().app_msg.clone().unwrap(); - assert_eq!(&decrypted_msg, msg); + let decrypted_commit = decrypted_commit.unwrap().into_commit().unwrap(); + let buffered_msg = decrypted_commit.buffered_messages.unwrap().remove(0); + let decrypted_msg = crate::DecryptedMessage::from(buffered_msg); + let decrypted_msg = &decrypted_msg.as_text().unwrap().plaintext; + assert_eq!(&decrypted_msg, &msg); }) .await } @@ -723,7 +764,8 @@ mod tests { for (i, (original, encrypted)) in messages.iter().rev().enumerate() { let decrypt = conversation.guard_of(&bob).await.decrypt_message(encrypted).await; if i < out_of_order_tolerance as usize { - let decrypted = decrypt.unwrap().app_msg.unwrap(); + let decrypt = decrypt.unwrap(); + let decrypted = &decrypt.as_text().unwrap().plaintext; assert_eq!(decrypted, original.as_bytes()); } else { assert!(matches!(decrypt.unwrap_err(), Error::DuplicateMessage)) @@ -750,8 +792,8 @@ mod tests { .await .unwrap(); - let sender_client_id = decrypted.sender_client_id.unwrap(); - assert_eq!(sender_client_id, alice.get_client_id().await); + let sender_client_id = &decrypted.as_text().unwrap().sender_client_id; + assert_eq!(sender_client_id, &alice.get_client_id().await); }) .await } @@ -789,7 +831,7 @@ mod tests { .decrypt_message(&bob_message1) .await .unwrap(); - assert_eq!(decrypt.app_msg.unwrap(), b"Hello Bob"); + assert_eq!(decrypt.as_text().unwrap().plaintext, b"Hello Bob"); // Moving the epochs once more should cause an error let conversation = conversation.update_notify().await; diff --git a/crypto/src/mls/conversation/mutable/encrypt.rs b/crypto/src/mls/conversation/mutable/encrypt.rs index 47fa53d6eb0..303810a6bf9 100644 --- a/crypto/src/mls/conversation/mutable/encrypt.rs +++ b/crypto/src/mls/conversation/mutable/encrypt.rs @@ -70,9 +70,8 @@ mod tests { .await .decrypt_message(encrypted) .await - .unwrap() - .app_msg .unwrap(); + let decrypted = &decrypted.as_text().unwrap().plaintext; assert_eq!(&decrypted[..], &msg[..]); }) .await @@ -93,9 +92,8 @@ mod tests { .await .decrypt_message(encrypted) .await - .unwrap() - .app_msg .unwrap(); + let decrypted = &decrypted.as_text().unwrap().plaintext; assert_eq!(&decrypted[..], &msg[..]); let msg = b"Hello bob again"; @@ -106,9 +104,8 @@ mod tests { .await .decrypt_message(encrypted) .await - .unwrap() - .app_msg .unwrap(); + let decrypted = &decrypted.as_text().unwrap().plaintext; assert_eq!(&decrypted[..], &msg[..]); }) .await diff --git a/crypto/src/mls/conversation/mutable/own_commit.rs b/crypto/src/mls/conversation/mutable/own_commit.rs index c776fc33d8e..dbb740e5f37 100644 --- a/crypto/src/mls/conversation/mutable/own_commit.rs +++ b/crypto/src/mls/conversation/mutable/own_commit.rs @@ -7,7 +7,10 @@ use openmls::{ use openmls_traits::OpenMlsCryptoProvider as _; use super::{ConversationMut, Error, Result}; -use crate::{DecryptedMessage, RecursiveError, mls::credential::ext::CredentialExt}; +use crate::{ + DecryptedMessage, RecursiveError, + mls::{conversation::mutable::decrypt::Commit, credential::ext::CredentialExt}, +}; impl ConversationMut { /// Returns the confirmation tag from a public message that is an own commit. @@ -95,14 +98,13 @@ impl ConversationMut { .await .map_err(RecursiveError::mls_credential("extracting identity"))?; - Ok(DecryptedMessage { - identity, - delay: self.compute_next_commit_delay().await, + let decrypted_message = DecryptedMessage::Commit(Commit { is_active: self.group().await.is_active(), - app_msg: None, - sender_client_id: None, buffered_messages: None, - }) + identity, + }); + + Ok(decrypted_message) } } diff --git a/crypto/src/mls/conversation/pending.rs b/crypto/src/mls/conversation/pending.rs index 7fa5df62627..351b3c4b051 100644 --- a/crypto/src/mls/conversation/pending.rs +++ b/crypto/src/mls/conversation/pending.rs @@ -182,14 +182,11 @@ impl PendingConversation { .await .map_err(RecursiveError::mls_credential("extracting identity"))?; - Ok(DecryptedMessage { - app_msg: None, + Ok(DecryptedMessage::Commit(super::mutable::decrypt::Commit { is_active: conversation.group().await.is_active(), - delay: conversation.compute_next_commit_delay().await, - sender_client_id: None, - identity, buffered_messages, - }) + identity, + })) } /// This merges the commit generated by [TransactionContext::join_by_external_commit], @@ -288,6 +285,8 @@ mod tests { async fn should_buffer_and_reapply_messages_after_external_commit_merged(case: TestContext) { let [alice, bob, charlie, debbie] = case.sessions().await; Box::pin(async move { + use crate::mls::conversation::mutable::decrypt::Commit; + let conversation = case.create_conversation([&alice, &debbie]).await; // Bob tries to join Alice's group with an external commit let (commit_guard, _pending_conversation) = conversation.external_join_unmerged(&bob).await; @@ -346,10 +345,10 @@ mod tests { .unwrap(); // Finally, Bob receives the green light from the DS and he can merge the external commit - let DecryptedMessage { + let DecryptedMessage::Commit(Commit { buffered_messages: Some(restored_messages), .. - } = pending_conversation + }) = pending_conversation .try_process_own_join_commit(external_commit.to_bytes().unwrap()) .await .unwrap() @@ -357,12 +356,14 @@ mod tests { panic!("Alice's messages should have been restored at this point"); }; - for (idx, msg) in restored_messages.iter().enumerate() { + for (idx, buffered_message) in restored_messages.into_iter().enumerate() { + let text = DecryptedMessage::from(buffered_message).into_text(); if idx == 0 { // the only application message - assert_eq!(msg.app_msg.as_deref(), Some(b"Hello Bob !" as _)); + let msg = text.unwrap().plaintext; + assert_eq!(msg, b"Hello Bob !"); } else { - assert!(msg.app_msg.is_none()); + assert!(text.is_err()); } } diff --git a/crypto/src/test_utils/context.rs b/crypto/src/test_utils/context.rs index 88f5bcf1286..5812e672230 100644 --- a/crypto/src/test_utils/context.rs +++ b/crypto/src/test_utils/context.rs @@ -238,7 +238,7 @@ impl SessionContext { let client_id = mls_identity.client_id.clone().unwrap(); let client_id_bytes = client_id.as_bytes(); - let decrypted_identity = &decrypted.identity; + let decrypted_identity = decrypted.identity(); let leaf: Vec = certificate.certificates.first().unwrap().clone().into(); let identity = leaf diff --git a/crypto/src/test_utils/test_conversation/mod.rs b/crypto/src/test_utils/test_conversation/mod.rs index 3f5e3750fd1..9e76b21dbb3 100644 --- a/crypto/src/test_utils/test_conversation/mod.rs +++ b/crypto/src/test_utils/test_conversation/mod.rs @@ -194,10 +194,10 @@ impl<'a> TestConversation<'a> { .await .map_err(RecursiveError::mls_conversation( "decrypting message; receiver <- sender", - ))? - .app_msg - .ok_or(TestError::ImplementationError)?; - assert_eq!(&msg[..], &decrypted[..]); + ))?; + let plaintext = &decrypted.as_text().ok_or(TestError::ImplementationError)?.plaintext; + + assert_eq!(&msg[..], &plaintext[..]); Ok(()) } diff --git a/interop/src/clients/InteropClient/InteropClient/InteropClientApp.swift b/interop/src/clients/InteropClient/InteropClient/InteropClientApp.swift index 64f10591985..0273e673636 100644 --- a/interop/src/clients/InteropClient/InteropClient/InteropClientApp.swift +++ b/interop/src/clients/InteropClient/InteropClient/InteropClientApp.swift @@ -213,9 +213,10 @@ struct InteropClientApp: App { ) } - if let plaintext = decryptedMessage.message { + switch decryptedMessage { + case .text(let plaintext, _, _): return plaintext.base64EncodedString() - } else { + case .commit, .proposal: return "decrypted protocol message" } diff --git a/interop/src/clients/android-interop/src/main/java/com/wire/androidinterop/InteropActionHandler.kt b/interop/src/clients/android-interop/src/main/java/com/wire/androidinterop/InteropActionHandler.kt index d07c097868c..a69401e37cc 100644 --- a/interop/src/clients/android-interop/src/main/java/com/wire/androidinterop/InteropActionHandler.kt +++ b/interop/src/clients/android-interop/src/main/java/com/wire/androidinterop/InteropActionHandler.kt @@ -6,6 +6,7 @@ import com.wire.crypto.ConversationId import com.wire.crypto.CoreCrypto import com.wire.crypto.Credential import com.wire.crypto.DatabaseKey +import com.wire.crypto.DecryptedMessage import com.wire.crypto.DeviceId import com.wire.crypto.HistorySecret import com.wire.crypto.KeyPackage @@ -67,12 +68,16 @@ class InteropActionHandler(val coreCrypto: CoreCrypto) { } is InteropAction.MLS.DecryptMessage -> { - coreCrypto.transaction { context -> - context.decryptMessage(ConversationId(bytes = action.conversationId), action.message) - }.message?.let { - return Result.success(Base64.Default.encode(it)) + when ( + val decryptedMessage = coreCrypto.transaction { context -> + context.decryptMessage(ConversationId(bytes = action.conversationId), action.message) + } + ) { + is DecryptedMessage.Text -> Result.success(Base64.Default.encode(decryptedMessage.plaintext)) + + is DecryptedMessage.Commit, + is DecryptedMessage.Proposal -> Result.success("decrypted protocol message") } - Result.success("decrypted protocol message") } is InteropAction.MLS.EncryptMessage -> { diff --git a/interop/src/clients/corecrypto/web/mls.ts b/interop/src/clients/corecrypto/web/mls.ts index d2648dc42be..d2fdc1811c3 100644 --- a/interop/src/clients/corecrypto/web/mls.ts +++ b/interop/src/clients/corecrypto/web/mls.ts @@ -113,13 +113,16 @@ export async function encryptMessage() { } export async function decryptMessage() { - const { ConversationId } = await import("./corecrypto.js"); + const { ConversationId, DecryptedMessage } = await import("./corecrypto.js"); const [cId, encMessage] = arguments; const conversationId = new ConversationId(Uint8Array.from(Object.values(cId))); const encryptedMessage = Uint8Array.from(Object.values(encMessage)); - const { message } = await window.cc.transaction((ctx) => + const decryptedMessage = await window.cc.transaction((ctx) => ctx.decryptMessage(conversationId, encryptedMessage) ); - return new Uint8Array(message); + if (DecryptedMessage.Text.instanceOf(decryptedMessage)) { + return decryptedMessage.inner.plaintext; + } + return null; } diff --git a/interop/src/main.rs b/interop/src/main.rs index a6331b0321c..9a19910be79 100644 --- a/interop/src/main.rs +++ b/interop/src/main.rs @@ -259,8 +259,9 @@ async fn run_mls_test(chrome_driver_addr: &std::net::SocketAddr, web_server: &st .unwrap() .decrypt_message(message_to_decrypt) .await? - .app_msg - .ok_or_else(|| anyhow!("[MLS] No message received on master client"))?; + .into_text() + .map_err(|_| anyhow!("[MLS] No message received on master client"))? + .plaintext; let decrypted_master = String::from_utf8(decrypted_master_raw)?;