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

Olm unwedging | Discrepency between what the spec says and rust-sdk implementation

Open
#3,356 0 comments 1 reaction 0 assignees View on GitHub
A-Encryption
Dominant language
Rust
Stars
2.3k
Forks
500
Avg merge
1d 16h
Merged PRs (30d)
106

Description

Reference: [Olm unwedging MSC](https://github.com/uhoreg/matrix-doc/blob/olm_unwedging/proposals/1719-olm_unwedging.md)

> When a device receives an olm-encrypted message that it cannot decrypt, it should assume that the olm session has become corrupted and create a new olm session to replace it. It should then send a dummy message, using that session, to the other party in order to inform them of the new session. To send a dummy message, clients may send an event with type m.dummy, and with empty contents.

As per spec:
> If a client has multiple sessions established with another device, it should use the session from which it last received and successfully decrypted a message. For these purposes, a session that has not received any messages should use its creation time as the time that it last received a message.

However the rust sdk is not using storing any timestamp for "last received and successfully decrypted" message.

There is a `last_used_time` timestamp, that is updated after any encryption or decryption:
https://github.com/matrix-org/matrix-rust-sdk/blob/dddc607e077511aa8c28a09856c0628d316c4dd4/crates/matrix-sdk-crypto/src/olm/session.rs#L82-L94

https://github.com/matrix-org/matrix-rust-sdk/blob/dddc607e077511aa8c28a09856c0628d316c4dd4/crates/matrix-sdk-crypto/src/olm/session.rs#L126-L132

But this `last_used_time` doesn't appear to be used anywhere.

And to decide what session to use when encrypting, the `rust-sdk` relies on the session `creation_time` to pick up the best one:
https://github.com/matrix-org/matrix-rust-sdk/blob/dddc607e077511aa8c28a09856c0628d316c4dd4/crates/matrix-sdk-crypto/src/identities/device.rs#L692

This issue is not to say that there could be an issue by using `creation_time` instead of `last successful decryption time` to pick up a session, but to note the difference with the spec.

Outcome could be to:
- Update the spec
- Make the rust-sdk use`last successful decryption time`

Contributor guide

Open the contributing guide

Research direction

Start by reading the Olm unwedging MSC, then inspect crates/matrix-sdk-crypto/src/olm/session.rs around the last_used_time updates and identities/device.rs around session selection. Compare the implementation with the spec's last-successful-decryption rule. Done means the project has resolved whether to update the SDK or the spec and aligned the relevant behavior accordingly.

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.