fix: verify GroupInfo

Cargo.toml

@@ -49,25 +49,25 @@ branch = "2.x"
 package = "openmls"
 git = "https://github.com/wireapp/openmls"
 #tag = "v1.0.0-pre.core-crypto-1.0.0"
-branch = "feat/rfc9420"
+branch = "fix/verify-group-info"
 
 [patch.crates-io.openmls_traits]
 package = "openmls_traits"
 git = "https://github.com/wireapp/openmls"
 #tag = "v1.0.0-pre.core-crypto-1.0.0"
-branch = "feat/rfc9420"
+branch = "fix/verify-group-info"
 
 [patch.crates-io.openmls_basic_credential]
 package = "openmls_basic_credential"
 git = "https://github.com/wireapp/openmls"
 #tag = "v1.0.0-pre.core-crypto-1.0.0"
-branch = "feat/rfc9420"
+branch = "fix/verify-group-info"
 
 [patch.crates-io.openmls_x509_credential]
 package = "openmls_x509_credential"
 git = "https://github.com/wireapp/openmls"
 #tag = "v1.0.0-pre.core-crypto-1.0.0"
-branch = "feat/rfc9420"
+branch = "fix/verify-group-info"
 
 [patch.crates-io.hpke]
 git = "https://github.com/wireapp/rust-hpke.git"

crypto/src/mls/conversation/handshake.rs

@@ -752,9 +752,8 @@ pub mod tests {
                             .remove_members_from_conversation(&id, &[bob_central.get_client_id()])
                             .await
                             .unwrap();
-                        let group_info = commit_bundle.group_info.get_group_info();
                         alice_central.commit_accepted(&id).await.unwrap();
-
+                        let group_info = commit_bundle.group_info.get_group_info();
                         assert!(guest_central
                             .try_join_from_group_info(&case, &id, group_info, vec![&mut alice_central])
                             .await

crypto/src/mls/conversation/leaf_node_validation.rs

@@ -21,11 +21,9 @@ pub mod tests {
 
         /// The validity of a LeafNode needs to be verified at the following stages:
         /// When a LeafNode is downloaded in a KeyPackage, before it is used to add the client to the group
-        // #[apply(all_cred_cipher)]
-        // #[wasm_bindgen_test]
-        #[async_std::test]
-        pub async fn should_validate_leaf_node_when_adding(/*case: TestCase*/) {
-            let case = TestCase::default();
+        #[apply(all_cred_cipher)]
+        #[wasm_bindgen_test]
+        pub async fn should_validate_leaf_node_when_adding(case: TestCase) {
             run_test_with_client_ids(
                 case.clone(),
                 ["alice", "bob"],

crypto/src/mls/credential/mod.rs

@@ -443,13 +443,16 @@ pub mod tests {
             )
             .await?;
 
+        println!("- Create conversation");
         creator_central
             .new_conversation(&id, creator_ct, case.cfg.clone())
             .await?;
         let guest = guest_central.rand_key_package_of_type(case, guest_ct).await;
+        println!("- Invite all members");
         creator_central
             .invite_all_members(case, &id, [(&mut guest_central, guest)])
             .await?;
+        println!("- Try talking");
         creator_central.try_talk_to(&id, &mut guest_central).await?;
         Ok((creator_central, guest_central, id))
     }

crypto/src/mls/external_commit.rs

@@ -17,17 +17,15 @@
 use openmls::prelude::{group_info::VerifiableGroupInfo, MlsGroup, MlsMessageOut, Proposal, Sender, StagedCommit};
 use openmls_traits::OpenMlsCryptoProvider;
 
-use core_crypto_keystore::entities::MlsPendingMessage;
-use core_crypto_keystore::{entities::PersistedMlsPendingGroup, CryptoKeystoreMls};
+use core_crypto_keystore::{entities::MlsPendingMessage, entities::PersistedMlsPendingGroup, CryptoKeystoreMls};
 use tls_codec::Serialize;
 
-use crate::prelude::decrypt::MlsBufferedConversationDecryptMessage;
 use crate::{
     group_store::GroupStoreValue,
     prelude::{
-        id::ClientId, ConversationId, CoreCryptoCallbacks, CryptoError, CryptoResult, MlsCentral, MlsCiphersuite,
-        MlsConversation, MlsConversationConfiguration, MlsCredentialType, MlsCustomConfiguration, MlsError,
-        MlsGroupInfoBundle,
+        decrypt::MlsBufferedConversationDecryptMessage, id::ClientId, ConversationId, CoreCryptoCallbacks, CryptoError,
+        CryptoResult, MlsCentral, MlsCiphersuite, MlsConversation, MlsConversationConfiguration, MlsCredentialType,
+        MlsCustomConfiguration, MlsError, MlsGroupInfoBundle,
     },
 };
 
@@ -268,8 +266,9 @@ impl MlsConversation {
 
 #[cfg(test)]
 pub mod tests {
-    use crate::{prelude::MlsConversationInitBundle, test_utils::*, CryptoError};
+    use crate::{prelude::MlsConversationInitBundle, test_utils::*, CryptoError, MlsError};
     use openmls::prelude::*;
+    use openmls::treesync::errors::{LifetimeError, TreeSyncFromNodesError};
     use wasm_bindgen_test::*;
 
     use crate::prelude::MlsConversationConfiguration;
@@ -762,6 +761,55 @@ pub mod tests {
         .await
     }
 
+    #[apply(all_cred_cipher)]
+    #[wasm_bindgen_test]
+    pub async fn should_fail_when_invalid_group_info(case: TestCase) {
+        run_test_with_client_ids(
+            case.clone(),
+            ["alice", "bob", "guest"],
+            move |[mut alice_central, mut bob_central, mut guest_central]| {
+                Box::pin(async move {
+                    let expiration_time = 14;
+                    let start = fluvio_wasm_timer::Instant::now();
+                    let id = conversation_id();
+                    alice_central
+                        .new_conversation(&id, case.credential_type, case.cfg.clone())
+                        .await
+                        .unwrap();
+
+                    let invalid_kp = bob_central.new_keypackage(&case, Lifetime::new(expiration_time)).await;
+                    alice_central
+                        .add_members_to_conversation(&id, vec![invalid_kp.into()])
+                        .await
+                        .unwrap();
+                    alice_central.commit_accepted(&id).await.unwrap();
+
+                    let elapsed = start.elapsed();
+                    // Give time to the certificate to expire
+                    let expiration_time = core::time::Duration::from_secs(expiration_time);
+                    if expiration_time > elapsed {
+                        async_std::task::sleep(expiration_time - elapsed + core::time::Duration::from_secs(1)).await;
+                    }
+
+                    let group_info = alice_central.get_group_info(&id).await;
+
+                    let join_ext_commit = guest_central
+                        .join_by_external_commit(group_info, case.custom_cfg(), case.credential_type)
+                        .await;
+                    assert!(matches!(
+                        join_ext_commit.unwrap_err(),
+                        CryptoError::MlsError(MlsError::MlsExternalCommitError(ExternalCommitError::PublicGroupError(
+                            CreationFromExternalError::TreeSyncError(TreeSyncFromNodesError::LeafNodeValidationError(
+                                LeafNodeValidationError::Lifetime(LifetimeError::NotCurrent),
+                            )),
+                        )))
+                    ))
+                })
+            },
+        )
+        .await
+    }
+
     #[apply(all_cred_cipher)]
     #[wasm_bindgen_test]
     pub async fn group_should_have_right_config(case: TestCase) {

crypto/src/mls/external_proposal.rs

@@ -1,9 +1,8 @@
 use std::collections::HashSet;
 
-use openmls::prelude::{JoinProposal, LeafNodeIndex};
 use openmls::{
     group::QueuedProposal,
-    prelude::{GroupEpoch, GroupId, MlsMessageOut, Proposal, Sender},
+    prelude::{GroupEpoch, GroupId, JoinProposal, LeafNodeIndex, MlsMessageOut, Proposal, Sender},
 };
 
 use crate::{
@@ -136,8 +135,7 @@ impl MlsCentral {
             .generate_one_keypackage_from_credential_bundle(&self.mls_backend, ciphersuite, cb)
             .await?;
 
-        let ext_proposal = JoinProposal::new(kp, group_id, epoch, &cb.signature_key).map_err(MlsError::from)?;
-        Ok(ext_proposal)
+        Ok(JoinProposal::new(kp, group_id, epoch, &cb.signature_key).map_err(MlsError::from)?)
     }
 }
 

crypto/src/test_utils/central.rs

@@ -15,24 +15,25 @@
 // along with this program. If not, see http://www.gnu.org/licenses/.
 
 use crate::{
+    e2e_identity::{
+        device_status::DeviceStatus,
+        id::{QualifiedE2eiClientId, WireQualifiedClientId},
+    },
     mls::credential::{ext::CredentialExt, CredentialBundle},
     prelude::{
         CertificateBundle, Client, ClientId, ConversationId, CryptoError, CryptoResult, MlsCentral, MlsCiphersuite,
         MlsConversation, MlsConversationConfiguration, MlsConversationDecryptMessage, MlsConversationInitBundle,
-        MlsCredentialType, MlsCustomConfiguration, MlsError,
+        MlsCredentialType, MlsCustomConfiguration, MlsError, WireIdentity,
     },
     test_utils::{MessageExt, TestCase},
 };
 use openmls::prelude::{
-    group_info::VerifiableGroupInfo, Capabilities, Credential, CredentialWithKey, CryptoConfig, HpkePublicKey,
-    KeyPackage, KeyPackageIn, LeafNodeIndex, Lifetime, MlsMessageIn, QueuedProposal, SignaturePublicKey, StagedCommit,
+    group_info::VerifiableGroupInfo, Credential, CredentialWithKey, CryptoConfig, HpkePublicKey, KeyPackage,
+    KeyPackageIn, LeafNodeIndex, Lifetime, MlsMessageIn, QueuedProposal, SignaturePublicKey, StagedCommit,
 };
 use openmls_traits::{types::SignatureScheme, OpenMlsCryptoProvider};
 use tls_codec::{Deserialize, Serialize};
 
-use crate::e2e_identity::device_status::DeviceStatus;
-use crate::e2e_identity::id::{QualifiedE2eiClientId, WireQualifiedClientId};
-use crate::prelude::WireIdentity;
 use core_crypto_keystore::entities::{
     EntityFindParams, MlsCredential, MlsEncryptionKeyPair, MlsHpkePrivateKey, MlsKeyPackage, MlsSignatureKeyPair,
 };
@@ -188,7 +189,7 @@ impl MlsCentral {
                 other.get_conversation_unchecked(id).await.members().len(),
                 size_before + N
             );
-            self.try_talk_to(id, other).await?;
+            // self.try_talk_to(id, other).await?;
         }
 
         Ok(())
@@ -215,7 +216,7 @@ impl MlsCentral {
         for other in others {
             let commit = commit.tls_serialize_detached().map_err(MlsError::from)?;
             other.decrypt_message(id, commit).await.unwrap();
-            self.try_talk_to(id, other).await?;
+            // self.try_talk_to(id, other).await?;
         }
         Ok(())
     }
@@ -536,6 +537,10 @@ impl MlsCentral {
             assert!(!identity.thumbprint.is_empty());
         }
     }
+
+    pub async fn members_count(&mut self, id: &ConversationId) -> u32 {
+        self.get_conversation_unchecked(id).await.members().len() as u32
+    }
 }
 
 impl MlsConversation {

mls-provider/Cargo.toml

@@ -16,6 +16,7 @@ default = []
 raw-rand-access = [] # TESTING ONLY
 
 [dependencies]
+openmls = { version = "1", features = ["crypto-subtle"] }
 openmls_traits = "0.2"
 tls_codec = { workspace = true }
 aes-gcm = "0.10"
@@ -36,6 +37,7 @@ rand_core = "0.6"
 rand_chacha = "0.3"
 zeroize = "1.5"
 thiserror = "1.0"
+hex = "0.4"
 
 [dependencies.hpke]
 version = "0.10"