Skip to content

refactor: let mls_init() take ClientId directly #1803

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

Merged
fewerner merged 1 commit into from
Feb 4, 2026
Merged

refactor: let mls_init() take ClientId directly #1803

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
10 changes: 3 additions & 7 deletions crypto-ffi/src/core_crypto_context/mls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,8 @@
use std::{sync::Arc, time::Duration};

use core_crypto::{
Ciphersuite as CryptoCiphersuite, ClientIdentifier, CredentialFindFilters, MlsConversationConfiguration,
RecursiveError, VerifiableGroupInfo, mls::conversation::Conversation as _,
transaction_context::Error as TransactionError,
Ciphersuite as CryptoCiphersuite, CredentialFindFilters, MlsConversationConfiguration, RecursiveError,
VerifiableGroupInfo, mls::conversation::Conversation as _, transaction_context::Error as TransactionError,
};
use tls_codec::Deserialize as _;

Expand Down Expand Up @@ -49,10 +48,7 @@ impl CoreCryptoContext {
pub async fn mls_init(&self, client_id: &Arc<ClientId>, transport: Arc<dyn MlsTransport>) -> CoreCryptoResult<()> {
let transport = callback_shim(transport);
self.inner
.mls_init(
ClientIdentifier::Basic(client_id.as_ref().as_ref().to_owned()),
transport,
)
.mls_init(client_id.as_ref().as_ref().to_owned(), transport)
.await?;
Ok(())
}
Expand Down
15 changes: 7 additions & 8 deletions crypto/benches/utils/mls.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,10 +5,10 @@ use std::{

use async_lock::RwLock;
use core_crypto::{
CertificateBundle, Ciphersuite, ClientId, ClientIdentifier, ConnectionType, ConversationId, CoreCrypto,
Credential as CcCredential, CredentialFindFilters, CredentialRef, CredentialType, Database, DatabaseKey,
HistorySecret, MlsCommitBundle, MlsConversationConfiguration, MlsCryptoProvider, MlsGroupInfoBundle, MlsTransport,
MlsTransportData, MlsTransportResponse,
CertificateBundle, Ciphersuite, ClientId, ConnectionType, ConversationId, CoreCrypto, Credential as CcCredential,
CredentialFindFilters, CredentialRef, CredentialType, Database, DatabaseKey, HistorySecret, MlsCommitBundle,
MlsConversationConfiguration, MlsCryptoProvider, MlsGroupInfoBundle, MlsTransport, MlsTransportData,
MlsTransportResponse,
};
use criterion::BenchmarkId;
use openmls::{
Expand Down Expand Up @@ -165,21 +165,20 @@ pub(crate) async fn new_central(
} else {
ConnectionType::Persistent(&path)
};
let client_id = ClientId::from(Alphanumeric.sample_string(&mut rand::thread_rng(), 10).into_bytes());
let client_identifier = ClientIdentifier::from(client_id.clone());
let session_id = ClientId::from(Alphanumeric.sample_string(&mut rand::thread_rng(), 10).into_bytes());
let db = Database::open(connection_type, &DatabaseKey::generate()).await.unwrap();

let cc = CoreCrypto::new(db);
let delivery_service = Arc::<CoreCryptoTransportSuccessProvider>::default();
let tx = cc.new_transaction().await.unwrap();
tx.mls_init(client_identifier, delivery_service.clone()).await.unwrap();
tx.mls_init(session_id.clone(), delivery_service.clone()).await.unwrap();
tx.finish().await.unwrap();

let ctx = cc.new_transaction().await.unwrap();

let credential = match certificate_bundle {
Some(certificate_bundle) => CcCredential::x509(ciphersuite, certificate_bundle.to_owned()).unwrap(),
None => CcCredential::basic(ciphersuite, client_id).unwrap(),
None => CcCredential::basic(ciphersuite, session_id).unwrap(),
};
let credential_ref = ctx.add_credential(credential).await.unwrap();

Expand Down
20 changes: 11 additions & 9 deletions crypto/src/ephemeral.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@ use obfuscate::{Obfuscate, Obfuscated};
use openmls::prelude::KeyPackageSecretEncapsulation;

use crate::{
Ciphersuite, ClientId, ClientIdRef, ClientIdentifier, CoreCrypto, CoreCryptoTransportNotImplementedProvider,
Credential, Error, MlsError, RecursiveError, Result, Session,
Ciphersuite, ClientId, ClientIdRef, CoreCrypto, CoreCryptoTransportNotImplementedProvider, Credential, Error,
MlsError, RecursiveError, Result, Session,
mls_provider::{DatabaseKey, MlsCryptoProvider},
};

Expand Down Expand Up @@ -66,10 +66,9 @@ impl Obfuscate for HistorySecret {
/// This implementation lives here instead of there for organizational reasons.
pub(crate) async fn generate_history_secret(ciphersuite: Ciphersuite) -> Result<HistorySecret> {
// generate a new completely arbitrary client id
let client_id = uuid::Uuid::new_v4();
let client_id = format!("{HISTORY_CLIENT_ID_PREFIX}-{client_id}");
let client_id = ClientId::from(client_id.into_bytes());
let identifier = ClientIdentifier::Basic(client_id.clone());
let session_id = uuid::Uuid::new_v4();
let session_id = format!("{HISTORY_CLIENT_ID_PREFIX}-{session_id}");
let session_id = ClientId::from(session_id.into_bytes());

let database = Database::open(ConnectionType::InMemory, &DatabaseKey::generate())
.await
Expand All @@ -82,14 +81,14 @@ pub(crate) async fn generate_history_secret(ciphersuite: Ciphersuite) -> Result<
.map_err(RecursiveError::transaction("creating new transaction"))?;

let transport = Arc::new(CoreCryptoTransportNotImplementedProvider::default());
tx.mls_init(identifier, transport)
tx.mls_init(session_id.clone(), transport)
.await
.map_err(RecursiveError::transaction("initializing ephemeral cc"))?;
let session = tx
.session()
.await
.map_err(RecursiveError::transaction("Getting mls session"))?;
let credential = Credential::basic(ciphersuite, client_id.clone()).map_err(RecursiveError::mls_credential(
let credential = Credential::basic(ciphersuite, session_id.clone()).map_err(RecursiveError::mls_credential(
"generating basic credential for ephemeral client",
))?;
let credential_ref = tx
Expand All @@ -112,7 +111,10 @@ pub(crate) async fn generate_history_secret(ciphersuite: Ciphersuite) -> Result<
// there
let _ = tx.abort().await;

Ok(HistorySecret { client_id, key_package })
Ok(HistorySecret {
client_id: session_id,
key_package,
})
}

pub(crate) fn is_history_client(client_id: impl Borrow<ClientIdRef>) -> bool {
Expand Down
12 changes: 8 additions & 4 deletions crypto/src/mls/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,16 +110,20 @@ mod tests {
x509_test_chain.register_with_central(&context).await;

// phase 2: init mls_client
let client_id = ClientId::from("alice");
let session_id = ClientId::from("alice");
let identifier = match case.credential_type {
CredentialType::Basic => ClientIdentifier::Basic(client_id.clone()),
CredentialType::Basic => ClientIdentifier::Basic(session_id),
CredentialType::X509 => {
CertificateBundle::rand_identifier(&client_id, &[x509_test_chain.find_local_intermediate_ca()])
CertificateBundle::rand_identifier(&session_id, &[x509_test_chain.find_local_intermediate_ca()])
}
};
let session_id = identifier
.get_id()
.expect("get session_id from identifier")
.into_owned();
context
.mls_init(
identifier.clone(),
session_id.clone(),
Arc::new(CoreCryptoTransportSuccessProvider::default()),
)
.await
Expand Down
14 changes: 9 additions & 5 deletions crypto/src/proteus.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -637,15 +637,19 @@ mod tests {
// proteus is initialized, prekeys can be generated
assert!(transaction.proteus_new_prekey(1).await.is_ok());
// 👇 and so a unique 'client_id' can be fetched from wire-server
let client_id = ClientId::from("alice");
let session_id = ClientId::from("alice");
let transport = Arc::new(CoreCryptoTransportSuccessProvider::default());
let identifier = match case.credential_type {
CredentialType::Basic => ClientIdentifier::Basic(client_id),
CredentialType::Basic => ClientIdentifier::Basic(session_id),
CredentialType::X509 => {
CertificateBundle::rand_identifier(&client_id, &[x509_test_chain.find_local_intermediate_ca()])
CertificateBundle::rand_identifier(&session_id, &[x509_test_chain.find_local_intermediate_ca()])
}
};
let transport = Arc::new(CoreCryptoTransportSuccessProvider::default());
transaction.mls_init(identifier.clone(), transport).await.unwrap();
let session_id = identifier
.get_id()
.expect("Getting session id from identifier")
.into_owned();
transaction.mls_init(session_id, transport).await.unwrap();
let credential = Credential::from_identifier(&identifier, case.ciphersuite()).unwrap();
let credential_ref = transaction.add_credential(credential).await.unwrap();

Expand Down
43 changes: 25 additions & 18 deletions crypto/src/test_utils/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,12 @@ impl SessionContext {

let transaction = core_crypto.new_transaction().await.unwrap();

let session_id = identifier
.get_id()
.map_err(RecursiveError::mls_client("getting client id"))?
.into_owned();
transaction
.mls_init(identifier.clone(), context.transport.clone())
.mls_init(session_id, context.transport.clone())
.await
.map_err(RecursiveError::transaction("mls init"))?;

Expand Down Expand Up @@ -275,9 +279,9 @@ impl SessionContext {
self.set_session(new_session).await;
}

pub async fn reinit_session(&self, identifier: ClientIdentifier) {
pub async fn reinit_session(&self, session_id: ClientId) {
self.transaction
.mls_init(identifier, self.mls_transport().await)
.mls_init(session_id, self.mls_transport().await)
.await
.unwrap();

Expand Down Expand Up @@ -323,28 +327,31 @@ impl SessionContext {
) -> Result<()> {
let user_uuid = uuid::Uuid::new_v4();
let rnd_id = rand::random::<usize>();
let client_id = format!("{}:{rnd_id:x}@members.wire.com", user_uuid.hyphenated());
let client_id = ClientId(client_id.into_bytes());
let session_id = ClientId(format!("{}:{rnd_id:x}@members.wire.com", user_uuid.hyphenated()).into_bytes());

let credential;
let identifier;
match case.credential_type {
let (session_id, credential) = match case.credential_type {
CredentialType::Basic => {
identifier = ClientIdentifier::Basic(client_id.clone());
credential = Credential::basic(case.ciphersuite(), client_id).unwrap();
let credential =
Credential::basic(case.ciphersuite(), session_id.clone()).expect("creating basic credential ");

(session_id, credential)
}
CredentialType::X509 => {
let signer = signer.expect("Missing intermediate CA").to_owned();
let cert = CertificateBundle::rand(&client_id, &signer);
identifier = ClientIdentifier::X509([(case.signature_scheme(), cert.clone())].into());
credential = Credential::x509(case.ciphersuite(), cert).unwrap();
}
};
let signer = signer.expect("Missing intermediate CA");
let cert = CertificateBundle::rand(&session_id, signer);
let session_id = cert.get_client_id().expect("Getting client id from certificate bundle");

self.reinit_session(identifier).await;
let credential = Credential::x509(case.ciphersuite(), cert).expect("creating x509 credential");

self.transaction.add_credential(credential).await.unwrap();
(session_id, credential)
}
};

self.reinit_session(session_id).await;
self.transaction
.add_credential(credential)
.await
.expect("adding credential");
Ok(())
}
}
Expand Down
15 changes: 6 additions & 9 deletions crypto/src/transaction_context/e2e_identity/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,14 @@ mod init_certificates;
mod rotate;
mod stash;

use std::{
collections::{HashMap, HashSet},
sync::Arc,
};
use std::{collections::HashSet, sync::Arc};

pub use error::{Error, Result};
use wire_e2e_identity::x509_check::extract_crl_uris;

use super::TransactionContext;
use crate::{
CertificateBundle, Ciphersuite, ClientId, ClientIdentifier, Credential, CredentialRef, E2eiEnrollment,
MlsTransport, RecursiveError,
CertificateBundle, Ciphersuite, ClientId, Credential, CredentialRef, E2eiEnrollment, MlsTransport, RecursiveError,
e2e_identity::NewCrlDistributionPoints,
mls::credential::{crl::get_new_crl_distribution_points, x509::CertificatePrivateKey},
};
Expand Down Expand Up @@ -106,9 +102,10 @@ impl TransactionContext {
let credential_ref = credential.save(database).await.map_err(RecursiveError::mls_credential(
"saving credential in e2ei_mls_init_only",
))?;

let identifier = ClientIdentifier::X509(HashMap::from([(ciphersuite.signature_algorithm(), cert_bundle)]));
self.mls_init(identifier, transport)
let session_id = cert_bundle.get_client_id().map_err(RecursiveError::mls_credential(
"Getting session id from certificate bundle",
))?;
self.mls_init(session_id, transport)
.await
.map_err(RecursiveError::transaction("initializing mls"))?;
Ok((credential_ref, crl_new_distribution_points))
Expand Down
2 changes: 1 addition & 1 deletion crypto/src/transaction_context/e2e_identity/rotate.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -430,7 +430,7 @@ mod tests {
keystore.commit_transaction().await.unwrap();
keystore.new_transaction().await.unwrap();

alice.reinit_session(alice.identifier.clone()).await;
alice.reinit_session(alice.get_client_id().await).await;

let new_session = alice.session().await;
// Verify that Alice has the same credentials
Expand Down
12 changes: 4 additions & 8 deletions crypto/src/transaction_context/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,8 @@ use wire_e2e_identity::pki_env::PkiEnvironment;
#[cfg(feature = "proteus")]
use crate::proteus::ProteusCentral;
use crate::{
ClientId, ClientIdentifier, CoreCrypto, CredentialFindFilters, CredentialRef, KeystoreError, MlsConversation,
MlsError, MlsTransport, RecursiveError, Session,
ClientId, CoreCrypto, CredentialFindFilters, CredentialRef, KeystoreError, MlsConversation, MlsError, MlsTransport,
RecursiveError, Session,
group_store::GroupStore,
mls::{self, HasSessionAndCrypto},
mls_provider::{Database, MlsCryptoProvider},
Expand Down Expand Up @@ -261,12 +261,8 @@ impl TransactionContext {
}

/// Initializes the MLS client of [super::CoreCrypto].
pub async fn mls_init(&self, identifier: ClientIdentifier, transport: Arc<dyn MlsTransport>) -> Result<()> {
pub async fn mls_init(&self, session_id: ClientId, transport: Arc<dyn MlsTransport>) -> Result<()> {
let database = self.database().await?;
let client_id = identifier
.get_id()
.map_err(RecursiveError::mls_client("getting client id"))?
.into_owned();

let pki_env_provider = self
.pki_environment_option()
Expand All @@ -276,7 +272,7 @@ impl TransactionContext {

let crypto_provider = MlsCryptoProvider::new_with_pki_env(database, pki_env_provider);
let database = self.database().await?;
let session = Session::new(client_id.clone(), crypto_provider, database, transport);
let session = Session::new(session_id.clone(), crypto_provider, database, transport);
self.set_mls_session(session).await?;

Ok(())
Expand Down
2 changes: 1 addition & 1 deletion interop/src/main.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -160,7 +160,7 @@ async fn run_mls_test(chrome_driver_addr: &std::net::SocketAddr, web_server: &st
let transaction = cc.new_transaction().await?;
let success_provider = Arc::new(MlsTransportSuccessProvider::default());
transaction
.mls_init(master_client_id.clone().into(), success_provider.clone())
.mls_init(master_client_id.clone(), success_provider.clone())
.await?;
let credential_ref = transaction.add_credential(credential).await?;
transaction
Expand Down
Loading