From 7fe95c64ceaccecee8dd1c1c32922117809bc421 Mon Sep 17 00:00:00 2001 From: beltram Date: Mon, 11 Dec 2023 10:50:44 +0100 Subject: [PATCH] feat: add methods to take a RatchetTree from a GroupInfo without allocating memory --- openmls/src/extensions/mod.rs | 2 +- .../src/extensions/ratchet_tree_extension.rs | 2 +- openmls/src/group/mls_group/membership.rs | 8 ++ openmls/src/group/public_group/mod.rs | 2 +- openmls/src/messages/group_info.rs | 79 ++++++++++++++++--- openmls/src/treesync/mod.rs | 15 +++- .../kats/kat_tree_operations.rs | 2 +- .../kats/kat_tree_validation.rs | 2 +- .../tests_and_kats/kats/kat_treekem.rs | 2 +- .../tests_and_kats/tests/test_diff.rs | 2 +- 10 files changed, 96 insertions(+), 20 deletions(-) diff --git a/openmls/src/extensions/mod.rs b/openmls/src/extensions/mod.rs index 089ec39dab..7beacd2074 100644 --- a/openmls/src/extensions/mod.rs +++ b/openmls/src/extensions/mod.rs @@ -223,7 +223,7 @@ pub struct UnknownExtension(pub Vec); /// A list of extensions with unique extension types. #[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, Deserialize, tls_codec::TlsSize)] pub struct Extensions { - unique: Vec, + pub(crate) unique: Vec, } impl tls_codec::Serialize for Extensions { diff --git a/openmls/src/extensions/ratchet_tree_extension.rs b/openmls/src/extensions/ratchet_tree_extension.rs index 10135d1ee3..21f5aef9ba 100644 --- a/openmls/src/extensions/ratchet_tree_extension.rs +++ b/openmls/src/extensions/ratchet_tree_extension.rs @@ -16,7 +16,7 @@ use crate::treesync::{RatchetTree, RatchetTreeIn}; PartialEq, Eq, Clone, Debug, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, )] pub struct RatchetTreeExtension { - ratchet_tree: RatchetTreeIn, + pub(crate) ratchet_tree: RatchetTreeIn, } impl RatchetTreeExtension { diff --git a/openmls/src/group/mls_group/membership.rs b/openmls/src/group/mls_group/membership.rs index a232c84df3..832c5b478a 100644 --- a/openmls/src/group/mls_group/membership.rs +++ b/openmls/src/group/mls_group/membership.rs @@ -202,6 +202,14 @@ impl MlsGroup { self.group.public_group().members() } + pub fn members_credentials(&self) -> impl Iterator + '_ { + self.group + .public_group() + .treesync() + .raw_leaves() + .map(|ln| ln.credential()) + } + /// Returns the [`Credential`] of a member corresponding to the given /// leaf index. Returns `None` if the member can not be found in this group. pub fn member(&self, leaf_index: LeafNodeIndex) -> Option<&Credential> { diff --git a/openmls/src/group/public_group/mod.rs b/openmls/src/group/public_group/mod.rs index 6dfe9002e7..014c5f3cea 100644 --- a/openmls/src/group/public_group/mod.rs +++ b/openmls/src/group/public_group/mod.rs @@ -121,7 +121,7 @@ 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)?; let group_info: GroupInfo = { let signer_signature_key = treesync diff --git a/openmls/src/messages/group_info.rs b/openmls/src/messages/group_info.rs index 7bdb50d82b..4e09713aeb 100644 --- a/openmls/src/messages/group_info.rs +++ b/openmls/src/messages/group_info.rs @@ -1,27 +1,30 @@ //! This module contains all types related to group info handling. -use openmls_traits::{types::Ciphersuite, OpenMlsCryptoProvider}; use thiserror::Error; use tls_codec::{Deserialize, Serialize, TlsDeserialize, TlsSerialize, TlsSize}; +use openmls_traits::{types::Ciphersuite, OpenMlsCryptoProvider}; + use crate::{ binary_tree::LeafNodeIndex, ciphersuite::{ signable::{Signable, SignedStruct, Verifiable, VerifiedStruct}, AeadKey, AeadNonce, Signature, }, - extensions::Extensions, + extensions::{Extension, Extensions}, group::{group_context::GroupContext, GroupId}, messages::ConfirmationTag, + treesync::{RatchetTree, TreeSync}, }; const SIGNATURE_GROUP_INFO_LABEL: &str = "GroupInfoTBS"; -/// A type that represents a group info of which the signature has not been verified. -/// It implements the [`Verifiable`] trait and can be turned into a group info by calling -/// `verify(...)` with the signature key of the [`Credential`](crate::credentials::Credential). -/// When receiving a serialized group info, it can only be deserialized into a -/// [`VerifiableGroupInfo`], which can then be turned into a group info as described above. +/// A type that represents a group info of which the signature has not been +/// verified. It implements the [`Verifiable`] trait and can be turned into a +/// group info by calling `verify(...)` with the signature key of the +/// [`Credential`](crate::credentials::Credential). When receiving a serialized +/// group info, it can only be deserialized into a [`VerifiableGroupInfo`], +/// which can then be turned into a group info as described above. #[derive(Debug, PartialEq, Clone, TlsDeserialize, TlsSize)] #[cfg_attr(any(test, feature = "test-utils"), derive(TlsSerialize))] pub struct VerifiableGroupInfo { @@ -38,6 +41,21 @@ pub enum GroupInfoError { /// Malformed. #[error("Malformed.")] Malformed, + /// The required RatchetTreeExtension is missing + #[error("The required RatchetTreeExtension is missing")] + MissingRatchetTreeExtension, + /// Invalid + #[error("Invalid")] + Invalid, + /// Ratchet Tree error + #[error(transparent)] + RatchetTreeError(#[from] crate::treesync::RatchetTreeError), + /// TreeSyncFromNodesError + #[error(transparent)] + TreeSyncFromNodesError(#[from] crate::treesync::errors::TreeSyncFromNodesError), + /// A RatchetTree extension is required for this operation + #[error("A RatchetTree extension is required for this operation")] + RequiredRatchetTree, } impl VerifiableGroupInfo { @@ -67,21 +85,24 @@ impl VerifiableGroupInfo { /// Get (unverified) ciphersuite of the verifiable group info. /// - /// Note: This method should only be used when necessary to verify the group info signature. + /// Note: This method should only be used when necessary to verify the group + /// info signature. pub fn ciphersuite(&self) -> Ciphersuite { self.payload.group_context.ciphersuite() } /// Get (unverified) signer of the verifiable group info. /// - /// Note: This method should only be used when necessary to verify the group info signature. + /// Note: This method should only be used when necessary to verify the group + /// info signature. pub(crate) fn signer(&self) -> LeafNodeIndex { self.payload.signer } /// Get (unverified) extensions of the verifiable group info. /// - /// Note: This method should only be used when necessary to verify the group info signature. + /// Note: This method should only be used when necessary to verify the group + /// info signature. pub(crate) fn extensions(&self) -> &Extensions { &self.payload.extensions } @@ -97,6 +118,44 @@ impl VerifiableGroupInfo { pub(crate) fn context(&self) -> &GroupContext { &self.payload.group_context } + + /// Do whatever it takes not to clone the RatchetTree + pub fn take_ratchet_tree( + mut self, + backend: &impl OpenMlsCryptoProvider, + ) -> Result { + let cs = self.ciphersuite(); + + let ratchet_tree = self + .payload + .extensions + .unique + .iter_mut() + .find_map(|e| match e { + Extension::RatchetTree(rt) => { + // we have to clone it here as well.. + Some(rt.ratchet_tree.clone()) + } + _ => None, + }) + .ok_or(GroupInfoError::MissingRatchetTreeExtension)? + .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)?; + + let signer_signature_key = treesync + .leaf(self.signer()) + .ok_or(GroupInfoError::Invalid)? + .signature_key() + .clone() + .into_signature_public_key_enriched(cs.signature_algorithm()); + + self.verify::(backend.crypto(), &signer_signature_key) + .map_err(|_| GroupInfoError::Invalid)?; + + Ok(ratchet_tree) + } } #[cfg(test)] diff --git a/openmls/src/treesync/mod.rs b/openmls/src/treesync/mod.rs index 9911d7c63d..f8e423a953 100644 --- a/openmls/src/treesync/mod.rs +++ b/openmls/src/treesync/mod.rs @@ -217,7 +217,16 @@ impl RatchetTree { /// A ratchet tree made of unverified nodes. This is used for deserialization /// and verification. #[derive( - PartialEq, Eq, Clone, Debug, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Default, + PartialEq, + Eq, + Clone, + Debug, + Serialize, + Deserialize, + TlsDeserialize, + TlsSerialize, + TlsSize, )] pub struct RatchetTreeIn(Vec>); @@ -426,14 +435,14 @@ impl TreeSync { pub(crate) fn from_ratchet_tree( backend: &impl OpenMlsCryptoProvider, ciphersuite: Ciphersuite, - ratchet_tree: RatchetTree, + ratchet_tree: &RatchetTree, ) -> Result { // TODO #800: Unmerged leaves should be checked let mut ts_nodes: Vec> = 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() { + for (node_index, node_option) in ratchet_tree.0.iter().enumerate() { let ts_node_option: TreeNode = match node_option { Some(node) => { let node = node.clone(); diff --git a/openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs b/openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs index c3a61ffe02..f72d138146 100644 --- a/openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs +++ b/openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs @@ -63,7 +63,7 @@ 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) + let tree_before = TreeSync::from_ratchet_tree(backend, ciphersuite, &ratchet_tree) .map_err(|e| format!("Error while creating tree sync: {e:?}"))?; let group_context = GroupContext::new( diff --git a/openmls/src/treesync/tests_and_kats/kats/kat_tree_validation.rs b/openmls/src/treesync/tests_and_kats/kats/kat_tree_validation.rs index 6ea094d387..4f3752a4c3 100644 --- a/openmls/src/treesync/tests_and_kats/kats/kat_tree_validation.rs +++ b/openmls/src/treesync/tests_and_kats/kats/kat_tree_validation.rs @@ -106,7 +106,7 @@ fn run_test_vector(test: TestElement, backend: &impl OpenMlsCryptoProvider) -> R .into_verified(ciphersuite, backend.crypto(), group_id) .unwrap(); - let treesync = TreeSync::from_ratchet_tree(backend, ciphersuite, ratchet_tree.clone()) + let treesync = TreeSync::from_ratchet_tree(backend, ciphersuite, &ratchet_tree) .map_err(|e| format!("Error while creating tree sync: {e:?}"))?; let diff = treesync.empty_diff(); diff --git a/openmls/src/treesync/tests_and_kats/kats/kat_treekem.rs b/openmls/src/treesync/tests_and_kats/kats/kat_treekem.rs index 381e0518e6..c223a9095d 100644 --- a/openmls/src/treesync/tests_and_kats/kats/kat_treekem.rs +++ b/openmls/src/treesync/tests_and_kats/kats/kat_treekem.rs @@ -98,7 +98,7 @@ pub fn run_test_vector(test: TreeKemTest, backend: &impl OpenMlsCryptoProvider) .into_verified(ciphersuite, backend.crypto(), group_id) .unwrap(); - TreeSync::from_ratchet_tree(backend, ciphersuite, ratchet_tree).unwrap() + TreeSync::from_ratchet_tree(backend, ciphersuite, &ratchet_tree).unwrap() }; let full_leaf_nodes = { diff --git a/openmls/src/treesync/tests_and_kats/tests/test_diff.rs b/openmls/src/treesync/tests_and_kats/tests/test_diff.rs index 8d58d0881b..3a7b4c112c 100644 --- a/openmls/src/treesync/tests_and_kats/tests/test_diff.rs +++ b/openmls/src/treesync/tests_and_kats/tests/test_diff.rs @@ -35,7 +35,7 @@ async fn test_free_leaf_computation( ]); // Get the encryption key pair from the leaf. - let tree = TreeSync::from_ratchet_tree(backend, ciphersuite, ratchet_tree) + let tree = TreeSync::from_ratchet_tree(backend, ciphersuite, &ratchet_tree) .expect("error generating tree"); // Create and add a new leaf. It should go to leaf index 1