matrix-org / matrix-org/matrix-rust-sdk

inbound group session update can race with reset backup

Open
#3,943 1 comment 0 reactions 0 assignees View on GitHub

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

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.