Skip to content

Commit

Permalink
feat: add methods to take a RatchetTree from a GroupInfo without allo…
Browse files Browse the repository at this point in the history
…cating memory
  • Loading branch information
beltram committed Dec 12, 2023
1 parent 33a9dbf commit 978cb51
Show file tree
Hide file tree
Showing 10 changed files with 139 additions and 49 deletions.
2 changes: 1 addition & 1 deletion openmls/src/extensions/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -223,7 +223,7 @@ pub struct UnknownExtension(pub Vec<u8>);
/// A list of extensions with unique extension types.
#[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, Deserialize, tls_codec::TlsSize)]
pub struct Extensions {
unique: Vec<Extension>,
pub(crate) unique: Vec<Extension>,
}

impl tls_codec::Serialize for Extensions {
Expand Down
2 changes: 1 addition & 1 deletion openmls/src/extensions/ratchet_tree_extension.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
8 changes: 8 additions & 0 deletions openmls/src/group/mls_group/membership.rs
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,14 @@ impl MlsGroup {
self.group.public_group().members()
}

pub fn members_credentials(&self) -> impl Iterator<Item = &Credential> + '_ {
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> {
Expand Down
10 changes: 6 additions & 4 deletions openmls/src/group/public_group/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,8 @@ impl PublicGroup {
})
}

/// Create a [`PublicGroup`] instance to start tracking an existing MLS group.
/// Create a [`PublicGroup`] instance to start tracking an existing MLS
/// group.
///
/// This function performs basic validation checks and returns an error if
/// one of the checks fails. See [`CreationFromExternalError`] for more
Expand Down Expand Up @@ -261,7 +262,8 @@ impl PublicGroup {
self.treesync().export_ratchet_tree()
}

/// Add the [`QueuedProposal`] to the [`PublicGroup`]s internal [`ProposalStore`].
/// Add the [`QueuedProposal`] to the [`PublicGroup`]s internal
/// [`ProposalStore`].
pub fn add_proposal(&mut self, proposal: QueuedProposal) {
self.proposal_store.add(proposal)
}
Expand Down Expand Up @@ -304,8 +306,8 @@ impl PublicGroup {
&self.confirmation_tag
}

/// Return a reference to the leaf at the given `LeafNodeIndex` or `None` if the
/// leaf is blank.
/// Return a reference to the leaf at the given `LeafNodeIndex` or `None` if
/// the leaf is blank.
pub fn leaf(&self, leaf_index: LeafNodeIndex) -> Option<&LeafNode> {
self.treesync().leaf(leaf_index)
}
Expand Down
79 changes: 69 additions & 10 deletions openmls/src/messages/group_info.rs
Original file line number Diff line number Diff line change
@@ -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 {
Expand All @@ -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 {
Expand Down Expand Up @@ -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
}
Expand All @@ -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<RatchetTree, GroupInfoError> {
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.clone())?;

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::<GroupInfo>(backend.crypto(), &signer_signature_key)
.map_err(|_| GroupInfoError::Invalid)?;

Ok(ratchet_tree)
}
}

#[cfg(test)]
Expand Down
60 changes: 39 additions & 21 deletions openmls/src/treesync/mod.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
//! This module implements the ratchet tree component of MLS.
//!
//! It exposes the [`Node`] enum that can contain either a [`LeafNode`] or a [`ParentNode`].
//! It exposes the [`Node`] enum that can contain either a [`LeafNode`] or a
//! [`ParentNode`].
// # Internal documentation
//
Expand Down Expand Up @@ -88,7 +89,8 @@ pub use node::{leaf_node::LeafNode, parent_node::ParentNode, Node};
#[cfg(any(feature = "test-utils", test))]
pub mod tests_and_kats;

/// An exported ratchet tree as used in, e.g., [`GroupInfo`](crate::messages::group_info::GroupInfo).
/// An exported ratchet tree as used in, e.g.,
/// [`GroupInfo`](crate::messages::group_info::GroupInfo).
#[derive(PartialEq, Eq, Clone, Debug, Serialize, Deserialize, TlsSerialize, TlsSize)]
pub struct RatchetTree(Vec<Option<Node>>);

Expand Down Expand Up @@ -120,9 +122,11 @@ pub enum RatchetTreeError {
}

impl RatchetTree {
/// Create a [`RatchetTree`] from a vector of nodes stripping all trailing blank nodes.
/// Create a [`RatchetTree`] from a vector of nodes stripping all trailing
/// blank nodes.
///
/// Note: The caller must ensure to call this with a vector that is *not* empty after removing all trailing blank nodes.
/// Note: The caller must ensure to call this with a vector that is *not*
/// empty after removing all trailing blank nodes.
fn trimmed(mut nodes: Vec<Option<Node>>) -> Self {
// Remove all trailing blank nodes.
match nodes.iter().enumerate().rfind(|(_, node)| node.is_some()) {
Expand All @@ -131,7 +135,8 @@ impl RatchetTree {
nodes.resize(rightmost_nonempty_position + 1, None);
}
None => {
// If there is no rightmost non-blank node, the vector consist of blank nodes only.
// If there is no rightmost non-blank node, the vector consist of blank nodes
// only.
nodes.clear();
}
}
Expand All @@ -152,15 +157,17 @@ impl RatchetTree {
// We can check this by only looking at the last node (if any).
match nodes.last() {
Some(None) => {
// The ratchet tree is not empty, i.e., has a last node, *but* the last node *is* blank.
// The ratchet tree is not empty, i.e., has a last node, *but* the last node
// *is* blank.
Err(RatchetTreeError::TrailingBlankNodes)
}
None => {
// The ratchet tree is empty.
Err(RatchetTreeError::MissingNodes)
}
Some(Some(_)) => {
// The ratchet tree is not empty, i.e., has a last node, and the last node is not blank.
// The ratchet tree is not empty, i.e., has a last node, and the last node is
// not blank.

// Verify the nodes.
let signature_scheme = ciphersuite.signature_algorithm();
Expand Down Expand Up @@ -217,7 +224,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<Option<NodeIn>>);

Expand Down Expand Up @@ -468,10 +484,10 @@ impl TreeSync {
Ok(tree_sync)
}

/// Find the `LeafNodeIndex` which a new leaf would have if it were added to the
/// tree. This is either the left-most blank node or, if there are no blank
/// leaves, the leaf count, since adding a member would extend the tree by
/// one leaf.
/// Find the `LeafNodeIndex` which a new leaf would have if it were added to
/// the tree. This is either the left-most blank node or, if there are
/// no blank leaves, the leaf count, since adding a member would extend
/// the tree by one leaf.
pub(crate) fn free_leaf_index(&self) -> LeafNodeIndex {
let diff = self.empty_diff();
diff.free_leaf_index()
Expand Down Expand Up @@ -536,7 +552,8 @@ impl TreeSync {
self.tree.raw_leaves().filter_map(|tsn| tsn.node().as_ref())
}

/// Returns an indexed list of [`LeafNodeIndex`]es containing only full nodes.
/// Returns an indexed list of [`LeafNodeIndex`]es containing only full
/// nodes.
pub(crate) fn full_leaves_indexed(&self) -> impl Iterator<Item = (LeafNodeIndex, &LeafNode)> {
self.tree
.leaves()
Expand All @@ -556,8 +573,8 @@ impl TreeSync {

/// Returns a list of [`Member`]s containing only full nodes.
///
/// XXX: For performance reasons we probably want to have this in a borrowing
/// version as well. But it might well go away again.
/// XXX: For performance reasons we probably want to have this in a
/// borrowing version as well. But it might well go away again.
pub(crate) fn full_leave_members(&self) -> impl Iterator<Item = Member> + '_ {
self.tree
.leaves()
Expand Down Expand Up @@ -624,8 +641,8 @@ impl TreeSync {
RatchetTree::trimmed(nodes)
}

/// Return a reference to the leaf at the given `LeafNodeIndex` or `None` if the
/// leaf is blank.
/// Return a reference to the leaf at the given `LeafNodeIndex` or `None` if
/// the leaf is blank.
pub(crate) fn leaf(&self, leaf_index: LeafNodeIndex) -> Option<&LeafNode> {
let tsn = self.tree.leaf(leaf_index);
tsn.node().as_ref()
Expand Down Expand Up @@ -654,8 +671,8 @@ impl TreeSync {
/// node, as well as the derived [`EncryptionKeyPair`]s. Returns an error if
/// the target leaf is outside of the tree.
///
/// Returns TreeSyncSetPathError::PublicKeyMismatch if the derived keys don't
/// match with the existing ones.
/// Returns TreeSyncSetPathError::PublicKeyMismatch if the derived keys
/// don't match with the existing ones.
///
/// Returns TreeSyncSetPathError::LibraryError if the sender_index is not
/// in the tree.
Expand All @@ -667,8 +684,9 @@ impl TreeSync {
sender_index: LeafNodeIndex,
leaf_index: LeafNodeIndex,
) -> Result<(Vec<EncryptionKeyPair>, CommitSecret), DerivePathError> {
// We assume both nodes are in the tree, since the sender_index must be in the tree
// Skip the nodes in the subtree path for which we are an unmerged leaf.
// We assume both nodes are in the tree, since the sender_index must be in the
// tree Skip the nodes in the subtree path for which we are an unmerged
// leaf.
let subtree_path = self.tree.subtree_path(leaf_index, sender_index);
let mut keypairs = Vec::new();
for parent_index in subtree_path {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@
//! Verification:
//! * Compute `candidate_tree_after` by applying `proposal` sent by the member
//! with index `proposal_sender` to `tree_before`.
//! * Verify that serialized `candidate_tree_after` matches the provided `tree_after`
//! value.
//! * Verify that serialized `candidate_tree_after` matches the provided
//! `tree_after` value.
use ::serde::Deserialize;
use openmls_traits::OpenMlsCryptoProvider;
Expand Down
12 changes: 6 additions & 6 deletions openmls/src/treesync/tests_and_kats/kats/kat_tree_validation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -26,14 +26,14 @@
//! [the `ratchet_tree` extension](https://tools.ietf.org/html/draft-ietf-mls-protocol-17#section-12.4.3.3)
//!
//! Verification:
//! * Verify that the resolution of each node in tree with node index `i` matches
//! `resolutions[i]`.
//! * Verify that the resolution of each node in tree with node index `i`
//! matches `resolutions[i]`.
//! * Verify that the tree hash of each node in tree with node index `i` matches
//! `tree_hashes[i]`.
//! * [Verify the parent hashes](https://tools.ietf.org/html/draft-ietf-mls-protocol-17#section-7.9.2)
//! of `tree` as when joining the group.
//! * Verify the signatures on all leaves of `tree` using the provided `group_id`
//! as context.
//! * Verify the signatures on all leaves of `tree` using the provided
//! `group_id` as context.
//!
//! ### Origins of Test Trees
//! Trees in the test vector are ordered according to increasing complexity. Let
Expand All @@ -53,8 +53,8 @@
//! leaves `1`, `2` and `3`.
//! * Trees with skipping trailing blanks in the parent hash links:
//! `get_tree(n)` for `n` in `[3, 34]`.
//! * A tree with unmerged leaves: start with `get_tree(7)`, then the leaf
//! with index `0` adds a member.
//! * A tree with unmerged leaves: start with `get_tree(7)`, then the leaf with
//! index `0` adds a member.
//! * A tree with unmerged leaves and skipping blanks in the parent hash links:
//! the tree from [Figure 20](https://tools.ietf.org/html/draft-ietf-mls-protocol-17#appendix-A).
Expand Down
Loading

0 comments on commit 978cb51

Please sign in to comment.