From 35cc116ffec8a5e9f86326976984df9534aaeda2 Mon Sep 17 00:00:00 2001 From: raphaelrobert Date: Wed, 8 Nov 2023 18:24:57 +0100 Subject: [PATCH 1/4] Add ability to set RequiredCapabilities in MlsGroupConfig builder (#1448) --- openmls/src/group/mls_group/config.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/openmls/src/group/mls_group/config.rs b/openmls/src/group/mls_group/config.rs index edadd4bf8c..a78c5e4ff6 100644 --- a/openmls/src/group/mls_group/config.rs +++ b/openmls/src/group/mls_group/config.rs @@ -93,6 +93,11 @@ impl MlsGroupConfig { self.use_ratchet_tree_extension } + /// Returns the [`MlsGroupConfig`] required capabilities extension. + pub fn required_capabilities(&self) -> &RequiredCapabilitiesExtension { + &self.required_capabilities + } + /// Returns the [`MlsGroupConfig`] sender ratchet configuration. pub fn sender_ratchet_configuration(&self) -> &SenderRatchetConfiguration { &self.sender_ratchet_configuration @@ -178,6 +183,15 @@ impl MlsGroupConfigBuilder { self } + /// Sets the `required_capabilities` property of the MlsGroupConfig. + pub fn required_capabilities( + mut self, + required_capabilities: RequiredCapabilitiesExtension, + ) -> Self { + self.config.required_capabilities = required_capabilities; + self + } + /// Sets the `sender_ratchet_configuration` property of the MlsGroupConfig. /// See [`SenderRatchetConfiguration`] for more information. pub fn sender_ratchet_configuration( From f18a8132191563a9ddb03d0e102e2f449f8fa552 Mon Sep 17 00:00:00 2001 From: Franziskus Kiefer Date: Mon, 13 Nov 2023 14:12:05 +0100 Subject: [PATCH 2/4] Stop building fuzzer as part of the workspace on CI (#1450) --- .github/workflows/build_test_workspace.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/build_test_workspace.yml b/.github/workflows/build_test_workspace.yml index 49ab519589..329904db80 100644 --- a/.github/workflows/build_test_workspace.yml +++ b/.github/workflows/build_test_workspace.yml @@ -34,6 +34,6 @@ jobs: - uses: dtolnay/rust-toolchain@stable - uses: Swatinem/rust-cache@v2 - name: Build workspace - run: cargo build --workspace --all-targets + run: cargo build --workspace --all-targets --exclude openmls-fuzz - name: Test workspace run: cargo test --workspace --all-targets --exclude=openmls --exclude openmls-fuzz From a4b7886055897ae119258c0ae6fd79cc0019af8d Mon Sep 17 00:00:00 2001 From: Konrad Kohbrok Date: Mon, 13 Nov 2023 14:30:59 +0100 Subject: [PATCH 3/4] Add last resort extension (#1449) Co-authored-by: Franziskus Kiefer --- openmls/src/extensions/codec.rs | 7 ++ openmls/src/extensions/last_resort.rs | 28 ++++++ openmls/src/extensions/mod.rs | 60 ++++++++----- openmls/src/extensions/test_extensions.rs | 101 +++++++++++++++++++++- openmls/src/group/mls_group/creation.rs | 24 +++-- openmls/src/key_packages/mod.rs | 5 ++ 6 files changed, 193 insertions(+), 32 deletions(-) create mode 100644 openmls/src/extensions/last_resort.rs diff --git a/openmls/src/extensions/codec.rs b/openmls/src/extensions/codec.rs index a254eee176..936ee63fa2 100644 --- a/openmls/src/extensions/codec.rs +++ b/openmls/src/extensions/codec.rs @@ -8,6 +8,8 @@ use crate::extensions::{ UnknownExtension, }; +use super::last_resort::LastResortExtension; + fn vlbytes_len_len(length: usize) -> usize { if length < 0x40 { 1 @@ -34,6 +36,7 @@ impl Size for Extension { Extension::RequiredCapabilities(e) => e.tls_serialized_len(), Extension::ExternalPub(e) => e.tls_serialized_len(), Extension::ExternalSenders(e) => e.tls_serialized_len(), + Extension::LastResort(e) => e.tls_serialized_len(), Extension::Unknown(_, e) => e.0.len(), }; @@ -65,6 +68,7 @@ impl Serialize for Extension { Extension::RequiredCapabilities(e) => e.tls_serialize(&mut extension_data), 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::Unknown(_, e) => extension_data .write_all(e.0.as_slice()) .map(|_| e.0.len()) @@ -111,6 +115,9 @@ impl Deserialize for Extension { ExtensionType::ExternalSenders => Extension::ExternalSenders( ExternalSendersExtension::tls_deserialize(&mut extension_data)?, ), + ExtensionType::LastResort => { + Extension::LastResort(LastResortExtension::tls_deserialize(&mut extension_data)?) + } ExtensionType::Unknown(unknown) => { Extension::Unknown(unknown, UnknownExtension(extension_data.to_vec())) } diff --git a/openmls/src/extensions/last_resort.rs b/openmls/src/extensions/last_resort.rs new file mode 100644 index 0000000000..895b5a4b41 --- /dev/null +++ b/openmls/src/extensions/last_resort.rs @@ -0,0 +1,28 @@ +use tls_codec::{TlsDeserialize, TlsSerialize, TlsSize}; + +use super::{Deserialize, Serialize}; + +/// ```c +/// // draft-ietf-mls-extensions-03 +/// struct {} LastResort; +/// ``` +#[derive( + PartialEq, + Eq, + Clone, + Debug, + Serialize, + Deserialize, + TlsSerialize, + TlsDeserialize, + TlsSize, + Default, +)] +pub struct LastResortExtension {} + +impl LastResortExtension { + /// Create a new `last_resort` extension. + pub fn new() -> Self { + Self::default() + } +} diff --git a/openmls/src/extensions/mod.rs b/openmls/src/extensions/mod.rs index d5ee4d2b56..46d3932cc2 100644 --- a/openmls/src/extensions/mod.rs +++ b/openmls/src/extensions/mod.rs @@ -1,13 +1,13 @@ //! # Extensions //! //! In MLS, extensions appear in the following places: -//! - In [`KeyPackages`](`crate::key_packages`), to describe client capabilities and aspects of their -//! participation in the group. +//! - In [`KeyPackages`](`crate::key_packages`), to describe client capabilities +//! and aspects of their participation in the group. //! - In the `GroupInfo`, to tell new members of a group what parameters are -//! being used by the group, and to provide any additional details required -//! to join the group. -//! - In the `GroupContext` object, to ensure that all members of the group -//! have the same view of the parameters in use. +//! being used by the group, and to provide any additional details required to +//! join the group. +//! - In the `GroupContext` object, to ensure that all members of the group have +//! the same view of the parameters in use. //! //! Note that `GroupInfo` and `GroupContext` are not exposed in OpenMLS' public //! API. @@ -31,6 +31,7 @@ mod application_id_extension; mod codec; mod external_pub_extension; mod external_sender_extension; +mod last_resort; mod ratchet_tree_extension; mod required_capabilities; use errors::*; @@ -44,6 +45,7 @@ pub use external_pub_extension::ExternalPubExtension; pub use external_sender_extension::{ ExternalSender, ExternalSendersExtension, SenderExtensionIndex, }; +pub use last_resort::LastResortExtension; pub use ratchet_tree_extension::RatchetTreeExtension; pub use required_capabilities::RequiredCapabilitiesExtension; @@ -71,8 +73,8 @@ pub enum ExtensionType { /// application-defined identifier to a KeyPackage. ApplicationId, - /// The ratchet tree extensions provides the whole public state of the ratchet - /// tree. + /// The ratchet tree extensions provides the whole public state of the + /// ratchet tree. RatchetTree, /// The required capabilities extension defines the configuration of a group @@ -87,6 +89,10 @@ pub enum ExtensionType { /// of senders that are permitted to send external proposals to the group. ExternalSenders, + /// KeyPackage extension that marks a KeyPackage for use in a last resort + /// scenario. + LastResort, + /// A currently unknown extension type. Unknown(u16), } @@ -125,6 +131,7 @@ impl From for ExtensionType { 3 => ExtensionType::RequiredCapabilities, 4 => ExtensionType::ExternalPub, 5 => ExtensionType::ExternalSenders, + 10 => ExtensionType::LastResort, unknown => ExtensionType::Unknown(unknown), } } @@ -138,6 +145,7 @@ impl From for u16 { ExtensionType::RequiredCapabilities => 3, ExtensionType::ExternalPub => 4, ExtensionType::ExternalSenders => 5, + ExtensionType::LastResort => 10, ExtensionType::Unknown(unknown) => unknown, } } @@ -153,6 +161,7 @@ impl ExtensionType { | ExtensionType::RequiredCapabilities | ExtensionType::ExternalPub | ExtensionType::ExternalSenders + | ExtensionType::LastResort ) } } @@ -182,12 +191,15 @@ pub enum Extension { /// A [`RequiredCapabilitiesExtension`] RequiredCapabilities(RequiredCapabilitiesExtension), - /// A [`ExternalPubExtension`] + /// An [`ExternalPubExtension`] ExternalPub(ExternalPubExtension), - /// A [`ExternalPubExtension`] + /// An [`ExternalSendersExtension`] ExternalSenders(ExternalSendersExtension), + /// A [`LastResortExtension`] + LastResort(LastResortExtension), + /// A currently unknown extension. Unknown(u16, UnknownExtension), } @@ -234,7 +246,8 @@ impl Extensions { /// Create an extension list with multiple extensions. /// - /// This function will fail when the list of extensions contains duplicate extension types. + /// This function will fail when the list of extensions contains duplicate + /// extension types. pub fn from_vec(extensions: Vec) -> Result { extensions.try_into() } @@ -246,7 +259,8 @@ impl Extensions { /// Add an extension to the extension list. /// - /// Returns an error when there already is an extension with the same extension type. + /// Returns an error when there already is an extension with the same + /// extension type. pub fn add(&mut self, extension: Extension) -> Result<(), InvalidExtensionError> { if self.contains(extension.extension_type()) { return Err(InvalidExtensionError::Duplicate); @@ -268,7 +282,8 @@ impl Extensions { /// Remove an extension from the extension list. /// - /// Returns the removed extension or `None` when there is no extension with the given extension type. + /// Returns the removed extension or `None` when there is no extension with + /// the given extension type. pub fn remove(&mut self, extension_type: ExtensionType) -> Option { if let Some(pos) = self .unique @@ -281,7 +296,8 @@ impl Extensions { } } - /// Returns `true` iff the extension list contains an extension with the given extension type. + /// Returns `true` iff the extension list contains an extension with the + /// given extension type. pub fn contains(&self, extension_type: ExtensionType) -> bool { self.unique .iter() @@ -335,7 +351,8 @@ impl Extensions { }) } - /// Get a reference to the [`RequiredCapabilitiesExtension`] if there is any. + /// Get a reference to the [`RequiredCapabilitiesExtension`] if there is + /// any. pub fn required_capabilities(&self) -> Option<&RequiredCapabilitiesExtension> { self.find_by_type(ExtensionType::RequiredCapabilities) .and_then(|e| match e { @@ -389,8 +406,8 @@ impl Extension { } /// Get a reference to this extension as [`RequiredCapabilitiesExtension`]. - /// Returns an [`ExtensionError::InvalidExtensionType`] error if called on an - /// [`Extension`] that's not a [`RequiredCapabilitiesExtension`]. + /// Returns an [`ExtensionError::InvalidExtensionType`] error if called on + /// an [`Extension`] that's not a [`RequiredCapabilitiesExtension`]. pub fn as_required_capabilities_extension( &self, ) -> Result<&RequiredCapabilitiesExtension, ExtensionError> { @@ -403,8 +420,8 @@ impl Extension { } /// Get a reference to this extension as [`ExternalPubExtension`]. - /// Returns an [`ExtensionError::InvalidExtensionType`] error if called on an - /// [`Extension`] that's not a [`ExternalPubExtension`]. + /// Returns an [`ExtensionError::InvalidExtensionType`] error if called on + /// an [`Extension`] that's not a [`ExternalPubExtension`]. pub fn as_external_pub_extension(&self) -> Result<&ExternalPubExtension, ExtensionError> { match self { Self::ExternalPub(e) => Ok(e), @@ -415,8 +432,8 @@ impl Extension { } /// Get a reference to this extension as [`ExternalSendersExtension`]. - /// Returns an [`ExtensionError::InvalidExtensionType`] error if called on an - /// [`Extension`] that's not a [`ExternalSendersExtension`]. + /// Returns an [`ExtensionError::InvalidExtensionType`] error if called on + /// an [`Extension`] that's not a [`ExternalSendersExtension`]. pub fn as_external_senders_extension( &self, ) -> Result<&ExternalSendersExtension, ExtensionError> { @@ -437,6 +454,7 @@ impl Extension { Extension::RequiredCapabilities(_) => ExtensionType::RequiredCapabilities, Extension::ExternalPub(_) => ExtensionType::ExternalPub, Extension::ExternalSenders(_) => ExtensionType::ExternalSenders, + Extension::LastResort(_) => ExtensionType::LastResort, Extension::Unknown(kind, _) => ExtensionType::Unknown(*kind), } } diff --git a/openmls/src/extensions/test_extensions.rs b/openmls/src/extensions/test_extensions.rs index f84d360498..4c9b567f7a 100644 --- a/openmls/src/extensions/test_extensions.rs +++ b/openmls/src/extensions/test_extensions.rs @@ -3,17 +3,20 @@ //! Proper testing is done through the public APIs. use openmls_rust_crypto::OpenMlsRustCrypto; +use openmls_traits::key_store::OpenMlsKeyStore; use tls_codec::{Deserialize, Serialize}; use super::*; use crate::{ credentials::*, framing::*, - group::{errors::*, *}, + group::{config::CryptoConfig, errors::*, *}, key_packages::*, messages::proposals::ProposalType, + prelude::Capabilities, schedule::psk::store::ResumptionPskStore, test_utils::*, + versions::ProtocolVersion, }; #[test] @@ -206,7 +209,8 @@ fn ratchet_tree_extension(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvi #[test] fn required_capabilities() { - // A raw required capabilities extension with the default values for openmls (none). + // A raw required capabilities extension with the default values for openmls + // (none). let extension_bytes = vec![0, 3, 3, 0, 0, 0]; let ext = Extension::RequiredCapabilities(RequiredCapabilitiesExtension::default()); @@ -243,3 +247,96 @@ fn required_capabilities() { assert_eq!(ext, ext_decoded); assert_eq!(extension_bytes, encoded); } + +#[apply(ciphersuites_and_providers)] +fn last_resort_extension(ciphersuite: Ciphersuite, provider: &impl OpenMlsProvider) { + let last_resort = Extension::LastResort(LastResortExtension::default()); + + // Build a KeyPackage with a last resort extension + let credential = Credential::new(b"Bob".to_vec(), CredentialType::Basic).unwrap(); + let signer = + openmls_basic_credential::SignatureKeyPair::new(ciphersuite.signature_algorithm()).unwrap(); + + let extensions = Extensions::single(last_resort); + let crypto_config = CryptoConfig::with_default_version(ciphersuite); + let capabilities = Capabilities::new( + None, + None, + // Add last resort extension as supported extension + Some(&[ExtensionType::LastResort]), + None, + None, + ); + let kp = KeyPackage::builder() + .key_package_extensions(extensions) + .leaf_node_capabilities(capabilities) + .build( + crypto_config, + provider, + &signer, + CredentialWithKey { + credential: credential.clone(), + signature_key: signer.to_public_vec().into(), + }, + ) + .expect("error building key package with last resort extension"); + assert!(kp.last_resort()); + let encoded_kp = kp + .tls_serialize_detached() + .expect("error encoding key package with last resort extension"); + let decoded_kp = KeyPackageIn::tls_deserialize(&mut encoded_kp.as_slice()) + .expect("error decoding key package with last resort extension") + .validate(provider.crypto(), ProtocolVersion::default()) + .expect("error validating key package with last resort extension"); + assert!(decoded_kp.last_resort()); + + // If we join a group using a last resort KP, it shouldn't be deleted from the + // provider. + + let alice_credential_with_key_and_signer = tests::utils::generate_credential_with_key( + "Alice".into(), + ciphersuite.signature_algorithm(), + provider, + ); + + let mls_group_config = MlsGroupConfigBuilder::new() + .crypto_config(CryptoConfig::with_default_version(ciphersuite)) + .build(); + + // === Alice creates a group === + let mut alice_group = MlsGroup::new( + provider, + &alice_credential_with_key_and_signer.signer, + &mls_group_config, + alice_credential_with_key_and_signer.credential_with_key, + ) + .expect("An unexpected error occurred."); + + // === Alice adds Bob === + + let (_message, welcome, _group_info) = alice_group + .add_members( + provider, + &alice_credential_with_key_and_signer.signer, + &[kp.clone()], + ) + .expect("An unexpected error occurred."); + + alice_group.merge_pending_commit(provider).unwrap(); + + let _bob_group = MlsGroup::new_from_welcome( + provider, + &mls_group_config, + welcome.into_welcome().expect("Unexpected MLS message"), + Some(alice_group.export_ratchet_tree().into()), + ) + .expect("An unexpected error occurred."); + + // This should not have deleted the KP from the store + let kp: Option = provider.key_store().read( + kp.hash_ref(provider.crypto()) + .expect("error hashing kp") + .as_slice(), + ); + assert!(kp.is_some()); +} diff --git a/openmls/src/group/mls_group/creation.rs b/openmls/src/group/mls_group/creation.rs index 6e05429454..df2164bea4 100644 --- a/openmls/src/group/mls_group/creation.rs +++ b/openmls/src/group/mls_group/creation.rs @@ -17,7 +17,8 @@ use crate::{ impl MlsGroup { // === Group creation === - /// Creates a new group with the creator as the only member (and a random group ID). + /// Creates a new group with the creator as the only member (and a random + /// group ID). /// /// This function removes the private key corresponding to the /// `key_package` from the key store. @@ -36,7 +37,8 @@ impl MlsGroup { ) } - /// Creates a new group with a given group ID with the creator as the only member. + /// Creates a new group with a given group ID with the creator as the only + /// member. pub fn new_with_group_id( provider: &impl OpenMlsProvider, signer: &impl Signer, @@ -134,11 +136,15 @@ impl MlsGroup { }; // Delete the [`KeyPackage`] and the corresponding private key from the - // key store - key_package_bundle - .key_package - .delete(provider) - .map_err(WelcomeError::KeyStoreError)?; + // key store, but only if it doesn't have a last resort extension. + if !key_package_bundle.key_package().last_resort() { + key_package_bundle + .key_package + .delete(provider) + .map_err(WelcomeError::KeyStoreError)?; + } else { + log::debug!("Key package has last resort extension, not deleting"); + } let mut group = CoreGroup::new_from_welcome( welcome, @@ -174,8 +180,8 @@ impl MlsGroup { /// group info. For more information on the external init process, /// please see Section 11.2.1 in the MLS specification. /// - /// Note: If there is a group member in the group with the same identity as us, - /// this will create a remove proposal. + /// Note: If there is a group member in the group with the same identity as + /// us, this will create a remove proposal. pub fn join_by_external_commit( provider: &impl OpenMlsProvider, signer: &impl Signer, diff --git a/openmls/src/key_packages/mod.rs b/openmls/src/key_packages/mod.rs index 7f0a8733b8..3df6073074 100644 --- a/openmls/src/key_packages/mod.rs +++ b/openmls/src/key_packages/mod.rs @@ -376,6 +376,11 @@ impl KeyPackage { pub fn hpke_init_key(&self) -> &HpkePublicKey { &self.payload.init_key } + + /// Check if this KeyPackage is a last resort key package. + pub fn last_resort(&self) -> bool { + self.payload.extensions.contains(ExtensionType::LastResort) + } } /// Crate visible `KeyPackage` functions. From c86f4b0d3b146477954322de7aea00a52df0c95c Mon Sep 17 00:00:00 2001 From: Andrew Plaza Date: Mon, 13 Nov 2023 10:03:33 -0500 Subject: [PATCH 4/4] relax send + sync from OpenMlsProvider (#1451) --- traits/src/traits.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/traits/src/traits.rs b/traits/src/traits.rs index a16b19c957..6ad592b535 100644 --- a/traits/src/traits.rs +++ b/traits/src/traits.rs @@ -13,7 +13,7 @@ pub mod types; /// /// An implementation of this trait must be passed in to the public OpenMLS API /// to perform randomness generation, cryptographic operations, and key storage. -pub trait OpenMlsProvider: Send + Sync { +pub trait OpenMlsProvider { type CryptoProvider: crypto::OpenMlsCrypto; type RandProvider: random::OpenMlsRand; type KeyStoreProvider: key_store::OpenMlsKeyStore;