Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Faster room joins: Device list cache not flushed when it is discovered that we incorrectly thought we shared a room with a remote user for some period in the past #13887

Open
squahtx opened this issue Sep 23, 2022 · 2 comments
Labels
A-Device-List-Tracking Telling clients about other devices. Often related to E2EE. A-Federated-Join joins over federation generally suck O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.

Comments

@squahtx
Copy link
Contributor

squahtx commented Sep 23, 2022

See the skipped TestPartialStateJoin/Device_list_tracking/Device_list_tracking_for_user_incorrectly_believed_to_be_in_room_when_they_rejoin_before_the_partial_state_join_completes complement test.

That test case is roughly:

  1. A room starts with @charlie:remote, @derek:remote (moderator) and @fred:remote (admin)
  2. @fred:remote leaves the room.
  3. @alice:local partial state joins the room.
  4. @elsie:remote joins the room.
  5. @alice:local does a /keys/query request and the local homeserver caches @elsie:remote's device list.
  6. @fred:remote "bans" @derek:remote, citing their join event as auth. The local homeserver does not know that @fred:remote is no longer in the room and accepts the event.
  7. @derek:remote kicks @elsie:remote. The local homeserver incorrectly rejects the kick because it thinks @derek:remote has been banned. @elsie:remote stops notifying the local homeserver about device list updates.
  8. @elsie:remote updates their device list.
  9. @elsie:remote rejoins the room (or joins another shared room) and will notify the local homeserver about device list updates again.
  10. The partial state join completes and @elsie:remote is discovered to not have been in the room / shared a room the whole time.
  11. (bad) If @alice:local does a /keys/query request, a stale response is returned.
@squahtx squahtx added A-Federated-Join joins over federation generally suck A-Device-List-Tracking Telling clients about other devices. Often related to E2EE. S-Minor Blocks non-critical functionality, workarounds exist. O-Uncommon Most users are unlikely to come across this or unexpected workflow labels Sep 23, 2022
@squahtx
Copy link
Contributor Author

squahtx commented Sep 23, 2022

After discussions with @erikjohnston, we've decided to deprioritize this edge case, since there are other known bugs in device list tracking when servers disagree about the members in a room.

@H-Shay H-Shay added the T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues. label Sep 23, 2022
@richvdh
Copy link
Member

richvdh commented Oct 5, 2022

We think this is an edge-case, and no worse than existing bugs in device-list tracking (such as #13651), so not making it a priority for a fix.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A-Device-List-Tracking Telling clients about other devices. Often related to E2EE. A-Federated-Join joins over federation generally suck O-Uncommon Most users are unlikely to come across this or unexpected workflow S-Minor Blocks non-critical functionality, workarounds exist. T-Defect Bugs, crashes, hangs, security vulnerabilities, or other reported issues.
Projects
None yet
Development

No branches or pull requests

3 participants