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

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

Open
BillCarsonFr opened this issue Apr 26, 2024 · 0 comments

Comments

@BillCarsonFr
Copy link
Member

Reference: Olm unwedging MSC

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:

pub async fn decrypt(&mut self, message: &OlmMessage) -> Result<String, DecryptionError> {
let mut inner = self.inner.lock().await;
Span::current().record("session_id", inner.session_id());
let plaintext = inner.decrypt(message)?;
debug!(session=?inner, "Decrypted an Olm message");
let plaintext = String::from_utf8_lossy(&plaintext).to_string();
self.last_use_time = SecondsSinceUnixEpoch::now();
Ok(plaintext)
}

pub(crate) async fn encrypt_helper(&mut self, plaintext: &str) -> OlmMessage {
let mut session = self.inner.lock().await;
let message = session.encrypt(plaintext);
self.last_use_time = SecondsSinceUnixEpoch::now();
debug!(?session, "Successfully encrypted an event");
message
}

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:

sessions.sort_by_key(|s| s.creation_time);

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 uselast successful decryption time
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant