Skip to content

Commit a4da6ba

Browse files
authored
Exclude insecure devices on Olm encryption (#5457)
Fixes the encrypting part of #4147 Probably easiest to review commit-by-commit <!-- description of the changes in this PR --> - [x] Public API changes documented in changelogs (optional) <!-- Sign-off, if not part of the commits --> <!-- See CONTRIBUTING.md if you don't know what this is --> Signed-off-by:
1 parent 033c6bd commit a4da6ba

File tree

16 files changed

+1091
-47
lines changed

16 files changed

+1091
-47
lines changed

bindings/matrix-sdk-crypto-ffi/src/machine.rs

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,8 @@ use matrix_sdk_crypto::{
1818
olm::ExportedRoomKey,
1919
store::types::{BackupDecryptionKey, Changes},
2020
types::requests::ToDeviceRequest,
21-
DecryptionSettings, LocalTrust, OlmMachine as InnerMachine, UserIdentity as SdkUserIdentity,
21+
CollectStrategy, DecryptionSettings, LocalTrust, OlmMachine as InnerMachine,
22+
UserIdentity as SdkUserIdentity,
2223
};
2324
use ruma::{
2425
api::{
@@ -832,6 +833,7 @@ impl OlmMachine {
832833
device_id: String,
833834
event_type: String,
834835
content: String,
836+
share_strategy: CollectStrategy,
835837
) -> Result<Option<Request>, CryptoStoreError> {
836838
let user_id = parse_user_id(&user_id)?;
837839
let device_id = device_id.as_str().into();
@@ -840,8 +842,11 @@ impl OlmMachine {
840842
let device = self.runtime.block_on(self.inner.get_device(&user_id, device_id, None))?;
841843

842844
if let Some(device) = device {
843-
let encrypted_content =
844-
self.runtime.block_on(device.encrypt_event_raw(&event_type, &content))?;
845+
let encrypted_content = self.runtime.block_on(device.encrypt_event_raw(
846+
&event_type,
847+
&content,
848+
share_strategy,
849+
))?;
845850

846851
let request = ToDeviceRequest::new(
847852
user_id.as_ref(),

crates/matrix-sdk-crypto/CHANGELOG.md

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,8 @@ All notable changes to this project will be documented in this file.
66

77
## [Unreleased] - ReleaseDate
88

9-
## [0.13.0] - 2025-07-10
10-
119
### Features
1210

13-
- [**breaking**] Add a new `VerificationLevel::MismatchedSender` to indicate that the sender of an event appears to have been tampered with.
14-
([#5219](https://github.com/matrix-org/matrix-rust-sdk/pull/5219))
15-
1611
- [**breaking**]: When in "exclude insecure devices" mode, refuse to decrypt
1712
incoming to-device messages from unverified devices, except for some
1813
exceptions for certain event types. To support this, a new variant has been
@@ -25,6 +20,17 @@ All notable changes to this project will be documented in this file.
2520
Affected methods are `OlmMachine::receive_sync_changes`,
2621
`RehydratedDevice::receive_events`, and several internal methods.
2722
([#5319](https://github.com/matrix-org/matrix-rust-sdk/pull/5319)
23+
- [**breaking**] The `Device::encrypt_event_raw` and (experimental)
24+
`OlmMachine::encrypt_content_for_devices` have new `share_strategy` parameters
25+
to ensure that the recipients are sufficiently trusted.
26+
([#5457](https://github.com/matrix-org/matrix-rust-sdk/pull/5457/))
27+
28+
## [0.13.0] - 2025-07-10
29+
30+
### Features
31+
32+
- [**breaking**] Add a new `VerificationLevel::MismatchedSender` to indicate that the sender of an event appears to have been tampered with.
33+
([#5219](https://github.com/matrix-org/matrix-rust-sdk/pull/5219))
2834

2935
### Refactor
3036

crates/matrix-sdk-crypto/src/error.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,10 @@ pub enum OlmError {
7777
#[error("encryption failed due to an error collecting the recipient devices: {0}")]
7878
SessionRecipientCollectionError(SessionRecipientCollectionError),
7979

80+
/// Encrypted content is withheld from this device
81+
#[error("encryption content is withheld from this: {0}")]
82+
Withheld(WithheldCode),
83+
8084
/// Refused to decrypt because the sender was not verified or did not meet
8185
/// the required VerificationLevel.
8286
#[error(

crates/matrix-sdk-crypto/src/identities/device.rs

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ use crate::{
4343
olm::{
4444
InboundGroupSession, OutboundGroupSession, Session, ShareInfo, SignedJsonObject, VerifyJson,
4545
},
46+
session_manager::{withheld_code_for_device_for_share_strategy, CollectStrategy},
4647
store::{
4748
caches::SequenceNumber,
4849
types::{Changes, DeviceChanges},
@@ -462,6 +463,8 @@ impl Device {
462463
/// * `event_type` - The type of the event to be sent.
463464
/// * `content` - The content of the event to be sent. This should be a type
464465
/// that implements the `Serialize` trait.
466+
/// * `share_strategy` - The share strategy to use to determine whether we
467+
/// should encrypt to the device.
465468
///
466469
/// # Returns
467470
///
@@ -472,7 +475,19 @@ impl Device {
472475
&self,
473476
event_type: &str,
474477
content: &Value,
478+
share_strategy: CollectStrategy,
475479
) -> OlmResult<Raw<ToDeviceEncryptedEventContent>> {
480+
if let Some(withheld_code) = withheld_code_for_device_for_share_strategy(
481+
&self.inner,
482+
share_strategy,
483+
&self.own_identity,
484+
&self.device_owner_identity,
485+
)
486+
.await?
487+
{
488+
return Err(OlmError::Withheld(withheld_code));
489+
}
490+
476491
let (used_session, raw_encrypted) = self.encrypt(event_type, content).await?;
477492

478493
// Persist the used session

crates/matrix-sdk-crypto/src/machine/mod.rs

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,8 @@ use vodozemac::{
6363
Curve25519PublicKey, Ed25519Signature,
6464
};
6565

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

1168+
let (allowed_devices, mut blocked_devices) =
1169+
split_devices_for_share_strategy(&self.inner.store, devices, share_strategy).await?;
1170+
11671171
let result = self
11681172
.inner
11691173
.group_session_manager
1170-
.encrypt_content_for_devices(devices, event_type, content.clone(), &mut changes)
1174+
.encrypt_content_for_devices(allowed_devices, event_type, content.clone(), &mut changes)
11711175
.await;
11721176

11731177
// Persist any changes we might have collected.
@@ -1182,7 +1186,10 @@ impl OlmMachine {
11821186
);
11831187
}
11841188

1185-
result
1189+
result.map(|(to_device_requests, mut withheld)| {
1190+
withheld.append(&mut blocked_devices);
1191+
(to_device_requests, withheld)
1192+
})
11861193
}
11871194
/// Collect the devices belonging to the given user, and send the details of
11881195
/// a room key bundle to those devices.

crates/matrix-sdk-crypto/src/machine/test_helpers.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ use tokio::sync::Mutex;
3939
use crate::{
4040
machine::tests,
4141
olm::PrivateCrossSigningIdentity,
42+
session_manager::CollectStrategy,
4243
store::{types::Changes, CryptoStoreWrapper, MemoryStore},
4344
types::{
4445
events::ToDeviceEvent,
@@ -194,7 +195,7 @@ pub async fn send_and_receive_encrypted_to_device_test_helper(
194195
sender.get_device(recipient.user_id(), recipient.device_id(), None).await.unwrap().unwrap();
195196

196197
let raw_encrypted = device
197-
.encrypt_event_raw(event_type, content)
198+
.encrypt_event_raw(event_type, content, CollectStrategy::AllDevices)
198199
.await
199200
.expect("Should have encrypted the content");
200201

crates/matrix-sdk-crypto/src/machine/tests/send_encrypted_to_device.rs

Lines changed: 50 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ use crate::{
3434
tests::{self, decryption_verification_state::mark_alice_identity_as_verified_test_helper},
3535
},
3636
olm::SenderData,
37+
session_manager::CollectStrategy,
3738
types::{
3839
events::{
3940
room::encrypted::ToDeviceEncryptedEventContent, EventType as _, ToDeviceCustomEvent,
@@ -443,7 +444,7 @@ async fn test_processed_to_device_variants() {
443444

444445
let device = alice.get_device(bob.user_id(), bob.device_id(), None).await.unwrap().unwrap();
445446
let raw_encrypted = device
446-
.encrypt_event_raw(custom_event_type, &custom_content)
447+
.encrypt_event_raw(custom_event_type, &custom_content, CollectStrategy::AllDevices)
447448
.await
448449
.expect("Should have encryted the content");
449450

@@ -600,7 +601,7 @@ async fn test_send_encrypted_to_device_no_session() {
600601
.await
601602
.unwrap()
602603
.unwrap()
603-
.encrypt_event_raw(custom_event_type, &custom_content)
604+
.encrypt_event_raw(custom_event_type, &custom_content, CollectStrategy::AllDevices)
604605
.await;
605606

606607
assert_matches!(encryption_result, Err(OlmError::MissingSession));
@@ -700,3 +701,50 @@ async fn make_alice_unverified(alice: &OlmMachine, bob: &OlmMachine) {
700701
alice.receive_keys_query_response(&TransactionId::new(), &kq_response).await.unwrap();
701702
bob.receive_keys_query_response(&TransactionId::new(), &kq_response).await.unwrap();
702703
}
704+
705+
#[async_test]
706+
/// Test that when we get an error when we try to encrypt to a device that
707+
/// doesn't satisfy the share strategy.
708+
async fn test_share_strategy_prevents_encryption() {
709+
use matrix_sdk_common::deserialized_responses::WithheldCode;
710+
use matrix_sdk_test::test_json::keys_query_sets::KeyDistributionTestData as DataSet;
711+
use ruma::TransactionId;
712+
713+
use crate::CrossSigningKeyExport;
714+
715+
// Create the local user (`@me`), and import the public identity keys
716+
let machine = OlmMachine::new(DataSet::me_id(), DataSet::me_device_id()).await;
717+
let keys_query = DataSet::me_keys_query_response();
718+
machine.mark_request_as_sent(&TransactionId::new(), &keys_query).await.unwrap();
719+
720+
// Also import the private cross signing keys
721+
machine
722+
.import_cross_signing_keys(CrossSigningKeyExport {
723+
master_key: DataSet::MASTER_KEY_PRIVATE_EXPORT.to_owned().into(),
724+
self_signing_key: DataSet::SELF_SIGNING_KEY_PRIVATE_EXPORT.to_owned().into(),
725+
user_signing_key: DataSet::USER_SIGNING_KEY_PRIVATE_EXPORT.to_owned().into(),
726+
})
727+
.await
728+
.unwrap();
729+
730+
let keys_query = DataSet::dan_keys_query_response();
731+
let txn_id = TransactionId::new();
732+
machine.mark_request_as_sent(&txn_id, &keys_query).await.unwrap();
733+
734+
let custom_event_type = "m.new_device";
735+
736+
let custom_content = json!({
737+
"device_id": "XYZABCDE",
738+
"rooms": ["!726s6s6q:example.com"]
739+
});
740+
741+
let encryption_result = machine
742+
.get_device(DataSet::dan_id(), DataSet::dan_unsigned_device_id(), None)
743+
.await
744+
.unwrap()
745+
.unwrap()
746+
.encrypt_event_raw(custom_event_type, &custom_content, CollectStrategy::OnlyTrustedDevices)
747+
.await;
748+
749+
assert_matches!(encryption_result, Err(OlmError::Withheld(WithheldCode::Unverified)));
750+
}

crates/matrix-sdk-crypto/src/session_manager/group_sessions/mod.rs

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,12 @@ use ruma::{
3535
UserId,
3636
};
3737
use serde::Serialize;
38-
pub(crate) use share_strategy::CollectRecipientsResult;
38+
#[cfg(feature = "experimental-send-custom-to-device")]
39+
pub(crate) use share_strategy::split_devices_for_share_strategy;
3940
pub use share_strategy::CollectStrategy;
41+
pub(crate) use share_strategy::{
42+
withheld_code_for_device_for_share_strategy, CollectRecipientsResult,
43+
};
4044
use tracing::{debug, error, info, instrument, trace, warn, Instrument};
4145

4246
use crate::{

0 commit comments

Comments
 (0)