Skip to content

Refuse to decrypt to-device messages from unverified devices (when in exclude insecure mode) #5319

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

Open
wants to merge 18 commits into
base: main
Choose a base branch
from
Open
Changes from 1 commit
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
97ca9ee
refactor(crypto): Pass DecryptionSettings in to OlmMachine::decrypt_t…
andybalaam Jun 24, 2025
94b28a1
refactor(crypto): Extract a method to check for to-device events from…
andybalaam Jun 25, 2025
c3c6025
refactor(tests): Take a reference to content in send_and_receive_encr…
andybalaam Jul 1, 2025
e638677
refactor(tests): Re-use send_and_receive_encrypted_to_device_test_hel…
andybalaam Jul 1, 2025
92d064e
refactor(tests): Pass decryption_settings in to send_and_receive_encr…
andybalaam Jul 1, 2025
b899348
feat(crypto): Refuse to decrypt to-device messages from unverified de…
andybalaam Jul 1, 2025
4c1ef53
Apply suggestions from code review
andybalaam Jul 2, 2025
56a51a4
Handle unverified senders as UTD when receiving to-device events
andybalaam Jul 22, 2025
c932b6b
Support encryption being disabled
andybalaam Jul 22, 2025
e6748a0
Move unverified sender check into Account
andybalaam Jul 23, 2025
e5fac47
Return a bool from is_from_verified_device_or_allowed_type to clarify…
andybalaam Jul 23, 2025
3924042
Remove reference to OlmMachine::decrypt_to_device_event from changelog
andybalaam Jul 23, 2025
02ba63c
Use a match in sending_device_is_verified
andybalaam Jul 23, 2025
550fa11
Refactor into satisfies_sender_trust_requirement to try to make Megol…
andybalaam Jul 23, 2025
a71782c
Merge branch 'main' into andybalaam/filter-untrusted-to-device
andybalaam Jul 23, 2025
d862637
Satisfy clippy
andybalaam Jul 23, 2025
c163158
Format code
andybalaam Jul 23, 2025
b6de979
Typos
andybalaam Jul 23, 2025
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
45 changes: 25 additions & 20 deletions crates/matrix-sdk-crypto/src/machine/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -868,27 +868,13 @@ impl OlmMachine {
let mut decrypted =
transaction.account().await?.decrypt_to_device_event(&self.inner.store, event).await?;

let from_dehydrated_device =
self.to_device_event_is_from_dehydrated_device(&decrypted, &event.sender).await?;

// Check whether this event is from a dehydrated device - if so, return Ok(None)
// to skip it because we don't expect ever to receive an event from a
// dehydrated device.
if from_dehydrated_device {
// Device is dehydrated: ignore this event
warn!(
sender = ?event.sender,
session = ?decrypted.session,
"Received a to-device event from a dehydrated device. This is unexpected: ignoring event"
);
Err(DecryptToDeviceError::FromDehydratedDevice)
} else {
// Device is not dehydrated: handle it as normal e.g. create a Megolm session
self.handle_decrypted_to_device_event(transaction.cache(), &mut decrypted, changes)
.await?;
// Return early if the sending device is decrypted
self.check_to_device_event_is_not_from_dehydrated_device(&decrypted, &event.sender).await?;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's kinda tripping me up that we flattened out the branches here. A bit worried that we'll somehow miss this subtle ? that prevents us from accepting secrets from dehydrated devices.

That being said, I don't have an idea how to improve it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I don't love it, but I think splitting into a separate function is good.

I could make it an explicit if and return but as the method name says "check" and we have a comment saying "Return early" maybe it's good enough?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Attempted to make it clearer in e5fac47


Ok(decrypted)
}
// Device is not dehydrated: handle it as normal e.g. create a Megolm session
self.handle_decrypted_to_device_event(transaction.cache(), &mut decrypted, changes).await?;

Ok(decrypted)
}

#[instrument(
Expand Down Expand Up @@ -1507,6 +1493,25 @@ impl OlmMachine {
})
}

/// Return an error if the supplied to-device event was sent from a
/// dehydrated device.
async fn check_to_device_event_is_not_from_dehydrated_device(
&self,
decrypted: &OlmDecryptionInfo,
sender_user_id: &UserId,
) -> Result<(), DecryptToDeviceError> {
if self.to_device_event_is_from_dehydrated_device(decrypted, sender_user_id).await? {
warn!(
sender = ?sender_user_id,
session = ?decrypted.session,
"Received a to-device event from a dehydrated device. This is unexpected: ignoring event"
);
Err(DecryptToDeviceError::FromDehydratedDevice)
} else {
Ok(())
}
}

/// Decide whether a decrypted to-device event was sent from a dehydrated
/// device.
///
Expand Down