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

Race condition when sharing secrets to a new session

Open
#2,667 2 comments 0 reactions 0 assignees View on GitHub
Dominant language
Rust
Stars
2.3k
Forks
500
Avg merge
1d 16h
Merged PRs (30d)
106

Description

I can reproduce this scenario consistently on my local dev setup.

Some how related to this panic detected on web https://github.com/vector-im/element-web/issues/26301

After a verification, the existing session will try to send the secrets to the new one:
- If there are no existing olm session with the new device, web will panic
- If there is an existing olm session then it's fine (olm session can simply be created by sending a message in a room).

This what happens in the racy scenario

There is no olm session to encrypt the secret, so it's added to the `wait_queue` (would be retried on next key claim)
```
INFO /Users/valere/.cargo/git/checkouts/matrix-rust-sdk-1f4927f82a3d27bb/bcbb7c6/crates/matrix-sdk-crypto/src/gossiping/machine.rs:295 Secret request is missing an Olm session, putting the request in the wait queue
secret_name = "m.cross_signing.master"
```

The key/claim is actually made right after (in the middle of the secret sharing process)

```
rageshake.ts:74 rust-crypto: successfully made HTTP request: POST /_matrix/client/v3/keys/claim
rageshake.ts:74 INFO /Users/valere/.cargo/git/checkouts/matrix-rust-sdk-1f4927f82a3d27bb/bcbb7c6/crates/matrix-sdk-crypto/src/session_manager/sessions.rs:418 Established new Olm sessions
```

Then after that we can see that the next secret is shared twice:

```
INFO /Users/valere/.cargo/git/checkouts/matrix-rust-sdk-1f4927f82a3d27bb/bcbb7c6/crates/matrix-sdk-crypto/src/gossiping/machine.rs:285 Sharing a secret with a device
user_id = "@backup_test:localhost:8480"
device_id = "OKYPXQIAZM"
secret_name = "m.megolm_backup.v1"
```
Then again just after
```
INFO /Users/valere/.cargo/git/checkouts/matrix-rust-sdk-1f4927f82a3d27bb/bcbb7c6/crates/matrix-sdk-crypto/src/gossiping/machine.rs:285 Sharing a secret with a device
user_id = "@backup_test:localhost:8480"
device_id = "OKYPXQIAZM"
secret_name = "m.megolm_backup.v1"
```

We have again the log for msk twice:
```
INFO /Users/valere/.cargo/git/checkouts/matrix-rust-sdk-1f4927f82a3d27bb/bcbb7c6/crates/matrix-sdk-crypto/src/gossiping/machine.rs:285 Sharing a secret with a device
user_id = "@backup_test:localhost:8480"
device_id = "OKYPXQIAZM"
secret_name = "m.cross_signing.master"

INFO /Users/valere/.cargo/git/checkouts/matrix-rust-sdk-1f4927f82a3d27bb/bcbb7c6/crates/matrix-sdk-crypto/src/gossiping/machine.rs:285 Sharing a secret with a device
user_id = "@backup_test:localhost:8480"
device_id = "OKYPXQIAZM"
secret_name = "m.cross_signing.master"
```

Contributor guide

Open the contributing guide

Research direction

Start by reading the logged paths in matrix-sdk-crypto/gossiping/machine.rs and matrix-sdk-crypto/session_manager/sessions.rs, focusing on the wait_queue and key-claim sequence. Reproduce the verification flow described in the issue and compare the logs around the new Olm session. Done means the race no longer causes a panic or duplicate secret sharing.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
cryptography
Issue type
Bug
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
35/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.