fix: verify GroupInfo

openmls/src/binary_tree/array_representation/treemath.rs

@@ -58,7 +58,7 @@ impl LeafNodeIndex {
     }
 
     /// Warning: Only use when the node index represents a leaf node
-    fn from_tree_index(node_index: u32) -> Self {
+    pub fn from_tree_index(node_index: u32) -> Self {
         debug_assert!(node_index % 2 == 0);
         LeafNodeIndex(node_index / 2)
     }

openmls/src/credentials/mod.rs

@@ -37,6 +37,7 @@ mod codec;
 mod tests;
 
 use errors::*;
+
 use openmls_x509_credential::X509Ext;
 use x509_cert::{der::Decode, PkiPath};
 

openmls/src/group/core_group/new_from_external_init.rs

@@ -52,7 +52,8 @@ impl CoreGroup {
             verifiable_group_info,
             // Existing proposals are discarded when joining by external commit.
             ProposalStore::new(),
-        )?;
+        )
+        .await?;
         let group_context = public_group.group_context();
 
         // Obtain external_pub from GroupInfo extensions.

openmls/src/group/core_group/new_from_welcome.rs

@@ -151,7 +151,8 @@ impl CoreGroup {
             ratchet_tree,
             verifiable_group_info,
             ProposalStore::new(),
-        )?;
+        )
+        .await?;
 
         KeyPackageIn::from(key_package.clone())
             .validate(backend, ProtocolVersion::Mls10, &public_group)

openmls/src/group/public_group/mod.rs

@@ -102,7 +102,7 @@ impl PublicGroup {
     /// This function performs basic validation checks and returns an error if
     /// one of the checks fails. See [`CreationFromExternalError`] for more
     /// details.
-    pub fn from_external(
+    pub async fn from_external(
         backend: &impl OpenMlsCryptoProvider,
         ratchet_tree: RatchetTreeIn,
         verifiable_group_info: VerifiableGroupInfo,
@@ -122,7 +122,8 @@ impl PublicGroup {
         // Create a RatchetTree from the given nodes. We have to do this before
         // verifying the group info, since we need to find the Credential to verify the
         // signature against.
-        let treesync = TreeSync::from_ratchet_tree(backend, ciphersuite, ratchet_tree)?;
+        let treesync =
+            TreeSync::from_ratchet_tree(backend, ciphersuite, ratchet_tree, group_id, true).await?;
 
         let group_info: GroupInfo = {
             let signer_signature_key = treesync

openmls/src/group/public_group/tests.rs

@@ -61,6 +61,7 @@ async fn public_group(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProv
         verifiable_group_info,
         ProposalStore::new(),
     )
+    .await
     .unwrap();
 
     // === Alice adds Bob ===

openmls/src/messages/group_info.rs

@@ -120,7 +120,7 @@ impl VerifiableGroupInfo {
     }
 
     /// Do whatever it takes not to clone the RatchetTree
-    pub fn take_ratchet_tree(
+    pub async fn take_ratchet_tree(
         mut self,
         backend: &impl OpenMlsCryptoProvider,
     ) -> Result<RatchetTree, GroupInfoError> {
@@ -142,7 +142,9 @@ impl VerifiableGroupInfo {
             .into_verified(cs, backend.crypto(), self.group_id())?;
 
         // although it clones the ratchet tree here...
-        let treesync = TreeSync::from_ratchet_tree(backend, cs, ratchet_tree.clone())?;
+        let group_id = self.group_id();
+        let treesync =
+            TreeSync::from_ratchet_tree(backend, cs, ratchet_tree.clone(), group_id, true).await?;
 
         let signer_signature_key = treesync
             .leaf(self.signer())

openmls/src/treesync/errors.rs

@@ -152,6 +152,9 @@ pub enum TreeSyncFromNodesError {
     /// See [`RatchetTreeError`] for more details.
     #[error(transparent)]
     RatchetTreeError(#[from] RatchetTreeError),
+    /// See [`LeafNodeValidationError`] for more details.
+    #[error(transparent)]
+    LeafNodeValidationError(#[from] LeafNodeValidationError),
 }
 
 /// TreeSync parent hash error
@@ -262,14 +265,15 @@ pub enum LeafNodeValidationError {
     /// The credential used by a member is not supported by this leaf node.
     #[error("The credential used by a member is not supported by this leaf node.")]
     MemberCredentialNotSupportedByLeafNode,
-    #[error(transparent)]
-    InvalidCredential(#[from] CredentialError),
     /// A library error occurred.
     #[error(transparent)]
     LibraryError(#[from] LibraryError),
     /// See [`SignatureError`] for more details.
     #[error(transparent)]
     SignatureError(#[from] SignatureError),
+    /// See [`CredentialError`] for more details.
+    #[error(transparent)]
+    InvalidCredential(#[from] CredentialError),
 }
 
 /// Errors that can happen during lifetime validation.

openmls/src/treesync/mod.rs

@@ -83,6 +83,7 @@ pub use node::encryption_keys::test_utils;
 pub use node::encryption_keys::EncryptionKey;
 
 // Public re-exports
+use crate::treesync::node::leaf_node::LeafNodeIn;
 pub use node::{leaf_node::LeafNode, parent_node::ParentNode, Node};
 
 // Tests
@@ -172,13 +173,13 @@ impl RatchetTree {
                 // Verify the nodes.
                 let signature_scheme = ciphersuite.signature_algorithm();
                 let mut verified_nodes = Vec::new();
-                for (index, node) in nodes.into_iter().enumerate() {
-                    let verified_node = match (index % 2, node) {
+                for (node_index, node) in nodes.into_iter().enumerate() {
+                    let verified_node = match (node_index % 2, node) {
                         // Even indices must be leaf nodes.
                         (0, Some(NodeIn::LeafNode(leaf_node))) => {
                             let tree_position = TreePosition::new(
                                 group_id.clone(),
-                                LeafNodeIndex::new((index / 2) as u32),
+                                LeafNodeIndex::from_tree_index(node_index as u32),
                             );
                             let verifiable_leaf_node =
                                 leaf_node.try_into_verifiable_leaf_node(Some(tree_position))?;
@@ -439,22 +440,37 @@ impl TreeSync {
     /// A helper function that generates a [`TreeSync`] instance from the given
     /// slice of nodes. It verifies that the provided encryption key is present
     /// in the tree and that the invariants documented in [`TreeSync`] hold.
-    pub(crate) fn from_ratchet_tree(
+    pub(crate) async fn from_ratchet_tree(
         backend: &impl OpenMlsCryptoProvider,
         ciphersuite: Ciphersuite,
         ratchet_tree: RatchetTree,
+        group_id: &GroupId,
+        validate_leaf_node: bool,
     ) -> Result<Self, TreeSyncFromNodesError> {
         // TODO #800: Unmerged leaves should be checked
         let mut ts_nodes: Vec<TreeNode<TreeSyncLeafNode, TreeSyncParentNode>> =
             Vec::with_capacity(ratchet_tree.0.len());
 
         // Set the leaf indices in all the leaves and convert the node types.
         for (node_index, node_option) in ratchet_tree.0.into_iter().enumerate() {
-            let ts_node_option: TreeNode<TreeSyncLeafNode, TreeSyncParentNode> = match node_option {
-                Some(node) => {
-                    let node = node.clone();
-                    TreeSyncNode::from(node).into()
+            let ts_node: TreeNode<TreeSyncLeafNode, TreeSyncParentNode> = match node_option {
+                Some(Node::LeafNode(ln)) => {
+                    let ln = if validate_leaf_node {
+                        let index = node_index
+                            .try_into()
+                            .map_err(|_| LibraryError::custom("failed converting a usize -> u32"));
+                        let index = LeafNodeIndex::from_tree_index(index?);
+                        let tree_position = TreePosition::new(group_id.clone(), index);
+                        let ln = LeafNodeIn::from(ln)
+                            .try_into_verifiable_leaf_node(Some(tree_position))?;
+                        ln.validate(backend, ciphersuite.signature_algorithm(), None)
+                            .await?
+                    } else {
+                        ln
+                    };
+                    TreeSyncNode::from(Node::LeafNode(ln)).into()
                 }
+                Some(Node::ParentNode(pn)) => TreeSyncNode::from(Node::ParentNode(pn)).into(),
                 None => {
                     if node_index % 2 == 0 {
                         TreeNode::Leaf(TreeSyncLeafNode::blank())
@@ -463,7 +479,7 @@ impl TreeSync {
                     }
                 }
             };
-            ts_nodes.push(ts_node_option);
+            ts_nodes.push(ts_node);
         }
         let tree = MlsBinaryTree::new(ts_nodes).map_err(|_| PublicTreeError::MalformedTree)?;
         let mut tree_sync = Self {

openmls/src/treesync/node/leaf_node.rs

@@ -29,7 +29,10 @@ use crate::treesync::errors::LeafNodeValidationError;
 mod capabilities;
 mod codec;
 
+use crate::prelude::PublicGroup;
+use crate::treesync::node::validate::ValidatableLeafNode;
 pub use capabilities::*;
+use openmls_traits::types::SignatureScheme;
 
 /// Private module to ensure protection.
 mod private_mod {
@@ -706,15 +709,17 @@ impl LeafNodeIn {
                 })
             }
             LeafNodeSource::Update => {
-                let tree_position = tree_position.ok_or(LibraryError::custom("Internal error"))?;
+                let tree_position = tree_position
+                    .ok_or(LibraryError::custom("No tree position for Update LeafNode"))?;
                 VerifiableLeafNode::Update(VerifiableUpdateLeafNode {
                     payload: self.payload,
                     signature: self.signature,
                     tree_position,
                 })
             }
             LeafNodeSource::Commit(_) => {
-                let tree_position = tree_position.ok_or(LibraryError::custom("Internal error"))?;
+                let tree_position = tree_position
+                    .ok_or(LibraryError::custom("No tree position for Commit LeafNode"))?;
                 VerifiableLeafNode::Commit(VerifiableCommitLeafNode {
                     payload: self.payload,
                     signature: self.signature,
@@ -774,6 +779,22 @@ impl VerifiableLeafNode {
             VerifiableLeafNode::Commit(v) => v.signature_key(),
         }
     }
+
+    pub(crate) async fn validate(
+        self,
+        backend: &impl OpenMlsCryptoProvider,
+        sc: SignatureScheme,
+        group: Option<&PublicGroup>,
+    ) -> Result<LeafNode, LeafNodeValidationError> {
+        match (self, group) {
+            (VerifiableLeafNode::KeyPackage(ln), None) => ln.standalone_validate(backend, sc).await,
+            (VerifiableLeafNode::KeyPackage(ln), Some(group)) => ln.validate(group, backend).await,
+            (VerifiableLeafNode::Update(ln), None) => ln.standalone_validate(backend, sc).await,
+            (VerifiableLeafNode::Update(ln), Some(group)) => ln.validate(group, backend).await,
+            (VerifiableLeafNode::Commit(ln), None) => ln.standalone_validate(backend, sc).await,
+            (VerifiableLeafNode::Commit(ln), Some(group)) => ln.validate(group, backend).await,
+        }
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]

openmls/src/treesync/node/validate.rs

@@ -13,8 +13,8 @@ use crate::{
         },
     },
 };
-use itertools::Itertools;
 use openmls_traits::{crypto::OpenMlsCrypto, types::SignatureScheme, OpenMlsCryptoProvider};
+use std::collections::HashSet;
 
 impl ValidatableLeafNode for VerifiableCommitLeafNode {
     fn signature_key(&self) -> &SignaturePublicKey {
@@ -47,7 +47,7 @@ impl ValidatableLeafNode for VerifiableUpdateLeafNode {
         backend: &impl OpenMlsCryptoProvider,
     ) -> Result<LeafNode, LeafNodeValidationError> {
         self.validate_replaced_encryption_key(group)?;
-        self.generic_validate(backend, group).await
+        self.validate_default(group, backend).await
     }
 
     fn signature_key(&self) -> &SignaturePublicKey {
@@ -91,13 +91,23 @@ impl VerifiableUpdateLeafNode {
 #[cfg_attr(target_family = "wasm", async_trait::async_trait(?Send))]
 #[cfg_attr(not(target_family = "wasm"), async_trait::async_trait)]
 impl ValidatableLeafNode for VerifiableKeyPackageLeafNode {
+    async fn standalone_validate(
+        self,
+        backend: &impl OpenMlsCryptoProvider,
+        signature_scheme: SignatureScheme,
+    ) -> Result<LeafNode, LeafNodeValidationError> {
+        self.validate_lifetime()?;
+        self.standalone_validate_default(backend, signature_scheme)
+            .await
+    }
+
     async fn validate(
         self,
         group: &PublicGroup,
         backend: &impl OpenMlsCryptoProvider,
     ) -> Result<LeafNode, LeafNodeValidationError> {
         self.validate_lifetime()?;
-        self.generic_validate(backend, group).await
+        self.validate_default(group, backend).await
     }
 
     fn signature_key(&self) -> &SignaturePublicKey {
@@ -144,8 +154,18 @@ where
         backend: &impl OpenMlsCryptoProvider,
         signature_scheme: SignatureScheme,
     ) -> Result<LeafNode, LeafNodeValidationError> {
-        let extension_types = self.extension_types();
+        self.standalone_validate_default(backend, signature_scheme)
+            .await
+    }
+
+    async fn standalone_validate_default(
+        self,
+        backend: &impl OpenMlsCryptoProvider,
+        signature_scheme: SignatureScheme,
+    ) -> Result<LeafNode, LeafNodeValidationError> {
         self.validate_credential(backend).await?;
+
+        let extension_types = self.extension_types();
         let leaf_node = self.verify_signature(backend.crypto(), signature_scheme)?;
         Self::validate_extension_support(&leaf_node, &extension_types[..])?;
 
@@ -158,28 +178,20 @@ where
         group: &PublicGroup,
         backend: &impl OpenMlsCryptoProvider,
     ) -> Result<LeafNode, LeafNodeValidationError> {
-        self.generic_validate(backend, group).await
+        self.validate_default(group, backend).await
     }
 
-    /// Validation regardless of [LeafNode]'s source
-    async fn generic_validate(
+    async fn validate_default(
         self,
-        backend: &impl OpenMlsCryptoProvider,
         group: &PublicGroup,
+        backend: &impl OpenMlsCryptoProvider,
     ) -> Result<LeafNode, LeafNodeValidationError> {
         self.validate_capabilities(group)?;
         self.validate_credential_type(group)?;
         let tree = group.treesync();
-        self.validate_signature_key_unique(tree)?;
-        self.validate_encryption_key_unique(tree)?;
-        self.validate_credential(backend).await?;
-
-        let extension_types = self.extension_types();
+        self.validate_signature_encryption_key_unique(tree)?;
         let signature_scheme = group.ciphersuite().signature_algorithm();
-        let leaf_node = self.verify_signature(backend.crypto(), signature_scheme)?;
-        Self::validate_extension_support(&leaf_node, &extension_types[..])?;
-
-        Ok(leaf_node)
+        self.standalone_validate(backend, signature_scheme).await
     }
 
     fn signature_key(&self) -> &SignaturePublicKey;
@@ -233,24 +245,21 @@ where
     }
 
     /// Verify that the following fields are unique among the members of the group: signature_key
-    fn validate_signature_key_unique(
+    fn validate_signature_encryption_key_unique(
         &self,
         tree: &TreeSync,
     ) -> Result<(), LeafNodeValidationError> {
-        if !tree.raw_leaves().map(LeafNode::signature_key).all_unique() {
-            return Err(LeafNodeValidationError::SignatureKeyAlreadyInUse);
-        }
-        Ok(())
-    }
-    /// Verify that the following fields are unique among the members of the group: encryption_key
-    fn validate_encryption_key_unique(
-        &self,
-        tree: &TreeSync,
-    ) -> Result<(), LeafNodeValidationError> {
-        if !tree.raw_leaves().map(LeafNode::encryption_key).all_unique() {
-            return Err(LeafNodeValidationError::EncryptionKeyAlreadyInUse);
+        let size = tree.tree_size().leaf_count() as usize;
+        let mut used_signature_keys = HashSet::with_capacity(size);
+        let mut used_encryption_keys = HashSet::with_capacity(size);
+        for ln in tree.raw_leaves() {
+            if !used_signature_keys.insert(ln.signature_key()) {
+                return Err(LeafNodeValidationError::SignatureKeyAlreadyInUse);
+            }
+            if !used_encryption_keys.insert(ln.encryption_key()) {
+                return Err(LeafNodeValidationError::EncryptionKeyAlreadyInUse);
+            }
         }
-
         Ok(())
     }
 

openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs

@@ -63,8 +63,10 @@ async fn run_test_vector(
 
     let ratchet_tree = RatchetTree::from(RatchetTreeIn::from_nodes(nodes));
 
-    let tree_before = TreeSync::from_ratchet_tree(backend, ciphersuite, ratchet_tree)
-        .map_err(|e| format!("Error while creating tree sync: {e:?}"))?;
+    let tree_before =
+        TreeSync::from_ratchet_tree(backend, ciphersuite, ratchet_tree, &group_id, false)
+            .await
+            .map_err(|e| format!("Error while creating tree sync: {e:?}"))?;
 
     let group_context = GroupContext::new(
         ciphersuite,