From 84c190de90ab51b35e1d1e87e512fb9fc89dd708 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Fischer?= Date: Fri, 17 Nov 2023 07:18:49 +0100 Subject: [PATCH 01/13] Fix link to traits in openmls_rust_crypto README (#1452) --- openmls_rust_crypto/README.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/openmls_rust_crypto/README.md b/openmls_rust_crypto/README.md index 5a34750b4b..9ee818ed19 100644 --- a/openmls_rust_crypto/README.md +++ b/openmls_rust_crypto/README.md @@ -1,8 +1,8 @@ # Rust Crypto Backend -This crate implements the [OpenMLS traits](../traits/Readme.md) using the following rust crates: [hkdf], [sha2], [p256], [p384], [x25519-dalek-ng], [ed25519-dalek] [chacha20poly1305], [aes-gcm]. +This crate implements the [OpenMLS traits](../traits/README.md) using the following rust crates: [hkdf], [sha2], [p256], [p384], [x25519-dalek-ng], [ed25519-dalek] [chacha20poly1305], [aes-gcm]. -[hkdf]: https://docs.rs/hkdf/ +[hkdf]: https://docs.rs/hkdf [sha2]: https://docs.rs/sha2 [p256]: https://docs.rs/p256 [p384]: https://docs.rs/p384 From 81841e57e2dcae99cd68099666513b2a3ec1ca69 Mon Sep 17 00:00:00 2001 From: raphaelrobert Date: Sun, 3 Dec 2023 08:09:23 +0100 Subject: [PATCH 02/13] Add vector deserialization KAT (#1455) --- openmls/src/kat_vl.rs | 69 +++++++++++++++++++++++ openmls/src/lib.rs | 3 + openmls/test_vectors/deserialization.json | 58 +++++++++++++++++++ 3 files changed, 130 insertions(+) create mode 100644 openmls/src/kat_vl.rs create mode 100644 openmls/test_vectors/deserialization.json diff --git a/openmls/src/kat_vl.rs b/openmls/src/kat_vl.rs new file mode 100644 index 0000000000..0a755ac9f6 --- /dev/null +++ b/openmls/src/kat_vl.rs @@ -0,0 +1,69 @@ +//! ## Vector Deserialization +//! +//! Parameters: +//! * Header for variable length encoded Vector +//! * The encoded length +//! +//! +//! Format: +//! ``` text +//! [ +//! { +//! "vlbytes_header": /* hex-encoded binary data */, +//! "length": /* integer for the encoded size */ +//! }, +//! ... +//! ] +//! ``` +//! +//! This Test contains a List of Serialized Variable Length Headers (`vlbytes_header`) and a length. +//! +//! Verification: +//! * Decode the `vlbytes_header` +//! * Verify that the decoded length matches the given `length` + +use serde::Deserialize; +use tls_codec::{Deserialize as TlsDeserialize, VLBytes}; + +use crate::test_utils::read; + +#[derive(Deserialize)] +struct TestElement { + #[serde(with = "hex")] + vlbytes_header: Vec, + length: u32, +} + +fn run_test_vector(element: TestElement) -> Result<(), String> { + let total_length = element.vlbytes_header.len() + element.length as usize; + let mut serialized_vec = vec![0; total_length]; + serialized_vec[..element.vlbytes_header.len()].copy_from_slice(&element.vlbytes_header); + let deserialized_vec = VLBytes::tls_deserialize_exact(serialized_vec.as_slice()).unwrap(); + let deserialized_vec_len = deserialized_vec.as_slice().len(); + + if deserialized_vec_len != element.length as usize { + panic!( + "Deserialized length does not match expected length.\n\ + Expected: {0}\n\ + Actual: {1}", + element.length, deserialized_vec_len + ); + } + Ok(()) +} + +#[test] +fn read_test_vectors_deserialize() { + let _ = pretty_env_logger::try_init(); + log::debug!("Reading test vectors ..."); + + let tests: Vec = read("test_vectors/deserialization.json"); + + for test_vector in tests { + match run_test_vector(test_vector) { + Ok(_) => {} + Err(e) => panic!("Error while checking deserialization test vector.\n{e:?}"), + } + } + log::trace!("Finished test vector verification"); +} diff --git a/openmls/src/lib.rs b/openmls/src/lib.rs index 1dc94ba086..1a9438dd02 100644 --- a/openmls/src/lib.rs +++ b/openmls/src/lib.rs @@ -159,6 +159,9 @@ pub use rstest_reuse; #[macro_use] pub mod test_utils; +#[cfg(test)] +pub mod kat_vl; + // === Modules === #[macro_use] diff --git a/openmls/test_vectors/deserialization.json b/openmls/test_vectors/deserialization.json new file mode 100644 index 0000000000..81c767b3b7 --- /dev/null +++ b/openmls/test_vectors/deserialization.json @@ -0,0 +1,58 @@ +[ + { + "vlbytes_header": "00", + "length": 0 + }, + { + "vlbytes_header": "0d", + "length": 13 + }, + { + "vlbytes_header": "36", + "length": 54 + }, + { + "vlbytes_header": "3f", + "length": 63 + }, + { + "vlbytes_header": "4040", + "length": 64 + }, + { + "vlbytes_header": "40ff", + "length": 255 + }, + { + "vlbytes_header": "4185", + "length": 389 + }, + { + "vlbytes_header": "4aaa", + "length": 2730 + }, + { + "vlbytes_header": "4fff", + "length": 4095 + }, + { + "vlbytes_header": "7fff", + "length": 16383 + }, + { + "vlbytes_header": "80004000", + "length": 16384 + }, + { + "vlbytes_header": "8000beef", + "length": 48879 + }, + { + "vlbytes_header": "8000dead", + "length": 57005 + }, + { + "vlbytes_header": "bfffffff", + "length": 1073741823 + } + ] \ No newline at end of file From d249c76bb959d15cb465fcd35842c1921fe829a3 Mon Sep 17 00:00:00 2001 From: Franziskus Kiefer Date: Tue, 5 Dec 2023 14:10:18 +0100 Subject: [PATCH 03/13] bump tls_codec and hpke-rs (#1453) * update hpke-rs to 0.2.0 * update tls-codec to 0.4.0 --- Cargo.toml | 4 ++-- basic_credential/Cargo.toml | 4 ++-- openmls/Cargo.toml | 9 +++++++-- openmls_rust_crypto/Cargo.toml | 12 ++++++------ 4 files changed, 17 insertions(+), 12 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 1771a61b2c..e836719528 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -9,10 +9,10 @@ members = [ "memory_keystore", "delivery-service/ds", "delivery-service/ds-lib", - "basic_credential" + "basic_credential", ] resolver = "2" # Central dependency management for some crates [workspace.dependencies] -tls_codec = { version = "0.3.0", features = ["derive", "serde", "mls"] } +tls_codec = { version = "0.4.0", features = ["derive", "serde", "mls"] } diff --git a/basic_credential/Cargo.toml b/basic_credential/Cargo.toml index 5749f3d98d..c008f3c1cc 100644 --- a/basic_credential/Cargo.toml +++ b/basic_credential/Cargo.toml @@ -20,5 +20,5 @@ p256 = { version = "0.13" } rand = "0.8" [features] -clonable = [] # Make the keys clonable -test-utils = [] # Only use for tests! +clonable = [] # Make the keys clonable +test-utils = [] # Only use for tests! diff --git a/openmls/Cargo.toml b/openmls/Cargo.toml index 6537e526c1..ce6d33c2a8 100644 --- a/openmls/Cargo.toml +++ b/openmls/Cargo.toml @@ -24,7 +24,10 @@ serde_json = { version = "1.0", optional = true } # Crypto providers required for KAT and testing - "test-utils" feature itertools = { version = "0.10", optional = true } openmls_rust_crypto = { version = "0.2.0", path = "../openmls_rust_crypto", optional = true } -openmls_basic_credential = { version = "0.2.0", path = "../basic_credential", optional = true, features = ["clonable", "test-utils"] } +openmls_basic_credential = { version = "0.2.0", path = "../basic_credential", optional = true, features = [ + "clonable", + "test-utils", +] } rstest = { version = "^0.16", optional = true } rstest_reuse = { version = "0.4", optional = true } @@ -50,7 +53,9 @@ hex = { version = "0.4", features = ["serde"] } itertools = "0.10" lazy_static = "1.4" openmls = { path = ".", features = ["test-utils"] } -openmls_traits = { version = "0.2.0", path = "../traits", features = ["test-utils"] } +openmls_traits = { version = "0.2.0", path = "../traits", features = [ + "test-utils", +] } pretty_env_logger = "0.5" rstest = "^0.16" rstest_reuse = "0.4" diff --git a/openmls_rust_crypto/Cargo.toml b/openmls_rust_crypto/Cargo.toml index fdc7f7062e..6bb3ef2994 100644 --- a/openmls_rust_crypto/Cargo.toml +++ b/openmls_rust_crypto/Cargo.toml @@ -12,6 +12,10 @@ readme = "README.md" [dependencies] openmls_traits = { version = "0.2.0", path = "../traits" } openmls_memory_keystore = { version = "0.2.0", path = "../memory_keystore" } +hpke = { version = "0.2.0", package = "hpke-rs", default-features = false, features = [ + "hazmat", + "serialization", +] } # Rust Crypto dependencies sha2 = { version = "0.10" } aes-gcm = { version = "0.10" } @@ -22,12 +26,8 @@ p256 = { version = "0.13" } hkdf = { version = "0.12" } rand = "0.8" rand_chacha = { version = "0.3" } -hpke = { version = "0.1.1", package = "hpke-rs", default-features = false, features = [ - "hazmat", - "serialization", -] } -hpke-rs-crypto = { version = "0.1.2" } -hpke-rs-rust-crypto = { version = "0.1.2" } +hpke-rs-crypto = { version = "0.2.0" } +hpke-rs-rust-crypto = { version = "0.2.0" } tls_codec = { workspace = true } thiserror = "1.0" serde = { version = "^1.0", features = ["derive"] } From e0356c7aa4b1da77b0b9e8b293b04b895b60ea32 Mon Sep 17 00:00:00 2001 From: Konrad Kohbrok Date: Wed, 3 Jan 2024 17:18:40 +0100 Subject: [PATCH 04/13] minor changes to silence clippy warnings (#1466) --- openmls/src/credentials/codec.rs | 3 +-- openmls/src/group/mod.rs | 7 +------ openmls/src/prelude.rs | 10 ++++++++-- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/openmls/src/credentials/codec.rs b/openmls/src/credentials/codec.rs index c4169936d4..0cc8df9169 100644 --- a/openmls/src/credentials/codec.rs +++ b/openmls/src/credentials/codec.rs @@ -31,8 +31,7 @@ impl tls_codec::Serialize for Credential { impl tls_codec::Deserialize for Credential { fn tls_deserialize(bytes: &mut R) -> Result { let val = u16::tls_deserialize(bytes)?; - let credential_type = CredentialType::try_from(val) - .map_err(|e| tls_codec::Error::DecodingError(e.to_string()))?; + let credential_type = CredentialType::from(val); match credential_type { CredentialType::Basic => Ok(Credential::from(MlsCredentialType::Basic( BasicCredential::tls_deserialize(bytes)?, diff --git a/openmls/src/group/mod.rs b/openmls/src/group/mod.rs index 5ffde23222..10be650ff2 100644 --- a/openmls/src/group/mod.rs +++ b/openmls/src/group/mod.rs @@ -29,10 +29,9 @@ pub(crate) mod errors; pub use core_group::proposals::*; pub use core_group::staged_commit::StagedCommit; pub use errors::*; -pub use group_context::*; +pub use group_context::GroupContext; pub use mls_group::config::*; pub use mls_group::membership::*; -pub use mls_group::processing::*; pub use mls_group::*; pub use public_group::*; @@ -41,11 +40,7 @@ pub use public_group::*; pub(crate) use core_group::create_commit_params::*; #[cfg(any(feature = "test-utils", test))] pub(crate) mod tests; -#[cfg(any(feature = "test-utils", test))] -pub use group_context::GroupContext; use openmls_traits::random::OpenMlsRand; -#[cfg(any(feature = "test-utils", test))] -pub use proposals::*; /// A group ID. The group ID is chosen by the creator of the group and should be globally unique. #[derive( diff --git a/openmls/src/prelude.rs b/openmls/src/prelude.rs index de04bfffe6..6c55797973 100644 --- a/openmls/src/prelude.rs +++ b/openmls/src/prelude.rs @@ -4,7 +4,7 @@ // MlsGroup pub use crate::group::{config::CryptoConfig, core_group::Member, ser::*, *}; -pub use crate::group::public_group::{errors::*, process::*, *}; +pub use crate::group::public_group::{errors::*, PublicGroup}; // Ciphersuite pub use crate::ciphersuite::{hash_ref::KeyPackageRef, signable::*, signature::*, *}; @@ -22,7 +22,13 @@ pub use crate::versions::*; pub use crate::extensions::{errors::*, *}; // Framing -pub use crate::framing::{message_in::*, message_out::*, sender::*, validation::*, *}; +pub use crate::framing::{ + message_in::{MlsMessageIn, MlsMessageInBody, ProtocolMessage}, + message_out::MlsMessageOut, + sender::Sender, + validation::{ApplicationMessage, ProcessedMessage, ProcessedMessageContent}, + *, +}; // Key packages pub use crate::key_packages::{errors::*, *}; From b1c79163bdf7c46c80e6dee7f900088b8d389087 Mon Sep 17 00:00:00 2001 From: beltram <7527120+beltram@users.noreply.github.com> Date: Thu, 4 Jan 2024 13:31:35 +0100 Subject: [PATCH 05/13] perf: small perf improvements (#1467) * perf: cut down one iteration over the tree in 'free_leaf_index' * perf: remove needless node clone in 'TreeSync::from_ratchet_tree' --- openmls/src/treesync/diff.rs | 4 ++-- openmls/src/treesync/mod.rs | 5 +---- 2 files changed, 3 insertions(+), 6 deletions(-) diff --git a/openmls/src/treesync/diff.rs b/openmls/src/treesync/diff.rs index e732d91795..4b7a83901e 100644 --- a/openmls/src/treesync/diff.rs +++ b/openmls/src/treesync/diff.rs @@ -199,13 +199,13 @@ impl<'a> TreeSyncDiff<'a> { /// Find and return the index of either the left-most blank leaf, or, if /// there are no blank leaves, the leaf count. pub(crate) fn free_leaf_index(&self) -> LeafNodeIndex { - let leaf_count = self.diff.leaves().count() as u32; - + let mut leaf_count = 0; // Search for blank leaves in existing leaves for (leaf_index, leaf_id) in self.diff.leaves() { if leaf_id.node().is_none() { return leaf_index; } + leaf_count += 1; } // Return the next free virtual blank leaf diff --git a/openmls/src/treesync/mod.rs b/openmls/src/treesync/mod.rs index 083b057c49..994d074bad 100644 --- a/openmls/src/treesync/mod.rs +++ b/openmls/src/treesync/mod.rs @@ -437,10 +437,7 @@ impl TreeSync { // 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 = match node_option { - Some(node) => { - let node = node.clone(); - TreeSyncNode::from(node).into() - } + Some(node) => TreeSyncNode::from(node).into(), None => { if node_index % 2 == 0 { TreeNode::Leaf(TreeSyncLeafNode::blank()) From 16f425f53d6530cf8b26fd92e8654fac6f168539 Mon Sep 17 00:00:00 2001 From: Jan Winkelmann <146678+keks@users.noreply.github.com> Date: Thu, 4 Jan 2024 17:14:15 +0100 Subject: [PATCH 06/13] Add `update_path_leaf_node` method to `StagedCommit`. (#1461) In order to validate the credentials of the leaf node in the optional update path of a staged commit, the caller needs to be able to access it. They can now do so using the new method `update_path_leaf_node`. The test was added in mls_group instead of core_group, because we need too much high-level functionality to do it with the low-level API. Co-authored-by: Jan Winkelmann (keks) Co-authored-by: Franziskus Kiefer Co-authored-by: raphaelrobert --- openmls/src/group/core_group/mod.rs | 6 + openmls/src/group/core_group/staged_commit.rs | 141 ++++++++------ openmls/src/group/mls_group/test_mls_group.rs | 180 ++++++++++++++++++ 3 files changed, 272 insertions(+), 55 deletions(-) diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index 3983d127ab..5fb10ceb0e 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -907,6 +907,11 @@ impl CoreGroup { PathComputationResult::default() }; + let update_path_leaf_node = path_computation_result + .encrypted_path + .as_ref() + .map(|path| path.leaf_node().clone()); + // Create commit message let commit = Commit { proposals: proposal_reference_list, @@ -1075,6 +1080,7 @@ impl CoreGroup { // The committer is not allowed to include their own update // proposal, so there is no extra keypair to store here. None, + update_path_leaf_node, ); let staged_commit = StagedCommit::new( proposal_queue, diff --git a/openmls/src/group/core_group/staged_commit.rs b/openmls/src/group/core_group/staged_commit.rs index 5e62c3d46f..3573dc384b 100644 --- a/openmls/src/group/core_group/staged_commit.rs +++ b/openmls/src/group/core_group/staged_commit.rs @@ -153,66 +153,83 @@ impl CoreGroup { } // Determine if Commit has a path - let (commit_secret, new_keypairs, new_leaf_keypair_option) = if let Some(path) = - commit.path.clone() - { - // Update the public group - // ValSem202: Path must be the right length - diff.apply_received_update_path(provider.crypto(), ciphersuite, sender_index, &path)?; - - // Update group context - diff.update_group_context(provider.crypto())?; - - let decryption_keypairs: Vec<&EncryptionKeyPair> = old_epoch_keypairs - .iter() - .chain(leaf_node_keypairs.iter()) - .collect(); + let (commit_secret, new_keypairs, new_leaf_keypair_option, update_path_leaf_node) = + if let Some(path) = commit.path.clone() { + // Update the public group + // ValSem202: Path must be the right length + diff.apply_received_update_path( + provider.crypto(), + ciphersuite, + sender_index, + &path, + )?; + + // Update group context + diff.update_group_context(provider.crypto())?; + + let decryption_keypairs: Vec<&EncryptionKeyPair> = old_epoch_keypairs + .iter() + .chain(leaf_node_keypairs.iter()) + .collect(); - // ValSem203: Path secrets must decrypt correctly - // ValSem204: Public keys from Path must be verified and match the private keys from the direct path - let (new_keypairs, commit_secret) = diff.decrypt_path( - provider.crypto(), - &decryption_keypairs, - self.own_leaf_index(), - sender_index, - path.nodes(), - &apply_proposals_values.exclusion_list(), - )?; + // ValSem203: Path secrets must decrypt correctly + // ValSem204: Public keys from Path must be verified and match the private keys from the direct path + let (new_keypairs, commit_secret) = diff.decrypt_path( + provider.crypto(), + &decryption_keypairs, + self.own_leaf_index(), + sender_index, + path.nodes(), + &apply_proposals_values.exclusion_list(), + )?; + + // Check if one of our update proposals was applied. If so, we + // need to store that keypair separately, because after merging + // it needs to be removed from the key store separately and in + // addition to the removal of the keypairs of the previous + // epoch. + let new_leaf_keypair_option = if let Some(leaf) = diff.leaf(self.own_leaf_index()) { + leaf_node_keypairs.into_iter().find_map(|keypair| { + if leaf.encryption_key() == keypair.public_key() { + Some(keypair) + } else { + None + } + }) + } else { + // We should have an own leaf at this point. + debug_assert!(false); + None + }; - // Check if one of our update proposals was applied. If so, we - // need to store that keypair separately, because after merging - // it needs to be removed from the key store separately and in - // addition to the removal of the keypairs of the previous - // epoch. - let new_leaf_keypair_option = if let Some(leaf) = diff.leaf(self.own_leaf_index()) { - leaf_node_keypairs.into_iter().find_map(|keypair| { - if leaf.encryption_key() == keypair.public_key() { - Some(keypair) - } else { - None - } - }) + // Return the leaf node in the update path so the credential can be validated. + // Since the diff has already been updated, this should be the same as the leaf + // at the sender index. + let update_path_leaf_node = Some(path.leaf_node().clone()); + debug_assert_eq!(diff.leaf(sender_index), path.leaf_node().into()); + + ( + commit_secret, + new_keypairs, + new_leaf_keypair_option, + update_path_leaf_node, + ) } else { - // We should have an own leaf at this point. - debug_assert!(false); - None - }; - (commit_secret, new_keypairs, new_leaf_keypair_option) - } else { - if apply_proposals_values.path_required { - // ValSem201 - return Err(StageCommitError::RequiredPathNotFound); - } + if apply_proposals_values.path_required { + // ValSem201 + return Err(StageCommitError::RequiredPathNotFound); + } - // Even if there is no path, we have to update the group context. - diff.update_group_context(provider.crypto())?; + // Even if there is no path, we have to update the group context. + diff.update_group_context(provider.crypto())?; - ( - CommitSecret::zero_secret(ciphersuite, self.version()), - vec![], - None, - ) - }; + ( + CommitSecret::zero_secret(ciphersuite, self.version()), + vec![], + None, + None, + ) + }; // Update the confirmed transcript hash before we compute the confirmation tag. diff.update_confirmed_transcript_hash(provider.crypto(), mls_content)?; @@ -269,6 +286,7 @@ impl CoreGroup { staged_diff, new_keypairs, new_leaf_keypair_option, + update_path_leaf_node, ))); Ok(StagedCommit::new(proposal_queue, staged_commit_state)) @@ -417,6 +435,16 @@ impl StagedCommit { self.staged_proposal_queue.psk_proposals() } + /// Returns the leaf node of the (optional) update path. + pub fn update_path_leaf_node(&self) -> Option<&LeafNode> { + match self.state { + StagedCommitState::PublicState(_) => None, + StagedCommitState::GroupMember(ref group_member_state) => { + group_member_state.update_path_leaf_node.as_ref() + } + } + } + /// Returns an iterator over all [`QueuedProposal`]s. pub(crate) fn queued_proposals(&self) -> impl Iterator { self.staged_proposal_queue.queued_proposals() @@ -461,6 +489,7 @@ pub(crate) struct MemberStagedCommitState { staged_diff: StagedPublicGroupDiff, new_keypairs: Vec, new_leaf_keypair_option: Option, + update_path_leaf_node: Option, } impl MemberStagedCommitState { @@ -470,6 +499,7 @@ impl MemberStagedCommitState { staged_diff: StagedPublicGroupDiff, new_keypairs: Vec, new_leaf_keypair_option: Option, + update_path_leaf_node: Option, ) -> Self { Self { group_epoch_secrets, @@ -477,6 +507,7 @@ impl MemberStagedCommitState { staged_diff, new_keypairs, new_leaf_keypair_option, + update_path_leaf_node, } } diff --git a/openmls/src/group/mls_group/test_mls_group.rs b/openmls/src/group/mls_group/test_mls_group.rs index 8cae8a6679..71feaa5ac7 100644 --- a/openmls/src/group/mls_group/test_mls_group.rs +++ b/openmls/src/group/mls_group/test_mls_group.rs @@ -1,6 +1,7 @@ use core_group::test_core_group::setup_client; use openmls_rust_crypto::OpenMlsRustCrypto; use openmls_traits::{key_store::OpenMlsKeyStore, OpenMlsProvider}; +use tls_codec::{Deserialize, Serialize}; use crate::{ binary_tree::LeafNodeIndex, @@ -334,6 +335,185 @@ fn test_invalid_plaintext(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvi ); } +#[apply(ciphersuites_and_providers)] +fn test_commit_with_update_path_leaf_node( + ciphersuite: Ciphersuite, + provider: &impl OpenMlsProvider, +) { + let group_id = GroupId::from_slice(b"Test Group"); + + let (alice_credential_with_key, _alice_kpb, alice_signer, _alice_pk) = + setup_client("Alice", ciphersuite, provider); + let (_bob_credential, bob_kpb, _bob_signer, _bob_pk) = + setup_client("Bob", ciphersuite, provider); + + // Define the MlsGroup configuration + let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + + // === Alice creates a group === + let mut alice_group = MlsGroup::new_with_group_id( + provider, + &alice_signer, + &mls_group_config, + group_id, + alice_credential_with_key.clone(), + ) + .expect("An unexpected error occurred."); + + // There should be no pending commit after group creation. + assert!(alice_group.pending_commit().is_none()); + + let bob_key_package = bob_kpb.key_package(); + + // === Alice adds Bob to the group === + let (proposal, _) = alice_group + .propose_add_member(provider, &alice_signer, bob_key_package) + .expect("error creating self-update proposal"); + + let alice_processed_message = alice_group + .process_message(provider, proposal.into_protocol_message().unwrap()) + .expect("Could not process messages."); + assert!(alice_group.pending_commit().is_none()); + + if let ProcessedMessageContent::ProposalMessage(staged_proposal) = + alice_processed_message.into_content() + { + alice_group.store_pending_proposal(*staged_proposal); + } else { + unreachable!("Expected a StagedCommit."); + } + + println!("\nCreating commit with add proposal."); + let (_msg, welcome_option, _group_info) = alice_group + .self_update(provider, &alice_signer) + .expect("error creating self-update commit"); + println!("Done creating commit."); + + // Merging the pending commit should clear the pending commit and we should + // end up in the same state as bob. + alice_group + .merge_pending_commit(provider) + .expect("error merging pending commit"); + assert!(alice_group.pending_commit().is_none()); + assert!(alice_group.pending_proposals().next().is_none()); + + let mut bob_group = MlsGroup::new_from_welcome( + provider, + &mls_group_config, + welcome_option + .expect("no welcome after commit") + .into_welcome() + .expect("Unexpected message type."), + Some(alice_group.export_ratchet_tree().into()), + ) + .expect("error creating group from welcome"); + + assert_eq!( + bob_group.export_ratchet_tree(), + alice_group.export_ratchet_tree() + ); + assert_eq!( + bob_group.export_secret(provider.crypto(), "test", &[], ciphersuite.hash_length()), + alice_group.export_secret(provider.crypto(), "test", &[], ciphersuite.hash_length()) + ); + // Bob is added and the state aligns. + + // === Make a new, empty commit and check that the leaf node credentials match === + + println!("\nCreating commit with add proposal."); + let (commit_msg, _welcome_option, _group_info) = alice_group + .self_update(provider, &alice_signer) + .expect("error creating self-update commit"); + println!("Done creating commit."); + + // empty commits should only produce a single message + assert!(_welcome_option.is_none()); + assert!(_group_info.is_none()); + + // There should be a pending commit after issuing a self-update commit. + let alice_pending_commit = alice_group + .pending_commit() + .expect("alice should have the self-update as pending commit"); + + // The credential on the update_path leaf node should be set and be the same as alice's + // credential + let alice_update_path_leaf_node = alice_pending_commit + .update_path_leaf_node() + .expect("expected alice's staged commit to have an update path"); + assert_eq!( + alice_update_path_leaf_node.credential(), + &alice_credential_with_key.credential + ); + + // great, they match! now commit + alice_group + .merge_pending_commit(provider) + .expect("alice failed to merge the pending empty commit"); + + // === transfer message to bob and process it === + + // this requires serializing and deserializing + let mut wire_msg = Vec::::new(); + commit_msg + .tls_serialize(&mut wire_msg) + .expect("alice failed serializing her message"); + let msg_in = MlsMessageIn::tls_deserialize(&mut &wire_msg[..]) + .expect("bob failed deserializing alice's message"); + + // neither party should have pending proposals + assert!(alice_group.pending_proposals().next().is_none()); + assert!(bob_group.pending_proposals().next().is_none()); + + // neither should have pending commits after merging and before processing + assert!(bob_group.pending_commit().is_none()); + assert!(alice_group.pending_commit().is_none()); + + // further process the deserialized message + let processed_message = bob_group + .process_message(provider, msg_in) + .expect("bob failed processing alice's message"); + + // the processed message must be a staged commit message + assert!(matches!( + processed_message.content(), + ProcessedMessageContent::StagedCommitMessage(_) + )); + + if let ProcessedMessageContent::StagedCommitMessage(staged_commit) = + processed_message.into_content() + { + // bob must check the credential in the leaf node of the update_path of alice's commit + let bob_update_path_leaf_node = staged_commit + .update_path_leaf_node() + .expect("staged commit received by bob should carry an update path with a leaf node"); + assert_eq!( + bob_update_path_leaf_node.credential(), + &alice_credential_with_key.credential + ); + + // bob merges alice's message + bob_group + .merge_staged_commit(provider, *staged_commit) + .expect("bob failed merging alice's empty commit (staged)"); + + // finally, the state should match + assert_eq!( + bob_group.export_ratchet_tree(), + alice_group.export_ratchet_tree() + ); + assert_eq!( + bob_group.export_secret(provider.crypto(), "test", &[], ciphersuite.hash_length()), + alice_group.export_secret(provider.crypto(), "test", &[], ciphersuite.hash_length()) + ); + } else { + unreachable!() + } + + // neither should have pending commits after merging and processing + assert!(bob_group.pending_commit().is_none()); + assert!(alice_group.pending_commit().is_none()); +} + #[apply(ciphersuites_and_providers)] fn test_pending_commit_logic(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let group_id = GroupId::from_slice(b"Test Group"); From 2b8121d258ea6a5c512141afbd12e8bb4590c1d5 Mon Sep 17 00:00:00 2001 From: Konrad Kohbrok Date: Mon, 8 Jan 2024 09:33:20 +0100 Subject: [PATCH 07/13] Derive DeserializeBytes alongside Deserialize (#1468) --- basic_credential/src/lib.rs | 6 +- delivery-service/ds-lib/src/lib.rs | 5 +- .../array_representation/treemath.rs | 3 +- openmls/src/ciphersuite/codec.rs | 25 +++- openmls/src/ciphersuite/hash_ref.rs | 4 +- openmls/src/ciphersuite/hpke.rs | 4 +- openmls/src/ciphersuite/mac.rs | 4 +- openmls/src/ciphersuite/mod.rs | 2 +- openmls/src/ciphersuite/reuse_guard.rs | 2 +- openmls/src/ciphersuite/signature.rs | 25 +++- openmls/src/credentials/codec.rs | 28 +++-- openmls/src/credentials/mod.rs | 38 ++++-- .../extensions/application_id_extension.rs | 13 +- openmls/src/extensions/codec.rs | 14 ++- .../src/extensions/external_pub_extension.rs | 13 +- .../extensions/external_sender_extension.rs | 25 +++- openmls/src/extensions/last_resort.rs | 3 +- openmls/src/extensions/mod.rs | 50 ++++++-- .../src/extensions/ratchet_tree_extension.rs | 13 +- .../src/extensions/required_capabilities.rs | 3 +- openmls/src/framing/message_in.rs | 2 +- openmls/src/framing/mls_content_in.rs | 22 +++- openmls/src/framing/mod.rs | 24 +++- openmls/src/framing/private_message_in.rs | 8 +- openmls/src/framing/public_message.rs | 14 ++- openmls/src/framing/public_message_in.rs | 14 ++- openmls/src/framing/sender.rs | 17 ++- openmls/src/group/group_context.rs | 11 +- openmls/src/group/mod.rs | 2 + openmls/src/key_packages/key_package_in.rs | 24 +++- openmls/src/key_packages/lifetime.rs | 14 ++- openmls/src/messages/group_info.rs | 16 ++- openmls/src/messages/mod.rs | 34 +++++- openmls/src/messages/proposals.rs | 113 +++++++++++++++--- openmls/src/messages/proposals_in.rs | 43 ++++++- openmls/src/schedule/mod.rs | 4 +- openmls/src/schedule/psk.rs | 7 +- openmls/src/treesync/mod.rs | 13 +- openmls/src/treesync/node.rs | 13 +- openmls/src/treesync/node/codec.rs | 27 ++++- openmls/src/treesync/node/encryption_keys.rs | 22 +++- openmls/src/treesync/node/leaf_node.rs | 38 +++++- .../treesync/node/leaf_node/capabilities.rs | 13 +- openmls/src/treesync/node/parent_node.rs | 13 +- openmls/src/treesync/treekem.rs | 24 +++- openmls/src/versions.rs | 3 +- traits/src/types.rs | 38 +++++- 47 files changed, 716 insertions(+), 137 deletions(-) diff --git a/basic_credential/src/lib.rs b/basic_credential/src/lib.rs index d600dbc789..7a1d441689 100644 --- a/basic_credential/src/lib.rs +++ b/basic_credential/src/lib.rs @@ -15,13 +15,15 @@ use openmls_traits::{ use p256::ecdsa::{signature::Signer as P256Signer, Signature, SigningKey}; use rand::rngs::OsRng; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; /// A signature key pair for the basic credential. /// /// This can be used as keys to implement the MLS basic credential. It is a simple /// private and public key pair with corresponding signature scheme. -#[derive(TlsSerialize, TlsSize, TlsDeserialize, serde::Serialize, serde::Deserialize)] +#[derive( + TlsSerialize, TlsSize, TlsDeserialize, TlsDeserializeBytes, serde::Serialize, serde::Deserialize, +)] #[cfg_attr(feature = "clonable", derive(Clone))] pub struct SignatureKeyPair { private: Vec, diff --git a/delivery-service/ds-lib/src/lib.rs b/delivery-service/ds-lib/src/lib.rs index 05c750a19b..85c683e043 100644 --- a/delivery-service/ds-lib/src/lib.rs +++ b/delivery-service/ds-lib/src/lib.rs @@ -9,8 +9,8 @@ use std::collections::HashSet; use openmls::prelude::*; use tls_codec::{ - TlsByteSliceU16, TlsByteVecU16, TlsByteVecU32, TlsByteVecU8, TlsDeserialize, TlsSerialize, - TlsSize, TlsVecU32, + TlsByteSliceU16, TlsByteVecU16, TlsByteVecU32, TlsByteVecU8, TlsDeserialize, + TlsDeserializeBytes, TlsSerialize, TlsSize, TlsVecU32, }; /// Information about a client. @@ -38,6 +38,7 @@ pub struct ClientInfo { PartialEq, TlsSerialize, TlsDeserialize, + TlsDeserializeBytes, TlsSize, serde::Serialize, serde::Deserialize, diff --git a/openmls/src/binary_tree/array_representation/treemath.rs b/openmls/src/binary_tree/array_representation/treemath.rs index f0c89e6a63..b74380e539 100644 --- a/openmls/src/binary_tree/array_representation/treemath.rs +++ b/openmls/src/binary_tree/array_representation/treemath.rs @@ -1,7 +1,7 @@ use std::cmp::Ordering; use serde::{Deserialize, Serialize}; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; pub(crate) const MAX_TREE_SIZE: u32 = 1 << 30; pub(crate) const MIN_TREE_SIZE: u32 = 1; @@ -19,6 +19,7 @@ pub(crate) const MIN_TREE_SIZE: u32 = 1; Serialize, Deserialize, TlsDeserialize, + TlsDeserializeBytes, TlsSerialize, TlsSize, )] diff --git a/openmls/src/ciphersuite/codec.rs b/openmls/src/ciphersuite/codec.rs index 49ba8f24e6..c8b0296517 100644 --- a/openmls/src/ciphersuite/codec.rs +++ b/openmls/src/ciphersuite/codec.rs @@ -3,22 +3,25 @@ use std::io::{Read, Write}; +use ::tls_codec::Error; +use tls_codec::{Deserialize, DeserializeBytes, Serialize, Size}; + use crate::ciphersuite::*; -impl tls_codec::Size for Secret { +impl Size for Secret { fn tls_serialized_len(&self) -> usize { self.value.tls_serialized_len() } } -impl tls_codec::Serialize for Secret { - fn tls_serialize(&self, writer: &mut W) -> Result { +impl Serialize for Secret { + fn tls_serialize(&self, writer: &mut W) -> Result { self.value.tls_serialize(writer) } } -impl tls_codec::Deserialize for Secret { - fn tls_deserialize(bytes: &mut R) -> Result { +impl Deserialize for Secret { + fn tls_deserialize(bytes: &mut R) -> Result { let value = Vec::tls_deserialize(bytes)?; Ok(Secret { value: value.into(), @@ -27,3 +30,15 @@ impl tls_codec::Deserialize for Secret { }) } } + +impl DeserializeBytes for Secret { + fn tls_deserialize_bytes(bytes: &[u8]) -> Result<(Self, &[u8]), Error> + where + Self: Sized, + { + let mut bytes_ref = bytes; + let secret = Secret::tls_deserialize(&mut bytes_ref)?; + let remainder = &bytes[secret.tls_serialized_len()..]; + Ok((secret, remainder)) + } +} diff --git a/openmls/src/ciphersuite/hash_ref.rs b/openmls/src/ciphersuite/hash_ref.rs index 9c5ae312e9..567e1e0568 100644 --- a/openmls/src/ciphersuite/hash_ref.rs +++ b/openmls/src/ciphersuite/hash_ref.rs @@ -29,7 +29,8 @@ use openmls_traits::{crypto::OpenMlsCrypto, types::CryptoError}; use serde::{Deserialize, Serialize}; use tls_codec::{ - Serialize as TlsSerializeTrait, TlsDeserialize, TlsSerialize, TlsSize, VLByteSlice, VLBytes, + Serialize as TlsSerializeTrait, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, + VLByteSlice, VLBytes, }; use super::Ciphersuite; @@ -48,6 +49,7 @@ const PROPOSAL_REF_LABEL: &[u8; 26] = b"MLS 1.0 Proposal Reference"; PartialOrd, Deserialize, TlsDeserialize, + TlsDeserializeBytes, TlsSerialize, TlsSize, )] diff --git a/openmls/src/ciphersuite/hpke.rs b/openmls/src/ciphersuite/hpke.rs index 46d2805ba1..061c4d1249 100644 --- a/openmls/src/ciphersuite/hpke.rs +++ b/openmls/src/ciphersuite/hpke.rs @@ -39,7 +39,7 @@ use openmls_traits::{ types::{Ciphersuite, CryptoError, HpkeCiphertext}, }; use thiserror::Error; -use tls_codec::{Serialize, TlsDeserialize, TlsSerialize, TlsSize, VLBytes}; +use tls_codec::{Serialize, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, VLBytes}; use super::LABEL_PREFIX; @@ -68,7 +68,7 @@ impl From for Error { } } -#[derive(Debug, Clone, TlsSerialize, TlsDeserialize, TlsSize)] +#[derive(Debug, Clone, TlsSerialize, TlsDeserialize, TlsDeserializeBytes, TlsSize)] pub struct EncryptContext { label: VLBytes, context: VLBytes, diff --git a/openmls/src/ciphersuite/mac.rs b/openmls/src/ciphersuite/mac.rs index a5b87e6781..d61fb39a7a 100644 --- a/openmls/src/ciphersuite/mac.rs +++ b/openmls/src/ciphersuite/mac.rs @@ -3,7 +3,9 @@ use super::*; /// 7.1 Content Authentication /// /// opaque MAC; -#[derive(Debug, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize)] +#[derive( + Debug, Clone, Serialize, Deserialize, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, +)] pub(crate) struct Mac { pub(crate) mac_value: VLBytes, } diff --git a/openmls/src/ciphersuite/mod.rs b/openmls/src/ciphersuite/mod.rs index a6ed47d79f..f7e7e279c7 100644 --- a/openmls/src/ciphersuite/mod.rs +++ b/openmls/src/ciphersuite/mod.rs @@ -4,7 +4,7 @@ //! See `codec.rs` and `ciphersuites.rs` for internals. use crate::versions::ProtocolVersion; -use ::tls_codec::{TlsDeserialize, TlsSerialize, TlsSize, VLBytes}; +use ::tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, VLBytes}; use openmls_traits::{ crypto::OpenMlsCrypto, random::OpenMlsRand, diff --git a/openmls/src/ciphersuite/reuse_guard.rs b/openmls/src/ciphersuite/reuse_guard.rs index 32edd20ef7..1bcf04a3b9 100644 --- a/openmls/src/ciphersuite/reuse_guard.rs +++ b/openmls/src/ciphersuite/reuse_guard.rs @@ -3,7 +3,7 @@ use super::*; /// Re-use guard size. pub(crate) const REUSE_GUARD_BYTES: usize = 4; -#[derive(Debug, Clone, Copy, TlsSerialize, TlsDeserialize, TlsSize)] +#[derive(Debug, Clone, Copy, TlsSerialize, TlsDeserialize, TlsDeserializeBytes, TlsSize)] #[cfg_attr(test, derive(PartialEq, Eq))] pub struct ReuseGuard { pub(in crate::ciphersuite) value: [u8; REUSE_GUARD_BYTES], diff --git a/openmls/src/ciphersuite/signature.rs b/openmls/src/ciphersuite/signature.rs index 594590df04..47d94e7b1c 100644 --- a/openmls/src/ciphersuite/signature.rs +++ b/openmls/src/ciphersuite/signature.rs @@ -8,7 +8,16 @@ use super::{LABEL_PREFIX, *}; /// Signature. #[derive( - Debug, PartialEq, Eq, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Debug, + PartialEq, + Eq, + Clone, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub struct Signature { value: VLBytes, @@ -30,7 +39,7 @@ impl From> for Signature { /// opaque content = Content; /// } SignContent; /// ``` -#[derive(Debug, Clone, TlsSerialize, TlsDeserialize, TlsSize)] +#[derive(Debug, Clone, TlsSerialize, TlsDeserialize, TlsDeserializeBytes, TlsSize)] pub struct SignContent { label: VLBytes, content: VLBytes, @@ -53,7 +62,17 @@ impl From<(&str, &[u8])> for SignContent { /// A public signature key. #[derive( - Eq, PartialEq, Hash, Debug, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Eq, + PartialEq, + Hash, + Debug, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct SignaturePublicKey { pub(in crate::ciphersuite) value: VLBytes, diff --git a/openmls/src/credentials/codec.rs b/openmls/src/credentials/codec.rs index 0cc8df9169..407c382041 100644 --- a/openmls/src/credentials/codec.rs +++ b/openmls/src/credentials/codec.rs @@ -1,8 +1,10 @@ use std::io::Read; +use tls_codec::{Deserialize, DeserializeBytes, Error, Serialize, Size}; + use super::*; -impl tls_codec::Size for Credential { +impl Size for Credential { #[inline] fn tls_serialized_len(&self) -> usize { self.credential_type.tls_serialized_len() @@ -13,32 +15,44 @@ impl tls_codec::Size for Credential { } } -impl tls_codec::Serialize for Credential { - fn tls_serialize(&self, writer: &mut W) -> Result { +impl Serialize for Credential { + fn tls_serialize(&self, writer: &mut W) -> Result { match &self.credential { MlsCredentialType::Basic(basic_credential) => { let written = CredentialType::Basic.tls_serialize(writer)?; basic_credential.tls_serialize(writer).map(|l| l + written) } // TODO #134: implement encoding for X509 certificates - MlsCredentialType::X509(_) => Err(tls_codec::Error::EncodingError( + MlsCredentialType::X509(_) => Err(Error::EncodingError( "X509 certificates are not yet implemented.".to_string(), )), } } } -impl tls_codec::Deserialize for Credential { - fn tls_deserialize(bytes: &mut R) -> Result { +impl Deserialize for Credential { + fn tls_deserialize(bytes: &mut R) -> Result { let val = u16::tls_deserialize(bytes)?; let credential_type = CredentialType::from(val); match credential_type { CredentialType::Basic => Ok(Credential::from(MlsCredentialType::Basic( BasicCredential::tls_deserialize(bytes)?, ))), - _ => Err(tls_codec::Error::DecodingError(format!( + _ => Err(Error::DecodingError(format!( "{credential_type:?} can not be deserialized." ))), } } } + +impl DeserializeBytes for Credential { + fn tls_deserialize_bytes(bytes: &[u8]) -> Result<(Self, &[u8]), Error> + where + Self: Sized, + { + let mut bytes_ref = bytes; + let credential = Credential::tls_deserialize(&mut bytes_ref)?; + let remainder = &bytes[credential.tls_serialized_len()..]; + Ok((credential, remainder)) + } +} diff --git a/openmls/src/credentials/mod.rs b/openmls/src/credentials/mod.rs index 25f7fdd4f2..18919788f2 100644 --- a/openmls/src/credentials/mod.rs +++ b/openmls/src/credentials/mod.rs @@ -25,7 +25,10 @@ use std::io::{Read, Write}; use serde::{Deserialize, Serialize}; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize, VLBytes}; +use tls_codec::{ + Deserialize as TlsDeserializeTrait, DeserializeBytes, Error, Serialize as TlsSerializeTrait, + Size, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, VLBytes, +}; // Private mod codec; @@ -80,14 +83,14 @@ pub enum CredentialType { Unknown(u16), } -impl tls_codec::Size for CredentialType { +impl Size for CredentialType { fn tls_serialized_len(&self) -> usize { 2 } } -impl tls_codec::Deserialize for CredentialType { - fn tls_deserialize(bytes: &mut R) -> Result +impl TlsDeserializeTrait for CredentialType { + fn tls_deserialize(bytes: &mut R) -> Result where Self: Sized, { @@ -98,14 +101,26 @@ impl tls_codec::Deserialize for CredentialType { } } -impl tls_codec::Serialize for CredentialType { - fn tls_serialize(&self, writer: &mut W) -> Result { +impl TlsSerializeTrait for CredentialType { + fn tls_serialize(&self, writer: &mut W) -> Result { writer.write_all(&u16::from(*self).to_be_bytes())?; Ok(2) } } +impl DeserializeBytes for CredentialType { + fn tls_deserialize_bytes(bytes: &[u8]) -> Result<(Self, &[u8]), Error> + where + Self: Sized, + { + let mut bytes_ref = bytes; + let credential_type = CredentialType::tls_deserialize(&mut bytes_ref)?; + let remainder = &bytes[credential_type.tls_serialized_len()..]; + Ok((credential_type, remainder)) + } +} + impl From for CredentialType { fn from(value: u16) -> Self { match value { @@ -237,7 +252,16 @@ impl From for Credential { /// OpenMLS provides an implementation of signature keys for convenience in the /// `openmls_basic_credential` crate. #[derive( - Debug, Clone, PartialEq, Eq, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + Clone, + PartialEq, + Eq, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct BasicCredential { identity: VLBytes, diff --git a/openmls/src/extensions/application_id_extension.rs b/openmls/src/extensions/application_id_extension.rs index 5cd911e015..b90309e7a3 100644 --- a/openmls/src/extensions/application_id_extension.rs +++ b/openmls/src/extensions/application_id_extension.rs @@ -1,4 +1,4 @@ -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize, VLBytes}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, VLBytes}; use super::{Deserialize, Serialize}; @@ -8,7 +8,16 @@ use super::{Deserialize, Serialize}; /// The application id extension allows applications to add an explicit, /// application-defined identifier to a KeyPackage. #[derive( - PartialEq, Eq, Clone, Debug, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + PartialEq, + Eq, + Clone, + Debug, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct ApplicationIdExtension { key_id: VLBytes, diff --git a/openmls/src/extensions/codec.rs b/openmls/src/extensions/codec.rs index 936ee63fa2..58c8eb594a 100644 --- a/openmls/src/extensions/codec.rs +++ b/openmls/src/extensions/codec.rs @@ -1,6 +1,6 @@ use std::io::{Read, Write}; -use tls_codec::{Deserialize, Serialize, Size, VLBytes}; +use tls_codec::{Deserialize, DeserializeBytes, Serialize, Size, VLBytes}; use crate::extensions::{ ApplicationIdExtension, Extension, ExtensionType, ExternalPubExtension, @@ -124,3 +124,15 @@ impl Deserialize for Extension { }) } } + +impl DeserializeBytes for Extension { + fn tls_deserialize_bytes(bytes: &[u8]) -> Result<(Self, &[u8]), tls_codec::Error> + where + Self: Sized, + { + let mut bytes_ref = bytes; + let extension = Extension::tls_deserialize(&mut bytes_ref)?; + let remainder = &bytes[extension.tls_serialized_len()..]; + Ok((extension, remainder)) + } +} diff --git a/openmls/src/extensions/external_pub_extension.rs b/openmls/src/extensions/external_pub_extension.rs index f32e88617b..1ea37e7030 100644 --- a/openmls/src/extensions/external_pub_extension.rs +++ b/openmls/src/extensions/external_pub_extension.rs @@ -1,4 +1,4 @@ -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; use super::{Deserialize, Serialize}; use crate::ciphersuite::HpkePublicKey; @@ -10,7 +10,16 @@ use crate::ciphersuite::HpkePublicKey; /// } ExternalPub; /// ``` #[derive( - PartialEq, Eq, Clone, Debug, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + PartialEq, + Eq, + Clone, + Debug, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct ExternalPubExtension { external_pub: HpkePublicKey, diff --git a/openmls/src/extensions/external_sender_extension.rs b/openmls/src/extensions/external_sender_extension.rs index c45dd56b64..2e34bad47e 100644 --- a/openmls/src/extensions/external_sender_extension.rs +++ b/openmls/src/extensions/external_sender_extension.rs @@ -1,5 +1,5 @@ use serde::{Deserialize, Serialize}; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; use crate::{ciphersuite::SignaturePublicKey, credentials::Credential}; @@ -13,7 +13,16 @@ use crate::{ciphersuite::SignaturePublicKey, credentials::Credential}; /// } ExternalSender; /// ``` #[derive( - Clone, PartialEq, Eq, Debug, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Clone, + PartialEq, + Eq, + Debug, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct ExternalSender { signature_key: SignaturePublicKey, @@ -47,7 +56,17 @@ impl ExternalSender { pub type ExternalSendersExtension = Vec; /// Identifies an external sender in the `ExternalSendersExtension`. #[derive( - Debug, PartialEq, Eq, Copy, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + PartialEq, + Eq, + Copy, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct SenderExtensionIndex(u32); diff --git a/openmls/src/extensions/last_resort.rs b/openmls/src/extensions/last_resort.rs index 895b5a4b41..9ea0e2b57a 100644 --- a/openmls/src/extensions/last_resort.rs +++ b/openmls/src/extensions/last_resort.rs @@ -1,4 +1,4 @@ -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; use super::{Deserialize, Serialize}; @@ -15,6 +15,7 @@ use super::{Deserialize, Serialize}; Deserialize, TlsSerialize, TlsDeserialize, + TlsDeserializeBytes, TlsSize, Default, )] diff --git a/openmls/src/extensions/mod.rs b/openmls/src/extensions/mod.rs index 46d3932cc2..9c2554d015 100644 --- a/openmls/src/extensions/mod.rs +++ b/openmls/src/extensions/mod.rs @@ -48,6 +48,10 @@ pub use external_sender_extension::{ pub use last_resort::LastResortExtension; pub use ratchet_tree_extension::RatchetTreeExtension; pub use required_capabilities::RequiredCapabilitiesExtension; +use tls_codec::{ + Deserialize as TlsDeserializeTrait, DeserializeBytes, Error, Serialize as TlsSerializeTrait, + Size, TlsSize, +}; #[cfg(test)] mod test_extensions; @@ -97,14 +101,14 @@ pub enum ExtensionType { Unknown(u16), } -impl tls_codec::Size for ExtensionType { +impl Size for ExtensionType { fn tls_serialized_len(&self) -> usize { 2 } } -impl tls_codec::Deserialize for ExtensionType { - fn tls_deserialize(bytes: &mut R) -> Result +impl TlsDeserializeTrait for ExtensionType { + fn tls_deserialize(bytes: &mut R) -> Result where Self: Sized, { @@ -115,8 +119,20 @@ impl tls_codec::Deserialize for ExtensionType { } } -impl tls_codec::Serialize for ExtensionType { - fn tls_serialize(&self, writer: &mut W) -> Result { +impl DeserializeBytes for ExtensionType { + fn tls_deserialize_bytes(bytes: &[u8]) -> Result<(Self, &[u8]), Error> + where + Self: Sized, + { + let mut bytes_ref = bytes; + let extension_type = ExtensionType::tls_deserialize(&mut bytes_ref)?; + let remainder = &bytes[extension_type.tls_serialized_len()..]; + Ok((extension_type, remainder)) + } +} + +impl TlsSerializeTrait for ExtensionType { + fn tls_serialize(&self, writer: &mut W) -> Result { writer.write_all(&u16::from(*self).to_be_bytes())?; Ok(2) @@ -209,25 +225,37 @@ pub enum Extension { pub struct UnknownExtension(pub Vec); /// A list of extensions with unique extension types. -#[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, Deserialize, tls_codec::TlsSize)] +#[derive(Default, Debug, Clone, PartialEq, Eq, Serialize, Deserialize, TlsSize)] pub struct Extensions { unique: Vec, } -impl tls_codec::Serialize for Extensions { - fn tls_serialize(&self, writer: &mut W) -> Result { +impl TlsSerializeTrait for Extensions { + fn tls_serialize(&self, writer: &mut W) -> Result { self.unique.tls_serialize(writer) } } -impl tls_codec::Deserialize for Extensions { - fn tls_deserialize(bytes: &mut R) -> Result +impl TlsDeserializeTrait for Extensions { + fn tls_deserialize(bytes: &mut R) -> Result where Self: Sized, { let candidate: Vec = Vec::tls_deserialize(bytes)?; Extensions::try_from(candidate) - .map_err(|_| tls_codec::Error::DecodingError("Found duplicate extensions".into())) + .map_err(|_| Error::DecodingError("Found duplicate extensions".into())) + } +} + +impl DeserializeBytes for Extensions { + fn tls_deserialize_bytes(bytes: &[u8]) -> Result<(Self, &[u8]), Error> + where + Self: Sized, + { + let mut bytes_ref = bytes; + let extensions = Extensions::tls_deserialize(&mut bytes_ref)?; + let remainder = &bytes[extensions.tls_serialized_len()..]; + Ok((extensions, remainder)) } } diff --git a/openmls/src/extensions/ratchet_tree_extension.rs b/openmls/src/extensions/ratchet_tree_extension.rs index 10135d1ee3..7df70b302c 100644 --- a/openmls/src/extensions/ratchet_tree_extension.rs +++ b/openmls/src/extensions/ratchet_tree_extension.rs @@ -1,4 +1,4 @@ -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; use super::{Deserialize, Serialize}; use crate::treesync::{RatchetTree, RatchetTreeIn}; @@ -13,7 +13,16 @@ use crate::treesync::{RatchetTree, RatchetTreeIn}; /// optional ratchet_tree; /// ``` #[derive( - PartialEq, Eq, Clone, Debug, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + PartialEq, + Eq, + Clone, + Debug, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct RatchetTreeExtension { ratchet_tree: RatchetTreeIn, diff --git a/openmls/src/extensions/required_capabilities.rs b/openmls/src/extensions/required_capabilities.rs index 00d128f072..d00081beaf 100644 --- a/openmls/src/extensions/required_capabilities.rs +++ b/openmls/src/extensions/required_capabilities.rs @@ -1,4 +1,4 @@ -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; use crate::{credentials::CredentialType, messages::proposals::ProposalType}; @@ -37,6 +37,7 @@ use super::{Deserialize, ExtensionError, ExtensionType, Serialize}; Deserialize, TlsSerialize, TlsDeserialize, + TlsDeserializeBytes, TlsSize, )] pub struct RequiredCapabilitiesExtension { diff --git a/openmls/src/framing/message_in.rs b/openmls/src/framing/message_in.rs index c67c1c3cf8..caced3c3a4 100644 --- a/openmls/src/framing/message_in.rs +++ b/openmls/src/framing/message_in.rs @@ -68,7 +68,7 @@ pub struct MlsMessageIn { /// } /// } MLSMessage; /// ``` -#[derive(Debug, PartialEq, Clone, TlsDeserialize, TlsSize)] +#[derive(Debug, PartialEq, Clone, TlsDeserialize, TlsDeserializeBytes, TlsSize)] #[cfg_attr(feature = "test-utils", derive(TlsSerialize))] #[repr(u16)] pub enum MlsMessageInBody { diff --git a/openmls/src/framing/mls_content_in.rs b/openmls/src/framing/mls_content_in.rs index 74d6e2a0cf..7fe86ae897 100644 --- a/openmls/src/framing/mls_content_in.rs +++ b/openmls/src/framing/mls_content_in.rs @@ -22,7 +22,7 @@ use openmls_traits::{crypto::OpenMlsCrypto, types::Ciphersuite}; use serde::{Deserialize, Serialize}; use tls_codec::{ Deserialize as TlsDeserializeTrait, Serialize as TlsSerializeTrait, Size, TlsDeserialize, - TlsSerialize, TlsSize, VLBytes, + TlsDeserializeBytes, TlsSerialize, TlsSize, VLBytes, }; /// ```c @@ -37,7 +37,15 @@ use tls_codec::{ /// } FramedContent; /// ``` #[derive( - Debug, PartialEq, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub(crate) struct FramedContentIn { pub(super) group_id: GroupId, @@ -91,7 +99,15 @@ impl From for FramedContentIn { /// } FramedContent; /// ``` #[derive( - Debug, PartialEq, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] #[repr(u8)] pub(crate) enum FramedContentBodyIn { diff --git a/openmls/src/framing/mod.rs b/openmls/src/framing/mod.rs index 17910d381f..41fa1feb0f 100644 --- a/openmls/src/framing/mod.rs +++ b/openmls/src/framing/mod.rs @@ -109,7 +109,17 @@ pub(crate) mod test_framing; /// | 0x0005 | mls_key_package | Y | RFC XXXX | /// | 0xf000 - 0xffff | Reserved for Private Use | N/A | RFC XXXX | #[derive( - PartialEq, Eq, Clone, Copy, Debug, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + PartialEq, + Eq, + Clone, + Copy, + Debug, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] #[repr(u16)] pub enum WireFormat { @@ -160,7 +170,17 @@ impl<'a> FramingParameters<'a> { /// } ContentType; /// ``` #[derive( - PartialEq, Eq, Clone, Copy, Debug, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + PartialEq, + Eq, + Clone, + Copy, + Debug, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] #[repr(u8)] pub enum ContentType { diff --git a/openmls/src/framing/private_message_in.rs b/openmls/src/framing/private_message_in.rs index 0724b0cb63..0bb9cb788a 100644 --- a/openmls/src/framing/private_message_in.rs +++ b/openmls/src/framing/private_message_in.rs @@ -1,6 +1,8 @@ use openmls_traits::crypto::OpenMlsCrypto; use openmls_traits::types::Ciphersuite; -use tls_codec::{Deserialize, Serialize, TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{ + Deserialize, Serialize, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, +}; use super::{ codec::deserialize_ciphertext_content, mls_auth_content::FramedContentAuthData, @@ -31,7 +33,9 @@ use super::*; /// opaque ciphertext; /// } PrivateMessage; /// ``` -#[derive(Debug, PartialEq, Eq, Clone, TlsSerialize, TlsSize, TlsDeserialize)] +#[derive( + Debug, PartialEq, Eq, Clone, TlsSerialize, TlsSize, TlsDeserialize, TlsDeserializeBytes, +)] pub struct PrivateMessageIn { group_id: GroupId, epoch: GroupEpoch, diff --git a/openmls/src/framing/public_message.rs b/openmls/src/framing/public_message.rs index 7d16d8163a..143833593f 100644 --- a/openmls/src/framing/public_message.rs +++ b/openmls/src/framing/public_message.rs @@ -6,7 +6,9 @@ use std::io::Write; use openmls_traits::{crypto::OpenMlsCrypto, types::Ciphersuite}; -use tls_codec::{Serialize as TlsSerializeTrait, TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{ + Serialize as TlsSerializeTrait, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, +}; use super::{ mls_auth_content::{AuthenticatedContent, FramedContentAuthData}, @@ -17,7 +19,15 @@ use crate::{error::LibraryError, versions::ProtocolVersion}; /// Wrapper around a `Mac` used for type safety. #[derive( - Debug, PartialEq, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub(crate) struct MembershipTag(pub(crate) Mac); diff --git a/openmls/src/framing/public_message_in.rs b/openmls/src/framing/public_message_in.rs index b26c6b3f51..e7a6d9715e 100644 --- a/openmls/src/framing/public_message_in.rs +++ b/openmls/src/framing/public_message_in.rs @@ -218,7 +218,7 @@ impl<'a> TryFrom<&'a PublicMessageIn> for InterimTranscriptHashInput<'a> { } impl TlsDeserializeTrait for PublicMessageIn { - fn tls_deserialize(bytes: &mut R) -> Result { + fn tls_deserialize(bytes: &mut R) -> Result { let content = FramedContentIn::tls_deserialize(bytes)?; let auth = FramedContentAuthData::deserialize(bytes, content.body.content_type())?; let membership_tag = if content.sender.is_member() { @@ -231,6 +231,18 @@ impl TlsDeserializeTrait for PublicMessageIn { } } +impl DeserializeBytes for PublicMessageIn { + fn tls_deserialize_bytes(bytes: &[u8]) -> Result<(Self, &[u8]), Error> + where + Self: Sized, + { + let mut bytes_ref = bytes; + let message = PublicMessageIn::tls_deserialize(&mut bytes_ref)?; + let remainder = &bytes[message.tls_serialized_len()..]; + Ok((message, remainder)) + } +} + impl Size for PublicMessageIn { #[inline] fn tls_serialized_len(&self) -> usize { diff --git a/openmls/src/framing/sender.rs b/openmls/src/framing/sender.rs index 693577a0f0..fdfb288cd1 100644 --- a/openmls/src/framing/sender.rs +++ b/openmls/src/framing/sender.rs @@ -3,7 +3,7 @@ use crate::{binary_tree::array_representation::LeafNodeIndex, extensions::SenderExtensionIndex}; use super::*; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; /// All possible sender types according to the MLS protocol spec. /// @@ -33,7 +33,16 @@ use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; /// } Sender; /// ``` #[derive( - Debug, PartialEq, Eq, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + PartialEq, + Eq, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] #[repr(u8)] pub enum Sender { @@ -77,7 +86,7 @@ impl Sender { } } -#[derive(Clone, TlsDeserialize, TlsSerialize, TlsSize)] +#[derive(Clone, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize)] #[cfg_attr(test, derive(Debug))] pub(crate) struct MlsSenderData { pub(crate) leaf_index: LeafNodeIndex, @@ -100,7 +109,7 @@ impl MlsSenderData { } } -#[derive(Clone, TlsDeserialize, TlsSerialize, TlsSize)] +#[derive(Clone, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize)] pub(crate) struct MlsSenderDataAad { pub(crate) group_id: GroupId, pub(crate) epoch: GroupEpoch, diff --git a/openmls/src/group/group_context.rs b/openmls/src/group/group_context.rs index 77618b7b9c..8344e07625 100644 --- a/openmls/src/group/group_context.rs +++ b/openmls/src/group/group_context.rs @@ -29,7 +29,16 @@ use crate::{ /// state agreed upon by each member of the group. ///``` #[derive( - Debug, Clone, PartialEq, Eq, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + Clone, + PartialEq, + Eq, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct GroupContext { protocol_version: ProtocolVersion, diff --git a/openmls/src/group/mod.rs b/openmls/src/group/mod.rs index 10be650ff2..8dd6e77dfd 100644 --- a/openmls/src/group/mod.rs +++ b/openmls/src/group/mod.rs @@ -54,6 +54,7 @@ use openmls_traits::random::OpenMlsRand; Deserialize, Serialize, TlsDeserialize, + TlsDeserializeBytes, TlsSerialize, TlsSize, )] @@ -105,6 +106,7 @@ impl GroupId { Deserialize, Serialize, TlsDeserialize, + TlsDeserializeBytes, TlsSerialize, TlsSize, )] diff --git a/openmls/src/key_packages/key_package_in.rs b/openmls/src/key_packages/key_package_in.rs index 712804d947..ab0efad7bb 100644 --- a/openmls/src/key_packages/key_package_in.rs +++ b/openmls/src/key_packages/key_package_in.rs @@ -10,7 +10,9 @@ use crate::{ }; use openmls_traits::{crypto::OpenMlsCrypto, types::Ciphersuite}; use serde::{Deserialize, Serialize}; -use tls_codec::{Serialize as TlsSerializeTrait, TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{ + Serialize as TlsSerializeTrait, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, +}; use super::{ errors::KeyPackageVerifyError, KeyPackage, KeyPackageTbs, SIGNATURE_KEY_PACKAGE_LABEL, @@ -70,7 +72,15 @@ mod private_mod { /// } KeyPackageTBS; /// ``` #[derive( - Debug, Clone, PartialEq, TlsSize, TlsSerialize, TlsDeserialize, Serialize, Deserialize, + Debug, + Clone, + PartialEq, + TlsSize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + Serialize, + Deserialize, )] struct KeyPackageTbsIn { protocol_version: ProtocolVersion, @@ -82,7 +92,15 @@ struct KeyPackageTbsIn { /// The key package struct. #[derive( - Debug, PartialEq, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct KeyPackageIn { payload: KeyPackageTbsIn, diff --git a/openmls/src/key_packages/lifetime.rs b/openmls/src/key_packages/lifetime.rs index 9da1f46573..9db89f1dd5 100644 --- a/openmls/src/key_packages/lifetime.rs +++ b/openmls/src/key_packages/lifetime.rs @@ -1,7 +1,7 @@ use std::time::{SystemTime, UNIX_EPOCH}; use serde::{Deserialize, Serialize}; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; /// This value is used as the default lifetime if no default lifetime is configured. /// The value is in seconds and amounts to 3 * 28 Days, i.e. about 3 months. @@ -35,7 +35,17 @@ const MAX_LEAF_NODE_LIFETIME_RANGE_SECONDS: u64 = /// } Lifetime; /// ``` #[derive( - PartialEq, Eq, Copy, Clone, Debug, TlsSerialize, TlsSize, TlsDeserialize, Serialize, Deserialize, + PartialEq, + Eq, + Copy, + Clone, + Debug, + TlsSerialize, + TlsSize, + TlsDeserialize, + TlsDeserializeBytes, + Serialize, + Deserialize, )] pub struct Lifetime { not_before: u64, diff --git a/openmls/src/messages/group_info.rs b/openmls/src/messages/group_info.rs index 98558fa0f3..98dbf7fec1 100644 --- a/openmls/src/messages/group_info.rs +++ b/openmls/src/messages/group_info.rs @@ -4,7 +4,9 @@ use openmls_traits::crypto::OpenMlsCrypto; use openmls_traits::types::Ciphersuite; use serde::{Deserialize as SerdeDeserialize, Serialize as SerdeSerialize}; use thiserror::Error; -use tls_codec::{Deserialize, Serialize, TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{ + Deserialize, Serialize, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, +}; use crate::{ binary_tree::LeafNodeIndex, @@ -24,7 +26,7 @@ const SIGNATURE_GROUP_INFO_LABEL: &str = "GroupInfoTBS"; /// `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)] +#[derive(Debug, PartialEq, Clone, TlsDeserialize, TlsDeserializeBytes, TlsSize)] #[cfg_attr(any(test, feature = "test-utils"), derive(TlsSerialize))] pub struct VerifiableGroupInfo { payload: GroupInfoTBS, @@ -208,7 +210,15 @@ impl From for GroupContext { /// } GroupInfoTBS; /// ``` #[derive( - Debug, PartialEq, Clone, TlsDeserialize, TlsSerialize, TlsSize, SerdeSerialize, SerdeDeserialize, + Debug, + PartialEq, + Clone, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, + SerdeSerialize, + SerdeDeserialize, )] pub(crate) struct GroupInfoTBS { group_context: GroupContext, diff --git a/openmls/src/messages/mod.rs b/openmls/src/messages/mod.rs index fc00d2533b..f2d5263d9e 100644 --- a/openmls/src/messages/mod.rs +++ b/openmls/src/messages/mod.rs @@ -56,7 +56,9 @@ use self::{proposals::*, proposals_in::ProposalOrRefIn}; /// opaque encrypted_group_info; /// } Welcome; /// ``` -#[derive(Clone, Debug, Eq, PartialEq, TlsDeserialize, TlsSerialize, TlsSize)] +#[derive( + Clone, Debug, Eq, PartialEq, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, +)] pub struct Welcome { cipher_suite: Ciphersuite, secrets: Vec, @@ -103,7 +105,9 @@ impl Welcome { /// EncryptedGroupSecrets /// /// This is part of a [`Welcome`] message. It can be used to correlate the correct secrets with each new member. -#[derive(Clone, Debug, Eq, PartialEq, TlsDeserialize, TlsSerialize, TlsSize)] +#[derive( + Clone, Debug, Eq, PartialEq, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, +)] pub struct EncryptedGroupSecrets { /// Key package reference of the new member new_member: KeyPackageRef, @@ -169,7 +173,15 @@ impl Commit { } #[derive( - Debug, PartialEq, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Debug, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub(crate) struct CommitIn { proposals: Vec, @@ -265,7 +277,15 @@ impl From for CommitIn { /// Confirmation tag field of PublicMessage. For type safety this is a wrapper /// around a `Mac`. #[derive( - Debug, PartialEq, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Debug, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub struct ConfirmationTag(pub(crate) Mac); @@ -278,7 +298,9 @@ pub struct ConfirmationTag(pub(crate) Mac); /// opaque path_secret<1..255>; /// } PathSecret; /// ``` -#[derive(Debug, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize)] +#[derive( + Debug, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsDeserializeBytes, TlsSize, +)] #[cfg_attr(any(feature = "test-utils", test), derive(PartialEq, Clone))] pub(crate) struct PathSecret { pub(crate) path_secret: Secret, @@ -382,7 +404,7 @@ pub(crate) enum PathSecretError { /// PreSharedKeyID psks; /// } GroupSecrets; /// ``` -#[derive(Debug, TlsDeserialize, TlsSize)] +#[derive(Debug, TlsDeserialize, TlsDeserializeBytes, TlsSize)] pub(crate) struct GroupSecrets { pub(crate) joiner_secret: JoinerSecret, pub(crate) path_secret: Option, diff --git a/openmls/src/messages/proposals.rs b/openmls/src/messages/proposals.rs index 9b758f366d..1390e6c3db 100644 --- a/openmls/src/messages/proposals.rs +++ b/openmls/src/messages/proposals.rs @@ -10,7 +10,10 @@ use std::io::{Read, Write}; use openmls_traits::{crypto::OpenMlsCrypto, types::Ciphersuite}; use serde::{Deserialize, Serialize}; use thiserror::Error; -use tls_codec::{Serialize as TlsSerializeTrait, TlsDeserialize, TlsSerialize, TlsSize, VLBytes}; +use tls_codec::{ + Deserialize as TlsDeserializeTrait, DeserializeBytes, Error, Serialize as TlsSerializeTrait, + Size, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, VLBytes, +}; use crate::{ binary_tree::array_representation::LeafNodeIndex, @@ -82,14 +85,14 @@ pub enum ProposalType { Unknown(u16), } -impl tls_codec::Size for ProposalType { +impl Size for ProposalType { fn tls_serialized_len(&self) -> usize { 2 } } -impl tls_codec::Deserialize for ProposalType { - fn tls_deserialize(bytes: &mut R) -> Result +impl TlsDeserializeTrait for ProposalType { + fn tls_deserialize(bytes: &mut R) -> Result where Self: Sized, { @@ -100,14 +103,26 @@ impl tls_codec::Deserialize for ProposalType { } } -impl tls_codec::Serialize for ProposalType { - fn tls_serialize(&self, writer: &mut W) -> Result { +impl TlsSerializeTrait for ProposalType { + fn tls_serialize(&self, writer: &mut W) -> Result { writer.write_all(&u16::from(*self).to_be_bytes())?; Ok(2) } } +impl DeserializeBytes for ProposalType { + fn tls_deserialize_bytes(bytes: &[u8]) -> Result<(Self, &[u8]), Error> + where + Self: Sized, + { + let mut bytes_ref = bytes; + let proposal_type = ProposalType::tls_deserialize(&mut bytes_ref)?; + let remainder = &bytes[proposal_type.tls_serialized_len()..]; + Ok((proposal_type, remainder)) + } +} + impl ProposalType { /// Check whether a proposal type is supported or not. Returns `true` /// if a proposal is supported and `false` otherwise. @@ -291,7 +306,16 @@ impl UpdateProposal { /// } Remove; /// ``` #[derive( - Debug, PartialEq, Eq, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Debug, + PartialEq, + Eq, + Clone, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub struct RemoveProposal { pub(crate) removed: LeafNodeIndex, @@ -316,7 +340,16 @@ impl RemoveProposal { /// } PreSharedKey; /// ``` #[derive( - Debug, PartialEq, Eq, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Debug, + PartialEq, + Eq, + Clone, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub struct PreSharedKeyProposal { psk: PreSharedKeyId, @@ -352,7 +385,16 @@ impl PreSharedKeyProposal { /// } ReInit; /// ``` #[derive( - Debug, PartialEq, Eq, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Debug, + PartialEq, + Eq, + Clone, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub struct ReInitProposal { pub(crate) group_id: GroupId, @@ -373,7 +415,16 @@ pub struct ReInitProposal { /// } ExternalInit; /// ``` #[derive( - Debug, PartialEq, Eq, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Debug, + PartialEq, + Eq, + Clone, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub struct ExternalInitProposal { kem_output: VLBytes, @@ -400,7 +451,15 @@ impl From> for ExternalInitProposal { /// /// This is not yet supported. #[derive( - Debug, PartialEq, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Debug, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub struct AppAckProposal { received_ranges: Vec, @@ -418,7 +477,16 @@ pub struct AppAckProposal { /// } GroupContextExtensions; /// ``` #[derive( - Debug, PartialEq, Eq, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Debug, + PartialEq, + Eq, + Clone, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub struct GroupContextExtensionProposal { extensions: Extensions, @@ -455,7 +523,16 @@ impl GroupContextExtensionProposal { /// We only implement the values (1, 2), other values are not valid /// and will yield `ProposalOrRefTypeError::UnknownValue` when decoded. #[derive( - PartialEq, Clone, Copy, Debug, TlsSerialize, TlsDeserialize, TlsSize, Serialize, Deserialize, + PartialEq, + Clone, + Copy, + Debug, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, + Serialize, + Deserialize, )] #[repr(u8)] pub enum ProposalOrRefType { @@ -537,7 +614,15 @@ impl ProposalRef { /// } MessageRange; /// ``` #[derive( - Debug, PartialEq, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Debug, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub(crate) struct MessageRange { sender: KeyPackageRef, diff --git a/openmls/src/messages/proposals_in.rs b/openmls/src/messages/proposals_in.rs index b5f41308c5..2c5b23579e 100644 --- a/openmls/src/messages/proposals_in.rs +++ b/openmls/src/messages/proposals_in.rs @@ -17,7 +17,7 @@ use crate::{ use openmls_traits::{crypto::OpenMlsCrypto, types::Ciphersuite}; use serde::{Deserialize, Serialize}; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; use super::proposals::{ AddProposal, AppAckProposal, ExternalInitProposal, GroupContextExtensionProposal, @@ -46,7 +46,15 @@ use super::proposals::{ /// ``` #[allow(clippy::large_enum_variant)] #[derive( - Debug, PartialEq, Clone, Serialize, Deserialize, TlsSize, TlsSerialize, TlsDeserialize, + Debug, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsSize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, )] #[allow(missing_docs)] #[repr(u16)] @@ -132,7 +140,15 @@ impl ProposalIn { /// } Add; /// ``` #[derive( - Debug, PartialEq, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct AddProposalIn { key_package: KeyPackageIn, @@ -171,7 +187,16 @@ impl AddProposalIn { /// } Update; /// ``` #[derive( - Debug, PartialEq, Eq, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Debug, + PartialEq, + Eq, + Clone, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub struct UpdateProposalIn { leaf_node: LeafNodeIn, @@ -214,7 +239,15 @@ impl UpdateProposalIn { /// Type of Proposal, either by value or by reference. #[derive( - Debug, PartialEq, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] #[repr(u8)] #[allow(missing_docs)] diff --git a/openmls/src/schedule/mod.rs b/openmls/src/schedule/mod.rs index 4d42b47589..c969c0c529 100644 --- a/openmls/src/schedule/mod.rs +++ b/openmls/src/schedule/mod.rs @@ -123,7 +123,7 @@ use openmls_traits::{crypto::OpenMlsCrypto, types::*}; use serde::{Deserialize, Serialize}; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; use crate::{ binary_tree::array_representation::{LeafNodeIndex, TreeSize}, @@ -345,7 +345,7 @@ impl InitSecret { } } -#[derive(Debug, TlsDeserialize, TlsSerialize, TlsSize)] +#[derive(Debug, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize)] pub(crate) struct JoinerSecret { secret: Secret, } diff --git a/openmls/src/schedule/psk.rs b/openmls/src/schedule/psk.rs index 66b430954d..16671eb202 100644 --- a/openmls/src/schedule/psk.rs +++ b/openmls/src/schedule/psk.rs @@ -38,6 +38,7 @@ use crate::{ Deserialize, Serialize, TlsDeserialize, + TlsDeserializeBytes, TlsSerialize, TlsSize, )] @@ -67,6 +68,7 @@ pub enum ResumptionPskUsage { Deserialize, Serialize, TlsDeserialize, + TlsDeserializeBytes, TlsSerialize, TlsSize, )] @@ -90,7 +92,7 @@ impl ExternalPsk { /// Contains the secret part of the PSK as well as the /// public part that is used as a marker for injection into the key schedule. -#[derive(Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize)] +#[derive(Serialize, Deserialize, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize)] pub(crate) struct PskBundle { secret: Secret, } @@ -110,6 +112,7 @@ impl MlsEntity for PskBundle { Deserialize, Serialize, TlsDeserialize, + TlsDeserializeBytes, TlsSerialize, TlsSize, )] @@ -156,6 +159,7 @@ impl ResumptionPsk { Deserialize, Serialize, TlsDeserialize, + TlsDeserializeBytes, TlsSerialize, TlsSize, )] @@ -216,6 +220,7 @@ pub enum PskType { Deserialize, Serialize, TlsDeserialize, + TlsDeserializeBytes, TlsSerialize, TlsSize, )] diff --git a/openmls/src/treesync/mod.rs b/openmls/src/treesync/mod.rs index 994d074bad..3ac426a981 100644 --- a/openmls/src/treesync/mod.rs +++ b/openmls/src/treesync/mod.rs @@ -35,7 +35,7 @@ use openmls_traits::{ }; use serde::{Deserialize, Serialize}; use thiserror::Error; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; use self::{ diff::{StagedTreeSyncDiff, TreeSyncDiff}, @@ -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, + PartialEq, + Eq, + Clone, + Debug, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub struct RatchetTreeIn(Vec>); diff --git a/openmls/src/treesync/node.rs b/openmls/src/treesync/node.rs index 9bfb352dab..8dab53795b 100644 --- a/openmls/src/treesync/node.rs +++ b/openmls/src/treesync/node.rs @@ -2,7 +2,7 @@ //! variants of the enum are `LeafNode` and [`ParentNode`], both of which are //! defined in the respective [`leaf_node`] and [`parent_node`] submodules. use serde::{Deserialize, Serialize}; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; use self::{leaf_node::LeafNodeIn, parent_node::ParentNode}; @@ -37,7 +37,16 @@ pub enum Node { } #[derive( - Debug, PartialEq, Eq, Clone, Serialize, Deserialize, TlsSize, TlsDeserialize, TlsSerialize, + Debug, + PartialEq, + Eq, + Clone, + Serialize, + Deserialize, + TlsSize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, )] #[repr(u8)] pub enum NodeIn { diff --git a/openmls/src/treesync/node/codec.rs b/openmls/src/treesync/node/codec.rs index 2f0be4f516..0d4dc9acec 100644 --- a/openmls/src/treesync/node/codec.rs +++ b/openmls/src/treesync/node/codec.rs @@ -1,9 +1,14 @@ -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{ + Deserialize, DeserializeBytes, Error, Size, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, + TlsSize, +}; use super::parent_node::{UnmergedLeaves, UnmergedLeavesError}; /// Node type. Can be either `Leaf` or `Parent`. -#[derive(PartialEq, Clone, Copy, Debug, TlsSerialize, TlsDeserialize, TlsSize)] +#[derive( + PartialEq, Clone, Copy, Debug, TlsSerialize, TlsDeserialize, TlsDeserializeBytes, TlsSize, +)] #[repr(u8)] enum MlsNodeType { Leaf = 1, @@ -12,16 +17,28 @@ enum MlsNodeType { // Implementations for `ParentNode` -impl tls_codec::Deserialize for UnmergedLeaves { - fn tls_deserialize(bytes: &mut R) -> Result +impl Deserialize for UnmergedLeaves { + fn tls_deserialize(bytes: &mut R) -> Result where Self: Sized, { let list = Vec::tls_deserialize(bytes)?; Self::try_from(list).map_err(|e| match e { UnmergedLeavesError::NotSorted => { - tls_codec::Error::DecodingError("Unmerged leaves not sorted".into()) + Error::DecodingError("Unmerged leaves not sorted".into()) } }) } } + +impl DeserializeBytes for UnmergedLeaves { + fn tls_deserialize_bytes(bytes: &[u8]) -> Result<(Self, &[u8]), Error> + where + Self: Sized, + { + let mut bytes_ref = bytes; + let unmerged_leaves = UnmergedLeaves::tls_deserialize(&mut bytes_ref)?; + let remainder = &bytes[unmerged_leaves.tls_serialized_len()..]; + Ok((unmerged_leaves, remainder)) + } +} diff --git a/openmls/src/treesync/node/encryption_keys.rs b/openmls/src/treesync/node/encryption_keys.rs index 61447ef0a6..26c903e420 100644 --- a/openmls/src/treesync/node/encryption_keys.rs +++ b/openmls/src/treesync/node/encryption_keys.rs @@ -7,7 +7,7 @@ use openmls_traits::{ OpenMlsProvider, }; use serde::{Deserialize, Serialize}; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize, VLBytes}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, VLBytes}; use crate::{ ciphersuite::{hpke, HpkePrivateKey, HpkePublicKey, Secret}, @@ -19,7 +19,17 @@ use crate::{ /// [`EncryptionKey`] contains an HPKE public key that allows the encryption of /// path secrets in MLS commits. #[derive( - Debug, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, PartialEq, Eq, Hash, + Debug, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, + PartialEq, + Eq, + Hash, )] pub struct EncryptionKey { key: HpkePublicKey, @@ -66,7 +76,9 @@ impl EncryptionKey { } } -#[derive(Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize)] +#[derive( + Clone, Serialize, Deserialize, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, +)] #[cfg_attr(test, derive(PartialEq, Eq))] pub(crate) struct EncryptionPrivateKey { key: HpkePrivateKey, @@ -137,7 +149,9 @@ impl From for EncryptionKey { } } -#[derive(Debug, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize)] +#[derive( + Debug, Clone, Serialize, Deserialize, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, +)] #[cfg_attr(test, derive(PartialEq, Eq))] pub(crate) struct EncryptionKeyPair { public_key: EncryptionKey, diff --git a/openmls/src/treesync/node/leaf_node.rs b/openmls/src/treesync/node/leaf_node.rs index 362e39f5ae..5277a566a1 100644 --- a/openmls/src/treesync/node/leaf_node.rs +++ b/openmls/src/treesync/node/leaf_node.rs @@ -1,7 +1,10 @@ //! This module contains the [`LeafNode`] struct and its implementation. use openmls_traits::{signatures::Signer, types::Ciphersuite, OpenMlsProvider}; use serde::{Deserialize, Serialize}; -use tls_codec::{Serialize as TlsSerializeTrait, TlsDeserialize, TlsSerialize, TlsSize, VLBytes}; +use tls_codec::{ + Serialize as TlsSerializeTrait, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, + VLBytes, +}; #[cfg(test)] use openmls_traits::key_store::OpenMlsKeyStore; @@ -509,7 +512,16 @@ impl LeafNode { /// } LeafNode; /// ``` #[derive( - Debug, Clone, PartialEq, Eq, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + Clone, + PartialEq, + Eq, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] struct LeafNodePayload { encryption_key: EncryptionKey, @@ -521,7 +533,16 @@ struct LeafNodePayload { } #[derive( - Debug, Clone, PartialEq, Eq, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + Clone, + PartialEq, + Eq, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] #[repr(u8)] pub enum LeafNodeSource { @@ -658,7 +679,16 @@ impl TreePosition { const LEAF_NODE_SIGNATURE_LABEL: &str = "LeafNodeTBS"; #[derive( - Debug, Clone, PartialEq, Eq, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + Clone, + PartialEq, + Eq, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct LeafNodeIn { payload: LeafNodePayload, diff --git a/openmls/src/treesync/node/leaf_node/capabilities.rs b/openmls/src/treesync/node/leaf_node/capabilities.rs index 6f909d5651..e07ff477c8 100644 --- a/openmls/src/treesync/node/leaf_node/capabilities.rs +++ b/openmls/src/treesync/node/leaf_node/capabilities.rs @@ -1,6 +1,6 @@ use openmls_traits::types::{Ciphersuite, VerifiableCiphersuite}; use serde::{Deserialize, Serialize}; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; #[cfg(doc)] use super::LeafNode; @@ -24,7 +24,16 @@ use crate::{ /// } Capabilities; /// ``` #[derive( - Debug, Clone, PartialEq, Eq, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + Clone, + PartialEq, + Eq, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct Capabilities { pub(super) versions: Vec, diff --git a/openmls/src/treesync/node/parent_node.rs b/openmls/src/treesync/node/parent_node.rs index 379297693f..b1f7ec053b 100644 --- a/openmls/src/treesync/node/parent_node.rs +++ b/openmls/src/treesync/node/parent_node.rs @@ -6,7 +6,7 @@ use openmls_traits::types::{Ciphersuite, HpkeCiphertext}; use rayon::prelude::*; use serde::{Deserialize, Serialize}; use thiserror::*; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize, VLBytes}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, VLBytes}; use super::encryption_keys::{EncryptionKey, EncryptionKeyPair}; use crate::{ @@ -22,7 +22,16 @@ use crate::{ /// parent hash and unmerged leaves. Additionally, it may contain the private /// key corresponding to the public key. #[derive( - Debug, Eq, PartialEq, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + Eq, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct ParentNode { pub(super) encryption_key: EncryptionKey, diff --git a/openmls/src/treesync/treekem.rs b/openmls/src/treesync/treekem.rs index 42fa7dd25b..ffdd089542 100644 --- a/openmls/src/treesync/treekem.rs +++ b/openmls/src/treesync/treekem.rs @@ -12,7 +12,7 @@ use openmls_traits::{ }; use rayon::prelude::*; use serde::{Deserialize, Serialize}; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; use super::{ diff::TreeSyncDiff, @@ -240,7 +240,16 @@ pub(crate) struct DecryptPathParams<'a> { /// } UpdatePathNode; /// ``` #[derive( - Debug, Eq, PartialEq, Clone, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, + Debug, + Eq, + PartialEq, + Clone, + Serialize, + Deserialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSerialize, + TlsSize, )] pub struct UpdatePathNode { pub(super) public_key: EncryptionKey, @@ -363,7 +372,16 @@ impl UpdatePath { } #[derive( - Debug, PartialEq, Eq, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + PartialEq, + Eq, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct UpdatePathIn { leaf_node: LeafNodeIn, diff --git a/openmls/src/versions.rs b/openmls/src/versions.rs index 5a33a7b5ad..0f33a72190 100644 --- a/openmls/src/versions.rs +++ b/openmls/src/versions.rs @@ -5,7 +5,7 @@ use serde::{Deserialize, Serialize}; use std::fmt; use thiserror::Error; -use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; +use tls_codec::{TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize}; // Public types @@ -29,6 +29,7 @@ use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; Serialize, Deserialize, TlsDeserialize, + TlsDeserializeBytes, TlsSerialize, TlsSize, )] diff --git a/traits/src/types.rs b/traits/src/types.rs index 76af00a1b9..ab64b7e48c 100644 --- a/traits/src/types.rs +++ b/traits/src/types.rs @@ -5,7 +5,9 @@ use std::ops::Deref; use serde::{Deserialize, Serialize}; -use tls_codec::{SecretVLBytes, TlsDeserialize, TlsSerialize, TlsSize, VLBytes}; +use tls_codec::{ + SecretVLBytes, TlsDeserialize, TlsDeserializeBytes, TlsSerialize, TlsSize, VLBytes, +}; use crate::key_store::{MlsEntity, MlsEntityId}; @@ -86,6 +88,7 @@ impl HashType { Deserialize, TlsSerialize, TlsDeserialize, + TlsDeserializeBytes, TlsSize, )] #[repr(u16)] @@ -224,7 +227,16 @@ pub enum HpkeAeadType { /// } HPKECiphertext; /// ``` #[derive( - Debug, PartialEq, Eq, Clone, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + PartialEq, + Eq, + Clone, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct HpkeCiphertext { pub kem_output: VLBytes, @@ -233,7 +245,14 @@ pub struct HpkeCiphertext { /// A simple type for HPKE private keys. #[derive( - Debug, Clone, serde::Serialize, serde::Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Debug, + Clone, + serde::Serialize, + serde::Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] #[cfg_attr(feature = "test-utils", derive(PartialEq, Eq))] #[serde(transparent)] @@ -292,7 +311,17 @@ impl From> for ExporterSecret { /// /// Used to accept unknown values, e.g., in `Capabilities`. #[derive( - Clone, Copy, Debug, PartialEq, Eq, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, + Clone, + Copy, + Debug, + PartialEq, + Eq, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsDeserializeBytes, + TlsSize, )] pub struct VerifiableCiphersuite(u16); @@ -323,6 +352,7 @@ impl From for VerifiableCiphersuite { Serialize, Deserialize, TlsDeserialize, + TlsDeserializeBytes, TlsSerialize, TlsSize, )] From 281e59fb3086d4f1ad87234f4df3acaaf92927b2 Mon Sep 17 00:00:00 2001 From: Jan Winkelmann <146678+keks@users.noreply.github.com> Date: Wed, 10 Jan 2024 11:36:04 +0100 Subject: [PATCH 08/13] Add credentials_to_verify method to StagedCommit (#1470) A StagedCommit can carry credentials in several different places, all of which have to be verified. To make this easier on the caller, we add the `credentials_to_verify` method on StagedCommit, which returns an iterator over all the credentials in a StagedCommit that need to be verified. Also we add infrastructure in test_framework for letting the test code decide which credentials would pass validation, by passing in a closure that takes a &Credential and returns a bool. Co-authored-by: Jan Winkelmann (keks) --- openmls/src/group/core_group/staged_commit.rs | 53 ++++- openmls/src/group/mls_group/test_mls_group.rs | 186 +++++++++++++++++- openmls/src/messages/proposals.rs | 5 + .../src/test_utils/test_framework/client.rs | 11 +- openmls/src/test_utils/test_framework/mod.rs | 81 ++++++-- openmls/tests/test_decryption_key_index.rs | 9 +- openmls/tests/test_interop_scenarios.rs | 74 +++++-- openmls/tests/test_managed_api.rs | 17 +- 8 files changed, 387 insertions(+), 49 deletions(-) diff --git a/openmls/src/group/core_group/staged_commit.rs b/openmls/src/group/core_group/staged_commit.rs index 3573dc384b..8fccbf42b2 100644 --- a/openmls/src/group/core_group/staged_commit.rs +++ b/openmls/src/group/core_group/staged_commit.rs @@ -435,6 +435,11 @@ impl StagedCommit { self.staged_proposal_queue.psk_proposals() } + /// Returns an iterator over all [`QueuedProposal`]s. + pub(crate) fn queued_proposals(&self) -> impl Iterator { + self.staged_proposal_queue.queued_proposals() + } + /// Returns the leaf node of the (optional) update path. pub fn update_path_leaf_node(&self) -> Option<&LeafNode> { match self.state { @@ -445,9 +450,51 @@ impl StagedCommit { } } - /// Returns an iterator over all [`QueuedProposal`]s. - pub(crate) fn queued_proposals(&self) -> impl Iterator { - self.staged_proposal_queue.queued_proposals() + /// Returns the credentials that the caller needs to verify are valid. + pub fn credentials_to_verify(&self) -> impl Iterator { + let update_path_leaf_node_cred = if let Some(node) = self.update_path_leaf_node() { + vec![node.credential()] + } else { + vec![] + }; + + update_path_leaf_node_cred + .into_iter() + .chain( + self.queued_proposals() + .flat_map(|proposal: &QueuedProposal| match proposal.proposal() { + Proposal::Update(update_proposal) => { + vec![update_proposal.leaf_node().credential()].into_iter() + } + Proposal::Add(add_proposal) => { + vec![add_proposal.key_package().leaf_node().credential()].into_iter() + } + Proposal::GroupContextExtensions(gce_proposal) => gce_proposal + .extensions() + .iter() + .flat_map(|extension| { + match extension { + Extension::ExternalSenders(external_senders) => { + external_senders + .iter() + .map(|external_sender| external_sender.credential()) + .collect() + } + _ => vec![], + } + .into_iter() + }) + // TODO: ideally we wouldn't collect in between here, but the match arms + // have to all return the same type. We solve this by having them all + // be vec::IntoIter, but it would be nice if we just didn't have to + // do this. + // It might be possible to solve this by letting all match arms + // evaluate to a dyn Iterator. + .collect::>() + .into_iter(), + _ => vec![].into_iter(), + }), + ) } /// Returns `true` if the member was removed through a proposal covered by this Commit message diff --git a/openmls/src/group/mls_group/test_mls_group.rs b/openmls/src/group/mls_group/test_mls_group.rs index 71feaa5ac7..46dd2eea94 100644 --- a/openmls/src/group/mls_group/test_mls_group.rs +++ b/openmls/src/group/mls_group/test_mls_group.rs @@ -10,7 +10,8 @@ use crate::{ key_packages::*, messages::proposals::*, test_utils::test_framework::{ - errors::ClientError, ActionType::Commit, CodecUse, MlsGroupTestSetup, + errors::ClientError, noop_authentication_service, ActionType::Commit, CodecUse, + MlsGroupTestSetup, }, test_utils::*, }; @@ -251,7 +252,7 @@ fn test_invalid_plaintext(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvi ); // Create a basic group with more than 4 members to create a tree with intermediate nodes. let group_id = setup - .create_random_group(10, ciphersuite) + .create_random_group(10, ciphersuite, noop_authentication_service) .expect("An unexpected error occurred."); let mut groups = setup.groups.write().expect("An unexpected error occurred."); let group = groups @@ -311,7 +312,12 @@ fn test_invalid_plaintext(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvi let error = setup // We're the "no_client" id to prevent the original sender from treating // this message as his own and merging the pending commit. - .distribute_to_members("no_client".as_bytes(), group, &msg_invalid_signature.into()) + .distribute_to_members( + "no_client".as_bytes(), + group, + &msg_invalid_signature.into(), + &noop_authentication_service, + ) .expect_err("No error when distributing message with invalid signature."); assert_eq!( @@ -324,7 +330,12 @@ fn test_invalid_plaintext(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvi let error = setup // We're the "no_client" id to prevent the original sender from treating // this message as his own and merging the pending commit. - .distribute_to_members("no_client".as_bytes(), group, &msg_invalid_sender.into()) + .distribute_to_members( + "no_client".as_bytes(), + group, + &msg_invalid_sender.into(), + &noop_authentication_service, + ) .expect_err("No error when distributing message with invalid signature."); assert_eq!( @@ -335,6 +346,171 @@ fn test_invalid_plaintext(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvi ); } +#[apply(ciphersuites_and_providers)] +fn test_verify_staged_commit_credentials( + ciphersuite: Ciphersuite, + provider: &impl OpenMlsProvider, +) { + let group_id = GroupId::from_slice(b"Test Group"); + + let (alice_credential_with_key, _alice_kpb, alice_signer, _alice_pk) = + setup_client("Alice", ciphersuite, provider); + let (_bob_credential, bob_kpb, _bob_signer, _bob_pk) = + setup_client("Bob", ciphersuite, provider); + + // Define the MlsGroup configuration + let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + + // === Alice creates a group === + let mut alice_group = MlsGroup::new_with_group_id( + provider, + &alice_signer, + &mls_group_config, + group_id, + alice_credential_with_key.clone(), + ) + .expect("An unexpected error occurred."); + + // There should be no pending commit after group creation. + assert!(alice_group.pending_commit().is_none()); + + let bob_key_package = bob_kpb.key_package(); + + // === Alice adds Bob to the group === + let (proposal, _) = alice_group + .propose_add_member(provider, &alice_signer, bob_key_package) + .expect("error creating self-update proposal"); + + let alice_processed_message = alice_group + .process_message(provider, proposal.into_protocol_message().unwrap()) + .expect("Could not process messages."); + assert!(alice_group.pending_commit().is_none()); + + if let ProcessedMessageContent::ProposalMessage(staged_proposal) = + alice_processed_message.into_content() + { + alice_group.store_pending_proposal(*staged_proposal); + } else { + unreachable!("Expected a StagedCommit."); + } + + let (_msg, welcome_option, _group_info) = alice_group + .self_update(provider, &alice_signer) + .expect("error creating self-update commit"); + + // Merging the pending commit should clear the pending commit and we should + // end up in the same state as bob. + alice_group + .merge_pending_commit(provider) + .expect("error merging pending commit"); + assert!(alice_group.pending_commit().is_none()); + assert!(alice_group.pending_proposals().next().is_none()); + + let mut bob_group = MlsGroup::new_from_welcome( + provider, + &mls_group_config, + welcome_option + .expect("no welcome after commit") + .into_welcome() + .expect("Unexpected message type."), + Some(alice_group.export_ratchet_tree().into()), + ) + .expect("error creating group from welcome"); + + assert_eq!( + bob_group.export_ratchet_tree(), + alice_group.export_ratchet_tree() + ); + assert_eq!( + bob_group.export_secret(provider.crypto(), "test", &[], ciphersuite.hash_length()), + alice_group.export_secret(provider.crypto(), "test", &[], ciphersuite.hash_length()) + ); + // Bob is added and the state aligns. + + // === Make a new, empty commit and check that the leaf node credentials match === + let (commit_msg, _welcome_option, _group_info) = alice_group + .self_update(provider, &alice_signer) + .expect("error creating self-update commit"); + + // empty commits should only produce a single message + assert!(_welcome_option.is_none()); + assert!(_group_info.is_none()); + + // There should be a pending commit after issuing a self-update commit. + let alice_pending_commit = alice_group + .pending_commit() + .expect("alice should have the self-update as pending commit"); + + // The commit contains only Alice's credentials, in the update path leaf node. + for cred in alice_pending_commit.credentials_to_verify() { + assert_eq!(cred, &alice_credential_with_key.credential); + } + + // great, they match! now commit + alice_group + .merge_pending_commit(provider) + .expect("alice failed to merge the pending empty commit"); + + // === transfer message to bob and process it === + + // this requires serializing and deserializing + let mut wire_msg = Vec::::new(); + commit_msg + .tls_serialize(&mut wire_msg) + .expect("alice failed serializing her message"); + let msg_in = MlsMessageIn::tls_deserialize(&mut &wire_msg[..]) + .expect("bob failed deserializing alice's message"); + + // neither party should have pending proposals + assert!(alice_group.pending_proposals().next().is_none()); + assert!(bob_group.pending_proposals().next().is_none()); + + // neither should have pending commits after merging and before processing + assert!(bob_group.pending_commit().is_none()); + assert!(alice_group.pending_commit().is_none()); + + // further process the deserialized message + let processed_message = bob_group + .process_message(provider, msg_in) + .expect("bob failed processing alice's message"); + + // the processed message must be a staged commit message + assert!(matches!( + processed_message.content(), + ProcessedMessageContent::StagedCommitMessage(_) + )); + + if let ProcessedMessageContent::StagedCommitMessage(staged_commit) = + processed_message.into_content() + { + // The commit contains only Alice's credentials, in the update path leaf node. + for cred in staged_commit.credentials_to_verify() { + assert_eq!(cred, &alice_credential_with_key.credential); + } + + // bob merges alice's message + bob_group + .merge_staged_commit(provider, *staged_commit) + .expect("bob failed merging alice's empty commit (staged)"); + + // finally, the state should match + assert_eq!( + bob_group.export_ratchet_tree(), + alice_group.export_ratchet_tree() + ); + assert_eq!( + bob_group.export_secret(provider.crypto(), "test", &[], ciphersuite.hash_length()), + alice_group.export_secret(provider.crypto(), "test", &[], ciphersuite.hash_length()) + ); + } else { + unreachable!() + } + + // neither should have pending commits after merging and processing + assert!(bob_group.pending_commit().is_none()); + assert!(alice_group.pending_commit().is_none()); +} + #[apply(ciphersuites_and_providers)] fn test_commit_with_update_path_leaf_node( ciphersuite: Ciphersuite, @@ -420,7 +596,7 @@ fn test_commit_with_update_path_leaf_node( // === Make a new, empty commit and check that the leaf node credentials match === - println!("\nCreating commit with add proposal."); + println!("\nCreating self-update commit."); let (commit_msg, _welcome_option, _group_info) = alice_group .self_update(provider, &alice_signer) .expect("error creating self-update commit"); diff --git a/openmls/src/messages/proposals.rs b/openmls/src/messages/proposals.rs index 1390e6c3db..6bf5813c55 100644 --- a/openmls/src/messages/proposals.rs +++ b/openmls/src/messages/proposals.rs @@ -498,6 +498,11 @@ impl GroupContextExtensionProposal { pub(crate) fn new(extensions: Extensions) -> Self { Self { extensions } } + + /// Get the extensions of the proposal + pub(crate) fn extensions(&self) -> &Extensions { + &self.extensions + } } // Crate-only types diff --git a/openmls/src/test_utils/test_framework/client.rs b/openmls/src/test_utils/test_framework/client.rs index 3229a9dcad..bcbe185b82 100644 --- a/openmls/src/test_utils/test_framework/client.rs +++ b/openmls/src/test_utils/test_framework/client.rs @@ -134,10 +134,11 @@ impl Client { /// Have the client process the given messages. Returns an error if an error /// occurs during message processing or if no group exists for one of the /// messages. - pub fn receive_messages_for_group( + pub fn receive_messages_for_group bool>( &self, message: &ProtocolMessage, sender_id: &[u8], + authentication_service: &AS, ) -> Result<(), ClientError> { let mut group_states = self.groups.write().expect("An unexpected error occurred."); let group_id = message.group_id(); @@ -163,6 +164,14 @@ impl Client { group_state.store_pending_proposal(*staged_proposal); } ProcessedMessageContent::StagedCommitMessage(staged_commit) => { + for credential in staged_commit.credentials_to_verify() { + if !authentication_service(credential) { + println!( + "authentication service callback denied credential {credential:?}" + ); + return Err(ClientError::NoMatchingCredential); + } + } group_state.merge_staged_commit(&self.crypto, *staged_commit)?; } } diff --git a/openmls/src/test_utils/test_framework/mod.rs b/openmls/src/test_utils/test_framework/mod.rs index 6f9f230220..c4d0b6f47c 100644 --- a/openmls/src/test_utils/test_framework/mod.rs +++ b/openmls/src/test_utils/test_framework/mod.rs @@ -278,13 +278,14 @@ impl MlsGroupTestSetup { /// Distribute a set of messages sent by the sender with identity /// `sender_id` to their intended recipients in group `Group`. This function /// also verifies that all members of that group agree on the public tree. - pub fn distribute_to_members( + pub fn distribute_to_members bool>( &self, // We need the sender to know a group member that we know can not have // been removed from the group. sender_id: &[u8], group: &mut Group, message: &MlsMessageIn, + authentication_service: &AS, ) -> Result<(), ClientError> { // Test serialization if mandated by config let message: ProtocolMessage = match self.use_codec { @@ -302,7 +303,7 @@ impl MlsGroupTestSetup { // Distribute message to all members, except to the sender in the case of application messages let results: Result, _> = group .members - .par_iter() + .iter() .filter_map(|(_index, member_id)| { if message.content_type() == ContentType::Application && member_id == sender_id { None @@ -316,7 +317,7 @@ impl MlsGroupTestSetup { .expect("An unexpected error occurred.") .read() .expect("An unexpected error occurred."); - member.receive_messages_for_group(&message, sender_id) + member.receive_messages_for_group(&message, sender_id, &authentication_service) }) .collect(); @@ -352,7 +353,11 @@ impl MlsGroupTestSetup { /// each group member encrypt an application message and delivers all of /// these messages to all other members. This function panics if any of the /// above tests fail. - pub fn check_group_states(&self, group: &mut Group) { + pub fn check_group_states bool>( + &self, + group: &mut Group, + authentication_service: AS, + ) { let clients = self.clients.read().expect("An unexpected error occurred."); let messages = group .members @@ -393,7 +398,7 @@ impl MlsGroupTestSetup { .collect::, MlsMessageOut)>>(); drop(clients); for (sender_id, message) in messages { - self.distribute_to_members(&sender_id, group, &message.into()) + self.distribute_to_members(&sender_id, group, &message.into(), &authentication_service) .expect("Error sending messages to clients while checking group states."); } } @@ -478,10 +483,11 @@ impl MlsGroupTestSetup { } /// Create a random group of size `group_size` and return the `GroupId` - pub fn create_random_group( + pub fn create_random_group bool>( &self, target_group_size: usize, ciphersuite: Ciphersuite, + authentication_service: AS, ) -> Result { // Create the initial group. let group_id = self.create_group(ciphersuite)?; @@ -501,7 +507,13 @@ impl MlsGroupTestSetup { // Add between 1 and 5 new members. let number_of_adds = ((OsRng.next_u32() as usize) % 5 % new_members.len()) + 1; let members_to_add = new_members.drain(0..number_of_adds).collect(); - self.add_clients(ActionType::Commit, group, &adder_id.1, members_to_add)?; + self.add_clients( + ActionType::Commit, + group, + &adder_id.1, + members_to_add, + &authentication_service, + )?; } Ok(group_id) } @@ -509,12 +521,13 @@ impl MlsGroupTestSetup { /// Have the client with identity `client_id` either propose or commit /// (depending on `action_type`) a self update in group `group`. Will throw /// an error if the client is not actually a member of group `group`. - pub fn self_update( + pub fn self_update bool>( &self, action_type: ActionType, group: &mut Group, client_id: &[u8], leaf_node: Option, + authentication_service: &AS, ) -> Result<(), SetupError> { let clients = self.clients.read().expect("An unexpected error occurred."); let client = clients @@ -524,7 +537,12 @@ impl MlsGroupTestSetup { .expect("An unexpected error occurred."); let (messages, welcome_option, _) = client.self_update(action_type, &group.group_id, leaf_node)?; - self.distribute_to_members(&client.identity, group, &messages.into())?; + self.distribute_to_members( + &client.identity, + group, + &messages.into(), + authentication_service, + )?; if let Some(welcome) = welcome_option { self.deliver_welcome(welcome, group)?; } @@ -537,12 +555,13 @@ impl MlsGroupTestSetup { /// * the `adder` is not part of the group /// * the `addee` is already part of the group /// * the `addee` doesn't support the group's ciphersuite. - pub fn add_clients( + pub fn add_clients bool>( &self, action_type: ActionType, group: &mut Group, adder_id: &[u8], addees: Vec>, + authentication_service: &AS, ) -> Result<(), SetupError> { let clients = self.clients.read().expect("An unexpected error occurred."); let adder = clients @@ -570,7 +589,7 @@ impl MlsGroupTestSetup { let (messages, welcome_option, _) = adder.add_members(action_type, &group.group_id, &key_packages)?; for message in messages { - self.distribute_to_members(adder_id, group, &message.into())?; + self.distribute_to_members(adder_id, group, &message.into(), authentication_service)?; } if let Some(welcome) = welcome_option { self.deliver_welcome(welcome, group)?; @@ -582,12 +601,13 @@ impl MlsGroupTestSetup { /// removal the `target_members` from the Group `group`. If the `remover` or /// one of the `target_members` is not part of the group, it returns an /// error. - pub fn remove_clients( + pub fn remove_clients bool>( &self, action_type: ActionType, group: &mut Group, remover_id: &[u8], target_members: &[LeafNodeIndex], + authentication_service: AS, ) -> Result<(), SetupError> { let clients = self.clients.read().expect("An unexpected error occurred."); let remover = clients @@ -598,7 +618,12 @@ impl MlsGroupTestSetup { let (messages, welcome_option, _) = remover.remove_members(action_type, &group.group_id, target_members)?; for message in messages { - self.distribute_to_members(remover_id, group, &message.into())?; + self.distribute_to_members( + remover_id, + group, + &message.into(), + &authentication_service, + )?; } if let Some(welcome) = welcome_option { self.deliver_welcome(welcome, group)?; @@ -609,7 +634,11 @@ impl MlsGroupTestSetup { /// This function picks a random member of group `group` and has them /// perform a random commit- or proposal action. TODO #133: This won't work /// yet due to the missing proposal validation. - pub fn perform_random_operation(&self, group: &mut Group) -> Result<(), SetupError> { + pub fn perform_random_operation bool>( + &self, + group: &mut Group, + authentication_service: &AS, + ) -> Result<(), SetupError> { // Who's going to do it? let member_id = group.random_group_member(); println!("Member performing the operation: {member_id:?}"); @@ -626,7 +655,13 @@ impl MlsGroupTestSetup { match operation_type { 0 => { println!("Performing a self-update with action type: {action_type:?}"); - self.self_update(action_type, group, &member_id.1, None)?; + self.self_update( + action_type, + group, + &member_id.1, + None, + authentication_service, + )?; } 1 => { // If it's a single-member group, don't remove anyone. @@ -683,6 +718,7 @@ impl MlsGroupTestSetup { group, &member_id.1, &target_member_leaf_indices, + authentication_service, )? }; } @@ -701,7 +737,13 @@ impl MlsGroupTestSetup { .expect("An unexpected error occurred."); println!("{action_type:?}: Adding new clients: {new_member_ids:?}"); // Have the adder add them to the group. - self.add_clients(action_type, group, &member_id.1, new_member_ids)?; + self.add_clients( + action_type, + group, + &member_id.1, + new_member_ids, + authentication_service, + )?; } } _ => return Err(SetupError::Unknown), @@ -709,3 +751,10 @@ impl MlsGroupTestSetup { Ok(()) } } + +// This function signature is used in the callback for credential validation. +// This particular function just accepts any credential. If you want to validate +// credentials in your test, don't use this. +pub fn noop_authentication_service(_cred: &Credential) -> bool { + true +} diff --git a/openmls/tests/test_decryption_key_index.rs b/openmls/tests/test_decryption_key_index.rs index 9d6de463ae..691fa49205 100644 --- a/openmls/tests/test_decryption_key_index.rs +++ b/openmls/tests/test_decryption_key_index.rs @@ -2,7 +2,7 @@ use openmls::{ prelude::*, test_utils::{ - test_framework::{ActionType, CodecUse, MlsGroupTestSetup}, + test_framework::{noop_authentication_service, ActionType, CodecUse, MlsGroupTestSetup}, *, }, *, @@ -20,9 +20,10 @@ fn decryption_key_index_computation(ciphersuite: Ciphersuite) { number_of_clients, CodecUse::StructMessages, ); + // Create a basic group with more than 4 members to create a tree with intermediate nodes. let group_id = setup - .create_random_group(10, ciphersuite) + .create_random_group(10, ciphersuite, noop_authentication_service) .expect("An unexpected error occurred."); let mut groups = setup.groups.write().expect("An unexpected error occurred."); let group = groups @@ -45,6 +46,7 @@ fn decryption_key_index_computation(ciphersuite: Ciphersuite) { group, remover_id, &[LeafNodeIndex::new(2)], + noop_authentication_service, ) .expect("An unexpected error occurred."); @@ -64,11 +66,12 @@ fn decryption_key_index_computation(ciphersuite: Ciphersuite) { group, remover_id, &[LeafNodeIndex::new(3)], + noop_authentication_service, ) .expect("An unexpected error occurred."); // Since the decryption failure doesn't cause a panic, but only an error // message in the callback, we also have to check that the group states // match for all group members. - setup.check_group_states(group); + setup.check_group_states(group, noop_authentication_service); } diff --git a/openmls/tests/test_interop_scenarios.rs b/openmls/tests/test_interop_scenarios.rs index d5cef989a5..2602de215c 100644 --- a/openmls/tests/test_interop_scenarios.rs +++ b/openmls/tests/test_interop_scenarios.rs @@ -1,6 +1,8 @@ use openmls::{ prelude::*, - test_utils::test_framework::{ActionType, CodecUse, MlsGroupTestSetup}, + test_utils::test_framework::{ + noop_authentication_service, ActionType, CodecUse, MlsGroupTestSetup, + }, test_utils::*, *, }; @@ -45,11 +47,17 @@ fn one_to_one_join(ciphersuite: Ciphersuite) { .expect("An unexpected error occurred."); setup - .add_clients(ActionType::Commit, group, &alice_id, bob_id) + .add_clients( + ActionType::Commit, + group, + &alice_id, + bob_id, + &noop_authentication_service, + ) .expect("Error adding Bob"); // Check that group members agree on a group state. - setup.check_group_states(group); + setup.check_group_states(group, noop_authentication_service); } // # 3-party join @@ -92,7 +100,13 @@ fn three_party_join(ciphersuite: Ciphersuite) { // Create the add commit and deliver the welcome. setup - .add_clients(ActionType::Commit, group, &alice_id, bob_id) + .add_clients( + ActionType::Commit, + group, + &alice_id, + bob_id, + &noop_authentication_service, + ) .expect("Error adding Bob"); // A vector including Charly's id. @@ -101,11 +115,17 @@ fn three_party_join(ciphersuite: Ciphersuite) { .expect("An unexpected error occurred."); setup - .add_clients(ActionType::Commit, group, &alice_id, charly_id) + .add_clients( + ActionType::Commit, + group, + &alice_id, + charly_id, + &noop_authentication_service, + ) .expect("Error adding Charly"); // Check that group members agree on a group state. - setup.check_group_states(group); + setup.check_group_states(group, noop_authentication_service); } // # Multiple joins at once @@ -147,11 +167,17 @@ fn multiple_joins(ciphersuite: Ciphersuite) { // Create the add commit and deliver the welcome. setup - .add_clients(ActionType::Commit, group, &alice_id, bob_charly_id) + .add_clients( + ActionType::Commit, + group, + &alice_id, + bob_charly_id, + &noop_authentication_service, + ) .expect("Error adding Bob and Charly"); // Check that group members agree on a group state. - setup.check_group_states(group); + setup.check_group_states(group, noop_authentication_service); } // TODO #192, #286, #289: The external join test should go here. @@ -175,7 +201,7 @@ fn update(ciphersuite: Ciphersuite) { // Create a group with two members. Includes the process of adding Bob. let group_id = setup - .create_random_group(2, ciphersuite) + .create_random_group(2, ciphersuite, noop_authentication_service) .expect("Error while trying to create group."); let mut groups = setup.groups.write().expect("An unexpected error occurred."); let group = groups @@ -189,11 +215,17 @@ fn update(ciphersuite: Ciphersuite) { // Let Alice create an update with a self-generated KeyPackageBundle. setup - .self_update(ActionType::Commit, group, &alice_id, None) + .self_update( + ActionType::Commit, + group, + &alice_id, + None, + &noop_authentication_service, + ) .expect("Error self-updating."); // Check that group members agree on a group state. - setup.check_group_states(group); + setup.check_group_states(group, noop_authentication_service); } // # Remove @@ -217,7 +249,7 @@ fn remove(ciphersuite: Ciphersuite) { // Create a group with two members. Includes the process of adding Bob. let group_id = setup - .create_random_group(2, ciphersuite) + .create_random_group(2, ciphersuite, noop_authentication_service) .expect("Error while trying to create group."); let mut groups = setup.groups.write().expect("An unexpected error occurred."); let group = groups @@ -240,11 +272,12 @@ fn remove(ciphersuite: Ciphersuite) { group, &alice_id, &[LeafNodeIndex::new(bob_index)], + noop_authentication_service, ) .expect("Error removing Bob from the group."); // Check that group members agree on a group state. - setup.check_group_states(group); + setup.check_group_states(group, noop_authentication_service); } // TODO #141, #284: The external PSK, resumption and re-init tests should go @@ -274,7 +307,7 @@ fn large_group_lifecycle(ciphersuite: Ciphersuite) { // a one-person group and then adding new members in bunches of up to 5, // each bunch by a random group member. let group_id = setup - .create_random_group(number_of_clients, ciphersuite) + .create_random_group(number_of_clients, ciphersuite, noop_authentication_service) .expect("Error while trying to create group."); let mut groups = setup.groups.write().expect("An unexpected error occurred."); let group = groups @@ -287,7 +320,13 @@ fn large_group_lifecycle(ciphersuite: Ciphersuite) { // delivered to each member. for (_, member_id) in &group_members { setup - .self_update(ActionType::Commit, group, member_id, None) + .self_update( + ActionType::Commit, + group, + member_id, + None, + &noop_authentication_service, + ) .expect("Error while updating group.") } @@ -304,12 +343,13 @@ fn large_group_lifecycle(ciphersuite: Ciphersuite) { group, &remover_id.1, &[LeafNodeIndex::new(target_id.0)], + noop_authentication_service, ) .expect("Error while removing group member."); group_members = group.members().collect::)>>(); - setup.check_group_states(group); + setup.check_group_states(group, noop_authentication_service); } // Check that group members agree on a group state. - setup.check_group_states(group); + setup.check_group_states(group, noop_authentication_service); } diff --git a/openmls/tests/test_managed_api.rs b/openmls/tests/test_managed_api.rs index c95b520fda..44c8e81914 100644 --- a/openmls/tests/test_managed_api.rs +++ b/openmls/tests/test_managed_api.rs @@ -1,6 +1,8 @@ use openmls::{ prelude::*, - test_utils::test_framework::{ActionType, CodecUse, MlsGroupTestSetup}, + test_utils::test_framework::{ + noop_authentication_service, ActionType, CodecUse, MlsGroupTestSetup, + }, test_utils::*, *, }; @@ -17,7 +19,7 @@ fn test_mls_group_api(ciphersuite: Ciphersuite) { ); let group_id = setup - .create_random_group(3, ciphersuite) + .create_random_group(3, ciphersuite, noop_authentication_service) .expect("An unexpected error occurred."); let mut groups = setup.groups.write().expect("An unexpected error occurred."); let group = groups @@ -30,7 +32,13 @@ fn test_mls_group_api(ciphersuite: Ciphersuite) { .random_new_members_for_group(group, 2) .expect("An unexpected error occurred."); setup - .add_clients(ActionType::Commit, group, &adder_id, new_members) + .add_clients( + ActionType::Commit, + group, + &adder_id, + new_members, + &noop_authentication_service, + ) .expect("An unexpected error occurred."); // Remove a member @@ -42,9 +50,10 @@ fn test_mls_group_api(ciphersuite: Ciphersuite) { group, &remover_id, &[LeafNodeIndex::new(target_index)], + noop_authentication_service, ) .expect("An unexpected error occurred."); // Check that all group members agree on the same group state. - setup.check_group_states(group); + setup.check_group_states(group, noop_authentication_service); } From 3291f0dac42f87f3ec5a47aee81c9cef9573ae74 Mon Sep 17 00:00:00 2001 From: Jan Winkelmann <146678+keks@users.noreply.github.com> Date: Wed, 10 Jan 2024 11:55:42 +0100 Subject: [PATCH 09/13] Refactor VerifiedStruct and Verifiable (#1472) The current version of the two traits is the way it is because it attempted to prevent the caller from being able to convert to the Type implementing VerifiedStruct without checking the signature. However, the approach that was taken does not work. This commit refactors the two traits and performs the conversion in the same method as the verification. Unfortunately, this means that the verify method has to be defined per impl instead of being able to rely on a default one. There probably is no straightforward way around that, so this simple solution is preferred. Co-authored-by: Konrad Kohbrok --- openmls/src/ciphersuite/signable.rs | 95 +++++++------------ .../ciphersuite/tests/kat_crypto_basics.rs | 21 ++-- openmls/src/framing/mls_auth_content_in.rs | 31 +++--- openmls/src/key_packages/key_package_in.rs | 33 +++---- openmls/src/messages/group_info.rs | 26 ++--- openmls/src/treesync/node/leaf_node.rs | 71 +++++++------- 6 files changed, 129 insertions(+), 148 deletions(-) diff --git a/openmls/src/ciphersuite/signable.rs b/openmls/src/ciphersuite/signable.rs index ce04e2badf..52b885fb43 100644 --- a/openmls/src/ciphersuite/signable.rs +++ b/openmls/src/ciphersuite/signable.rs @@ -53,32 +53,6 @@ pub trait SignedStruct { fn from_payload(payload: T, signature: Signature) -> Self; } -/// This trait must be implemented by all structs that contain a verified -/// self-signature. -pub trait VerifiedStruct { - /// This type is used to prevent users of the trait from bypassing `verify` - /// by simply calling `from_verifiable`. `Seal` should be a dummy type - /// defined in a private module as follows: - /// ``` - /// mod private_mod { - /// pub struct Seal; - /// - /// impl Default for Seal { - /// fn default() -> Self { - /// Seal {} - /// } - /// } - /// } - /// ``` - type SealingType: Default; - - /// Build a verified struct version from the payload struct. This function - /// is only meant to be called by the implementation of the `Verifiable` - /// trait corresponding to this `VerifiedStruct`. - #[doc(hidden)] - fn from_verifiable(verifiable: T, _seal: Self::SealingType) -> Self; -} - /// The `Signable` trait is implemented by all struct that are being signed. /// The implementation has to provide the `unsigned_payload` function. pub trait Signable: Sized { @@ -117,6 +91,10 @@ pub trait Signable: Sized { } } +/// This marker trait must be implemented by all structs that contain a verified +/// self-signature. +pub trait VerifiedStruct {} + /// The verifiable trait must be implemented by any struct that is signed with /// a credential. The actual `verify` method is provided. /// The `unsigned_payload` and `signature` functions have to be implemented for @@ -127,6 +105,10 @@ pub trait Signable: Sized { /// struct implementing them aren't well defined. Not that both traits define an /// `unsigned_payload` function. pub trait Verifiable: Sized { + /// The type used for representing the verified data. Must implement the marker trait + /// [`VerifiedStruct`]. + type VerifiedStruct: VerifiedStruct; + /// Return the unsigned, serialized payload that should be verified. fn unsigned_payload(&self) -> Result, tls_codec::Error>; @@ -137,22 +119,17 @@ pub trait Verifiable: Sized { fn label(&self) -> &str; /// Verifies the payload against the given `credential`. - /// The signature is fetched via the [`Verifiable::signature()`] function and - /// the payload via [`Verifiable::unsigned_payload()`]. + /// Usually this is implemented by first checking that `self.verify_no_out()` + /// does not return an error, and then converting the value into + /// `Self::VerifiedStruct`. /// /// Returns `Ok(Self::VerifiedOutput)` if the signature is valid and /// `CredentialError::InvalidSignature` otherwise. - fn verify( + fn verify( self, crypto: &impl OpenMlsCrypto, pk: &OpenMlsSignaturePublicKey, - ) -> Result - where - T: VerifiedStruct, - { - verify(crypto, &self, pk)?; - Ok(T::from_verifiable(self, T::SealingType::default())) - } + ) -> Result; /// Verifies the payload against the given `credential`. /// The signature is fetched via the [`Verifiable::signature()`] function and @@ -165,32 +142,24 @@ pub trait Verifiable: Sized { crypto: &impl OpenMlsCrypto, pk: &OpenMlsSignaturePublicKey, ) -> Result<(), SignatureError> { - verify(crypto, self, pk) + let payload = self + .unsigned_payload() + .map_err(|_| SignatureError::VerificationError)?; + let sign_content = SignContent::new(self.label(), payload.into()); + let payload = match sign_content.tls_serialize_detached() { + Ok(p) => p, + Err(e) => { + log::error!("Serializing SignContent failed, {:?}", e); + return Err(SignatureError::VerificationError); + } + }; + crypto + .verify_signature( + pk.signature_scheme(), + &payload, + pk.as_slice(), + self.signature().value(), + ) + .map_err(|_| SignatureError::VerificationError) } } - -fn verify( - crypto: &impl OpenMlsCrypto, - verifiable: &impl Verifiable, - pk: &OpenMlsSignaturePublicKey, -) -> Result<(), SignatureError> { - let payload = verifiable - .unsigned_payload() - .map_err(|_| SignatureError::VerificationError)?; - let sign_content = SignContent::new(verifiable.label(), payload.into()); - let payload = match sign_content.tls_serialize_detached() { - Ok(p) => p, - Err(e) => { - log::error!("Serializing SignContent failed, {:?}", e); - return Err(SignatureError::VerificationError); - } - }; - crypto - .verify_signature( - pk.signature_scheme(), - &payload, - pk.as_slice(), - verifiable.signature().value(), - ) - .map_err(|_| SignatureError::VerificationError) -} diff --git a/openmls/src/ciphersuite/tests/kat_crypto_basics.rs b/openmls/src/ciphersuite/tests/kat_crypto_basics.rs index 625e508e33..1fdecc601b 100644 --- a/openmls/src/ciphersuite/tests/kat_crypto_basics.rs +++ b/openmls/src/ciphersuite/tests/kat_crypto_basics.rs @@ -154,6 +154,8 @@ impl SignedStruct for MySignature { } impl Verifiable for ParsedSignWithLabel { + type VerifiedStruct = (); + fn unsigned_payload(&self) -> Result, tls_codec::Error> { Ok(self.content.clone()) } @@ -165,14 +167,19 @@ impl Verifiable for ParsedSignWithLabel { fn label(&self) -> &str { &self.label } + + fn verify( + self, + crypto: &impl openmls_traits::crypto::OpenMlsCrypto, + pk: &crate::ciphersuite::OpenMlsSignaturePublicKey, + ) -> Result { + self.verify_no_out(crypto, pk)?; + Ok(()) + } } // Dummy implementation -impl VerifiedStruct for () { - type SealingType = u8; - - fn from_verifiable(_: ParsedSignWithLabel, _seal: Self::SealingType) -> Self {} -} +impl VerifiedStruct for () {} impl Signable for ParsedSignWithLabel { type SignedOutput = MySignature; @@ -294,7 +301,7 @@ pub fn run_test_vector( // verify signature parsed .clone() - .verify::<()>( + .verify( provider.crypto(), &OpenMlsSignaturePublicKey::new( public.clone().into(), @@ -307,7 +314,7 @@ pub fn run_test_vector( // verify own signature parsed.signature = my_signature.0; parsed - .verify::<()>( + .verify( provider.crypto(), &OpenMlsSignaturePublicKey::new(public.into(), ciphersuite.signature_algorithm()) .unwrap(), diff --git a/openmls/src/framing/mls_auth_content_in.rs b/openmls/src/framing/mls_auth_content_in.rs index c9db02a412..426a2f4bf5 100644 --- a/openmls/src/framing/mls_auth_content_in.rs +++ b/openmls/src/framing/mls_auth_content_in.rs @@ -24,12 +24,6 @@ use crate::{ #[cfg(doc)] use super::{PrivateMessageIn, PublicMessageIn}; -/// Private module to ensure protection of [`AuthenticatedContent`]. -mod private_mod { - #[derive(Default)] - pub(crate) struct Seal; -} - /// 6 Message Framing /// /// ```c @@ -194,6 +188,8 @@ impl VerifiableAuthenticatedContentIn { } impl Verifiable for VerifiableAuthenticatedContentIn { + type VerifiedStruct = AuthenticatedContentIn; + fn unsigned_payload(&self) -> Result, tls_codec::Error> { self.tbs.tls_serialize_detached() } @@ -205,20 +201,23 @@ impl Verifiable for VerifiableAuthenticatedContentIn { fn label(&self) -> &str { "FramedContentTBS" } -} -impl VerifiedStruct for AuthenticatedContentIn { - fn from_verifiable(v: VerifiableAuthenticatedContentIn, _seal: Self::SealingType) -> Self { - AuthenticatedContentIn { - wire_format: v.tbs.wire_format, - content: v.tbs.content, - auth: v.auth, - } + fn verify( + self, + crypto: &impl OpenMlsCrypto, + pk: &OpenMlsSignaturePublicKey, + ) -> Result { + self.verify_no_out(crypto, pk)?; + Ok(AuthenticatedContentIn { + wire_format: self.tbs.wire_format, + content: self.tbs.content, + auth: self.auth, + }) } - - type SealingType = private_mod::Seal; } +impl VerifiedStruct for AuthenticatedContentIn {} + impl SignedStruct for AuthenticatedContentIn { fn from_payload(tbs: FramedContentTbsIn, signature: Signature) -> Self { let auth = FramedContentAuthData { diff --git a/openmls/src/key_packages/key_package_in.rs b/openmls/src/key_packages/key_package_in.rs index ab0efad7bb..0b0d9ad3f7 100644 --- a/openmls/src/key_packages/key_package_in.rs +++ b/openmls/src/key_packages/key_package_in.rs @@ -5,7 +5,7 @@ use crate::{ ciphersuite::{signable::*, *}, credentials::*, extensions::Extensions, - treesync::node::leaf_node::{LeafNode, LeafNodeIn, VerifiableLeafNode}, + treesync::node::leaf_node::{LeafNodeIn, VerifiableLeafNode}, versions::ProtocolVersion, }; use openmls_traits::{crypto::OpenMlsCrypto, types::Ciphersuite}; @@ -31,6 +31,8 @@ impl VerifiableKeyPackage { } impl Verifiable for VerifiableKeyPackage { + type VerifiedStruct = KeyPackage; + fn unsigned_payload(&self) -> Result, tls_codec::Error> { self.payload.tls_serialize_detached() } @@ -42,23 +44,22 @@ impl Verifiable for VerifiableKeyPackage { fn label(&self) -> &str { SIGNATURE_KEY_PACKAGE_LABEL } -} -impl VerifiedStruct for KeyPackage { - type SealingType = private_mod::Seal; - - fn from_verifiable(verifiable: VerifiableKeyPackage, _seal: Self::SealingType) -> Self { - Self { - payload: verifiable.payload, - signature: verifiable.signature, - } + fn verify( + self, + crypto: &impl OpenMlsCrypto, + pk: &OpenMlsSignaturePublicKey, + ) -> Result { + self.verify_no_out(crypto, pk)?; + + Ok(KeyPackage { + payload: self.payload, + signature: self.signature, + }) } } -mod private_mod { - #[derive(Default)] - pub struct Seal; -} +impl VerifiedStruct for KeyPackage {} /// The unsigned payload of a key package. /// @@ -143,7 +144,7 @@ impl KeyPackageIn { let leaf_node = match leaf_node { VerifiableLeafNode::KeyPackage(leaf_node) => leaf_node - .verify::(crypto, signature_key) + .verify(crypto, signature_key) .map_err(|_| KeyPackageVerifyError::InvalidLeafNodeSignature)?, _ => return Err(KeyPackageVerifyError::InvalidLeafNodeSourceType), }; @@ -168,7 +169,7 @@ impl KeyPackageIn { // Verify the KeyPackage signature let key_package = VerifiableKeyPackage::new(key_package_tbs, self.signature) - .verify::(crypto, signature_key) + .verify(crypto, signature_key) .map_err(|_| KeyPackageVerifyError::InvalidSignature)?; // Extension included in the extensions or leaf_node.extensions fields diff --git a/openmls/src/messages/group_info.rs b/openmls/src/messages/group_info.rs index 98dbf7fec1..4aa4d297ad 100644 --- a/openmls/src/messages/group_info.rs +++ b/openmls/src/messages/group_info.rs @@ -272,6 +272,8 @@ impl SignedStruct for GroupInfo { } impl Verifiable for VerifiableGroupInfo { + type VerifiedStruct = GroupInfo; + fn unsigned_payload(&self) -> Result, tls_codec::Error> { self.payload.tls_serialize_detached() } @@ -283,20 +285,18 @@ impl Verifiable for VerifiableGroupInfo { fn label(&self) -> &str { SIGNATURE_GROUP_INFO_LABEL } -} -impl VerifiedStruct for GroupInfo { - type SealingType = private_mod::Seal; - - fn from_verifiable(v: VerifiableGroupInfo, _seal: Self::SealingType) -> Self { - Self { - payload: v.payload, - signature: v.signature, - } + fn verify( + self, + crypto: &impl OpenMlsCrypto, + pk: &crate::ciphersuite::OpenMlsSignaturePublicKey, + ) -> Result { + self.verify_no_out(crypto, pk)?; + Ok(GroupInfo { + payload: self.payload, + signature: self.signature, + }) } } -mod private_mod { - #[derive(Default)] - pub struct Seal; -} +impl VerifiedStruct for GroupInfo {} diff --git a/openmls/src/treesync/node/leaf_node.rs b/openmls/src/treesync/node/leaf_node.rs index 5277a566a1..b50aa7bf67 100644 --- a/openmls/src/treesync/node/leaf_node.rs +++ b/openmls/src/treesync/node/leaf_node.rs @@ -35,12 +35,6 @@ mod codec; pub use capabilities::*; -/// Private module to ensure protection. -mod private_mod { - #[derive(Default)] - pub(crate) struct Seal; -} - pub(crate) struct NewLeafNodeParams { pub(crate) config: CryptoConfig, pub(crate) credential_with_key: CredentialWithKey, @@ -790,6 +784,8 @@ impl VerifiableKeyPackageLeafNode { } impl Verifiable for VerifiableKeyPackageLeafNode { + type VerifiedStruct = LeafNode; + fn unsigned_payload(&self) -> Result, tls_codec::Error> { self.payload.tls_serialize_detached() } @@ -801,19 +797,22 @@ impl Verifiable for VerifiableKeyPackageLeafNode { fn label(&self) -> &str { LEAF_NODE_SIGNATURE_LABEL } -} -impl VerifiedStruct for LeafNode { - fn from_verifiable(verifiable: VerifiableKeyPackageLeafNode, _seal: Self::SealingType) -> Self { - Self { - payload: verifiable.payload, - signature: verifiable.signature, - } + fn verify( + self, + crypto: &impl openmls_traits::crypto::OpenMlsCrypto, + pk: &crate::ciphersuite::OpenMlsSignaturePublicKey, + ) -> Result { + self.verify_no_out(crypto, pk)?; + Ok(LeafNode { + payload: self.payload, + signature: self.signature, + }) } - - type SealingType = private_mod::Seal; } +impl VerifiedStruct for LeafNode {} + #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct VerifiableUpdateLeafNode { payload: LeafNodePayload, @@ -832,6 +831,8 @@ impl VerifiableUpdateLeafNode { } impl Verifiable for VerifiableUpdateLeafNode { + type VerifiedStruct = LeafNode; + fn unsigned_payload(&self) -> Result, tls_codec::Error> { let tree_info_tbs = match &self.tree_position { Some(tree_position) => TreeInfoTbs::Commit(tree_position.clone()), @@ -851,17 +852,18 @@ impl Verifiable for VerifiableUpdateLeafNode { fn label(&self) -> &str { LEAF_NODE_SIGNATURE_LABEL } -} -impl VerifiedStruct for LeafNode { - fn from_verifiable(verifiable: VerifiableUpdateLeafNode, _seal: Self::SealingType) -> Self { - Self { - payload: verifiable.payload, - signature: verifiable.signature, - } + fn verify( + self, + crypto: &impl openmls_traits::crypto::OpenMlsCrypto, + pk: &crate::ciphersuite::OpenMlsSignaturePublicKey, + ) -> Result { + self.verify_no_out(crypto, pk)?; + Ok(LeafNode { + payload: self.payload, + signature: self.signature, + }) } - - type SealingType = private_mod::Seal; } #[derive(Debug, Clone, PartialEq, Eq)] @@ -882,6 +884,8 @@ impl VerifiableCommitLeafNode { } impl Verifiable for VerifiableCommitLeafNode { + type VerifiedStruct = LeafNode; + fn unsigned_payload(&self) -> Result, tls_codec::Error> { let tree_info_tbs = match &self.tree_position { Some(tree_position) => TreeInfoTbs::Commit(tree_position.clone()), @@ -902,17 +906,18 @@ impl Verifiable for VerifiableCommitLeafNode { fn label(&self) -> &str { LEAF_NODE_SIGNATURE_LABEL } -} -impl VerifiedStruct for LeafNode { - fn from_verifiable(verifiable: VerifiableCommitLeafNode, _seal: Self::SealingType) -> Self { - Self { - payload: verifiable.payload, - signature: verifiable.signature, - } + fn verify( + self, + crypto: &impl openmls_traits::crypto::OpenMlsCrypto, + pk: &crate::ciphersuite::OpenMlsSignaturePublicKey, + ) -> Result { + self.verify_no_out(crypto, pk)?; + Ok(LeafNode { + payload: self.payload, + signature: self.signature, + }) } - - type SealingType = private_mod::Seal; } impl Signable for LeafNodeTbs { From 741316ddcf83fe0ba80a78baa7bc74094aab053a Mon Sep 17 00:00:00 2001 From: Konrad Kohbrok Date: Wed, 10 Jan 2024 14:51:15 +0100 Subject: [PATCH 10/13] MlsGroup: Builder, CreateConfig and JoinConfig (#1464) --- CHANGELOG.md | 2 + book/src/forward_secrecy.md | 6 +- book/src/user_manual/create_group.md | 10 +- book/src/user_manual/group_config.md | 22 +- .../user_manual/join_from_external_commit.md | 4 +- book/src/user_manual/join_from_welcome.md | 3 +- cli/src/user.rs | 4 +- delivery-service/ds/src/test.rs | 6 +- interop_client/src/main.rs | 14 +- openmls/src/extensions/test_extensions.rs | 6 +- .../group/core_group/kat_passive_client.rs | 21 +- openmls/src/group/mls_group/builder.rs | 201 +++++++++++++++++ openmls/src/group/mls_group/config.rs | 205 +++++++++++++----- openmls/src/group/mls_group/creation.rs | 85 ++------ openmls/src/group/mls_group/mod.rs | 9 +- openmls/src/group/mls_group/ser.rs | 2 +- openmls/src/group/mls_group/test_mls_group.rs | 120 ++++++++-- openmls/src/group/public_group/tests.rs | 10 +- .../src/group/tests/external_add_proposal.rs | 12 +- .../group/tests/external_remove_proposal.rs | 2 +- .../src/group/tests/test_commit_validation.rs | 8 +- .../tests/test_external_commit_validation.rs | 6 +- .../group/tests/test_framing_validation.rs | 6 +- openmls/src/group/tests/test_past_secrets.rs | 6 +- .../group/tests/test_proposal_validation.rs | 11 +- .../src/group/tests/test_remove_operation.rs | 8 +- .../group/tests/test_wire_format_policy.rs | 5 +- openmls/src/lib.rs | 4 +- openmls/src/messages/tests/test_welcome.rs | 10 +- .../src/test_utils/test_framework/client.rs | 10 +- openmls/src/test_utils/test_framework/mod.rs | 18 +- openmls/src/treesync/tests_and_kats/tests.rs | 8 +- openmls/tests/book_code.rs | 55 ++++- openmls/tests/test_decryption_key_index.rs | 4 +- openmls/tests/test_external_commit.rs | 18 +- openmls/tests/test_interop_scenarios.rs | 12 +- openmls/tests/test_managed_api.rs | 4 +- openmls/tests/test_mls_group.rs | 32 +-- 38 files changed, 676 insertions(+), 293 deletions(-) create mode 100644 openmls/src/group/mls_group/builder.rs diff --git a/CHANGELOG.md b/CHANGELOG.md index a534c0200e..16712e2efc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed +- [#1464](https://github.com/openmls/openmls/pull/1464): Add builder pattern for `MlsGroup`; split `MlsGroupJoinConfig` into `MlsGroupCreateConfig` and `MlsGroupJoinConfig` + ## 0.5.0 (XXXX-XX-XX) This release has many breaking API changes, a few of them are listed below: diff --git a/book/src/forward_secrecy.md b/book/src/forward_secrecy.md index 1e11e1db09..3ad479246c 100644 --- a/book/src/forward_secrecy.md +++ b/book/src/forward_secrecy.md @@ -46,8 +46,8 @@ When an encrypted message is received, the corresponding decryption key is deriv OpenMLS can address 3 scenarios: -- The Delivery Service cannot guarantee that application messages from one epoch are sent before the beginning of the next epoch. To address this, applications can configure their groups to keep the necessary key material around for past epochs by setting the `max_past_epochs` field in the `MlsGroupConfig` to the desired number of epochs. +- The Delivery Service cannot guarantee that application messages from one epoch are sent before the beginning of the next epoch. To address this, applications can configure their groups to keep the necessary key material around for past epochs by setting the `max_past_epochs` field in the `MlsGroupCreateConfig` to the desired number of epochs. -- The Delivery Service cannot guarantee that application messages will arrive in order within the same epoch. To address this, applications can configure the `out_of_order_tolerance` parameter of the `SenderRatchetConfiguration`. The configuration can be set as the `sender_ratchet_configuration` parameter of the `MlsGroupConfig`. +- The Delivery Service cannot guarantee that application messages will arrive in order within the same epoch. To address this, applications can configure the `out_of_order_tolerance` parameter of the `SenderRatchetConfiguration`. The configuration can be set as the `sender_ratchet_configuration` parameter of the `MlsGroupCreateConfig`. -- The Delivery Service cannot guarantee that application messages won't be dropped within the same epoch. To address this, applications can configure the `maximum_forward_distance` parameter of the `SenderRatchetConfiguration`. The configuration can be set as the `sender_ratchet_configuration` parameter of the `MlsGroupConfig`. +- The Delivery Service cannot guarantee that application messages won't be dropped within the same epoch. To address this, applications can configure the `maximum_forward_distance` parameter of the `SenderRatchetConfiguration`. The configuration can be set as the `sender_ratchet_configuration` parameter of the `MlsGroupCreateConfig`. diff --git a/book/src/user_manual/create_group.md b/book/src/user_manual/create_group.md index 0551a5cf7b..8ff8f2c506 100644 --- a/book/src/user_manual/create_group.md +++ b/book/src/user_manual/create_group.md @@ -1,6 +1,6 @@ # Creating groups -Before a group can be created, a group configuration (`MlsGroupConfiguration`) needs to be defined. The default values of configuration parameters are picked for safety. However, check all parameters carefully to ascertain if they match your implementation's requirements. See [Group configuration](group_config.md) for more details. +There are two ways to create a group: Either by building an `MlsGroup` directly, or by using an `MlsGroupCreateConfig`. The former is slightly simpler, while the latter allows the creating of multiple groups using the same configuration. See [Group configuration](./group_config.md) for more details on group parameters. In addition to the group configuration, the client should define all supported and required extensions for the group. The negotiation mechanism for extension in MLS consists in setting an initial list of extensions at group creation time and choosing key packages of subsequent new members accordingly. @@ -10,12 +10,18 @@ In practice, the supported and required extensions are set by adding them to the {{#include ../../../openmls/tests/book_code.rs:create_key_package}} ``` -After that, the group can be created: +After that, the group can be created either using a config: ```rust,no_run,noplayground {{#include ../../../openmls/tests/book_code.rs:alice_create_group}} ``` +... or using the builder pattern: + +```rust,no_run,noplayground +{{#include ../../../openmls/tests/book_code.rs:alice_create_group_with_builder}} +``` + Note: Every group is assigned a random group ID during creation. The group ID cannot be changed and remains immutable throughout the group's lifetime. Choosing it randomly makes sure that the group ID doesn't collide with any other group ID in the same system. If someone else already gave you a group ID, e.g., a provider server, you can also create a group using a specific group ID: diff --git a/book/src/user_manual/group_config.md b/book/src/user_manual/group_config.md index 6ac147fea9..cd826f9adc 100644 --- a/book/src/user_manual/group_config.md +++ b/book/src/user_manual/group_config.md @@ -1,8 +1,8 @@ # Group configuration -The group configuration can be specified by building a `MlsGroupConfig` object or choosing the default value. The default value contains safe values for all parameters and is suitable for scenarios without particular requirements. +Two very similar structs can help configure groups upon their creation: `MlsGroupJoinConfig` and `MlsGroupCreateConfig`. -The following parameters can be set: +`MlsGroupJoinConfig` contains the following runtime-relevant configuration options for an `MlsGroup` and can be set on a per-client basis when a group is joined. | Name | Type | Explanation | | ------------------------------ | ------------------------------- | ------------------------------------------------------------------------------------------------ | @@ -11,11 +11,25 @@ The following parameters can be set: | `max_past_epochs` | `usize` | Maximum number of past epochs for which application messages can be decrypted. The default is 0. | | `number_of_resumption_psks` | `usize` | Number of resumption psks to keep. The default is 0. | | `use_ratchet_tree_extension` | `bool` | Flag indicating the Ratchet Tree Extension should be used. The default is `false`. | -| `required_capabilities` | `RequiredCapabilitiesExtension` | Required capabilities (extensions and proposal types). | | `sender_ratchet_configuration` | `SenderRatchetConfiguration` | Sender ratchet configuration. | -Example configuration: +`MlsGroupCreateConfig` contains an `MlsGroupJoinConfig`, as well as a few additional parameters that are part of the group state that is agreed-upon by all group members. It can be set at the time of a group's creation and contains the following additional configuration options. + +| Name | Type | Explanation | +| ------------------------------ | ------------------------------- | ------------------------------------------------------------------------------------------------ | +| `required_capabilities` | `RequiredCapabilitiesExtension` | Required capabilities (extensions and proposal types). | +| `external_senders` | `ExternalSendersExtensions` | List credentials of non-group members that are allowed to send proposals to the group. | + +Both ways of group configurations can be specified by using the struct's builder pattern, or choosing their default values. The default value contains safe values for all parameters and is suitable for scenarios without particular requirements. + +Example join configuration: ```rust,no_run,noplayground {{#include ../../../openmls/tests/book_code.rs:mls_group_config_example}} ``` + +Example create configuration: + +```rust,no_run,noplayground +{{#include ../../../openmls/tests/book_code.rs:mls_group_create_config_example}} +``` diff --git a/book/src/user_manual/join_from_external_commit.md b/book/src/user_manual/join_from_external_commit.md index f74e84703a..22fc5fc534 100644 --- a/book/src/user_manual/join_from_external_commit.md +++ b/book/src/user_manual/join_from_external_commit.md @@ -2,7 +2,7 @@ To join a group with an external commit message, a new `MlsGroup` can be instantiated directly from the `GroupInfo`. The `GroupInfo`/Ratchet Tree should be shared over a secure channel. -If the RatchetTree extension is not in the required capabilities, then the ratchet tree needs to be provided. +If the RatchetTree extension is not included in the `GroupInfo` as a `GroupInfoExtension`, then the ratchet tree needs to be provided. The `GroupInfo` can be obtained either from a call to `export_group_info`from the `MlsGroup`: @@ -16,7 +16,7 @@ Or from a call to a function that results in a staged commit: {{#include ../../../openmls/tests/book_code.rs:alice_exports_group_info}} ``` -Calling `join_by_external_commit` will join the group and leave it with a commit pending to be merged. +Calling `join_by_external_commit` requires an `MlsGroupJoinConfig` (see [Group configuration](./group_config.md) for more details). The function creates an `MlsGroup` and leave it with a commit pending to be merged. ```rust,no_run,noplayground {{#include ../../../openmls/tests/book_code.rs:charlie_joins_external_commit}} diff --git a/book/src/user_manual/join_from_welcome.md b/book/src/user_manual/join_from_welcome.md index 59f5af53fb..22edf603b5 100644 --- a/book/src/user_manual/join_from_welcome.md +++ b/book/src/user_manual/join_from_welcome.md @@ -1,7 +1,6 @@ # Join a group from a Welcome message -To join a group from a `Welcome` message, a new `MlsGroup` can be instantiated directly from the `Welcome` message. -If the group configuration does not use the ratchet tree extension, the ratchet tree needs to be provided. +To join a group from a `Welcome` message, a new `MlsGroup` can be instantiated directly from the `Welcome` message and an `MlsGroupJoinConfig` (see [Group configuration](./group_config.md) for more details). If the group configuration does not use the ratchet tree extension, the ratchet tree needs to be provided. ```rust,no_run,noplayground {{#include ../../../openmls/tests/book_code.rs:bob_joins_with_welcome}} diff --git a/cli/src/user.rs b/cli/src/user.rs index 3885d292db..dfd864a1f1 100644 --- a/cli/src/user.rs +++ b/cli/src/user.rs @@ -526,7 +526,7 @@ impl User { // NOTE: Since the DS currently doesn't distribute copies of the group's ratchet // tree, we need to include the ratchet_tree_extension. - let group_config = MlsGroupConfig::builder() + let group_config = MlsGroupCreateConfig::builder() .use_ratchet_tree_extension(true) .build(); @@ -672,7 +672,7 @@ impl User { } // NOTE: Since the DS currently doesn't distribute copies of the group's ratchet // tree, we need to include the ratchet_tree_extension. - let group_config = MlsGroupConfig::builder() + let group_config = MlsGroupJoinConfig::builder() .use_ratchet_tree_extension(true) .build(); let mut mls_group = MlsGroup::new_from_welcome(&self.crypto, &group_config, welcome, None) diff --git a/delivery-service/ds/src/test.rs b/delivery-service/ds/src/test.rs index fafded0b8f..5d0ae7616c 100644 --- a/delivery-service/ds/src/test.rs +++ b/delivery-service/ds/src/test.rs @@ -148,7 +148,7 @@ async fn test_list_clients() { #[actix_rt::test] async fn test_group() { let crypto = &OpenMlsRustCrypto::default(); - let mls_group_config = MlsGroupConfig::default(); + let mls_group_create_config = MlsGroupCreateConfig::default(); let data = web::Data::new(DsData::default()); let app = test::init_service( App::new() @@ -257,7 +257,7 @@ async fn test_group() { let mut group = MlsGroup::new_with_group_id( crypto, &signer_1, - &mls_group_config, + &mls_group_create_config, group_id, credential_with_key_1, ) @@ -317,7 +317,7 @@ async fn test_group() { let mut group_on_client2 = MlsGroup::new_from_welcome( crypto, - &mls_group_config, + mls_group_create_config.join_config(), welcome_msg .into_welcome() .expect("Unexpected message type."), diff --git a/interop_client/src/main.rs b/interop_client/src/main.rs index 9590b07514..8274586174 100644 --- a/interop_client/src/main.rs +++ b/interop_client/src/main.rs @@ -17,7 +17,7 @@ use openmls::{ credentials::{Credential, CredentialType, CredentialWithKey}, framing::{MlsMessageIn, MlsMessageInBody, MlsMessageOut, ProcessedMessageContent}, group::{ - GroupEpoch, GroupId, MlsGroup, MlsGroupConfig, WireFormatPolicy, + GroupEpoch, GroupId, MlsGroup, MlsGroupCreateConfig, MlsGroupJoinConfig, WireFormatPolicy, PURE_CIPHERTEXT_WIRE_FORMAT_POLICY, PURE_PLAINTEXT_WIRE_FORMAT_POLICY, }, key_packages::KeyPackage, @@ -230,7 +230,7 @@ impl MlsClient for MlsClientImpl { // Note: We just use some values here that make live testing work. // There is nothing special about the used numbers and they // can be increased (or decreased) depending on the available scenarios. - let mls_group_config = MlsGroupConfig::builder() + let mls_group_config = MlsGroupCreateConfig::builder() .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .max_past_epochs(32) .number_of_resumption_psks(32) @@ -375,7 +375,7 @@ impl MlsClient for MlsClientImpl { // Note: We just use some values here that make live testing work. // There is nothing special about the used numbers and they // can be increased (or decreased) depending on the available scenarios. - let mls_group_config = MlsGroupConfig::builder() + let mls_group_config = MlsGroupJoinConfig::builder() .max_past_epochs(32) .number_of_resumption_psks(32) .sender_ratchet_configuration(SenderRatchetConfiguration::default()) @@ -521,7 +521,7 @@ impl MlsClient for MlsClientImpl { let mls_group_config = { let wire_format_policy = wire_format_policy(request.encrypt_handshake); - MlsGroupConfig::builder() + MlsGroupJoinConfig::builder() .max_past_epochs(32) .number_of_resumption_psks(32) .sender_ratchet_configuration(SenderRatchetConfiguration::default()) @@ -818,7 +818,7 @@ impl MlsClient for MlsClientImpl { // Note: We just use some values here that make live testing work. // There is nothing special about the used numbers and they // can be increased (or decreased) depending on the available scenarios. - let mls_group_config = MlsGroupConfig::builder() + let mls_group_config = MlsGroupJoinConfig::builder() .use_ratchet_tree_extension(true) .max_past_epochs(32) .number_of_resumption_psks(32) @@ -866,7 +866,7 @@ impl MlsClient for MlsClientImpl { // Note: We just use some values here that make live testing work. // There is nothing special about the used numbers and they // can be increased (or decreased) depending on the available scenarios. - let mls_group_config = MlsGroupConfig::builder() + let mls_group_config = MlsGroupJoinConfig::builder() .max_past_epochs(32) .number_of_resumption_psks(32) .use_ratchet_tree_extension(true) @@ -920,7 +920,7 @@ impl MlsClient for MlsClientImpl { // Note: We just use some values here that make live testing work. // There is nothing special about the used numbers and they // can be increased (or decreased) depending on the available scenarios. - let mls_group_config = MlsGroupConfig::builder() + let mls_group_config = MlsGroupJoinConfig::builder() .max_past_epochs(32) .number_of_resumption_psks(32) .use_ratchet_tree_extension(true) diff --git a/openmls/src/extensions/test_extensions.rs b/openmls/src/extensions/test_extensions.rs index 4c9b567f7a..6badafcbe6 100644 --- a/openmls/src/extensions/test_extensions.rs +++ b/openmls/src/extensions/test_extensions.rs @@ -299,7 +299,7 @@ fn last_resort_extension(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvid provider, ); - let mls_group_config = MlsGroupConfigBuilder::new() + let mls_group_create_config = MlsGroupCreateConfig::builder() .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -307,7 +307,7 @@ fn last_resort_extension(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvid let mut alice_group = MlsGroup::new( provider, &alice_credential_with_key_and_signer.signer, - &mls_group_config, + &mls_group_create_config, alice_credential_with_key_and_signer.credential_with_key, ) .expect("An unexpected error occurred."); @@ -326,7 +326,7 @@ fn last_resort_extension(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvid let _bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected MLS message"), Some(alice_group.export_ratchet_tree().into()), ) diff --git a/openmls/src/group/core_group/kat_passive_client.rs b/openmls/src/group/core_group/kat_passive_client.rs index 78178d020f..b0a9d495b3 100644 --- a/openmls/src/group/core_group/kat_passive_client.rs +++ b/openmls/src/group/core_group/kat_passive_client.rs @@ -118,8 +118,7 @@ pub fn run_test_vector(test_vector: PassiveClientWelcomeTestVector) { return; } - let group_config = MlsGroupConfig::builder() - .crypto_config(CryptoConfig::with_default_version(cipher_suite)) + let group_config = MlsGroupJoinConfig::builder() .use_ratchet_tree_extension(true) .wire_format_policy(WireFormatPolicy::new( OutgoingWireFormatPolicy::AlwaysPlaintext, @@ -128,7 +127,11 @@ pub fn run_test_vector(test_vector: PassiveClientWelcomeTestVector) { .number_of_resumption_psks(16) .build(); - let mut passive_client = PassiveClient::new(group_config, test_vector.external_psks.clone()); + let mut passive_client = PassiveClient::new( + group_config, + cipher_suite, + test_vector.external_psks.clone(), + ); passive_client.inject_key_package( test_vector.key_package, @@ -198,12 +201,16 @@ fn test_write_vectors() { struct PassiveClient { provider: OpenMlsRustCrypto, - group_config: MlsGroupConfig, + group_config: MlsGroupJoinConfig, group: Option, } impl PassiveClient { - fn new(group_config: MlsGroupConfig, psks: Vec) -> Self { + fn new( + group_config: MlsGroupJoinConfig, + ciphersuite: Ciphersuite, + psks: Vec, + ) -> Self { let provider = OpenMlsRustCrypto::default(); // Load all PSKs into key store. @@ -213,7 +220,7 @@ impl PassiveClient { // The nonce is not saved, so it can be empty... let psk_id = PreSharedKeyId::external(psk.psk_id, vec![]); psk_id - .write_to_key_store(&provider, group_config.crypto_config.ciphersuite, &psk.psk) + .write_to_key_store(&provider, ciphersuite, &psk.psk) .unwrap(); } @@ -333,7 +340,7 @@ impl PassiveClient { } pub fn generate_test_vector(cipher_suite: Ciphersuite) -> PassiveClientWelcomeTestVector { - let group_config = MlsGroupConfig::builder() + let group_config = MlsGroupCreateConfig::builder() .crypto_config(CryptoConfig::with_default_version(cipher_suite)) .use_ratchet_tree_extension(true) .build(); diff --git a/openmls/src/group/mls_group/builder.rs b/openmls/src/group/mls_group/builder.rs new file mode 100644 index 0000000000..fe7040bb12 --- /dev/null +++ b/openmls/src/group/mls_group/builder.rs @@ -0,0 +1,201 @@ +use openmls_traits::{key_store::OpenMlsKeyStore, signatures::Signer, OpenMlsProvider}; + +use crate::{ + credentials::CredentialWithKey, + extensions::ExternalSendersExtension, + group::{ + config::CryptoConfig, public_group::errors::PublicGroupBuildError, CoreGroup, + CoreGroupBuildError, CoreGroupConfig, GroupId, MlsGroupCreateConfig, + MlsGroupCreateConfigBuilder, NewGroupError, ProposalStore, WireFormatPolicy, + }, + prelude::{LibraryError, Lifetime, SenderRatchetConfiguration}, +}; + +use super::{InnerState, MlsGroup, MlsGroupState}; + +#[derive(Default)] +pub struct MlsGroupBuilder { + group_id: Option, + mls_group_create_config_builder: MlsGroupCreateConfigBuilder, +} + +impl MlsGroupBuilder { + pub(super) fn new() -> Self { + Self::default() + } + + /// Sets the group ID of the [`MlsGroup`]. + pub fn with_group_id(mut self, group_id: GroupId) -> Self { + self.group_id = Some(group_id); + self + } + + /// Build a new group as configured by this builder. + pub fn build( + self, + provider: &impl OpenMlsProvider, + signer: &impl Signer, + credential_with_key: CredentialWithKey, + ) -> Result> { + self.build_internal(provider, signer, credential_with_key, None) + } + + /// Build a new group with the given group ID. + /// + /// If an [`MlsGroupCreateConfig`] is provided, it will be used to configure the + /// group. Otherwise, the internal builder is used to build one with the + /// parameters set on this builder. + pub(super) fn build_internal( + self, + provider: &impl OpenMlsProvider, + signer: &impl Signer, + credential_with_key: CredentialWithKey, + mls_group_create_config_option: Option, + ) -> Result> { + let mls_group_create_config = mls_group_create_config_option + .unwrap_or_else(|| self.mls_group_create_config_builder.build()); + let group_id = self + .group_id + .unwrap_or_else(|| GroupId::random(provider.rand())); + // TODO #751 + let group_config = CoreGroupConfig { + add_ratchet_tree_extension: mls_group_create_config + .join_config + .use_ratchet_tree_extension, + }; + + let mut group = CoreGroup::builder( + group_id, + mls_group_create_config.crypto_config, + credential_with_key, + ) + .with_config(group_config) + .with_required_capabilities(mls_group_create_config.required_capabilities.clone()) + .with_external_senders(mls_group_create_config.external_senders.clone()) + .with_max_past_epoch_secrets(mls_group_create_config.join_config.max_past_epochs) + .with_lifetime(*mls_group_create_config.lifetime()) + .build(provider, signer) + .map_err(|e| match e { + CoreGroupBuildError::LibraryError(e) => e.into(), + // We don't support PSKs yet + CoreGroupBuildError::Psk(e) => { + log::debug!("Unexpected PSK error: {:?}", e); + LibraryError::custom("Unexpected PSK error").into() + } + CoreGroupBuildError::KeyStoreError(e) => NewGroupError::KeyStoreError(e), + CoreGroupBuildError::PublicGroupBuildError(e) => match e { + PublicGroupBuildError::LibraryError(e) => e.into(), + PublicGroupBuildError::UnsupportedProposalType => { + NewGroupError::UnsupportedProposalType + } + PublicGroupBuildError::UnsupportedExtensionType => { + NewGroupError::UnsupportedExtensionType + } + PublicGroupBuildError::InvalidExtensions(e) => NewGroupError::InvalidExtensions(e), + }, + })?; + + // We already add a resumption PSK for epoch 0 to make things more unified. + let resumption_psk = group.group_epoch_secrets().resumption_psk(); + group + .resumption_psk_store + .add(group.context().epoch(), resumption_psk.clone()); + + let mls_group = MlsGroup { + mls_group_config: mls_group_create_config.join_config.clone(), + group, + proposal_store: ProposalStore::new(), + own_leaf_nodes: vec![], + aad: vec![], + group_state: MlsGroupState::Operational, + state_changed: InnerState::Changed, + }; + + Ok(mls_group) + } + + // MlsGroupCreateConfigBuilder options + + /// Sets the `wire_format` property of the MlsGroup. + pub fn with_wire_format_policy(mut self, wire_format_policy: WireFormatPolicy) -> Self { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .wire_format_policy(wire_format_policy); + self + } + + /// Sets the `padding_size` property of the MlsGroup. + pub fn padding_size(mut self, padding_size: usize) -> Self { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .padding_size(padding_size); + self + } + + /// Sets the `max_past_epochs` property of the MlsGroup. + /// This allows application messages from previous epochs to be decrypted. + /// + /// **WARNING** + /// + /// This feature enables the storage of message secrets from past epochs. + /// It is a trade-off between functionality and forward secrecy and should only be enabled + /// if the Delivery Service cannot guarantee that application messages will be sent in + /// the same epoch in which they were generated. The number for `max_epochs` should be + /// as low as possible. + pub fn max_past_epochs(mut self, max_past_epochs: usize) -> Self { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .max_past_epochs(max_past_epochs); + self + } + + /// Sets the `number_of_resumption_psks` property of the MlsGroup. + pub fn number_of_resumption_psks(mut self, number_of_resumption_psks: usize) -> Self { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .number_of_resumption_psks(number_of_resumption_psks); + self + } + + /// Sets the `use_ratchet_tree_extension` property of the MlsGroup. + pub fn use_ratchet_tree_extension(mut self, use_ratchet_tree_extension: bool) -> Self { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .use_ratchet_tree_extension(use_ratchet_tree_extension); + self + } + + /// Sets the `sender_ratchet_configuration` property of the MlsGroup. + /// See [`SenderRatchetConfiguration`] for more information. + pub fn sender_ratchet_configuration( + mut self, + sender_ratchet_configuration: SenderRatchetConfiguration, + ) -> Self { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .sender_ratchet_configuration(sender_ratchet_configuration); + self + } + + /// Sets the `lifetime` of the group creator's leaf. + pub fn lifetime(mut self, lifetime: Lifetime) -> Self { + self.mls_group_create_config_builder = + self.mls_group_create_config_builder.lifetime(lifetime); + self + } + + /// Sets the `crypto_config` of the MlsGroup. + pub fn crypto_config(mut self, config: CryptoConfig) -> Self { + self.mls_group_create_config_builder = + self.mls_group_create_config_builder.crypto_config(config); + self + } + + /// Sets the `external_senders` property of the MlsGroup. + pub fn external_senders(mut self, external_senders: ExternalSendersExtension) -> Self { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .external_senders(external_senders); + self + } +} diff --git a/openmls/src/group/mls_group/config.rs b/openmls/src/group/mls_group/config.rs index a78c5e4ff6..b7366cdf00 100644 --- a/openmls/src/group/mls_group/config.rs +++ b/openmls/src/group/mls_group/config.rs @@ -1,18 +1,18 @@ //! Configuration module for [`MlsGroup`] configurations. //! -//! ## Building an MlsGroupConfig -//! The [`MlsGroupConfigBuilder`] makes it easy to build configurations for the +//! ## Building an MlsGroupCreateConfig +//! The [`MlsGroupCreateConfigBuilder`] makes it easy to build configurations for the //! [`MlsGroup`]. //! //! ``` //! use openmls::prelude::*; //! -//! let group_config = MlsGroupConfig::builder() +//! let group_config = MlsGroupCreateConfig::builder() //! .use_ratchet_tree_extension(true) //! .build(); //! ``` //! -//! See [`MlsGroupConfigBuilder`](MlsGroupConfigBuilder#implementations) for +//! See [`MlsGroupCreateConfigBuilder`](MlsGroupCreateConfigBuilder#implementations) for //! all options that can be configured. //! //! ### Wire format policies @@ -22,7 +22,7 @@ //! ``` //! use openmls::prelude::*; //! -//! let group_config = MlsGroupConfig::builder() +//! let group_config = MlsGroupCreateConfig::builder() //! .wire_format_policy(MIXED_CIPHERTEXT_WIRE_FORMAT_POLICY) //! .build(); //! ``` @@ -34,10 +34,12 @@ use crate::{ }; use serde::{Deserialize, Serialize}; -/// Specifies the configuration parameters for a [`MlsGroup`]. Refer to -/// the [User Manual](https://openmls.tech/book/user_manual/group_config.html) for more information about the different configuration values. +/// The [`MlsGroupJoinConfig`] contains all configuration parameters that are +/// relevant to group operation at runtime. It is used to configure the group's +/// behaviour when joining an existing group. To configure a newly created +/// group, use [`MlsGroupCreateConfig`]. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] -pub struct MlsGroupConfig { +pub struct MlsGroupJoinConfig { /// Defines the wire format policy for outgoing and incoming handshake messages. /// Application are always encrypted regardless. pub(crate) wire_format_policy: WireFormatPolicy, @@ -50,65 +52,155 @@ pub struct MlsGroupConfig { pub(crate) number_of_resumption_psks: usize, /// Flag to indicate the Ratchet Tree Extension should be used pub(crate) use_ratchet_tree_extension: bool, + /// Sender ratchet configuration + pub(crate) sender_ratchet_configuration: SenderRatchetConfiguration, +} + +impl MlsGroupJoinConfig { + /// Returns a builder for [`MlsGroupJoinConfig`]. + pub fn builder() -> MlsGroupJoinConfigBuilder { + MlsGroupJoinConfigBuilder::new() + } + + /// Returns the wire format policy set in this [`MlsGroupJoinConfig`]. + pub fn wire_format_policy(&self) -> WireFormatPolicy { + self.wire_format_policy + } + + /// Returns the padding size set in this [`MlsGroupJoinConfig`]. + pub fn padding_size(&self) -> usize { + self.padding_size + } + + /// Returns the [`SenderRatchetConfiguration`] set in this [`MlsGroupJoinConfig`]. + pub fn sender_ratchet_configuration(&self) -> &SenderRatchetConfiguration { + &self.sender_ratchet_configuration + } +} + +/// Specifies configuration for the creation of an [`MlsGroup`]. Refer to the +/// [User Manual](https://openmls.tech/book/user_manual/group_config.html) for +/// more information about the different configuration values. +#[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] +pub struct MlsGroupCreateConfig { /// Required capabilities (extensions and proposal types) pub(crate) required_capabilities: RequiredCapabilitiesExtension, /// Senders authorized to send external remove proposals pub(crate) external_senders: ExternalSendersExtension, - /// Sender ratchet configuration - pub(crate) sender_ratchet_configuration: SenderRatchetConfiguration, /// Lifetime of the own leaf node pub(crate) lifetime: Lifetime, /// Ciphersuite and protocol version pub(crate) crypto_config: CryptoConfig, + /// Configuration parameters relevant to group operation at runtime + pub(crate) join_config: MlsGroupJoinConfig, +} + +/// Builder struct for an [`MlsGroupJoinConfig`]. +#[derive(Default)] +pub struct MlsGroupJoinConfigBuilder { + join_config: MlsGroupJoinConfig, +} + +impl MlsGroupJoinConfigBuilder { + /// Creates a new builder with default values. + fn new() -> Self { + Self { + join_config: MlsGroupJoinConfig::default(), + } + } + + /// Sets the `wire_format` property of the [`MlsGroupJoinConfig`]. + pub fn wire_format_policy(mut self, wire_format_policy: WireFormatPolicy) -> Self { + self.join_config.wire_format_policy = wire_format_policy; + self + } + + /// Sets the `padding_size` property of the [`MlsGroupJoinConfig`]. + pub fn padding_size(mut self, padding_size: usize) -> Self { + self.join_config.padding_size = padding_size; + self + } + + /// Sets the `max_past_epochs` property of the [`MlsGroupJoinConfig`]. + pub fn max_past_epochs(mut self, max_past_epochs: usize) -> Self { + self.join_config.max_past_epochs = max_past_epochs; + self + } + + /// Sets the `number_of_resumption_psks` property of the [`MlsGroupJoinConfig`]. + pub fn number_of_resumption_psks(mut self, number_of_resumption_psks: usize) -> Self { + self.join_config.number_of_resumption_psks = number_of_resumption_psks; + self + } + + /// Sets the `use_ratchet_tree_extension` property of the [`MlsGroupJoinConfig`]. + pub fn use_ratchet_tree_extension(mut self, use_ratchet_tree_extension: bool) -> Self { + self.join_config.use_ratchet_tree_extension = use_ratchet_tree_extension; + self + } + + /// Sets the `sender_ratchet_configuration` property of the [`MlsGroupJoinConfig`]. + pub fn sender_ratchet_configuration( + mut self, + sender_ratchet_configuration: SenderRatchetConfiguration, + ) -> Self { + self.join_config.sender_ratchet_configuration = sender_ratchet_configuration; + self + } + + /// Finalizes the builder and returns an [`MlsGroupJoinConfig`]. + pub fn build(self) -> MlsGroupJoinConfig { + self.join_config + } } -impl MlsGroupConfig { - /// Returns a builder for [`MlsGroupConfig`] - pub fn builder() -> MlsGroupConfigBuilder { - MlsGroupConfigBuilder::new() +impl MlsGroupCreateConfig { + /// Returns a builder for [`MlsGroupCreateConfig`] + pub fn builder() -> MlsGroupCreateConfigBuilder { + MlsGroupCreateConfigBuilder::new() } - /// Returns the [`MlsGroupConfig`] wire format policy. + /// Returns the [`MlsGroupCreateConfig`] wire format policy. pub fn wire_format_policy(&self) -> WireFormatPolicy { - self.wire_format_policy + self.join_config.wire_format_policy } - /// Returns the [`MlsGroupConfig`] padding size. + /// Returns the [`MlsGroupCreateConfig`] padding size. pub fn padding_size(&self) -> usize { - self.padding_size + self.join_config.padding_size } - /// Returns the [`MlsGroupConfig`] max past epochs. + /// Returns the [`MlsGroupCreateConfig`] max past epochs. pub fn max_past_epochs(&self) -> usize { - self.max_past_epochs + self.join_config.max_past_epochs } - /// Returns the [`MlsGroupConfig`] number of resumption psks. + /// Returns the [`MlsGroupCreateConfig`] number of resumption psks. pub fn number_of_resumption_psks(&self) -> usize { - self.number_of_resumption_psks + self.join_config.number_of_resumption_psks } - /// Returns the [`MlsGroupConfig`] boolean flag that indicates whether ratchet_tree_extension should be used. + /// Returns the [`MlsGroupCreateConfig`] boolean flag that indicates whether ratchet_tree_extension should be used. pub fn use_ratchet_tree_extension(&self) -> bool { - self.use_ratchet_tree_extension + self.join_config.use_ratchet_tree_extension } - /// Returns the [`MlsGroupConfig`] required capabilities extension. + /// Returns the [`MlsGroupCreateConfig`] required capabilities extension. pub fn required_capabilities(&self) -> &RequiredCapabilitiesExtension { &self.required_capabilities } - /// Returns the [`MlsGroupConfig`] sender ratchet configuration. + /// Returns the [`MlsGroupCreateConfig`] sender ratchet configuration. pub fn sender_ratchet_configuration(&self) -> &SenderRatchetConfiguration { - &self.sender_ratchet_configuration + &self.join_config.sender_ratchet_configuration } - /// Returns the [`MlsGroupConfig`] external senders extension + /// Returns the [`MlsGroupCreateConfig`] external senders extension pub fn external_senders(&self) -> &ExternalSendersExtension { &self.external_senders } - /// Returns the [`MlsGroupConfig`] lifetime configuration. + /// Returns the [`MlsGroupCreateConfig`] lifetime configuration. pub fn lifetime(&self) -> &Lifetime { &self.lifetime } @@ -128,35 +220,40 @@ impl MlsGroupConfig { .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build() } + + /// Returns the [`MlsGroupJoinConfig`] of groups created with this create config. + pub fn join_config(&self) -> &MlsGroupJoinConfig { + &self.join_config + } } -/// Builder for an [`MlsGroupConfig`]. +/// Builder for an [`MlsGroupCreateConfig`]. #[derive(Default)] -pub struct MlsGroupConfigBuilder { - config: MlsGroupConfig, +pub struct MlsGroupCreateConfigBuilder { + config: MlsGroupCreateConfig, } -impl MlsGroupConfigBuilder { +impl MlsGroupCreateConfigBuilder { /// Creates a new builder with default values. - pub fn new() -> Self { - MlsGroupConfigBuilder { - config: MlsGroupConfig::default(), + fn new() -> Self { + MlsGroupCreateConfigBuilder { + config: MlsGroupCreateConfig::default(), } } - /// Sets the `wire_format` property of the MlsGroupConfig. + /// Sets the `wire_format` property of the MlsGroupCreateConfig. pub fn wire_format_policy(mut self, wire_format_policy: WireFormatPolicy) -> Self { - self.config.wire_format_policy = wire_format_policy; + self.config.join_config.wire_format_policy = wire_format_policy; self } - /// Sets the `padding_size` property of the MlsGroupConfig. + /// Sets the `padding_size` property of the MlsGroupCreateConfig. pub fn padding_size(mut self, padding_size: usize) -> Self { - self.config.padding_size = padding_size; + self.config.join_config.padding_size = padding_size; self } - /// Sets the `max_past_epochs` property of the MlsGroupConfig. + /// Sets the `max_past_epochs` property of the MlsGroupCreateConfig. /// This allows application messages from previous epochs to be decrypted. /// /// **WARNING** @@ -167,23 +264,23 @@ impl MlsGroupConfigBuilder { /// the same epoch in which they were generated. The number for `max_epochs` should be /// as low as possible. pub fn max_past_epochs(mut self, max_past_epochs: usize) -> Self { - self.config.max_past_epochs = max_past_epochs; + self.config.join_config.max_past_epochs = max_past_epochs; self } - /// Sets the `number_of_resumption_psks` property of the MlsGroupConfig. + /// Sets the `number_of_resumption_psks` property of the MlsGroupCreateConfig. pub fn number_of_resumption_psks(mut self, number_of_resumption_psks: usize) -> Self { - self.config.number_of_resumption_psks = number_of_resumption_psks; + self.config.join_config.number_of_resumption_psks = number_of_resumption_psks; self } - /// Sets the `use_ratchet_tree_extension` property of the MlsGroupConfig. + /// Sets the `use_ratchet_tree_extension` property of the MlsGroupCreateConfig. pub fn use_ratchet_tree_extension(mut self, use_ratchet_tree_extension: bool) -> Self { - self.config.use_ratchet_tree_extension = use_ratchet_tree_extension; + self.config.join_config.use_ratchet_tree_extension = use_ratchet_tree_extension; self } - /// Sets the `required_capabilities` property of the MlsGroupConfig. + /// Sets the `required_capabilities` property of the MlsGroupCreateConfig. pub fn required_capabilities( mut self, required_capabilities: RequiredCapabilitiesExtension, @@ -192,36 +289,36 @@ impl MlsGroupConfigBuilder { self } - /// Sets the `sender_ratchet_configuration` property of the MlsGroupConfig. + /// Sets the `sender_ratchet_configuration` property of the MlsGroupCreateConfig. /// See [`SenderRatchetConfiguration`] for more information. pub fn sender_ratchet_configuration( mut self, sender_ratchet_configuration: SenderRatchetConfiguration, ) -> Self { - self.config.sender_ratchet_configuration = sender_ratchet_configuration; + self.config.join_config.sender_ratchet_configuration = sender_ratchet_configuration; self } - /// Sets the `lifetime` property of the MlsGroupConfig. + /// Sets the `lifetime` property of the MlsGroupCreateConfig. pub fn lifetime(mut self, lifetime: Lifetime) -> Self { self.config.lifetime = lifetime; self } - /// Sets the `crypto_config` property of the MlsGroupConfig. + /// Sets the `crypto_config` property of the MlsGroupCreateConfig. pub fn crypto_config(mut self, config: CryptoConfig) -> Self { self.config.crypto_config = config; self } - /// Sets the `external_senders` property of the MlsGroupConfig. + /// Sets the `external_senders` property of the MlsGroupCreateConfig. pub fn external_senders(mut self, external_senders: ExternalSendersExtension) -> Self { self.config.external_senders = external_senders; self } - /// Finalizes the builder and retursn an `[MlsGroupConfig`]. - pub fn build(self) -> MlsGroupConfig { + /// Finalizes the builder and retursn an `[MlsGroupCreateConfig`]. + pub fn build(self) -> MlsGroupCreateConfig { self.config } } diff --git a/openmls/src/group/mls_group/creation.rs b/openmls/src/group/mls_group/creation.rs index df2164bea4..378233544a 100644 --- a/openmls/src/group/mls_group/creation.rs +++ b/openmls/src/group/mls_group/creation.rs @@ -1,13 +1,12 @@ use openmls_traits::signatures::Signer; -use super::*; +use super::{builder::MlsGroupBuilder, *}; use crate::{ ciphersuite::HpkePrivateKey, credentials::CredentialWithKey, group::{ core_group::create_commit_params::CreateCommitParams, - errors::{CoreGroupBuildError, ExternalCommitError, WelcomeError}, - public_group::errors::PublicGroupBuildError, + errors::{ExternalCommitError, WelcomeError}, }, messages::group_info::{GroupInfo, VerifiableGroupInfo}, schedule::psk::store::ResumptionPskStore, @@ -17,6 +16,12 @@ use crate::{ impl MlsGroup { // === Group creation === + /// Creates a builder which can be used to configure and build + /// a new [`MlsGroup`]. + pub fn builder() -> MlsGroupBuilder { + MlsGroupBuilder::new() + } + /// Creates a new group with the creator as the only member (and a random /// group ID). /// @@ -25,15 +30,14 @@ impl MlsGroup { pub fn new( provider: &impl OpenMlsProvider, signer: &impl Signer, - mls_group_config: &MlsGroupConfig, + mls_group_create_config: &MlsGroupCreateConfig, credential_with_key: CredentialWithKey, ) -> Result> { - Self::new_with_group_id( + MlsGroupBuilder::new().build_internal( provider, signer, - mls_group_config, - GroupId::random(provider.rand()), credential_with_key, + Some(mls_group_create_config.clone()), ) } @@ -42,63 +46,18 @@ impl MlsGroup { pub fn new_with_group_id( provider: &impl OpenMlsProvider, signer: &impl Signer, - mls_group_config: &MlsGroupConfig, + mls_group_create_config: &MlsGroupCreateConfig, group_id: GroupId, credential_with_key: CredentialWithKey, ) -> Result> { - // TODO #751 - let group_config = CoreGroupConfig { - add_ratchet_tree_extension: mls_group_config.use_ratchet_tree_extension, - }; - - let mut group = CoreGroup::builder( - group_id, - mls_group_config.crypto_config, - credential_with_key, - ) - .with_config(group_config) - .with_required_capabilities(mls_group_config.required_capabilities.clone()) - .with_external_senders(mls_group_config.external_senders.clone()) - .with_max_past_epoch_secrets(mls_group_config.max_past_epochs) - .with_lifetime(*mls_group_config.lifetime()) - .build(provider, signer) - .map_err(|e| match e { - CoreGroupBuildError::LibraryError(e) => e.into(), - // We don't support PSKs yet - CoreGroupBuildError::Psk(e) => { - log::debug!("Unexpected PSK error: {:?}", e); - LibraryError::custom("Unexpected PSK error").into() - } - CoreGroupBuildError::KeyStoreError(e) => NewGroupError::KeyStoreError(e), - CoreGroupBuildError::PublicGroupBuildError(e) => match e { - PublicGroupBuildError::LibraryError(e) => e.into(), - PublicGroupBuildError::UnsupportedProposalType => { - NewGroupError::UnsupportedProposalType - } - PublicGroupBuildError::UnsupportedExtensionType => { - NewGroupError::UnsupportedExtensionType - } - PublicGroupBuildError::InvalidExtensions(e) => NewGroupError::InvalidExtensions(e), - }, - })?; - - // We already add a resumption PSK for epoch 0 to make things more unified. - let resumption_psk = group.group_epoch_secrets().resumption_psk(); - group - .resumption_psk_store - .add(group.context().epoch(), resumption_psk.clone()); - - let mls_group = MlsGroup { - mls_group_config: mls_group_config.clone(), - group, - proposal_store: ProposalStore::new(), - own_leaf_nodes: vec![], - aad: vec![], - group_state: MlsGroupState::Operational, - state_changed: InnerState::Changed, - }; - - Ok(mls_group) + MlsGroupBuilder::new() + .with_group_id(group_id) + .build_internal( + provider, + signer, + credential_with_key, + Some(mls_group_create_config.clone()), + ) } /// Creates a new group from a [`Welcome`] message. Returns an error @@ -107,7 +66,7 @@ impl MlsGroup { // TODO: #1326 This should take an MlsMessage rather than a Welcome message. pub fn new_from_welcome( provider: &impl OpenMlsProvider, - mls_group_config: &MlsGroupConfig, + mls_group_config: &MlsGroupJoinConfig, welcome: Welcome, ratchet_tree: Option, ) -> Result> { @@ -187,7 +146,7 @@ impl MlsGroup { signer: &impl Signer, ratchet_tree: Option, verifiable_group_info: VerifiableGroupInfo, - mls_group_config: &MlsGroupConfig, + mls_group_config: &MlsGroupJoinConfig, aad: &[u8], credential_with_key: CredentialWithKey, ) -> Result<(Self, MlsMessageOut, Option), ExternalCommitError> { diff --git a/openmls/src/group/mls_group/mod.rs b/openmls/src/group/mls_group/mod.rs index b06b9259cd..64a9982c02 100644 --- a/openmls/src/group/mls_group/mod.rs +++ b/openmls/src/group/mls_group/mod.rs @@ -19,6 +19,7 @@ use openmls_traits::{key_store::OpenMlsKeyStore, types::Ciphersuite, OpenMlsProv // Private mod application; +mod builder; mod creation; mod exporting; mod updates; @@ -148,8 +149,8 @@ pub enum MlsGroupState { /// more information. #[derive(Debug)] pub struct MlsGroup { - // The group configuration. See `MlsGroupCongig` for more information. - mls_group_config: MlsGroupConfig, + // The group configuration. See [`MlsGroupJoinConfig`] for more information. + mls_group_config: MlsGroupJoinConfig, // the internal `CoreGroup` used for lower level operations. See `CoreGroup` for more // information. group: CoreGroup, @@ -176,12 +177,12 @@ impl MlsGroup { // === Configuration === /// Returns the configuration. - pub fn configuration(&self) -> &MlsGroupConfig { + pub fn configuration(&self) -> &MlsGroupJoinConfig { &self.mls_group_config } /// Sets the configuration. - pub fn set_configuration(&mut self, mls_group_config: &MlsGroupConfig) { + pub fn set_configuration(&mut self, mls_group_config: &MlsGroupJoinConfig) { self.mls_group_config = mls_group_config.clone(); // Since the state of the group might be changed, arm the state flag diff --git a/openmls/src/group/mls_group/ser.rs b/openmls/src/group/mls_group/ser.rs index b8b82333e4..7977fad823 100644 --- a/openmls/src/group/mls_group/ser.rs +++ b/openmls/src/group/mls_group/ser.rs @@ -17,7 +17,7 @@ use serde::{ )] #[derive(Serialize, Deserialize)] pub struct SerializedMlsGroup { - mls_group_config: MlsGroupConfig, + mls_group_config: MlsGroupJoinConfig, group: CoreGroup, proposal_store: ProposalStore, own_leaf_nodes: Vec, diff --git a/openmls/src/group/mls_group/test_mls_group.rs b/openmls/src/group/mls_group/test_mls_group.rs index 46dd2eea94..b22ec84b6a 100644 --- a/openmls/src/group/mls_group/test_mls_group.rs +++ b/openmls/src/group/mls_group/test_mls_group.rs @@ -14,6 +14,7 @@ use crate::{ MlsGroupTestSetup, }, test_utils::*, + tree::sender_ratchet::SenderRatchetConfiguration, }; #[apply(ciphersuites_and_providers)] @@ -24,7 +25,7 @@ fn test_mls_group_persistence(ciphersuite: Ciphersuite, provider: &impl OpenMlsP setup_client("Alice", ciphersuite, provider); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + let mls_group_config = MlsGroupCreateConfig::test_default(ciphersuite); // === Alice creates a group === let mut alice_group = MlsGroup::new_with_group_id( @@ -72,7 +73,7 @@ fn remover(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { setup_client("Charly", ciphersuite, provider); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfigBuilder::new() + let mls_group_create_config = MlsGroupCreateConfig::builder() .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -80,7 +81,7 @@ fn remover(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential_with_key, ) @@ -97,7 +98,7 @@ fn remover(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let mut bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), Some(alice_group.export_ratchet_tree().into()), ) @@ -132,7 +133,7 @@ fn remover(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let mut charlie_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), Some(bob_group.export_ratchet_tree().into()), ) @@ -209,13 +210,13 @@ fn export_secret(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { setup_client("Alice", ciphersuite, provider); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + let mls_group_create_config = MlsGroupCreateConfig::test_default(ciphersuite); // === Alice creates a group === let alice_group = MlsGroup::new_with_group_id( provider, &alice_signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential_with_key, ) @@ -242,11 +243,11 @@ fn export_secret(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { #[apply(ciphersuites_and_providers)] fn test_invalid_plaintext(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { // Some basic setup functions for the MlsGroup. - let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + let mls_group_create_config = MlsGroupCreateConfig::test_default(ciphersuite); let number_of_clients = 20; let setup = MlsGroupTestSetup::new( - mls_group_config, + mls_group_create_config, number_of_clients, CodecUse::StructMessages, ); @@ -359,7 +360,7 @@ fn test_verify_staged_commit_credentials( setup_client("Bob", ciphersuite, provider); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + let mls_group_config = MlsGroupCreateConfig::test_default(ciphersuite); // === Alice creates a group === let mut alice_group = MlsGroup::new_with_group_id( @@ -408,7 +409,7 @@ fn test_verify_staged_commit_credentials( let mut bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_config.join_config(), welcome_option .expect("no welcome after commit") .into_welcome() @@ -524,13 +525,13 @@ fn test_commit_with_update_path_leaf_node( setup_client("Bob", ciphersuite, provider); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + let mls_group_create_config = MlsGroupCreateConfig::test_default(ciphersuite); // === Alice creates a group === let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential_with_key.clone(), ) @@ -575,7 +576,7 @@ fn test_commit_with_update_path_leaf_node( let mut bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome_option .expect("no welcome after commit") .into_welcome() @@ -700,13 +701,13 @@ fn test_pending_commit_logic(ciphersuite: Ciphersuite, provider: &impl OpenMlsPr setup_client("Bob", ciphersuite, provider); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + let mls_group_create_config = MlsGroupCreateConfig::test_default(ciphersuite); // === Alice creates a group === let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential_with_key, ) @@ -817,7 +818,7 @@ fn test_pending_commit_logic(ciphersuite: Ciphersuite, provider: &impl OpenMlsPr let mut bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome_option .expect("no welcome after commit") .into_welcome() @@ -874,7 +875,7 @@ fn key_package_deletion(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvide let bob_key_package = bob_kpb.key_package(); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfigBuilder::new() + let mls_group_create_config = MlsGroupCreateConfig::builder() .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -882,7 +883,7 @@ fn key_package_deletion(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvide let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential_with_key, ) @@ -898,7 +899,7 @@ fn key_package_deletion(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvide // === Bob joins the group === let _bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), Some(alice_group.export_ratchet_tree().into()), ) @@ -940,7 +941,7 @@ fn remove_prosposal_by_ref(ciphersuite: Ciphersuite, provider: &impl OpenMlsProv let charlie_key_package = charlie_kpb.key_package(); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfigBuilder::new() + let mls_group_create_config = MlsGroupCreateConfig::builder() .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -948,7 +949,7 @@ fn remove_prosposal_by_ref(ciphersuite: Ciphersuite, provider: &impl OpenMlsProv let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential_with_key, ) @@ -961,7 +962,7 @@ fn remove_prosposal_by_ref(ciphersuite: Ciphersuite, provider: &impl OpenMlsProv alice_group.merge_pending_commit(provider).unwrap(); let mut bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().unwrap(), Some(alice_group.export_ratchet_tree().into()), ) @@ -1001,3 +1002,76 @@ fn remove_prosposal_by_ref(ciphersuite: Ciphersuite, provider: &impl OpenMlsProv _ => unreachable!("Expected a StagedCommit."), } } + +// Test that the builder pattern accurately configures the new group. +#[apply(ciphersuites_and_providers)] +fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { + let (alice_credential_with_key, _alice_kpb, alice_signer, _alice_pk) = + setup_client("Alice", ciphersuite, provider); + + // Variables for the MlsGroup configuration + let test_group_id = GroupId::from_slice(b"Test Group"); + let test_lifetime = Lifetime::new(3600); + let test_wire_format_policy = PURE_CIPHERTEXT_WIRE_FORMAT_POLICY; + let test_padding_size = 100; + let test_external_senders = vec![ExternalSender::new( + alice_credential_with_key.signature_key.clone(), + alice_credential_with_key.credential.clone(), + )]; + let test_crypto_config = CryptoConfig::with_default_version(ciphersuite); + let test_sender_ratchet_config = SenderRatchetConfiguration::new(10, 2000); + let test_max_past_epochs = 10; + let test_number_of_resumption_psks = 5; + + // === Alice creates a group === + let alice_group = MlsGroup::builder() + .with_group_id(test_group_id.clone()) + .padding_size(test_padding_size) + .sender_ratchet_configuration(test_sender_ratchet_config.clone()) + .external_senders(test_external_senders.clone()) + .crypto_config(test_crypto_config) + .with_wire_format_policy(test_wire_format_policy) + .lifetime(test_lifetime) + .use_ratchet_tree_extension(true) + .max_past_epochs(test_max_past_epochs) + .number_of_resumption_psks(test_number_of_resumption_psks) + .build(provider, &alice_signer, alice_credential_with_key) + .expect("error creating group using builder"); + + // Check that the group was created with the correct configuration + + // first the config + let group_config = alice_group.configuration(); + assert_eq!(group_config.padding_size(), test_padding_size); + assert_eq!( + group_config.sender_ratchet_configuration(), + &test_sender_ratchet_config + ); + assert_eq!(group_config.wire_format_policy(), test_wire_format_policy); + assert!(group_config.use_ratchet_tree_extension); + assert_eq!(group_config.max_past_epochs, test_max_past_epochs); + assert_eq!( + group_config.number_of_resumption_psks, + test_number_of_resumption_psks + ); + + // and the rest of the parameters + let group_context = alice_group.export_group_context(); + assert_eq!(alice_group.group_id(), &test_group_id); + let external_senders = group_context + .extensions() + .external_senders() + .expect("error getting external senders"); + assert_eq!(external_senders, &test_external_senders); + let crypto_config = CryptoConfig { + ciphersuite, + version: group_context.protocol_version(), + }; + assert_eq!(crypto_config, test_crypto_config); + let lifetime = alice_group + .own_leaf() + .expect("error getting own leaf") + .life_time() + .expect("leaf doesn't have a lifetime"); + assert_eq!(lifetime, &test_lifetime); +} diff --git a/openmls/src/group/public_group/tests.rs b/openmls/src/group/public_group/tests.rs index a83230d00e..d4dc01ce61 100644 --- a/openmls/src/group/public_group/tests.rs +++ b/openmls/src/group/public_group/tests.rs @@ -11,7 +11,7 @@ use crate::{ }, group::{ config::CryptoConfig, test_core_group::setup_client, GroupId, MlsGroup, - MlsGroupConfigBuilder, ProposalStore, StagedCommit, PURE_PLAINTEXT_WIRE_FORMAT_POLICY, + MlsGroupCreateConfig, ProposalStore, StagedCommit, PURE_PLAINTEXT_WIRE_FORMAT_POLICY, }, messages::proposals::Proposal, }; @@ -31,7 +31,7 @@ fn public_group(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { // Define the MlsGroup configuration // Set plaintext wire format policy s.t. the public group can track changes. - let mls_group_config = MlsGroupConfigBuilder::new() + let mls_group_create_config = MlsGroupCreateConfig::builder() .wire_format_policy(PURE_PLAINTEXT_WIRE_FORMAT_POLICY) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -40,7 +40,7 @@ fn public_group(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential_with_key, ) @@ -95,7 +95,7 @@ fn public_group(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let mut bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), Some(alice_group.export_ratchet_tree().into()), ) @@ -139,7 +139,7 @@ fn public_group(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let mut charlie_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), Some(bob_group.export_ratchet_tree().into()), ) diff --git a/openmls/src/group/tests/external_add_proposal.rs b/openmls/src/group/tests/external_add_proposal.rs index 40e2ad5332..6dbc55ae39 100644 --- a/openmls/src/group/tests/external_add_proposal.rs +++ b/openmls/src/group/tests/external_add_proposal.rs @@ -36,7 +36,7 @@ fn new_test_group( generate_credential_with_key(identity.into(), ciphersuite.signature_algorithm(), provider); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::builder() + let mls_group_create_config = MlsGroupCreateConfig::builder() .wire_format_policy(wire_format_policy) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -45,7 +45,7 @@ fn new_test_group( MlsGroup::new_with_group_id( provider, &credential_with_keys.signer, - &mls_group_config, + &mls_group_create_config, group_id, credential_with_keys.credential_with_key.clone(), ) @@ -83,9 +83,8 @@ fn validation_test_setup( .expect("error merging pending commit"); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::builder() + let mls_group_config = MlsGroupJoinConfig::builder() .wire_format_policy(wire_format_policy) - .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); let bob_group = MlsGroup::new_from_welcome( @@ -194,13 +193,12 @@ fn external_add_proposal_should_succeed(ciphersuite: Ciphersuite, provider: &imp assert_eq!(bob_group.members().count(), 3); // Finally, Charlie can join with the Welcome - let cfg = MlsGroupConfig::builder() + let mls_group_config = MlsGroupJoinConfig::builder() .wire_format_policy(policy) - .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); let charlie_group = MlsGroup::new_from_welcome( provider, - &cfg, + &mls_group_config, welcome.unwrap().into_welcome().unwrap(), Some(alice_group.export_ratchet_tree().into()), ) diff --git a/openmls/src/group/tests/external_remove_proposal.rs b/openmls/src/group/tests/external_remove_proposal.rs index 24da3f2d9a..bb057e4e35 100644 --- a/openmls/src/group/tests/external_remove_proposal.rs +++ b/openmls/src/group/tests/external_remove_proposal.rs @@ -27,7 +27,7 @@ fn new_test_group( generate_credential_with_key(identity.into(), ciphersuite.signature_algorithm(), provider); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::builder() + let mls_group_config = MlsGroupCreateConfig::builder() .wire_format_policy(wire_format_policy) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .external_senders(external_senders) diff --git a/openmls/src/group/tests/test_commit_validation.rs b/openmls/src/group/tests/test_commit_validation.rs index 8fed69bad1..2b734e6602 100644 --- a/openmls/src/group/tests/test_commit_validation.rs +++ b/openmls/src/group/tests/test_commit_validation.rs @@ -64,7 +64,7 @@ fn validation_test_setup( // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::builder() + let mls_group_create_config = MlsGroupCreateConfig::builder() .wire_format_policy(wire_format_policy) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -73,7 +73,7 @@ fn validation_test_setup( let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_credential.signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential.credential_with_key.clone(), ) @@ -95,7 +95,7 @@ fn validation_test_setup( let bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.clone(), Some(alice_group.export_ratchet_tree().into()), ) @@ -103,7 +103,7 @@ fn validation_test_setup( let charlie_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome, Some(alice_group.export_ratchet_tree().into()), ) diff --git a/openmls/src/group/tests/test_external_commit_validation.rs b/openmls/src/group/tests/test_external_commit_validation.rs index d0c2de87a4..8071501ebb 100644 --- a/openmls/src/group/tests/test_external_commit_validation.rs +++ b/openmls/src/group/tests/test_external_commit_validation.rs @@ -753,7 +753,7 @@ mod utils { group::{ config::CryptoConfig, tests::utils::{generate_credential_with_key, CredentialWithKeyAndSigner}, - MlsGroup, MlsGroupConfig, WireFormatPolicy, + MlsGroup, MlsGroupCreateConfig, WireFormatPolicy, }, }; @@ -783,7 +783,7 @@ mod utils { generate_credential_with_key("Bob".into(), ciphersuite.signature_algorithm(), provider); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::builder() + let mls_group_create_config = MlsGroupCreateConfig::builder() .wire_format_policy(wire_format_policy) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -792,7 +792,7 @@ mod utils { let alice_group = MlsGroup::new( provider, &alice_credential.signer, - &mls_group_config, + &mls_group_create_config, alice_credential.credential_with_key.clone(), ) .unwrap(); diff --git a/openmls/src/group/tests/test_framing_validation.rs b/openmls/src/group/tests/test_framing_validation.rs index b217a72bd9..cbc63d6664 100644 --- a/openmls/src/group/tests/test_framing_validation.rs +++ b/openmls/src/group/tests/test_framing_validation.rs @@ -60,7 +60,7 @@ fn validation_test_setup( ); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::builder() + let mls_group_create_config = MlsGroupCreateConfig::builder() .wire_format_policy(wire_format_policy) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -69,7 +69,7 @@ fn validation_test_setup( let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_credential.signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential.credential_with_key.clone(), ) @@ -90,7 +90,7 @@ fn validation_test_setup( let bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), Some(alice_group.export_ratchet_tree().into()), ) diff --git a/openmls/src/group/tests/test_past_secrets.rs b/openmls/src/group/tests/test_past_secrets.rs index dc33390996..6586b37dcb 100644 --- a/openmls/src/group/tests/test_past_secrets.rs +++ b/openmls/src/group/tests/test_past_secrets.rs @@ -40,7 +40,7 @@ fn test_past_secrets_in_group(ciphersuite: Ciphersuite, provider: &impl OpenMlsP // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::builder() + let mls_group_create_config = MlsGroupCreateConfig::builder() .max_past_epochs(max_epochs / 2) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -49,7 +49,7 @@ fn test_past_secrets_in_group(ciphersuite: Ciphersuite, provider: &impl OpenMlsP let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_credential_with_keys.signer, - &mls_group_config, + &mls_group_create_config, group_id.clone(), alice_credential_with_keys.credential_with_key.clone(), ) @@ -70,7 +70,7 @@ fn test_past_secrets_in_group(ciphersuite: Ciphersuite, provider: &impl OpenMlsP let mut bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), Some(alice_group.export_ratchet_tree().into()), ) diff --git a/openmls/src/group/tests/test_proposal_validation.rs b/openmls/src/group/tests/test_proposal_validation.rs index da6b902850..12a4bf6987 100644 --- a/openmls/src/group/tests/test_proposal_validation.rs +++ b/openmls/src/group/tests/test_proposal_validation.rs @@ -61,7 +61,7 @@ fn create_group_with_members( let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_credential_with_key_and_signer.signer, - &MlsGroupConfigBuilder::new() + &MlsGroupCreateConfig::builder() .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(), GroupId::from_slice(b"Alice's Friends"), @@ -106,7 +106,7 @@ fn new_test_group( generate_credential_with_key(identity.into(), ciphersuite.signature_algorithm(), provider); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::builder() + let mls_group_create_config = MlsGroupCreateConfig::builder() .wire_format_policy(wire_format_policy) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -115,7 +115,7 @@ fn new_test_group( MlsGroup::new_with_group_id( provider, &credential_with_key_and_signer.signer, - &mls_group_config, + &mls_group_create_config, group_id, credential_with_key_and_signer.credential_with_key.clone(), ) @@ -155,9 +155,8 @@ fn validation_test_setup( alice_group.merge_pending_commit(provider).unwrap(); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::builder() + let mls_group_config = MlsGroupJoinConfig::builder() .wire_format_policy(wire_format_policy) - .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); let bob_group = MlsGroup::new_from_welcome( @@ -611,7 +610,7 @@ fn test_valsem101b(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_credential_with_key.signer, - &MlsGroupConfigBuilder::new() + &MlsGroupCreateConfig::builder() .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(), GroupId::from_slice(b"Alice's Friends"), diff --git a/openmls/src/group/tests/test_remove_operation.rs b/openmls/src/group/tests/test_remove_operation.rs index 213026ef36..dc8c148f70 100644 --- a/openmls/src/group/tests/test_remove_operation.rs +++ b/openmls/src/group/tests/test_remove_operation.rs @@ -61,7 +61,7 @@ fn test_remove_operation_variants(ciphersuite: Ciphersuite, provider: &impl Open ); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfigBuilder::new() + let mls_group_create_config = MlsGroupCreateConfig::builder() .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -69,7 +69,7 @@ fn test_remove_operation_variants(ciphersuite: Ciphersuite, provider: &impl Open let mut alice_group = MlsGroup::new_with_group_id( &alice_provider, &alice_credential_with_key_and_signer.signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential_with_key_and_signer.credential_with_key, ) @@ -92,7 +92,7 @@ fn test_remove_operation_variants(ciphersuite: Ciphersuite, provider: &impl Open let mut bob_group = MlsGroup::new_from_welcome( &bob_provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.clone(), Some(alice_group.export_ratchet_tree().into()), ) @@ -100,7 +100,7 @@ fn test_remove_operation_variants(ciphersuite: Ciphersuite, provider: &impl Open let mut charlie_group = MlsGroup::new_from_welcome( &charlie_provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome, Some(alice_group.export_ratchet_tree().into()), ) diff --git a/openmls/src/group/tests/test_wire_format_policy.rs b/openmls/src/group/tests/test_wire_format_policy.rs index 69d20ee9d4..c1acfa16bd 100644 --- a/openmls/src/group/tests/test_wire_format_policy.rs +++ b/openmls/src/group/tests/test_wire_format_policy.rs @@ -28,7 +28,7 @@ fn create_group( generate_credential_with_key("Alice".into(), ciphersuite.signature_algorithm(), provider); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::builder() + let mls_group_config = MlsGroupCreateConfig::builder() .wire_format_policy(wire_format_policy) .use_ratchet_tree_extension(true) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) @@ -74,9 +74,8 @@ fn receive_message( .merge_pending_commit(provider) .expect("error merging pending commit"); - let mls_group_config = MlsGroupConfig::builder() + let mls_group_config = MlsGroupJoinConfig::builder() .wire_format_policy(alice_group.configuration().wire_format_policy()) - .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); let mut bob_group = MlsGroup::new_from_welcome( diff --git a/openmls/src/lib.rs b/openmls/src/lib.rs index 1a9438dd02..1d3a2b4414 100644 --- a/openmls/src/lib.rs +++ b/openmls/src/lib.rs @@ -88,7 +88,7 @@ //! let mut sasha_group = MlsGroup::new( //! provider, //! &sasha_signer, -//! &MlsGroupConfig::default(), +//! &MlsGroupCreateConfig::default(), //! sasha_credential_with_key, //! ) //! .expect("An unexpected error occurred."); @@ -124,7 +124,7 @@ //! // Now Maxim can join the group. //! let mut maxim_group = MlsGroup::new_from_welcome( //! provider, -//! &MlsGroupConfig::default(), +//! &MlsGroupJoinConfig::default(), //! welcome, //! // The public tree is need and transferred out of band. //! // It is also possible to use the [`RatchetTreeExtension`] diff --git a/openmls/src/messages/tests/test_welcome.rs b/openmls/src/messages/tests/test_welcome.rs index 0371b5b848..0eeea55d9b 100644 --- a/openmls/src/messages/tests/test_welcome.rs +++ b/openmls/src/messages/tests/test_welcome.rs @@ -15,7 +15,7 @@ use crate::{ extensions::Extensions, group::{ config::CryptoConfig, errors::WelcomeError, GroupContext, GroupId, MlsGroup, - MlsGroupConfigBuilder, + MlsGroupCreateConfig, }, messages::{ group_info::{GroupInfoTBS, VerifiableGroupInfo}, @@ -48,7 +48,7 @@ fn test_welcome_context_mismatch(ciphersuite: Ciphersuite, provider: &impl OpenM }; let group_id = GroupId::random(provider.rand()); - let mls_group_config = MlsGroupConfigBuilder::new() + let mls_group_create_config = MlsGroupCreateConfig::builder() .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -64,7 +64,7 @@ fn test_welcome_context_mismatch(ciphersuite: Ciphersuite, provider: &impl OpenM let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential_with_key, ) @@ -163,7 +163,7 @@ fn test_welcome_context_mismatch(ciphersuite: Ciphersuite, provider: &impl OpenM // Bob tries to join the group let err = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome, Some(alice_group.export_ratchet_tree().into()), ) @@ -196,7 +196,7 @@ fn test_welcome_context_mismatch(ciphersuite: Ciphersuite, provider: &impl OpenM let _group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), original_welcome, Some(alice_group.export_ratchet_tree().into()), ) diff --git a/openmls/src/test_utils/test_framework/client.rs b/openmls/src/test_utils/test_framework/client.rs index bcbe185b82..417ebfe9d6 100644 --- a/openmls/src/test_utils/test_framework/client.rs +++ b/openmls/src/test_utils/test_framework/client.rs @@ -79,12 +79,12 @@ impl Client { Ok(key_package) } - /// Create a group with the given [MlsGroupConfig] and [Ciphersuite], and return the created [GroupId]. + /// Create a group with the given [MlsGroupCreateConfig] and [Ciphersuite], and return the created [GroupId]. /// /// Returns an error if the client doesn't support the `ciphersuite`. pub fn create_group( &self, - mls_group_config: MlsGroupConfig, + mls_group_create_config: MlsGroupCreateConfig, ciphersuite: Ciphersuite, ) -> Result { let credential_with_key = self @@ -101,7 +101,7 @@ impl Client { let group_state = MlsGroup::new( &self.crypto, &signer, - &mls_group_config, + &mls_group_create_config, credential_with_key.clone(), )?; let group_id = group_state.group_id().clone(); @@ -113,12 +113,12 @@ impl Client { } /// Join a group based on the given `welcome` and `ratchet_tree`. The group - /// is created with the given `MlsGroupConfig`. Throws an error if no + /// is created with the given `MlsGroupCreateConfig`. Throws an error if no /// `KeyPackage` exists matching the `Welcome`, if the client doesn't /// support the ciphersuite, or if an error occurs processing the `Welcome`. pub fn join_group( &self, - mls_group_config: MlsGroupConfig, + mls_group_config: MlsGroupJoinConfig, welcome: Welcome, ratchet_tree: Option, ) -> Result<(), ClientError> { diff --git a/openmls/src/test_utils/test_framework/mod.rs b/openmls/src/test_utils/test_framework/mod.rs index c4d0b6f47c..1e83be5e9b 100644 --- a/openmls/src/test_utils/test_framework/mod.rs +++ b/openmls/src/test_utils/test_framework/mod.rs @@ -60,7 +60,7 @@ pub struct Group { pub group_id: GroupId, pub members: Vec<(usize, Vec)>, pub ciphersuite: Ciphersuite, - pub group_config: MlsGroupConfig, + pub group_config: MlsGroupJoinConfig, pub public_tree: RatchetTree, pub exporter_secret: Vec, } @@ -108,7 +108,7 @@ pub struct MlsGroupTestSetup { pub groups: RwLock>, // This maps key package hashes to client ids. pub waiting_for_welcome: RwLock, Vec>>, - pub default_mgc: MlsGroupConfig, + pub default_mgp: MlsGroupCreateConfig, /// Flag to indicate if messages should be serialized and de-serialized as /// part of message distribution pub use_codec: CodecUse, @@ -132,10 +132,14 @@ pub struct MlsGroupTestSetup { impl MlsGroupTestSetup { /// Create a new `MlsGroupTestSetup` with the given default - /// `MlsGroupConfig` and the given number of clients. For lifetime + /// `MlsGroupCreateConfig` and the given number of clients. For lifetime /// reasons, `create_clients` has to be called in addition with the same /// number of clients. - pub fn new(default_mgc: MlsGroupConfig, number_of_clients: usize, use_codec: CodecUse) -> Self { + pub fn new( + default_mgp: MlsGroupCreateConfig, + number_of_clients: usize, + use_codec: CodecUse, + ) -> Self { let mut clients = HashMap::new(); for i in 0..number_of_clients { let identity = i.to_be_bytes().to_vec(); @@ -175,7 +179,7 @@ impl MlsGroupTestSetup { clients: RwLock::new(clients), groups, waiting_for_welcome, - default_mgc, + default_mgp, use_codec, } } @@ -458,7 +462,7 @@ impl MlsGroupTestSetup { .read() .expect("An unexpected error occurred."); let mut groups = self.groups.write().expect("An unexpected error occurred."); - let group_id = group_creator.create_group(self.default_mgc.clone(), ciphersuite)?; + let group_id = group_creator.create_group(self.default_mgp.clone(), ciphersuite)?; let creator_groups = group_creator .groups .read() @@ -474,7 +478,7 @@ impl MlsGroupTestSetup { group_id: group_id.clone(), members: member_ids, ciphersuite, - group_config: self.default_mgc.clone(), + group_config: self.default_mgp.join_config.clone(), public_tree, exporter_secret, }; diff --git a/openmls/src/treesync/tests_and_kats/tests.rs b/openmls/src/treesync/tests_and_kats/tests.rs index 57d35802d7..dae428b523 100644 --- a/openmls/src/treesync/tests_and_kats/tests.rs +++ b/openmls/src/treesync/tests_and_kats/tests.rs @@ -3,7 +3,7 @@ use openmls_rust_crypto::OpenMlsRustCrypto; use crate::{ group::{ tests::utils::{generate_credential_with_key, CredentialWithKeyAndSigner}, - MlsGroup, MlsGroupConfig, + MlsGroup, MlsGroupCreateConfig, }, key_packages::KeyPackage, prelude::*, @@ -22,7 +22,7 @@ fn that_commit_secret_is_derived_from_end_of_update_path_not_root( ) { let _ = provider; // get rid of warning let crypto_config = CryptoConfig::with_default_version(ciphersuite); - let mls_group_config = MlsGroupConfig::builder() + let mls_group_create_config = MlsGroupCreateConfig::builder() .crypto_config(crypto_config) .use_ratchet_tree_extension(true) .build(); @@ -94,7 +94,7 @@ fn that_commit_secret_is_derived_from_end_of_update_path_not_root( let mut alice_group = MlsGroup::new( &alice.provider, &alice.credential_with_key_and_signer.signer, - &mls_group_config, + &mls_group_create_config, alice .credential_with_key_and_signer .credential_with_key @@ -120,7 +120,7 @@ fn that_commit_secret_is_derived_from_end_of_update_path_not_root( let mut charlie_group = { MlsGroup::new_from_welcome( &charlie.provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().unwrap(), None, ) diff --git a/openmls/tests/book_code.rs b/openmls/tests/book_code.rs index 94ac69e5d1..3ac0574b8e 100644 --- a/openmls/tests/book_code.rs +++ b/openmls/tests/book_code.rs @@ -126,27 +126,27 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { provider, ); - // ANCHOR: mls_group_config_example - let mls_group_config = MlsGroupConfig::builder() + // ANCHOR: mls_group_create_config_example + let mls_group_create_config = MlsGroupCreateConfig::builder() .padding_size(100) .sender_ratchet_configuration(SenderRatchetConfiguration::new( 10, // out_of_order_tolerance 2000, // maximum_forward_distance )) .external_senders(vec![ExternalSender::new( - ds_credential_with_key.signature_key, - ds_credential_with_key.credential, + ds_credential_with_key.signature_key.clone(), + ds_credential_with_key.credential.clone(), )]) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .use_ratchet_tree_extension(true) .build(); - // ANCHOR_END: mls_group_config_example + // ANCHOR_END: mls_group_create_config_example // ANCHOR: alice_create_group let mut alice_group = MlsGroup::new( provider, &alice_signature_keys, - &mls_group_config, + &mls_group_create_config, alice_credential.clone(), ) .expect("An unexpected error occurred."); @@ -160,7 +160,7 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_signature_keys, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential.clone(), ) @@ -169,6 +169,26 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { // Silence "unused variable" and "does not need to be mutable" warnings. let _ignore_mut_warning = &mut alice_group; + + // ANCHOR: alice_create_group_with_builder + let mut alice_group = MlsGroup::builder() + .padding_size(100) + .sender_ratchet_configuration(SenderRatchetConfiguration::new( + 10, // out_of_order_tolerance + 2000, // maximum_forward_distance + )) + .external_senders(vec![ExternalSender::new( + ds_credential_with_key.signature_key, + ds_credential_with_key.credential, + )]) + .crypto_config(CryptoConfig::with_default_version(ciphersuite)) + .use_ratchet_tree_extension(true) + .build(provider, &alice_signature_keys, alice_credential.clone()) + .expect("An unexpected error occurred."); + // ANCHOR_END: alice_create_group_with_builder + + // Silence "unused variable" and "does not need to be mutable" warnings. + let _ignore_mut_warning = &mut alice_group; } let group_id = alice_group.group_id().clone(); @@ -216,6 +236,17 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { assert_eq!(members[0].credential.identity(), b"Alice"); assert_eq!(members[1].credential.identity(), b"Bob"); + // ANCHOR: mls_group_config_example + let mls_group_config = MlsGroupJoinConfig::builder() + .padding_size(100) + .sender_ratchet_configuration(SenderRatchetConfiguration::new( + 10, // out_of_order_tolerance + 2000, // maximum_forward_distance + )) + .use_ratchet_tree_extension(true) + .build(); + // ANCHOR_END: mls_group_config_example + // ANCHOR: bob_joins_with_welcome let mut bob_group = MlsGroup::new_from_welcome( provider, @@ -479,7 +510,7 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let mut charlie_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), Some(bob_group.export_ratchet_tree().into()), ) @@ -911,7 +942,7 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { // Bob creates a new group let mut bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome_option .expect("Welcome was not returned") .into_welcome() @@ -1141,7 +1172,7 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome .unwrap() .into_welcome() @@ -1226,7 +1257,7 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let mut bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), Some(alice_group.export_ratchet_tree().into()), ) @@ -1271,7 +1302,7 @@ fn test_empty_input_errors(ciphersuite: Ciphersuite, provider: &impl OpenMlsProv ); // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + let mls_group_config = MlsGroupCreateConfig::test_default(ciphersuite); // === Alice creates a group === let mut alice_group = MlsGroup::new_with_group_id( diff --git a/openmls/tests/test_decryption_key_index.rs b/openmls/tests/test_decryption_key_index.rs index 691fa49205..c3e4752d1e 100644 --- a/openmls/tests/test_decryption_key_index.rs +++ b/openmls/tests/test_decryption_key_index.rs @@ -13,10 +13,10 @@ fn decryption_key_index_computation(ciphersuite: Ciphersuite) { println!("Testing ciphersuite {ciphersuite:?}"); // Some basic setup functions for the MlsGroup. - let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + let mls_group_create_config = MlsGroupCreateConfig::test_default(ciphersuite); let number_of_clients = 20; let setup = MlsGroupTestSetup::new( - mls_group_config, + mls_group_create_config, number_of_clients, CodecUse::StructMessages, ); diff --git a/openmls/tests/test_external_commit.rs b/openmls/tests/test_external_commit.rs index ec404c6c34..8a0967156d 100644 --- a/openmls/tests/test_external_commit.rs +++ b/openmls/tests/test_external_commit.rs @@ -9,7 +9,7 @@ fn create_alice_group( provider: &impl OpenMlsProvider, use_ratchet_tree_extension: bool, ) -> (MlsGroup, CredentialWithKey, SignatureKeyPair) { - let group_config = MlsGroupConfigBuilder::new() + let group_config = MlsGroupCreateConfig::builder() .use_ratchet_tree_extension(use_ratchet_tree_extension) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -88,9 +88,7 @@ fn test_external_commit(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvide &bob_signature_keys, None, verifiable_group_info, - &MlsGroupConfigBuilder::new() - .crypto_config(CryptoConfig::with_default_version(ciphersuite)) - .build(), + &MlsGroupJoinConfig::default(), b"", bob_credential, ) @@ -111,9 +109,7 @@ fn test_external_commit(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvide &bob_signature_keys, None, verifiable_group_info_broken, - &MlsGroupConfigBuilder::new() - .crypto_config(CryptoConfig::with_default_version(ciphersuite)) - .build(), + &MlsGroupJoinConfig::default(), b"", bob_credential, ) @@ -155,9 +151,7 @@ fn test_group_info(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { &bob_signature_keys, None, verifiable_group_info, - &MlsGroupConfigBuilder::new() - .crypto_config(CryptoConfig::with_default_version(ciphersuite)) - .build(), + &MlsGroupJoinConfig::default(), b"", bob_credential, ) @@ -208,9 +202,7 @@ fn test_group_info(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { &bob_signature_keys, None, verifiable_group_info, - &MlsGroupConfigBuilder::new() - .crypto_config(CryptoConfig::with_default_version(ciphersuite)) - .build(), + &MlsGroupJoinConfig::default(), b"", bob_credential, ) diff --git a/openmls/tests/test_interop_scenarios.rs b/openmls/tests/test_interop_scenarios.rs index 2602de215c..6e54699445 100644 --- a/openmls/tests/test_interop_scenarios.rs +++ b/openmls/tests/test_interop_scenarios.rs @@ -22,7 +22,7 @@ fn one_to_one_join(ciphersuite: Ciphersuite) { println!("Testing ciphersuite {ciphersuite:?}"); let number_of_clients = 2; let setup = MlsGroupTestSetup::new( - MlsGroupConfig::test_default(ciphersuite), + MlsGroupCreateConfig::test_default(ciphersuite), number_of_clients, CodecUse::StructMessages, ); @@ -74,7 +74,7 @@ fn three_party_join(ciphersuite: Ciphersuite) { let number_of_clients = 3; let setup = MlsGroupTestSetup::new( - MlsGroupConfig::test_default(ciphersuite), + MlsGroupCreateConfig::test_default(ciphersuite), number_of_clients, CodecUse::StructMessages, ); @@ -141,7 +141,7 @@ fn multiple_joins(ciphersuite: Ciphersuite) { let number_of_clients = 3; let setup = MlsGroupTestSetup::new( - MlsGroupConfig::test_default(ciphersuite), + MlsGroupCreateConfig::test_default(ciphersuite), number_of_clients, CodecUse::StructMessages, ); @@ -194,7 +194,7 @@ fn update(ciphersuite: Ciphersuite) { let number_of_clients = 2; let setup = MlsGroupTestSetup::new( - MlsGroupConfig::test_default(ciphersuite), + MlsGroupCreateConfig::test_default(ciphersuite), number_of_clients, CodecUse::StructMessages, ); @@ -242,7 +242,7 @@ fn remove(ciphersuite: Ciphersuite) { let number_of_clients = 2; let setup = MlsGroupTestSetup::new( - MlsGroupConfig::test_default(ciphersuite), + MlsGroupCreateConfig::test_default(ciphersuite), number_of_clients, CodecUse::StructMessages, ); @@ -298,7 +298,7 @@ fn large_group_lifecycle(ciphersuite: Ciphersuite) { // "Large" is 20 for now. let number_of_clients = 20; let setup = MlsGroupTestSetup::new( - MlsGroupConfig::test_default(ciphersuite), + MlsGroupCreateConfig::test_default(ciphersuite), number_of_clients, CodecUse::StructMessages, ); diff --git a/openmls/tests/test_managed_api.rs b/openmls/tests/test_managed_api.rs index 44c8e81914..089f242b09 100644 --- a/openmls/tests/test_managed_api.rs +++ b/openmls/tests/test_managed_api.rs @@ -10,10 +10,10 @@ use openmls::{ #[apply(ciphersuites)] fn test_mls_group_api(ciphersuite: Ciphersuite) { // Some basic setup functions for the MlsGroup. - let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + let mls_group_create_config = MlsGroupCreateConfig::test_default(ciphersuite); let number_of_clients = 20; let setup = MlsGroupTestSetup::new( - mls_group_config, + mls_group_create_config, number_of_clients, CodecUse::SerializedMessages, ); diff --git a/openmls/tests/test_mls_group.rs b/openmls/tests/test_mls_group.rs index edd25906e0..e7b6db6d47 100644 --- a/openmls/tests/test_mls_group.rs +++ b/openmls/tests/test_mls_group.rs @@ -79,7 +79,7 @@ fn mls_group_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvide // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::builder() + let mls_group_create_config = MlsGroupCreateConfig::builder() .wire_format_policy(*wire_format_policy) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -88,7 +88,7 @@ fn mls_group_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvide let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_signer, - &mls_group_config, + &mls_group_create_config, group_id.clone(), alice_credential.clone(), ) @@ -133,7 +133,7 @@ fn mls_group_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvide let mut bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), Some(alice_group.export_ratchet_tree().into()), ) @@ -349,7 +349,7 @@ fn mls_group_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvide let mut charlie_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), Some(bob_group.export_ratchet_tree().into()), ) @@ -700,7 +700,7 @@ fn mls_group_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvide // Bob creates a new group let mut bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome_option .expect("Welcome was not returned") .into_welcome() @@ -892,7 +892,7 @@ fn mls_group_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvide let mut bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), Some(alice_group.export_ratchet_tree().into()), ) @@ -968,7 +968,7 @@ fn addition_order(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::builder() + let mls_group_config = MlsGroupCreateConfig::builder() .wire_format_policy(*wire_format_policy) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .build(); @@ -1047,14 +1047,14 @@ fn test_empty_input_errors(ciphersuite: Ciphersuite, provider: &impl OpenMlsProv ciphersuite.signature_algorithm(), ); - // Define the MlsGroup configuration - let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + // Define the MlsGroupCreateConfig + let mls_group_create_config = MlsGroupCreateConfig::test_default(ciphersuite); // === Alice creates a group === let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential, ) @@ -1108,7 +1108,7 @@ fn mls_group_ratchet_tree_extension(ciphersuite: Ciphersuite, provider: &impl Op &bob_signer, ); - let mls_group_config = MlsGroupConfig::builder() + let mls_group_create_config = MlsGroupCreateConfig::builder() .wire_format_policy(*wire_format_policy) .use_ratchet_tree_extension(true) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) @@ -1118,7 +1118,7 @@ fn mls_group_ratchet_tree_extension(ciphersuite: Ciphersuite, provider: &impl Op let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_signer, - &mls_group_config, + &mls_group_create_config, group_id.clone(), alice_credential.clone(), ) @@ -1132,7 +1132,7 @@ fn mls_group_ratchet_tree_extension(ciphersuite: Ciphersuite, provider: &impl Op // === Bob joins using the ratchet tree extension === let _bob_group = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), None, ) @@ -1164,13 +1164,13 @@ fn mls_group_ratchet_tree_extension(ciphersuite: Ciphersuite, provider: &impl Op &bob_signer, ); - let mls_group_config = MlsGroupConfig::test_default(ciphersuite); + let mls_group_create_config = MlsGroupCreateConfig::test_default(ciphersuite); // === Alice creates a group === let mut alice_group = MlsGroup::new_with_group_id( provider, &alice_signer, - &mls_group_config, + &mls_group_create_config, group_id, alice_credential.clone(), ) @@ -1184,7 +1184,7 @@ fn mls_group_ratchet_tree_extension(ciphersuite: Ciphersuite, provider: &impl Op // === Bob tries to join without the ratchet tree extension === let error = MlsGroup::new_from_welcome( provider, - &mls_group_config, + mls_group_create_config.join_config(), welcome.into_welcome().expect("Unexpected message type."), None, ) From 596adef0826834ed8f07a056d4bcb84779612c77 Mon Sep 17 00:00:00 2001 From: Jan Winkelmann <146678+keks@users.noreply.github.com> Date: Thu, 11 Jan 2024 16:53:43 +0100 Subject: [PATCH 11/13] Add `with_group_context_extensions` method to group builders (#1473) Add a method with_group_context_extensions to the group builders, that sets an arbitrary set of extensions. The set is of type Extensions, so we know that each extension is only set once. Any RequiredCapability extension mentioned in that set will be overwritten, and if ExternalSenders is specified using the dedicated method, that one is preferred and the one provided using with_group_context_extensions is dropped. --- book/src/user_manual/create_group.md | 8 ++ openmls/src/extensions/errors.rs | 5 + openmls/src/extensions/test_extensions.rs | 121 +++++++++++++++++++++- openmls/src/group/core_group/mod.rs | 17 +++ openmls/src/group/mls_group/builder.rs | 16 ++- openmls/src/group/mls_group/config.rs | 32 +++++- openmls/src/group/mls_group/errors.rs | 2 +- openmls/src/group/public_group/builder.rs | 40 +++++-- openmls/tests/book_code.rs | 24 +++++ 9 files changed, 250 insertions(+), 15 deletions(-) diff --git a/book/src/user_manual/create_group.md b/book/src/user_manual/create_group.md index 8ff8f2c506..b0116b0cb5 100644 --- a/book/src/user_manual/create_group.md +++ b/book/src/user_manual/create_group.md @@ -29,3 +29,11 @@ If someone else already gave you a group ID, e.g., a provider server, you can al ```rust,no_run,noplayground {{#include ../../../openmls/tests/book_code.rs:alice_create_group_with_group_id}} ``` + +The Builder provides methods for setting required capabilities and external senders. +The information passed into these lands in the group context, in the form of extensions. +Should the user want to add further extensions, they can use the `with_group_context_extensions` method: + +```rust,no_run,noplayground +{{#include ../../../openmls/tests/book_code.rs:alice_create_group_with_builder_with_extensions}} +``` diff --git a/openmls/src/extensions/errors.rs b/openmls/src/extensions/errors.rs index 65939f0599..bdc7f9d58a 100644 --- a/openmls/src/extensions/errors.rs +++ b/openmls/src/extensions/errors.rs @@ -92,4 +92,9 @@ pub enum InvalidExtensionError { /// The specified extension could not be found. #[error("The specified extension could not be found.")] NotFound, + /// The provided extension list contains an extension that is not allowed in group contexts + #[error( + "The provided extension list contains an extension that is not allowed in group contexts." + )] + IllegalInGroupContext, } diff --git a/openmls/src/extensions/test_extensions.rs b/openmls/src/extensions/test_extensions.rs index 6badafcbe6..8fee2437bc 100644 --- a/openmls/src/extensions/test_extensions.rs +++ b/openmls/src/extensions/test_extensions.rs @@ -13,7 +13,8 @@ use crate::{ group::{config::CryptoConfig, errors::*, *}, key_packages::*, messages::proposals::ProposalType, - prelude::Capabilities, + prelude::{Capabilities, RatchetTreeIn}, + prelude_test::HpkePublicKey, schedule::psk::store::ResumptionPskStore, test_utils::*, versions::ProtocolVersion, @@ -248,6 +249,124 @@ fn required_capabilities() { assert_eq!(extension_bytes, encoded); } +#[apply(ciphersuites_and_providers)] +fn with_group_context_extensions(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { + // create an extension that we can check for later + let test_extension = Extension::Unknown(0xf023, UnknownExtension(vec![0xca, 0xfe])); + let extensions = Extensions::single(test_extension.clone()); + + let alice_credential_with_key_and_signer = tests::utils::generate_credential_with_key( + "Alice".into(), + ciphersuite.signature_algorithm(), + provider, + ); + + let mls_group_create_config = MlsGroupCreateConfig::builder() + .with_group_context_extensions(extensions) + .expect("failed to apply extensions at group config builder") + .crypto_config(CryptoConfig::with_default_version(ciphersuite)) + .build(); + + // === Alice creates a group === + let alice_group = MlsGroup::new( + provider, + &alice_credential_with_key_and_signer.signer, + &mls_group_create_config, + alice_credential_with_key_and_signer.credential_with_key, + ) + .expect("An unexpected error occurred."); + + // === Group contains extension === + let found_test_extension = alice_group + .export_group_context() + .extensions() + .find_by_type(ExtensionType::Unknown(0xf023)) + .expect("failed to get test extensions from group context"); + assert_eq!(found_test_extension, &test_extension); +} + +#[apply(ciphersuites_and_providers)] +fn wrong_extension_with_group_context_extensions( + ciphersuite: Ciphersuite, + provider: &impl OpenMlsProvider, +) { + // Extension types that are known to not be allowed here: + // - application id + // - external pub + // - ratchet tree + + let alice_credential_with_key_and_signer = tests::utils::generate_credential_with_key( + "Alice".into(), + ciphersuite.signature_algorithm(), + provider, + ); + + let crypto_config = CryptoConfig::with_default_version(ciphersuite); + + // create an extension that we can check for later + let test_extension = Extension::ApplicationId(ApplicationIdExtension::new(&[0xca, 0xfe])); + let extensions = Extensions::single(test_extension.clone()); + + let err = MlsGroup::builder() + .with_group_context_extensions(extensions.clone()) + .expect_err("builder accepted non-group-context extension"); + + assert_eq!(err, InvalidExtensionError::IllegalInGroupContext); + let err = PublicGroup::builder( + GroupId::from_slice(&[0xbe, 0xef]), + crypto_config, + alice_credential_with_key_and_signer + .credential_with_key + .clone(), + ) + .with_group_context_extensions(extensions) + .expect_err("builder accepted non-group-context extension"); + + assert_eq!(err, InvalidExtensionError::IllegalInGroupContext); + // create an extension that we can check for later + let test_extension = + Extension::ExternalPub(ExternalPubExtension::new(HpkePublicKey::new(vec![]))); + let extensions = Extensions::single(test_extension.clone()); + + let err = MlsGroup::builder() + .with_group_context_extensions(extensions.clone()) + .expect_err("builder accepted non-group-context extension"); + assert_eq!(err, InvalidExtensionError::IllegalInGroupContext); + + let err = PublicGroup::builder( + GroupId::from_slice(&[0xbe, 0xef]), + crypto_config, + alice_credential_with_key_and_signer + .credential_with_key + .clone(), + ) + .with_group_context_extensions(extensions) + .expect_err("builder accepted non-group-context extension"); + assert_eq!(err, InvalidExtensionError::IllegalInGroupContext); + + // create an extension that we can check for later + let test_extension = Extension::RatchetTree(RatchetTreeExtension::new( + RatchetTreeIn::from_nodes(vec![]).into(), + )); + let extensions = Extensions::single(test_extension.clone()); + + let err = MlsGroup::builder() + .with_group_context_extensions(extensions.clone()) + .expect_err("builder accepted non-group-context extension"); + assert_eq!(err, InvalidExtensionError::IllegalInGroupContext); + + let err = PublicGroup::builder( + GroupId::from_slice(&[0xbe, 0xef]), + crypto_config, + alice_credential_with_key_and_signer + .credential_with_key + .clone(), + ) + .with_group_context_extensions(extensions) + .expect_err("builder accepted non-group-context extension"); + assert_eq!(err, InvalidExtensionError::IllegalInGroupContext); +} + #[apply(ciphersuites_and_providers)] fn last_resort_extension(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let last_resort = Extension::LastResort(LastResortExtension::default()); diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index 5fb10ceb0e..b56b53f8b6 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -58,6 +58,7 @@ use crate::{ ciphersuite::{signable::Signable, HpkePublicKey}, credentials::*, error::LibraryError, + extensions::errors::InvalidExtensionError, framing::{mls_auth_content::AuthenticatedContent, *}, group::{config::CryptoConfig, *}, key_packages::*, @@ -200,6 +201,22 @@ impl CoreGroupBuilder { } self } + + /// Sets initial group context extensions. Note that RequiredCapabilities + /// extensions will be overwritten, and should be set using a call to + /// `required_capabilities`. If `ExternalSenders` extensions are provided + /// both in this call, and a call to `external_senders`, only the one from + /// the call to `external_senders` will be included. + pub(crate) fn with_group_context_extensions( + mut self, + extensions: Extensions, + ) -> Result { + self.public_group_builder = self + .public_group_builder + .with_group_context_extensions(extensions)?; + Ok(self) + } + /// Set the number of past epochs the group should keep secrets. pub fn with_max_past_epoch_secrets(mut self, max_past_epochs: usize) -> Self { self.max_past_epochs = max_past_epochs; diff --git a/openmls/src/group/mls_group/builder.rs b/openmls/src/group/mls_group/builder.rs index fe7040bb12..ed8ec42069 100644 --- a/openmls/src/group/mls_group/builder.rs +++ b/openmls/src/group/mls_group/builder.rs @@ -2,7 +2,7 @@ use openmls_traits::{key_store::OpenMlsKeyStore, signatures::Signer, OpenMlsProv use crate::{ credentials::CredentialWithKey, - extensions::ExternalSendersExtension, + extensions::{errors::InvalidExtensionError, Extensions, ExternalSendersExtension}, group::{ config::CryptoConfig, public_group::errors::PublicGroupBuildError, CoreGroup, CoreGroupBuildError, CoreGroupConfig, GroupId, MlsGroupCreateConfig, @@ -13,7 +13,7 @@ use crate::{ use super::{InnerState, MlsGroup, MlsGroupState}; -#[derive(Default)] +#[derive(Default, Debug)] pub struct MlsGroupBuilder { group_id: Option, mls_group_create_config_builder: MlsGroupCreateConfigBuilder, @@ -72,6 +72,7 @@ impl MlsGroupBuilder { .with_config(group_config) .with_required_capabilities(mls_group_create_config.required_capabilities.clone()) .with_external_senders(mls_group_create_config.external_senders.clone()) + .with_group_context_extensions(mls_group_create_config.group_context_extensions.clone())? .with_max_past_epoch_secrets(mls_group_create_config.join_config.max_past_epochs) .with_lifetime(*mls_group_create_config.lifetime()) .build(provider, signer) @@ -198,4 +199,15 @@ impl MlsGroupBuilder { .external_senders(external_senders); self } + + /// Sets the initial group context extensions + pub fn with_group_context_extensions( + mut self, + extensions: Extensions, + ) -> Result { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .with_group_context_extensions(extensions)?; + Ok(self) + } } diff --git a/openmls/src/group/mls_group/config.rs b/openmls/src/group/mls_group/config.rs index b7366cdf00..c270ece7d2 100644 --- a/openmls/src/group/mls_group/config.rs +++ b/openmls/src/group/mls_group/config.rs @@ -29,7 +29,7 @@ use super::*; use crate::{ - group::config::CryptoConfig, key_packages::Lifetime, + extensions::errors::InvalidExtensionError, group::config::CryptoConfig, key_packages::Lifetime, tree::sender_ratchet::SenderRatchetConfiguration, }; use serde::{Deserialize, Serialize}; @@ -93,6 +93,8 @@ pub struct MlsGroupCreateConfig { pub(crate) crypto_config: CryptoConfig, /// Configuration parameters relevant to group operation at runtime pub(crate) join_config: MlsGroupJoinConfig, + /// List of initial group context extensions + pub(crate) group_context_extensions: Extensions, } /// Builder struct for an [`MlsGroupJoinConfig`]. @@ -200,6 +202,13 @@ impl MlsGroupCreateConfig { &self.external_senders } + /// Returns the [`Extensions`] set as the initial group context. + /// This does not contain the initial group context extensions + /// added from builder calls to `external_senders` or `required_capabilities`. + pub fn group_context_extensions(&self) -> &Extensions { + &self.group_context_extensions + } + /// Returns the [`MlsGroupCreateConfig`] lifetime configuration. pub fn lifetime(&self) -> &Lifetime { &self.lifetime @@ -228,7 +237,7 @@ impl MlsGroupCreateConfig { } /// Builder for an [`MlsGroupCreateConfig`]. -#[derive(Default)] +#[derive(Default, Debug)] pub struct MlsGroupCreateConfigBuilder { config: MlsGroupCreateConfig, } @@ -317,6 +326,25 @@ impl MlsGroupCreateConfigBuilder { self } + /// Sets initial group context extensions. Note that RequiredCapabilities + /// extensions will be overwritten, and should be set using a call to + /// `required_capabilities`. If `ExternalSenders` extensions are provided + /// both in this call, and a call to `external_senders`, only the one from + /// the call to `external_senders` will be included. + pub fn with_group_context_extensions( + mut self, + extensions: Extensions, + ) -> Result { + let is_valid_in_group_context = extensions.application_id().is_none() + && extensions.ratchet_tree().is_none() + && extensions.external_pub().is_none(); + if !is_valid_in_group_context { + return Err(InvalidExtensionError::IllegalInGroupContext); + } + self.config.group_context_extensions = extensions; + Ok(self) + } + /// Finalizes the builder and retursn an `[MlsGroupCreateConfig`]. pub fn build(self) -> MlsGroupCreateConfig { self.config diff --git a/openmls/src/group/mls_group/errors.rs b/openmls/src/group/mls_group/errors.rs index 906f6d1d0e..a23e47667e 100644 --- a/openmls/src/group/mls_group/errors.rs +++ b/openmls/src/group/mls_group/errors.rs @@ -38,7 +38,7 @@ pub enum NewGroupError { UnsupportedExtensionType, /// Invalid extensions set in configuration #[error("Invalid extensions set in configuration")] - InvalidExtensions(InvalidExtensionError), + InvalidExtensions(#[from] InvalidExtensionError), } /// EmptyInput error diff --git a/openmls/src/group/public_group/builder.rs b/openmls/src/group/public_group/builder.rs index aac7ac329f..3da16bdf98 100644 --- a/openmls/src/group/public_group/builder.rs +++ b/openmls/src/group/public_group/builder.rs @@ -5,8 +5,8 @@ use crate::{ credentials::CredentialWithKey, error::LibraryError, extensions::{ - errors::ExtensionError, Extension, Extensions, ExternalSendersExtension, - RequiredCapabilitiesExtension, + errors::{ExtensionError, InvalidExtensionError}, + Extension, Extensions, ExternalSendersExtension, RequiredCapabilitiesExtension, }, group::{config::CryptoConfig, GroupContext, GroupId}, key_packages::Lifetime, @@ -18,6 +18,7 @@ use crate::{ }, }; +#[derive(Debug)] pub(crate) struct TempBuilderPG1 { group_id: GroupId, crypto_config: CryptoConfig, @@ -26,6 +27,7 @@ pub(crate) struct TempBuilderPG1 { required_capabilities: Option, external_senders: Option, leaf_extensions: Option, + group_context_extensions: Option, } impl TempBuilderPG1 { @@ -52,6 +54,20 @@ impl TempBuilderPG1 { self } + pub(crate) fn with_group_context_extensions( + mut self, + extensions: Extensions, + ) -> Result { + let is_valid_in_group_context = extensions.application_id().is_none() + && extensions.ratchet_tree().is_none() + && extensions.external_pub().is_none(); + if !is_valid_in_group_context { + return Err(InvalidExtensionError::IllegalInGroupContext); + } + self.group_context_extensions = Some(extensions); + Ok(self) + } + pub(crate) fn get_secrets( self, provider: &impl OpenMlsProvider, @@ -87,17 +103,22 @@ impl TempBuilderPG1 { _ => LibraryError::custom("Unexpected ExtensionError").into(), })?; let required_capabilities = Extension::RequiredCapabilities(required_capabilities); - let extensions = - if let Some(ext_senders) = self.external_senders.map(Extension::ExternalSenders) { - vec![required_capabilities, ext_senders] - } else { - vec![required_capabilities] - }; + + let mut group_context_extensions = if let Some(exts) = self.group_context_extensions { + exts + } else { + Extensions::empty() + }; + group_context_extensions.add_or_replace(required_capabilities); + if let Some(ext_senders) = self.external_senders { + group_context_extensions.add_or_replace(Extension::ExternalSenders(ext_senders)); + } + let group_context = GroupContext::create_initial_group_context( self.crypto_config.ciphersuite, self.group_id, treesync.tree_hash().to_vec(), - Extensions::from_vec(extensions)?, + group_context_extensions, ); let next_builder = TempBuilderPG2 { treesync, @@ -172,6 +193,7 @@ impl PublicGroup { required_capabilities: None, external_senders: None, leaf_extensions: None, + group_context_extensions: None, } } } diff --git a/openmls/tests/book_code.rs b/openmls/tests/book_code.rs index 3ac0574b8e..b6b8c1399b 100644 --- a/openmls/tests/book_code.rs +++ b/openmls/tests/book_code.rs @@ -170,6 +170,30 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { // Silence "unused variable" and "does not need to be mutable" warnings. let _ignore_mut_warning = &mut alice_group; + let external_senders_list = vec![]; + + // ANCHOR: alice_create_group_with_builder_with_extensions + // we are adding an external senders list as an example. + let extensions = + Extensions::from_vec(vec![Extension::ExternalSenders(external_senders_list)]) + .expect("failed to create extensions list"); + + let mut alice_group = MlsGroup::builder() + .padding_size(100) + .sender_ratchet_configuration(SenderRatchetConfiguration::new( + 10, // out_of_order_tolerance + 2000, // maximum_forward_distance + )) + .with_group_context_extensions(extensions) // NB: the builder method returns a Result + .expect("failed to apply group context extensions") + .use_ratchet_tree_extension(true) + .build(provider, &alice_signature_keys, alice_credential.clone()) + .expect("An unexpected error occurred."); + // ANCHOR_END: alice_create_group_with_builder_with_extensions + + // Silence "unused variable" and "does not need to be mutable" warnings. + let _ignore_mut_warning = &mut alice_group; + // ANCHOR: alice_create_group_with_builder let mut alice_group = MlsGroup::builder() .padding_size(100) From d533eff6e9bbb5ac7e6804155a37bc01841e9ae2 Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Fri, 12 Jan 2024 09:39:37 -0800 Subject: [PATCH 12/13] Handle change in Verifiable trait --- openmls/src/extensions/protected_metadata.rs | 27 ++++++++++++-------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/openmls/src/extensions/protected_metadata.rs b/openmls/src/extensions/protected_metadata.rs index c87b79bad3..617e9a598e 100644 --- a/openmls/src/extensions/protected_metadata.rs +++ b/openmls/src/extensions/protected_metadata.rs @@ -156,9 +156,15 @@ impl Signable for ProtectedMetadataTbs { /// XXX: This really should not be implemented on [`ProtectedMetadata`] but on /// the verifiable version. mod verifiable { + use openmls_traits::crypto::OpenMlsCrypto; + + use crate::prelude_test::OpenMlsSignaturePublicKey; + use super::*; impl Verifiable for ProtectedMetadata { + type VerifiedStruct = ProtectedMetadata; + fn unsigned_payload(&self) -> Result, tls_codec::Error> { self.payload.tls_serialize_detached() } @@ -170,23 +176,22 @@ mod verifiable { fn label(&self) -> &str { SIGNATURE_LABEL } + + fn verify( + self, + _crypto: &impl OpenMlsCrypto, + _pk: &OpenMlsSignaturePublicKey, + ) -> Result { + Ok(self) + } } + impl VerifiedStruct for ProtectedMetadata {} + mod private_mod { #[derive(Default)] pub struct Seal; } - - impl VerifiedStruct for ProtectedMetadata { - type SealingType = private_mod::Seal; - - fn from_verifiable(v: ProtectedMetadata, _seal: Self::SealingType) -> Self { - Self { - payload: v.payload, - signature: v.signature, - } - } - } } #[cfg(test)] From 3ed6ac85098cabafd3fc0942d181f90d3b01337b Mon Sep 17 00:00:00 2001 From: Nicholas Molnar <65710+neekolas@users.noreply.github.com> Date: Fri, 12 Jan 2024 09:42:58 -0800 Subject: [PATCH 13/13] Fix bad import --- openmls/src/extensions/protected_metadata.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/openmls/src/extensions/protected_metadata.rs b/openmls/src/extensions/protected_metadata.rs index 617e9a598e..593367a36b 100644 --- a/openmls/src/extensions/protected_metadata.rs +++ b/openmls/src/extensions/protected_metadata.rs @@ -158,7 +158,7 @@ impl Signable for ProtectedMetadataTbs { mod verifiable { use openmls_traits::crypto::OpenMlsCrypto; - use crate::prelude_test::OpenMlsSignaturePublicKey; + use crate::prelude::OpenMlsSignaturePublicKey; use super::*;