matrix-org / matrix-org/matrix-rust-sdk
inbound group session update can race with reset backup
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 2.3k
- Forks
- 500
- Avg merge
- 1d 16h
- Merged PRs (30d)
- 106
Description
In various places in the codebase, we carry out a read-modify-write operation on existing Inbound Group Sessions. Examples include:
* Marking sessions as backed up or not-backed-up
* Updating the senderdata in response to a /keys/query response (ref #3753)
* Updating the senderdata and the "latest ratchet" when we decrypt a message
Most of these operations should not happen concurrently, since they are all handled from within the application's "sync loop".
However, marking a session as not-backed-up (in particular, when backup is disabled or otherwise reset) can happen out-of-band. In this case, because there is no locking between the operations, it is possible for a write to be lost, leading to sessions not being backed up (potentially causing unable-to-decrypt errors on future devices).
(The converse is not true: marking a session as not-backed-up is treated as an atomic operation by the store implementations, so other updates cannot be lost by a reset-backup operation.)
This would be fixed by the proposed fix to https://github.com/element-hq/element-web/issues/26892, *provided we extend it to the sqlite implementation*.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by reviewing the proposed fix referenced from element-web issue #26892, then locate the SQLite store implementation for Inbound Group Sessions. Trace the read-modify-write paths for backup status and reset-backup operations. Done means the SQLite implementation applies the proposed locking or atomicity fix so concurrent updates cannot lose a backup-status write.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust, sqlite
- Domain
- databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Needs clarification
- Newbie friendliness
- 28/100