From 9c325a2bd4411d6f1bf1a9180876fd189da362b0 Mon Sep 17 00:00:00 2001 From: Valere Date: Tue, 25 Jun 2024 15:55:33 +0200 Subject: [PATCH] feat(sdk-crypto): Add Identity based room key sharing strategy --- .../src/identities/device.rs | 12 ++ .../group_sessions/share_strategy.rs | 128 +++++++++++++++++- 2 files changed, 139 insertions(+), 1 deletion(-) diff --git a/crates/matrix-sdk-crypto/src/identities/device.rs b/crates/matrix-sdk-crypto/src/identities/device.rs index 79cd50eea52..072c9b0ebdd 100644 --- a/crates/matrix-sdk-crypto/src/identities/device.rs +++ b/crates/matrix-sdk-crypto/src/identities/device.rs @@ -764,6 +764,18 @@ impl ReadOnlyDevice { ) } + pub(crate) fn is_cross_signed_by_owner( + &self, + device_owner_identity: &ReadOnlyUserIdentities, + ) -> bool { + match device_owner_identity { + ReadOnlyUserIdentities::Own(identity) => identity.is_device_signed(self).is_ok(), + ReadOnlyUserIdentities::Other(device_identity) => { + device_identity.is_device_signed(self).is_ok() + } + } + } + /// Encrypt the given content for this device. /// /// # Arguments diff --git a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs index 5e2e295d255..cf76badb34d 100644 --- a/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs +++ b/crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs @@ -42,7 +42,11 @@ pub enum CollectStrategy { /// trusted via interactive verification. /// - It is the current own device of the user. only_allow_trusted_devices: bool, - }, // XXX some new strategy to be defined later + }, + /// Share based on identity. Only distribute to devices signed by their + /// owner. If a user has no published identity he will not be receive + /// room keys. + IdentityBasedStrategy, } impl CollectStrategy { @@ -50,6 +54,11 @@ impl CollectStrategy { pub const fn new_device_based(only_allow_trusted_devices: bool) -> Self { CollectStrategy::DeviceBasedStrategy { only_allow_trusted_devices } } + + /// Creates an identity based strategy + pub const fn new_identity_based() -> Self { + CollectStrategy::IdentityBasedStrategy + } } impl Default for CollectStrategy { @@ -136,6 +145,13 @@ pub(crate) async fn collect_session_recipients( only_allow_trusted_devices, ) } + CollectStrategy::IdentityBasedStrategy => { + let device_owner_identity = store.get_user_identity(user_id).await?; + split_recipients_withhelds_for_user_based_on_identity( + user_devices, + &device_owner_identity, + ) + } }; let recipients = recipient_devices.allowed_devices; @@ -225,6 +241,41 @@ fn split_recipients_withhelds_for_user( RecipientDevices { allowed_devices: recipients, denied_devices_with_code: withheld_recipients } } +fn split_recipients_withhelds_for_user_based_on_identity( + user_devices: HashMap, + device_owner_identity: &Option, +) -> RecipientDevices { + let mut allowed_devices: Vec = Default::default(); + let mut denied_devices_with_code: Vec<(ReadOnlyDevice, WithheldCode)> = Default::default(); + + match device_owner_identity { + None => { + // withheld all the users devices, we need to have an identity for this + // distribution mode + denied_devices_with_code + .extend(user_devices.into_values().map(|d| (d, WithheldCode::Unauthorised))); + } + Some(device_owner_identity) => { + // Only accept devices signed by the current identity + let (recipients, withheld_recipients): ( + Vec, + Vec<(ReadOnlyDevice, WithheldCode)>, + ) = user_devices.into_values().partition_map(|d| { + if d.is_cross_signed_by_owner(device_owner_identity) { + Either::Left(d) + } else { + Either::Right((d, WithheldCode::Unauthorised)) + } + }); + + allowed_devices.extend(recipients); + denied_devices_with_code.extend(withheld_recipients); + } + } + + RecipientDevices { allowed_devices, denied_devices_with_code } +} + #[cfg(test)] mod tests { @@ -402,6 +453,81 @@ mod tests { assert_eq!(code.as_str(), WithheldCode::Unverified.as_str()); } + #[async_test] + async fn test_share_with_identity_strategy() { + let machine = set_up_test_machine().await; + + let fake_room_id = room_id!("!roomid:localhost"); + + let strategy = CollectStrategy::new_identity_based(); + + let encryption_settings = + EncryptionSettings { sharing_strategy: strategy.clone(), ..Default::default() }; + + let id_keys = machine.identity_keys(); + let group_session = OutboundGroupSession::new( + machine.device_id().into(), + Arc::new(id_keys), + fake_room_id, + encryption_settings.clone(), + ) + .unwrap(); + + let share_result = collect_session_recipients( + machine.store(), + vec![ + KeyDistributionTestData::dan_id(), + KeyDistributionTestData::dave_id(), + KeyDistributionTestData::good_id(), + ] + .into_iter(), + &encryption_settings, + &group_session, + ) + .await + .unwrap(); + + assert!(!share_result.should_rotate); + + let dave_devices_shared = share_result.devices.get(KeyDistributionTestData::dave_id()); + let good_devices_shared = share_result.devices.get(KeyDistributionTestData::good_id()); + // dave has no published identity so will not receive the key + assert!(dave_devices_shared.unwrap().is_empty()); + + // @good has properly signed his devices, he should get the keys + assert_eq!(good_devices_shared.unwrap().len(), 2); + + // dan has one of his devices self signed, so should get + // the key + let dan_devices_shared = + share_result.devices.get(KeyDistributionTestData::dan_id()).unwrap(); + + assert_eq!(dan_devices_shared.len(), 1); + let dan_device_that_will_get_the_key = &dan_devices_shared[0]; + assert_eq!( + dan_device_that_will_get_the_key.device_id().as_str(), + KeyDistributionTestData::dan_signed_device_id() + ); + + // Check withhelds for others + let (_, code) = share_result + .withheld_devices + .iter() + .find(|(d, _)| d.device_id() == KeyDistributionTestData::dan_unsigned_device_id()) + .expect("This dan's device should receive a withheld code"); + + assert_eq!(code.as_str(), WithheldCode::Unauthorised.as_str()); + + // Check withhelds for others + let (_, code) = share_result + .withheld_devices + .iter() + .find(|(d, _)| d.device_id() == KeyDistributionTestData::dave_device_id()) + .expect("This dave device should receive a withheld code"); + + assert_eq!(code.as_str(), WithheldCode::Unauthorised.as_str()); + } + #[async_test] async fn test_should_rotate_based_on_visibility() { let machine = set_up_test_machine().await;