fix: verify GroupInfo

openmls/Cargo.toml

@@ -24,7 +24,7 @@ x509-cert = "0.2"
 subtle = "2.5"
 fluvio-wasm-timer = "0.2"
 indexmap = "2.0"
-itertools = "0.11"
+itertools = "0.12"
 
 # Only required for tests.
 rand = { version = "0.8", optional = true, features = ["getrandom"] }

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/errors.rs

@@ -6,7 +6,7 @@ use crate::error::LibraryError;
 use thiserror::Error;
 
 /// An error that occurs in methods of a [`super::Credential`].
-#[derive(Error, Debug, PartialEq, Clone)]
+#[derive(Error, Debug, Eq, PartialEq, Clone)]
 pub enum CredentialError {
     /// A library error occurred.
     #[error(transparent)]
@@ -26,4 +26,7 @@ pub enum CredentialError {
     /// x509 certificate chain is either unordered or a child is missigned by its issuer
     #[error("Invalid x509 certificate chain.")]
     InvalidCertificateChain,
+    /// one of x509 certificate chain member is expired
+    #[error("Expired x509 certificate chain.")]
+    ExpiredCertificate,
 }

openmls/src/credentials/mod.rs

@@ -35,6 +35,7 @@ mod codec;
 #[cfg(test)]
 mod tests;
 use errors::*;
+use openmls_traits::types::CryptoError;
 use openmls_x509_credential::X509Ext;
 use x509_cert::{der::Decode, PkiPath};
 
@@ -273,6 +274,25 @@ impl Credential {
             MlsCredentialType::X509(cert) => cert.identity.as_slice(),
         }
     }
+
+    pub fn verify(&self) -> Result<(), CredentialError> {
+        match &self.credential {
+            MlsCredentialType::X509(cert) => {
+                // TODO: implement this on the whole chain with certval
+                let ee_cert = cert
+                    .certificates
+                    .first()
+                    .ok_or(CredentialError::InvalidCertificateChain)?;
+                let ee_cert = x509_cert::Certificate::from_der(ee_cert.as_slice())?;
+                ee_cert.is_valid().map_err(|e| match e {
+                    CryptoError::ExpiredCertificate => CredentialError::ExpiredCertificate,
+                    _ => CredentialError::InvalidCertificateChain,
+                })?;
+            }
+            MlsCredentialType::Basic(_) => {}
+        }
+        Ok(())
+    }
 }
 
 impl From<MlsCredentialType> for Credential {

openmls/src/group/core_group/new_from_welcome.rs

@@ -26,6 +26,9 @@ impl CoreGroup {
     ) -> Result<Self, WelcomeError<KeyStore::Error>> {
         log::debug!("CoreGroup::new_from_welcome_internal");
 
+        let key_package = KeyPackageIn::from(key_package.clone())
+            .standalone_validate(backend.crypto(), ProtocolVersion::default())?;
+
         // Read the encryption key pair from the key store and delete it there.
         // TODO #1207: Key store access happens as early as possible so it can
         // be pulled up later more easily.
@@ -153,12 +156,6 @@ impl CoreGroup {
             ProposalStore::new(),
         )?;
 
-        KeyPackageIn::from(key_package.clone()).validate(
-            backend.crypto(),
-            ProtocolVersion::Mls10,
-            &public_group,
-        )?;
-
         // Find our own leaf in the tree.
         let own_leaf_index = public_group
             .members()

openmls/src/group/public_group/mod.rs

@@ -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)?;
 
         let group_info: GroupInfo = {
             let signer_signature_key = treesync

openmls/src/messages/group_info.rs

@@ -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)?;
 
         let signer_signature_key = treesync
             .leaf(self.signer())

openmls/src/treesync/errors.rs

@@ -5,6 +5,7 @@
 use thiserror::Error;
 
 use super::*;
+use crate::prelude::CredentialError;
 use crate::{
     binary_tree::MlsBinaryTreeDiffError, ciphersuite::signable::SignatureError,
     error::LibraryError, extensions::errors::ExtensionError,
@@ -152,6 +153,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
@@ -268,6 +272,9 @@ pub enum LeafNodeValidationError {
     /// See [`SignatureError`] for more details.
     #[error(transparent)]
     SignatureError(#[from] SignatureError),
+    /// See [`CredentialError`] for more details.
+    #[error(transparent)]
+    CredentialError(#[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))?;
@@ -443,18 +444,32 @@ impl TreeSync {
         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.crypto(), ciphersuite.signature_algorithm(), None)?
+                    } 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 +478,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,11 @@ 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::crypto::OpenMlsCrypto;
+use openmls_traits::types::SignatureScheme;
 
 /// Private module to ensure protection.
 mod private_mod {
@@ -706,15 +710,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 +780,22 @@ impl VerifiableLeafNode {
             VerifiableLeafNode::Commit(v) => v.signature_key(),
         }
     }
+
+    pub(crate) fn validate(
+        self,
+        crypto: &impl OpenMlsCrypto,
+        sc: SignatureScheme,
+        group: Option<&PublicGroup>,
+    ) -> Result<LeafNode, LeafNodeValidationError> {
+        match (self, group) {
+            (VerifiableLeafNode::KeyPackage(ln), None) => ln.standalone_validate(crypto, sc),
+            (VerifiableLeafNode::KeyPackage(ln), Some(group)) => ln.validate(group, crypto),
+            (VerifiableLeafNode::Update(ln), None) => ln.standalone_validate(crypto, sc),
+            (VerifiableLeafNode::Update(ln), Some(group)) => ln.validate(group, crypto),
+            (VerifiableLeafNode::Commit(ln), None) => ln.standalone_validate(crypto, sc),
+            (VerifiableLeafNode::Commit(ln), Some(group)) => ln.validate(group, crypto),
+        }
+    }
 }
 
 #[derive(Debug, Clone, PartialEq, Eq)]