Faster joins: filter out non local events when a room doesn't have its full state

[MatMaul]

Part of #12997.

This is an alternate to #14057, where we filter out non local events of various endpoints instead of explicitly calculating a safe chain of events. The filtered out events will either be omitted or redacted depending of the endpoint.

We then rely on logic where the other server should try out other participating servers if it received a redacted event. This logic may not yet be available/correct, TBC.

Endpoints covered by filter_events_for_server:

• get_missing_events
• backfill
• catching up federation

Normal streaming federation is already only sending out local events so this should be fine already.

state and state_ids should already bail out for all events of a partially joined room with #13823, and we think that this is fine.

Pull Request Checklist

• Pull request is based on the develop branch
• Pull request includes a changelog file. The entry should:
  • Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from EventStore to EventWorkerStore.".
  • Use markdown where necessary, mostly for code blocks.
  • End with either a period (.) or an exclamation mark (!).
  • Start with a capital letter.
  • Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry.
• Pull request includes a sign off
• Code style is correct
  (run the linters)

[MatMaul]

I am requesting a review mainly to check that it looks globally sane and that my assumptions in the top comment are fine.

I'll take care of having a look at the failing tests next week.

[squahtx]

Right now won't assert_host_in_room reject most remote homeservers in partial state rooms?

To resolve #14059, if we send an event out to a remote homeserver, we must be prepared to answer some of these queries that come back. Remote homeservers may either be in the servers_in_room list, or have a membership in the room that we know about. The latter case looks fine, but the former will be rejected I believe. There'll be some extra changes we'll have to make to handle those homeservers.

[squahtx]

Putting this back in the queue in case anyone familiar with faster room joins has extra opinions.

[reivilibre]

Right now won't assert_host_in_room reject most remote homeservers in partial state rooms?

Correct, indeed in the default case it won't allow any remotes to talk to you about partial-state rooms.

To resolve #14059
, if we send an event out to a remote homeserver, we must be prepared to answer some of these queries that come back. Remote homeservers may either be in the servers_in_room list, or have a membership in the room that we know about. The latter case looks fine, but the former will be rejected I believe. There'll be some extra
changes we'll have to make to handle those homeservers.

From memory, you can get away without answering most of them most of the time.

• You need /get_missing_events (at least partially) — you would have to divulge the prev_events of the PDU you're sending at the very least. This is something we need to spec but is otherwise existing behaviour, so it should be a spec clarification rather than fresh spec.
• Unsure about /backfill... I'm not sure it plays any part in what happens when a homeserver receives a PDU (worth checking though)
• /state and/or /state_ids are technically things you need to answer, but the whole point of a partial-state room is that we don't have the state, so we couldn't answer if we wanted to. However, making the simplifying assumption that the remotes we're sending our PDUs to are always up to date, then we can get away without answering these queries!
  • In discussion with Rich, we are happy to accept that, whilst you have a partial-state room, the idea of sending PDUs to remote homeservers who have not caught up is something we can accept is broken. In real cases, the partial-stateness of a room is likely to be shortlived and it's unlikely that a given remote destination falls out of date in that timespan.

[MatMaul]

Right now won't assert_host_in_room reject most remote homeservers in partial state rooms?

Indeed thanks, I've added allow_partial_state_rooms=True on get_missing_events and backfill since we would do the filtering in filter_events_for_server now.

I also fixed the race I believe, thanks for catching it.

[reivilibre]

As a note: to prevent divulging our events from before the most recent partial join, you could do the following:

• look at partial_state_rooms and get the event ID of the partial join.
• get the depth of the partial join.
• filter out events whose depth is less than the depth of the partial join.

That said, I don't think depth is bulletproof, so maybe this is just security theatre 🤔.

This is fine: events from a prior full-state portion of the room will be filtered so only the servers in the room at that time can see them, as far as I can tell.

[squahtx]

Looks good!

assert_host_in_room may be missing logic to check the servers_in_room list for partial state rooms.

[MatMaul]

First sorry for the merge before final validation, it's a mistake :/

I think we don't really need to do so ?

When allow_partial_state_rooms=True, assert_host_in_room should not raise for local events, since it checks membership and we have our own join event so it should be fine. It should raise for most non-local events, but they would be filtered later on anyway by filter_events_for_server.

[squahtx]

First sorry for the merge before final validation, it's a mistake :/

I think we don't really need to do so ?

When allow_partial_state_rooms=True, assert_host_in_room should not raise for local events, since it checks membership and we have our own join event so it should be fine. It should raise for most non-local events, but they would be filtered later on anyway by filter_events_for_server.

I'm very confused. assert_host_in_room looks like

synapse/synapse/handlers/event_auth.py

Lines 169 to 192 in 847e239

	async def assert_host_in_room(
	self, room_id: str, host: str, allow_partial_state_rooms: bool = False
	) -> None:
	"""
	Asserts that the host is in the room, or raises an AuthError.
	If the room is partial-stated, we raise an AuthError with the
	UNABLE_DUE_TO_PARTIAL_STATE error code, unless `allow_partial_state_rooms` is true.
	If allow_partial_state_rooms is True and the room is partial-stated,
	this function may return an incorrect result as we are not able to fully
	track server membership in a room without full state.
	"""
	if not allow_partial_state_rooms and await self._store.is_partial_state_room(
	room_id
	):
	raise AuthError(
	403,
	"Unable to authorise you right now; room is partial-stated here.",
	errcode=Codes.UNABLE_DUE_TO_PARTIAL_STATE,
	)
	if not await self.is_host_in_room(room_id, host):
	raise AuthError(403, "Host not in room.")

and we call it with host=the remote server. is_host_in_room is implemented using room memberships, which we don't have for most remote servers in the room.

[MatMaul]

Oh yeah I don't know where my brain went but I though we were checking the event origin and not the "origin" server as of the source of the backfill/get_missing_events request.

I'll send another PR to fix that.