diff --git a/openmls/Cargo.toml b/openmls/Cargo.toml index 0b9a71fc53..b9fa6e87e5 100644 --- a/openmls/Cargo.toml +++ b/openmls/Cargo.toml @@ -24,13 +24,13 @@ x509-cert = "0.2" subtle = "2.5" fluvio-wasm-timer = "0.2" indexmap = "2.0" +itertools = "0.11" # Only required for tests. rand = { version = "0.8", optional = true, features = ["getrandom"] } getrandom = { version = "0.2", optional = true, features = ["js"] } serde_json = { version = "1.0", optional = true } # Crypto backends 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 } async-lock = { version = "2.7", optional = true } rstest = { version = "^0.16", optional = true } @@ -42,7 +42,6 @@ default = [] crypto-subtle = [] # Enable subtle crypto APIs that have to be used with care. test-utils = [ "dep:serde_json", - "dep:itertools", "dep:openmls_rust_crypto", "dep:rand", "dep:getrandom", @@ -58,7 +57,6 @@ content-debug = [] # ☣️ Enable logging of sensitive message content [dev-dependencies] backtrace = "0.3" 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"] } diff --git a/openmls/benches/benchmark.rs b/openmls/benches/benchmark.rs index 97f47e36a8..db2c452b7b 100644 --- a/openmls/benches/benchmark.rs +++ b/openmls/benches/benchmark.rs @@ -22,7 +22,7 @@ fn criterion_benchmark(c: &mut Criterion) { c.bench_with_input( BenchmarkId::new( - format!("KeyPackage create bundle with ciphersuite"), + "KeyPackage create bundle with ciphersuite".to_string(), ciphersuite, ), &(&backend, signer, credential_with_key), diff --git a/openmls/src/binary_tree/array_representation/tree.rs b/openmls/src/binary_tree/array_representation/tree.rs index 64a4db0c5b..e0f5922af9 100644 --- a/openmls/src/binary_tree/array_representation/tree.rs +++ b/openmls/src/binary_tree/array_representation/tree.rs @@ -134,6 +134,11 @@ impl ABinaryTree { .map(|(index, leave)| (LeafNodeIndex::new(index as u32), leave)) } + /// Like [Self::leaves] but do not enumerate + pub(crate) fn raw_leaves(&self) -> impl Iterator { + self.leaf_nodes.iter() + } + /// Returns an iterator over a tuple of the parent index and a reference to /// a parent, sorted according to their position in the tree from left to /// right. diff --git a/openmls/src/credentials/errors.rs b/openmls/src/credentials/errors.rs index f1a566c986..f3cab97472 100644 --- a/openmls/src/credentials/errors.rs +++ b/openmls/src/credentials/errors.rs @@ -8,7 +8,7 @@ use thiserror::Error; /// An error that occurs in methods of a [`super::Credential`]. #[derive(Error, Debug, PartialEq, Clone)] pub enum CredentialError { - /// A library error occured. + /// A library error occurred. #[error(transparent)] LibraryError(#[from] LibraryError), /// The type of credential is not supported. diff --git a/openmls/src/credentials/mod.rs b/openmls/src/credentials/mod.rs index b5339be6db..5277aed58c 100644 --- a/openmls/src/credentials/mod.rs +++ b/openmls/src/credentials/mod.rs @@ -230,7 +230,7 @@ pub enum MlsCredentialType { /// ``` #[derive(Debug, PartialEq, Eq, Clone, Serialize, Deserialize)] pub struct Credential { - credential_type: CredentialType, + pub(crate) credential_type: CredentialType, credential: MlsCredentialType, } diff --git a/openmls/src/extensions/test_extensions.rs b/openmls/src/extensions/test_extensions.rs index 1d39de80b3..c90f3f70de 100644 --- a/openmls/src/extensions/test_extensions.rs +++ b/openmls/src/extensions/test_extensions.rs @@ -50,14 +50,14 @@ async fn ratchet_tree_extension(ciphersuite: Ciphersuite, backend: &impl OpenMls test_utils::new_credential(backend, b"Bob", ciphersuite.signature_algorithm()).await; // Generate KeyPackages - let bob_key_package_bundle = KeyPackageBundle::new( + let bob_kpb = KeyPackageBundle::new( backend, &bob_signature_keys, ciphersuite, bob_credential_with_key.clone(), ) .await; - let bob_key_package = bob_key_package_bundle.key_package(); + let bob_key_package = bob_kpb.key_package(); let config = CoreGroupConfig { add_ratchet_tree_extension: true, @@ -108,7 +108,8 @@ async fn ratchet_tree_extension(ciphersuite: Ciphersuite, backend: &impl OpenMls .welcome_option .expect("An unexpected error occurred."), None, - bob_key_package_bundle, + bob_kpb.key_package(), + bob_kpb.private_key().clone(), backend, ResumptionPskStore::new(1024), ) @@ -131,14 +132,14 @@ async fn ratchet_tree_extension(ciphersuite: Ciphersuite, backend: &impl OpenMls // === Alice creates a group without the ratchet tree extension === // Generate KeyPackages - let bob_key_package_bundle = KeyPackageBundle::new( + let bob_kpb = KeyPackageBundle::new( backend, &bob_signature_keys, ciphersuite, bob_credential_with_key, ) .await; - let bob_key_package = bob_key_package_bundle.key_package(); + let bob_key_package = bob_kpb.key_package(); let config = CoreGroupConfig { add_ratchet_tree_extension: false, @@ -188,7 +189,8 @@ async fn ratchet_tree_extension(ciphersuite: Ciphersuite, backend: &impl OpenMls .welcome_option .expect("An unexpected error occurred."), None, - bob_key_package_bundle, + bob_kpb.key_package(), + bob_kpb.private_key().clone(), backend, ResumptionPskStore::new(1024), ) diff --git a/openmls/src/framing/codec.rs b/openmls/src/framing/codec.rs index 9f3d67dba8..45f19fffac 100644 --- a/openmls/src/framing/codec.rs +++ b/openmls/src/framing/codec.rs @@ -67,7 +67,7 @@ impl Deserialize for MlsMessageIn { // KeyPackage version must match MlsMessage version. if let MlsMessageInBody::KeyPackage(key_package) = &body { - if !key_package.version_is_supported(version) { + if !key_package.is_version_supported(version) { return Err(tls_codec::Error::DecodingError( "KeyPackage version does not match MlsMessage version.".into(), )); diff --git a/openmls/src/framing/message_in.rs b/openmls/src/framing/message_in.rs index 5153e8e1cc..0f99bc753a 100644 --- a/openmls/src/framing/message_in.rs +++ b/openmls/src/framing/message_in.rs @@ -121,7 +121,7 @@ impl MlsMessageIn { pub fn into_keypackage(self) -> Option { match self.body { MlsMessageInBody::KeyPackage(key_package) => { - debug_assert!(key_package.version_is_supported(self.version)); + debug_assert!(key_package.is_version_supported(self.version)); Some(key_package.into()) } _ => None, diff --git a/openmls/src/framing/mls_auth_content_in.rs b/openmls/src/framing/mls_auth_content_in.rs index c3c09e3c01..214a618352 100644 --- a/openmls/src/framing/mls_auth_content_in.rs +++ b/openmls/src/framing/mls_auth_content_in.rs @@ -53,6 +53,7 @@ impl AuthenticatedContentIn { crypto: &impl OpenMlsCrypto, sender_context: Option, protocol_version: ProtocolVersion, + group: &PublicGroup, ) -> Result { Ok(AuthenticatedContent { wire_format: self.wire_format, @@ -61,6 +62,7 @@ impl AuthenticatedContentIn { crypto, sender_context, protocol_version, + group, )?, auth: self.auth, }) diff --git a/openmls/src/framing/mls_content_in.rs b/openmls/src/framing/mls_content_in.rs index 474a84892a..dce5797e9b 100644 --- a/openmls/src/framing/mls_content_in.rs +++ b/openmls/src/framing/mls_content_in.rs @@ -18,6 +18,7 @@ use super::{ ContentType, Sender, WireFormat, }; +use crate::prelude::PublicGroup; use openmls_traits::{crypto::OpenMlsCrypto, types::Ciphersuite}; use serde::{Deserialize, Serialize}; use tls_codec::{ @@ -55,15 +56,20 @@ impl FramedContentIn { crypto: &impl OpenMlsCrypto, sender_context: Option, protocol_version: ProtocolVersion, + group: &PublicGroup, ) -> Result { Ok(FramedContent { group_id: self.group_id, epoch: self.epoch, sender: self.sender, authenticated_data: self.authenticated_data, - body: self - .body - .validate(ciphersuite, crypto, sender_context, protocol_version)?, + body: self.body.validate( + ciphersuite, + crypto, + sender_context, + protocol_version, + group, + )?, }) } } @@ -135,12 +141,19 @@ impl FramedContentBodyIn { crypto: &impl OpenMlsCrypto, sender_context: Option, protocol_version: ProtocolVersion, + group: &PublicGroup, ) -> Result { Ok(match self { FramedContentBodyIn::Application(bytes) => FramedContentBody::Application(bytes), - FramedContentBodyIn::Proposal(proposal_in) => FramedContentBody::Proposal( - proposal_in.validate(crypto, ciphersuite, sender_context, protocol_version)?, - ), + FramedContentBodyIn::Proposal(proposal_in) => { + FramedContentBody::Proposal(proposal_in.validate( + crypto, + ciphersuite, + sender_context, + protocol_version, + group, + )?) + } FramedContentBodyIn::Commit(commit_in) => { let sender_context = sender_context .ok_or(LibraryError::custom("Forgot the commit sender context"))?; @@ -149,6 +162,7 @@ impl FramedContentBodyIn { crypto, sender_context, protocol_version, + group, )?) } }) diff --git a/openmls/src/framing/test_framing.rs b/openmls/src/framing/test_framing.rs index 67afe49fde..9c7d3faccc 100644 --- a/openmls/src/framing/test_framing.rs +++ b/openmls/src/framing/test_framing.rs @@ -392,18 +392,18 @@ async fn unknown_sender(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr test_utils::new_credential(backend, b"Charlie", ciphersuite.signature_algorithm()).await; // Generate KeyPackages - let bob_key_package_bundle = + let bob_kpb = KeyPackageBundle::new(backend, &bob_signature_keys, ciphersuite, bob_credential).await; - let bob_key_package = bob_key_package_bundle.key_package(); + let bob_key_package = bob_kpb.key_package(); - let charlie_key_package_bundle = KeyPackageBundle::new( + let charlie_kpb = KeyPackageBundle::new( backend, &charlie_signature_keys, ciphersuite, charlie_credential, ) .await; - let charlie_key_package = charlie_key_package_bundle.key_package(); + let charlie_key_package = charlie_kpb.key_package(); // Alice creates a group let mut group_alice = CoreGroup::builder( @@ -449,7 +449,8 @@ async fn unknown_sender(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr .welcome_option .expect("An unexpected error occurred."), Some(group_alice.public_group().export_ratchet_tree().into()), - bob_key_package_bundle, + bob_kpb.key_package(), + bob_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) @@ -496,7 +497,8 @@ async fn unknown_sender(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr .welcome_option .expect("An unexpected error occurred."), Some(group_alice.public_group().export_ratchet_tree().into()), - charlie_key_package_bundle, + charlie_kpb.key_package(), + charlie_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) @@ -631,14 +633,14 @@ pub(crate) async fn setup_alice_bob_group( test_utils::new_credential(backend, b"Bob", ciphersuite.signature_algorithm()).await; // Generate KeyPackages - let bob_key_package_bundle = KeyPackageBundle::new( + let bob_kpb = KeyPackageBundle::new( backend, &bob_signature_keys, ciphersuite, bob_credential.clone(), ) .await; - let bob_key_package = bob_key_package_bundle.key_package(); + let bob_key_package = bob_kpb.key_package(); // Alice creates a group let mut group_alice = CoreGroup::builder( @@ -695,7 +697,8 @@ pub(crate) async fn setup_alice_bob_group( .welcome_option .expect("commit didn't return a welcome as expected"), Some(group_alice.public_group().export_ratchet_tree().into()), - bob_key_package_bundle, + bob_kpb.key_package(), + bob_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) diff --git a/openmls/src/framing/validation.rs b/openmls/src/framing/validation.rs index 61cba2e8b3..92098e304d 100644 --- a/openmls/src/framing/validation.rs +++ b/openmls/src/framing/validation.rs @@ -278,6 +278,7 @@ impl UnverifiedMessage { ciphersuite: Ciphersuite, crypto: &impl OpenMlsCrypto, protocol_version: ProtocolVersion, + group: &PublicGroup, ) -> Result<(AuthenticatedContent, Credential), ProcessMessageError> { let content: AuthenticatedContentIn = match self.credential.mls_credential() { MlsCredentialType::Basic(_) => self @@ -315,8 +316,13 @@ impl UnverifiedMessage { .map_err(|_| ProcessMessageError::InvalidSignature)? } }; - let content = - content.validate(ciphersuite, crypto, self.sender_context, protocol_version)?; + let content = content.validate( + ciphersuite, + crypto, + self.sender_context, + protocol_version, + group, + )?; Ok((content, self.credential)) } diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index 4e7b94bacb..9e19b62833 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -349,27 +349,17 @@ impl CoreGroup { pub(crate) fn create_add_proposal( &self, framing_parameters: FramingParameters, - joiner_key_package: KeyPackage, + key_package: KeyPackage, signer: &impl Signer, ) -> Result { - if let Some(required_capabilities) = self.required_capabilities() { - joiner_key_package - .leaf_node() - .capabilities() - .supports_required_capabilities(required_capabilities)?; - } - let add_proposal = AddProposal { - key_package: joiner_key_package, - }; - let proposal = Proposal::Add(add_proposal); - AuthenticatedContent::member_proposal( + let proposal = Proposal::Add(AddProposal { key_package }); + Ok(AuthenticatedContent::member_proposal( framing_parameters, self.own_leaf_index(), proposal, self.context(), signer, - ) - .map_err(|e| e.into()) + )?) } // 11.1.2. Update @@ -442,7 +432,7 @@ impl CoreGroup { ) } - /// Checks if the memebers suuport the provided extensions. Pending proposals have to be passed + /// Checks if the members support the provided extensions. Pending proposals have to be passed /// as parameters as Remove Proposals should be ignored pub(crate) fn members_support_extensions<'a>( &self, @@ -719,11 +709,6 @@ impl CoreGroup { self.public_group.group_context().extensions() } - /// Get the required capabilities extension of this group. - pub(crate) fn required_capabilities(&self) -> Option<&RequiredCapabilitiesExtension> { - self.public_group.required_capabilities() - } - /// Returns `true` if the group uses the ratchet tree extension anf `false /// otherwise #[cfg(test)] diff --git a/openmls/src/group/core_group/new_from_welcome.rs b/openmls/src/group/core_group/new_from_welcome.rs index 2558e14992..e4cf491058 100644 --- a/openmls/src/group/core_group/new_from_welcome.rs +++ b/openmls/src/group/core_group/new_from_welcome.rs @@ -3,6 +3,7 @@ use openmls_traits::key_store::OpenMlsKeyStore; use crate::{ ciphersuite::hash_ref::HashReference, group::{core_group::*, errors::WelcomeError}, + prelude::HpkePrivateKey, schedule::{ errors::PskError, psk::{store::ResumptionPskStore, ResumptionPsk, ResumptionPskUsage}, @@ -18,12 +19,12 @@ impl CoreGroup { pub async fn new_from_welcome( welcome: Welcome, ratchet_tree: Option, - key_package_bundle: KeyPackageBundle, + key_package: &KeyPackage, + key_package_private_key: HpkePrivateKey, backend: &impl OpenMlsCryptoProvider, mut resumption_psk_store: ResumptionPskStore, ) -> Result> { log::debug!("CoreGroup::new_from_welcome_internal"); - let key_package = key_package_bundle.key_package(); // Read the encryption key pair from the key store and delete it there. // TODO #1207: Key store access happens as early as possible so it can @@ -56,7 +57,7 @@ impl CoreGroup { } let group_secrets = GroupSecrets::try_from_ciphertext( - key_package_bundle.private_key(), + &key_package_private_key, egs.encrypted_group_secrets(), welcome.encrypted_group_info(), ciphersuite, @@ -152,6 +153,12 @@ impl CoreGroup { ProposalStore::new(), )?; + KeyPackageIn::from(key_package.clone()).validate( + backend.crypto(), + ProtocolVersion::Mls10, + &public_group, + )?; + // Find our own leaf in the tree. let own_leaf_index = public_group .members() @@ -257,7 +264,7 @@ impl CoreGroup { welcome_secrets: &[EncryptedGroupSecrets], ) -> Option { for egs in welcome_secrets { - if hash_ref == egs.new_member() { + if &hash_ref == egs.new_member() { return Some(egs.clone()); } } diff --git a/openmls/src/group/core_group/process.rs b/openmls/src/group/core_group/process.rs index 220ef7a3a3..fd27c82340 100644 --- a/openmls/src/group/core_group/process.rs +++ b/openmls/src/group/core_group/process.rs @@ -49,8 +49,12 @@ impl CoreGroup { // Checks the following semantic validation: // - ValSem010 // - ValSem246 (as part of ValSem010) - let (content, credential) = - unverified_message.verify(self.ciphersuite(), backend.crypto(), self.version())?; + let (content, credential) = unverified_message.verify( + self.ciphersuite(), + backend.crypto(), + self.version(), + self.public_group(), + )?; match content.sender() { Sender::Member(_) | Sender::NewMemberCommit | Sender::NewMemberProposal => { diff --git a/openmls/src/group/core_group/proposals.rs b/openmls/src/group/core_group/proposals.rs index 63e07c3872..d3e23e9c52 100644 --- a/openmls/src/group/core_group/proposals.rs +++ b/openmls/src/group/core_group/proposals.rs @@ -68,7 +68,7 @@ impl ProposalStore { /// the encapsulating PublicMessage and the ProposalRef is attached. #[derive(Clone, Debug, PartialEq, Serialize, Deserialize)] pub struct QueuedProposal { - proposal: Proposal, + pub proposal: Proposal, proposal_reference: ProposalRef, sender: Sender, proposal_or_ref_type: ProposalOrRefType, @@ -462,8 +462,7 @@ impl ProposalQueue { &sender, ) }) - .collect::, _>>()? - .into_iter(), + .collect::, _>>()?, ); // Parse proposals and build adds and member list diff --git a/openmls/src/group/core_group/test_core_group.rs b/openmls/src/group/core_group/test_core_group.rs index 471bdb91e9..e1e5eede56 100644 --- a/openmls/src/group/core_group/test_core_group.rs +++ b/openmls/src/group/core_group/test_core_group.rs @@ -102,7 +102,7 @@ async fn test_failed_groupinfo_decryption( let (alice_credential_with_key, alice_signature_keys) = test_utils::new_credential(backend, b"Alice", ciphersuite.signature_algorithm()).await; - let key_package_bundle = KeyPackageBundle::new( + let kpb = KeyPackageBundle::new( backend, &alice_signature_keys, ciphersuite, @@ -162,8 +162,7 @@ async fn test_failed_groupinfo_decryption( flip_last_byte(&mut encrypted_group_secrets); let broken_secrets = vec![EncryptedGroupSecrets::new( - key_package_bundle - .key_package + kpb.key_package .hash_ref(backend.crypto()) .expect("Could not hash KeyPackage."), encrypted_group_secrets, @@ -187,7 +186,8 @@ async fn test_failed_groupinfo_decryption( let error = CoreGroup::new_from_welcome( broken_welcome, None, - key_package_bundle, + kpb.key_package(), + kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) @@ -340,12 +340,8 @@ async fn test_psks(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProvide let group_aad = b"Alice's test group"; let framing_parameters = FramingParameters::new(group_aad, WireFormat::PublicMessage); - let ( - alice_credential_with_key, - alice_signature_keys, - bob_key_package_bundle, - bob_signature_keys, - ) = setup_alice_bob(ciphersuite, backend).await; + let (alice_credential_with_key, alice_signature_keys, bob_kpb, bob_signature_keys) = + setup_alice_bob(ciphersuite, backend).await; // === Alice creates a group with a PSK === let psk_id = vec![1u8, 2, 3]; @@ -380,7 +376,7 @@ async fn test_psks(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProvide let bob_add_proposal = alice_group .create_add_proposal( framing_parameters, - bob_key_package_bundle.key_package().clone(), + bob_kpb.key_package().clone(), &alice_signature_keys, ) .expect("Could not create proposal"); @@ -417,7 +413,8 @@ async fn test_psks(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProvide .welcome_option .expect("An unexpected error occurred."), Some(ratchet_tree.into()), - bob_key_package_bundle, + bob_kpb.key_package(), + bob_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) @@ -473,7 +470,7 @@ async fn test_staged_commit_creation( let group_aad = b"Alice's test group"; let framing_parameters = FramingParameters::new(group_aad, WireFormat::PublicMessage); - let (alice_credential_with_key, alice_signature_keys, bob_key_package_bundle, _) = + let (alice_credential_with_key, alice_signature_keys, bob_kpb, _) = setup_alice_bob(ciphersuite, backend).await; // === Alice creates a group === @@ -490,7 +487,7 @@ async fn test_staged_commit_creation( let bob_add_proposal = alice_group .create_add_proposal( framing_parameters, - bob_key_package_bundle.key_package().clone(), + bob_kpb.key_package().clone(), &alice_signature_keys, ) .expect("Could not create proposal."); @@ -520,7 +517,8 @@ async fn test_staged_commit_creation( .welcome_option .expect("An unexpected error occurred."), Some(alice_group.public_group().export_ratchet_tree().into()), - bob_key_package_bundle, + bob_kpb.key_package(), + bob_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) @@ -702,7 +700,8 @@ async fn test_proposal_application_after_self_was_removed( .welcome_option .expect("An unexpected error occurred."), Some(ratchet_tree.into()), - bob_kpb, + bob_kpb.key_package(), + bob_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) @@ -787,7 +786,8 @@ async fn test_proposal_application_after_self_was_removed( .welcome_option .expect("An unexpected error occurred."), Some(ratchet_tree.into()), - charlie_kpb, + charlie_kpb.key_package(), + charlie_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) diff --git a/openmls/src/group/core_group/test_proposals.rs b/openmls/src/group/core_group/test_proposals.rs index 98fd82b8e9..de3515b50e 100644 --- a/openmls/src/group/core_group/test_proposals.rs +++ b/openmls/src/group/core_group/test_proposals.rs @@ -48,8 +48,9 @@ async fn proposal_queue_functions(ciphersuite: Ciphersuite, backend: &impl OpenM KeyPackageBundle::new(backend, &alice_signer, ciphersuite, alice_credential).await; let alice_update_key_package = alice_update_key_package_bundle.key_package(); let kpi = KeyPackageIn::from(alice_update_key_package.clone()); + assert!(kpi - .validate(backend.crypto(), ProtocolVersion::Mls10) + .standalone_validate(backend.crypto(), ProtocolVersion::Mls10) .is_ok()); let group_context = GroupContext::new( @@ -193,8 +194,9 @@ async fn proposal_queue_order(ciphersuite: Ciphersuite, backend: &impl OpenMlsCr KeyPackageBundle::new(backend, &alice_signer, ciphersuite, alice_credential).await; let alice_update_key_package = alice_update_key_package_bundle.key_package(); let kpi = KeyPackageIn::from(alice_update_key_package.clone()); + assert!(kpi - .validate(backend.crypto(), ProtocolVersion::Mls10) + .standalone_validate(backend.crypto(), ProtocolVersion::Mls10) .is_ok()); let group_context = GroupContext::new( @@ -341,7 +343,7 @@ async fn test_group_context_extensions( leaf_capabilities.clone(), ) .await; - let (_bob_credential_bundle, bob_key_package_bundle, _, _) = setup_client_with_extensions( + let (_bob_credential_bundle, bob_kpb, _, _) = setup_client_with_extensions( "Bob", ciphersuite, backend, @@ -350,7 +352,7 @@ async fn test_group_context_extensions( ) .await; - let bob_key_package = bob_key_package_bundle.key_package(); + let bob_key_package = bob_kpb.key_package(); let mut alice_group = CoreGroup::builder( GroupId::random(backend), @@ -396,7 +398,8 @@ async fn test_group_context_extensions( .welcome_option .expect("An unexpected error occurred."), Some(ratchet_tree.into()), - bob_key_package_bundle, + bob_kpb.key_package(), + bob_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) @@ -416,10 +419,9 @@ async fn test_group_context_extension_proposal_fails( let (alice_credential, _, alice_signer, _alice_pk) = setup_client("Alice", ciphersuite, backend).await; - let (_bob_credential_with_key, bob_key_package_bundle, _, _) = - setup_client("Bob", ciphersuite, backend).await; + let (_bob_credential_with_key, bob_kpb, _, _) = setup_client("Bob", ciphersuite, backend).await; - let bob_key_package = bob_key_package_bundle.key_package(); + let bob_key_package = bob_kpb.key_package(); // Set required capabilities let proposals = &[]; @@ -491,7 +493,8 @@ async fn test_group_context_extension_proposal_fails( .welcome_option .expect("An unexpected error occurred."), Some(ratchet_tree.into()), - bob_key_package_bundle, + bob_kpb.key_package(), + bob_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) @@ -529,10 +532,9 @@ async fn test_group_context_extension_proposal( let (alice_credential, _, alice_signer, _alice_pk) = setup_client("Alice", ciphersuite, backend).await; - let (_bob_credential_with_key, bob_key_package_bundle, _, _) = - setup_client("Bob", ciphersuite, backend).await; + let (_bob_credential_with_key, bob_kpb, _, _) = setup_client("Bob", ciphersuite, backend).await; - let bob_key_package = bob_key_package_bundle.key_package(); + let bob_key_package = bob_kpb.key_package(); let mut alice_group = CoreGroup::builder( GroupId::random(backend), @@ -577,7 +579,8 @@ async fn test_group_context_extension_proposal( .welcome_option .expect("An unexpected error occurred."), Some(ratchet_tree.into()), - bob_key_package_bundle, + bob_kpb.key_package(), + bob_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) diff --git a/openmls/src/group/errors.rs b/openmls/src/group/errors.rs index 1a8a57d1b8..5cc4362c5f 100644 --- a/openmls/src/group/errors.rs +++ b/openmls/src/group/errors.rs @@ -38,6 +38,9 @@ pub enum WelcomeError { /// See [`GroupInfoError`] for more details. #[error(transparent)] GroupInfo(#[from] GroupInfoError), + /// See [`KeyPackageVerifyError`] for more details. + #[error(transparent)] + KeyPackageVerifyError(#[from] KeyPackageVerifyError), /// No joiner secret found in the Welcome message. #[error("No joiner secret found in the Welcome message.")] JoinerSecretNotFound, @@ -303,6 +306,9 @@ pub enum ValidationError { /// The KeyPackage could not be validated. #[error(transparent)] KeyPackageVerifyError(#[from] KeyPackageVerifyError), + /// The LeafNode could not be validated. + #[error(transparent)] + LeafNodeValidationError(#[from] LeafNodeValidationError), /// The UpdatePath could not be validated. #[error(transparent)] UpdatePathError(#[from] UpdatePathError), @@ -428,9 +434,12 @@ pub enum CreateAddProposalError { /// See [`LibraryError`] for more details. #[error(transparent)] LibraryError(#[from] LibraryError), + /// See [`KeyPackageVerifyError`] for more details. + #[error(transparent)] + KeyPackageVerifyError(#[from] KeyPackageVerifyError), /// See [`LeafNodeValidationError`] for more details. #[error(transparent)] - LeafNodeValidation(#[from] LeafNodeValidationError), + LeafNodeValidationError(#[from] LeafNodeValidationError), } // === Crate errors === diff --git a/openmls/src/group/mls_group/config.rs b/openmls/src/group/mls_group/config.rs index 499f94ef34..b734280b4e 100644 --- a/openmls/src/group/mls_group/config.rs +++ b/openmls/src/group/mls_group/config.rs @@ -245,6 +245,15 @@ impl MlsGroupConfigBuilder { self } + /// Sets the group creator's required capabilities + pub fn required_capabilities( + mut self, + required_capabilities: RequiredCapabilitiesExtension, + ) -> Self { + self.config.required_capabilities = required_capabilities; + self + } + /// Finalizes the builder and retursn an `[MlsGroupConfig`]. pub fn build(self) -> MlsGroupConfig { self.config diff --git a/openmls/src/group/mls_group/creation.rs b/openmls/src/group/mls_group/creation.rs index 4825be1522..534e983984 100644 --- a/openmls/src/group/mls_group/creation.rs +++ b/openmls/src/group/mls_group/creation.rs @@ -123,7 +123,7 @@ impl MlsGroup { ResumptionPskStore::new(mls_group_config.number_of_resumption_psks); let mut key_package: Option = None; - for egs in welcome.secrets().iter() { + for egs in welcome.secrets() { if let Some(kp) = backend.key_store().read(egs.new_member().as_slice()).await { key_package.replace(kp); break; @@ -140,19 +140,12 @@ impl MlsGroup { .read::(key_package.hpke_init_key().as_slice()) .await .ok_or(WelcomeError::NoMatchingKeyPackage)?; - let key_package_bundle = KeyPackageBundle { - key_package, - private_key, - }; - - // Delete the [`KeyPackage`] and the corresponding private key from the - // key store - key_package_bundle.key_package.delete(backend).await?; let mut group = CoreGroup::new_from_welcome( welcome, ratchet_tree, - key_package_bundle, + &key_package, + private_key, backend, resumption_psk_store, ) @@ -169,6 +162,9 @@ impl MlsGroup { state_changed: InnerState::Changed, }; + // Delete the [`KeyPackage`] and the corresponding private key from the key store + key_package.delete(backend).await?; + Ok(mls_group) } diff --git a/openmls/src/group/mls_group/errors.rs b/openmls/src/group/mls_group/errors.rs index 9b38f9d047..b5a9b63404 100644 --- a/openmls/src/group/mls_group/errors.rs +++ b/openmls/src/group/mls_group/errors.rs @@ -8,6 +8,7 @@ use openmls_traits::types::CryptoError; use thiserror::Error; +use crate::prelude::KeyPackageVerifyError; use crate::{ credentials::errors::CredentialError, error::LibraryError, @@ -153,6 +154,9 @@ pub enum AddMembersError { /// See [`MlsGroupStateError`] for more details. #[error(transparent)] GroupStateError(#[from] MlsGroupStateError), + /// See [`KeyPackageVerifyError`] for more details. + #[error(transparent)] + KeyPackageVerifyError(#[from] KeyPackageVerifyError), } /// Propose add members error @@ -167,9 +171,12 @@ pub enum ProposeAddMemberError { /// See [`MlsGroupStateError`] for more details. #[error(transparent)] GroupStateError(#[from] MlsGroupStateError), - /// See [`LeafNodeValidationError`] for more details. + /// See [`KeyPackageVerifyError`] for more details. + #[error(transparent)] + KeyPackageVerifyError(#[from] KeyPackageVerifyError), + /// See [`CreateAddProposalError`] for more details. #[error(transparent)] - LeafNodeValidation(#[from] LeafNodeValidationError), + CreateAddProposalError(#[from] CreateAddProposalError), } /// Propose remove members error @@ -229,9 +236,12 @@ pub enum SelfUpdateError { /// See [`MlsGroupStateError`] for more details. #[error(transparent)] GroupStateError(#[from] MlsGroupStateError), + /// See [`PublicTreeError`] for more details. + #[error(transparent)] + PublicTreeError(#[from] PublicTreeError), /// Error accessing the key store. #[error("Error accessing the key store.")] - KeyStoreError, + KeyStoreError(KeyStoreError), } /// Propose self update error @@ -240,7 +250,6 @@ pub enum ProposeSelfUpdateError { /// See [`LibraryError`] for more details. #[error(transparent)] LibraryError(#[from] LibraryError), - /// See [`MlsGroupStateError`] for more details. #[error(transparent)] GroupStateError(#[from] MlsGroupStateError), @@ -250,6 +259,9 @@ pub enum ProposeSelfUpdateError { /// See [`PublicTreeError`] for more details. #[error(transparent)] PublicTreeError(#[from] PublicTreeError), + /// See [`LeafNodeValidationError`] for more details. + #[error(transparent)] + LeafNodeValidationError(#[from] LeafNodeValidationError), } /// Create group context ext proposal error @@ -396,4 +408,7 @@ pub enum ProposalError { /// See [`ValidationError`] for more details. #[error(transparent)] ValidationError(#[from] ValidationError), + /// See [`KeyPackageVerifyError`] for more details. + #[error(transparent)] + KeyPackageVerifyError(#[from] KeyPackageVerifyError), } diff --git a/openmls/src/group/mls_group/membership.rs b/openmls/src/group/mls_group/membership.rs index 9df7482db8..a232c84df3 100644 --- a/openmls/src/group/mls_group/membership.rs +++ b/openmls/src/group/mls_group/membership.rs @@ -9,6 +9,8 @@ use super::{ errors::{AddMembersError, LeaveGroupError, RemoveMembersError}, *, }; +use crate::prelude::KeyPackageIn; +use crate::versions::ProtocolVersion; use crate::{ binary_tree::array_representation::LeafNodeIndex, messages::group_info::GroupInfo, treesync::LeafNode, @@ -33,24 +35,27 @@ impl MlsGroup { &mut self, backend: &impl OpenMlsCryptoProvider, signer: &impl Signer, - key_packages: &[KeyPackage], + key_packages: Vec, ) -> Result<(MlsMessageOut, MlsMessageOut, Option), AddMembersError> { - self.is_operational()?; - if key_packages.is_empty() { return Err(AddMembersError::EmptyInput(EmptyInputError::AddMembers)); } + self.is_operational()?; + // Create inline add proposals from key packages let inline_proposals = key_packages - .iter() + .into_iter() .map(|key_package| { - Proposal::Add(AddProposal { - key_package: key_package.clone(), - }) + let key_package = key_package.validate( + backend.crypto(), + ProtocolVersion::Mls10, + self.group().public_group(), + )?; + Ok(Proposal::Add(AddProposal { key_package })) }) - .collect::>(); + .collect::, AddMembersError>>()?; // Create Commit over all proposals // TODO #751 diff --git a/openmls/src/group/mls_group/mod.rs b/openmls/src/group/mls_group/mod.rs index acbdfbe8b7..2dea8a478f 100644 --- a/openmls/src/group/mls_group/mod.rs +++ b/openmls/src/group/mls_group/mod.rs @@ -10,7 +10,7 @@ use crate::{ error::LibraryError, framing::{mls_auth_content::AuthenticatedContent, *}, group::*, - key_packages::{KeyPackage, KeyPackageBundle}, + key_packages::KeyPackage, messages::{proposals::*, Welcome}, schedule::ResumptionPskSecret, treesync::{node::leaf_node::LeafNode, RatchetTree}, @@ -422,7 +422,6 @@ impl MlsGroup { } /// Returns the underlying [CoreGroup]. - #[cfg(test)] pub(crate) fn group(&self) -> &CoreGroup { &self.group } diff --git a/openmls/src/group/mls_group/proposal.rs b/openmls/src/group/mls_group/proposal.rs index ba68443f70..1f66126eaa 100644 --- a/openmls/src/group/mls_group/proposal.rs +++ b/openmls/src/group/mls_group/proposal.rs @@ -6,13 +6,14 @@ use super::{ errors::{ProposalError, ProposeAddMemberError, ProposeRemoveMemberError}, MlsGroup, }; +use crate::prelude::KeyPackageIn; use crate::{ binary_tree::LeafNodeIndex, ciphersuite::hash_ref::ProposalRef, credentials::Credential, extensions::Extensions, framing::MlsMessageOut, - group::{errors::CreateAddProposalError, GroupId, QueuedProposal}, + group::{GroupId, QueuedProposal}, key_packages::KeyPackage, messages::proposals::ProposalOrRefType, prelude::LibraryError, @@ -93,12 +94,41 @@ macro_rules! impl_propose_fun { } impl MlsGroup { - impl_propose_fun!( - propose_add_member_by_value, - KeyPackage, - create_add_proposal, - ProposalOrRefType::Proposal - ); + /// Creates proposals to add an external PSK to the key schedule. + /// + /// Returns an error if there is a pending commit. + pub fn propose_add_member_by_value( + &mut self, + backend: &impl OpenMlsCryptoProvider, + signer: &impl Signer, + joiner_key_package: KeyPackageIn, + ) -> Result<(MlsMessageOut, ProposalRef), ProposalError> { + self.is_operational()?; + + let key_package = joiner_key_package.validate( + backend.crypto(), + ProtocolVersion::Mls10, + self.group().public_group(), + )?; + let proposal = + self.group + .create_add_proposal(self.framing_parameters(), key_package, signer)?; + + let queued_proposal = QueuedProposal::from_authenticated_content( + self.ciphersuite(), + backend, + proposal.clone(), + ProposalOrRefType::Proposal, + )?; + let proposal_ref = queued_proposal.proposal_reference().clone(); + self.proposal_store.add(queued_proposal); + + let mls_message = self.content_to_mls_message(proposal, backend)?; + + self.flag_state_change(); + + Ok((mls_message, proposal_ref)) + } impl_propose_fun!( propose_remove_member_by_value, @@ -132,10 +162,10 @@ impl MlsGroup { match propose { Propose::Add(key_package) => match ref_or_value { ProposalOrRefType::Proposal => { - self.propose_add_member_by_value(backend, signer, key_package) + self.propose_add_member_by_value(backend, signer, key_package.into()) } ProposalOrRefType::Reference => self - .propose_add_member(backend, signer, &key_package) + .propose_add_member(backend, signer, key_package.into()) .map_err(|e| e.into()), }, @@ -214,19 +244,18 @@ impl MlsGroup { &mut self, backend: &impl OpenMlsCryptoProvider, signer: &impl Signer, - key_package: &KeyPackage, + joiner_key_package: KeyPackageIn, ) -> Result<(MlsMessageOut, ProposalRef), ProposeAddMemberError> { self.is_operational()?; - let add_proposal = self - .group - .create_add_proposal(self.framing_parameters(), key_package.clone(), signer) - .map_err(|e| match e { - CreateAddProposalError::LibraryError(e) => e.into(), - CreateAddProposalError::LeafNodeValidation(error) => { - ProposeAddMemberError::LeafNodeValidation(error) - } - })?; + let key_package = joiner_key_package.validate( + backend.crypto(), + ProtocolVersion::Mls10, + self.group().public_group(), + )?; + let add_proposal = + self.group + .create_add_proposal(self.framing_parameters(), key_package, signer)?; let proposal = QueuedProposal::from_authenticated_content_by_ref( self.ciphersuite(), diff --git a/openmls/src/group/mls_group/test_mls_group.rs b/openmls/src/group/mls_group/test_mls_group.rs index e1f2d2b33f..4d343f5de2 100644 --- a/openmls/src/group/mls_group/test_mls_group.rs +++ b/openmls/src/group/mls_group/test_mls_group.rs @@ -97,7 +97,11 @@ async fn remover(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProvider) // === Alice adds Bob === let (_queued_message, welcome, _group_info) = alice_group - .add_members(backend, &alice_signer, &[bob_kpb.key_package().clone()]) + .add_members( + backend, + &alice_signer, + vec![bob_kpb.key_package().clone().into()], + ) .await .expect("Could not add member to group."); @@ -117,7 +121,11 @@ async fn remover(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProvider) // === Bob adds Charlie === let (queued_messages, welcome, _group_info) = bob_group - .add_members(backend, &bob_signer, &[charlie_kpb.key_package().clone()]) + .add_members( + backend, + &bob_signer, + vec![charlie_kpb.key_package().clone().into()], + ) .await .unwrap(); @@ -395,7 +403,7 @@ async fn test_pending_commit_logic(ciphersuite: Ciphersuite, backend: &impl Open // Let's add bob let (proposal, _) = alice_group - .propose_add_member(backend, &alice_signer, bob_key_package) + .propose_add_member(backend, &alice_signer, bob_key_package.clone().into()) .expect("error creating self-update proposal"); let alice_processed_message = alice_group @@ -428,7 +436,7 @@ async fn test_pending_commit_logic(ciphersuite: Ciphersuite, backend: &impl Open // If there is a pending commit, other commit- or proposal-creating actions // should fail. let error = alice_group - .add_members(backend, &alice_signer, &[bob_key_package.clone()]) + .add_members(backend, &alice_signer, vec![bob_key_package.clone().into()]) .await .expect_err("no error committing while a commit is pending"); assert!(matches!( @@ -436,7 +444,7 @@ async fn test_pending_commit_logic(ciphersuite: Ciphersuite, backend: &impl Open AddMembersError::GroupStateError(MlsGroupStateError::PendingCommit) )); let error = alice_group - .propose_add_member(backend, &alice_signer, bob_key_package) + .propose_add_member(backend, &alice_signer, bob_key_package.clone().into()) .expect_err("no error creating a proposal while a commit is pending"); assert!(matches!( error, @@ -582,7 +590,7 @@ async fn key_package_deletion(ciphersuite: Ciphersuite, backend: &impl OpenMlsCr // === Alice adds Bob === let (_queued_message, welcome, _group_info) = alice_group - .add_members(backend, &alice_signer, &[bob_key_package.clone()]) + .add_members(backend, &alice_signer, vec![bob_key_package.clone().into()]) .await .unwrap(); @@ -653,7 +661,7 @@ async fn remove_prosposal_by_ref(ciphersuite: Ciphersuite, backend: &impl OpenMl // alice adds bob and bob processes the welcome let (_, welcome, _) = alice_group - .add_members(backend, &alice_signer, &[bob_key_package]) + .add_members(backend, &alice_signer, vec![bob_key_package.clone().into()]) .await .unwrap(); alice_group.merge_pending_commit(backend).await.unwrap(); @@ -667,7 +675,7 @@ async fn remove_prosposal_by_ref(ciphersuite: Ciphersuite, backend: &impl OpenMl .unwrap(); // alice proposes to add charlie let (_, reference) = alice_group - .propose_add_member(backend, &alice_signer, charlie_key_package) + .propose_add_member(backend, &alice_signer, charlie_key_package.clone().into()) .unwrap(); assert_eq!(alice_group.proposal_store.proposals().count(), 1); diff --git a/openmls/src/group/mls_group/updates.rs b/openmls/src/group/mls_group/updates.rs index bf13181d4d..ff55f1b639 100644 --- a/openmls/src/group/mls_group/updates.rs +++ b/openmls/src/group/mls_group/updates.rs @@ -1,6 +1,8 @@ use core_group::create_commit_params::CreateCommitParams; use openmls_traits::signatures::Signer; +use crate::treesync::node::leaf_node::{LeafNodeIn, TreePosition, VerifiableLeafNode}; +use crate::treesync::node::validate::ValidatableLeafNode; use crate::{messages::group_info::GroupInfo, treesync::LeafNode, versions::ProtocolVersion}; use super::*; @@ -56,17 +58,13 @@ impl MlsGroup { .ok_or_else(|| LibraryError::custom("The tree is broken. Couldn't find own leaf."))? .clone(); - own_leaf - .update_and_re_sign( - None, - Some(leaf_node), - self.group_id().clone(), - self.own_leaf_index(), - signer, - ) - .map_err(|_| { - SelfUpdateError::LibraryError(LibraryError::custom("Could not resign the leaf")) - })?; + own_leaf.update_and_re_sign( + None, + Some(leaf_node), + self.group_id().clone(), + self.own_leaf_index(), + signer, + )?; let update_proposal = Proposal::Update(UpdateProposal { leaf_node: own_leaf, @@ -160,10 +158,8 @@ impl MlsGroup { ) -> Result> { self.is_operational()?; - // Here we clone our own leaf to rekey it such that we don't change the - // tree. - // The new leaf node will be applied later when the proposal is - // committed. + // Here we clone our own leaf to rekey it such that we don't change the tree. + // The new leaf node will be applied later when the proposal is committed. let mut own_leaf = self .own_leaf() .ok_or_else(|| LibraryError::custom("The tree is broken. Couldn't find own leaf."))? @@ -177,12 +173,23 @@ impl MlsGroup { backend, signer, )?; - // TODO #1207: Move to the top of the function. + keypair .write_to_key_store(backend) .await .map_err(ProposeSelfUpdateError::KeyStoreError)?; + let tree_position = TreePosition::new(self.group_id().clone(), self.own_leaf_index()); + let VerifiableLeafNode::Update(own_leaf) = + LeafNodeIn::from(own_leaf).try_into_verifiable_leaf_node(Some(tree_position))? + else { + return Err(LibraryError::custom( + "LeafNode source should have been set to 'update' at this point", + ) + .into()); + }; + let own_leaf = own_leaf.validate(self.group().public_group(), backend.crypto())?; + let update_proposal = self.group.create_update_proposal( self.framing_parameters(), own_leaf.clone(), @@ -226,7 +233,7 @@ impl MlsGroup { /// private key must be manually added to the key store. pub(crate) async fn _propose_explicit_self_update( &mut self, - _backend: &impl OpenMlsCryptoProvider, + backend: &impl OpenMlsCryptoProvider, signer: &impl Signer, leaf_node: LeafNode, leaf_node_signer: &impl Signer, @@ -238,14 +245,32 @@ impl MlsGroup { .ok_or_else(|| LibraryError::custom("The tree is broken. Couldn't find own leaf."))? .clone(); - own_leaf.update_and_re_sign( - None, - Some(leaf_node), - self.group_id().clone(), + let keypair = own_leaf.rekey( + self.group_id(), self.own_leaf_index(), + Some(leaf_node), + self.ciphersuite(), + ProtocolVersion::Mls10, + backend, leaf_node_signer, )?; + keypair + .write_to_key_store(backend) + .await + .map_err(ProposeSelfUpdateError::KeyStoreError)?; + + let tree_position = TreePosition::new(self.group_id().clone(), self.own_leaf_index()); + let VerifiableLeafNode::Update(own_leaf) = + LeafNodeIn::from(own_leaf).try_into_verifiable_leaf_node(Some(tree_position))? + else { + return Err(LibraryError::custom( + "LeafNode source should have been set to 'update' at this point", + ) + .into()); + }; + let own_leaf = own_leaf.validate(self.group().public_group(), backend.crypto())?; + let update_proposal = self.group.create_update_proposal( self.framing_parameters(), own_leaf.clone(), diff --git a/openmls/src/group/public_group/diff/compute_path.rs b/openmls/src/group/public_group/diff/compute_path.rs index 24140d3da0..7dae9463b3 100644 --- a/openmls/src/group/public_group/diff/compute_path.rs +++ b/openmls/src/group/public_group/diff/compute_path.rs @@ -1,8 +1,10 @@ use std::collections::HashSet; +use openmls_traits::types::Ciphersuite; use openmls_traits::{key_store::OpenMlsKeyStore, signatures::Signer, OpenMlsCryptoProvider}; use tls_codec::Serialize; +use crate::prelude::{Capabilities, CredentialType, ExtensionType, ProtocolVersion}; use crate::{ binary_tree::LeafNodeIndex, credentials::CredentialWithKey, @@ -62,15 +64,32 @@ impl<'a> PublicGroupDiff<'a> { // The KeyPackage is immediately put into the group. No need for // the init key. init_private_key: _, - } = KeyPackage::builder().build_without_key_storage( - CryptoConfig { - ciphersuite, - version, - }, - backend, - signer, - credential_with_key.ok_or(CreateCommitError::MissingCredential)?, - )?; + } = KeyPackage::builder() + .leaf_node_capabilities( + // TODO: factorize & have this injected by client + Capabilities::new( + Some(&[ProtocolVersion::Mls10]), + Some(&[ + Ciphersuite::MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519, + Ciphersuite::MLS_128_DHKEMP256_AES128GCM_SHA256_P256, + Ciphersuite::MLS_128_DHKEMX25519_CHACHA20POLY1305_SHA256_Ed25519, + Ciphersuite::MLS_256_DHKEMP384_AES256GCM_SHA384_P384, + Ciphersuite::MLS_128_X25519KYBER768DRAFT00_AES128GCM_SHA256_Ed25519, + ]), + Some(&[ExtensionType::PerDomainTrustAnchor]), + Some(&[]), + Some(&[CredentialType::Basic, CredentialType::X509]), + ), + ) + .build_without_key_storage( + CryptoConfig { + ciphersuite, + version, + }, + backend, + signer, + credential_with_key.ok_or(CreateCommitError::MissingCredential)?, + )?; let leaf_node: LeafNode = key_package.into(); self.diff diff --git a/openmls/src/group/public_group/mod.rs b/openmls/src/group/public_group/mod.rs index 532c08180b..87ac709e32 100644 --- a/openmls/src/group/public_group/mod.rs +++ b/openmls/src/group/public_group/mod.rs @@ -292,7 +292,7 @@ impl PublicGroup { } /// Get treesync. - fn treesync(&self) -> &TreeSync { + pub(crate) fn treesync(&self) -> &TreeSync { &self.treesync } diff --git a/openmls/src/group/public_group/process.rs b/openmls/src/group/public_group/process.rs index 7e60b362f3..518f28820b 100644 --- a/openmls/src/group/public_group/process.rs +++ b/openmls/src/group/public_group/process.rs @@ -161,7 +161,7 @@ impl PublicGroup { let unverified_message = self .parse_message(decrypted_message, None) .map_err(ProcessMessageError::from)?; - self.process_unverified_message(backend, unverified_message, &self.proposal_store) + self.process_unverified_message(backend, unverified_message, &self.proposal_store, self) } } @@ -198,12 +198,17 @@ impl PublicGroup { backend: &impl OpenMlsCryptoProvider, unverified_message: UnverifiedMessage, proposal_store: &ProposalStore, + group: &PublicGroup, ) -> Result { // Checks the following semantic validation: // - ValSem010 // - ValSem246 (as part of ValSem010) - let (content, credential) = - unverified_message.verify(self.ciphersuite(), backend.crypto(), self.version())?; + let (content, credential) = unverified_message.verify( + self.ciphersuite(), + backend.crypto(), + self.version(), + group, + )?; match content.sender() { Sender::Member(_) | Sender::NewMemberCommit | Sender::NewMemberProposal => { diff --git a/openmls/src/group/public_group/tests.rs b/openmls/src/group/public_group/tests.rs index 3cea877edc..630edfba8f 100644 --- a/openmls/src/group/public_group/tests.rs +++ b/openmls/src/group/public_group/tests.rs @@ -64,7 +64,11 @@ async fn public_group(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProv // === Alice adds Bob === let (message, welcome, _group_info) = alice_group - .add_members(backend, &alice_signer, &[bob_kpb.key_package().clone()]) + .add_members( + backend, + &alice_signer, + vec![bob_kpb.key_package().clone().into()], + ) .await .expect("Could not add member to group."); @@ -107,7 +111,11 @@ async fn public_group(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProv // === Bob adds Charlie === let (queued_messages, welcome, _group_info) = bob_group - .add_members(backend, &bob_signer, &[charlie_kpb.key_package().clone()]) + .add_members( + backend, + &bob_signer, + vec![charlie_kpb.key_package().clone().into()], + ) .await .unwrap(); diff --git a/openmls/src/group/public_group/validation.rs b/openmls/src/group/public_group/validation.rs index cd2fe1495a..8a2dca0c0c 100644 --- a/openmls/src/group/public_group/validation.rs +++ b/openmls/src/group/public_group/validation.rs @@ -265,7 +265,7 @@ impl PublicGroup { Ok(()) } - /// Validate capablities. This function implements the following checks: + /// Validate capabilities. This function implements the following checks: /// - ValSem106: Add Proposal: required capabilities /// - ValSem109: Update Proposal: required capabilities pub(crate) fn validate_capabilities( diff --git a/openmls/src/group/tests/external_add_proposal.rs b/openmls/src/group/tests/external_add_proposal.rs index e129ba1b76..6618a2db3f 100644 --- a/openmls/src/group/tests/external_add_proposal.rs +++ b/openmls/src/group/tests/external_add_proposal.rs @@ -81,7 +81,11 @@ async fn validation_test_setup( .await; let (_message, welcome, _group_info) = alice_group - .add_members(backend, &alice_signer_with_keys.signer, &[bob_key_package]) + .add_members( + backend, + &alice_signer_with_keys.signer, + vec![bob_key_package.clone().into()], + ) .await .expect("error adding Bob to group"); diff --git a/openmls/src/group/tests/external_remove_proposal.rs b/openmls/src/group/tests/external_remove_proposal.rs index f0880181a0..8c7b19a7d8 100644 --- a/openmls/src/group/tests/external_remove_proposal.rs +++ b/openmls/src/group/tests/external_remove_proposal.rs @@ -80,7 +80,11 @@ async fn validation_test_setup( .await; alice_group - .add_members(backend, &alice_signer_when_keys.signer, &[bob_key_package]) + .add_members( + backend, + &alice_signer_when_keys.signer, + vec![bob_key_package.clone().into()], + ) .await .expect("error adding Bob to group"); @@ -154,7 +158,11 @@ async fn external_remove_proposal_should_remove_member( .await .unwrap(); // commit the proposal - let ProcessedMessageContent::ProposalMessage(remove_proposal) = processed_message.into_content() else { panic!("Not a remove proposal");}; + let ProcessedMessageContent::ProposalMessage(remove_proposal) = + processed_message.into_content() + else { + panic!("Not a remove proposal"); + }; alice_group.store_pending_proposal(*remove_proposal); alice_group .commit_to_pending_proposals(backend, &alice_credential.signer) @@ -178,7 +186,11 @@ async fn external_remove_proposal_should_remove_member( .await .unwrap(); // commit the proposal - let ProcessedMessageContent::ProposalMessage(remove_proposal) = processed_message.into_content() else { panic!("Not a remove proposal");}; + let ProcessedMessageContent::ProposalMessage(remove_proposal) = + processed_message.into_content() + else { + panic!("Not a remove proposal"); + }; alice_group.store_pending_proposal(*remove_proposal); assert!(matches!( alice_group diff --git a/openmls/src/group/tests/test_commit_validation.rs b/openmls/src/group/tests/test_commit_validation.rs index 327208e6da..aa9ddfd052 100644 --- a/openmls/src/group/tests/test_commit_validation.rs +++ b/openmls/src/group/tests/test_commit_validation.rs @@ -87,7 +87,7 @@ async fn validation_test_setup( .add_members( backend, &alice_credential.signer, - &[bob_key_package, charlie_key_package], + vec![bob_key_package.into(), charlie_key_package.into()], ) .await .expect("error adding Bob to group"); diff --git a/openmls/src/group/tests/test_encoding.rs b/openmls/src/group/tests/test_encoding.rs index 9c59a52e58..0286b4dd5d 100644 --- a/openmls/src/group/tests/test_encoding.rs +++ b/openmls/src/group/tests/test_encoding.rs @@ -431,7 +431,7 @@ async fn test_welcome_message_encoding(backend: &impl OpenMlsCryptoProvider) { .expect("An unexpected error occurred.") .borrow(); - let charlie_key_package_bundle = charlie + let charlie_kpb = charlie .find_key_package_bundle(&charlie_key_package, backend) .expect("An unexpected error occurred."); @@ -440,7 +440,8 @@ async fn test_welcome_message_encoding(backend: &impl OpenMlsCryptoProvider) { assert!(CoreGroup::new_from_welcome( welcome, Some(group_state.public_group().export_ratchet_tree().into()), - charlie_key_package_bundle, + charlie_kpb.key_package(), + charlie_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) diff --git a/openmls/src/group/tests/test_external_commit_validation.rs b/openmls/src/group/tests/test_external_commit_validation.rs index a9d7406f25..b0dbf13669 100644 --- a/openmls/src/group/tests/test_external_commit_validation.rs +++ b/openmls/src/group/tests/test_external_commit_validation.rs @@ -194,7 +194,11 @@ async fn test_valsem242(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr .await; alice_group - .add_members(backend, &alice_credential.signer, &[bob_key_package]) + .add_members( + backend, + &alice_credential.signer, + vec![bob_key_package.clone().into()], + ) .await .unwrap(); alice_group.merge_pending_commit(backend).await.unwrap(); @@ -349,7 +353,11 @@ async fn test_valsem243(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr .await; alice_group - .add_members(backend, &alice_credential.signer, &[bob_key_package]) + .add_members( + backend, + &alice_credential.signer, + vec![bob_key_package.clone().into()], + ) .await .unwrap(); diff --git a/openmls/src/group/tests/test_framing.rs b/openmls/src/group/tests/test_framing.rs index afe248960e..5604874d9f 100644 --- a/openmls/src/group/tests/test_framing.rs +++ b/openmls/src/group/tests/test_framing.rs @@ -346,7 +346,7 @@ async fn bad_padding(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProvi Err(MessageDecryptionError::MalformedContent) ); } else { - assert!(matches!(verifiable_plaintext_result, Ok(_))) + assert!(verifiable_plaintext_result.is_ok()) } } } diff --git a/openmls/src/group/tests/test_framing_validation.rs b/openmls/src/group/tests/test_framing_validation.rs index ac92f29713..83c7f10e3d 100644 --- a/openmls/src/group/tests/test_framing_validation.rs +++ b/openmls/src/group/tests/test_framing_validation.rs @@ -87,7 +87,7 @@ async fn validation_test_setup( .add_members( backend, &alice_credential.signer, - &[bob_key_package.clone()], + vec![bob_key_package.clone().into()], ) .await .expect("Could not add member."); diff --git a/openmls/src/group/tests/test_gce_proposals.rs b/openmls/src/group/tests/test_gce_proposals.rs index 23718b5fa3..c738086f4b 100644 --- a/openmls/src/group/tests/test_gce_proposals.rs +++ b/openmls/src/group/tests/test_gce_proposals.rs @@ -191,7 +191,10 @@ async fn gce_proposal_can_roundtrip( .process_message(backend, MlsMessageIn::from(gce_proposal)) .await .unwrap(); - let ProcessedMessageContent::ProposalMessage(gce_proposal) = processed_message.into_content() else { panic!("Not a remove proposal");}; + let ProcessedMessageContent::ProposalMessage(gce_proposal) = processed_message.into_content() + else { + panic!("Not a remove proposal"); + }; bob_group.store_pending_proposal(*gce_proposal); let (commit, _, _) = bob_group .commit_to_pending_proposals(backend, &bob_signer) @@ -281,7 +284,10 @@ async fn validating_commit_with_more_than_one_gce_proposal_should_fail( .process_message(backend, MlsMessageIn::from(first_gce_proposal)) .await .unwrap(); - let ProcessedMessageContent::ProposalMessage(gce_proposal) = processed_message.into_content() else { panic!("Not a proposal");}; + let ProcessedMessageContent::ProposalMessage(gce_proposal) = processed_message.into_content() + else { + panic!("Not a proposal"); + }; bob_group.store_pending_proposal(*gce_proposal); // Bob creates a commit with just 1 GCE proposal @@ -362,7 +368,7 @@ async fn gce_proposal_must_be_applied_first_then_used_to_validate_other_add_prop .propose_add_member( backend, &alice_signer, - charlie_key_package_bundle.key_package(), + charlie_key_package_bundle.key_package().clone().into(), ) .unwrap(); @@ -370,14 +376,20 @@ async fn gce_proposal_must_be_applied_first_then_used_to_validate_other_add_prop .process_message(backend, MlsMessageIn::from(charlie_add_proposal)) .await .unwrap(); - let ProcessedMessageContent::ProposalMessage(add_proposal) = processed_message.into_content() else { panic!("Not a remove proposal");}; + let ProcessedMessageContent::ProposalMessage(add_proposal) = processed_message.into_content() + else { + panic!("Not a remove proposal"); + }; bob_group.store_pending_proposal(*add_proposal); let processed_message = bob_group .process_message(backend, MlsMessageIn::from(gce_proposal)) .await .unwrap(); - let ProcessedMessageContent::ProposalMessage(gce_proposal) = processed_message.into_content() else { panic!("Not a remove proposal");}; + let ProcessedMessageContent::ProposalMessage(gce_proposal) = processed_message.into_content() + else { + panic!("Not a remove proposal"); + }; bob_group.store_pending_proposal(*gce_proposal); assert_eq!(bob_group.pending_proposals().count(), 2); @@ -454,7 +466,11 @@ async fn gce_proposal_must_be_applied_first_then_used_to_validate_other_external .process_message(backend, MlsMessageIn::from(charlie_add_proposal)) .await .unwrap(); - let ProcessedMessageContent::ExternalJoinProposalMessage(charlie_add_proposal) = processed_message.into_content() else { panic!("Not a proposal");}; + let ProcessedMessageContent::ExternalJoinProposalMessage(charlie_add_proposal) = + processed_message.into_content() + else { + panic!("Not a proposal"); + }; alice_group.store_pending_proposal(*charlie_add_proposal); assert_eq!(alice_group.pending_proposals().count(), 2); @@ -506,7 +522,7 @@ async fn gce_proposal_must_be_applied_first_but_ignored_for_remove_proposals( .add_members( backend, &alice_signer, - &[charlie_key_package_bundle.key_package().clone()], + vec![charlie_key_package_bundle.key_package().clone().into()], ) .await .unwrap(); @@ -614,7 +630,7 @@ async fn gce_proposal_must_be_applied_first_but_ignored_for_external_remove_prop .add_members( backend, &alice_signer, - &[charlie_key_package_bundle.key_package().clone()], + vec![charlie_key_package_bundle.key_package().clone().into()], ) .await .unwrap(); @@ -639,7 +655,11 @@ async fn gce_proposal_must_be_applied_first_but_ignored_for_external_remove_prop .process_message(backend, MlsMessageIn::from(charlie_ext_remove_proposal)) .await .unwrap(); - let ProcessedMessageContent::ProposalMessage(charlie_ext_remove_proposal) = processed_message.into_content() else { panic!("Not a remove proposal");}; + let ProcessedMessageContent::ProposalMessage(charlie_ext_remove_proposal) = + processed_message.into_content() + else { + panic!("Not a remove proposal"); + }; alice_group.store_pending_proposal(*charlie_ext_remove_proposal); // Propose requiring ExternalSenders, which Charlie does not support @@ -712,7 +732,7 @@ pub async fn group_setup( .add_members( backend, &alice_signer, - &[bob_key_package_bundle.key_package().clone()], + vec![bob_key_package_bundle.key_package().clone().into()], ) .await .unwrap(); diff --git a/openmls/src/group/tests/test_group.rs b/openmls/src/group/tests/test_group.rs index c7fddfe64f..43515b228f 100644 --- a/openmls/src/group/tests/test_group.rs +++ b/openmls/src/group/tests/test_group.rs @@ -142,7 +142,7 @@ async fn create_commit_optional_path( .read::(bob_key_package.hpke_init_key().as_slice()) .await .unwrap(); - let bob_key_package_bundle = KeyPackageBundle { + let bob_kpb = KeyPackageBundle { key_package: bob_key_package, private_key: bob_private_key, }; @@ -153,7 +153,8 @@ async fn create_commit_optional_path( .welcome_option .expect("An unexpected error occurred."), Some(ratchet_tree.into()), - bob_key_package_bundle, + bob_kpb.key_package(), + bob_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) @@ -323,14 +324,14 @@ async fn group_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCrypto .await; // Generate KeyPackages - let bob_key_package_bundle = KeyPackageBundle::new( + let bob_kpb = KeyPackageBundle::new( backend, &bob_credential_with_keys.signer, ciphersuite, bob_credential_with_keys.credential_with_key.clone(), ) .await; - let bob_key_package = bob_key_package_bundle.key_package(); + let bob_key_package = bob_kpb.key_package(); // === Alice creates a group === let mut group_alice = CoreGroup::builder( @@ -384,7 +385,8 @@ async fn group_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCrypto .welcome_option .expect("An unexpected error occurred."), Some(ratchet_tree.into()), - bob_key_package_bundle, + bob_kpb.key_package(), + bob_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) @@ -673,14 +675,14 @@ async fn group_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCrypto ) .await; - let charlie_key_package_bundle = KeyPackageBundle::new( + let charlie_kpb = KeyPackageBundle::new( backend, &charlie_credential_with_keys.signer, ciphersuite, charlie_credential_with_keys.credential_with_key.clone(), ) .await; - let charlie_key_package = charlie_key_package_bundle.key_package().clone(); + let charlie_key_package = charlie_kpb.key_package().clone(); let add_charlie_proposal_bob = group_bob .create_add_proposal( @@ -742,7 +744,8 @@ async fn group_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCrypto .welcome_option .expect("An unexpected error occurred."), Some(ratchet_tree.into()), - charlie_key_package_bundle, + charlie_kpb.key_package(), + charlie_kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) diff --git a/openmls/src/group/tests/test_past_secrets.rs b/openmls/src/group/tests/test_past_secrets.rs index b97245fecd..0943e73c4c 100644 --- a/openmls/src/group/tests/test_past_secrets.rs +++ b/openmls/src/group/tests/test_past_secrets.rs @@ -70,7 +70,7 @@ async fn test_past_secrets_in_group( .add_members( backend, &alice_credential_with_keys.signer, - &[bob_key_package], + vec![bob_key_package.into()], ) .await .expect("An unexpected error occurred."); diff --git a/openmls/src/group/tests/test_proposal_validation.rs b/openmls/src/group/tests/test_proposal_validation.rs index eb26df5f48..5084f3d855 100644 --- a/openmls/src/group/tests/test_proposal_validation.rs +++ b/openmls/src/group/tests/test_proposal_validation.rs @@ -2,6 +2,7 @@ //! https://openmls.tech/book/message_validation.html#semantic-validation-of-proposals-covered-by-a-commit use openmls_rust_crypto::OpenMlsRustCrypto; +use openmls_traits::types::SignatureScheme; use openmls_traits::{ key_store::OpenMlsKeyStore, signatures::Signer, types::Ciphersuite, OpenMlsCryptoProvider, }; @@ -58,7 +59,7 @@ async fn generate_credential_with_key_and_key_package( async fn create_group_with_members( ciphersuite: Ciphersuite, alice_credential_with_key_and_signer: &CredentialWithKeyAndSigner, - member_key_packages: &[KeyPackage], + member_key_packages: Vec, backend: &impl OpenMlsCryptoProvider, ) -> Result<(MlsMessageIn, Welcome), AddMembersError> { let mut alice_group = MlsGroup::new_with_group_id( @@ -157,7 +158,7 @@ async fn validation_test_setup( .add_members( backend, &alice_credential_with_key_and_signer.signer, - &[bob_key_package], + vec![bob_key_package.into()], ) .await .unwrap(); @@ -304,7 +305,7 @@ async fn test_valsem101a(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoP let res = create_group_with_members( ciphersuite, &alice_credential_with_keys, - &[bob_key_package, charlie_key_package], + vec![bob_key_package.into(), charlie_key_package.into()], backend, ) .await; @@ -347,7 +348,7 @@ async fn test_valsem101a(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoP .add_members( backend, &alice_credential_with_key_and_signer.signer, - &[charlie_key_package], + vec![charlie_key_package.into()], ) .await .expect("Error creating self-update") @@ -471,7 +472,7 @@ async fn test_valsem102(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr let res = create_group_with_members( ciphersuite, &alice_credential_with_key, - &[bob_key_package, charlie_key_package], + vec![bob_key_package.into(), charlie_key_package.into()], backend, ) .await; @@ -514,7 +515,7 @@ async fn test_valsem102(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr .add_members( backend, &alice_credential_with_key_and_signer.signer, - &[charlie_key_package.clone()], + vec![charlie_key_package.clone().into()], ) .await .expect("Error creating self-update") @@ -662,7 +663,7 @@ async fn test_valsem101b(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoP .add_members( backend, &alice_credential_with_key.signer, - &[bob_key_package, target_key_package], + vec![bob_key_package.into(), target_key_package.into()], ) .await .expect_err("was able to add user with same signature key as a group member!"); @@ -678,7 +679,7 @@ async fn test_valsem101b(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoP .add_members( backend, &alice_credential_with_key.signer, - &[bob_key_package, target_key_package], + vec![bob_key_package.into(), target_key_package.into()], ) .await .expect("failed to add user with different signature keypair!"); @@ -688,7 +689,7 @@ async fn test_valsem101b(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoP .add_members( backend, &alice_credential_with_key.signer, - &[bob_key_package.clone()], + vec![bob_key_package.clone().into()], ) .await .unwrap(); @@ -707,7 +708,7 @@ async fn test_valsem101b(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoP .propose_remove_member(backend, &alice_credential_with_key.signer, bob_index) .unwrap(); alice_group - .add_members(backend, &alice_credential_with_key.signer, &[target_key_package]) + .add_members(backend, &alice_credential_with_key.signer, vec![target_key_package.into()]) .await .expect( "failed to add a user with the same identity as someone in the group (with a remove proposal)!", @@ -892,7 +893,7 @@ async fn test_valsem103_valsem104(ciphersuite: Ciphersuite, backend: &impl OpenM let res = create_group_with_members( ciphersuite, &alice_credential_with_key, - &[bob_key_package], + vec![bob_key_package.into()], backend, ) .await; @@ -903,9 +904,9 @@ async fn test_valsem103_valsem104(ciphersuite: Ciphersuite, backend: &impl OpenM res.expect_err("was able to add user with colliding init and encryption keys!"); assert!(matches!( err, - AddMembersError::CreateCommitError(CreateCommitError::ProposalValidationError( - ProposalValidationError::InitEncryptionKeyCollision - )) + AddMembersError::KeyPackageVerifyError( + KeyPackageVerifyError::InitKeyEqualsEncryptionKey + ) )); } KeyUniqueness::PositiveDifferentKey => { @@ -1081,17 +1082,16 @@ async fn test_valsem105(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr generate_credential_with_key_and_key_package("Charlie".into(), ciphersuite, backend) .await; - let kpi = KeyPackageIn::from(charlie_key_package.clone()); - kpi.validate(backend.crypto(), ProtocolVersion::Mls10) + let kpi: KeyPackageIn = charlie_key_package.clone().into(); + kpi.standalone_validate(backend.crypto(), ProtocolVersion::Mls10) .unwrap(); // Let's just pick a ciphersuite that's not the one we're testing right now. - let wrong_ciphersuite = match ciphersuite { - Ciphersuite::MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519 => { - Ciphersuite::MLS_128_DHKEMP256_AES128GCM_SHA256_P256 - } + let wrong_ciphersuite = match ciphersuite.signature_algorithm() { + SignatureScheme::ED25519 => Ciphersuite::MLS_128_DHKEMP256_AES128GCM_SHA256_P256, _ => Ciphersuite::MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519, }; + match key_package_version { KeyPackageTestVersion::WrongCiphersuite => { charlie_key_package.set_ciphersuite(wrong_ciphersuite) @@ -1130,13 +1130,6 @@ async fn test_valsem105(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr ) .await; - // Let's just pick a ciphersuite that's not the one we're testing right now. - let wrong_ciphersuite = match ciphersuite { - Ciphersuite::MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519 => { - Ciphersuite::MLS_128_DHKEMP256_AES128GCM_SHA256_P256 - } - _ => Ciphersuite::MLS_128_DHKEMX25519_AES128GCM_SHA256_Ed25519, - }; match key_package_version { KeyPackageTestVersion::WrongCiphersuite => { charlie_key_package.set_ciphersuite(wrong_ciphersuite) @@ -1171,13 +1164,26 @@ async fn test_valsem105(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr for proposal_inclusion in [ProposalInclusion::ByReference, ProposalInclusion::ByValue] { match proposal_inclusion { ProposalInclusion::ByReference => { - let _proposal = alice_group - .propose_add_member( - backend, - &alice_credential_with_key_and_signer.signer, - &test_kp, - ) - .unwrap(); + let proposal_result = alice_group.propose_add_member( + backend, + &alice_credential_with_key_and_signer.signer, + test_kp.clone().into(), + ); + + match key_package_version { + KeyPackageTestVersion::WrongCiphersuite => { + assert!(matches!( + proposal_result.unwrap_err(), + ProposeAddMemberError::KeyPackageVerifyError( + KeyPackageVerifyError::InvalidSignature + ) + )); + } + KeyPackageTestVersion::WrongVersion => {} + KeyPackageTestVersion::UnsupportedVersion => {} + KeyPackageTestVersion::UnsupportedCiphersuite => {} + KeyPackageTestVersion::ValidTestCase => {} + } let result = alice_group .commit_to_pending_proposals( @@ -1191,15 +1197,17 @@ async fn test_valsem105(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr KeyPackageTestVersion::ValidTestCase => { result.unwrap(); } + KeyPackageTestVersion::WrongCiphersuite + | KeyPackageTestVersion::WrongVersion => { + result.unwrap(); + } _ => { assert!(matches!( result.expect_err( "no error when committing add with key package with insufficient capabilities", ), - CommitToPendingProposalsError::CreateCommitError( - _ - ) - )) + CommitToPendingProposalsError::CreateCommitError(_) + )); } } } @@ -1208,7 +1216,7 @@ async fn test_valsem105(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr .add_members( backend, &alice_credential_with_key_and_signer.signer, - &[test_kp_2.clone()], + vec![test_kp_2.clone().into()], ) .await; @@ -1216,14 +1224,28 @@ async fn test_valsem105(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr KeyPackageTestVersion::ValidTestCase => { result.unwrap(); } + KeyPackageTestVersion::WrongCiphersuite => { + assert!(matches!( + result.unwrap_err(), + AddMembersError::KeyPackageVerifyError( + KeyPackageVerifyError::InvalidSignature + ) + )) + } + KeyPackageTestVersion::WrongVersion => { + assert!(matches!( + result.unwrap_err(), + AddMembersError::KeyPackageVerifyError( + KeyPackageVerifyError::InvalidProtocolVersion + ) + )) + } _ => { assert!(matches!( result.expect_err( "no error when committing add with key package with insufficient capabilities", ), - AddMembersError::CreateCommitError( - _ - ) + AddMembersError::CreateCommitError(_) )) } } @@ -1318,7 +1340,7 @@ async fn test_valsem105(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr ); let expected_error_2 = ProcessMessageError::ValidationError( ValidationError::KeyPackageVerifyError( - KeyPackageVerifyError::InvalidLeafNodeSignature, + KeyPackageVerifyError::InvalidSignature, ), ); let expected_error_3 = ProcessMessageError::ValidationError( @@ -1669,6 +1691,7 @@ async fn test_valsem108(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr /// Encryption key must be unique among existing members #[apply(ciphersuites_and_backends)] #[wasm_bindgen_test::wasm_bindgen_test] +#[ignore] // Not testable since update proposal methods impose rekeying and do not delegate this to callers async fn test_valsem110(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoProvider) { // Before we can test creation or reception of (invalid) proposals, we set // up a new group with Alice and Bob. diff --git a/openmls/src/group/tests/test_remove_operation.rs b/openmls/src/group/tests/test_remove_operation.rs index eb02d66d80..effd5011c2 100644 --- a/openmls/src/group/tests/test_remove_operation.rs +++ b/openmls/src/group/tests/test_remove_operation.rs @@ -93,7 +93,7 @@ async fn test_remove_operation_variants( .add_members( &alice_backend, &alice_credential_with_key_and_signer.signer, - &[bob_key_package, charlie_key_package], + vec![bob_key_package.into(), charlie_key_package.into()], ) .await .expect("An unexpected error occurred."); diff --git a/openmls/src/group/tests/test_update_extensions.rs b/openmls/src/group/tests/test_update_extensions.rs index 05af2fee7f..54865c0376 100644 --- a/openmls/src/group/tests/test_update_extensions.rs +++ b/openmls/src/group/tests/test_update_extensions.rs @@ -93,7 +93,10 @@ async fn gce_commit_can_roundtrip(ciphersuite: Ciphersuite, backend: &impl OpenM .process_message(backend, MlsMessageIn::from(gce_commit)) .await .unwrap(); - let ProcessedMessageContent::StagedCommitMessage(gce_commit) = processed_message.into_content() else { panic!("Not a remove proposal");}; + let ProcessedMessageContent::StagedCommitMessage(gce_commit) = processed_message.into_content() + else { + panic!("Not a remove proposal"); + }; bob_group .merge_staged_commit(backend, *gce_commit) .await diff --git a/openmls/src/group/tests/test_wire_format_policy.rs b/openmls/src/group/tests/test_wire_format_policy.rs index 647919a6d7..586276001a 100644 --- a/openmls/src/group/tests/test_wire_format_policy.rs +++ b/openmls/src/group/tests/test_wire_format_policy.rs @@ -73,7 +73,7 @@ async fn receive_message( .await; let (_message, welcome, _group_info) = alice_group - .add_members(backend, alice_signer, &[bob_key_package]) + .add_members(backend, alice_signer, vec![bob_key_package.into()]) .await .expect("Could not add member."); diff --git a/openmls/src/group/tests/utils.rs b/openmls/src/group/tests/utils.rs index 9152ac6d53..3baee12765 100644 --- a/openmls/src/group/tests/utils.rs +++ b/openmls/src/group/tests/utils.rs @@ -246,7 +246,7 @@ pub(crate) async fn setup( y.key_package() .hash_ref(backend.crypto()) .expect("Could not hash KeyPackage.") - == x.new_member() + == *x.new_member() }) }) .expect("An unexpected error occurred."); @@ -258,10 +258,10 @@ pub(crate) async fn setup( y.key_package() .hash_ref(backend.crypto()) .expect("Could not hash KeyPackage.") - == member_secret.new_member() + == *member_secret.new_member() }) .expect("An unexpected error occurred."); - let key_package_bundle = new_group_member + let kpb = new_group_member .key_package_bundles .borrow_mut() .remove(kpb_position); @@ -270,7 +270,8 @@ pub(crate) async fn setup( let new_group = match CoreGroup::new_from_welcome( welcome.clone(), Some(core_group.public_group().export_ratchet_tree().into()), - key_package_bundle, + kpb.key_package(), + kpb.private_key.clone(), backend, ResumptionPskStore::new(1024), ) diff --git a/openmls/src/key_packages/errors.rs b/openmls/src/key_packages/errors.rs index db9cda1b40..e6c9dd6df4 100644 --- a/openmls/src/key_packages/errors.rs +++ b/openmls/src/key_packages/errors.rs @@ -4,6 +4,7 @@ use thiserror::Error; +use crate::prelude::LeafNodeValidationError; use crate::{ciphersuite::signable::SignatureError, error::LibraryError}; /// KeyPackage verify error @@ -15,15 +16,15 @@ pub enum KeyPackageVerifyError { /// The lifetime of the leaf node is not valid. #[error("The lifetime of the leaf node is not valid.")] InvalidLifetime, - /// The lifetime of the leaf node is missing. - #[error("The lifetime of the leaf node is missing.")] - MissingLifetime, /// A key package extension is not supported in the leaf's capabilities. #[error("A key package extension is not supported in the leaf's capabilities.")] UnsupportedExtension, /// The key package signature is not valid. #[error("The key package signature is not valid.")] InvalidSignature, + /// The leaf node is not valid. + #[error(transparent)] + InvalidLeafNode(#[from] LeafNodeValidationError), /// The leaf node signature is not valid. #[error("The leaf node signature is not valid.")] InvalidLeafNodeSignature, diff --git a/openmls/src/key_packages/key_package_in.rs b/openmls/src/key_packages/key_package_in.rs index 2fb54bc286..62cb6d5067 100644 --- a/openmls/src/key_packages/key_package_in.rs +++ b/openmls/src/key_packages/key_package_in.rs @@ -5,7 +5,11 @@ use crate::{ ciphersuite::{signable::*, *}, credentials::*, extensions::Extensions, - treesync::node::leaf_node::{LeafNode, LeafNodeIn, VerifiableLeafNode}, + prelude::PublicGroup, + treesync::{ + node::leaf_node::{LeafNodeIn, VerifiableLeafNode}, + node::validate::ValidatableLeafNode, + }, versions::ProtocolVersion, }; use openmls_traits::{crypto::OpenMlsCrypto, types::Ciphersuite}; @@ -114,24 +118,52 @@ impl KeyPackageIn { self, crypto: &impl OpenMlsCrypto, protocol_version: ProtocolVersion, + group: &PublicGroup, + ) -> Result { + self._validate(crypto, protocol_version, Some(group)) + } + + /// Verify that this key package is valid disregarding the group it is supposed to be used with. + pub fn standalone_validate( + self, + crypto: &impl OpenMlsCrypto, + protocol_version: ProtocolVersion, + ) -> Result { + self._validate(crypto, protocol_version, None) + } + + fn _validate( + self, + crypto: &impl OpenMlsCrypto, + protocol_version: ProtocolVersion, + group: Option<&PublicGroup>, ) -> Result { // We first need to verify the LeafNode inside the KeyPackage - let leaf_node = self.payload.leaf_node.clone().into_verifiable_leaf_node(); + let signature_scheme = self.payload.ciphersuite.signature_algorithm(); let signature_key = &OpenMlsSignaturePublicKey::from_signature_key( self.payload.leaf_node.signature_key().clone(), - self.payload.ciphersuite.signature_algorithm(), + signature_scheme, ); - let leaf_node = match leaf_node { - VerifiableLeafNode::KeyPackage(leaf_node) => leaf_node - .verify::(crypto, signature_key) - .map_err(|_| KeyPackageVerifyError::InvalidLeafNodeSignature)?, + let verifiable_leaf_node = self + .payload + .leaf_node + .clone() + .try_into_verifiable_leaf_node(None)?; + let leaf_node = match verifiable_leaf_node { + VerifiableLeafNode::KeyPackage(leaf_node) => { + if let Some(group) = group { + leaf_node.validate(group, crypto)? + } else { + leaf_node.standalone_validate(crypto, signature_scheme)? + } + } _ => return Err(KeyPackageVerifyError::InvalidLeafNodeSourceType), }; // Verify that the protocol version is valid - if !self.version_is_supported(protocol_version) { + if !self.is_version_supported(protocol_version) { return Err(KeyPackageVerifyError::InvalidProtocolVersion); } @@ -140,48 +172,26 @@ impl KeyPackageIn { return Err(KeyPackageVerifyError::InitKeyEqualsEncryptionKey); } - let key_package_tbs = KeyPackageTbs { - protocol_version: self.payload.protocol_version, - ciphersuite: self.payload.ciphersuite, - init_key: self.payload.init_key, - leaf_node, - extensions: self.payload.extensions, - }; - // Verify the KeyPackage signature - let key_package = VerifiableKeyPackage::new(key_package_tbs, self.signature) + let key_package = VerifiableKeyPackage::new(self.payload.into(), self.signature) .verify::(crypto, signature_key) .map_err(|_| KeyPackageVerifyError::InvalidSignature)?; // Extension included in the extensions or leaf_node.extensions fields // MUST be included in the leaf_node.capabilities field. + let leaf_node = &key_package.payload.leaf_node; for extension in key_package.payload.extensions.iter() { - if !key_package - .payload - .leaf_node - .supports_extension(&extension.extension_type()) - { + if !leaf_node.supports_extension(&extension.extension_type()) { return Err(KeyPackageVerifyError::UnsupportedExtension); } } - // Ensure validity of the life time extension in the leaf node. - if let Some(life_time) = key_package.payload.leaf_node.life_time() { - if !life_time.is_valid() { - return Err(KeyPackageVerifyError::InvalidLifetime); - } - } else { - // This assumes that we only verify key packages with leaf nodes - // that were created for the key package. - return Err(KeyPackageVerifyError::MissingLifetime); - } - Ok(key_package) } /// Returns true if the protocol version is supported by this key package and /// false otherwise. - pub(crate) fn version_is_supported(&self, protocol_version: ProtocolVersion) -> bool { + pub(crate) fn is_version_supported(&self, protocol_version: ProtocolVersion) -> bool { self.payload.protocol_version == protocol_version } } diff --git a/openmls/src/key_packages/lifetime.rs b/openmls/src/key_packages/lifetime.rs index 6f089dd70e..7086dc6569 100644 --- a/openmls/src/key_packages/lifetime.rs +++ b/openmls/src/key_packages/lifetime.rs @@ -66,7 +66,7 @@ impl Lifetime { .duration_since(UNIX_EPOCH) .map(|duration| duration.as_secs()) { - Ok(elapsed) => self.not_before < elapsed && elapsed < self.not_after, + Ok(elapsed) => (self.not_before < elapsed) && (elapsed < self.not_after), Err(_) => { log::error!("SystemTime before UNIX EPOCH."); false diff --git a/openmls/src/key_packages/mod.rs b/openmls/src/key_packages/mod.rs index de78312915..87c4fe0716 100644 --- a/openmls/src/key_packages/mod.rs +++ b/openmls/src/key_packages/mod.rs @@ -656,6 +656,7 @@ pub(crate) struct KeyPackageBundle { } // Public `KeyPackageBundle` functions. +#[cfg(test)] impl KeyPackageBundle { /// Get a reference to the public part of this bundle, i.e. the [`KeyPackage`]. pub fn key_package(&self) -> &KeyPackage { diff --git a/openmls/src/key_packages/test_key_packages.rs b/openmls/src/key_packages/test_key_packages.rs index 07f456480e..4e733fc6d2 100644 --- a/openmls/src/key_packages/test_key_packages.rs +++ b/openmls/src/key_packages/test_key_packages.rs @@ -47,7 +47,7 @@ async fn generate_key_package(ciphersuite: Ciphersuite, backend: &impl OpenMlsCr let kpi = KeyPackageIn::from(key_package); assert!(kpi - .validate(backend.crypto(), ProtocolVersion::Mls10) + .standalone_validate(backend.crypto(), ProtocolVersion::Mls10) .is_ok()); } @@ -100,7 +100,7 @@ async fn application_id_extension(ciphersuite: Ciphersuite, backend: &impl OpenM let kpi = KeyPackageIn::from(key_package.clone()); assert!(kpi - .validate(backend.crypto(), ProtocolVersion::Mls10) + .standalone_validate(backend.crypto(), ProtocolVersion::Mls10) .is_ok()); // Check ID @@ -133,11 +133,11 @@ async fn key_package_validation(ciphersuite: Ciphersuite, backend: &impl OpenMls .tls_serialize_detached() .expect("An unexpected error occurred."); - let key_package_in = KeyPackageIn::tls_deserialize(&mut encoded.as_slice()).unwrap(); - let err = key_package_in - .validate(backend.crypto(), ProtocolVersion::Mls10) - .unwrap_err(); + let kpi = KeyPackageIn::tls_deserialize(&mut encoded.as_slice()).unwrap(); + let err = kpi + .standalone_validate(backend.crypto(), ProtocolVersion::Mls10) + .unwrap_err(); // Expect an invalid protocol version error assert_eq!(err, KeyPackageVerifyError::InvalidProtocolVersion); @@ -152,11 +152,11 @@ async fn key_package_validation(ciphersuite: Ciphersuite, backend: &impl OpenMls .tls_serialize_detached() .expect("An unexpected error occurred."); - let key_package_in = KeyPackageIn::tls_deserialize(&mut encoded.as_slice()).unwrap(); - let err = key_package_in - .validate(backend.crypto(), ProtocolVersion::Mls10) - .unwrap_err(); + let kpi = KeyPackageIn::tls_deserialize(&mut encoded.as_slice()).unwrap(); + let err = kpi + .standalone_validate(backend.crypto(), ProtocolVersion::Mls10) + .unwrap_err(); // Expect an invalid init/encryption key error assert_eq!(err, KeyPackageVerifyError::InitKeyEqualsEncryptionKey); } diff --git a/openmls/src/lib.rs b/openmls/src/lib.rs index 6ede86d116..562ba5c5e6 100644 --- a/openmls/src/lib.rs +++ b/openmls/src/lib.rs @@ -101,7 +101,7 @@ //! // The key package has to be retrieved from Maxim in some way. Most likely //! // via a server storing key packages for users. //! let (mls_message_out, welcome_out, group_info) = sasha_group -//! .add_members(backend, &sasha_signer, &[maxim_key_package]) +//! .add_members(backend, &sasha_signer, vec![maxim_key_package.into()]) //! .await //! .expect("Could not add members."); //! diff --git a/openmls/src/messages/mod.rs b/openmls/src/messages/mod.rs index b5bd58c232..82cf125dbf 100644 --- a/openmls/src/messages/mod.rs +++ b/openmls/src/messages/mod.rs @@ -12,6 +12,7 @@ use serde::{Deserialize, Serialize}; use thiserror::Error; use tls_codec::{Deserialize as TlsDeserializeTrait, Serialize as TlsSerializeTrait, *}; +use crate::prelude::PublicGroup; #[cfg(test)] use crate::schedule::psk::{ExternalPsk, Psk}; use crate::{ @@ -120,8 +121,8 @@ impl EncryptedGroupSecrets { } /// Returns the encrypted group secrets' new [`KeyPackageRef`]. - pub fn new_member(&self) -> KeyPackageRef { - self.new_member.clone() + pub fn new_member(&self) -> &KeyPackageRef { + &self.new_member } /// Returns a reference to the encrypted group secrets' encrypted group secrets. @@ -194,11 +195,12 @@ impl CommitIn { crypto: &impl OpenMlsCrypto, sender_context: SenderContext, protocol_version: ProtocolVersion, + group: &PublicGroup, ) -> Result { let proposals = self .proposals .into_iter() - .map(|p| p.validate(crypto, ciphersuite, protocol_version)) + .map(|p| p.validate(crypto, ciphersuite, protocol_version, group)) .collect::, _>>()?; let path = if let Some(path) = self.path { @@ -232,7 +234,7 @@ impl CommitIn { TreePosition::new(group_id, new_leaf_index) } }; - Some(path.into_verified(ciphersuite, crypto, tree_position)?) + Some(path.into_verified(crypto, tree_position, group)?) } else { None }; diff --git a/openmls/src/messages/proposals.rs b/openmls/src/messages/proposals.rs index abdc16fbee..87c3f0d3a0 100644 --- a/openmls/src/messages/proposals.rs +++ b/openmls/src/messages/proposals.rs @@ -274,7 +274,7 @@ impl Proposal { /// ``` #[derive(Debug, PartialEq, Clone, Serialize, Deserialize, TlsSerialize, TlsSize)] pub struct AddProposal { - pub(crate) key_package: KeyPackage, + pub key_package: KeyPackage, } impl AddProposal { diff --git a/openmls/src/messages/proposals_in.rs b/openmls/src/messages/proposals_in.rs index 1f289d390a..4f1d897379 100644 --- a/openmls/src/messages/proposals_in.rs +++ b/openmls/src/messages/proposals_in.rs @@ -6,7 +6,7 @@ //! [`ProposalType::is_supported()`] can be used. use crate::{ - ciphersuite::{hash_ref::ProposalRef, signable::Verifiable}, + ciphersuite::hash_ref::ProposalRef, credentials::CredentialWithKey, framing::SenderContext, group::errors::ValidationError, @@ -15,6 +15,8 @@ use crate::{ versions::ProtocolVersion, }; +use crate::prelude::PublicGroup; +use crate::treesync::node::validate::ValidatableLeafNode; use openmls_traits::{crypto::OpenMlsCrypto, types::Ciphersuite}; use serde::{Deserialize, Serialize}; use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; @@ -99,15 +101,16 @@ impl ProposalIn { ciphersuite: Ciphersuite, sender_context: Option, protocol_version: ProtocolVersion, + group: &PublicGroup, ) -> Result { Ok(match self { ProposalIn::Add(add) => { - Proposal::Add(add.validate(crypto, protocol_version, ciphersuite)?) + Proposal::Add(add.validate(crypto, protocol_version, ciphersuite, group)?) } ProposalIn::Update(update) => { let sender_context = sender_context.ok_or(ValidationError::CommitterIncludedOwnUpdate)?; - Proposal::Update(update.validate(crypto, ciphersuite, sender_context)?) + Proposal::Update(update.validate(crypto, sender_context, group)?) } ProposalIn::Remove(remove) => Proposal::Remove(remove), ProposalIn::PreSharedKey(psk) => Proposal::PreSharedKey(psk), @@ -149,8 +152,9 @@ impl AddProposalIn { crypto: &impl OpenMlsCrypto, protocol_version: ProtocolVersion, ciphersuite: Ciphersuite, + group: &PublicGroup, ) -> Result { - let key_package = self.key_package.validate(crypto, protocol_version)?; + let key_package = self.key_package.validate(crypto, protocol_version, group)?; // Verify that the ciphersuite is valid if key_package.ciphersuite() != ciphersuite { return Err(ValidationError::InvalidAddProposalCiphersuite); @@ -182,27 +186,20 @@ impl UpdateProposalIn { pub(crate) fn validate( self, crypto: &impl OpenMlsCrypto, - ciphersuite: Ciphersuite, sender_context: SenderContext, + group: &PublicGroup, ) -> Result { - let leaf_node = match self.leaf_node.into_verifiable_leaf_node() { - VerifiableLeafNode::Update(mut leaf_node) => { - let tree_position = match sender_context { - SenderContext::Member((group_id, leaf_index)) => { - TreePosition::new(group_id, leaf_index) - } - _ => return Err(ValidationError::InvalidSenderType), - }; - leaf_node.add_tree_position(tree_position); - let pk = &leaf_node - .signature_key() - .clone() - .into_signature_public_key_enriched(ciphersuite.signature_algorithm()); - - leaf_node - .verify(crypto, pk) - .map_err(|_| ValidationError::InvalidLeafNodeSignature)? + let tree_position = match sender_context { + SenderContext::Member((group_id, leaf_index)) => { + TreePosition::new(group_id, leaf_index) } + _ => return Err(ValidationError::InvalidSenderType), + }; + let verifiable_leaf_node = self + .leaf_node + .try_into_verifiable_leaf_node(Some(tree_position))?; + let leaf_node = match verifiable_leaf_node { + VerifiableLeafNode::Update(leaf_node) => leaf_node.validate(group, crypto)?, _ => return Err(ValidationError::InvalidLeafNodeSourceType), }; @@ -232,10 +229,11 @@ impl ProposalOrRefIn { crypto: &impl OpenMlsCrypto, ciphersuite: Ciphersuite, protocol_version: ProtocolVersion, + group: &PublicGroup, ) -> Result { Ok(match self { ProposalOrRefIn::Proposal(proposal_in) => ProposalOrRef::Proposal( - proposal_in.validate(crypto, ciphersuite, None, protocol_version)?, + proposal_in.validate(crypto, ciphersuite, None, protocol_version, group)?, ), ProposalOrRefIn::Reference(reference) => ProposalOrRef::Reference(reference), }) diff --git a/openmls/src/messages/tests/test_welcome.rs b/openmls/src/messages/tests/test_welcome.rs index f3b632df10..53aae69003 100644 --- a/openmls/src/messages/tests/test_welcome.rs +++ b/openmls/src/messages/tests/test_welcome.rs @@ -80,7 +80,7 @@ async fn test_welcome_context_mismatch( .expect("An unexpected error occurred."); let (_queued_message, welcome, _group_info) = alice_group - .add_members(backend, &alice_signer, &[bob_kp.clone()]) + .add_members(backend, &alice_signer, vec![bob_kp.clone().into()]) .await .expect("Could not add member to group."); diff --git a/openmls/src/prelude.rs b/openmls/src/prelude.rs index 1e43f3a6b9..00999d5f54 100644 --- a/openmls/src/prelude.rs +++ b/openmls/src/prelude.rs @@ -44,7 +44,7 @@ pub use crate::binary_tree::LeafNodeIndex; // TreeSync pub use crate::treesync::{ - errors::{ApplyUpdatePathError, PublicTreeError}, + errors::{ApplyUpdatePathError, LeafNodeValidationError, PublicTreeError}, node::leaf_node::{Capabilities, LeafNode}, node::parent_node::ParentNode, node::Node, diff --git a/openmls/src/test_utils/test_framework/client.rs b/openmls/src/test_utils/test_framework/client.rs index 6b6a7ff179..a956bdda2d 100644 --- a/openmls/src/test_utils/test_framework/client.rs +++ b/openmls/src/test_utils/test_framework/client.rs @@ -249,7 +249,7 @@ impl Client { &self, action_type: ActionType, group_id: &GroupId, - key_packages: &[KeyPackage], + key_packages: Vec, ) -> Result<(Vec, Option, Option), ClientError> { let mut groups = self.groups.write().await; let group = groups @@ -280,7 +280,7 @@ impl Client { let mut messages = Vec::new(); for key_package in key_packages { let message = group - .propose_add_member(&self.crypto, &signer, key_package) + .propose_add_member(&self.crypto, &signer, key_package.clone()) .map(|(out, _)| out)?; messages.push(message); } diff --git a/openmls/src/test_utils/test_framework/mod.rs b/openmls/src/test_utils/test_framework/mod.rs index c48f498bef..fafe0bc304 100644 --- a/openmls/src/test_utils/test_framework/mod.rs +++ b/openmls/src/test_utils/test_framework/mod.rs @@ -258,7 +258,6 @@ impl MlsGroupTestSetup { } CodecUse::StructMessages => welcome, }; - if self.use_codec == CodecUse::SerializedMessages {} let clients = self.clients.read().await; for egs in welcome.secrets() { let client_id = self @@ -567,10 +566,10 @@ impl MlsGroupTestSetup { let key_package = self .get_fresh_key_package(&addee, group.ciphersuite) .await?; - key_packages.push(key_package); + key_packages.push(key_package.into()); } let (messages, welcome_option, _) = adder - .add_members(action_type, &group.group_id, &key_packages) + .add_members(action_type, &group.group_id, key_packages) .await?; for message in messages { self.distribute_to_members(adder_id, group, &message.into()) diff --git a/openmls/src/tree/tests_and_kats/kats/kat_message_protection.rs b/openmls/src/tree/tests_and_kats/kats/kat_message_protection.rs index 4946b82611..b5980f5c6b 100644 --- a/openmls/src/tree/tests_and_kats/kats/kat_message_protection.rs +++ b/openmls/src/tree/tests_and_kats/kats/kat_message_protection.rs @@ -417,7 +417,12 @@ pub async fn run_test_vector( .parse_message(decrypted_message, group.message_secrets_store()) .unwrap(); let processed_message: AuthenticatedContent = processed_unverified_message - .verify(ciphersuite, backend.crypto(), ProtocolVersion::Mls10) + .verify( + ciphersuite, + backend.crypto(), + ProtocolVersion::Mls10, + group.public_group(), + ) .unwrap() .0; match processed_message.content().to_owned() { @@ -557,7 +562,12 @@ pub async fn run_test_vector( .parse_message(decrypted_message, group.message_secrets_store()) .unwrap(); let processed_message: AuthenticatedContent = processed_unverified_message - .verify(ciphersuite, backend.crypto(), ProtocolVersion::Mls10) + .verify( + ciphersuite, + backend.crypto(), + ProtocolVersion::Mls10, + group.public_group(), + ) .unwrap() .0; match processed_message.content().to_owned() { @@ -600,7 +610,12 @@ pub async fn run_test_vector( .parse_message(decrypted_message, group.message_secrets_store()) .unwrap(); let processed_message: AuthenticatedContent = processed_unverified_message - .verify(ciphersuite, backend.crypto(), ProtocolVersion::Mls10) + .verify( + ciphersuite, + backend.crypto(), + ProtocolVersion::Mls10, + group.public_group(), + ) .unwrap() .0; match processed_message.content().to_owned() { diff --git a/openmls/src/treesync/diff.rs b/openmls/src/treesync/diff.rs index 021d556cb0..013f04236b 100644 --- a/openmls/src/treesync/diff.rs +++ b/openmls/src/treesync/diff.rs @@ -105,7 +105,7 @@ impl<'a> TreeSyncDiff<'a> { direct_path .into_iter() - .zip(copath_resolutions.into_iter()) + .zip(copath_resolutions) .filter_map(|(index, resolution)| { // Filter out the nodes whose copath resolution is empty if !resolution.is_empty() { @@ -130,7 +130,7 @@ impl<'a> TreeSyncDiff<'a> { copath .into_iter() - .zip(copath_resolutions.into_iter()) + .zip(copath_resolutions) .filter_map(|(index, resolution)| { // Filter out the nodes whose copath resolution is empty if !resolution.is_empty() { @@ -659,7 +659,6 @@ impl<'a> TreeSyncDiff<'a> { // other child, the parent hash is valid. if left_descendant.is_none() ^ right_descendant.is_some() { return Err(TreeSyncParentHashError::InvalidParentHash); - } else { } } } diff --git a/openmls/src/treesync/errors.rs b/openmls/src/treesync/errors.rs index 541aa912ef..d225230bad 100644 --- a/openmls/src/treesync/errors.rs +++ b/openmls/src/treesync/errors.rs @@ -250,6 +250,9 @@ pub enum LeafNodeValidationError { /// The leaf node's encryption key is already used in the group. #[error("The leaf node's encryption key is already used in the group.")] EncryptionKeyAlreadyInUse, + /// The leaf node's encryption key is already used by the leaf node it tries to replace. + #[error("The leaf node's encryption key is already used by the leaf node it tries to replace")] + UpdatedEncryptionKeyAlreadyInUse, /// The leaf node source is invalid in the given context. #[error("The leaf node source is invalid in the given context.")] InvalidLeafNodeSource, @@ -259,6 +262,12 @@ pub enum LeafNodeValidationError { /// The credential used by a member is not supported by this leaf node. #[error("The credential used by a member is not supported by this leaf node.")] MemberCredentialNotSupportedByLeafNode, + /// A library error occurred. + #[error(transparent)] + LibraryError(#[from] LibraryError), + /// See [`SignatureError`] for more details. + #[error(transparent)] + SignatureError(#[from] SignatureError), } /// Errors that can happen during lifetime validation. @@ -281,4 +290,7 @@ pub enum UpdatePathError { /// See [`SignatureError`] for more details. #[error(transparent)] SignatureError(#[from] SignatureError), + /// See [`LeafNodeValidationError`] for more details. + #[error(transparent)] + LeafNodeValidationError(#[from] LeafNodeValidationError), } diff --git a/openmls/src/treesync/mod.rs b/openmls/src/treesync/mod.rs index eeb7e3fc93..0cd5b34e80 100644 --- a/openmls/src/treesync/mod.rs +++ b/openmls/src/treesync/mod.rs @@ -114,6 +114,9 @@ pub enum RatchetTreeError { /// Wrong node type. #[error("Wrong node type.")] WrongNodeType, + /// See [`LeafNodeValidationError`] for more details. + #[error(transparent)] + LeafNodeValidationError(#[from] LeafNodeValidationError), } impl RatchetTree { @@ -160,6 +163,7 @@ impl RatchetTree { // The ratchet tree is not empty, i.e., has a last node, and the last node is not blank. // Verify the nodes. + let signature_scheme = ciphersuite.signature_algorithm(); let mut verified_nodes = Vec::new(); for (index, node) in nodes.into_iter().enumerate() { let verified_node = match (index % 2, node) { @@ -169,30 +173,27 @@ impl RatchetTree { group_id.clone(), LeafNodeIndex::new((index / 2) as u32), ); - let verifiable_leaf_node = leaf_node.into_verifiable_leaf_node(); + let verifiable_leaf_node = + leaf_node.try_into_verifiable_leaf_node(Some(tree_position))?; + let signature_key = verifiable_leaf_node .signature_key() .clone() - .into_signature_public_key_enriched( - ciphersuite.signature_algorithm(), - ); - Some(Node::LeafNode(match verifiable_leaf_node { - VerifiableLeafNode::KeyPackage(leaf_node) => leaf_node - .verify(crypto, &signature_key) - .map_err(|_| RatchetTreeError::InvalidNodeSignature)?, - VerifiableLeafNode::Update(mut leaf_node) => { - leaf_node.add_tree_position(tree_position); - leaf_node - .verify(crypto, &signature_key) - .map_err(|_| RatchetTreeError::InvalidNodeSignature)? + .into_signature_public_key_enriched(signature_scheme); + + let leaf_node = match verifiable_leaf_node { + VerifiableLeafNode::KeyPackage(leaf_node) => { + leaf_node.verify(crypto, &signature_key) + } + VerifiableLeafNode::Update(leaf_node) => { + leaf_node.verify(crypto, &signature_key) } - VerifiableLeafNode::Commit(mut leaf_node) => { - leaf_node.add_tree_position(tree_position); - leaf_node - .verify(crypto, &signature_key) - .map_err(|_| RatchetTreeError::InvalidNodeSignature)? + VerifiableLeafNode::Commit(leaf_node) => { + leaf_node.verify(crypto, &signature_key) } - })) + } + .map_err(|_| RatchetTreeError::InvalidNodeSignature)?; + Some(Node::LeafNode(leaf_node)) } // Odd indices must be parent nodes. (1, Some(NodeIn::ParentNode(parent_node))) => { @@ -530,6 +531,11 @@ impl TreeSync { .filter_map(|(_, tsn)| tsn.node().as_ref()) } + /// Returns a list of nodes. + pub(crate) fn raw_leaves(&self) -> impl Iterator { + self.tree.raw_leaves().filter_map(|tsn| tsn.node().as_ref()) + } + /// Returns an indexed list of [`LeafNodeIndex`]es containing only full nodes. pub(crate) fn full_leaves_indexed(&self) -> impl Iterator { self.tree diff --git a/openmls/src/treesync/node.rs b/openmls/src/treesync/node.rs index 9bfb352dab..5d615c085e 100644 --- a/openmls/src/treesync/node.rs +++ b/openmls/src/treesync/node.rs @@ -12,6 +12,7 @@ mod codec; pub(crate) mod encryption_keys; pub(crate) mod leaf_node; pub(crate) mod parent_node; +pub(crate) mod validate; /// Container enum for leaf and parent nodes. /// diff --git a/openmls/src/treesync/node/leaf_node.rs b/openmls/src/treesync/node/leaf_node.rs index f0316d5f9d..54334d9cae 100644 --- a/openmls/src/treesync/node/leaf_node.rs +++ b/openmls/src/treesync/node/leaf_node.rs @@ -262,6 +262,11 @@ impl LeafNode { leaf_node_tbs.payload.credential = leaf_node.credential().clone(); leaf_node_tbs.payload.signature_key = leaf_node.signature_key().clone(); leaf_node_tbs.payload.encryption_key = leaf_node.encryption_key().clone(); + + // The application MAY specify other changes to the leaf node, e.g. [..] updated capabilities, or different extensions + // cf https://www.rfc-editor.org/rfc/rfc9420.html#section-7.5-7 + leaf_node_tbs.payload.capabilities = leaf_node.capabilities().clone(); + leaf_node_tbs.payload.extensions = leaf_node.extensions().clone(); } else if let Some(new_encryption_key) = new_encryption_key { leaf_node_tbs.payload.encryption_key = new_encryption_key; } else { @@ -375,7 +380,7 @@ impl LeafNode { } /// Return a reference to [`Capabilities`]. - pub(crate) fn capabilities(&self) -> &Capabilities { + pub fn capabilities(&self) -> &Capabilities { &self.payload.capabilities } @@ -534,13 +539,13 @@ impl LeafNode { #[derive( Debug, Clone, PartialEq, Eq, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, )] -struct LeafNodePayload { - encryption_key: EncryptionKey, - signature_key: SignaturePublicKey, - credential: Credential, - capabilities: Capabilities, - leaf_node_source: LeafNodeSource, - extensions: Extensions, +pub(crate) struct LeafNodePayload { + pub(crate) encryption_key: EncryptionKey, + pub(crate) signature_key: SignaturePublicKey, + pub(crate) credential: Credential, + pub(crate) capabilities: Capabilities, + pub(crate) leaf_node_source: LeafNodeSource, + pub(crate) extensions: Extensions, } #[derive( @@ -666,7 +671,7 @@ impl TreeInfoTbs { #[derive(Debug, Clone, PartialEq, Eq, TlsSerialize, TlsSize)] pub(crate) struct TreePosition { group_id: GroupId, - leaf_index: LeafNodeIndex, + pub(crate) leaf_index: LeafNodeIndex, } impl TreePosition { @@ -689,32 +694,34 @@ pub struct LeafNodeIn { } impl LeafNodeIn { - pub(crate) fn into_verifiable_leaf_node(self) -> VerifiableLeafNode { - match self.payload.leaf_node_source { + pub(crate) fn try_into_verifiable_leaf_node( + self, + tree_position: Option, + ) -> Result { + Ok(match self.payload.leaf_node_source { LeafNodeSource::KeyPackage(_) => { - let verifiable = VerifiableKeyPackageLeafNode { + VerifiableLeafNode::KeyPackage(VerifiableKeyPackageLeafNode { payload: self.payload, signature: self.signature, - }; - VerifiableLeafNode::KeyPackage(verifiable) + }) } LeafNodeSource::Update => { - let verifiable = VerifiableUpdateLeafNode { + let tree_position = tree_position.ok_or(LibraryError::custom("Internal error"))?; + VerifiableLeafNode::Update(VerifiableUpdateLeafNode { payload: self.payload, signature: self.signature, - tree_position: None, - }; - VerifiableLeafNode::Update(verifiable) + tree_position, + }) } LeafNodeSource::Commit(_) => { - let verifiable = VerifiableCommitLeafNode { + let tree_position = tree_position.ok_or(LibraryError::custom("Internal error"))?; + VerifiableLeafNode::Commit(VerifiableCommitLeafNode { payload: self.payload, signature: self.signature, - tree_position: None, - }; - VerifiableLeafNode::Commit(verifiable) + tree_position, + }) } - } + }) } /// Returns the `signature_key` as byte slice. @@ -771,7 +778,7 @@ impl VerifiableLeafNode { #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct VerifiableKeyPackageLeafNode { - payload: LeafNodePayload, + pub(crate) payload: LeafNodePayload, signature: Signature, } @@ -808,32 +815,28 @@ impl VerifiedStruct for LeafNode { #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct VerifiableUpdateLeafNode { - payload: LeafNodePayload, + pub(crate) payload: LeafNodePayload, signature: Signature, - tree_position: Option, + pub(crate) tree_position: TreePosition, } impl VerifiableUpdateLeafNode { - pub(crate) fn add_tree_position(&mut self, tree_info: TreePosition) { - self.tree_position = Some(tree_info); - } - pub(crate) fn signature_key(&self) -> &SignaturePublicKey { &self.payload.signature_key } + + pub(crate) fn encryption_key(&self) -> &EncryptionKey { + &self.payload.encryption_key + } } impl Verifiable for VerifiableUpdateLeafNode { fn unsigned_payload(&self) -> Result, tls_codec::Error> { - let tree_info_tbs = match &self.tree_position { - Some(tree_position) => TreeInfoTbs::Commit(tree_position.clone()), - None => return Err(tls_codec::Error::InvalidInput), - }; - let leaf_node_tbs = LeafNodeTbs { + LeafNodeTbs { payload: self.payload.clone(), - tree_info_tbs, - }; - leaf_node_tbs.tls_serialize_detached() + tree_info_tbs: TreeInfoTbs::Commit(self.tree_position.clone()), + } + .tls_serialize_detached() } fn signature(&self) -> &Signature { @@ -858,16 +861,12 @@ impl VerifiedStruct for LeafNode { #[derive(Debug, Clone, PartialEq, Eq)] pub(crate) struct VerifiableCommitLeafNode { - payload: LeafNodePayload, + pub(crate) payload: LeafNodePayload, signature: Signature, - tree_position: Option, + tree_position: TreePosition, } impl VerifiableCommitLeafNode { - pub(crate) fn add_tree_position(&mut self, tree_info: TreePosition) { - self.tree_position = Some(tree_info); - } - pub(crate) fn signature_key(&self) -> &SignaturePublicKey { &self.payload.signature_key } @@ -875,16 +874,11 @@ impl VerifiableCommitLeafNode { impl Verifiable for VerifiableCommitLeafNode { fn unsigned_payload(&self) -> Result, tls_codec::Error> { - let tree_info_tbs = match &self.tree_position { - Some(tree_position) => TreeInfoTbs::Commit(tree_position.clone()), - None => return Err(tls_codec::Error::InvalidInput), - }; - let leaf_node_tbs = LeafNodeTbs { + LeafNodeTbs { payload: self.payload.clone(), - tree_info_tbs, - }; - - leaf_node_tbs.tls_serialize_detached() + tree_info_tbs: TreeInfoTbs::Commit(self.tree_position.clone()), + } + .tls_serialize_detached() } fn signature(&self) -> &Signature { diff --git a/openmls/src/treesync/node/leaf_node/capabilities.rs b/openmls/src/treesync/node/leaf_node/capabilities.rs index 4e8bb445a6..73970981a1 100644 --- a/openmls/src/treesync/node/leaf_node/capabilities.rs +++ b/openmls/src/treesync/node/leaf_node/capabilities.rs @@ -27,11 +27,11 @@ use crate::{ Debug, Clone, PartialEq, Eq, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, )] pub struct Capabilities { - pub(super) versions: Vec, - pub(super) ciphersuites: Vec, - pub(super) extensions: Vec, - pub(super) proposals: Vec, - pub(super) credentials: Vec, + pub versions: Vec, + pub ciphersuites: Vec, + pub extensions: Vec, + pub proposals: Vec, + pub credentials: Vec, } impl Capabilities { diff --git a/openmls/src/treesync/node/validate.rs b/openmls/src/treesync/node/validate.rs new file mode 100644 index 0000000000..d0781c2766 --- /dev/null +++ b/openmls/src/treesync/node/validate.rs @@ -0,0 +1,247 @@ +use crate::{ + prelude::{ + Capabilities, CredentialType, ExtensionType, Extensions, SignaturePublicKey, Verifiable, + }, + prelude::{Extension, LeafNode, LibraryError, PublicGroup, VerifiedStruct}, + treesync::node::leaf_node::LeafNodeSource, + treesync::TreeSync, + treesync::{ + errors::{LeafNodeValidationError, LifetimeError}, + node::leaf_node::{ + VerifiableCommitLeafNode, VerifiableKeyPackageLeafNode, VerifiableUpdateLeafNode, + }, + }, +}; +use itertools::Itertools; +use openmls_traits::{crypto::OpenMlsCrypto, types::SignatureScheme}; + +impl ValidatableLeafNode for VerifiableCommitLeafNode { + fn signature_key(&self) -> &SignaturePublicKey { + self.signature_key() + } + + fn capabilities(&self) -> &Capabilities { + &self.payload.capabilities + } + + fn credential_type(&self) -> &CredentialType { + &self.payload.credential.credential_type + } + + fn extensions(&self) -> &Extensions { + &self.payload.extensions + } +} + +impl ValidatableLeafNode for VerifiableUpdateLeafNode { + fn validate( + self, + group: &PublicGroup, + crypto: &impl OpenMlsCrypto, + ) -> Result { + self.validate_replaced_encryption_key(group)?; + self.generic_validate(crypto, group) + } + + fn signature_key(&self) -> &SignaturePublicKey { + self.signature_key() + } + + fn capabilities(&self) -> &Capabilities { + &self.payload.capabilities + } + + fn credential_type(&self) -> &CredentialType { + &self.payload.credential.credential_type + } + + fn extensions(&self) -> &Extensions { + &self.payload.extensions + } +} + +impl VerifiableUpdateLeafNode { + /// Verify that encryption_key represents a different public key than the encryption_key in the leaf node being replaced by the Update proposal. + fn validate_replaced_encryption_key( + &self, + group: &PublicGroup, + ) -> Result<(), LeafNodeValidationError> { + let index = self.tree_position.leaf_index; + let leaf_to_replace = group + .leaf(index) + .ok_or(LibraryError::custom("Invalid update proposal"))?; + if leaf_to_replace.encryption_key() == self.encryption_key() { + return Err(LeafNodeValidationError::UpdatedEncryptionKeyAlreadyInUse); + } + Ok(()) + } +} + +impl ValidatableLeafNode for VerifiableKeyPackageLeafNode { + fn validate( + self, + group: &PublicGroup, + crypto: &impl OpenMlsCrypto, + ) -> Result { + self.validate_lifetime()?; + self.generic_validate(crypto, group) + } + + fn signature_key(&self) -> &SignaturePublicKey { + self.signature_key() + } + + fn capabilities(&self) -> &Capabilities { + &self.payload.capabilities + } + + fn credential_type(&self) -> &CredentialType { + &self.payload.credential.credential_type + } + + fn extensions(&self) -> &Extensions { + &self.payload.extensions + } +} + +impl VerifiableKeyPackageLeafNode { + fn validate_lifetime(&self) -> Result<(), LeafNodeValidationError> { + let LeafNodeSource::KeyPackage(lifetime) = self.payload.leaf_node_source else { + return Err(LeafNodeValidationError::InvalidLeafNodeSource); + }; + if !lifetime.is_valid() { + return Err(LeafNodeValidationError::Lifetime(LifetimeError::NotCurrent)); + } + Ok(()) + } +} + +pub(crate) trait ValidatableLeafNode: Verifiable + Sized +where + LeafNode: VerifiedStruct, +{ + fn standalone_validate( + self, + crypto: &impl OpenMlsCrypto, + signature_scheme: SignatureScheme, + ) -> Result { + let extension_types = self.extension_types(); + let leaf_node = self.verify_signature(crypto, signature_scheme)?; + Self::validate_extension_support(&leaf_node, &extension_types[..])?; + Ok(leaf_node) + } + + /// Validate a LeafNode as per https://www.rfc-editor.org/rfc/rfc9420.html#name-leaf-node-validation + fn validate( + self, + group: &PublicGroup, + crypto: &impl OpenMlsCrypto, + ) -> Result { + self.generic_validate(crypto, group) + } + + /// Validation regardless of [LeafNode]'s source + fn generic_validate( + self, + crypto: &impl OpenMlsCrypto, + group: &PublicGroup, + ) -> Result { + self.validate_capabilities(group)?; + self.validate_credential_type(group)?; + let tree = group.treesync(); + self.validate_signature_key_unique(tree)?; + self.validate_encryption_key_unique(tree)?; + + let extension_types = self.extension_types(); + let signature_scheme = group.ciphersuite().signature_algorithm(); + let leaf_node = self.verify_signature(crypto, signature_scheme)?; + Self::validate_extension_support(&leaf_node, &extension_types[..])?; + Ok(leaf_node) + } + + fn signature_key(&self) -> &SignaturePublicKey; + fn capabilities(&self) -> &Capabilities; + fn credential_type(&self) -> &CredentialType; + fn extensions(&self) -> &Extensions; + + fn extension_types(&self) -> Vec { + self.extensions() + .iter() + .map(Extension::extension_type) + .collect::>() + } + + /// Verify that the LeafNode is compatible with the group's parameters. + /// If the GroupContext has a required_capabilities extension, then the required extensions, proposals, + /// and credential types MUST be listed in the LeafNode's capabilities field. + fn validate_capabilities(&self, group: &PublicGroup) -> Result<(), LeafNodeValidationError> { + if let Some(group_required_capabilities) = group.required_capabilities() { + self.capabilities() + .supports_required_capabilities(group_required_capabilities)?; + } + Ok(()) + } + + /// (1) Verify that the credential type is supported by all members of the group, as specified by the capabilities field of each member's LeafNode + /// (2) and that the capabilities field of this LeafNode indicates support for all the credential types currently in use by other members. + fn validate_credential_type(&self, group: &PublicGroup) -> Result<(), LeafNodeValidationError> { + for leaf_node in group.treesync().raw_leaves() { + // (1) + let own_ct = self.credential_type(); + if !leaf_node.capabilities().contains_credential(own_ct) { + return Err(LeafNodeValidationError::LeafNodeCredentialNotSupportedByMember); + } + + // (2) + let leaf_node_ct = leaf_node.credential().credential_type(); + if !self.capabilities().contains_credential(&leaf_node_ct) { + return Err(LeafNodeValidationError::MemberCredentialNotSupportedByLeafNode); + } + } + Ok(()) + } + + /// Verify that the following fields are unique among the members of the group: signature_key + fn validate_signature_key_unique( + &self, + tree: &TreeSync, + ) -> Result<(), LeafNodeValidationError> { + if !tree.raw_leaves().map(LeafNode::signature_key).all_unique() { + return Err(LeafNodeValidationError::SignatureKeyAlreadyInUse); + } + Ok(()) + } + /// Verify that the following fields are unique among the members of the group: encryption_key + fn validate_encryption_key_unique( + &self, + tree: &TreeSync, + ) -> Result<(), LeafNodeValidationError> { + if !tree.raw_leaves().map(LeafNode::encryption_key).all_unique() { + return Err(LeafNodeValidationError::EncryptionKeyAlreadyInUse); + } + + Ok(()) + } + + /// Verify that the extensions in the LeafNode are supported by checking that the ID for each extension in the extensions + /// field is listed in the capabilities.extensions field of the LeafNode. + fn validate_extension_support( + leaf_node: &LeafNode, + extensions: &[ExtensionType], + ) -> Result<(), LeafNodeValidationError> { + leaf_node.check_extension_support(extensions) + } + + /// Verify that the signature on the LeafNode is valid using signature_key. + fn verify_signature( + self, + crypto: &impl OpenMlsCrypto, + signature_scheme: SignatureScheme, + ) -> Result { + let pk = self + .signature_key() + .clone() + .into_signature_public_key_enriched(signature_scheme); + Ok(self.verify::(crypto, &pk)?) + } +} diff --git a/openmls/src/treesync/tests_and_kats/kats/kat_tree_validation.rs b/openmls/src/treesync/tests_and_kats/kats/kat_tree_validation.rs index 5ca6f56711..6ea094d387 100644 --- a/openmls/src/treesync/tests_and_kats/kats/kat_tree_validation.rs +++ b/openmls/src/treesync/tests_and_kats/kats/kat_tree_validation.rs @@ -100,7 +100,7 @@ fn run_test_vector(test: TestElement, backend: &impl OpenMlsCryptoProvider) -> R let group_id = &GroupId::from_slice(test.group_id.as_slice()); let Ok(ratchet_tree_in) = RatchetTreeIn::tls_deserialize_exact(test.tree) else { // ! Some trees are malformed in the test vectors, ignore them - return Ok(()) + return Ok(()); }; let ratchet_tree = ratchet_tree_in .into_verified(ciphersuite, backend.crypto(), group_id) diff --git a/openmls/src/treesync/tests_and_kats/tests.rs b/openmls/src/treesync/tests_and_kats/tests.rs index cd14121b56..3af61bfb1e 100644 --- a/openmls/src/treesync/tests_and_kats/tests.rs +++ b/openmls/src/treesync/tests_and_kats/tests.rs @@ -107,7 +107,11 @@ async fn that_commit_secret_is_derived_from_end_of_update_path_not_root( .add_members( &alice.backend, &alice.credential_with_key_and_signer.signer, - &[bob.key_package, charlie.key_package, dave.key_package], + vec![ + bob.key_package.into(), + charlie.key_package.into(), + dave.key_package.into(), + ], ) .await .expect("Adding members failed."); diff --git a/openmls/src/treesync/treekem.rs b/openmls/src/treesync/treekem.rs index 72f52c0121..0904ae253a 100644 --- a/openmls/src/treesync/treekem.rs +++ b/openmls/src/treesync/treekem.rs @@ -26,10 +26,12 @@ use super::{ }; use crate::{ binary_tree::array_representation::LeafNodeIndex, - ciphersuite::{hpke, signable::Verifiable, HpkePublicKey}, + ciphersuite::{hpke, HpkePublicKey}, error::LibraryError, messages::{proposals::AddProposal, EncryptedGroupSecrets, GroupSecrets, PathSecret}, + prelude::PublicGroup, schedule::{psk::PreSharedKeyId, CommitSecret, JoinerSecret}, + treesync::node::validate::ValidatableLeafNode, treesync::node::NodeReference, versions::ProtocolVersion, }; @@ -379,21 +381,16 @@ impl UpdatePathIn { /// Return a verified [`UpdatePath`]. pub(crate) fn into_verified( self, - ciphersuite: Ciphersuite, crypto: &impl OpenMlsCrypto, tree_position: TreePosition, + group: &PublicGroup, ) -> Result { let leaf_node_in = self.leaf_node().clone(); - let verifiable_leaf_node = leaf_node_in.into_verifiable_leaf_node(); + let verifiable_leaf_node = + leaf_node_in.try_into_verifiable_leaf_node(Some(tree_position))?; match verifiable_leaf_node { - VerifiableLeafNode::Commit(mut commit_leaf_node) => { - let pk = &commit_leaf_node - .signature_key() - .clone() - .into_signature_public_key_enriched(ciphersuite.signature_algorithm()); - commit_leaf_node.add_tree_position(tree_position); - - let leaf_node: LeafNode = commit_leaf_node.verify(crypto, pk)?; + VerifiableLeafNode::Commit(commit_leaf_node) => { + let leaf_node = commit_leaf_node.validate(group, crypto)?; Ok(UpdatePath { leaf_node, nodes: self.nodes, diff --git a/openmls/src/utils.rs b/openmls/src/utils.rs index 573eeed07a..5724702412 100644 --- a/openmls/src/utils.rs +++ b/openmls/src/utils.rs @@ -79,6 +79,6 @@ pub mod vector_converter { V: Deserialize<'de>, { let container: Vec<_> = serde::Deserialize::deserialize(des)?; - Ok(T::from_iter(container.into_iter())) + Ok(T::from_iter(container)) } } diff --git a/openmls/tests/book_code.rs b/openmls/tests/book_code.rs index a69311309f..6d05f622c6 100644 --- a/openmls/tests/book_code.rs +++ b/openmls/tests/book_code.rs @@ -167,7 +167,7 @@ async fn book_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoP // === Alice adds Bob === // ANCHOR: alice_adds_bob let (mls_message_out, welcome, group_info) = alice_group - .add_members(backend, &alice_signature_keys, &[bob_key_package]) + .add_members(backend, &alice_signature_keys, vec![bob_key_package.into()]) .await .expect("Could not add members."); // ANCHOR_END: alice_adds_bob @@ -450,7 +450,11 @@ async fn book_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoP .await; let (queued_message, welcome, _group_info) = bob_group - .add_members(backend, &bob_signature_keys, &[charlie_key_package]) + .add_members( + backend, + &bob_signature_keys, + vec![charlie_key_package.into()], + ) .await .unwrap(); @@ -838,7 +842,11 @@ async fn book_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoP // Create AddProposal and remove it // ANCHOR: rollback_proposal_by_ref let (_mls_message_out, proposal_ref) = alice_group - .propose_add_member(backend, &alice_signature_keys, &bob_key_package) + .propose_add_member( + backend, + &alice_signature_keys, + bob_key_package.clone().into(), + ) .expect("Could not create proposal to add Bob"); alice_group .remove_pending_proposal(backend.key_store(), &proposal_ref) @@ -849,7 +857,11 @@ async fn book_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoP // Create AddProposal and process it // ANCHOR: propose_add let (mls_message_out, _proposal_ref) = alice_group - .propose_add_member(backend, &alice_signature_keys, &bob_key_package) + .propose_add_member( + backend, + &alice_signature_keys, + bob_key_package.clone().into(), + ) .expect("Could not create proposal to add Bob"); // ANCHOR_END: propose_add @@ -1260,7 +1272,7 @@ async fn book_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoP // Add Bob to the group let (_queued_message, welcome, _group_info) = alice_group - .add_members(backend, &alice_signature_keys, &[bob_key_package]) + .add_members(backend, &alice_signature_keys, vec![bob_key_package.into()]) .await .expect("Could not add Bob"); @@ -1331,7 +1343,7 @@ async fn test_empty_input_errors(ciphersuite: Ciphersuite, backend: &impl OpenMl assert!(matches!( alice_group - .add_members(backend, &alice_signature_keys, &[]) + .add_members(backend, &alice_signature_keys, vec![]) .await .expect_err("No EmptyInputError when trying to pass an empty slice to `add_members`."), AddMembersError::EmptyInput(EmptyInputError::AddMembers) diff --git a/openmls/tests/test_mls_group.rs b/openmls/tests/test_mls_group.rs index 8f53c191e6..ad78b3c3c2 100644 --- a/openmls/tests/test_mls_group.rs +++ b/openmls/tests/test_mls_group.rs @@ -87,7 +87,7 @@ async fn mls_group_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCr // === Alice adds Bob === let welcome = match alice_group - .add_members(backend, &alice_signer, &[bob_key_package]) + .add_members(backend, &alice_signer, vec![bob_key_package.into()]) .await { Ok((_, welcome, _)) => welcome, @@ -327,7 +327,7 @@ async fn mls_group_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCr .await; let (queued_message, welcome, _group_info) = bob_group - .add_members(backend, &bob_signer, &[charlie_key_package]) + .add_members(backend, &bob_signer, vec![charlie_key_package.into()]) .await .unwrap(); @@ -648,7 +648,7 @@ async fn mls_group_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCr // Create AddProposal and process it let (queued_message, _) = alice_group - .propose_add_member(backend, &alice_signer, &bob_key_package) + .propose_add_member(backend, &alice_signer, bob_key_package.into()) .expect("Could not create proposal to add Bob"); let charlie_processed_message = charlie_group @@ -917,7 +917,7 @@ async fn mls_group_operations(ciphersuite: Ciphersuite, backend: &impl OpenMlsCr // Add Bob to the group let (_queued_message, welcome, _group_info) = alice_group - .add_members(backend, &alice_signer, &[bob_key_package]) + .add_members(backend, &alice_signer, vec![bob_key_package.into()]) .await .expect("Could not add Bob"); @@ -998,7 +998,7 @@ async fn test_empty_input_errors(ciphersuite: Ciphersuite, backend: &impl OpenMl assert!(matches!( alice_group - .add_members(backend, &alice_signer, &[]) + .add_members(backend, &alice_signer, vec![]) .await .expect_err("No EmptyInputError when trying to pass an empty slice to `add_members`."), AddMembersError::EmptyInput(EmptyInputError::AddMembers) @@ -1061,7 +1061,7 @@ async fn mls_group_ratchet_tree_extension( // === Alice adds Bob === let (_queued_message, welcome, _group_info) = alice_group - .add_members(backend, &alice_signer, &[bob_key_package.clone()]) + .add_members(backend, &alice_signer, vec![bob_key_package.into()]) .await .unwrap(); @@ -1109,7 +1109,7 @@ async fn mls_group_ratchet_tree_extension( // === Alice adds Bob === let (_queued_message, welcome, _group_info) = alice_group - .add_members(backend, &alice_signer, &[bob_key_package]) + .add_members(backend, &alice_signer, vec![bob_key_package.into()]) .await .unwrap(); @@ -1178,7 +1178,7 @@ async fn addition_order(ciphersuite: Ciphersuite, backend: &impl OpenMlsCryptoPr .add_members( backend, &alice_signer, - &[bob_key_package, charlie_key_package], + vec![bob_key_package.into(), charlie_key_package.into()], ) .await { diff --git a/traits/src/types.rs b/traits/src/types.rs index bf2626f8cb..23ec0b0174 100644 --- a/traits/src/types.rs +++ b/traits/src/types.rs @@ -323,6 +323,14 @@ impl From for VerifiableCiphersuite { } } +impl TryFrom for Ciphersuite { + type Error = tls_codec::Error; + + fn try_from(value: VerifiableCiphersuite) -> Result { + value.0.try_into() + } +} + /// MLS ciphersuites. #[allow(non_camel_case_types)] #[allow(clippy::upper_case_acronyms)]