MSC1719: olm session unwedging #1719

uhoreg · 2018-11-14T03:02:31Z

dbkr · 2018-11-14T08:07:13Z

Yep, this looks like a good summary. Thanks!

dbkr · 2018-11-14T11:19:46Z

Actually the kicker here is that we don't know what session an incoming olm message was intended for if we can't decrypt it, so we can't mark that session as 'replaced'. Thinking about how else we could do it.

uhoreg · 2018-11-14T15:21:35Z

An example implementation of this is https://github.com/matrix-org/matrix-js-sdk/pull/780/files

turt2live · 2018-11-26T03:37:33Z

proposals/1719-olm_unwedging.md

+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


Can m.dummy only be sent in the context of unwedging? If not, how should the client handle it? By ignoring the event the client opens itself up to problems similar to https://github.com/vector-im/riot-web/issues/3261

Probably doesn't necessarily need to be in the context of unwedging, but I can't really think of any other reason a client would send a dummy message, and I think we could just define m.dummy as "Ignore this message". This is a to_device message, and won't be displayed in any way to the user, so I don't think it will introduce any problems in the timeline.

Oh, I thought this was a timeline event. Would be good to clarify that it is a to_device event (if it isn't already - I'm willing to believe I just missed that).

It's kind of hidden in the fact that the message is olm-encrypted. I guess that technically, it could be a timeline event, if you used olm for your room encryption rather than megolm, but ... let's just say that people won't do that. ;)

proposals/1719-olm_unwedging.md

uhoreg · 2019-05-22T16:54:46Z

@mscbot fcp merge

mscbot · 2019-05-22T16:54:54Z

Team member @uhoreg has proposed to merge this. The next step is review by the rest of the tagged people:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

anoadragon453 · 2019-05-23T09:00:32Z

proposals/1719-olm_unwedging.md

+## Proposal
+
+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


I'm guessing this is already working in practice - but sometimes I receive undecryptable messages that I can decrypt after a little why. Possibly this MSC is the reason it does eventually become decrypted? But I was always chalking it up to server lag. If the latter is the case, shouldn't we wait a little bit before attempting to start another session? Or does rate limiting make this all ok anyways?

With megolm, if you receive a message that you can't decrypt, then it could be that the key will come in later, and then you'll be able to decrypt it. With olm, if you receive a message that you can't decrypt, then there's no way to recover, and waiting for a while won't help.

anoadragon453 · 2019-05-23T09:01:09Z

proposals/1719-olm_unwedging.md

+send a dummy message, clients may send an event with type `m.dummy`, and with
+empty contents.
+
+In order to avoid creating too many extra sessions, a client should rate-limit


Can a client DoS another client by ignoring this rate limit? Does the server protect against this?

A client could drain another client's one-time keys, preventing others from starting sessions. There isn't really a good way of fixing that. Note that this MSC doesn't change what a client is able to do, so any DoS that one client can perform with this MSC could also be done without the MSC.

proposals/1719-olm_unwedging.md

Co-Authored-By: Andrew Morgan <[email protected]>

anoadragon453 · 2019-05-24T10:56:07Z

LGTM

@mscbot reviewed

erikjohnston · 2019-05-29T16:36:41Z

proposals/1719-olm_unwedging.md

+
+An attacker could use this to create a new session on a device that they are
+able to read. However, this would require the attacker to have compromised the
+device's keys.


I don't know much about the crypto here, but this feels like it could weaken security? If an attacker has compromised the device keys can they restart a session easily today? Do we want to warn the user this has happened?

Previously, clients would use the session with the smallest session ID, so an attacker could still create a new session to be used. They'd might just need to try a few times before they got a session ID that was small enough.

I should also note that Signal basically does the same thing as this proposal.

Signal's documentation for this is https://signal.org/docs/specifications/sesame/

Cool, I guess

"Other people are doing it it must be fine (TM)" :p

richvdh

generally lgtm. I've got a couple of questions.

proposals/1719-olm_unwedging.md

richvdh · 2019-05-29T18:16:05Z

proposals/1719-olm_unwedging.md

+and for sessions that have never received a message, it should use the creation
+time of the session.  The spec will be changed to read:
+
+> If a client has multiple sessions established with another device, it should


this sounds like it fixes element-hq/element-web#2330, and maybe element-hq/element-web#4011 ?

I think 4011 was an implementation issue that was fixed by matrix-org/matrix-js-sdk#857. But this does fix 2330 -- I had been waiting for more testing before closing it.

proposals/1719-olm_unwedging.md

Co-Authored-By: Richard van der Hoff <[email protected]>

mscbot · 2019-05-30T10:38:11Z

🔔 This is now entering its final comment period, as per the review above. 🔔

As per [MSC1719](#1719) No known alterations have been made to the proposal. Implementation proof: matrix-org/matrix-js-sdk#780

turt2live · 2019-05-30T22:49:10Z

Spec PR for when this leaves FCP: #2059 (assuming no last minute comments)

mscbot · 2019-06-04T14:12:06Z

The final comment period, with a disposition to merge, as per the review above, is now complete.

turt2live · 2019-06-04T17:15:04Z

merged 🎉

…ls#1719))

uhoreg added 2 commits November 13, 2018 22:00

initial work on documenting olm unwedging

b535226

rename to match MSC number

d0bfdc1

we don't actually know which session got stuck, so rate-limit by device

495df02

add note about re-requesting megolm keys

2b58052

uhoreg added e2e proposal A matrix spec change proposal T-Core labels Nov 14, 2018

uhoreg changed the title ~~MSC: olm session unwedging~~ MSC1719: olm session unwedging Nov 15, 2018

turt2live reviewed Nov 26, 2018

View reviewed changes

dbkr mentioned this pull request Nov 28, 2018

Restoring a phone (or browser tab) from backup will break Olm if the other device still exists. element-hq/element-web#3822

Closed

turt2live added the proposal-in-review label Dec 31, 2018

anoadragon453 removed the T-Core label Jan 4, 2019

poljar mentioned this pull request Feb 14, 2019

Support Olm session unwedging. matrix-nio/matrix-nio#5

Closed

uhoreg added 3 commits May 22, 2019 12:43

1 hour seems to be fine

7705006

add a security note

d39baba

clarify wording

dd74baa

mscbot added proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. disposition-merge labels May 22, 2019

anoadragon453 reviewed May 23, 2019

View reviewed changes

proposals/1719-olm_unwedging.md Outdated Show resolved Hide resolved

remove duplicate consideration

ac08c84

Co-Authored-By: Andrew Morgan <[email protected]>

erikjohnston reviewed May 29, 2019

View reviewed changes

richvdh reviewed May 29, 2019

View reviewed changes

uhoreg and others added 2 commits May 29, 2019 15:01

fix typo

ffb70a2

Co-Authored-By: Richard van der Hoff <[email protected]>

add some clarifications

6929579

mscbot added final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. and removed proposed-final-comment-period Currently awaiting signoff of a majority of team members in order to enter the final comment period. labels May 30, 2019

turt2live self-assigned this May 30, 2019

turt2live added a commit that referenced this pull request May 30, 2019

Add Olm unwedging

54f74cd

As per [MSC1719](#1719) No known alterations have been made to the proposal. Implementation proof: matrix-org/matrix-js-sdk#780

turt2live mentioned this pull request May 30, 2019

Add Olm unwedging #2059

Merged

turt2live added spec-pr-in-review A proposal which has been PR'd against the spec and is in review and removed proposal-in-review labels May 30, 2019

mscbot added the finished-final-comment-period label Jun 4, 2019

mscbot removed the final-comment-period This MSC has entered a final comment period in interest to approval, postpone, or delete in 5 days. label Jun 4, 2019

turt2live merged commit b92b147 into matrix-org:master Jun 4, 2019

turt2live added merged A proposal whose PR has merged into the spec! and removed finished-final-comment-period spec-pr-in-review A proposal which has been PR'd against the spec and is in review labels Jun 4, 2019

This was referenced Sep 24, 2019

Restart broken Olm sessions element-hq/riot-meta#242

Open

Restart broken Olm sessions element-hq/riot-android#2773

Open

Restart broken Olm sessions element-hq/element-ios#2129

Closed

BillCarsonFr mentioned this pull request Jan 8, 2020

[crypto] add /discardsession session command element-hq/element-android#819

Closed

manuroe mentioned this pull request Jan 15, 2020

Restart broken Olm sessions element-hq/element-android#847

Closed

manuroe mentioned this pull request Feb 7, 2020

MXCrypto: Restart broken Olm sessions matrix-org/matrix-ios-sdk#778

Merged

poljar mentioned this pull request Feb 25, 2020

Risk of logging ratchet_key in olm::Session::describe matrix-org/olm#26

Closed

bmarty added a commit to element-hq/element-android that referenced this pull request Apr 20, 2020

Restart broken Olm sessions ([MSC1719](matrix-org/matrix-spec-proposa…

d722540

…ls#1719))

bmarty added a commit to element-hq/element-android that referenced this pull request Apr 20, 2020

Restart broken Olm sessions ([MSC1719](matrix-org/matrix-spec-proposa…

a6368c4

…ls#1719))

bmarty mentioned this pull request Apr 20, 2020

Feature/unwedging element-hq/element-android#1261

Merged

turt2live added the kind:core MSC which is critical to the protocol's success label Apr 20, 2020

MSC1719: olm session unwedging #1719

MSC1719: olm session unwedging #1719

Uh oh!

Conversation

uhoreg commented Nov 14, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dbkr commented Nov 14, 2018

Uh oh!

dbkr commented Nov 14, 2018

Uh oh!

uhoreg commented Nov 14, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

uhoreg commented May 22, 2019

Uh oh!

mscbot commented May 22, 2019 • edited by richvdh Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

anoadragon453 commented May 24, 2019

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

richvdh left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mscbot commented May 30, 2019

Uh oh!

turt2live commented May 30, 2019

Uh oh!

mscbot commented Jun 4, 2019

Uh oh!

turt2live commented Jun 4, 2019

Uh oh!

Uh oh!

uhoreg commented Nov 14, 2018 •

edited

Loading

mscbot commented May 22, 2019 •

edited by richvdh

Loading