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

UTD event was not decrypted after keys were received #4189

Closed
Tracked by #245
richvdh opened this issue Oct 30, 2024 · 10 comments
Closed
Tracked by #245

UTD event was not decrypted after keys were received #4189

richvdh opened this issue Oct 30, 2024 · 10 comments
Assignees

Comments

@richvdh
Copy link
Member

richvdh commented Oct 30, 2024

The keys to a given event arrived a few seconds after the event, but there doesn't appear to have been any attempt to retry the decryption.

@richvdh
Copy link
Member Author

richvdh commented Oct 30, 2024

This is tracked on the EXA and EXI repos as element-hq/element-x-android#3747 and element-hq/element-x-ios#3449 respectively

@richvdh
Copy link
Member Author

richvdh commented Oct 30, 2024

Possibly related: #3768

@poljar
Copy link
Contributor

poljar commented Nov 14, 2024

I think #3768 is not related.

What I think is happening here is the following:

  1. The user has the timeline open for a given room and an event is received in this room.
  2. The notification client receives a push and receives the room key for this event.
  3. Because the notification client received the room key, the timeline which uses the main client will not be notified about a new room key being received. Thus no attempt at a re-decryption will happen.
  4. UTD in the timeline until you reload the timeline it or restart the app.

At least two (or now three) rageshakes were cases where the notification client received the room key.

@bnjbvr
Copy link
Member

bnjbvr commented Nov 18, 2024

Would it make sense to have a generalization of Backup::room_keys_for_room_stream that applied to all keys, not only those received from the backup source?

@poljar
Copy link
Contributor

poljar commented Nov 18, 2024

Would it make sense to have a generalization of Backup::room_keys_for_room_stream that applied to all keys, not only those received from the backup source?

We have that stream but it lives in the OlmMachine which gets destroyed regularly due to the cross-process logic and thus the stream also gets destroyed.

@bnjbvr
Copy link
Member

bnjbvr commented Nov 18, 2024

We could put it temporarily next to the crypto store cross-process lock, since it has to live in the Client for the same reason; maybe we could bundle this into a struct with all the fields that ought to be migrated back to the OlmMachine once the migration is over?

@poljar
Copy link
Contributor

poljar commented Nov 18, 2024

Well the stream works this way:

  1. Room keys get from various sources are prepared to be persisted into the crypto store
  2. We collect the info about the room keys
  3. We persist the room keys
  4. We fire out the previously collected info into a stream

We could move the stream out of the OlmMachine but that doesn't change the fact that some parts of the room keys are received in a different process which uses a different Client.

Of course the easy fix for EXA is to use the stream from the OlmMachine since the machine doesn't get destroyed there.

@stefanceriu
Copy link
Member

The iOS workaround has been implemented in element-hq/element-x-ios#3628

@richvdh
Copy link
Member Author

richvdh commented Dec 19, 2024

seems like we should close this, and maybe replace it with one that documents more clearly the situations in which the implemented workaround is insufficient?

@poljar
Copy link
Contributor

poljar commented Dec 19, 2024

Sure, leftover stuff is described in #4445.

@poljar poljar closed this as completed Dec 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants