This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Ensure that we correctly auth events returned by send_join
#11012
Merged
+59
−84
Merged
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
d37c283
Hoist `_check_for_soft_fail` and `_maybe_kick_guest_users` checks
richvdh 44a0762
Stop papering over broken send_join responses
richvdh 3701fee
process_remote_join: use auth_and_persist_outliers
richvdh 50f58a0
Also auth the join event itself
richvdh 32e9237
changelog
richvdh 8c751f0
Drop events with missing auth events
richvdh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this mean that you will not be able to join a room via a server that has missing events in its auth chain? Is that a situation we're in right now with Matrix HQ on various servers? Or do we always have all the events, they've just been rejected?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
er, good question. I'm not entirely sure how to find out. (My private HS OOMs whenever I try to join HQ)
I'm pretty sure that if any servers do have missing auth events, they're going to be pretty broken by not having an auth chain cover.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
hrm, well, one problem it does introduce is that it makes old matrix-dev unjoinable, because there are events in there whose signatures cannot be verified. I think they should be dropped before we get here, so I'll dig a little more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sigh. Inevitably, I now can't repeat this. And I can't understand how it could have happened :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh, the problem was that I had a copy of one of the now-unverifiable events in my database, but not all of its auth events, apparently.
So this raises the question of: what should we do about events returned as part of a send_join whose auth_events are now unverifiable?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess the answer here is reasonably clear: we should do the same as we do when we fetch the state at a backwards extremity. Which is to say, we should simply exclude any bits of state whose auth events we cannot find. (Whether that is correct behaviour is highly debatable, but it's hard to do much about without substantial MSC1228-style redesign of the protocol).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, I agree
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, this is now done in 8c751f0.