matrix-org / matrix-rust-sdk

Matrix Client-Server SDK for Rust
Apache License 2.0
1.19k stars 238 forks source link

Allow room keys for inbound group sessions to be updated when we receive a "better" copy #3703

Open BillCarsonFr opened 2 months ago

BillCarsonFr commented 2 months ago

When we receive a Megolm key (session) with a particular session ID from somewhere, we need to store it into the local store. The key is flagged with imported if it was not received directly from a m.room_key (file import, backup, ..)

Messages decrypted with an imported key should have a warning (authenticity cannot be guaranteed on this device a.k.a gray shield)

It is possible to receive the same key from different sources (and at different ratchet index) at different times. Some scenario where it creates unwanted shields:

Scenario 1: ratcheted key after root downloaded from backup

Alice create a new login, get access to key backup. The key s0 (owned by bob) is in backup at index 0. There are 3 messages encrypted with that index. If bob sends now a new message, the ratcheted key s0 will be distributed to Alice's new login via a m.room_key As per the current algorithm https://github.com/matrix-org/matrix-rust-sdk/blob/62137e5a3ef032a5f4c3ff77f98f5d0051bf55ad/crates/matrix-sdk-crypto/src/machine.rs#L839-L846

=> the ratcheted key will be discarded and we keep the better one, but it is still imported=true (the ratcheted key is imported=false)

Scenario 2: Better key downloaded from backup replaces a ratcheted key received as a m.room_key

https://github.com/matrix-org/matrix-rust-sdk/blob/62137e5a3ef032a5f4c3ff77f98f5d0051bf55ad/crates/matrix-sdk-crypto/src/store/mod.rs#L1702-L1704

The backup key will replace the existing non imported key

See https://github.com/element-hq/element-web/issues/26526

Solution

Implement proper megolm key updating. When receiving a m.room_key, If we have an existing better key that is mark as imported, we could keep the existing better key and update the imported flag (if the key connects properly)

See legacy implementation https://github.com/element-hq/element-android/blob/7073b1647c3897b5a30c4886db5975a26f16c6a1/matrix-sdk-android/src/kotlinCrypto/java/org/matrix/android/sdk/internal/crypto/MXOlmDevice.kt#L667

See full algorithm details here https://docs.google.com/document/d/1IJkeUn5rNehDapKY75xQ9rutoElVjQHeSdw8dQpb_O0/edit

BillCarsonFr commented 3 days ago

https://github.com/element-hq/element-x-ios-rageshakes/issues/2216