From 596adef0826834ed8f07a056d4bcb84779612c77 Mon Sep 17 00:00:00 2001 From: Jan Winkelmann <146678+keks@users.noreply.github.com> Date: Thu, 11 Jan 2024 16:53:43 +0100 Subject: [PATCH] Add `with_group_context_extensions` method to group builders (#1473) Add a method with_group_context_extensions to the group builders, that sets an arbitrary set of extensions. The set is of type Extensions, so we know that each extension is only set once. Any RequiredCapability extension mentioned in that set will be overwritten, and if ExternalSenders is specified using the dedicated method, that one is preferred and the one provided using with_group_context_extensions is dropped. --- book/src/user_manual/create_group.md | 8 ++ openmls/src/extensions/errors.rs | 5 + openmls/src/extensions/test_extensions.rs | 121 +++++++++++++++++++++- openmls/src/group/core_group/mod.rs | 17 +++ openmls/src/group/mls_group/builder.rs | 16 ++- openmls/src/group/mls_group/config.rs | 32 +++++- openmls/src/group/mls_group/errors.rs | 2 +- openmls/src/group/public_group/builder.rs | 40 +++++-- openmls/tests/book_code.rs | 24 +++++ 9 files changed, 250 insertions(+), 15 deletions(-) diff --git a/book/src/user_manual/create_group.md b/book/src/user_manual/create_group.md index 8ff8f2c506..b0116b0cb5 100644 --- a/book/src/user_manual/create_group.md +++ b/book/src/user_manual/create_group.md @@ -29,3 +29,11 @@ If someone else already gave you a group ID, e.g., a provider server, you can al ```rust,no_run,noplayground {{#include ../../../openmls/tests/book_code.rs:alice_create_group_with_group_id}} ``` + +The Builder provides methods for setting required capabilities and external senders. +The information passed into these lands in the group context, in the form of extensions. +Should the user want to add further extensions, they can use the `with_group_context_extensions` method: + +```rust,no_run,noplayground +{{#include ../../../openmls/tests/book_code.rs:alice_create_group_with_builder_with_extensions}} +``` diff --git a/openmls/src/extensions/errors.rs b/openmls/src/extensions/errors.rs index 65939f0599..bdc7f9d58a 100644 --- a/openmls/src/extensions/errors.rs +++ b/openmls/src/extensions/errors.rs @@ -92,4 +92,9 @@ pub enum InvalidExtensionError { /// The specified extension could not be found. #[error("The specified extension could not be found.")] NotFound, + /// The provided extension list contains an extension that is not allowed in group contexts + #[error( + "The provided extension list contains an extension that is not allowed in group contexts." + )] + IllegalInGroupContext, } diff --git a/openmls/src/extensions/test_extensions.rs b/openmls/src/extensions/test_extensions.rs index 6badafcbe6..8fee2437bc 100644 --- a/openmls/src/extensions/test_extensions.rs +++ b/openmls/src/extensions/test_extensions.rs @@ -13,7 +13,8 @@ use crate::{ group::{config::CryptoConfig, errors::*, *}, key_packages::*, messages::proposals::ProposalType, - prelude::Capabilities, + prelude::{Capabilities, RatchetTreeIn}, + prelude_test::HpkePublicKey, schedule::psk::store::ResumptionPskStore, test_utils::*, versions::ProtocolVersion, @@ -248,6 +249,124 @@ fn required_capabilities() { assert_eq!(extension_bytes, encoded); } +#[apply(ciphersuites_and_providers)] +fn with_group_context_extensions(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { + // create an extension that we can check for later + let test_extension = Extension::Unknown(0xf023, UnknownExtension(vec![0xca, 0xfe])); + let extensions = Extensions::single(test_extension.clone()); + + let alice_credential_with_key_and_signer = tests::utils::generate_credential_with_key( + "Alice".into(), + ciphersuite.signature_algorithm(), + provider, + ); + + let mls_group_create_config = MlsGroupCreateConfig::builder() + .with_group_context_extensions(extensions) + .expect("failed to apply extensions at group config builder") + .crypto_config(CryptoConfig::with_default_version(ciphersuite)) + .build(); + + // === Alice creates a group === + let alice_group = MlsGroup::new( + provider, + &alice_credential_with_key_and_signer.signer, + &mls_group_create_config, + alice_credential_with_key_and_signer.credential_with_key, + ) + .expect("An unexpected error occurred."); + + // === Group contains extension === + let found_test_extension = alice_group + .export_group_context() + .extensions() + .find_by_type(ExtensionType::Unknown(0xf023)) + .expect("failed to get test extensions from group context"); + assert_eq!(found_test_extension, &test_extension); +} + +#[apply(ciphersuites_and_providers)] +fn wrong_extension_with_group_context_extensions( + ciphersuite: Ciphersuite, + provider: &impl OpenMlsProvider, +) { + // Extension types that are known to not be allowed here: + // - application id + // - external pub + // - ratchet tree + + let alice_credential_with_key_and_signer = tests::utils::generate_credential_with_key( + "Alice".into(), + ciphersuite.signature_algorithm(), + provider, + ); + + let crypto_config = CryptoConfig::with_default_version(ciphersuite); + + // create an extension that we can check for later + let test_extension = Extension::ApplicationId(ApplicationIdExtension::new(&[0xca, 0xfe])); + let extensions = Extensions::single(test_extension.clone()); + + let err = MlsGroup::builder() + .with_group_context_extensions(extensions.clone()) + .expect_err("builder accepted non-group-context extension"); + + assert_eq!(err, InvalidExtensionError::IllegalInGroupContext); + let err = PublicGroup::builder( + GroupId::from_slice(&[0xbe, 0xef]), + crypto_config, + alice_credential_with_key_and_signer + .credential_with_key + .clone(), + ) + .with_group_context_extensions(extensions) + .expect_err("builder accepted non-group-context extension"); + + assert_eq!(err, InvalidExtensionError::IllegalInGroupContext); + // create an extension that we can check for later + let test_extension = + Extension::ExternalPub(ExternalPubExtension::new(HpkePublicKey::new(vec![]))); + let extensions = Extensions::single(test_extension.clone()); + + let err = MlsGroup::builder() + .with_group_context_extensions(extensions.clone()) + .expect_err("builder accepted non-group-context extension"); + assert_eq!(err, InvalidExtensionError::IllegalInGroupContext); + + let err = PublicGroup::builder( + GroupId::from_slice(&[0xbe, 0xef]), + crypto_config, + alice_credential_with_key_and_signer + .credential_with_key + .clone(), + ) + .with_group_context_extensions(extensions) + .expect_err("builder accepted non-group-context extension"); + assert_eq!(err, InvalidExtensionError::IllegalInGroupContext); + + // create an extension that we can check for later + let test_extension = Extension::RatchetTree(RatchetTreeExtension::new( + RatchetTreeIn::from_nodes(vec![]).into(), + )); + let extensions = Extensions::single(test_extension.clone()); + + let err = MlsGroup::builder() + .with_group_context_extensions(extensions.clone()) + .expect_err("builder accepted non-group-context extension"); + assert_eq!(err, InvalidExtensionError::IllegalInGroupContext); + + let err = PublicGroup::builder( + GroupId::from_slice(&[0xbe, 0xef]), + crypto_config, + alice_credential_with_key_and_signer + .credential_with_key + .clone(), + ) + .with_group_context_extensions(extensions) + .expect_err("builder accepted non-group-context extension"); + assert_eq!(err, InvalidExtensionError::IllegalInGroupContext); +} + #[apply(ciphersuites_and_providers)] fn last_resort_extension(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { let last_resort = Extension::LastResort(LastResortExtension::default()); diff --git a/openmls/src/group/core_group/mod.rs b/openmls/src/group/core_group/mod.rs index 5fb10ceb0e..b56b53f8b6 100644 --- a/openmls/src/group/core_group/mod.rs +++ b/openmls/src/group/core_group/mod.rs @@ -58,6 +58,7 @@ use crate::{ ciphersuite::{signable::Signable, HpkePublicKey}, credentials::*, error::LibraryError, + extensions::errors::InvalidExtensionError, framing::{mls_auth_content::AuthenticatedContent, *}, group::{config::CryptoConfig, *}, key_packages::*, @@ -200,6 +201,22 @@ impl CoreGroupBuilder { } self } + + /// Sets initial group context extensions. Note that RequiredCapabilities + /// extensions will be overwritten, and should be set using a call to + /// `required_capabilities`. If `ExternalSenders` extensions are provided + /// both in this call, and a call to `external_senders`, only the one from + /// the call to `external_senders` will be included. + pub(crate) fn with_group_context_extensions( + mut self, + extensions: Extensions, + ) -> Result { + self.public_group_builder = self + .public_group_builder + .with_group_context_extensions(extensions)?; + Ok(self) + } + /// Set the number of past epochs the group should keep secrets. pub fn with_max_past_epoch_secrets(mut self, max_past_epochs: usize) -> Self { self.max_past_epochs = max_past_epochs; diff --git a/openmls/src/group/mls_group/builder.rs b/openmls/src/group/mls_group/builder.rs index fe7040bb12..ed8ec42069 100644 --- a/openmls/src/group/mls_group/builder.rs +++ b/openmls/src/group/mls_group/builder.rs @@ -2,7 +2,7 @@ use openmls_traits::{key_store::OpenMlsKeyStore, signatures::Signer, OpenMlsProv use crate::{ credentials::CredentialWithKey, - extensions::ExternalSendersExtension, + extensions::{errors::InvalidExtensionError, Extensions, ExternalSendersExtension}, group::{ config::CryptoConfig, public_group::errors::PublicGroupBuildError, CoreGroup, CoreGroupBuildError, CoreGroupConfig, GroupId, MlsGroupCreateConfig, @@ -13,7 +13,7 @@ use crate::{ use super::{InnerState, MlsGroup, MlsGroupState}; -#[derive(Default)] +#[derive(Default, Debug)] pub struct MlsGroupBuilder { group_id: Option, mls_group_create_config_builder: MlsGroupCreateConfigBuilder, @@ -72,6 +72,7 @@ impl MlsGroupBuilder { .with_config(group_config) .with_required_capabilities(mls_group_create_config.required_capabilities.clone()) .with_external_senders(mls_group_create_config.external_senders.clone()) + .with_group_context_extensions(mls_group_create_config.group_context_extensions.clone())? .with_max_past_epoch_secrets(mls_group_create_config.join_config.max_past_epochs) .with_lifetime(*mls_group_create_config.lifetime()) .build(provider, signer) @@ -198,4 +199,15 @@ impl MlsGroupBuilder { .external_senders(external_senders); self } + + /// Sets the initial group context extensions + pub fn with_group_context_extensions( + mut self, + extensions: Extensions, + ) -> Result { + self.mls_group_create_config_builder = self + .mls_group_create_config_builder + .with_group_context_extensions(extensions)?; + Ok(self) + } } diff --git a/openmls/src/group/mls_group/config.rs b/openmls/src/group/mls_group/config.rs index b7366cdf00..c270ece7d2 100644 --- a/openmls/src/group/mls_group/config.rs +++ b/openmls/src/group/mls_group/config.rs @@ -29,7 +29,7 @@ use super::*; use crate::{ - group::config::CryptoConfig, key_packages::Lifetime, + extensions::errors::InvalidExtensionError, group::config::CryptoConfig, key_packages::Lifetime, tree::sender_ratchet::SenderRatchetConfiguration, }; use serde::{Deserialize, Serialize}; @@ -93,6 +93,8 @@ pub struct MlsGroupCreateConfig { pub(crate) crypto_config: CryptoConfig, /// Configuration parameters relevant to group operation at runtime pub(crate) join_config: MlsGroupJoinConfig, + /// List of initial group context extensions + pub(crate) group_context_extensions: Extensions, } /// Builder struct for an [`MlsGroupJoinConfig`]. @@ -200,6 +202,13 @@ impl MlsGroupCreateConfig { &self.external_senders } + /// Returns the [`Extensions`] set as the initial group context. + /// This does not contain the initial group context extensions + /// added from builder calls to `external_senders` or `required_capabilities`. + pub fn group_context_extensions(&self) -> &Extensions { + &self.group_context_extensions + } + /// Returns the [`MlsGroupCreateConfig`] lifetime configuration. pub fn lifetime(&self) -> &Lifetime { &self.lifetime @@ -228,7 +237,7 @@ impl MlsGroupCreateConfig { } /// Builder for an [`MlsGroupCreateConfig`]. -#[derive(Default)] +#[derive(Default, Debug)] pub struct MlsGroupCreateConfigBuilder { config: MlsGroupCreateConfig, } @@ -317,6 +326,25 @@ impl MlsGroupCreateConfigBuilder { self } + /// Sets initial group context extensions. Note that RequiredCapabilities + /// extensions will be overwritten, and should be set using a call to + /// `required_capabilities`. If `ExternalSenders` extensions are provided + /// both in this call, and a call to `external_senders`, only the one from + /// the call to `external_senders` will be included. + pub fn with_group_context_extensions( + mut self, + extensions: Extensions, + ) -> Result { + let is_valid_in_group_context = extensions.application_id().is_none() + && extensions.ratchet_tree().is_none() + && extensions.external_pub().is_none(); + if !is_valid_in_group_context { + return Err(InvalidExtensionError::IllegalInGroupContext); + } + self.config.group_context_extensions = extensions; + Ok(self) + } + /// Finalizes the builder and retursn an `[MlsGroupCreateConfig`]. pub fn build(self) -> MlsGroupCreateConfig { self.config diff --git a/openmls/src/group/mls_group/errors.rs b/openmls/src/group/mls_group/errors.rs index 906f6d1d0e..a23e47667e 100644 --- a/openmls/src/group/mls_group/errors.rs +++ b/openmls/src/group/mls_group/errors.rs @@ -38,7 +38,7 @@ pub enum NewGroupError { UnsupportedExtensionType, /// Invalid extensions set in configuration #[error("Invalid extensions set in configuration")] - InvalidExtensions(InvalidExtensionError), + InvalidExtensions(#[from] InvalidExtensionError), } /// EmptyInput error diff --git a/openmls/src/group/public_group/builder.rs b/openmls/src/group/public_group/builder.rs index aac7ac329f..3da16bdf98 100644 --- a/openmls/src/group/public_group/builder.rs +++ b/openmls/src/group/public_group/builder.rs @@ -5,8 +5,8 @@ use crate::{ credentials::CredentialWithKey, error::LibraryError, extensions::{ - errors::ExtensionError, Extension, Extensions, ExternalSendersExtension, - RequiredCapabilitiesExtension, + errors::{ExtensionError, InvalidExtensionError}, + Extension, Extensions, ExternalSendersExtension, RequiredCapabilitiesExtension, }, group::{config::CryptoConfig, GroupContext, GroupId}, key_packages::Lifetime, @@ -18,6 +18,7 @@ use crate::{ }, }; +#[derive(Debug)] pub(crate) struct TempBuilderPG1 { group_id: GroupId, crypto_config: CryptoConfig, @@ -26,6 +27,7 @@ pub(crate) struct TempBuilderPG1 { required_capabilities: Option, external_senders: Option, leaf_extensions: Option, + group_context_extensions: Option, } impl TempBuilderPG1 { @@ -52,6 +54,20 @@ impl TempBuilderPG1 { self } + pub(crate) fn with_group_context_extensions( + mut self, + extensions: Extensions, + ) -> Result { + let is_valid_in_group_context = extensions.application_id().is_none() + && extensions.ratchet_tree().is_none() + && extensions.external_pub().is_none(); + if !is_valid_in_group_context { + return Err(InvalidExtensionError::IllegalInGroupContext); + } + self.group_context_extensions = Some(extensions); + Ok(self) + } + pub(crate) fn get_secrets( self, provider: &impl OpenMlsProvider, @@ -87,17 +103,22 @@ impl TempBuilderPG1 { _ => LibraryError::custom("Unexpected ExtensionError").into(), })?; let required_capabilities = Extension::RequiredCapabilities(required_capabilities); - let extensions = - if let Some(ext_senders) = self.external_senders.map(Extension::ExternalSenders) { - vec![required_capabilities, ext_senders] - } else { - vec![required_capabilities] - }; + + let mut group_context_extensions = if let Some(exts) = self.group_context_extensions { + exts + } else { + Extensions::empty() + }; + group_context_extensions.add_or_replace(required_capabilities); + if let Some(ext_senders) = self.external_senders { + group_context_extensions.add_or_replace(Extension::ExternalSenders(ext_senders)); + } + let group_context = GroupContext::create_initial_group_context( self.crypto_config.ciphersuite, self.group_id, treesync.tree_hash().to_vec(), - Extensions::from_vec(extensions)?, + group_context_extensions, ); let next_builder = TempBuilderPG2 { treesync, @@ -172,6 +193,7 @@ impl PublicGroup { required_capabilities: None, external_senders: None, leaf_extensions: None, + group_context_extensions: None, } } } diff --git a/openmls/tests/book_code.rs b/openmls/tests/book_code.rs index 3ac0574b8e..b6b8c1399b 100644 --- a/openmls/tests/book_code.rs +++ b/openmls/tests/book_code.rs @@ -170,6 +170,30 @@ fn book_operations(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { // Silence "unused variable" and "does not need to be mutable" warnings. let _ignore_mut_warning = &mut alice_group; + let external_senders_list = vec![]; + + // ANCHOR: alice_create_group_with_builder_with_extensions + // we are adding an external senders list as an example. + let extensions = + Extensions::from_vec(vec![Extension::ExternalSenders(external_senders_list)]) + .expect("failed to create extensions list"); + + let mut alice_group = MlsGroup::builder() + .padding_size(100) + .sender_ratchet_configuration(SenderRatchetConfiguration::new( + 10, // out_of_order_tolerance + 2000, // maximum_forward_distance + )) + .with_group_context_extensions(extensions) // NB: the builder method returns a Result + .expect("failed to apply group context extensions") + .use_ratchet_tree_extension(true) + .build(provider, &alice_signature_keys, alice_credential.clone()) + .expect("An unexpected error occurred."); + // ANCHOR_END: alice_create_group_with_builder_with_extensions + + // Silence "unused variable" and "does not need to be mutable" warnings. + let _ignore_mut_warning = &mut alice_group; + // ANCHOR: alice_create_group_with_builder let mut alice_group = MlsGroup::builder() .padding_size(100)