From 29a68833e9dbb83106b9a2a7d58fab7f051f8f49 Mon Sep 17 00:00:00 2001 From: SimonThormeyer Date: Fri, 17 Jan 2025 09:22:29 +0100 Subject: [PATCH 1/7] refactor!: remove conversation init bundle It was previously returned when creating an external join commit, but as this is send via the transport API now, we don't need it anymore. --- crypto-ffi/bindings/js/src/CoreCrypto.ts | 1 - .../bindings/js/src/CoreCryptoContext.ts | 1 - .../bindings/js/src/CoreCryptoInstance.ts | 1 - crypto-ffi/bindings/js/src/CoreCryptoMLS.ts | 26 ---------- .../main/kotlin/com/wire/crypto/MlsModel.kt | 8 --- crypto-ffi/src/generic/mod.rs | 27 +--------- crypto-ffi/src/wasm/mod.rs | 51 ------------------- crypto/src/lib.rs | 1 - crypto/src/mls/external_commit.rs | 31 +---------- 9 files changed, 3 insertions(+), 144 deletions(-) diff --git a/crypto-ffi/bindings/js/src/CoreCrypto.ts b/crypto-ffi/bindings/js/src/CoreCrypto.ts index c857d43249..d44aa47c2f 100644 --- a/crypto-ffi/bindings/js/src/CoreCrypto.ts +++ b/crypto-ffi/bindings/js/src/CoreCrypto.ts @@ -53,7 +53,6 @@ export type { GroupInfoBundle, BufferedDecryptedMessage, CommitBundle, - ConversationInitBundle, DecryptedMessage, } from "./CoreCryptoMLS.js"; diff --git a/crypto-ffi/bindings/js/src/CoreCryptoContext.ts b/crypto-ffi/bindings/js/src/CoreCryptoContext.ts index ff2466b7ad..d0b959b14a 100644 --- a/crypto-ffi/bindings/js/src/CoreCryptoContext.ts +++ b/crypto-ffi/bindings/js/src/CoreCryptoContext.ts @@ -13,7 +13,6 @@ import { ClientId, CommitBundle, ConversationId, - ConversationInitBundle, CredentialType, DecryptedMessage, WelcomeBundle, diff --git a/crypto-ffi/bindings/js/src/CoreCryptoInstance.ts b/crypto-ffi/bindings/js/src/CoreCryptoInstance.ts index 0b1c7583e7..59af28989a 100644 --- a/crypto-ffi/bindings/js/src/CoreCryptoInstance.ts +++ b/crypto-ffi/bindings/js/src/CoreCryptoInstance.ts @@ -36,7 +36,6 @@ import { ConversationId, ClientId, Ciphersuite, - ConversationInitBundle, WelcomeBundle, CommitBundle, DecryptedMessage, diff --git a/crypto-ffi/bindings/js/src/CoreCryptoMLS.ts b/crypto-ffi/bindings/js/src/CoreCryptoMLS.ts index 4ec55fb82c..c7a3293425 100644 --- a/crypto-ffi/bindings/js/src/CoreCryptoMLS.ts +++ b/crypto-ffi/bindings/js/src/CoreCryptoMLS.ts @@ -172,32 +172,6 @@ export enum RatchetTreeType { ByRef = 0x03, } -export interface ConversationInitBundle { - /** - * Conversation ID of the conversation created - * - * @readonly - */ - conversationId: ConversationId; - /** - * TLS-serialized MLS External Commit that needs to be fanned out - * - * @readonly - */ - commit: Uint8Array; - /** - * MLS Public Group State (aka Group Info) which becomes valid when the external commit - * is accepted by the Delivery Service - * - * @readonly - */ - groupInfo: GroupInfoBundle; - /** - * New CRL distribution points that appeared by the introduction of a new credential - */ - crlNewDistributionPoints?: string[]; -} - export interface WelcomeBundle { /** * Conversation ID diff --git a/crypto-ffi/bindings/jvm/src/main/kotlin/com/wire/crypto/MlsModel.kt b/crypto-ffi/bindings/jvm/src/main/kotlin/com/wire/crypto/MlsModel.kt index 0f3e784ab7..39063b5fc3 100644 --- a/crypto-ffi/bindings/jvm/src/main/kotlin/com/wire/crypto/MlsModel.kt +++ b/crypto-ffi/bindings/jvm/src/main/kotlin/com/wire/crypto/MlsModel.kt @@ -233,14 +233,6 @@ data class CommitBundle( fun com.wire.crypto.uniffi.CommitBundle.lift() = CommitBundle(commit.toMlsMessage(), welcome?.toWelcome(), groupInfo.lift(), null) -fun com.wire.crypto.uniffi.ConversationInitBundle.lift() = - CommitBundle( - commit.toMlsMessage(), - null, - groupInfo.lift(), - crlNewDistributionPoints?.toCrlDistributionPoint(), - ) - /** Returned when a Proposal is created. Helps roll backing a local proposal */ data class ProposalBundle( /** The proposal message to send to the DS */ diff --git a/crypto-ffi/src/generic/mod.rs b/crypto-ffi/src/generic/mod.rs index 2430f425a8..492f1759c8 100644 --- a/crypto-ffi/src/generic/mod.rs +++ b/crypto-ffi/src/generic/mod.rs @@ -35,8 +35,8 @@ use core_crypto::{ prelude::{ ClientIdentifier, EntropySeed, KeyPackageIn, KeyPackageRef, MlsBufferedConversationDecryptMessage, MlsCentral, MlsCentralConfiguration, MlsCiphersuite, MlsCommitBundle, MlsConversationConfiguration, - MlsConversationDecryptMessage, MlsConversationInitBundle, MlsCustomConfiguration, MlsGroupInfoBundle, - MlsProposalBundle, VerifiableGroupInfo, + MlsConversationDecryptMessage, MlsCustomConfiguration, MlsGroupInfoBundle, MlsProposalBundle, + VerifiableGroupInfo, }, InnermostErrorMessage, RecursiveError, }; @@ -624,29 +624,6 @@ impl TryFrom for ProposalBundle { } } -#[derive(Debug, uniffi::Record)] -pub struct ConversationInitBundle { - pub conversation_id: Vec, - pub commit: Vec, - pub group_info: GroupInfoBundle, - pub crl_new_distribution_points: Option>, -} - -impl TryFrom for ConversationInitBundle { - type Error = CoreCryptoError; - - fn try_from(mut from: MlsConversationInitBundle) -> Result { - let conversation_id = std::mem::take(&mut from.conversation_id); - let (commit, gi, crl_new_distribution_points) = from.to_bytes()?; - Ok(Self { - conversation_id, - commit, - group_info: gi.into(), - crl_new_distribution_points: crl_new_distribution_points.into(), - }) - } -} - #[derive(Debug, uniffi::Record)] /// See [core_crypto::prelude::decrypt::MlsConversationDecryptMessage] pub struct DecryptedMessage { diff --git a/crypto-ffi/src/wasm/mod.rs b/crypto-ffi/src/wasm/mod.rs index b0707affe5..f3416c2b55 100644 --- a/crypto-ffi/src/wasm/mod.rs +++ b/crypto-ffi/src/wasm/mod.rs @@ -672,57 +672,6 @@ impl TryFrom for ProposalBundle { } } -#[wasm_bindgen(skip_jsdoc, getter_with_clone)] -#[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -pub struct ConversationInitBundle { - conversation_id: ConversationId, - commit: Vec, - group_info: GroupInfoBundle, - /// New CRL Distribution of members of this group - crl_new_distribution_points: Option>, -} - -#[wasm_bindgen] -impl ConversationInitBundle { - #[wasm_bindgen(getter)] - pub fn conversation_id(&self) -> Uint8Array { - Uint8Array::from(&*self.conversation_id) - } - - #[wasm_bindgen(getter)] - pub fn commit(&self) -> Uint8Array { - Uint8Array::from(&*self.commit) - } - - #[wasm_bindgen(getter)] - pub fn group_info(&self) -> GroupInfoBundle { - self.group_info.clone() - } - - #[wasm_bindgen(getter)] - pub fn crl_new_distribution_points(&self) -> Option { - self.crl_new_distribution_points - .clone() - .map(|crl_dp| crl_dp.iter().cloned().map(JsValue::from).collect::()) - } -} - -impl TryFrom for ConversationInitBundle { - type Error = CoreCryptoError; - - fn try_from(mut from: MlsConversationInitBundle) -> Result { - let conversation_id = std::mem::take(&mut from.conversation_id); - let (commit, pgs, crl_new_distribution_points) = from.to_bytes()?; - - Ok(Self { - conversation_id, - commit, - group_info: pgs.into(), - crl_new_distribution_points: crl_new_distribution_points.into(), - }) - } -} - #[wasm_bindgen] #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] /// see [core_crypto::prelude::WelcomeBundle] diff --git a/crypto/src/lib.rs b/crypto/src/lib.rs index 674170d735..dc8476792a 100644 --- a/crypto/src/lib.rs +++ b/crypto/src/lib.rs @@ -94,7 +94,6 @@ pub mod prelude { ConversationId, MlsConversation, }, credential::{typ::MlsCredentialType, x509::CertificateBundle}, - external_commit::MlsConversationInitBundle, proposal::{MlsProposal, MlsProposalRef}, MlsCentral, }, diff --git a/crypto/src/mls/external_commit.rs b/crypto/src/mls/external_commit.rs index f338d71869..3a9583fa7a 100644 --- a/crypto/src/mls/external_commit.rs +++ b/crypto/src/mls/external_commit.rs @@ -14,9 +14,8 @@ // You should have received a copy of the GNU General Public License // along with this program. If not, see http://www.gnu.org/licenses/. -use openmls::prelude::{group_info::VerifiableGroupInfo, MlsGroup, MlsMessageOut}; +use openmls::prelude::{group_info::VerifiableGroupInfo, MlsGroup}; use openmls_traits::OpenMlsCryptoProvider; -use tls_codec::Serialize; use core_crypto_keystore::{ connection::FetchFromDatabase, @@ -27,7 +26,6 @@ use core_crypto_keystore::{ use super::Result; use crate::{ context::CentralContext, - e2e_identity::init_certificates::NewCrlDistributionPoint, mls, mls::credential::crl::{extract_crl_uris_from_group, get_new_crl_distribution_points}, prelude::{ @@ -39,33 +37,6 @@ use crate::{ use crate::prelude::{MlsCommitBundle, WelcomeBundle}; -/// Returned when a commit is created -#[derive(Debug)] -pub struct MlsConversationInitBundle { - /// Identifier of the conversation joined by external commit - pub conversation_id: ConversationId, - /// The external commit message - pub commit: MlsMessageOut, - /// `GroupInfo` which becomes valid when the external commit is accepted by the Delivery Service - pub group_info: MlsGroupInfoBundle, - /// New CRL distribution points that appeared by the introduction of a new credential - pub crl_new_distribution_points: NewCrlDistributionPoint, -} - -impl MlsConversationInitBundle { - /// Serializes both wrapped objects into TLS and return them as a tuple of byte arrays. - /// 0 -> external commit - /// 1 -> public group state - #[allow(clippy::type_complexity)] - pub fn to_bytes(self) -> Result<(Vec, MlsGroupInfoBundle, NewCrlDistributionPoint)> { - let commit = self - .commit - .tls_serialize_detached() - .map_err(MlsError::wrap("serializing detached commit"))?; - Ok((commit, self.group_info, self.crl_new_distribution_points)) - } -} - impl CentralContext { /// Issues an external commit and stores the group in a temporary table. This method is /// intended for example when a new client wants to join the user's existing groups. From 6b65f7d52f3e06d701026a32b288db4052c2cb0d Mon Sep 17 00:00:00 2001 From: SimonThormeyer Date: Fri, 17 Jan 2025 09:44:51 +0100 Subject: [PATCH 2/7] refactor: use wasm_bindgen `WelcomeBundle` directly No need for a wrapper type. It is identical. --- crypto-ffi/bindings/js/src/CoreCrypto.ts | 1 + .../bindings/js/src/CoreCryptoContext.ts | 26 ++++++------------- crypto-ffi/bindings/js/src/CoreCryptoMLS.ts | 17 ++---------- crypto-ffi/src/wasm/mod.rs | 4 ++- 4 files changed, 14 insertions(+), 34 deletions(-) diff --git a/crypto-ffi/bindings/js/src/CoreCrypto.ts b/crypto-ffi/bindings/js/src/CoreCrypto.ts index d44aa47c2f..dc4d6650ec 100644 --- a/crypto-ffi/bindings/js/src/CoreCrypto.ts +++ b/crypto-ffi/bindings/js/src/CoreCrypto.ts @@ -41,6 +41,7 @@ export { GroupInfoEncryptionType, RatchetTreeType, DeviceStatus, + WelcomeBundle, } from "./CoreCryptoMLS.js"; export type { ProposalRef, diff --git a/crypto-ffi/bindings/js/src/CoreCryptoContext.ts b/crypto-ffi/bindings/js/src/CoreCryptoContext.ts index d0b959b14a..7467553f08 100644 --- a/crypto-ffi/bindings/js/src/CoreCryptoContext.ts +++ b/crypto-ffi/bindings/js/src/CoreCryptoContext.ts @@ -323,9 +323,9 @@ export class CoreCryptoContext { /** * Ingest a TLS-serialized MLS welcome message to join an existing MLS group * - * Important: you have to catch the error with this reason "Although this Welcome seems valid, the local KeyPackage - * it references has already been deleted locally. Join this group with an external commit", ignore it and then try - * to join this group with an external commit. + * You have to catch the error with this reason "Although this Welcome seems valid, the local KeyPackage + * it references has already been deleted locally. Join this group with an external commit", ignore it and then + * join this group via {@link CoreCryptoContext.joinByExternalCommit}. * * @param welcomeMessage - TLS-serialized MLS Welcome message * @param configuration - configuration of the MLS group @@ -335,21 +335,11 @@ export class CoreCryptoContext { welcomeMessage: Uint8Array, configuration: Partial = {} ): Promise { - try { - const { keyRotationSpan, wirePolicy } = configuration || {}; - const config = new CustomConfiguration(keyRotationSpan, wirePolicy); - const ffiRet: CoreCryptoFfiTypes.WelcomeBundle = - await CoreCryptoError.asyncMapErr( - this.#ctx.process_welcome_message(welcomeMessage, config) - ); - - return { - id: ffiRet.id, - crlNewDistributionPoints: ffiRet.crl_new_distribution_points, - }; - } catch (e) { - throw CoreCryptoError.fromStdError(e as Error); - } + const { keyRotationSpan, wirePolicy } = configuration || {}; + const config = new CustomConfiguration(keyRotationSpan, wirePolicy); + return await CoreCryptoError.asyncMapErr( + this.#ctx.process_welcome_message(welcomeMessage, config) + ); } /** diff --git a/crypto-ffi/bindings/js/src/CoreCryptoMLS.ts b/crypto-ffi/bindings/js/src/CoreCryptoMLS.ts index c7a3293425..fcd2b5dd41 100644 --- a/crypto-ffi/bindings/js/src/CoreCryptoMLS.ts +++ b/crypto-ffi/bindings/js/src/CoreCryptoMLS.ts @@ -5,6 +5,8 @@ import { WireIdentity, } from "./core-crypto-ffi.js"; +export { WelcomeBundle } from "./core-crypto-ffi.js"; + /** * see [core_crypto::prelude::CiphersuiteName] */ @@ -172,21 +174,6 @@ export enum RatchetTreeType { ByRef = 0x03, } -export interface WelcomeBundle { - /** - * Conversation ID - * - * @readonly - */ - id: Uint8Array; - /** - * New CRL Distribution of members of this group - * - * @readonly - */ - crlNewDistributionPoints?: string[]; -} - /** * This is a wrapper for all the possible outcomes you can get after decrypting a message */ diff --git a/crypto-ffi/src/wasm/mod.rs b/crypto-ffi/src/wasm/mod.rs index f3416c2b55..0898897595 100644 --- a/crypto-ffi/src/wasm/mod.rs +++ b/crypto-ffi/src/wasm/mod.rs @@ -674,7 +674,6 @@ impl TryFrom for ProposalBundle { #[wasm_bindgen] #[derive(Debug, Clone, serde::Serialize, serde::Deserialize)] -/// see [core_crypto::prelude::WelcomeBundle] pub struct WelcomeBundle { /// Identifier of the joined conversation id: ConversationId, @@ -684,12 +683,15 @@ pub struct WelcomeBundle { #[wasm_bindgen] impl WelcomeBundle { + /// Identifier of the joined conversation #[wasm_bindgen(getter)] pub fn id(&self) -> Uint8Array { Uint8Array::from(&*self.id) } + /// New CRL Distribution of members of this group #[wasm_bindgen(getter)] + #[wasm_bindgen(js_name = "crlNewDistributionPoints")] pub fn crl_new_distribution_points(&self) -> Option { self.crl_new_distribution_points .clone() From 286f114d96477ed3d4e95e5228f986158a9fcce8 Mon Sep 17 00:00:00 2001 From: SimonThormeyer Date: Fri, 17 Jan 2025 10:25:47 +0100 Subject: [PATCH 3/7] fix(web): fix TS wrapper according to mls transport API changes [WPB-15407] --- crypto-ffi/bindings/js/src/CoreCrypto.ts | 1 - .../bindings/js/src/CoreCryptoContext.ts | 112 ++++-------------- .../bindings/js/src/CoreCryptoInstance.ts | 11 +- 3 files changed, 28 insertions(+), 96 deletions(-) diff --git a/crypto-ffi/bindings/js/src/CoreCrypto.ts b/crypto-ffi/bindings/js/src/CoreCrypto.ts index dc4d6650ec..e56853722e 100644 --- a/crypto-ffi/bindings/js/src/CoreCrypto.ts +++ b/crypto-ffi/bindings/js/src/CoreCrypto.ts @@ -48,7 +48,6 @@ export type { MlsTransportResponse, ConversationId, ClientId, - WelcomeBundle, ProposalBundle, MlsTransport, GroupInfoBundle, diff --git a/crypto-ffi/bindings/js/src/CoreCryptoContext.ts b/crypto-ffi/bindings/js/src/CoreCryptoContext.ts index 7467553f08..4eb0dbc8bc 100644 --- a/crypto-ffi/bindings/js/src/CoreCryptoContext.ts +++ b/crypto-ffi/bindings/js/src/CoreCryptoContext.ts @@ -11,7 +11,6 @@ import { CoreCryptoError } from "./CoreCryptoError.js"; import { Ciphersuite, ClientId, - CommitBundle, ConversationId, CredentialType, DecryptedMessage, @@ -449,119 +448,56 @@ export class CoreCryptoContext { } /** - * Creates an update commit which forces every client to update their LeafNode in the conversation + * Update the keying material of the conversation. * * @param conversationId - The ID of the conversation - * - * @returns A {@link CommitBundle} */ - async updateKeyingMaterial( - conversationId: ConversationId - ): Promise { - try { - const ffiRet: CoreCryptoFfiTypes.CommitBundle = - await CoreCryptoError.asyncMapErr( - this.#ctx.update_keying_material(conversationId) - ); - - const gi = ffiRet.group_info; - - return { - welcome: ffiRet.welcome, - commit: ffiRet.commit, - groupInfo: { - encryptionType: gi.encryption_type, - ratchetTreeType: gi.ratchet_tree_type, - payload: gi.payload, - }, - }; - } catch (e) { - throw CoreCryptoError.fromStdError(e as Error); - } + async updateKeyingMaterial(conversationId: ConversationId): Promise { + return await CoreCryptoError.asyncMapErr( + this.#ctx.update_keying_material(conversationId) + ); } /** - * Commits the local pending proposals and returns the {@link CommitBundle} object containing what can result from this operation. + * Commits the local pending proposals. * - * @param conversationId - The ID of the conversation + * Sends the corresponding commit via {@link MlsTransport.sendCommitBundle} + * and merges it if the call is successful. * - * @returns A {@link CommitBundle} or `undefined` when there was no pending proposal to commit + * @param conversationId - The ID of the conversation */ async commitPendingProposals( conversationId: ConversationId - ): Promise { - try { - const ffiCommitBundle: CoreCryptoFfiTypes.CommitBundle | undefined = - await CoreCryptoError.asyncMapErr( - this.#ctx.commit_pending_proposals(conversationId) - ); - - if (!ffiCommitBundle) { - return undefined; - } - - const gi = ffiCommitBundle.group_info; - - return { - welcome: ffiCommitBundle.welcome, - commit: ffiCommitBundle.commit, - groupInfo: { - encryptionType: gi.encryption_type, - ratchetTreeType: gi.ratchet_tree_type, - payload: gi.payload, - }, - }; - } catch (e) { - throw CoreCryptoError.fromStdError(e as Error); - } + ): Promise { + return await CoreCryptoError.asyncMapErr( + this.#ctx.commit_pending_proposals(conversationId) + ); } /** - * Allows to create an external commit to "apply" to join a group through its GroupInfo. + * "Apply" to join a group through its GroupInfo. * - * If the Delivery Service rejects it, you can retry by just - * calling again the function again. + * Sends the corresponding commit via {@link MlsTransport.sendCommitBundle} + * and creates the group if the call is successful. * * @param groupInfo - a TLS encoded GroupInfo fetched from the Delivery Service * @param credentialType - kind of Credential to use for joining this group. If {@link CredentialType.Basic} is * chosen and no Credential has been created yet for it, a new one will be generated. * @param configuration - configuration of the MLS group * When {@link CredentialType.X509} is chosen, it fails when no Credential has been created for the given {@link Ciphersuite}. - * @returns see {@link ConversationInitBundle} + * + * @return see {@link WelcomeBundle} */ async joinByExternalCommit( groupInfo: Uint8Array, credentialType: CredentialType, configuration: Partial = {} - ): Promise { - try { - const { keyRotationSpan, wirePolicy } = configuration || {}; - const config = new CustomConfiguration(keyRotationSpan, wirePolicy); - const ffiInitMessage: CoreCryptoFfiTypes.ConversationInitBundle = - await CoreCryptoError.asyncMapErr( - this.#ctx.join_by_external_commit( - groupInfo, - config, - credentialType - ) - ); - - const gi = ffiInitMessage.group_info; - - return { - conversationId: ffiInitMessage.conversation_id, - commit: ffiInitMessage.commit, - groupInfo: { - encryptionType: gi.encryption_type, - ratchetTreeType: gi.ratchet_tree_type, - payload: gi.payload, - }, - crlNewDistributionPoints: - ffiInitMessage.crl_new_distribution_points, - }; - } catch (e) { - throw CoreCryptoError.fromStdError(e as Error); - } + ): Promise { + const { keyRotationSpan, wirePolicy } = configuration || {}; + const config = new CustomConfiguration(keyRotationSpan, wirePolicy); + return await CoreCryptoError.asyncMapErr( + this.#ctx.join_by_external_commit(groupInfo, config, credentialType) + ); } /** diff --git a/crypto-ffi/bindings/js/src/CoreCryptoInstance.ts b/crypto-ffi/bindings/js/src/CoreCryptoInstance.ts index 59af28989a..9eea4a95d7 100644 --- a/crypto-ffi/bindings/js/src/CoreCryptoInstance.ts +++ b/crypto-ffi/bindings/js/src/CoreCryptoInstance.ts @@ -26,6 +26,7 @@ import { WireIdentity, ConversationConfiguration, CustomConfiguration, + WelcomeBundle, } from "./core-crypto-ffi.js"; import { CoreCryptoError } from "./CoreCryptoError.js"; @@ -36,8 +37,6 @@ import { ConversationId, ClientId, Ciphersuite, - WelcomeBundle, - CommitBundle, DecryptedMessage, MlsTransport, } from "./CoreCryptoMLS.js"; @@ -668,9 +667,7 @@ export class CoreCrypto { * @deprecated Create a transaction with {@link CoreCrypto.transaction} * and use {@link CoreCryptoContext.updateKeyingMaterial} instead. */ - async updateKeyingMaterial( - conversationId: ConversationId - ): Promise { + async updateKeyingMaterial(conversationId: ConversationId): Promise { return await this.transaction( async (ctx) => await ctx.updateKeyingMaterial(conversationId) ); @@ -696,7 +693,7 @@ export class CoreCrypto { */ async commitPendingProposals( conversationId: ConversationId - ): Promise { + ): Promise { return await this.transaction( async (ctx) => await ctx.commitPendingProposals(conversationId) ); @@ -712,7 +709,7 @@ export class CoreCrypto { groupInfo: Uint8Array, credentialType: CredentialType, configuration: Partial = {} - ): Promise { + ): Promise { return await this.transaction( async (ctx) => await ctx.joinByExternalCommit( From 4937eade1ee673011ea117f8bc92b93fe86804c8 Mon Sep 17 00:00:00 2001 From: SimonThormeyer Date: Fri, 17 Jan 2025 10:31:26 +0100 Subject: [PATCH 4/7] refactor: remove unnecessary try/catch in `createConversation()`. This try/catch is unnecessary. We don't process the result of the promise here, so we don't need to try on it. --- .../bindings/js/src/CoreCryptoContext.ts | 40 +++++++++---------- 1 file changed, 18 insertions(+), 22 deletions(-) diff --git a/crypto-ffi/bindings/js/src/CoreCryptoContext.ts b/crypto-ffi/bindings/js/src/CoreCryptoContext.ts index 4eb0dbc8bc..7fd667287f 100644 --- a/crypto-ffi/bindings/js/src/CoreCryptoContext.ts +++ b/crypto-ffi/bindings/js/src/CoreCryptoContext.ts @@ -214,28 +214,24 @@ export class CoreCryptoContext { creatorCredentialType: CredentialType, configuration: Partial = {} ) { - try { - const { - ciphersuite, - externalSenders, - custom = {}, - } = configuration || {}; - const config = new ConversationConfiguration( - ciphersuite, - externalSenders, - (custom as CustomConfiguration)?.keyRotationSpan, - (custom as CustomConfiguration)?.wirePolicy - ); - return await CoreCryptoError.asyncMapErr( - this.#ctx.create_conversation( - conversationId, - creatorCredentialType, - config - ) - ); - } catch (e) { - throw CoreCryptoError.fromStdError(e as Error); - } + const { + ciphersuite, + externalSenders, + custom = {}, + } = configuration || {}; + const config = new ConversationConfiguration( + ciphersuite, + externalSenders, + (custom as CustomConfiguration)?.keyRotationSpan, + (custom as CustomConfiguration)?.wirePolicy + ); + return await CoreCryptoError.asyncMapErr( + this.#ctx.create_conversation( + conversationId, + creatorCredentialType, + config + ) + ); } /** From 9d0dc599513073a83003860df2197d3075a7c8d6 Mon Sep 17 00:00:00 2001 From: SimonThormeyer Date: Fri, 17 Jan 2025 13:10:47 +0100 Subject: [PATCH 5/7] fix(web): broken error type mapping in try/catch patterns. Apparently the `CoreCryptoError.fromStdError()` function does not work as expected. We can easily fix error type mapping by doing an early return at least for the cases where this function is called with an error that is already an instance of `CoreCryptoError`. Now calling this function at least doesn't make things worse. --- crypto-ffi/bindings/js/src/CoreCryptoError.ts | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crypto-ffi/bindings/js/src/CoreCryptoError.ts b/crypto-ffi/bindings/js/src/CoreCryptoError.ts index 53f9ebd4aa..35a1435055 100644 --- a/crypto-ffi/bindings/js/src/CoreCryptoError.ts +++ b/crypto-ffi/bindings/js/src/CoreCryptoError.ts @@ -55,6 +55,9 @@ export class CoreCryptoError extends Error { } static fromStdError(e: Error): CoreCryptoError | Error { + if (e instanceof CoreCryptoError) { + return e; + } const opts = { cause: e.cause || undefined, stack: e.stack || undefined, From 7bfa9053b25bd7793be506d621d88b27d79ee570 Mon Sep 17 00:00:00 2001 From: SimonThormeyer Date: Fri, 17 Jan 2025 11:55:13 +0100 Subject: [PATCH 6/7] test: update error type mapping test --- crypto-ffi/bindings/js/test/CoreCrypto.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crypto-ffi/bindings/js/test/CoreCrypto.test.ts b/crypto-ffi/bindings/js/test/CoreCrypto.test.ts index 52b83e044d..7693f9f988 100644 --- a/crypto-ffi/bindings/js/test/CoreCrypto.test.ts +++ b/crypto-ffi/bindings/js/test/CoreCrypto.test.ts @@ -889,6 +889,10 @@ describe("Error type mapping", () => { createConversation(ALICE_ID, CONV_ID) // wdio wraps the error and prepends the original message with // the error type as prefix - ).rejects.toThrowError(new Error(`Error: ${expectedErrorMessage}`)); + ).rejects.toThrowError( + new Error( + `MlsErrorConversationAlreadyExists: ${expectedErrorMessage}` + ) + ); }); }); From 923496d7c62ec4a3b6174aba5f584169aa7c40f8 Mon Sep 17 00:00:00 2001 From: SimonThormeyer Date: Fri, 17 Jan 2025 10:50:45 +0100 Subject: [PATCH 7/7] docs: refine kotlin wrapper docs --- .../com/wire/crypto/CoreCryptoContext.kt | 37 +++++++------------ 1 file changed, 13 insertions(+), 24 deletions(-) diff --git a/crypto-ffi/bindings/jvm/src/main/kotlin/com/wire/crypto/CoreCryptoContext.kt b/crypto-ffi/bindings/jvm/src/main/kotlin/com/wire/crypto/CoreCryptoContext.kt index 0ac9ae06cb..2cd2c2a8e1 100644 --- a/crypto-ffi/bindings/jvm/src/main/kotlin/com/wire/crypto/CoreCryptoContext.kt +++ b/crypto-ffi/bindings/jvm/src/main/kotlin/com/wire/crypto/CoreCryptoContext.kt @@ -148,9 +148,7 @@ class CoreCryptoContext(private val cc: com.wire.crypto.uniffi.CoreCryptoContext /** * Prunes local KeyPackages after making sure they also have been deleted on the backend side. - * You should only use this after [CoreCryptoCentral.e2eiRotateAll] - * - * @param refs KeyPackage references from the [RotateBundle] + * You should only use this after calling [e2eiRotate] on all conversations. */ suspend fun deleteKeyPackages(refs: List) { // cannot be tested with the current API & helpers @@ -172,18 +170,14 @@ class CoreCryptoContext(private val cc: com.wire.crypto.uniffi.CoreCryptoContext suspend fun conversationEpoch(id: MLSGroupId): ULong = wrapException { cc.conversationEpoch(id.lower()) } /** - * Allows to create an external commit to "apply" to join a group through its GroupInfo. + * "Apply" to join a group through its GroupInfo. * - * If the DS accepts the external commit, you have to [mergePendingGroupFromExternalCommit] in - * order to get back a functional MLS group. On the opposite, if it rejects it, you can either - * retry by just calling again [joinByExternalCommit], no need to - * [clearPendingGroupFromExternalCommit]. If you want to abort the operation (too many retries - * or the user decided to abort), you can use [clearPendingGroupFromExternalCommit] in order not - * to bloat the user's storage but nothing bad can happen if you forget to except some storage - * space wasted. + * Sends the corresponding commit via [MlsTransport.sendCommitBundle] + * and creates the group if the call is successful. * * @param groupInfo a TLS encoded GroupInfo fetched from the Delivery Service * @param credentialType to join the group with + * @param configuration configuration of the MLS group */ suspend fun joinByExternalCommit( groupInfo: GroupInfo, @@ -274,7 +268,7 @@ class CoreCryptoContext(private val cc: com.wire.crypto.uniffi.CoreCryptoContext * * @param id conversation identifier * @param keyPackages of the new clients to add - * @return a [CommitBundle] to upload to the backend and if it succeeds call [commitAccepted] + * @return the potentially newly discovered certificate revocation list distribution points */ suspend fun addMember(id: MLSGroupId, keyPackages: List): List? { return wrapException { cc.addClientsToConversation(id.lower(), keyPackages.map { it.lower() }) } @@ -286,7 +280,6 @@ class CoreCryptoContext(private val cc: com.wire.crypto.uniffi.CoreCryptoContext * * @param id conversation identifier * @param members client identifier to delete - * @return a [CommitBundle] to upload to the backend and if it succeeds call [commitAccepted] */ suspend fun removeMember(id: MLSGroupId, members: List) { return wrapException { @@ -300,16 +293,13 @@ class CoreCryptoContext(private val cc: com.wire.crypto.uniffi.CoreCryptoContext * conversation. * * @param id conversation identifier - * @return a [CommitBundle] to upload to the backend and if it succeeds call [commitAccepted] */ suspend fun updateKeyingMaterial(id: MLSGroupId) = wrapException { cc.updateKeyingMaterial(id.lower()) } /** - * Commits the local pending proposals and returns the {@link CommitBundle} object containing - * what can result from this operation. + * Commits the local pending proposals. * * @param id conversation identifier - * @return a [CommitBundle] to upload to the backend and if it succeeds call [commitAccepted] */ suspend fun commitPendingProposals(id: MLSGroupId) { return wrapException { cc.commitPendingProposals(id.lower()) } @@ -341,8 +331,6 @@ class CoreCryptoContext(private val cc: com.wire.crypto.uniffi.CoreCryptoContext * be used to initialize a subconversation * * @param id conversation identifier - * @param keyLength the length of the key to be derived. If the value is higher than the bounds - * of `u16` or the context hash * 255, an error will be returned */ suspend fun getExternalSender(id: MLSGroupId): ExternalSenderKey { return wrapException { cc.getExternalSender(id.lower()).toExternalSenderKey() } @@ -579,14 +567,14 @@ class CoreCryptoContext(private val cc: com.wire.crypto.uniffi.CoreCryptoContext } /** - * Creates an update commit which replaces your leaf containing basic credentials with a leaf + * Replaces your leaf containing basic credentials with a leaf * node containing x509 credentials in the conversation. * - * NOTE: you can only call this after you've completed the enrollment for an end-to-end - * identity, calling this without a valid end-to-end identity will result in an error. + * NOTE: you can only call this after you've completed the enrollment for an end-to-end identity, and saved the + * resulting credential with [saveX509Credential]. + * Calling this without a valid end-to-end identity will result in an error. * * @param id conversation identifier - * @return a [CommitBundle] to upload to the backend and if it succeeds call [commitAccepted] */ suspend fun e2eiRotate(id: MLSGroupId) = wrapException { cc.e2eiRotate(id.lower()) } @@ -605,7 +593,8 @@ class CoreCryptoContext(private val cc: com.wire.crypto.uniffi.CoreCryptoContext * * @param enrollment - the enrollment instance used to fetch the certificates * @param certificateChain - the raw response from ACME server - * @return Potentially a list of new crl distribution points discovered in the certificate chain + * @return Potentially a list of new certificate revocation list distribution points discovered in the certificate + * chain */ suspend fun saveX509Credential( enrollment: E2EIEnrollment,