From 9417986f7799bdcd609db16c2c964af09d9fe9b5 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 8 Nov 2018 12:11:20 +0000 Subject: [PATCH 1/6] Drop PDUs of unknown rooms When we receive events over federation we will need to know the room version to be able to correctly handle them, e.g. once we start changing event formats. Currently, we attempt to handle events in unknown rooms. --- synapse/federation/federation_server.py | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index fa2cc550e2..38949fb49a 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -162,8 +162,30 @@ class FederationServer(FederationBase): p["age_ts"] = request_time - int(p["age"]) del p["age"] + # We try and pull out an event ID so that if latter checks fail we + # can log something sensible. We don't mandate an event ID here in + # case future event formats get rid of the key. + possible_event_id = p.get("event_id", "") + + # Now we get the room ID so that we can check that we know the + # version of the room. + room_id = p.get("room_id") + if not room_id: + logger.info( + "Ignoring PDU as does not have a room_id. Event ID: %s", + possible_event_id, + ) + continue + + try: + # In future we will actually use the room version to parse the + # PDU into an event. + yield self.store.get_room_version(room_id) + except NotFoundError: + logger.info("Ignoring PUD for unknown room_id: %s", room_id) + continue + event = event_from_pdu_json(p) - room_id = event.room_id pdus_by_room.setdefault(room_id, []).append(event) pdu_results = {} From 94896d7ffe775f48bc75994fb2caba85daac96de Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 8 Nov 2018 12:30:25 +0000 Subject: [PATCH 2/6] Newsfile --- changelog.d/4165.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4165.misc diff --git a/changelog.d/4165.misc b/changelog.d/4165.misc new file mode 100644 index 0000000000..415d80aeab --- /dev/null +++ b/changelog.d/4165.misc @@ -0,0 +1 @@ +Drop incoming events from federation for unknown rooms From 9bce065a53799eb6403480736630ca5d7fdfe19c Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 9 Nov 2018 11:28:22 +0000 Subject: [PATCH 3/6] Update synapse/federation/federation_server.py Co-Authored-By: erikjohnston --- synapse/federation/federation_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 38949fb49a..3bdc6024f8 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -162,7 +162,7 @@ class FederationServer(FederationBase): p["age_ts"] = request_time - int(p["age"]) del p["age"] - # We try and pull out an event ID so that if latter checks fail we + # We try and pull out an event ID so that if later checks fail we # can log something sensible. We don't mandate an event ID here in # case future event formats get rid of the key. possible_event_id = p.get("event_id", "") From 3cecf5340d4e2ad8944a79b0fd4213e242791cbe Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Fri, 9 Nov 2018 11:28:25 +0000 Subject: [PATCH 4/6] Update synapse/federation/federation_server.py Co-Authored-By: erikjohnston --- synapse/federation/federation_server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 3bdc6024f8..98722ae543 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -182,7 +182,7 @@ class FederationServer(FederationBase): # PDU into an event. yield self.store.get_room_version(room_id) except NotFoundError: - logger.info("Ignoring PUD for unknown room_id: %s", room_id) + logger.info("Ignoring PDU for unknown room_id: %s", room_id) continue event = event_from_pdu_json(p) From 30dd27afff3889293ec05b82c7f7e05d61c5b609 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 9 Nov 2018 11:34:45 +0000 Subject: [PATCH 5/6] Simplify to always drop events if server isn't in the room --- synapse/handlers/federation.py | 23 +++++++++-------------- 1 file changed, 9 insertions(+), 14 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 9ca5fd8724..e859ff5ffa 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -202,27 +202,22 @@ class FederationHandler(BaseHandler): self.room_queues[room_id].append((pdu, origin)) return - # If we're no longer in the room just ditch the event entirely. This - # is probably an old server that has come back and thinks we're still - # in the room (or we've been rejoined to the room by a state reset). + # If we're not in the room just ditch the event entirely. This is + # probably an old server that has come back and thinks we're still in + # the room (or we've been rejoined to the room by a state reset). # - # If we were never in the room then maybe our database got vaped and - # we should check if we *are* in fact in the room. If we are then we - # can magically rejoin the room. + # Note that if we were never in the room then we would have already + # dropped the event, since we wouldn't know the room version. is_in_room = yield self.auth.check_host_in_room( room_id, self.server_name ) if not is_in_room: - was_in_room = yield self.store.was_host_joined( - pdu.room_id, self.server_name, + logger.info( + "[%s %s] Ignoring PDU from %s as we're not in the room", + room_id, event_id, origin, ) - if was_in_room: - logger.info( - "[%s %s] Ignoring PDU from %s as we've left the room", - room_id, event_id, origin, - ) - defer.returnValue(None) + defer.returnValue(None) state = None auth_chain = [] From dc59ad533414d63a2b87da705d9792fca4ee8b36 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 9 Nov 2018 14:58:09 +0000 Subject: [PATCH 6/6] Remove hack to support rejoining rooms --- synapse/handlers/federation.py | 120 ++++++++++++--------------------- 1 file changed, 44 insertions(+), 76 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index e859ff5ffa..a3bb864bb2 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -552,86 +552,54 @@ class FederationHandler(BaseHandler): room_id, event_id, event, ) - # FIXME (erikj): Awful hack to make the case where we are not currently - # in the room work - # If state and auth_chain are None, then we don't need to do this check - # as we already know we have enough state in the DB to handle this - # event. - if state and auth_chain and not event.internal_metadata.is_outlier(): - is_in_room = yield self.auth.check_host_in_room( - room_id, - self.server_name - ) - else: - is_in_room = True + event_ids = set() + if state: + event_ids |= {e.event_id for e in state} + if auth_chain: + event_ids |= {e.event_id for e in auth_chain} + + seen_ids = yield self.store.have_seen_events(event_ids) + + if state and auth_chain is not None: + # If we have any state or auth_chain given to us by the replication + # layer, then we should handle them (if we haven't before.) + + event_infos = [] + + for e in itertools.chain(auth_chain, state): + if e.event_id in seen_ids: + continue + e.internal_metadata.outlier = True + auth_ids = e.auth_event_ids() + auth = { + (e.type, e.state_key): e for e in auth_chain + if e.event_id in auth_ids or e.type == EventTypes.Create + } + event_infos.append({ + "event": e, + "auth_events": auth, + }) + seen_ids.add(e.event_id) - if not is_in_room: logger.info( - "[%s %s] Got event for room we're not in", - room_id, event_id, + "[%s %s] persisting newly-received auth/state events %s", + room_id, event_id, [e["event"].event_id for e in event_infos] ) + yield self._handle_new_events(origin, event_infos) - try: - yield self._persist_auth_tree( - origin, auth_chain, state, event - ) - except AuthError as e: - raise FederationError( - "ERROR", - e.code, - e.msg, - affected=event_id, - ) - - else: - event_ids = set() - if state: - event_ids |= {e.event_id for e in state} - if auth_chain: - event_ids |= {e.event_id for e in auth_chain} - - seen_ids = yield self.store.have_seen_events(event_ids) - - if state and auth_chain is not None: - # If we have any state or auth_chain given to us by the replication - # layer, then we should handle them (if we haven't before.) - - event_infos = [] - - for e in itertools.chain(auth_chain, state): - if e.event_id in seen_ids: - continue - e.internal_metadata.outlier = True - auth_ids = e.auth_event_ids() - auth = { - (e.type, e.state_key): e for e in auth_chain - if e.event_id in auth_ids or e.type == EventTypes.Create - } - event_infos.append({ - "event": e, - "auth_events": auth, - }) - seen_ids.add(e.event_id) - - logger.info( - "[%s %s] persisting newly-received auth/state events %s", - room_id, event_id, [e["event"].event_id for e in event_infos] - ) - yield self._handle_new_events(origin, event_infos) - - try: - context = yield self._handle_new_event( - origin, - event, - state=state, - ) - except AuthError as e: - raise FederationError( - "ERROR", - e.code, - e.msg, - affected=event.event_id, - ) + try: + context = yield self._handle_new_event( + origin, + event, + state=state, + ) + except AuthError as e: + raise FederationError( + "ERROR", + e.code, + e.msg, + affected=event.event_id, + ) room = yield self.store.get_room(room_id)