The Wayback Machine - https://web.archive.org/web/20200714071403/https://github.com/vector-im/riot-web/issues/13229
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

Secret requests on login appear to be unreliable #13229

Closed
dbkr opened this issue Apr 16, 2020 · 1 comment
Closed

Secret requests on login appear to be unreliable #13229

dbkr opened this issue Apr 16, 2020 · 1 comment

Comments

@dbkr
Copy link
Member

@dbkr dbkr commented Apr 16, 2020

Works on my local dev server but looks to be reliably reproducible on michael's server. When logging in by verifying, SSK gets transferred but USK & backup key fail to be decrypted with BAD_MESSAGE_KEY_ID

@uhoreg
Copy link
Member

@uhoreg uhoreg commented Apr 16, 2020

It looks like js-sdk is processing the encrypted to-device events in parallel. For each event, it performs the following steps:

  • _decryptMessage in crypto/algorithms/olm.js checks all existing sessions to see if it can decrypt the message (there are no existing sessions yet)
  • it then calls _olmDevice.createInboundSession to create a new olm session
  • createInboundSession then
    • fetches the account from storage,
    • creates a new olm session (session.create_inbound_from)
    • removes the one-time key that was used for the message (account.remove_one_time_keys)
    • re-stores the account to storage

The problem here is that once the first event causes the one-time key to be removed, the other events will fail to create the inbound session, because it doesn't have the key any more. So to fix this, we need to change _decryptMessage to block trying to decrypt the second and third messages until it's done decrypting the first message, so that when it tries to decrypt the second and third messages, it finds the newly-created session rather than trying to create a new session. This blocking probably only needs to be done per-sender (e.g. decrypting a message from alice shouldn't block decrypting from bob) since they'll be using different one-time keys.

@dbkr dbkr self-assigned this Apr 17, 2020
@dbkr dbkr added this to In Progress in Workflow Apr 17, 2020
dbkr added a commit to matrix-org/matrix-js-sdk that referenced this issue Apr 17, 2020
If they overlap, they can both try to create new sessions, but this
removes the OTK so it will only work once.

Fixes vector-im/riot-web#13229
Workflow automation moved this from In Progress to In Test Apr 17, 2020
dbkr added a commit to matrix-org/matrix-js-sdk that referenced this issue Apr 20, 2020
If they overlap, they can both try to create new sessions, but this
removes the OTK so it will only work once.

Fixes vector-im/riot-web#13229
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Workflow
In Test
Linked pull requests

Successfully merging a pull request may close this issue.

3 participants
You can’t perform that action at this time.