From ac028f3fd105609497aa64cf9c6874254dafc6a1 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 1 Jun 2022 18:18:07 +0100 Subject: [PATCH 01/11] Rename some variables --- synapse/handlers/federation_event.py | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index b7c54e642fc9..664c1a276fae 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1536,12 +1536,14 @@ async def _check_event_auth( StateFilter.from_types(event_types) ) - auth_events_ids = self._event_auth_handler.compute_auth_events( + calculated_auth_event_ids = self._event_auth_handler.compute_auth_events( event, prev_state_ids, for_verification=True ) - auth_events_x = await self._store.get_events(auth_events_ids) + calculated_auth_events = await self._store.get_events_as_list( + calculated_auth_event_ids + ) calculated_auth_event_map = { - (e.type, e.state_key): e for e in auth_events_x.values() + (e.type, e.state_key): e for e in calculated_auth_events } try: From e5403a59b555cc7d2ef577aeee000b72d2ac5902 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 1 Jun 2022 18:15:29 +0100 Subject: [PATCH 02/11] Rip out auth-event reconciliation code This code attempts to update our reconcile our state with the remote's auth events. The spec says nothing about this, and it doesn't really seem like the right thing to do. --- synapse/handlers/federation_event.py | 172 +-------------------------- synapse/state/__init__.py | 26 ---- 2 files changed, 1 insertion(+), 197 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 664c1a276fae..90bb88d1ee9b 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1542,37 +1542,9 @@ async def _check_event_auth( calculated_auth_events = await self._store.get_events_as_list( calculated_auth_event_ids ) - calculated_auth_event_map = { - (e.type, e.state_key): e for e in calculated_auth_events - } try: - updated_auth_events = await self._update_auth_events_for_auth( - event, - calculated_auth_event_map=calculated_auth_event_map, - ) - except Exception: - # We don't really mind if the above fails, so lets not fail - # processing if it does. However, it really shouldn't fail so - # let's still log as an exception since we'll still want to fix - # any bugs. - logger.exception( - "Failed to double check auth events for %s with remote. " - "Ignoring failure and continuing processing of event.", - event.event_id, - ) - updated_auth_events = None - - if updated_auth_events: - context = await self._update_context_for_auth_events( - event, context, updated_auth_events - ) - auth_events_for_auth = updated_auth_events - else: - auth_events_for_auth = calculated_auth_event_map - - try: - check_state_dependent_auth_rules(event, auth_events_for_auth.values()) + check_state_dependent_auth_rules(event, calculated_auth_events) except AuthError as e: logger.warning("Failed auth resolution for %r because %s", event, e) context.rejected = RejectedReason.AUTH_ERROR @@ -1688,93 +1660,6 @@ async def _check_for_soft_fail( soft_failed_event_counter.inc() event.internal_metadata.soft_failed = True - async def _update_auth_events_for_auth( - self, - event: EventBase, - calculated_auth_event_map: StateMap[EventBase], - ) -> Optional[StateMap[EventBase]]: - """Helper for _check_event_auth. See there for docs. - - Checks whether a given event has the expected auth events. If it - doesn't then we talk to the remote server to compare state to see if - we can come to a consensus (e.g. if one server missed some valid - state). - - This attempts to resolve any potential divergence of state between - servers, but is not essential and so failures should not block further - processing of the event. - - Args: - event: - - calculated_auth_event_map: - Our calculated auth_events based on the state of the room - at the event's position in the DAG. - - Returns: - updated auth event map, or None if no changes are needed. - - """ - assert not event.internal_metadata.outlier - - # check for events which are in the event's claimed auth_events, but not - # in our calculated event map. - event_auth_events = set(event.auth_event_ids()) - different_auth = event_auth_events.difference( - e.event_id for e in calculated_auth_event_map.values() - ) - - if not different_auth: - return None - - logger.info( - "auth_events refers to events which are not in our calculated auth " - "chain: %s", - different_auth, - ) - - # XXX: currently this checks for redactions but I'm not convinced that is - # necessary? - different_events = await self._store.get_events_as_list(different_auth) - - # double-check they're all in the same room - we should already have checked - # this but it doesn't hurt to check again. - for d in different_events: - assert ( - d.room_id == event.room_id - ), f"Event {event.event_id} refers to auth_event {d.event_id} which is in a different room" - - # now we state-resolve between our own idea of the auth events, and the remote's - # idea of them. - - local_state = calculated_auth_event_map.values() - remote_auth_events = dict(calculated_auth_event_map) - remote_auth_events.update({(d.type, d.state_key): d for d in different_events}) - remote_state = remote_auth_events.values() - - room_version = await self._store.get_room_version_id(event.room_id) - new_state = await self._state_handler.resolve_events( - room_version, (local_state, remote_state), event - ) - different_state = { - (d.type, d.state_key): d - for d in new_state.values() - if calculated_auth_event_map.get((d.type, d.state_key)) != d - } - if not different_state: - logger.info("State res returned no new state") - return None - - logger.info( - "After state res: updating auth_events with new state %s", - different_state.values(), - ) - - # take a copy of calculated_auth_event_map before we modify it. - auth_events = dict(calculated_auth_event_map) - auth_events.update(different_state) - return auth_events - async def _load_or_fetch_auth_events_for_event( self, destination: str, event: EventBase ) -> Collection[EventBase]: @@ -1872,61 +1757,6 @@ async def _get_remote_auth_chain_for_event( await self._auth_and_persist_outliers(room_id, remote_auth_events) - async def _update_context_for_auth_events( - self, event: EventBase, context: EventContext, auth_events: StateMap[EventBase] - ) -> EventContext: - """Update the state_ids in an event context after auth event resolution, - storing the changes as a new state group. - - Args: - event: The event we're handling the context for - - context: initial event context - - auth_events: Events to update in the event context. - - Returns: - new event context - """ - # exclude the state key of the new event from the current_state in the context. - if event.is_state(): - event_key: Optional[Tuple[str, str]] = (event.type, event.state_key) - else: - event_key = None - state_updates = { - k: a.event_id for k, a in auth_events.items() if k != event_key - } - - current_state_ids = await context.get_current_state_ids() - current_state_ids = dict(current_state_ids) # type: ignore - - current_state_ids.update(state_updates) - - prev_state_ids = await context.get_prev_state_ids() - prev_state_ids = dict(prev_state_ids) - - prev_state_ids.update({k: a.event_id for k, a in auth_events.items()}) - - # create a new state group as a delta from the existing one. - prev_group = context.state_group - state_group = await self._state_storage_controller.store_state_group( - event.event_id, - event.room_id, - prev_group=prev_group, - delta_ids=state_updates, - current_state_ids=current_state_ids, - ) - - return EventContext.with_state( - storage=self._storage_controllers, - state_group=state_group, - state_group_before_event=context.state_group_before_event, - state_delta_due_to_event=state_updates, - prev_group=prev_group, - delta_ids=state_updates, - partial_state=context.partial_state, - ) - async def _run_push_actions_and_persist_event( self, event: EventBase, context: EventContext, backfilled: bool = False ) -> None: diff --git a/synapse/state/__init__.py b/synapse/state/__init__.py index d5cbdb3eef55..487f8ba58b18 100644 --- a/synapse/state/__init__.py +++ b/synapse/state/__init__.py @@ -24,7 +24,6 @@ DefaultDict, Dict, FrozenSet, - Iterable, List, Mapping, Optional, @@ -398,31 +397,6 @@ async def resolve_state_groups_for_events( ) return result - async def resolve_events( - self, - room_version: str, - state_sets: Collection[Iterable[EventBase]], - event: EventBase, - ) -> StateMap[EventBase]: - logger.info( - "Resolving state for %s with %d groups", event.room_id, len(state_sets) - ) - state_set_ids = [ - {(ev.type, ev.state_key): ev.event_id for ev in st} for st in state_sets - ] - - state_map = {ev.event_id: ev for st in state_sets for ev in st} - - new_state = await self._state_resolution_handler.resolve_events_with_store( - event.room_id, - room_version, - state_set_ids, - event_map=state_map, - state_res_store=StateResolutionStore(self.store), - ) - - return {key: state_map[ev_id] for key, ev_id in new_state.items()} - @attr.s(slots=True, auto_attribs=True) class _StateResMetrics: From 5b5ce5eb8d858d2d1ca26c2f363fac0a0e6cc9fd Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 1 Jun 2022 18:19:37 +0100 Subject: [PATCH 03/11] Do an early return when the two sets of auth events are the same. --- synapse/handlers/federation_event.py | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 90bb88d1ee9b..48b41212eced 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -12,6 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. +import collections import itertools import logging from http import HTTPStatus @@ -1539,6 +1540,14 @@ async def _check_event_auth( calculated_auth_event_ids = self._event_auth_handler.compute_auth_events( event, prev_state_ids, for_verification=True ) + + # if those are the same, we're done here. + if collections.Counter(event.auth_event_ids()) == collections.Counter( + calculated_auth_event_ids + ): + return context + + # otherwise, re-run the auth checks based on what we calculated. calculated_auth_events = await self._store.get_events_as_list( calculated_auth_event_ids ) From 7f6d721631634ffc36008e67ed2790be2f51a347 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 1 Jun 2022 18:20:09 +0100 Subject: [PATCH 04/11] Log the differences when the auth events are not the same. --- synapse/handlers/federation_event.py | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 48b41212eced..c05d51535bb7 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1552,6 +1552,29 @@ async def _check_event_auth( calculated_auth_event_ids ) + # log the differences + + claimed_auth_event_map = {(e.type, e.state_key): e for e in claimed_auth_events} + calculated_auth_event_map = { + (e.type, e.state_key): e for e in calculated_auth_events + } + logger.info( + "event's auth_events are different to our calculated auth_events. " + "Claimed but not calculated: %s. Calculated but not claimed: %s", + [ + ev + for k, ev in claimed_auth_event_map.items() + if k not in calculated_auth_event_map + or calculated_auth_event_map[k].event_id != ev.event_id + ], + [ + ev + for k, ev in calculated_auth_event_map.items() + if k not in claimed_auth_event_map + or claimed_auth_event_map[k].event_id != ev.event_id + ], + ) + try: check_state_dependent_auth_rules(event, calculated_auth_events) except AuthError as e: From 2b80c6e4ec01a92bc847df2897c1c0129f1b0be9 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 1 Jun 2022 18:48:06 +0100 Subject: [PATCH 05/11] Improve logging when auth fails This makes it more consistent with the logging when we check against the claimed auth events. --- synapse/handlers/federation_event.py | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index c05d51535bb7..899cfa6ba5bc 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1578,7 +1578,11 @@ async def _check_event_auth( try: check_state_dependent_auth_rules(event, calculated_auth_events) except AuthError as e: - logger.warning("Failed auth resolution for %r because %s", event, e) + logger.warning( + "While checking auth of %r against room state before the event: %s", + event, + e, + ) context.rejected = RejectedReason.AUTH_ERROR return context From 77c6b2d692a2b4dfc8a1dcd876458ea5e28fd527 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 17 Jun 2022 16:10:44 +0100 Subject: [PATCH 06/11] Update some comments --- synapse/handlers/federation_event.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 899cfa6ba5bc..cdd86e78777b 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1521,6 +1521,9 @@ async def _check_event_auth( ) # ... and check that the event passes auth at those auth events. + # https://spec.matrix.org/v1.3/server-server-api/#checks-performed-on-receipt-of-a-pdu: + # 4. Passes authorization rules based on the event’s auth events, + # otherwise it is rejected. try: await check_state_independent_auth_rules(self._store, event) check_state_dependent_auth_rules(event, claimed_auth_events) @@ -1531,7 +1534,10 @@ async def _check_event_auth( context.rejected = RejectedReason.AUTH_ERROR return context - # now check auth against what we think the auth events *should* be. + # now check the auth rules pass against the room state before the event + # https://spec.matrix.org/v1.3/server-server-api/#checks-performed-on-receipt-of-a-pdu: + # 5. Passes authorization rules based on the state before the event, + # otherwise it is rejected. event_types = event_auth.auth_types_for_event(event.room_version, event) prev_state_ids = await context.get_prev_state_ids( StateFilter.from_types(event_types) From 58fb17da11933d755cb6cbb26d2cd388da167d12 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 1 Jun 2022 18:39:22 +0100 Subject: [PATCH 07/11] Stop returning `EventContext` from `_check_event_auth` We now always return the same object that was passed in, so this is redundant. --- synapse/handlers/federation_event.py | 28 ++++++--------------- tests/rest/client/test_third_party_rules.py | 11 ++------ tests/test_federation.py | 8 ++---- 3 files changed, 12 insertions(+), 35 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index cdd86e78777b..a42421023d7e 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -335,7 +335,7 @@ async def on_send_membership_event( event.internal_metadata.send_on_behalf_of = origin context = await self._state_handler.compute_event_context(event) - context = await self._check_event_auth(origin, event, context) + await self._check_event_auth(origin, event, context) if context.rejected: raise SynapseError( 403, f"{event.membership} event was rejected", Codes.FORBIDDEN @@ -471,7 +471,7 @@ async def process_remote_join( partial_state=partial_state, ) - context = await self._check_event_auth(origin, event, context) + await self._check_event_auth(origin, event, context) if context.rejected: raise SynapseError(400, "Join event was rejected") @@ -1098,11 +1098,7 @@ async def _process_received_pdu( event, state_ids_before_event=state_ids, ) - context = await self._check_event_auth( - origin, - event, - context, - ) + await self._check_event_auth(origin, event, context) except AuthError as e: # FIXME richvdh 2021/10/07 I don't think this is reachable. Let's log it # for now @@ -1478,11 +1474,8 @@ async def prep(event: EventBase) -> None: ) async def _check_event_auth( - self, - origin: str, - event: EventBase, - context: EventContext, - ) -> EventContext: + self, origin: str, event: EventBase, context: EventContext + ) -> None: """ Checks whether an event should be rejected (for failing auth checks). @@ -1492,9 +1485,6 @@ async def _check_event_auth( context: The event context. - Returns: - The updated context object. - Raises: AuthError if we were unable to find copies of the event's auth events. (Most other failures just cause us to set `context.rejected`.) @@ -1509,7 +1499,7 @@ async def _check_event_auth( logger.warning("While validating received event %r: %s", event, e) # TODO: use a different rejected reason here? context.rejected = RejectedReason.AUTH_ERROR - return context + return # next, check that we have all of the event's auth events. # @@ -1532,7 +1522,7 @@ async def _check_event_auth( "While checking auth of %r against auth_events: %s", event, e ) context.rejected = RejectedReason.AUTH_ERROR - return context + return # now check the auth rules pass against the room state before the event # https://spec.matrix.org/v1.3/server-server-api/#checks-performed-on-receipt-of-a-pdu: @@ -1551,7 +1541,7 @@ async def _check_event_auth( if collections.Counter(event.auth_event_ids()) == collections.Counter( calculated_auth_event_ids ): - return context + return # otherwise, re-run the auth checks based on what we calculated. calculated_auth_events = await self._store.get_events_as_list( @@ -1591,8 +1581,6 @@ async def _check_event_auth( ) context.rejected = RejectedReason.AUTH_ERROR - return context - async def _maybe_kick_guest_users(self, event: EventBase) -> None: if event.type != EventTypes.GuestAccess: return diff --git a/tests/rest/client/test_third_party_rules.py b/tests/rest/client/test_third_party_rules.py index 5eb0f243f747..9a48e9286fe4 100644 --- a/tests/rest/client/test_third_party_rules.py +++ b/tests/rest/client/test_third_party_rules.py @@ -21,7 +21,6 @@ from synapse.api.errors import SynapseError from synapse.api.room_versions import RoomVersion from synapse.events import EventBase -from synapse.events.snapshot import EventContext from synapse.events.third_party_rules import load_legacy_third_party_event_rules from synapse.rest import admin from synapse.rest.client import account, login, profile, room @@ -113,14 +112,8 @@ async def approve_all_signature_checking( # Have this homeserver skip event auth checks. This is necessary due to # event auth checks ensuring that events were signed by the sender's homeserver. - async def _check_event_auth( - origin: str, - event: EventBase, - context: EventContext, - *args: Any, - **kwargs: Any, - ) -> EventContext: - return context + async def _check_event_auth(origin: Any, event: Any, context: Any) -> None: + pass hs.get_federation_event_handler()._check_event_auth = _check_event_auth # type: ignore[assignment] diff --git a/tests/test_federation.py b/tests/test_federation.py index 0cbef70bfa57..779fad1f6398 100644 --- a/tests/test_federation.py +++ b/tests/test_federation.py @@ -81,12 +81,8 @@ def setUp(self): self.handler = self.homeserver.get_federation_handler() federation_event_handler = self.homeserver.get_federation_event_handler() - async def _check_event_auth( - origin, - event, - context, - ): - return context + async def _check_event_auth(origin, event, context): + pass federation_event_handler._check_event_auth = _check_event_auth self.client = self.homeserver.get_federation_client() From 9f5baecb6f1073ba24541ecdfcde55a293f14895 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 24 Jun 2022 13:54:08 +0100 Subject: [PATCH 08/11] Reconcile state when processing incoming events during faster joins In order to handle incoming events over federation during a faster join, we need to relax the auth rules. Specifically, we need to accept that we may not have the sender's membership in our view of the room state. However, it should be in the `auth_events`, and state-resolving the auth events against our view should give the right thing. --- synapse/handlers/federation_event.py | 33 +++++++++++++++++++++++----- 1 file changed, 28 insertions(+), 5 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index a42421023d7e..f573a099f24e 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1528,13 +1528,36 @@ async def _check_event_auth( # https://spec.matrix.org/v1.3/server-server-api/#checks-performed-on-receipt-of-a-pdu: # 5. Passes authorization rules based on the state before the event, # otherwise it is rejected. - event_types = event_auth.auth_types_for_event(event.room_version, event) - prev_state_ids = await context.get_prev_state_ids( - StateFilter.from_types(event_types) - ) + # + # ... however, if we only have partial state for the room, then there is a good + # chance that we'll be missing some of the state needed to auth the new event. + # So, we state-resolve the auth event that we are given against the state that + # we know about, which ensures things like bans are applied. + if context.partial_state: + room_version = await self._store.get_room_version_id(event.room_id) + + local_state_id_map = await context.get_prev_state_ids() + claimed_auth_events_id_map = { + (ev.type, ev.state_key): ev.event_id for ev in claimed_auth_events + } + + state_for_auth_id_map = ( + await self._state_resolution_handler.resolve_events_with_store( + event.room_id, + room_version, + [local_state_id_map, claimed_auth_events_id_map], + event_map=None, + state_res_store=StateResolutionStore(self._store), + ) + ) + else: + event_types = event_auth.auth_types_for_event(event.room_version, event) + state_for_auth_id_map = await context.get_prev_state_ids( + StateFilter.from_types(event_types) + ) calculated_auth_event_ids = self._event_auth_handler.compute_auth_events( - event, prev_state_ids, for_verification=True + event, state_for_auth_id_map, for_verification=True ) # if those are the same, we're done here. From 0a11a39c1e9f3fdb9ffe785dc8bba78dc6681966 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 30 Jun 2022 18:43:44 +0100 Subject: [PATCH 09/11] Revert "Fix backfilled events being rejected for no `state_groups` (#10439)" I'm unconvinced that this test is correct, and it is failing. This reverts commit db6e7f15eaee81be54b960d040102900f20e3f74. --- tests/handlers/test_federation.py | 140 +----------------------------- 1 file changed, 1 insertion(+), 139 deletions(-) diff --git a/tests/handlers/test_federation.py b/tests/handlers/test_federation.py index 9b9c11fab73d..a9de866aeed5 100644 --- a/tests/handlers/test_federation.py +++ b/tests/handlers/test_federation.py @@ -12,7 +12,7 @@ # See the License for the specific language governing permissions and # limitations under the License. import logging -from typing import List, cast +from typing import cast from unittest import TestCase from twisted.test.proto_helpers import MemoryReactor @@ -50,8 +50,6 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: hs = self.setup_test_homeserver(federation_http_client=None) self.handler = hs.get_federation_handler() self.store = hs.get_datastores().main - self.state_storage_controller = hs.get_storage_controllers().state - self._event_auth_handler = hs.get_event_auth_handler() return hs def test_exchange_revoked_invite(self) -> None: @@ -314,142 +312,6 @@ def test_backfill_with_many_backward_extremities(self) -> None: ) self.get_success(d) - def test_backfill_floating_outlier_membership_auth(self) -> None: - """ - As the local homeserver, check that we can properly process a federated - event from the OTHER_SERVER with auth_events that include a floating - membership event from the OTHER_SERVER. - - Regression test, see #10439. - """ - OTHER_SERVER = "otherserver" - OTHER_USER = "@otheruser:" + OTHER_SERVER - - # create the room - user_id = self.register_user("kermit", "test") - tok = self.login("kermit", "test") - room_id = self.helper.create_room_as( - room_creator=user_id, - is_public=True, - tok=tok, - extra_content={ - "preset": "public_chat", - }, - ) - room_version = self.get_success(self.store.get_room_version(room_id)) - - prev_event_ids = self.get_success(self.store.get_prev_events_for_room(room_id)) - ( - most_recent_prev_event_id, - most_recent_prev_event_depth, - ) = self.get_success(self.store.get_max_depth_of(prev_event_ids)) - # mapping from (type, state_key) -> state_event_id - assert most_recent_prev_event_id is not None - prev_state_map = self.get_success( - self.state_storage_controller.get_state_ids_for_event( - most_recent_prev_event_id - ) - ) - # List of state event ID's - prev_state_ids = list(prev_state_map.values()) - auth_event_ids = prev_state_ids - auth_events = list( - self.get_success(self.store.get_events(auth_event_ids)).values() - ) - - # build a floating outlier member state event - fake_prev_event_id = "$" + random_string(43) - member_event_dict = { - "type": EventTypes.Member, - "content": { - "membership": "join", - }, - "state_key": OTHER_USER, - "room_id": room_id, - "sender": OTHER_USER, - "depth": most_recent_prev_event_depth, - "prev_events": [fake_prev_event_id], - "origin_server_ts": self.clock.time_msec(), - "signatures": {OTHER_SERVER: {"ed25519:key_version": "SomeSignatureHere"}}, - } - builder = self.hs.get_event_builder_factory().for_room_version( - room_version, member_event_dict - ) - member_event = self.get_success( - builder.build( - prev_event_ids=member_event_dict["prev_events"], - auth_event_ids=self._event_auth_handler.compute_auth_events( - builder, - prev_state_map, - for_verification=False, - ), - depth=member_event_dict["depth"], - ) - ) - # Override the signature added from "test" homeserver that we created the event with - member_event.signatures = member_event_dict["signatures"] - - # Add the new member_event to the StateMap - updated_state_map = dict(prev_state_map) - updated_state_map[ - (member_event.type, member_event.state_key) - ] = member_event.event_id - auth_events.append(member_event) - - # build and send an event authed based on the member event - message_event_dict = { - "type": EventTypes.Message, - "content": {}, - "room_id": room_id, - "sender": OTHER_USER, - "depth": most_recent_prev_event_depth, - "prev_events": prev_event_ids.copy(), - "origin_server_ts": self.clock.time_msec(), - "signatures": {OTHER_SERVER: {"ed25519:key_version": "SomeSignatureHere"}}, - } - builder = self.hs.get_event_builder_factory().for_room_version( - room_version, message_event_dict - ) - message_event = self.get_success( - builder.build( - prev_event_ids=message_event_dict["prev_events"], - auth_event_ids=self._event_auth_handler.compute_auth_events( - builder, - updated_state_map, - for_verification=False, - ), - depth=message_event_dict["depth"], - ) - ) - # Override the signature added from "test" homeserver that we created the event with - message_event.signatures = message_event_dict["signatures"] - - # Stub the /event_auth response from the OTHER_SERVER - async def get_event_auth( - destination: str, room_id: str, event_id: str - ) -> List[EventBase]: - return [ - event_from_pdu_json(ae.get_pdu_json(), room_version=room_version) - for ae in auth_events - ] - - self.handler.federation_client.get_event_auth = get_event_auth # type: ignore[assignment] - - with LoggingContext("receive_pdu"): - # Fake the OTHER_SERVER federating the message event over to our local homeserver - d = run_in_background( - self.hs.get_federation_event_handler().on_receive_pdu, - OTHER_SERVER, - message_event, - ) - self.get_success(d) - - # Now try and get the events on our local homeserver - stored_event = self.get_success( - self.store.get_event(message_event.event_id, allow_none=True) - ) - self.assertTrue(stored_event is not None) - @unittest.override_config( {"rc_invites": {"per_user": {"per_second": 0.5, "burst_count": 3}}} ) From ea54ba7b535371ff8eb63618b548ab2abde8cd12 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 1 Jun 2022 18:29:38 +0100 Subject: [PATCH 10/11] changelog --- changelog.d/12943.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/12943.misc diff --git a/changelog.d/12943.misc b/changelog.d/12943.misc new file mode 100644 index 000000000000..f66bb3ec32fc --- /dev/null +++ b/changelog.d/12943.misc @@ -0,0 +1 @@ +Remove code which incorrectly attempted to reconcile state with remote servers when processing incoming events. From 0988441b4260d61a5601a19a3ec7539965211caa Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 13 Jul 2022 13:43:32 +0100 Subject: [PATCH 11/11] add clarifying comment --- synapse/handlers/federation_event.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 5c0a6be1fc4c..c0efb55ef887 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1530,8 +1530,10 @@ async def _check_event_auth( # # ... however, if we only have partial state for the room, then there is a good # chance that we'll be missing some of the state needed to auth the new event. - # So, we state-resolve the auth event that we are given against the state that - # we know about, which ensures things like bans are applied. + # So, we state-resolve the auth events that we are given against the state that + # we know about, which ensures things like bans are applied. (Note that we'll + # already have checked we have all the auth events, in + # _load_or_fetch_auth_events_for_event above) if context.partial_state: room_version = await self._store.get_room_version_id(event.room_id)