diff --git a/CHANGELOG.md b/CHANGELOG.md index 16712e2efc..11a1376e1d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,11 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed - [#1464](https://github.com/openmls/openmls/pull/1464): Add builder pattern for `MlsGroup`; split `MlsGroupJoinConfig` into `MlsGroupCreateConfig` and `MlsGroupJoinConfig` +- [#1473](https://github.com/openmls/openmls/pull/1473): Allow setting group context extensions when building an MlsGroup(Config). +- [#1475](https://github.com/openmls/openmls/pull/1475): Fully process GroupContextExtension proposals +- [#1477](https://github.com/openmls/openmls/pull/1477): Allow setting leaf node extensions and capabilities of the group creator when creating an MlsGroup(Config) +- [#1478](https://github.com/openmls/openmls/pull/1478): Remove explicit functions to set `RequiredCapabilitiesExtension` and `ExternalSendersExtension` when building an MlsGroup(Config) in favor of the more general function to set group context extensions +- [#1479](https://github.com/openmls/openmls/pull/1479): Allow the use of extensions with `ExtensionType::Unknown` in group context, key packages and leaf nodes ## 0.5.0 (XXXX-XX-XX) diff --git a/book/src/message_validation.md b/book/src/message_validation.md index efdaa92761..8c4e2cf37e 100644 --- a/book/src/message_validation.md +++ b/book/src/message_validation.md @@ -80,6 +80,8 @@ The following is a list of the individual semantic validation steps performed by | `ValSem205` | Confirmation tag must be successfully verified | ✅ | ✅ | `openmls/src/group/tests/test_commit_validation.rs` | | `ValSem206` | Path leaf node encryption key must be unique among proposals & members | ✅ | ✅ | `openmls/src/group/tests/test_commit_validation.rs` | | `ValSem207` | Path encryption keys must be unique among proposals & members | ✅ | ✅ | `openmls/src/group/tests/test_commit_validation.rs` | +| `ValSem208` | Only one GroupContextExtensions proposal in a commit | ✅ | | | +| `ValSem209` | GroupContextExtensions proposals may only contain extensions support by all members | ✅ | | | ### External Commit message validation diff --git a/book/src/user_manual/group_config.md b/book/src/user_manual/group_config.md index cd826f9adc..dbaf943c14 100644 --- a/book/src/user_manual/group_config.md +++ b/book/src/user_manual/group_config.md @@ -17,8 +17,9 @@ Two very similar structs can help configure groups upon their creation: `MlsGrou | Name | Type | Explanation | | ------------------------------ | ------------------------------- | ------------------------------------------------------------------------------------------------ | -| `required_capabilities` | `RequiredCapabilitiesExtension` | Required capabilities (extensions and proposal types). | -| `external_senders` | `ExternalSendersExtensions` | List credentials of non-group members that are allowed to send proposals to the group. | +| `group_context_extensions` | `Extensions` | Optional group-level extensions, e.g. `RequiredCapabilitiesExtension`. | +| `capabilities` . | `Capabilities` | Lists the capabilities of the group's creator. | +| `leaf_extensions` . | `Extensions` | Extensions to be included in the group creator's leaf | Both ways of group configurations can be specified by using the struct's builder pattern, or choosing their default values. The default value contains safe values for all parameters and is suitable for scenarios without particular requirements. @@ -33,3 +34,7 @@ Example create configuration: ```rust,no_run,noplayground {{#include ../../../openmls/tests/book_code.rs:mls_group_create_config_example}} ``` + +## Unknown extensions + +Some extensions carry data, but don't alter the behaviour of the protocol (e.g. the application_id extension). OpenMLS allows the use of arbitrary such extensions in the group context, key packages and leaf nodes. Such extensions can be instantiated and retrieved through the use of the `UnknownExtension` struct and the `ExtensionType::Unknown` extension type. Such "unknown" extensions are handled transparently by OpenMLS, but can be used by the application, e.g. to have a group agree on pieces of data. diff --git a/openmls/src/extensions/codec.rs b/openmls/src/extensions/codec.rs index 15e0e3a48e..00408aa994 100644 --- a/openmls/src/extensions/codec.rs +++ b/openmls/src/extensions/codec.rs @@ -8,7 +8,7 @@ use crate::extensions::{ UnknownExtension, }; -use super::{last_resort::LastResortExtension, protected_metadata::ProtectedMetadata}; +use super::{last_resort::LastResortExtension, metadata::Metadata}; fn vlbytes_len_len(length: usize) -> usize { if length < 0x40 { @@ -37,7 +37,7 @@ impl Size for Extension { Extension::ExternalPub(e) => e.tls_serialized_len(), Extension::ExternalSenders(e) => e.tls_serialized_len(), Extension::LastResort(e) => e.tls_serialized_len(), - Extension::ProtectedMetadata(e) => e.tls_serialized_len(), + Extension::ImmutableMetadata(e) => e.tls_serialized_len(), Extension::Unknown(_, e) => e.0.len(), }; @@ -70,7 +70,7 @@ impl Serialize for Extension { Extension::ExternalPub(e) => e.tls_serialize(&mut extension_data), Extension::ExternalSenders(e) => e.tls_serialize(&mut extension_data), Extension::LastResort(e) => e.tls_serialize(&mut extension_data), - Extension::ProtectedMetadata(e) => e.tls_serialize(&mut extension_data), + Extension::ImmutableMetadata(e) => e.tls_serialize(&mut extension_data), Extension::Unknown(_, e) => extension_data .write_all(e.0.as_slice()) .map(|_| e.0.len()) @@ -120,9 +120,9 @@ impl Deserialize for Extension { ExtensionType::LastResort => { Extension::LastResort(LastResortExtension::tls_deserialize(&mut extension_data)?) } - ExtensionType::ProtectedMetadata => Extension::ProtectedMetadata( - ProtectedMetadata::tls_deserialize(&mut extension_data)?, - ), + ExtensionType::ImmutableMetadata => { + Extension::ImmutableMetadata(Metadata::tls_deserialize(&mut extension_data)?) + } ExtensionType::Unknown(unknown) => { Extension::Unknown(unknown, UnknownExtension(extension_data.to_vec())) } diff --git a/openmls/src/extensions/errors.rs b/openmls/src/extensions/errors.rs index bdc7f9d58a..e8eb93a310 100644 --- a/openmls/src/extensions/errors.rs +++ b/openmls/src/extensions/errors.rs @@ -97,4 +97,9 @@ pub enum InvalidExtensionError { "The provided extension list contains an extension that is not allowed in group contexts." )] IllegalInGroupContext, + /// The provided extension list contains an extension that is not allowed in leaf nodes + #[error( + "The provided extension list contains an extension that is not allowed in leaf nodes." + )] + IllegalInLeafNodes, } diff --git a/openmls/src/extensions/metadata.rs b/openmls/src/extensions/metadata.rs new file mode 100644 index 0000000000..f2e1ab53b8 --- /dev/null +++ b/openmls/src/extensions/metadata.rs @@ -0,0 +1,23 @@ +use super::{Deserialize, Serialize}; +use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; + +/// Metadata is an extension that keeps arbitrary application-specific metadata, in the form of a +/// byte sequence. The application is responsible for specifying a format and parsing the contents. +#[derive( + PartialEq, Eq, Clone, Debug, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, +)] +pub struct Metadata { + metadata: Vec, +} + +impl Metadata { + /// Create a new [`Metadata`] extension. + pub fn new(metadata: Vec) -> Self { + Self { metadata } + } + + /// Get the metadata bytes. + pub fn metadata(&self) -> &Vec { + &self.metadata + } +} diff --git a/openmls/src/extensions/mod.rs b/openmls/src/extensions/mod.rs index 6ca68e983e..df9d2414dc 100644 --- a/openmls/src/extensions/mod.rs +++ b/openmls/src/extensions/mod.rs @@ -32,7 +32,7 @@ mod codec; mod external_pub_extension; mod external_sender_extension; mod last_resort; -mod protected_metadata; +mod metadata; mod ratchet_tree_extension; mod required_capabilities; use errors::*; @@ -54,7 +54,7 @@ use tls_codec::{ Size, TlsSize, }; -pub use protected_metadata::ProtectedMetadata; +pub use metadata::Metadata; #[cfg(test)] mod test_extensions; @@ -100,9 +100,9 @@ pub enum ExtensionType { /// scenario. LastResort, - /// Protected metadata extension for policies of the group. GroupContext - /// extension - ProtectedMetadata, + /// Immutable metadata extension for the GroupContext. + /// This can only be set on creation of the group. + ImmutableMetadata, /// A currently unknown extension type. Unknown(u16), @@ -155,7 +155,7 @@ impl From for ExtensionType { 4 => ExtensionType::ExternalPub, 5 => ExtensionType::ExternalSenders, 10 => ExtensionType::LastResort, - 11 => ExtensionType::ProtectedMetadata, + 0xf000 => ExtensionType::ImmutableMetadata, unknown => ExtensionType::Unknown(unknown), } } @@ -170,7 +170,7 @@ impl From for u16 { ExtensionType::ExternalPub => 4, ExtensionType::ExternalSenders => 5, ExtensionType::LastResort => 10, - ExtensionType::ProtectedMetadata => 11, + ExtensionType::ImmutableMetadata => 0xf000, ExtensionType::Unknown(unknown) => unknown, } } @@ -187,7 +187,7 @@ impl ExtensionType { | ExtensionType::ExternalPub | ExtensionType::ExternalSenders | ExtensionType::LastResort - | ExtensionType::ProtectedMetadata + | ExtensionType::ImmutableMetadata ) } } @@ -226,8 +226,8 @@ pub enum Extension { /// A [`LastResortExtension`] LastResort(LastResortExtension), - /// A [`ProtectedMetadata`] extension - ProtectedMetadata(ProtectedMetadata), + /// An immutable [`Metadata`] extension + ImmutableMetadata(Metadata), /// A currently unknown extension. Unknown(u16, UnknownExtension), @@ -420,11 +420,11 @@ impl Extensions { }) } - /// Get a reference to the [`ProtectedMetadata`] if there is any. - pub fn protected_metadata(&self) -> Option<&ProtectedMetadata> { - self.find_by_type(ExtensionType::ProtectedMetadata) + /// Get a reference to the immutable [`Metadata`] if there is any. + pub fn immutable_metadata(&self) -> Option<&Metadata> { + self.find_by_type(ExtensionType::ImmutableMetadata) .and_then(|e| match e { - Extension::ProtectedMetadata(e) => Some(e), + Extension::ImmutableMetadata(e) => Some(e), _ => None, }) } @@ -495,14 +495,14 @@ impl Extension { } } - /// Get a reference to this extension as [`ProtectedMetadata`]. + /// Get a reference to this extension as immutable [`Metadata`]. /// Returns an [`ExtensionError::InvalidExtensionType`] error if called on - /// an [`Extension`] that's not a [`ProtectedMetadata`] extension. - pub fn as_protected_metadata_extension(&self) -> Result<&ProtectedMetadata, ExtensionError> { + /// an [`Extension`] that's not an immutable [`Metadata`] extension. + pub fn as_immutable_metadata_extension(&self) -> Result<&Metadata, ExtensionError> { match self { - Self::ProtectedMetadata(e) => Ok(e), + Self::ImmutableMetadata(e) => Ok(e), _ => Err(ExtensionError::InvalidExtensionType( - "This is not an ProtectedMetadata".into(), + "This is not an immutable metadata extensions".into(), )), } } @@ -517,7 +517,7 @@ impl Extension { Extension::ExternalPub(_) => ExtensionType::ExternalPub, Extension::ExternalSenders(_) => ExtensionType::ExternalSenders, Extension::LastResort(_) => ExtensionType::LastResort, - Extension::ProtectedMetadata(_) => ExtensionType::ProtectedMetadata, + Extension::ImmutableMetadata(_) => ExtensionType::ImmutableMetadata, Extension::Unknown(kind, _) => ExtensionType::Unknown(*kind), } } @@ -622,7 +622,7 @@ mod test { #[test] fn that_unknown_extensions_are_de_serialized_correctly() { - let extension_types = [0x0000u16, 0x0A0A, 0x7A7A, 0xF000, 0xFFFF]; + let extension_types = [0x0000u16, 0x0A0A, 0x7A7A, 0xF100, 0xFFFF]; let extension_datas = [vec![], vec![0], vec![1, 2, 3]]; for extension_type in extension_types.into_iter() { diff --git a/openmls/src/extensions/protected_metadata.rs b/openmls/src/extensions/protected_metadata.rs deleted file mode 100644 index 593367a36b..0000000000 --- a/openmls/src/extensions/protected_metadata.rs +++ /dev/null @@ -1,246 +0,0 @@ -use std::time::{SystemTime, UNIX_EPOCH}; - -use openmls_traits::signatures::Signer; -use tls_codec::{Serialize as TlsSerializeTrait, TlsDeserialize, TlsSerialize, TlsSize}; - -use super::{Deserialize, Serialize}; -use crate::{ - ciphersuite::{ - signable::{Signable, SignatureError, SignedStruct, Verifiable, VerifiedStruct}, - signature::Signature, - }, - credentials::Credential, -}; - -/// # Protected Metadata -/// -/// ```c -/// struct { -/// opaque signer_application_id; -/// Credential signer_credential; -/// SignaturePublicKey signature_key; -/// uint64 signing_time; -/// opaque metadata; -/// /* SignWithLabel(., "ProtectedMetadataTBS",ProtectedMetadata) */ -/// opaque signature; -/// } ProtectedMetadata; -/// ``` -/// -/// This extension must be verified by the application every time it is set or -/// changed. -/// The application **MUST** verify that -/// * the signature is valid -/// * the credential has been valid at `signing_time` -/// * the `signer_application_id` is equal to the `creator_application_id`. -/// -/// FIXME: This should NOT be deserializable. But we need to change more code for -/// that to be possible. -#[derive( - PartialEq, Eq, Clone, Debug, Serialize, Deserialize, TlsDeserialize, TlsSerialize, TlsSize, -)] -pub struct ProtectedMetadata { - payload: ProtectedMetadataTbs, - signature: Signature, -} - -impl ProtectedMetadata { - /// Create a new protected metadata extension and sign it. - pub fn new( - signer: &impl Signer, - signer_application_id: Vec, - signer_credential: Credential, - signature_key: Vec, - metadata: Vec, - ) -> Result { - let tbs = ProtectedMetadataTbs::new( - signer_application_id, - signer_credential, - signature_key, - metadata, - ); - tbs.sign(signer) - } - - /// Get the signer application ID as slice. - pub fn signer_application_id(&self) -> &[u8] { - self.payload.signer_application_id.as_ref() - } - - /// Get the signer [`Credential`]. - pub fn signer_credential(&self) -> &Credential { - &self.payload.signer_credential - } - - /// Get the signature key as slize. - pub fn signature_key(&self) -> &[u8] { - self.payload.signature_key.as_ref() - } - - /// Get the signing time as UNIX timestamp. - pub fn signing_time(&self) -> u64 { - self.payload.signing_time - } - - /// Get the serialized metadata as slice. - /// - /// This is opaque to OpenMLS. The caller must handle it appropriately. - pub fn metadata(&self) -> &[u8] { - self.payload.metadata.as_ref() - } -} - -impl SignedStruct for ProtectedMetadata { - fn from_payload(payload: ProtectedMetadataTbs, signature: Signature) -> Self { - ProtectedMetadata { payload, signature } - } -} - -/// # Protected Metadata -/// -/// ```c -/// /* SignWithLabel(., "ProtectedMetadataTBS",ProtectedMetadata) */ -/// struct { -/// opaque signer_application_id; -/// Credential signer_credential; -/// SignaturePublicKey signature_key; -/// uint64 signing_time; -/// opaque metadata; -/// } ProtectedMetadataTBS; -/// ``` -#[derive( - PartialEq, Eq, Clone, Debug, Serialize, Deserialize, TlsSerialize, TlsDeserialize, TlsSize, -)] -pub struct ProtectedMetadataTbs { - signer_application_id: Vec, - signer_credential: Credential, - signature_key: Vec, - signing_time: u64, - metadata: Vec, -} - -impl ProtectedMetadataTbs { - /// Create a protected metadata extension tbs. - fn new( - signer_application_id: Vec, - signer_credential: Credential, - signature_key: Vec, - metadata: impl Into>, - ) -> Self { - Self { - signer_application_id, - signer_credential, - signature_key, - signing_time: SystemTime::now() - .duration_since(UNIX_EPOCH) - .expect("SystemTime before UNIX EPOCH!") - .as_secs(), - metadata: metadata.into(), - } - } -} - -const SIGNATURE_LABEL: &str = "ProtectedMetadataTbs"; - -impl Signable for ProtectedMetadataTbs { - type SignedOutput = ProtectedMetadata; - - fn unsigned_payload(&self) -> Result, tls_codec::Error> { - self.tls_serialize_detached() - } - - fn label(&self) -> &str { - SIGNATURE_LABEL - } -} - -/// XXX: This really should not be implemented on [`ProtectedMetadata`] but on -/// the verifiable version. -mod verifiable { - use openmls_traits::crypto::OpenMlsCrypto; - - use crate::prelude::OpenMlsSignaturePublicKey; - - use super::*; - - impl Verifiable for ProtectedMetadata { - type VerifiedStruct = ProtectedMetadata; - - fn unsigned_payload(&self) -> Result, tls_codec::Error> { - self.payload.tls_serialize_detached() - } - - fn signature(&self) -> &Signature { - &self.signature - } - - fn label(&self) -> &str { - SIGNATURE_LABEL - } - - fn verify( - self, - _crypto: &impl OpenMlsCrypto, - _pk: &OpenMlsSignaturePublicKey, - ) -> Result { - Ok(self) - } - } - - impl VerifiedStruct for ProtectedMetadata {} - - mod private_mod { - #[derive(Default)] - pub struct Seal; - } -} - -#[cfg(test)] -mod tests { - use tls_codec::{Deserialize, Serialize}; - - use crate::{ - credentials::test_utils::new_credential, extensions::protected_metadata::ProtectedMetadata, - prelude_test::OpenMlsSignaturePublicKey, test_utils::*, - }; - - use super::*; - - #[apply(ciphersuites_and_providers)] - fn serialize_extension(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { - let creator_application_id = b"MetadataTestAppId".to_vec(); - - // Create metadata - let metadata = vec![1, 2, 3]; - - // Setup crypto - let (credential_with_key, signer) = new_credential( - provider, - b"Kreator", - crate::credentials::CredentialType::Basic, - ciphersuite.signature_algorithm(), - ); - let signature_key = - OpenMlsSignaturePublicKey::new(signer.public().into(), ciphersuite.into()).unwrap(); - - let signer_application_id = creator_application_id.clone(); - let extension = ProtectedMetadata::new( - &signer, - signer_application_id, - credential_with_key.credential.clone(), - signature_key.as_slice().to_vec(), - metadata.clone(), - ) - .unwrap(); - - // serialize and deserialize + verify - let serialized = extension.tls_serialize_detached().unwrap(); - let unverified = ProtectedMetadata::tls_deserialize_exact(serialized).unwrap(); - let deserialized: ProtectedMetadata = unverified - .verify(provider.crypto(), &signature_key) - .unwrap(); - assert_eq!(deserialized, extension); - - let xmtp_metadata = deserialized.metadata(); - assert_eq!(xmtp_metadata, metadata); - } -} diff --git a/openmls/src/extensions/required_capabilities.rs b/openmls/src/extensions/required_capabilities.rs index d00081beaf..9dd5b0f9bb 100644 --- a/openmls/src/extensions/required_capabilities.rs +++ b/openmls/src/extensions/required_capabilities.rs @@ -78,11 +78,6 @@ impl RequiredCapabilitiesExtension { /// Check if all extension and proposal types are supported. pub(crate) fn check_support(&self) -> Result<(), ExtensionError> { - for extension in self.extension_types() { - if !extension.is_supported() { - return Err(ExtensionError::UnsupportedExtensionType); - } - } for proposal in self.proposal_types() { if !proposal.is_supported() { return Err(ExtensionError::UnsupportedProposalType); diff --git a/openmls/src/extensions/test_extensions.rs b/openmls/src/extensions/test_extensions.rs index dd0f2c62e2..50eeeda8d2 100644 --- a/openmls/src/extensions/test_extensions.rs +++ b/openmls/src/extensions/test_extensions.rs @@ -251,6 +251,49 @@ fn required_capabilities() { assert_eq!(extension_bytes, encoded); } +#[apply(ciphersuites_and_providers)] +fn test_metadata(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { + // Create credentials and keys + let alice_credential_with_key_and_signer = tests::utils::generate_credential_with_key( + b"Alice".into(), + ciphersuite.signature_algorithm(), + provider, + ); + + // example metadata (opaque data -- test hex string is "1cedc0ffee") + let metadata = vec![0x1c, 0xed, 0xc0, 0xff, 0xee]; + let ext = Extension::Unknown(0xf001, UnknownExtension(metadata.clone())); + let extensions = Extensions::from_vec(vec![ext]).expect("could not build extensions struct"); + + let config = MlsGroupCreateConfig::builder() + .with_group_context_extensions(extensions) + .unwrap() + .build(); + + // === Alice creates a group with the ratchet tree extension === + let alice_group = MlsGroup::new( + provider, + &alice_credential_with_key_and_signer.signer, + &config, + alice_credential_with_key_and_signer + .credential_with_key + .clone(), + ) + .expect("failed to build group"); + + let got_metadata = alice_group + .export_group_context() + .extensions() + .find_by_type(ExtensionType::Unknown(0xf001)) + .expect("failed to read group metadata"); + + if let Extension::Unknown(0xf001, UnknownExtension(got_metadata)) = got_metadata { + assert_eq!(got_metadata, &metadata); + } else { + panic!("metadata extension has wrong extension enum variant") + } +} + #[apply(ciphersuites_and_providers)] fn with_group_context_extensions(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { // create an extension that we can check for later diff --git a/openmls/src/framing/validation.rs b/openmls/src/framing/validation.rs index 7108b70b73..391cdc2af7 100644 --- a/openmls/src/framing/validation.rs +++ b/openmls/src/framing/validation.rs @@ -55,6 +55,7 @@ use super::{ /// - ValSem005 /// - ValSem007 /// - ValSem009 +#[derive(Debug)] pub(crate) struct DecryptedMessage { verifiable_content: VerifiableAuthenticatedContentIn, } diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index b56b53f8b6..b17891cf1e 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -39,6 +39,7 @@ use tls_codec::Serialize as TlsSerializeTrait; use self::{ create_commit_params::{CommitType, CreateCommitParams}, + node::leaf_node::Capabilities, past_secrets::MessageSecretsStore, staged_commit::{MemberStagedCommitState, StagedCommit, StagedCommitState}, }; @@ -179,26 +180,10 @@ impl CoreGroupBuilder { self.psk_ids = psk_ids; self } - /// Set the [`RequiredCapabilitiesExtension`] of the [`CoreGroup`]. - pub(crate) fn with_required_capabilities( - mut self, - required_capabilities: RequiredCapabilitiesExtension, - ) -> Self { - self.public_group_builder = self - .public_group_builder - .with_required_capabilities(required_capabilities); - self - } - /// Set the [`ExternalSendersExtension`] of the [`CoreGroup`]. - pub(crate) fn with_external_senders( - mut self, - external_senders: ExternalSendersExtension, - ) -> Self { - if !external_senders.is_empty() { - self.public_group_builder = self - .public_group_builder - .with_external_senders(external_senders); - } + + /// Set the [`Capabilities`] of the group's creator. + pub(crate) fn with_capabilities(mut self, capabilities: Capabilities) -> Self { + self.public_group_builder = self.public_group_builder.with_capabilities(capabilities); self } @@ -217,6 +202,17 @@ impl CoreGroupBuilder { Ok(self) } + /// Sets extensions of the group creator's [`LeafNode`]. + pub(crate) fn with_leaf_node_extensions( + mut self, + extensions: Extensions, + ) -> Result { + self.public_group_builder = self + .public_group_builder + .with_leaf_node_extensions(extensions)?; + Ok(self) + } + /// Set the number of past epochs the group should keep secrets. pub fn with_max_past_epoch_secrets(mut self, max_past_epochs: usize) -> Self { self.max_past_epochs = max_past_epochs; @@ -452,10 +448,9 @@ impl CoreGroup { let required_capabilities = required_extension.as_required_capabilities_extension()?; // Ensure we support all the capabilities. required_capabilities.check_support()?; - // TODO #566/#1361: This needs to be re-enabled once we support GCEs - /* self.own_leaf_node()? - .capabilities() - .supports_required_capabilities(required_capabilities)?; */ + self.own_leaf_node()? + .capabilities() + .supports_required_capabilities(required_capabilities)?; // Ensure that all other leaf nodes support all the required // extensions as well. @@ -890,6 +885,11 @@ impl CoreGroup { .validate_update_proposals(&proposal_queue, *sender_index)?; } + // ValSem208 + // ValSem209 + self.public_group + .validate_group_context_extensions_proposal(&proposal_queue)?; + // Make a copy of the public group to apply proposals safely let mut diff = self.public_group.empty_diff(); @@ -915,12 +915,13 @@ impl CoreGroup { apply_proposals_values.exclusion_list(), params.commit_type(), signer, - params.take_credential_with_key() + params.take_credential_with_key(), + apply_proposals_values.extensions.clone() )? } else { // If path is not needed, update the group context and return // empty path processing results - diff.update_group_context(provider.crypto())?; + diff.update_group_context(provider.crypto(), apply_proposals_values.extensions.clone())?; PathComputationResult::default() }; diff --git a/openmls/src/group/core_group/proposals.rs b/openmls/src/group/core_group/proposals.rs index e5344133b6..57920e43f8 100644 --- a/openmls/src/group/core_group/proposals.rs +++ b/openmls/src/group/core_group/proposals.rs @@ -507,7 +507,7 @@ impl ProposalQueue { } } Proposal::GroupContextExtensions(_) => { - // TODO: Validate proposal? + valid_proposals.add(queued_proposal.proposal_reference()); proposal_pool.insert(queued_proposal.proposal_reference(), queued_proposal); } Proposal::AppAck(_) => unimplemented!("See #291"), @@ -530,10 +530,11 @@ impl ProposalQueue { // Only retain `adds` and `valid_proposals` let mut proposal_queue = ProposalQueue::default(); for proposal_reference in adds.iter().chain(valid_proposals.iter()) { - proposal_queue.add(match proposal_pool.get(proposal_reference) { - Some(queued_proposal) => queued_proposal.clone(), - None => return Err(ProposalQueueError::ProposalNotFound), - }); + let queued_proposal = proposal_pool + .get(proposal_reference) + .cloned() + .ok_or(ProposalQueueError::ProposalNotFound)?; + proposal_queue.add(queued_proposal); } Ok((proposal_queue, contains_own_updates)) } diff --git a/openmls/src/group/core_group/staged_commit.rs b/openmls/src/group/core_group/staged_commit.rs index c195cc7b85..0b0f2e9a38 100644 --- a/openmls/src/group/core_group/staged_commit.rs +++ b/openmls/src/group/core_group/staged_commit.rs @@ -165,7 +165,10 @@ impl CoreGroup { )?; // Update group context - diff.update_group_context(provider.crypto())?; + diff.update_group_context( + provider.crypto(), + apply_proposals_values.extensions.clone(), + )?; let decryption_keypairs: Vec<&EncryptionKeyPair> = old_epoch_keypairs .iter() @@ -221,7 +224,10 @@ impl CoreGroup { } // Even if there is no path, we have to update the group context. - diff.update_group_context(provider.crypto())?; + diff.update_group_context( + provider.crypto(), + apply_proposals_values.extensions.clone(), + )?; ( CommitSecret::zero_secret(ciphersuite, self.version()), diff --git a/openmls/src/group/core_group/test_proposals.rs b/openmls/src/group/core_group/test_proposals.rs index 397237bda1..197c2d3272 100644 --- a/openmls/src/group/core_group/test_proposals.rs +++ b/openmls/src/group/core_group/test_proposals.rs @@ -22,7 +22,6 @@ use crate::{ messages::proposals::{AddProposal, Proposal, ProposalOrRef, ProposalType}, schedule::psk::store::ResumptionPskStore, test_utils::*, - treesync::errors::LeafNodeValidationError, versions::ProtocolVersion, }; @@ -299,7 +298,10 @@ fn test_required_unsupported_proposals(ciphersuite: Ciphersuite, provider: &impl CryptoConfig::with_default_version(ciphersuite), alice_credential, ) - .with_required_capabilities(required_capabilities) + .with_group_context_extensions(Extensions::single(Extension::RequiredCapabilities( + required_capabilities, + ))) + .unwrap() .build(provider, &alice_signer) .expect_err( "CoreGroup creation must fail because AppAck proposals aren't supported in OpenMLS yet.", @@ -326,16 +328,9 @@ fn test_required_extension_key_package_mismatch( let bob_key_package = bob_key_package_bundle.key_package(); // Set required capabilities - let extensions = &[ - ExtensionType::RequiredCapabilities, - ExtensionType::ApplicationId, - ]; - let proposals = &[ - ProposalType::GroupContextExtensions, - ProposalType::Add, - ProposalType::Remove, - ProposalType::Update, - ]; + let extensions = &[ExtensionType::Unknown(0xff00)]; + // We don't support unknown proposals (yet) + let proposals = &[]; let credentials = &[CredentialType::Basic]; let required_capabilities = RequiredCapabilitiesExtension::new(extensions, proposals, credentials); @@ -345,7 +340,10 @@ fn test_required_extension_key_package_mismatch( CryptoConfig::with_default_version(ciphersuite), alice_credential, ) - .with_required_capabilities(required_capabilities) + .with_group_context_extensions(Extensions::single(Extension::RequiredCapabilities( + required_capabilities, + ))) + .expect("error adding group context extensions") .build(provider, &alice_signer) .expect("Error creating CoreGroup."); @@ -358,7 +356,9 @@ fn test_required_extension_key_package_mismatch( .expect_err("Proposal was created even though the key package didn't support the required extensions."); assert_eq!( e, - CreateAddProposalError::LeafNodeValidation(LeafNodeValidationError::UnsupportedExtensions) + CreateAddProposalError::LeafNodeValidation( + crate::treesync::errors::LeafNodeValidationError::UnsupportedExtensions + ) ); } @@ -392,7 +392,10 @@ fn test_group_context_extensions(ciphersuite: Ciphersuite, provider: &impl OpenM CryptoConfig::with_default_version(ciphersuite), alice_credential, ) - .with_required_capabilities(required_capabilities) + .with_group_context_extensions(Extensions::single(Extension::RequiredCapabilities( + required_capabilities, + ))) + .unwrap() .build(provider, &alice_signer) .expect("Error creating CoreGroup."); @@ -470,32 +473,13 @@ fn test_group_context_extension_proposal_fails( CryptoConfig::with_default_version(ciphersuite), alice_credential, ) - .with_required_capabilities(required_capabilities) + .with_group_context_extensions(Extensions::single(Extension::RequiredCapabilities( + required_capabilities, + ))) + .unwrap() .build(provider, &alice_signer) .expect("Error creating CoreGroup."); - // TODO: openmls/openmls#1130 add a test for unsupported required capabilities. - // We can't test this right now because we don't have a capability - // that is not a "default" proposal or extension. - // // Alice tries to add a required capability she doesn't support herself. - // let required_application_id = Extension::RequiredCapabilities( - // RequiredCapabilitiesExtension::new(&[ExtensionType::ApplicationId], &[]), - // ); - // let e = alice_group.create_group_context_ext_proposal( - // framing_parameters, - // &alice_credential_bundle, - // &[required_application_id.clone()], - // provider, - // ).expect_err("Alice was able to create a gce proposal with a required extensions she doesn't support."); - // assert_eq!( - // e, - // CreateGroupContextExtProposalError::TreeSyncError( - // crate::treesync::errors::TreeSyncError::UnsupportedExtension - // ) - // ); - // - // // Well, this failed luckily. - // Adding Bob let bob_add_proposal = alice_group .create_add_proposal(framing_parameters, bob_key_package.clone(), &alice_signer) diff --git a/openmls/src/group/errors.rs b/openmls/src/group/errors.rs index 1432c494a0..fb31308e9a 100644 --- a/openmls/src/group/errors.rs +++ b/openmls/src/group/errors.rs @@ -189,6 +189,11 @@ pub enum StageCommitError { /// See [`UpdatePathError`] for more details. #[error(transparent)] VerifiedUpdatePathError(#[from] UpdatePathError), + /// See [`GroupContextExtensionsProposalValidationError`] for more details. + #[error(transparent)] + GroupContextExtensionsProposalValidationError( + #[from] GroupContextExtensionsProposalValidationError, + ), } /// Create commit error @@ -233,6 +238,11 @@ pub enum CreateCommitError { /// See [`InvalidExtensionError`] for more details. #[error(transparent)] InvalidExtensionError(#[from] InvalidExtensionError), + /// See [`GroupContextExtensionsProposalValidationError`] for more details. + #[error(transparent)] + GroupContextExtensionsProposalValidationError( + #[from] GroupContextExtensionsProposalValidationError, + ), } /// Validation error @@ -500,16 +510,13 @@ pub(crate) enum CoreGroupParseMessageError { /// Create group context ext proposal error #[derive(Error, Debug, PartialEq, Clone)] -pub(crate) enum CreateGroupContextExtProposalError { +pub enum CreateGroupContextExtProposalError { /// See [`LibraryError`] for more details. #[error(transparent)] LibraryError(#[from] LibraryError), /// See [`KeyPackageExtensionSupportError`] for more details. #[error(transparent)] KeyPackageExtensionSupport(#[from] KeyPackageExtensionSupportError), - /// See [`TreeSyncError`] for more details. - #[error(transparent)] - TreeSyncError(#[from] TreeSyncError), /// See [`ExtensionError`] for more details. #[error(transparent)] Extension(#[from] ExtensionError), @@ -528,3 +535,22 @@ pub enum MergeCommitError { #[error("Error accessing the key store.")] KeyStoreError(KeyStoreError), } + +/// Error validation a GroupContextExtensions proposal. +#[derive(Error, Debug, PartialEq, Clone)] +pub enum GroupContextExtensionsProposalValidationError { + /// Commit has more than one GroupContextExtensions proposal. + #[error("Commit has more than one GroupContextExtensions proposal.")] + TooManyGCEProposals, + /// See [`LibraryError`] for more details. + #[error(transparent)] + LibraryError(#[from] LibraryError), + /// The new extensions set contails extensions that are not supported by all group members. + #[error( + "The new extensions set contails extensions that are not supported by all group members." + )] + ExtensionNotSupportedByAllMembers, + /// Proposal changes the immutable metadata extension, which is not allowed. + #[error("Proposal changes the immutable metadata extension, which is not allowed.")] + ChangedImmutableMetadata, +} diff --git a/openmls/src/group/group_context.rs b/openmls/src/group/group_context.rs index 8344e07625..d6b1cc93a9 100644 --- a/openmls/src/group/group_context.rs +++ b/openmls/src/group/group_context.rs @@ -159,6 +159,10 @@ impl GroupContext { self.confirmed_transcript_hash.as_slice() } + pub(crate) fn set_extensions(&mut self, extensions: Extensions) { + self.extensions = extensions; + } + /// Return the extensions. pub fn extensions(&self) -> &Extensions { &self.extensions diff --git a/openmls/src/group/mls_group/builder.rs b/openmls/src/group/mls_group/builder.rs index ed8ec42069..6607727dbb 100644 --- a/openmls/src/group/mls_group/builder.rs +++ b/openmls/src/group/mls_group/builder.rs @@ -2,13 +2,16 @@ use openmls_traits::{key_store::OpenMlsKeyStore, signatures::Signer, OpenMlsProv use crate::{ credentials::CredentialWithKey, - extensions::{errors::InvalidExtensionError, Extensions, ExternalSendersExtension}, + error::LibraryError, + extensions::{errors::InvalidExtensionError, Extensions}, group::{ config::CryptoConfig, public_group::errors::PublicGroupBuildError, CoreGroup, CoreGroupBuildError, CoreGroupConfig, GroupId, MlsGroupCreateConfig, MlsGroupCreateConfigBuilder, NewGroupError, ProposalStore, WireFormatPolicy, }, - prelude::{LibraryError, Lifetime, SenderRatchetConfiguration}, + key_packages::Lifetime, + tree::sender_ratchet::SenderRatchetConfiguration, + treesync::node::leaf_node::Capabilities, }; use super::{InnerState, MlsGroup, MlsGroupState}; @@ -70,9 +73,9 @@ impl MlsGroupBuilder { credential_with_key, ) .with_config(group_config) - .with_required_capabilities(mls_group_create_config.required_capabilities.clone()) - .with_external_senders(mls_group_create_config.external_senders.clone()) .with_group_context_extensions(mls_group_create_config.group_context_extensions.clone())? + .with_leaf_node_extensions(mls_group_create_config.leaf_node_extensions.clone())? + .with_capabilities(mls_group_create_config.capabilities.clone()) .with_max_past_epoch_secrets(mls_group_create_config.join_config.max_past_epochs) .with_lifetime(*mls_group_create_config.lifetime()) .build(provider, signer) @@ -192,22 +195,33 @@ impl MlsGroupBuilder { self } - /// Sets the `external_senders` property of the MlsGroup. - pub fn external_senders(mut self, external_senders: ExternalSendersExtension) -> Self { + /// Sets the initial group context extensions + pub fn with_group_context_extensions( + mut self, + extensions: Extensions, + ) -> Result { self.mls_group_create_config_builder = self .mls_group_create_config_builder - .external_senders(external_senders); - self + .with_group_context_extensions(extensions)?; + Ok(self) } - /// Sets the initial group context extensions - pub fn with_group_context_extensions( + /// Sets the initial leaf node extensions + pub fn with_leaf_node_extensions( mut self, extensions: Extensions, ) -> Result { self.mls_group_create_config_builder = self .mls_group_create_config_builder - .with_group_context_extensions(extensions)?; + .with_leaf_node_extensions(extensions)?; Ok(self) } + + /// Sets the group creator's [`Capabilities`] + pub fn with_capabilities(mut self, capabilities: Capabilities) -> Self { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .capabilities(capabilities); + self + } } diff --git a/openmls/src/group/mls_group/config.rs b/openmls/src/group/mls_group/config.rs index c270ece7d2..884e160388 100644 --- a/openmls/src/group/mls_group/config.rs +++ b/openmls/src/group/mls_group/config.rs @@ -30,7 +30,7 @@ use super::*; use crate::{ extensions::errors::InvalidExtensionError, group::config::CryptoConfig, key_packages::Lifetime, - tree::sender_ratchet::SenderRatchetConfiguration, + tree::sender_ratchet::SenderRatchetConfiguration, treesync::node::leaf_node::Capabilities, }; use serde::{Deserialize, Serialize}; @@ -83,10 +83,8 @@ impl MlsGroupJoinConfig { /// more information about the different configuration values. #[derive(Clone, Debug, Default, PartialEq, Eq, Serialize, Deserialize)] pub struct MlsGroupCreateConfig { - /// Required capabilities (extensions and proposal types) - pub(crate) required_capabilities: RequiredCapabilitiesExtension, - /// Senders authorized to send external remove proposals - pub(crate) external_senders: ExternalSendersExtension, + /// Capabilities advertised in the creator's leaf node + pub(crate) capabilities: Capabilities, /// Lifetime of the own leaf node pub(crate) lifetime: Lifetime, /// Ciphersuite and protocol version @@ -95,6 +93,8 @@ pub struct MlsGroupCreateConfig { pub(crate) join_config: MlsGroupJoinConfig, /// List of initial group context extensions pub(crate) group_context_extensions: Extensions, + /// List of initial leaf node extensions + pub(crate) leaf_node_extensions: Extensions, } /// Builder struct for an [`MlsGroupJoinConfig`]. @@ -187,21 +187,11 @@ impl MlsGroupCreateConfig { self.join_config.use_ratchet_tree_extension } - /// Returns the [`MlsGroupCreateConfig`] required capabilities extension. - pub fn required_capabilities(&self) -> &RequiredCapabilitiesExtension { - &self.required_capabilities - } - /// Returns the [`MlsGroupCreateConfig`] sender ratchet configuration. pub fn sender_ratchet_configuration(&self) -> &SenderRatchetConfiguration { &self.join_config.sender_ratchet_configuration } - /// Returns the [`MlsGroupCreateConfig`] external senders extension - pub fn external_senders(&self) -> &ExternalSendersExtension { - &self.external_senders - } - /// Returns the [`Extensions`] set as the initial group context. /// This does not contain the initial group context extensions /// added from builder calls to `external_senders` or `required_capabilities`. @@ -289,12 +279,9 @@ impl MlsGroupCreateConfigBuilder { self } - /// Sets the `required_capabilities` property of the MlsGroupCreateConfig. - pub fn required_capabilities( - mut self, - required_capabilities: RequiredCapabilitiesExtension, - ) -> Self { - self.config.required_capabilities = required_capabilities; + /// Sets the `capabilities` of the group creator's leaf node. + pub fn capabilities(mut self, capabilities: Capabilities) -> Self { + self.config.capabilities = capabilities; self } @@ -320,17 +307,7 @@ impl MlsGroupCreateConfigBuilder { self } - /// Sets the `external_senders` property of the MlsGroupCreateConfig. - pub fn external_senders(mut self, external_senders: ExternalSendersExtension) -> Self { - self.config.external_senders = external_senders; - self - } - - /// Sets initial group context extensions. Note that RequiredCapabilities - /// extensions will be overwritten, and should be set using a call to - /// `required_capabilities`. If `ExternalSenders` extensions are provided - /// both in this call, and a call to `external_senders`, only the one from - /// the call to `external_senders` will be included. + /// Sets initial group context extensions. pub fn with_group_context_extensions( mut self, extensions: Extensions, @@ -345,6 +322,23 @@ impl MlsGroupCreateConfigBuilder { Ok(self) } + /// Sets extensions of the group creator's [`LeafNode`]. + pub fn with_leaf_node_extensions( + mut self, + extensions: Extensions, + ) -> Result { + // None of the default extensions are leaf node extensions, so only + // unknown extensions can be leaf node extensions. + let is_valid_in_leaf_node = extensions + .iter() + .all(|e| matches!(e.extension_type(), ExtensionType::Unknown(_))); + if !is_valid_in_leaf_node { + return Err(InvalidExtensionError::IllegalInLeafNodes); + } + self.config.leaf_node_extensions = extensions; + Ok(self) + } + /// Finalizes the builder and retursn an `[MlsGroupCreateConfig`]. pub fn build(self) -> MlsGroupCreateConfig { self.config diff --git a/openmls/src/group/mls_group/errors.rs b/openmls/src/group/mls_group/errors.rs index a23e47667e..2b7a4fc207 100644 --- a/openmls/src/group/mls_group/errors.rs +++ b/openmls/src/group/mls_group/errors.rs @@ -10,9 +10,12 @@ use thiserror::Error; use crate::{ error::LibraryError, extensions::errors::InvalidExtensionError, - group::errors::{ - CreateAddProposalError, CreateCommitError, MergeCommitError, StageCommitError, - ValidationError, + group::{ + errors::{ + CreateAddProposalError, CreateCommitError, MergeCommitError, StageCommitError, + ValidationError, + }, + CreateGroupContextExtProposalError, }, schedule::errors::PskError, treesync::errors::{LeafNodeValidationError, PublicTreeError}, @@ -317,4 +320,7 @@ pub enum ProposalError { /// See [`ValidationError`] for more details. #[error(transparent)] ValidationError(#[from] ValidationError), + /// See [`CreateGroupContextExtProposalError`] for more details. + #[error(transparent)] + CreateGroupContextExtProposalError(#[from] CreateGroupContextExtProposalError), } diff --git a/openmls/src/group/mls_group/proposal.rs b/openmls/src/group/mls_group/proposal.rs index b15b21d75d..bc00077640 100644 --- a/openmls/src/group/mls_group/proposal.rs +++ b/openmls/src/group/mls_group/proposal.rs @@ -328,10 +328,11 @@ impl MlsGroup { ) -> Result<(MlsMessageOut, ProposalRef), ProposalError<()>> { self.is_operational()?; - let proposal = self - .group - .create_group_context_ext_proposal(self.framing_parameters(), extensions, signer) - .unwrap(); + let proposal = self.group.create_group_context_ext_proposal( + self.framing_parameters(), + extensions, + signer, + )?; let queued_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 b22ec84b6a..23eb00c826 100644 --- a/openmls/src/group/mls_group/test_mls_group.rs +++ b/openmls/src/group/mls_group/test_mls_group.rs @@ -5,10 +5,12 @@ use tls_codec::{Deserialize, Serialize}; use crate::{ binary_tree::LeafNodeIndex, + extensions::errors::InvalidExtensionError, framing::*, group::{config::CryptoConfig, errors::*, *}, key_packages::*, messages::proposals::*, + prelude::Capabilities, test_utils::test_framework::{ errors::ClientError, noop_authentication_service, ActionType::Commit, CodecUse, MlsGroupTestSetup, @@ -721,7 +723,7 @@ fn test_pending_commit_logic(ciphersuite: Ciphersuite, provider: &impl OpenMlsPr // Let's add bob let (proposal, _) = alice_group .propose_add_member(provider, &alice_signer, bob_key_package) - .expect("error creating self-update proposal"); + .expect("error creating add-bob proposal"); let alice_processed_message = alice_group .process_message(provider, proposal.into_protocol_message().unwrap()) @@ -1003,6 +1005,216 @@ fn remove_prosposal_by_ref(ciphersuite: Ciphersuite, provider: &impl OpenMlsProv } } +// Test that the builder pattern accurately configures the new group. +#[apply(ciphersuites_and_providers)] +fn immutable_metadata(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { + let (alice_credential_with_key, _alice_kpb, alice_signer, _alice_pk) = + setup_client("Alice", ciphersuite, provider); + + let metadata = Metadata::new(b"this is a test group".to_vec()); + + let extensions_with_metadata = + Extensions::single(Extension::ImmutableMetadata(metadata.clone())); + + // === Create a Group with Metadata === + let capabilities = Capabilities::new( + None, + None, + Some(&[ExtensionType::ImmutableMetadata]), + None, + None, + ); + let mut group_with_metadata = MlsGroup::builder() + .with_group_context_extensions(extensions_with_metadata.clone()) + .expect("error when setting initial metadata group extension") + .with_capabilities(capabilities) + .build(provider, &alice_signer, alice_credential_with_key.clone()) + .expect("error creating group using builder"); + + let got_metadata = group_with_metadata + .group() + .group_context_extensions() + .immutable_metadata() + .expect("error getting metadata from group"); + + // check we get back the same metadata we initially set + assert_eq!(got_metadata, &metadata); + + // changing the metadata should fail + let new_metadata = Metadata::new(b"this is a not test group".to_vec()); + + let new_extensions_with_metadata = + Extensions::single(Extension::ImmutableMetadata(new_metadata.clone())); + + group_with_metadata + .propose_group_context_extensions(provider, new_extensions_with_metadata, &alice_signer) + .expect("error proposing GCE proposal with same metadata"); + + assert_eq!(group_with_metadata.pending_proposals().count(), 1); + + group_with_metadata + .commit_to_pending_proposals(provider, &alice_signer) + .expect_err("should not have been able to commit to proposal that changes metadata"); + + group_with_metadata.clear_pending_proposals(); + + assert_eq!(group_with_metadata.pending_proposals().count(), 0); + + // using the same metadata should succeed + group_with_metadata + .propose_group_context_extensions(provider, extensions_with_metadata.clone(), &alice_signer) + .expect("error proposing GCE proposal with same metadata"); + + assert_eq!(group_with_metadata.pending_proposals().count(), 1); + + group_with_metadata + .commit_to_pending_proposals(provider, &alice_signer) + .expect("failed to commit to pending proposals"); + + group_with_metadata + .merge_pending_commit(provider) + .expect("error merging pending commit"); + + let got_metadata = group_with_metadata + .group() + .group_context_extensions() + .immutable_metadata() + .expect("couldn't get immutable_metadata"); + + // check we get back the same metadata we initially set + assert_eq!(got_metadata, &metadata); + + // === Create a Group without Metadata === + let mut group_without_metadata = MlsGroup::builder() + .build(provider, &alice_signer, alice_credential_with_key) + .expect("error creating group using builder"); + + assert!(group_without_metadata + .group() + .group_context_extensions() + .immutable_metadata() + .is_none()); + + // changing the metadata should fail + let new_metadata = Metadata::new(b"this is a new metadata".to_vec()); + + let new_extensions_with_metadata = + Extensions::single(Extension::ImmutableMetadata(new_metadata.clone())); + + group_without_metadata + .propose_group_context_extensions(provider, new_extensions_with_metadata, &alice_signer) + .expect("error proposing GCE proposal with metadata"); + + // since the GCEs are the same as befroe, the proposal handling logic "deduplicates" the proposal + // (with the implicit "identity proposal" I guess), so there isn't actually a proposal here + assert_eq!(group_with_metadata.pending_proposals().count(), 0); + + // using the same metadata should succeed + group_without_metadata + .propose_group_context_extensions(provider, Extensions::empty(), &alice_signer) + .expect("error proposing GCE proposal with no metadata"); + + // since the GCEs are empty, the proposal handling logic "deduplicates" the proposal + // (with the implicit "identity proposal" I guess), so there isn't actually a proposal here + assert_eq!(group_with_metadata.pending_proposals().count(), 0); + + // check we still get no metadata + assert!(group_without_metadata + .group() + .group_context_extensions() + .immutable_metadata() + .is_none()); + + // TODO: we need to test that processing an invalid commit also fails. + // however, we can't generate this commit, because our functions for + // constructing commits does not permit it. See #1476 +} + +// Test that the builder pattern accurately configures the new group. +#[apply(ciphersuites_and_providers)] +fn group_context_extensions_proposal(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { + let (alice_credential_with_key, _alice_kpb, alice_signer, _alice_pk) = + setup_client("Alice", ciphersuite, provider); + + // === Alice creates a group === + let mut alice_group = MlsGroup::builder() + .build(provider, &alice_signer, alice_credential_with_key) + .expect("error creating group using builder"); + + // No required capabilities, so no specifically required extensions. + assert!(alice_group + .group() + .group_context_extensions() + .required_capabilities() + .is_none()); + + let new_extensions = Extensions::single(Extension::RequiredCapabilities( + RequiredCapabilitiesExtension::new(&[ExtensionType::RequiredCapabilities], &[], &[]), + )); + + let new_extensions_2 = Extensions::single(Extension::RequiredCapabilities( + RequiredCapabilitiesExtension::new(&[ExtensionType::RatchetTree], &[], &[]), + )); + + alice_group + .propose_group_context_extensions(provider, new_extensions.clone(), &alice_signer) + .expect("failed to build group context extensions proposal"); + + assert_eq!(alice_group.pending_proposals().count(), 1); + + alice_group + .commit_to_pending_proposals(provider, &alice_signer) + .expect("failed to commit to pending proposals"); + + alice_group + .merge_pending_commit(provider) + .expect("error merging pending commit"); + + let required_capabilities = alice_group + .group() + .group_context_extensions() + .required_capabilities() + .expect("couldn't get required_capabilities"); + + // has required_capabilities as required capability + assert!(required_capabilities.extension_types() == [ExtensionType::RequiredCapabilities]); + + // === committing to two group context extensions should fail + + alice_group + .propose_group_context_extensions(provider, new_extensions, &alice_signer) + .expect("failed to build group context extensions proposal"); + + // the proposals need to be different or they will be deduplicated + alice_group + .propose_group_context_extensions(provider, new_extensions_2, &alice_signer) + .expect("failed to build group context extensions proposal"); + + assert_eq!(alice_group.pending_proposals().count(), 2); + + alice_group + .commit_to_pending_proposals(provider, &alice_signer) + .expect_err( + "expected error when committing to multiple group context extensions proposals", + ); + + // === can't update required required_capabilities to extensions that existing group members + // are not capable of + + // contains unsupported extension + let new_extensions = Extensions::single(Extension::RequiredCapabilities( + RequiredCapabilitiesExtension::new(&[ExtensionType::Unknown(0xf042)], &[], &[]), + )); + + alice_group + .propose_group_context_extensions(provider, new_extensions, &alice_signer) + .expect_err("expected an error building GCE proposal with bad required_capabilities"); + + // TODO: we need to test that processing a commit with multiple group context extensions + // proposal also fails. however, we can't generate this commit, because our functions for + // constructing commits does not permit it. See #1476 +} + // Test that the builder pattern accurately configures the new group. #[apply(ciphersuites_and_providers)] fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { @@ -1014,27 +1226,51 @@ fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let test_lifetime = Lifetime::new(3600); let test_wire_format_policy = PURE_CIPHERTEXT_WIRE_FORMAT_POLICY; let test_padding_size = 100; - let test_external_senders = vec![ExternalSender::new( + let test_external_senders = Extension::ExternalSenders(vec![ExternalSender::new( alice_credential_with_key.signature_key.clone(), alice_credential_with_key.credential.clone(), - )]; + )]); + let test_required_capabilities = Extension::RequiredCapabilities( + RequiredCapabilitiesExtension::new(&[ExtensionType::Unknown(0xff00)], &[], &[]), + ); + let test_gc_extensions = Extensions::from_vec(vec![ + test_external_senders.clone(), + test_required_capabilities.clone(), + ]) + .expect("error creating group context extensions"); + let test_crypto_config = CryptoConfig::with_default_version(ciphersuite); let test_sender_ratchet_config = SenderRatchetConfiguration::new(10, 2000); let test_max_past_epochs = 10; let test_number_of_resumption_psks = 5; + let test_capabilities = Capabilities::new( + None, + None, + Some(&[ExtensionType::Unknown(0xff00)]), + None, + None, + ); + let test_leaf_extensions = Extensions::single(Extension::Unknown( + 0xff00, + UnknownExtension(vec![0x00, 0x01, 0x02]), + )); // === Alice creates a group === let alice_group = MlsGroup::builder() .with_group_id(test_group_id.clone()) .padding_size(test_padding_size) .sender_ratchet_configuration(test_sender_ratchet_config.clone()) - .external_senders(test_external_senders.clone()) + .with_group_context_extensions(test_gc_extensions.clone()) + .expect("error adding group context extension to builder") .crypto_config(test_crypto_config) .with_wire_format_policy(test_wire_format_policy) .lifetime(test_lifetime) .use_ratchet_tree_extension(true) .max_past_epochs(test_max_past_epochs) .number_of_resumption_psks(test_number_of_resumption_psks) + .with_leaf_node_extensions(test_leaf_extensions.clone()) + .expect("error adding leaf node extension to builder") + .with_capabilities(test_capabilities.clone()) .build(provider, &alice_signer, alice_credential_with_key) .expect("error creating group using builder"); @@ -1061,17 +1297,119 @@ fn builder_pattern(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let external_senders = group_context .extensions() .external_senders() - .expect("error getting external senders"); - assert_eq!(external_senders, &test_external_senders); + .expect("error getting external senders") + .to_vec(); + assert_eq!( + Extension::ExternalSenders(external_senders), + test_external_senders + ); let crypto_config = CryptoConfig { ciphersuite, version: group_context.protocol_version(), }; assert_eq!(crypto_config, test_crypto_config); + let extensions = group_context.extensions(); + assert_eq!(extensions, &test_gc_extensions); let lifetime = alice_group .own_leaf() .expect("error getting own leaf") .life_time() .expect("leaf doesn't have a lifetime"); assert_eq!(lifetime, &test_lifetime); + let own_leaf = alice_group.own_leaf_node().expect("can't find own leaf"); + let capabilities = own_leaf.capabilities(); + assert_eq!(capabilities, &test_capabilities); + let leaf_extensions = own_leaf.extensions(); + assert_eq!(leaf_extensions, &test_leaf_extensions); + + // Make sure that building with an invalid leaf node extension fails + let invalid_leaf_extensions = + Extensions::single(Extension::ApplicationId(ApplicationIdExtension::new(&[ + 0x00, 0x01, 0x02, + ]))); + + let builder_err = MlsGroup::builder() + .with_leaf_node_extensions(invalid_leaf_extensions) + .expect_err("successfully built group with invalid leaf extensions"); + assert_eq!(builder_err, InvalidExtensionError::IllegalInLeafNodes); +} + +// Test that unknown group context and leaf node extensions can be used in groups +#[apply(ciphersuites_and_providers)] +fn unknown_extensions(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { + let (alice_credential_with_key, _alice_kpb, alice_signer, _alice_pk) = + setup_client("Alice", ciphersuite, provider); + + let unknown_gc_extension = Extension::Unknown(0xff00, UnknownExtension(vec![0, 1, 2, 3])); + let unknown_leaf_extension = Extension::Unknown(0xff01, UnknownExtension(vec![4, 5, 6, 7])); + let unknown_kp_extension = Extension::Unknown(0xff02, UnknownExtension(vec![8, 9, 10, 11])); + let required_extensions = &[ + ExtensionType::Unknown(0xff00), + ExtensionType::Unknown(0xff01), + ]; + let required_capabilities = + Extension::RequiredCapabilities(RequiredCapabilitiesExtension::new(&[], &[], &[])); + let capabilities = Capabilities::new(None, None, Some(required_extensions), None, None); + let test_gc_extensions = Extensions::from_vec(vec![ + unknown_gc_extension.clone(), + required_capabilities.clone(), + ]) + .expect("error creating group context extensions"); + let test_kp_extensions = Extensions::single(unknown_kp_extension.clone()); + + // === Alice creates a group === + let config = CryptoConfig { + ciphersuite, + version: crate::versions::ProtocolVersion::default(), + }; + let mut alice_group = MlsGroup::builder() + .crypto_config(config) + .with_capabilities(capabilities.clone()) + .with_leaf_node_extensions(Extensions::single(unknown_leaf_extension.clone())) + .expect("error adding unknown leaf extension to builder") + .with_group_context_extensions(test_gc_extensions.clone()) + .expect("error adding unknown extension to builder") + .build(provider, &alice_signer, alice_credential_with_key) + .expect("error creating group using builder"); + + // Check that everything was added successfully + let group_context = alice_group.export_group_context(); + assert_eq!(group_context.extensions(), &test_gc_extensions); + let leaf_node = alice_group.own_leaf().expect("error getting own leaf"); + assert_eq!( + leaf_node.extensions(), + &Extensions::single(unknown_leaf_extension) + ); + + // Now let's add Bob to the group and make sure that he joins the group successfully + + // === Alice adds Bob === + let (bob_credential_with_key, _bob_kpb, bob_signer, _bob_pk) = + setup_client("Bob", ciphersuite, provider); + + // Generate a KP that supports the unknown extensions + let bob_key_package = KeyPackage::builder() + .leaf_node_capabilities(capabilities) + .key_package_extensions(test_kp_extensions.clone()) + .build(config, provider, &bob_signer, bob_credential_with_key) + .expect("error building key package"); + + assert_eq!( + bob_key_package.extensions(), + &Extensions::single(unknown_kp_extension) + ); + + // alice adds bob and bob processes the welcome to ensure that the unknown + // extensions are processed correctly + let (_, welcome, _) = alice_group + .add_members(provider, &alice_signer, &[bob_key_package.clone()]) + .unwrap(); + alice_group.merge_pending_commit(provider).unwrap(); + let _bob_group = MlsGroup::new_from_welcome( + provider, + &MlsGroupJoinConfig::default(), + welcome.into_welcome().unwrap(), + Some(alice_group.export_ratchet_tree().into()), + ) + .expect("Error creating group from Welcome"); } diff --git a/openmls/src/group/public_group/builder.rs b/openmls/src/group/public_group/builder.rs index 3da16bdf98..0e02a525fc 100644 --- a/openmls/src/group/public_group/builder.rs +++ b/openmls/src/group/public_group/builder.rs @@ -6,9 +6,9 @@ use crate::{ error::LibraryError, extensions::{ errors::{ExtensionError, InvalidExtensionError}, - Extension, Extensions, ExternalSendersExtension, RequiredCapabilitiesExtension, + Extensions, }, - group::{config::CryptoConfig, GroupContext, GroupId}, + group::{config::CryptoConfig, ExtensionType, GroupContext, GroupId}, key_packages::Lifetime, messages::ConfirmationTag, schedule::CommitSecret, @@ -24,10 +24,9 @@ pub(crate) struct TempBuilderPG1 { crypto_config: CryptoConfig, credential_with_key: CredentialWithKey, lifetime: Option, - required_capabilities: Option, - external_senders: Option, - leaf_extensions: Option, - group_context_extensions: Option, + capabilities: Option, + leaf_node_extensions: Extensions, + group_context_extensions: Extensions, } impl TempBuilderPG1 { @@ -36,21 +35,8 @@ impl TempBuilderPG1 { self } - pub(crate) fn with_required_capabilities( - mut self, - required_capabilities: RequiredCapabilitiesExtension, - ) -> Self { - self.required_capabilities = Some(required_capabilities); - self - } - - pub(crate) fn with_external_senders( - mut self, - external_senders: ExternalSendersExtension, - ) -> Self { - if !external_senders.is_empty() { - self.external_senders = Some(external_senders); - } + pub(crate) fn with_capabilities(mut self, capabilities: Capabilities) -> Self { + self.capabilities = Some(capabilities); self } @@ -64,7 +50,23 @@ impl TempBuilderPG1 { if !is_valid_in_group_context { return Err(InvalidExtensionError::IllegalInGroupContext); } - self.group_context_extensions = Some(extensions); + self.group_context_extensions = extensions; + Ok(self) + } + + pub(crate) fn with_leaf_node_extensions( + mut self, + extensions: Extensions, + ) -> Result { + // None of the default extensions are leaf node extensions, so only + // unknown extensions can be leaf node extensions. + let is_valid_in_leaf_node = extensions + .iter() + .all(|e| matches!(e.extension_type(), ExtensionType::Unknown(_))); + if !is_valid_in_leaf_node { + return Err(InvalidExtensionError::IllegalInLeafNodes); + } + self.leaf_node_extensions = extensions; Ok(self) } @@ -73,52 +75,53 @@ impl TempBuilderPG1 { provider: &impl OpenMlsProvider, signer: &impl Signer, ) -> Result<(TempBuilderPG2, CommitSecret, EncryptionKeyPair), PublicGroupBuildError> { - let capabilities = self - .required_capabilities - .as_ref() - .map(|re| re.extension_types()); + // If there are no capabilities, we want to provide a default version + // plus anything in the required capabilities. + let (required_extensions, required_proposals, required_credentials) = + if let Some(required_capabilities) = + self.group_context_extensions.required_capabilities() + { + // Also, while we're at it, check if we support all required + // capabilities ourselves. + required_capabilities.check_support().map_err(|e| match e { + ExtensionError::UnsupportedProposalType => { + PublicGroupBuildError::UnsupportedProposalType + } + ExtensionError::UnsupportedExtensionType => { + PublicGroupBuildError::UnsupportedExtensionType + } + _ => LibraryError::custom("Unexpected ExtensionError").into(), + })?; + ( + Some(required_capabilities.extension_types()), + Some(required_capabilities.proposal_types()), + Some(required_capabilities.credential_types()), + ) + } else { + (None, None, None) + }; + let capabilities = self.capabilities.unwrap_or(Capabilities::new( + Some(&[self.crypto_config.version]), + Some(&[self.crypto_config.ciphersuite]), + required_extensions, + required_proposals, + required_credentials, + )); let (treesync, commit_secret, leaf_keypair) = TreeSync::new( provider, signer, self.crypto_config, self.credential_with_key, self.lifetime.unwrap_or_default(), - Capabilities::new( - Some(&[self.crypto_config.version]), // TODO: Allow more versions - Some(&[self.crypto_config.ciphersuite]), // TODO: allow more ciphersuites - capabilities, - None, - None, - ), - self.leaf_extensions.unwrap_or(Extensions::empty()), + capabilities, + self.leaf_node_extensions, )?; - let required_capabilities = self.required_capabilities.unwrap_or_default(); - required_capabilities.check_support().map_err(|e| match e { - ExtensionError::UnsupportedProposalType => { - PublicGroupBuildError::UnsupportedProposalType - } - ExtensionError::UnsupportedExtensionType => { - PublicGroupBuildError::UnsupportedExtensionType - } - _ => LibraryError::custom("Unexpected ExtensionError").into(), - })?; - let required_capabilities = Extension::RequiredCapabilities(required_capabilities); - - let mut group_context_extensions = if let Some(exts) = self.group_context_extensions { - exts - } else { - Extensions::empty() - }; - group_context_extensions.add_or_replace(required_capabilities); - if let Some(ext_senders) = self.external_senders { - group_context_extensions.add_or_replace(Extension::ExternalSenders(ext_senders)); - } let group_context = GroupContext::create_initial_group_context( self.crypto_config.ciphersuite, self.group_id, treesync.tree_hash().to_vec(), - group_context_extensions, + self.group_context_extensions, ); let next_builder = TempBuilderPG2 { treesync, @@ -190,10 +193,9 @@ impl PublicGroup { crypto_config, credential_with_key, lifetime: None, - required_capabilities: None, - external_senders: None, - leaf_extensions: None, - group_context_extensions: None, + capabilities: None, + leaf_node_extensions: Extensions::empty(), + group_context_extensions: Extensions::empty(), } } } diff --git a/openmls/src/group/public_group/diff.rs b/openmls/src/group/public_group/diff.rs index d9c7e94e01..e2b3eafb75 100644 --- a/openmls/src/group/public_group/diff.rs +++ b/openmls/src/group/public_group/diff.rs @@ -13,6 +13,7 @@ use super::PublicGroup; use crate::{ binary_tree::{array_representation::TreeSize, LeafNodeIndex}, error::LibraryError, + extensions::Extensions, framing::{mls_auth_content::AuthenticatedContent, public_message::InterimTranscriptHashInput}, group::GroupContext, messages::{proposals::AddProposal, ConfirmationTag, EncryptedGroupSecrets}, @@ -201,6 +202,7 @@ impl<'a> PublicGroupDiff<'a> { pub(crate) fn update_group_context( &mut self, crypto: &impl OpenMlsCrypto, + extensions: Option, ) -> Result<(), LibraryError> { // Calculate tree hash let new_tree_hash = self @@ -208,6 +210,9 @@ impl<'a> PublicGroupDiff<'a> { .compute_tree_hashes(crypto, self.group_context().ciphersuite())?; self.group_context.update_tree_hash(new_tree_hash); self.group_context.increment_epoch(); + if let Some(extensions) = extensions { + self.group_context.set_extensions(extensions); + } Ok(()) } diff --git a/openmls/src/group/public_group/diff/apply_proposals.rs b/openmls/src/group/public_group/diff/apply_proposals.rs index cf25af9d94..470c41a183 100644 --- a/openmls/src/group/public_group/diff/apply_proposals.rs +++ b/openmls/src/group/public_group/diff/apply_proposals.rs @@ -12,12 +12,14 @@ use crate::{ use super::*; /// This struct contain the return values of the `apply_proposals()` function +#[derive(Debug)] pub(crate) struct ApplyProposalsValues { pub(crate) path_required: bool, pub(crate) self_removed: bool, pub(crate) invitation_list: Vec<(LeafNodeIndex, AddProposal)>, pub(crate) presharedkeys: Vec, pub(crate) external_init_proposal_option: Option, + pub(crate) extensions: Option, } impl ApplyProposalsValues { @@ -140,6 +142,16 @@ impl<'a> PublicGroupDiff<'a> { }) .collect(); + // apply group context extension proposal + let extensions = proposal_queue + .filtered_by_type(ProposalType::GroupContextExtensions) + .find_map(|queued_proposal| match queued_proposal.proposal() { + Proposal::GroupContextExtensions(extensions) => { + Some(extensions.extensions().clone()) + } + _ => None, + }); + let proposals_require_path = proposal_queue .queued_proposals() .any(|p| p.proposal().is_path_required()); @@ -159,6 +171,7 @@ impl<'a> PublicGroupDiff<'a> { invitation_list, presharedkeys, external_init_proposal_option, + extensions, }) } } diff --git a/openmls/src/group/public_group/diff/compute_path.rs b/openmls/src/group/public_group/diff/compute_path.rs index 5561dabfed..b41d8784b9 100644 --- a/openmls/src/group/public_group/diff/compute_path.rs +++ b/openmls/src/group/public_group/diff/compute_path.rs @@ -7,6 +7,7 @@ use crate::{ binary_tree::LeafNodeIndex, credentials::CredentialWithKey, error::LibraryError, + extensions::Extensions, group::{ config::CryptoConfig, core_group::create_commit_params::CommitType, errors::CreateCommitError, @@ -35,6 +36,7 @@ pub(crate) struct PathComputationResult { } impl<'a> PublicGroupDiff<'a> { + #[allow(clippy::too_many_arguments)] pub(crate) fn compute_path( &mut self, provider: &impl OpenMlsProvider, @@ -43,6 +45,7 @@ impl<'a> PublicGroupDiff<'a> { commit_type: CommitType, signer: &impl Signer, credential_with_key: Option, + extensions: Option, ) -> Result> { let version = self.group_context().protocol_version(); let ciphersuite = self.group_context().ciphersuite(); @@ -101,7 +104,7 @@ impl<'a> PublicGroupDiff<'a> { // After we've processed the path, we can update the group context s.t. // the updated group context is used for path secret encryption. Note // that we have not yet updated the confirmed transcript hash. - self.update_group_context(provider.crypto())?; + self.update_group_context(provider.crypto(), extensions)?; let serialized_group_context = self .group_context() diff --git a/openmls/src/group/public_group/staged_commit.rs b/openmls/src/group/public_group/staged_commit.rs index c7cfa5a25b..2e583017e0 100644 --- a/openmls/src/group/public_group/staged_commit.rs +++ b/openmls/src/group/public_group/staged_commit.rs @@ -87,6 +87,9 @@ impl PublicGroup { // ValSem107 // ValSem108 self.validate_remove_proposals(&proposal_queue)?; + // ValSem208 + // ValSem209 + self.validate_group_context_extensions_proposal(&proposal_queue)?; // ValSem401 // ValSem402 // ValSem403 @@ -226,7 +229,7 @@ impl PublicGroup { }; // Update group context - diff.update_group_context(crypto)?; + diff.update_group_context(crypto, apply_proposals_values.extensions.clone())?; // Update the confirmed transcript hash before we compute the confirmation tag. diff.update_confirmed_transcript_hash(crypto, mls_content)?; diff --git a/openmls/src/group/public_group/validation.rs b/openmls/src/group/public_group/validation.rs index 6db6328592..d07f2c4c88 100644 --- a/openmls/src/group/public_group/validation.rs +++ b/openmls/src/group/public_group/validation.rs @@ -6,7 +6,8 @@ use std::collections::{BTreeSet, HashSet}; use openmls_traits::types::VerifiableCiphersuite; use super::PublicGroup; -#[cfg(test)] +use crate::group::GroupContextExtensionsProposalValidationError; +use crate::prelude::LibraryError; use crate::treesync::errors::LeafNodeValidationError; use crate::{ binary_tree::array_representation::LeafNodeIndex, @@ -515,7 +516,52 @@ impl PublicGroup { /// Returns a [`LeafNodeValidationError`] if an [`ExtensionType`] /// in `extensions` is not supported by a leaf in this tree. - #[cfg(test)] + pub(crate) fn validate_group_context_extensions_proposal( + &self, + proposal_queue: &ProposalQueue, + ) -> Result<(), GroupContextExtensionsProposalValidationError> { + let mut iter = proposal_queue.filtered_by_type(ProposalType::GroupContextExtensions); + + if let Some(queued_proposal) = iter.next() { + match queued_proposal.proposal() { + Proposal::GroupContextExtensions(extensions) => { + // Check that immutable metadata didn't change and wasn't added or removed. + let new_immutable_metadata = extensions.extensions().immutable_metadata(); + let current_immutable_metadata = + self.group_context.extensions().immutable_metadata(); + if new_immutable_metadata != current_immutable_metadata { + return Err( + GroupContextExtensionsProposalValidationError::ChangedImmutableMetadata, + ); + } + + let ext_type_list: Vec<_> = extensions + .extensions() + .iter() + .map(|ext| ext.extension_type()) + .collect(); + + self.check_extension_support(&ext_type_list).map_err(|_| GroupContextExtensionsProposalValidationError::ExtensionNotSupportedByAllMembers)? + } + _ => { + return Err(GroupContextExtensionsProposalValidationError::LibraryError( + LibraryError::custom( + "found non-gce proposal when filtered for gce proposals", + ), + )) + } + } + } + + // make sure that there is at most one proposal of this type + match iter.next() { + Some(_) => Err(GroupContextExtensionsProposalValidationError::TooManyGCEProposals), + None => Ok(()), + } + } + + /// Returns a [`LeafNodeValidationError`] if an [`ExtensionType`] + /// in `extensions` is not supported by a leaf in this tree. pub(crate) fn check_extension_support( &self, extensions: &[crate::extensions::ExtensionType], diff --git a/openmls/src/group/tests/external_remove_proposal.rs b/openmls/src/group/tests/external_remove_proposal.rs index bb057e4e35..e29d3d707b 100644 --- a/openmls/src/group/tests/external_remove_proposal.rs +++ b/openmls/src/group/tests/external_remove_proposal.rs @@ -30,7 +30,10 @@ fn new_test_group( let mls_group_config = MlsGroupCreateConfig::builder() .wire_format_policy(wire_format_policy) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) - .external_senders(external_senders) + .with_group_context_extensions(Extensions::single(Extension::ExternalSenders( + external_senders, + ))) + .unwrap() .build(); ( @@ -336,6 +339,6 @@ fn external_remove_proposal_should_fail_when_no_external_senders( .unwrap_err(); assert_eq!( error, - ProcessMessageError::ValidationError(ValidationError::NoExternalSendersExtension) + ProcessMessageError::ValidationError(ValidationError::UnauthorizedExternalSender) ); } diff --git a/openmls/src/treesync/node/leaf_node.rs b/openmls/src/treesync/node/leaf_node.rs index b50aa7bf67..fd1f47cf4f 100644 --- a/openmls/src/treesync/node/leaf_node.rs +++ b/openmls/src/treesync/node/leaf_node.rs @@ -27,7 +27,6 @@ use crate::{ versions::ProtocolVersion, }; -#[cfg(test)] use crate::treesync::errors::LeafNodeValidationError; mod capabilities; @@ -382,6 +381,20 @@ impl LeafNode { .contains(extension_type) || default_extensions().iter().any(|et| et == extension_type) } + /// + /// Check whether the this leaf node supports all the required extensions + /// in the provided list. + pub(crate) fn check_extension_support( + &self, + extensions: &[ExtensionType], + ) -> Result<(), LeafNodeValidationError> { + for required in extensions.iter() { + if !self.supports_extension(required) { + return Err(LeafNodeValidationError::UnsupportedExtensions); + } + } + Ok(()) + } } #[cfg(test)] @@ -411,20 +424,6 @@ impl LeafNode { pub fn capabilities_mut(&mut self) -> &mut Capabilities { &mut self.payload.capabilities } - - /// Check whether the this leaf node supports all the required extensions - /// in the provided list. - pub(crate) fn check_extension_support( - &self, - extensions: &[ExtensionType], - ) -> Result<(), LeafNodeValidationError> { - for required in extensions.iter() { - if !self.supports_extension(required) { - return Err(LeafNodeValidationError::UnsupportedExtensions); - } - } - Ok(()) - } } #[cfg(any(feature = "test-utils", test))] diff --git a/openmls/src/treesync/node/leaf_node/capabilities.rs b/openmls/src/treesync/node/leaf_node/capabilities.rs index e07ff477c8..f7eb9c5ac0 100644 --- a/openmls/src/treesync/node/leaf_node/capabilities.rs +++ b/openmls/src/treesync/node/leaf_node/capabilities.rs @@ -137,7 +137,7 @@ impl Capabilities { if required_capabilities .extension_types() .iter() - .any(|e| !self.extensions().contains(e)) + .any(|e| !(self.extensions().contains(e) || default_extensions().contains(e))) { return Err(LeafNodeValidationError::UnsupportedExtensions); } @@ -145,7 +145,7 @@ impl Capabilities { if required_capabilities .proposal_types() .iter() - .any(|p| !self.proposals().contains(p)) + .any(|p| !(self.proposals().contains(p) || default_proposals().contains(p))) { return Err(LeafNodeValidationError::UnsupportedProposals); } @@ -153,7 +153,7 @@ impl Capabilities { if required_capabilities .credential_types() .iter() - .any(|c| !self.credentials().contains(c)) + .any(|c| !(self.credentials().contains(c) || default_credentials().contains(c))) { return Err(LeafNodeValidationError::UnsupportedCredentials); } @@ -216,7 +216,13 @@ pub(super) fn default_ciphersuites() -> Vec { /// All extensions defined in the MLS spec are considered "default" by the spec. pub(super) fn default_extensions() -> Vec { - vec![ExtensionType::ApplicationId] + vec![ + ExtensionType::ApplicationId, + ExtensionType::RatchetTree, + ExtensionType::RequiredCapabilities, + ExtensionType::ExternalPub, + ExtensionType::ExternalSenders, + ] } /// All proposals defined in the MLS spec are considered "default" by the spec. diff --git a/openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs b/openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs index 1857191f8b..7726940212 100644 --- a/openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs +++ b/openmls/src/treesync/tests_and_kats/kats/kat_tree_operations.rs @@ -111,8 +111,9 @@ fn run_test_vector(test: TestElement, provider: &impl OpenMlsProvider) -> Result let mut diff = group.empty_diff(); - diff.apply_proposals(&proposal_queue, None).unwrap(); - diff.update_group_context(provider.crypto()).unwrap(); + let apply_proposal_values = diff.apply_proposals(&proposal_queue, None).unwrap(); + diff.update_group_context(provider.crypto(), apply_proposal_values.extensions.clone()) + .unwrap(); let staged_diff = diff .into_staged_diff(provider.crypto(), ciphersuite) diff --git a/openmls/tests/book_code.rs b/openmls/tests/book_code.rs index b6b8c1399b..9bf48cc137 100644 --- a/openmls/tests/book_code.rs +++ b/openmls/tests/book_code.rs @@ -133,11 +133,27 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { 10, // out_of_order_tolerance 2000, // maximum_forward_distance )) - .external_senders(vec![ExternalSender::new( - ds_credential_with_key.signature_key.clone(), - ds_credential_with_key.credential.clone(), - )]) + .with_group_context_extensions(Extensions::single(Extension::ExternalSenders(vec![ + ExternalSender::new( + ds_credential_with_key.signature_key.clone(), + ds_credential_with_key.credential.clone(), + ), + ]))) + .expect("error adding external senders extension to group context extensions") .crypto_config(CryptoConfig::with_default_version(ciphersuite)) + .capabilities(Capabilities::new( + None, // Defaults to the group's protocol version + None, // Defaults to the group's ciphersuite + None, // Defaults to all basic extension types + None, // Defaults to all basic proposal types + Some(&[CredentialType::Basic]), + )) + // Example leaf extension + .with_leaf_node_extensions(Extensions::single(Extension::Unknown( + 0xff00, + UnknownExtension(vec![0, 1, 2, 3]), + ))) + .expect("failed to configure leaf extensions") .use_ratchet_tree_extension(true) .build(); // ANCHOR_END: mls_group_create_config_example @@ -201,10 +217,6 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { 10, // out_of_order_tolerance 2000, // maximum_forward_distance )) - .external_senders(vec![ExternalSender::new( - ds_credential_with_key.signature_key, - ds_credential_with_key.credential, - )]) .crypto_config(CryptoConfig::with_default_version(ciphersuite)) .use_ratchet_tree_extension(true) .build(provider, &alice_signature_keys, alice_credential.clone())