Skip to content

Commit

Permalink
feat(sdk-crypto): Add Identity based room key sharing strategy (#3607)
Browse files Browse the repository at this point in the history
This sharing strategy is defined as part of MSC4153[1].

[1]: matrix-org/matrix-spec-proposals#4153
  • Loading branch information
BillCarsonFr authored Jul 3, 2024
1 parent cdc3743 commit d8b2b74
Show file tree
Hide file tree
Showing 2 changed files with 149 additions and 13 deletions.
29 changes: 19 additions & 10 deletions crates/matrix-sdk-crypto/src/identities/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -279,16 +279,9 @@ impl Device {

/// Is this device cross signed by its owner?
pub fn is_cross_signed_by_owner(&self) -> bool {
self.device_owner_identity.as_ref().is_some_and(|device_identity| match device_identity {
// If it's one of our own devices, just check that
// we signed the device.
ReadOnlyUserIdentities::Own(identity) => identity.is_device_signed(&self.inner).is_ok(),
// If it's a device from someone else, check
// if the other user has signed this device.
ReadOnlyUserIdentities::Other(device_identity) => {
device_identity.is_device_signed(&self.inner).is_ok()
}
})
self.device_owner_identity
.as_ref()
.is_some_and(|owner_identity| self.inner.is_cross_signed_by_owner(owner_identity))
}

/// Is the device owner verified by us?
Expand Down Expand Up @@ -771,6 +764,22 @@ impl ReadOnlyDevice {
)
}

pub(crate) fn is_cross_signed_by_owner(
&self,
device_owner_identity: &ReadOnlyUserIdentities,
) -> bool {
match device_owner_identity {
// If it's one of our own devices, just check that
// we signed the device.
ReadOnlyUserIdentities::Own(identity) => identity.is_device_signed(self).is_ok(),
// If it's a device from someone else, check
// if the other user has signed this device.
ReadOnlyUserIdentities::Other(device_identity) => {
device_identity.is_device_signed(self).is_ok()
}
}
}

/// Encrypt the given content for this device.
///
/// # Arguments
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,14 +42,23 @@ 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 receive
/// any room keys.
IdentityBasedStrategy,
}

impl CollectStrategy {
/// Creates a new legacy strategy, based on per device trust.
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 {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -225,6 +241,42 @@ 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<OwnedDeviceId, ReadOnlyDevice>,
device_owner_identity: &Option<ReadOnlyUserIdentities>,
) -> RecipientDevices {
match device_owner_identity {
None => {
// withheld all the users devices, we need to have an identity for this
// distribution mode
RecipientDevices {
allowed_devices: Vec::default(),
denied_devices_with_code: user_devices
.into_values()
.map(|d| (d, WithheldCode::Unauthorised))
.collect(),
}
}
Some(device_owner_identity) => {
// Only accept devices signed by the current identity
let (recipients, withheld_recipients): (
Vec<ReadOnlyDevice>,
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))
}
});
RecipientDevices {
allowed_devices: recipients,
denied_devices_with_code: withheld_recipients,
}
}
}
}

#[cfg(test)]
mod tests {

Expand Down Expand Up @@ -391,15 +443,90 @@ mod tests {
.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::Unverified.as_str());
assert_eq!(code, &WithheldCode::Unverified);

let (_, code) = share_result
.withheld_devices
.iter()
.find(|(d, _)| d.device_id() == KeyDistributionTestData::dave_device_id())
.expect("This daves's device should receive a withheld code");

assert_eq!(code.as_str(), WithheldCode::Unverified.as_str());
assert_eq!(code, &WithheldCode::Unverified);
}

#[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, &WithheldCode::Unauthorised);

// 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, &WithheldCode::Unauthorised);
}

#[async_test]
Expand Down

0 comments on commit d8b2b74

Please sign in to comment.