-
Notifications
You must be signed in to change notification settings - Fork 269
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
crypto: withhold outgoing messages to unsigned dehydrated devices #4551
Conversation
001dcf6
to
cac1067
Compare
// We only need the user identity if `only_allow_trusted_devices` or | ||
// `error_on_verified_user_problem` is set. | ||
let device_owner_identity = | ||
if only_allow_trusted_devices || error_on_verified_user_problem { | ||
store.get_user_identity(user_id).await? | ||
} else { | ||
None | ||
}; | ||
let device_owner_identity = store.get_user_identity(user_id).await?; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We now also need the identity if the user has a dehydrated device. Trying to optimise away this call in the cases where we weren't going to need the identity was getting too complicated, so let's just do it every time.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #4551 +/- ##
==========================================
+ Coverage 85.39% 85.40% +0.01%
==========================================
Files 286 286
Lines 32258 32275 +17
==========================================
+ Hits 27546 27564 +18
+ Misses 4712 4711 -1 ☔ View full report in Codecov by Sentry. |
I need to update this in view of #4313 (comment) |
aa82b96
to
919f073
Compare
(Now done) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we refactor the meat of the logic a bit, I find the function to be quite a challenging read.
crates/matrix-sdk-crypto/src/session_manager/group_sessions/share_strategy.rs
Outdated
Show resolved
Hide resolved
ecab4b1
to
c38feeb
Compare
c38feeb
to
54eda15
Compare
Now rebased on #4586, which fixes the failing test. |
Split the existing `set_up_test_machine` into two parts, so we can set up the test OlmMachine without importing data for other users
54eda15
to
051f595
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for taking the time to refactor this so this becomes more manageable.
I took the time to re-read all of our strategies and I think the code matches to what we described in the docs.
Great work.
@@ -966,6 +966,11 @@ impl DeviceData { | |||
pub fn first_time_seen_ts(&self) -> MilliSecondsSinceUnixEpoch { | |||
self.first_time_seen_ts | |||
} | |||
|
|||
/// True if this device is an MSC3814 dehydrated device. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we mention an MSC it would be nice to link to it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yup, done.
Per #4313, we should not send outgoing messages to dehydrated devices that are not signed by the current pinned/verified identity.
051f595
to
13cc4bc
Compare
Per #4313, we should not send outgoing messages to dehydrated devices that are not signed by the user's identity
Fixes #4313