-
Notifications
You must be signed in to change notification settings - Fork 315
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
base: main
Are you sure you want to change the base?
Changes from 1 commit
97ca9ee
94b28a1
c3c6025
e638677
92d064e
b899348
4c1ef53
56a51a4
c932b6b
e6748a0
e5fac47
3924042
02ba63c
550fa11
a71782c
d862637
c163158
b6de979
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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?; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 being said, I don't have an idea how to improve it. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
poljar marked this conversation as resolved.
Show resolved
Hide resolved
|
||
self.handle_decrypted_to_device_event(transaction.cache(), &mut decrypted, changes).await?; | ||
|
||
Ok(decrypted) | ||
} | ||
|
||
#[instrument( | ||
|
@@ -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. | ||
/// | ||
|
Uh oh!
There was an error while loading. Please reload this page.