Skip to content
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

inbound group session update can race with reset backup #3943

Open
richvdh opened this issue Sep 4, 2024 · 1 comment
Open

inbound group session update can race with reset backup #3943

richvdh opened this issue Sep 4, 2024 · 1 comment

Comments

@richvdh
Copy link
Member

richvdh commented Sep 4, 2024

In various places in the codebase, we carry out a read-modify-write operation on existing Inbound Group Sessions. Examples include:

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 element-hq/element-web#26892, provided we extend it to the sqlite implementation.

@richvdh
Copy link
Member Author

richvdh commented Sep 4, 2024

most of these operations should not happen concurrently, since they are all handled from within the application's "sync loop".

In more detail: in theory, an application should not call OlmMachine::receive_sync_changes at the same time as OlmMachine::mark_request_as_sent.

  • decrypting a message only happens in receive_sync_changes
  • marking as backed up only happens when a backup request completes, via mark_request_as_sent
  • handling a keys query response is done in mark_request_as_sent

So the only operation in the list above that can usually race is marking a session as not-backed-up.

@richvdh richvdh changed the title inbound group session update is racy inbound group session update can race with reset backup Sep 4, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant