fix: deduplicate CRL DPs

crypto-ffi/src/generic.rs

@@ -226,7 +226,7 @@ impl TryFrom<MlsConversationCreationMessage> for MemberAddedMessages {
             welcome,
             commit,
             group_info: group_info.into(),
-            crl_new_distribution_points,
+            crl_new_distribution_points: crl_new_distribution_points.into(),
         })
     }
 }
@@ -242,7 +242,7 @@ impl From<core_crypto::prelude::WelcomeBundle> for WelcomeBundle {
     fn from(w: core_crypto::prelude::WelcomeBundle) -> Self {
         Self {
             id: w.id,
-            crl_new_distribution_points: w.crl_new_distribution_points,
+            crl_new_distribution_points: w.crl_new_distribution_points.into(),
         }
     }
 }
@@ -367,7 +367,7 @@ impl TryFrom<MlsRotateBundle> for RotateBundle {
             commits,
             new_key_packages,
             key_package_refs_to_remove,
-            crl_new_distribution_points,
+            crl_new_distribution_points: crl_new_distribution_points.into(),
         })
     }
 }
@@ -387,7 +387,7 @@ impl TryFrom<MlsProposalBundle> for ProposalBundle {
         Ok(Self {
             proposal,
             proposal_ref,
-            crl_new_distribution_points,
+            crl_new_distribution_points: crl_new_distribution_points.into(),
         })
     }
 }
@@ -410,7 +410,7 @@ impl TryFrom<MlsConversationInitBundle> for ConversationInitBundle {
             conversation_id,
             commit,
             group_info: gi.into(),
-            crl_new_distribution_points,
+            crl_new_distribution_points: crl_new_distribution_points.into(),
         })
     }
 }
@@ -471,7 +471,7 @@ impl TryFrom<MlsConversationDecryptMessage> for DecryptedMessage {
             has_epoch_changed: from.has_epoch_changed,
             identity: from.identity.map(Into::into),
             buffered_messages,
-            crl_new_distribution_points: from.crl_new_distribution_points,
+            crl_new_distribution_points: from.crl_new_distribution_points.into(),
         })
     }
 }
@@ -494,7 +494,7 @@ impl TryFrom<MlsBufferedConversationDecryptMessage> for BufferedDecryptedMessage
             sender_client_id: from.sender_client_id.map(ClientId),
             has_epoch_changed: from.has_epoch_changed,
             identity: from.identity.map(Into::into),
-            crl_new_distribution_points: from.crl_new_distribution_points,
+            crl_new_distribution_points: from.crl_new_distribution_points.into(),
         })
     }
 }
@@ -1601,7 +1601,8 @@ impl CoreCrypto {
             .lock()
             .await
             .e2ei_register_intermediate_ca_pem(cert_pem)
-            .await?)
+            .await?
+            .into())
     }
 
     /// See [core_crypto::mls::MlsCentral::e2ei_register_crl]
@@ -1645,7 +1646,8 @@ impl CoreCrypto {
             .lock()
             .await
             .e2ei_mls_init_only(e2ei, certificate_chain, nb_key_package)
-            .await?)
+            .await?
+            .into())
     }
 
     /// See [core_crypto::mls::MlsCentral::e2ei_rotate_all]

crypto-ffi/src/wasm.rs

@@ -302,7 +302,7 @@ impl TryFrom<MlsConversationCreationMessage> for MemberAddedMessages {
             welcome,
             commit,
             group_info: pgs.into(),
-            crl_new_distribution_points,
+            crl_new_distribution_points: crl_new_distribution_points.into(),
         })
     }
 }
@@ -460,7 +460,7 @@ impl TryFrom<MlsRotateBundle> for RotateBundle {
             commits,
             new_key_packages,
             key_package_refs_to_remove,
-            crl_new_distribution_points,
+            crl_new_distribution_points: crl_new_distribution_points.into(),
         })
     }
 }
@@ -506,7 +506,7 @@ impl TryFrom<MlsProposalBundle> for ProposalBundle {
         Ok(Self {
             proposal,
             proposal_ref,
-            crl_new_distribution_points,
+            crl_new_distribution_points: crl_new_distribution_points.into(),
         })
     }
 }
@@ -558,7 +558,7 @@ impl TryFrom<MlsConversationInitBundle> for ConversationInitBundle {
             conversation_id,
             commit,
             group_info: pgs.into(),
-            crl_new_distribution_points,
+            crl_new_distribution_points: crl_new_distribution_points.into(),
         })
     }
 }
@@ -592,7 +592,7 @@ impl From<core_crypto::prelude::WelcomeBundle> for WelcomeBundle {
     fn from(w: core_crypto::prelude::WelcomeBundle) -> Self {
         Self {
             id: w.id,
-            crl_new_distribution_points: w.crl_new_distribution_points,
+            crl_new_distribution_points: w.crl_new_distribution_points.into(),
         }
     }
 }
@@ -651,7 +651,7 @@ impl TryFrom<MlsConversationDecryptMessage> for DecryptedMessage {
             has_epoch_changed: from.has_epoch_changed,
             identity: from.identity.map(Into::into),
             buffered_messages,
-            crl_new_distribution_points: from.crl_new_distribution_points,
+            crl_new_distribution_points: from.crl_new_distribution_points.into(),
         })
     }
 }
@@ -759,7 +759,7 @@ impl TryFrom<MlsBufferedConversationDecryptMessage> for BufferedDecryptedMessage
             sender_client_id: from.sender_client_id.map(ClientId::into),
             has_epoch_changed: from.has_epoch_changed,
             identity: from.identity.map(Into::into),
-            crl_new_distribution_points: from.crl_new_distribution_points,
+            crl_new_distribution_points: from.crl_new_distribution_points.into(),
         })
     }
 }
@@ -2534,7 +2534,7 @@ impl CoreCrypto {
                 let this = this.read().await;
                 let crls = this.e2ei_register_intermediate_ca_pem(cert_pem).await?;
 
-                let crls = if let Some(crls) = crls {
+                let crls = if let Some(crls) = &*crls {
                     js_sys::Array::from_iter(crls.into_iter().map(JsValue::from))
                 } else {
                     js_sys::Array::new()
@@ -2582,7 +2582,7 @@ impl CoreCrypto {
                     .e2ei_mls_init_only(enrollment, certificate_chain, nb_key_package)
                     .await?;
 
-                let crls = if let Some(crls) = crls {
+                let crls = if let Some(crls) = &*crls {
                     js_sys::Array::from_iter(crls.into_iter().map(JsValue::from))
                 } else {
                     js_sys::Array::new()

crypto/Cargo.toml

@@ -24,7 +24,7 @@ uniffi = ["dep:uniffi"]
 
 [dependencies]
 thiserror = "1.0"
-derive_more = { version = "0.99", features = ["from", "into", "deref"] }
+derive_more = { version = "0.99", features = ["from", "into", "deref", "deref_mut"] }
 strum = { version = "0.26", features = ["derive"] }
 cfg-if = "1.0"
 hex = "0.4"

crypto/src/e2e_identity/init_certificates.rs

@@ -2,13 +2,23 @@ use crate::{e2e_identity::CrlRegistration, prelude::MlsCentral, CryptoError, Cry
 use core_crypto_keystore::entities::{E2eiAcmeCA, E2eiCrl, E2eiIntermediateCert, EntityBase, UniqueEntity};
 use mls_crypto_provider::MlsCryptoProvider;
 use openmls_traits::OpenMlsCryptoProvider;
+use std::collections::HashSet;
 use std::ops::DerefMut;
 use wire_e2e_identity::prelude::x509::{
     extract_crl_uris, extract_expiration_from_crl,
     revocation::{PkiEnvironment, PkiEnvironmentParams},
 };
 use x509_cert::der::Decode;
 
+#[derive(Debug, Clone, derive_more::From, derive_more::Into, derive_more::Deref, derive_more::DerefMut)]
+pub struct NewCrlDistributionPoint(Option<HashSet<String>>);
+
+impl From<NewCrlDistributionPoint> for Option<Vec<String>> {
+    fn from(mut dp: NewCrlDistributionPoint) -> Self {
+        dp.take().map(|d| d.into_iter().collect())
+    }
+}
+
 impl MlsCentral {
     /// Registers a Root Trust Anchor CA for the use in E2EI processing.
     ///
@@ -64,29 +74,32 @@ impl MlsCentral {
     ///
     /// # Parameters
     /// * `cert_pem` - PEM certificate to register as an Intermediate CA
-    pub async fn e2ei_register_intermediate_ca_pem(&self, cert_pem: String) -> CryptoResult<Option<Vec<String>>> {
+    pub async fn e2ei_register_intermediate_ca_pem(&self, cert_pem: String) -> CryptoResult<NewCrlDistributionPoint> {
         // Parse/decode PEM cert
         let inter_ca = PkiEnvironment::decode_pem_cert(cert_pem).map_err(|e| CryptoError::E2eiError(e.into()))?;
         self.e2ei_register_intermediate_ca(inter_ca).await
     }
 
-    pub(crate) async fn e2ei_register_intermediate_ca_der(&self, cert_der: &[u8]) -> CryptoResult<Option<Vec<String>>> {
+    pub(crate) async fn e2ei_register_intermediate_ca_der(
+        &self,
+        cert_der: &[u8],
+    ) -> CryptoResult<NewCrlDistributionPoint> {
         let inter_ca = x509_cert::Certificate::from_der(cert_der)?;
         self.e2ei_register_intermediate_ca(inter_ca).await
     }
 
     async fn e2ei_register_intermediate_ca(
         &self,
         inter_ca: x509_cert::Certificate,
-    ) -> CryptoResult<Option<Vec<String>>> {
+    ) -> CryptoResult<NewCrlDistributionPoint> {
         // TrustAnchor must have been registered at this point
         let ta = E2eiAcmeCA::find_unique(self.mls_backend.key_store().borrow_conn().await?.deref_mut()).await?;
         let ta = x509_cert::Certificate::from_der(&ta.content)?;
 
         // the `/federation` endpoint from smallstep repeats the root CA
         // so we filter it out here so that clients don't have to do it
         if inter_ca == ta {
-            return Ok(None);
+            return Ok(None.into());
         }
 
         let intermediate_crl = extract_crl_uris(&inter_ca)
@@ -120,7 +133,7 @@ impl MlsCentral {
 
         self.init_pki_env().await?;
 
-        Ok(intermediate_crl)
+        Ok(intermediate_crl.into())
     }
 
     /// Registers a CRL for the use in E2EI processing.

crypto/src/e2e_identity/mod.rs

@@ -6,6 +6,7 @@ use wire_e2e_identity::prelude::{E2eiAcmeAuthorization, RustyE2eIdentity};
 use error::*;
 use mls_crypto_provider::MlsCryptoProvider;
 
+use crate::e2e_identity::init_certificates::NewCrlDistributionPoint;
 use crate::{
     e2e_identity::{crypto::E2eiSignatureKeypair, id::QualifiedE2eiClientId},
     mls::credential::x509::CertificatePrivateKey,
@@ -78,7 +79,7 @@ impl MlsCentral {
         enrollment: E2eiEnrollment,
         certificate_chain: String,
         nb_init_key_packages: Option<usize>,
-    ) -> CryptoResult<Option<Vec<String>>> {
+    ) -> CryptoResult<NewCrlDistributionPoint> {
         let sk = enrollment.get_sign_key_for_mls()?;
         let cs = enrollment.ciphersuite;
         let certificate_chain = enrollment

crypto/src/e2e_identity/rotate.rs

@@ -6,6 +6,7 @@ use openmls_traits::OpenMlsCryptoProvider;
 use core_crypto_keystore::{entities::MlsKeyPackage, CryptoKeystoreMls};
 use mls_crypto_provider::MlsCryptoProvider;
 
+use crate::e2e_identity::init_certificates::NewCrlDistributionPoint;
 use crate::{
     mls::credential::{ext::CredentialExt, x509::CertificatePrivateKey, CredentialBundle},
     prelude::{
@@ -239,7 +240,7 @@ pub struct MlsRotateBundle {
     /// All the now deprecated KeyPackages. Once deleted remotely, delete them locally with [MlsCentral::delete_keypackages]
     pub key_package_refs_to_remove: Vec<KeyPackageRef>,
     /// New CRL distribution points that appeared by the introduction of a new credential
-    pub crl_new_distribution_points: Option<Vec<String>>,
+    pub crl_new_distribution_points: NewCrlDistributionPoint,
 }
 
 impl MlsRotateBundle {
@@ -251,7 +252,7 @@ impl MlsRotateBundle {
         HashMap<String, MlsCommitBundle>,
         Vec<Vec<u8>>,
         Vec<Vec<u8>>,
-        Option<Vec<String>>,
+        NewCrlDistributionPoint,
     )> {
         use openmls::prelude::TlsSerializeTrait as _;
 

crypto/src/mls/conversation/commit.rs

@@ -6,9 +6,11 @@
 //! | 1+ pend. Proposal      | ✅              | ❌              |
 
 use openmls::prelude::{KeyPackageIn, LeafNode, LeafNodeIndex, MlsMessageOut};
+use std::collections::HashSet;
 
 use mls_crypto_provider::MlsCryptoProvider;
 
+use crate::e2e_identity::init_certificates::NewCrlDistributionPoint;
 use crate::{
     mls::credential::{crl::extract_dp, CredentialBundle},
     prelude::{Client, ClientId, ConversationId, CryptoError, CryptoResult, MlsCentral, MlsError, MlsGroupInfoBundle},
@@ -151,15 +153,10 @@ impl MlsConversation {
                 openmls::prelude::MlsCredentialType::X509(cert) => Some(cert),
                 _ => None,
             })
-            .try_fold(vec![], |mut acc, c| {
+            .try_fold(HashSet::new(), |mut acc, c| {
                 acc.extend(extract_dp(c)?);
                 CryptoResult::Ok(acc)
             })?;
-        let crl_new_distribution_points = if crl_new_distribution_points.is_empty() {
-            None
-        } else {
-            Some(crl_new_distribution_points)
-        };
 
         let (commit, welcome, gi) = self
             .group
@@ -173,6 +170,13 @@ impl MlsConversation {
 
         self.persist_group_when_changed(backend, false).await?;
 
+        let crl_new_distribution_points = if crl_new_distribution_points.is_empty() {
+            None
+        } else {
+            Some(crl_new_distribution_points)
+        }
+        .into();
+
         Ok(MlsConversationCreationMessage {
             welcome,
             commit,
@@ -303,7 +307,7 @@ pub struct MlsConversationCreationMessage {
     /// `GroupInfo` if the commit is merged
     pub group_info: MlsGroupInfoBundle,
     /// New CRL distribution points that appeared by the introduction of a new credential
-    pub crl_new_distribution_points: Option<Vec<String>>,
+    pub crl_new_distribution_points: NewCrlDistributionPoint,
 }
 
 impl MlsConversationCreationMessage {
@@ -312,7 +316,7 @@ impl MlsConversationCreationMessage {
     /// 1 -> commit
     /// 2 -> group_info
     #[allow(clippy::type_complexity)]
-    pub fn to_bytes(self) -> CryptoResult<(Vec<u8>, Vec<u8>, MlsGroupInfoBundle, Option<Vec<String>>)> {
+    pub fn to_bytes(self) -> CryptoResult<(Vec<u8>, Vec<u8>, MlsGroupInfoBundle, NewCrlDistributionPoint)> {
         use openmls::prelude::TlsSerializeTrait as _;
         let welcome = self.welcome.tls_serialize_detached().map_err(MlsError::from)?;
         let msg = self.commit.tls_serialize_detached().map_err(MlsError::from)?;