Don't use the Curve25519 sender key to store room keys #1091
Conversation
Now that we're not scoping the room keys by the Curve25519 sender key we're opening the door of multiple devices trying to insert the same room key into our store. This patch changes our logic so we only store room keys from an m.room_key event if we don't have one already or if the new key is a better version of the one we already have. This mostly assumes that the first room key with a given session id is coming from the creator of the room key.
poljar
force-pushed
the
poljar/store-group-sessions-without-sender-key
branch
from
8133aef
to
c4a019d
Compare
Codecov ReportBase: 77.96% // Head: 77.91% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1091 +/- ##
==========================================
- Coverage 77.96% 77.91% -0.06%
==========================================
Files 109 109
Lines 14718 14696 -22
==========================================
- Hits 11475 11450 -25
- Misses 3243 3246 +3
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| // be a trippled of `(room_id, sender_key, session_id)` now it's a | ||
| // tuple of `(room_id, session_id)` | ||
| // | ||
| // Let's just drop the whole object store. |
There was a problem hiding this comment.
is that save? The session will just be recovered during via sync?
There was a problem hiding this comment.
Nope, we're just dropping things in the indexeddb store since it's unused as of yet. I guess I could have just made a breaking change instead.
| @@ -129,12 +129,12 @@ impl ClientBuilder { | |||
| /// This is a shorthand for | |||
| /// <code>.[store_config](Self::store_config)([matrix_sdk_sled]::[make_store_config](matrix_sdk_sled::make_store_config)(path, passphrase)?)</code>. | |||
| #[cfg(feature = "sled")] | |||
| pub fn sled_store( | |||
| pub async fn sled_store( | |||
There was a problem hiding this comment.
hmm... this is a bit odd of a pattern ... generally, in the builder-pattern we expect to just set things and can chain them and don't have them do stuff yet but only really become active on build().await? ...
This isn't strictly about this PR, it just became more obvious that this isn't really proper when I noticed the new .await on its setter...
This implements MSC 3700, which is nowadays part of the spec.