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.

Sign up for GitHub

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

Jump to bottom

Race in decrypting events causes UISI #5001

Closed
richvdh opened this issue Sep 7, 2017 · 3 comments · Fixed by matrix-org/matrix-js-sdk#544
Closed

Race in decrypting events causes UISI #5001

richvdh opened this issue Sep 7, 2017 · 3 comments · Fixed by matrix-org/matrix-js-sdk#544
Labels
A-E2EE P1 S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect

Comments

@richvdh
Copy link
Member

richvdh commented Sep 7, 2017 •
edited
Loading

  • we receive the megolm event and try to decrypt; this fails (asynchronously)
  • we receive the olm event and decrypt the megolm keys. We add the key to the store, and also look for any recorded failures for that megolm session (currently none)
  • we go back to processing the megolm event. We record the failure, but now it's too late.

One solution might be to serialise access to the list of failures.
@richvdh richvdh mentioned this issue Sep 7, 2017
Closed
25 tasks
@lampholder lampholder added 001 mistake label T-Defect S-Major Severely degrades major functionality or product features, with no satisfactory workaround A-E2EE P1 and removed 001 mistake label labels Sep 8, 2017
@lampholder
Copy link
Member

Again, happy to negotiate on the priority here - I've P1'ed but I don't know what % of encrypted traffic this is likely impacting.

@richvdh
Copy link
Member Author

richvdh commented Sep 8, 2017

It seems to affect matthew quite heavily. Agreed it should be P1 but it's unlikely to get much progress while I'm on holiday...

richvdh added a commit to matrix-org/matrix-js-sdk that referenced this issue Sep 21, 2017
@richvdh
Fix a race in decrypting megolm messages
60a0331 
This fixes a race wherein it was possible for us to fail to decrypt a message,
if the keys arrived immediately after our attempt to decrypt it. In that case,
a retry *should* have been scheduled, but was not.

Fixes element-hq/element-web#5001.
@richvdh richvdh mentioned this issue Sep 21, 2017
Merged
lukebarnard1 pushed a commit to matrix-org/matrix-js-sdk that referenced this issue Sep 21, 2017
@richvdh @lukebarnard1
Fix a race in decrypting megolm messages (#544)
868c20b 
* Fix a race in decrypting megolm messages

This fixes a race wherein it was possible for us to fail to decrypt a message,
if the keys arrived immediately after our attempt to decrypt it. In that case,
a retry *should* have been scheduled, but was not.

Fixes element-hq/element-web#5001.

* WORDS
@richvdh
Copy link
Member Author

richvdh commented Sep 21, 2017

Fixed by matrix-org/matrix-js-sdk#544, I hope

@richvdh richvdh closed this as completed Sep 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
A-E2EE P1 S-Major Severely degrades major functionality or product features, with no satisfactory workaround T-Defect
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix a race in decrypting megolm messages matrix-org/matrix-js-sdk
2 participants
@richvdh @lampholder