Skip to content

Exclude insecure devices on Olm encryption #5457

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions bindings/matrix-sdk-crypto-ffi/src/machine.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ use matrix_sdk_crypto::{
olm::ExportedRoomKey,
store::types::{BackupDecryptionKey, Changes},
types::requests::ToDeviceRequest,
DecryptionSettings, LocalTrust, OlmMachine as InnerMachine, UserIdentity as SdkUserIdentity,
CollectStrategy, DecryptionSettings, LocalTrust, OlmMachine as InnerMachine,
UserIdentity as SdkUserIdentity,
};
use ruma::{
api::{
Expand Down Expand Up @@ -832,6 +833,7 @@ impl OlmMachine {
device_id: String,
event_type: String,
content: String,
share_strategy: CollectStrategy,
) -> Result<Option<Request>, CryptoStoreError> {
let user_id = parse_user_id(&user_id)?;
let device_id = device_id.as_str().into();
Expand All @@ -840,8 +842,11 @@ impl OlmMachine {
let device = self.runtime.block_on(self.inner.get_device(&user_id, device_id, None))?;

if let Some(device) = device {
let encrypted_content =
self.runtime.block_on(device.encrypt_event_raw(&event_type, &content))?;
let encrypted_content = self.runtime.block_on(device.encrypt_event_raw(
&event_type,
&content,
share_strategy,
))?;

let request = ToDeviceRequest::new(
user_id.as_ref(),
Expand Down
16 changes: 11 additions & 5 deletions crates/matrix-sdk-crypto/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,8 @@ All notable changes to this project will be documented in this file.

## [Unreleased] - ReleaseDate

## [0.13.0] - 2025-07-10

### Features

- [**breaking**] Add a new `VerificationLevel::MismatchedSender` to indicate that the sender of an event appears to have been tampered with.
([#5219](https://github.com/matrix-org/matrix-rust-sdk/pull/5219))

- [**breaking**]: When in "exclude insecure devices" mode, refuse to decrypt
incoming to-device messages from unverified devices, except for some
exceptions for certain event types. To support this, a new variant has been
Expand All @@ -25,6 +20,17 @@ All notable changes to this project will be documented in this file.
Affected methods are `OlmMachine::receive_sync_changes`,
`RehydratedDevice::receive_events`, and several internal methods.
([#5319](https://github.com/matrix-org/matrix-rust-sdk/pull/5319)
- [**breaking**] The `Device::encrypt_event_raw` and (experimental)
`OlmMachine::encrypt_content_for_devices` have new `share_strategy` parameters
to ensure that the recipients are sufficiently trusted.
([#5457](https://github.com/matrix-org/matrix-rust-sdk/pull/5457/))

## [0.13.0] - 2025-07-10

### Features

- [**breaking**] Add a new `VerificationLevel::MismatchedSender` to indicate that the sender of an event appears to have been tampered with.
([#5219](https://github.com/matrix-org/matrix-rust-sdk/pull/5219))

### Refactor

Expand Down
4 changes: 4 additions & 0 deletions crates/matrix-sdk-crypto/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ pub enum OlmError {
#[error("encryption failed due to an error collecting the recipient devices: {0}")]
SessionRecipientCollectionError(SessionRecipientCollectionError),

/// Encrypted content is withheld from this device
#[error("encryption content is withheld from this: {0}")]
Withheld(WithheldCode),

/// Refused to decrypt because the sender was not verified or did not meet
/// the required VerificationLevel.
#[error(
Expand Down
15 changes: 15 additions & 0 deletions crates/matrix-sdk-crypto/src/identities/device.rs
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ use crate::{
olm::{
InboundGroupSession, OutboundGroupSession, Session, ShareInfo, SignedJsonObject, VerifyJson,
},
session_manager::{withheld_code_for_device_for_share_strategy, CollectStrategy},
store::{
caches::SequenceNumber,
types::{Changes, DeviceChanges},
Expand Down Expand Up @@ -462,6 +463,8 @@ impl Device {
/// * `event_type` - The type of the event to be sent.
/// * `content` - The content of the event to be sent. This should be a type
/// that implements the `Serialize` trait.
/// * `share_strategy` - The share strategy to use to determine whether we
/// should encrypt to the device.
///
/// # Returns
///
Expand All @@ -472,7 +475,19 @@ impl Device {
&self,
event_type: &str,
content: &Value,
share_strategy: CollectStrategy,
) -> OlmResult<Raw<ToDeviceEncryptedEventContent>> {
if let Some(withheld_code) = withheld_code_for_device_for_share_strategy(
&self.inner,
share_strategy,
&self.own_identity,
&self.device_owner_identity,
)
.await?
{
return Err(OlmError::Withheld(withheld_code));
}

let (used_session, raw_encrypted) = self.encrypt(event_type, content).await?;

// Persist the used session
Expand Down
15 changes: 11 additions & 4 deletions crates/matrix-sdk-crypto/src/machine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,8 @@ use vodozemac::{
Curve25519PublicKey, Ed25519Signature,
};

#[cfg(feature = "experimental-send-custom-to-device")]
use crate::session_manager::split_devices_for_share_strategy;
use crate::{
backups::{BackupMachine, MegolmV1BackupKey},
dehydrated_devices::{DehydratedDevices, DehydrationError},
Expand Down Expand Up @@ -1159,15 +1161,17 @@ impl OlmMachine {
devices: Vec<DeviceData>,
event_type: &str,
content: &Value,
share_strategy: CollectStrategy,
) -> OlmResult<(Vec<ToDeviceRequest>, Vec<(DeviceData, WithheldCode)>)> {
// TODO: Use a `CollectStrategy` arguments to filter our devices depending on
// safety settings (like not sending to insecure devices).
let mut changes = Changes::default();

let (allowed_devices, mut blocked_devices) =
split_devices_for_share_strategy(&self.inner.store, devices, share_strategy).await?;

let result = self
.inner
.group_session_manager
.encrypt_content_for_devices(devices, event_type, content.clone(), &mut changes)
.encrypt_content_for_devices(allowed_devices, event_type, content.clone(), &mut changes)
.await;

// Persist any changes we might have collected.
Expand All @@ -1182,7 +1186,10 @@ impl OlmMachine {
);
}

result
result.map(|(to_device_requests, mut withheld)| {
withheld.append(&mut blocked_devices);
(to_device_requests, withheld)
})
}
/// Collect the devices belonging to the given user, and send the details of
/// a room key bundle to those devices.
Expand Down
3 changes: 2 additions & 1 deletion crates/matrix-sdk-crypto/src/machine/test_helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use tokio::sync::Mutex;
use crate::{
machine::tests,
olm::PrivateCrossSigningIdentity,
session_manager::CollectStrategy,
store::{types::Changes, CryptoStoreWrapper, MemoryStore},
types::{
events::ToDeviceEvent,
Expand Down Expand Up @@ -194,7 +195,7 @@ pub async fn send_and_receive_encrypted_to_device_test_helper(
sender.get_device(recipient.user_id(), recipient.device_id(), None).await.unwrap().unwrap();

let raw_encrypted = device
.encrypt_event_raw(event_type, content)
.encrypt_event_raw(event_type, content, CollectStrategy::AllDevices)
.await
.expect("Should have encrypted the content");

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use crate::{
tests::{self, decryption_verification_state::mark_alice_identity_as_verified_test_helper},
},
olm::SenderData,
session_manager::CollectStrategy,
types::{
events::{
room::encrypted::ToDeviceEncryptedEventContent, EventType as _, ToDeviceCustomEvent,
Expand Down Expand Up @@ -443,7 +444,7 @@ async fn test_processed_to_device_variants() {

let device = alice.get_device(bob.user_id(), bob.device_id(), None).await.unwrap().unwrap();
let raw_encrypted = device
.encrypt_event_raw(custom_event_type, &custom_content)
.encrypt_event_raw(custom_event_type, &custom_content, CollectStrategy::AllDevices)
.await
.expect("Should have encryted the content");

Expand Down Expand Up @@ -600,7 +601,7 @@ async fn test_send_encrypted_to_device_no_session() {
.await
.unwrap()
.unwrap()
.encrypt_event_raw(custom_event_type, &custom_content)
.encrypt_event_raw(custom_event_type, &custom_content, CollectStrategy::AllDevices)
.await;

assert_matches!(encryption_result, Err(OlmError::MissingSession));
Expand Down Expand Up @@ -700,3 +701,50 @@ async fn make_alice_unverified(alice: &OlmMachine, bob: &OlmMachine) {
alice.receive_keys_query_response(&TransactionId::new(), &kq_response).await.unwrap();
bob.receive_keys_query_response(&TransactionId::new(), &kq_response).await.unwrap();
}

#[async_test]
/// Test that when we get an error when we try to encrypt to a device that
/// doesn't satisfy the share strategy.
async fn test_share_strategy_prevents_encryption() {
use matrix_sdk_common::deserialized_responses::WithheldCode;
use matrix_sdk_test::test_json::keys_query_sets::KeyDistributionTestData as DataSet;
use ruma::TransactionId;

use crate::CrossSigningKeyExport;

// Create the local user (`@me`), and import the public identity keys
let machine = OlmMachine::new(DataSet::me_id(), DataSet::me_device_id()).await;
let keys_query = DataSet::me_keys_query_response();
machine.mark_request_as_sent(&TransactionId::new(), &keys_query).await.unwrap();

// Also import the private cross signing keys
machine
.import_cross_signing_keys(CrossSigningKeyExport {
master_key: DataSet::MASTER_KEY_PRIVATE_EXPORT.to_owned().into(),
self_signing_key: DataSet::SELF_SIGNING_KEY_PRIVATE_EXPORT.to_owned().into(),
user_signing_key: DataSet::USER_SIGNING_KEY_PRIVATE_EXPORT.to_owned().into(),
})
.await
.unwrap();

let keys_query = DataSet::dan_keys_query_response();
let txn_id = TransactionId::new();
machine.mark_request_as_sent(&txn_id, &keys_query).await.unwrap();

let custom_event_type = "m.new_device";

let custom_content = json!({
"device_id": "XYZABCDE",
"rooms": ["!726s6s6q:example.com"]
});

let encryption_result = machine
.get_device(DataSet::dan_id(), DataSet::dan_unsigned_device_id(), None)
.await
.unwrap()
.unwrap()
.encrypt_event_raw(custom_event_type, &custom_content, CollectStrategy::OnlyTrustedDevices)
.await;

assert_matches!(encryption_result, Err(OlmError::Withheld(WithheldCode::Unverified)));
}
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,12 @@ use ruma::{
UserId,
};
use serde::Serialize;
pub(crate) use share_strategy::CollectRecipientsResult;
#[cfg(feature = "experimental-send-custom-to-device")]
pub(crate) use share_strategy::split_devices_for_share_strategy;
pub use share_strategy::CollectStrategy;
pub(crate) use share_strategy::{
withheld_code_for_device_for_share_strategy, CollectRecipientsResult,
};
use tracing::{debug, error, info, instrument, trace, warn, Instrument};

use crate::{
Expand Down
Loading
Loading