From f2677cedfc06b38562918217bae3058364d68f02 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 8 Sep 2021 17:41:27 +0100 Subject: [PATCH 1/3] Reload auth events from db after fetching and persisting In `_update_auth_events_and_context_for_auth`, when we fetch the remote auth tree and persist the returned events: load the missing events from the database rather than using the copies we got from the remote server. This is mostly in preparation for additional refactors, but does have an advantage in that if we later get around to checking the rejected status, we'll be able to make use of it. --- synapse/handlers/federation_event.py | 17 +++++++++++------ 1 file changed, 11 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 10b3fdc22256..0433162101ed 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1550,16 +1550,21 @@ async def _update_auth_events_and_context_for_auth( await self.persist_events_and_notify( event.room_id, [(auth_event, missing_auth_event_context)] ) - - if auth_event.event_id in event_auth_events: - auth_events[ - (auth_event.type, auth_event.state_key) - ] = auth_event except AuthError: pass - except Exception: logger.exception("Failed to get auth chain") + else: + # load any auth events we might have persisted from the database. This + # has the side-effect of correctly setting the rejected_reason on them. + auth_events.update( + { + (ae.type, ae.state_key): ae + for ae in await self._store.get_events_as_list( + missing_auth, allow_rejected=True + ) + } + ) if event.internal_metadata.is_outlier(): # XXX: given that, for an outlier, we'll be working with the From 0f41b04f7f268c4f5d3857281557161b3db381d3 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 8 Sep 2021 15:17:34 +0100 Subject: [PATCH 2/3] Factor out `_get_remote_auth_chain_for_event` from `_update_auth_events_and_context_for_auth` --- synapse/handlers/federation_event.py | 107 +++++++++++++++------------ 1 file changed, 61 insertions(+), 46 deletions(-) diff --git a/synapse/handlers/federation_event.py b/synapse/handlers/federation_event.py index 0433162101ed..7d468bd2df12 100644 --- a/synapse/handlers/federation_event.py +++ b/synapse/handlers/federation_event.py @@ -1505,53 +1505,9 @@ async def _update_auth_events_and_context_for_auth( # If we don't have all the auth events, we need to get them. logger.info("auth_events contains unknown events: %s", missing_auth) try: - try: - remote_auth_chain = await self._federation_client.get_event_auth( - origin, event.room_id, event.event_id - ) - except RequestSendFailed as e1: - # The other side isn't around or doesn't implement the - # endpoint, so lets just bail out. - logger.info("Failed to get event auth from remote: %s", e1) - return context, auth_events - - seen_remotes = await self._store.have_seen_events( - event.room_id, [e.event_id for e in remote_auth_chain] + await self._get_remote_auth_chain_for_event( + origin, event.room_id, event.event_id ) - - for auth_event in remote_auth_chain: - if auth_event.event_id in seen_remotes: - continue - - if auth_event.event_id == event.event_id: - continue - - try: - auth_ids = auth_event.auth_event_ids() - auth = { - (e.type, e.state_key): e - for e in remote_auth_chain - if e.event_id in auth_ids or e.type == EventTypes.Create - } - auth_event.internal_metadata.outlier = True - - logger.debug( - "_check_event_auth %s missing_auth: %s", - event.event_id, - auth_event.event_id, - ) - missing_auth_event_context = EventContext.for_outlier() - missing_auth_event_context = await self._check_event_auth( - origin, - auth_event, - missing_auth_event_context, - claimed_auth_event_map=auth, - ) - await self.persist_events_and_notify( - event.room_id, [(auth_event, missing_auth_event_context)] - ) - except AuthError: - pass except Exception: logger.exception("Failed to get auth chain") else: @@ -1638,6 +1594,65 @@ async def _update_auth_events_and_context_for_auth( return context, auth_events + async def _get_remote_auth_chain_for_event( + self, destination: str, room_id: str, event_id: str + ) -> None: + """If we are missing some of an event's auth events, attempt to request them + + Args: + destination: where to fetch the auth tree from + room_id: the room in which we are lacking auth events + event_id: the event for which we are lacking auth events + """ + try: + remote_auth_chain = await self._federation_client.get_event_auth( + destination, room_id, event_id + ) + except RequestSendFailed as e1: + # The other side isn't around or doesn't implement the + # endpoint, so lets just bail out. + logger.info("Failed to get event auth from remote: %s", e1) + return + + seen_remotes = await self._store.have_seen_events( + room_id, [e.event_id for e in remote_auth_chain] + ) + + for auth_event in remote_auth_chain: + if auth_event.event_id in seen_remotes: + continue + + if auth_event.event_id == event_id: + continue + + try: + auth_ids = auth_event.auth_event_ids() + auth = { + (e.type, e.state_key): e + for e in remote_auth_chain + if e.event_id in auth_ids or e.type == EventTypes.Create + } + auth_event.internal_metadata.outlier = True + + logger.debug( + "_check_event_auth %s missing_auth: %s", + event_id, + auth_event.event_id, + ) + missing_auth_event_context = EventContext.for_outlier() + missing_auth_event_context = await self._check_event_auth( + destination, + auth_event, + missing_auth_event_context, + claimed_auth_event_map=auth, + ) + await self.persist_events_and_notify( + room_id, + [(auth_event, missing_auth_event_context)], + ) + except AuthError: + pass + async def _update_context_for_auth_events( self, event: EventBase, context: EventContext, auth_events: StateMap[EventBase] ) -> EventContext: From 3c4e3599251636e8a6dc54282b72c28c825ff2e6 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 22 Sep 2021 16:21:15 +0100 Subject: [PATCH 3/3] changelog --- changelog.d/10884.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10884.misc diff --git a/changelog.d/10884.misc b/changelog.d/10884.misc new file mode 100644 index 000000000000..9a765435dbe4 --- /dev/null +++ b/changelog.d/10884.misc @@ -0,0 +1 @@ +Clean up some of the federation event authentication code for clarity.