From 4bbfbf898ecfe9bd77b02a7e014d94163d1fd534 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 1 Jun 2015 17:02:23 +0100 Subject: [PATCH 1/8] Correctly pass in auth_events --- synapse/handlers/federation.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 46ce3699d7..caf6a1f8eb 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -262,7 +262,13 @@ class FederationHandler(BaseHandler): yield defer.gatherResults( [ - self._handle_new_event(dest, a) + self._handle_new_event( + dest, a, + auth_events={ + (e.type, e.state_key): e for e in auth_events + if e.event_id in [a_id for a_id, _ in a.auth_events] + } + ) for a in auth_events.values() ], consumeErrors=True, @@ -274,6 +280,10 @@ class FederationHandler(BaseHandler): dest, event_map[e_id], state=events_to_state[e_id], backfilled=True, + auth_events={ + (e.type, e.state_key): e for e in auth_events + if e.event_id in [a_id for a_id, _ in a.auth_events] + } ) for e_id in events_to_state ], From 3f04a08a0c922962e3a5439b1740424202dc4616 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Jun 2015 10:11:32 +0100 Subject: [PATCH 2/8] Don't process events we've already processed. Remember to process state events --- synapse/handlers/federation.py | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index caf6a1f8eb..acbb53d6c5 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -250,6 +250,7 @@ class FederationHandler(BaseHandler): # For each edge get the current state. auth_events = {} + state_events = {} events_to_state = {} for e_id in edges: state, auth = yield self.replication_layer.get_state_for_room( @@ -258,8 +259,13 @@ class FederationHandler(BaseHandler): event_id=e_id ) auth_events.update({a.event_id: a for a in auth}) + state_events.update({s.event_id: s for s in state}) events_to_state[e_id] = state + seen_events = yield self.store.have_events( + set(auth_events.keys()) | set(state_events.keys()) + ) + yield defer.gatherResults( [ self._handle_new_event( @@ -270,6 +276,22 @@ class FederationHandler(BaseHandler): } ) for a in auth_events.values() + if a.event_id not in seen_events + ], + consumeErrors=True, + ).addErrback(unwrapFirstError) + + yield defer.gatherResults( + [ + self._handle_new_event( + dest, s, + auth_events={ + (e.type, e.state_key): e for e in auth_events + if e.event_id in [a_id for a_id, _ in s.auth_events] + } + ) + for s in state_events.values() + if s.event_id not in seen_events ], consumeErrors=True, ).addErrback(unwrapFirstError) From fde0da6f19aeb6dee26fc0b89fcee9d27a50b8f4 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Jun 2015 10:19:38 +0100 Subject: [PATCH 3/8] Correctly look up auth_events --- synapse/handlers/federation.py | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index acbb53d6c5..5cd853d85e 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -271,9 +271,10 @@ class FederationHandler(BaseHandler): self._handle_new_event( dest, a, auth_events={ - (e.type, e.state_key): e for e in auth_events - if e.event_id in [a_id for a_id, _ in a.auth_events] - } + (auth_events[a_id].type, auth_events[a_id].state_key): + auth_events[a_id] + for a_id, _ in a.auth_events + }, ) for a in auth_events.values() if a.event_id not in seen_events @@ -286,9 +287,10 @@ class FederationHandler(BaseHandler): self._handle_new_event( dest, s, auth_events={ - (e.type, e.state_key): e for e in auth_events - if e.event_id in [a_id for a_id, _ in s.auth_events] - } + (auth_events[a_id].type, auth_events[a_id].state_key): + auth_events[a_id] + for a_id, _ in s.auth_events + }, ) for s in state_events.values() if s.event_id not in seen_events @@ -303,9 +305,10 @@ class FederationHandler(BaseHandler): state=events_to_state[e_id], backfilled=True, auth_events={ - (e.type, e.state_key): e for e in auth_events - if e.event_id in [a_id for a_id, _ in a.auth_events] - } + (auth_events[a_id].type, auth_events[a_id].state_key): + auth_events[a_id] + for a_id, _ in event_map[e_id].auth_events + }, ) for e_id in events_to_state ], From e552b78d50a2f162a536057af48c7815fb181c30 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Jun 2015 10:28:14 +0100 Subject: [PATCH 4/8] Add some logging --- synapse/handlers/federation.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 5cd853d85e..ced6fab16f 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -247,6 +247,11 @@ class FederationHandler(BaseHandler): if set(e_id for e_id, _ in ev.prev_events) - event_ids ] + logger.info( + "backfill: Got %d events with %d edges", + len(events), len(edges), + ) + # For each edge get the current state. auth_events = {} From 02410e92395f53b88017bf93a82be41afe1a839e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Jun 2015 10:58:35 +0100 Subject: [PATCH 5/8] Handle the fact we might be missing auth events --- synapse/handlers/federation.py | 36 +++++++++++++++++++--------------- 1 file changed, 20 insertions(+), 16 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index ced6fab16f..b333175349 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -264,6 +264,7 @@ class FederationHandler(BaseHandler): event_id=e_id ) auth_events.update({a.event_id: a for a in auth}) + auth_events.update({s.event_id: s for s in state}) state_events.update({s.event_id: s for s in state}) events_to_state[e_id] = state @@ -271,6 +272,25 @@ class FederationHandler(BaseHandler): set(auth_events.keys()) | set(state_events.keys()) ) + all_events = events + state_events.values() + auth_events.values() + required_auth = set( + a_id for event in all_events for a_id, _ in event.auth_events + ) + + missing_auth = required_auth - set(auth_events) + results = yield defer.gatherResults( + [ + self.replication_layer.get_pdu( + [dest], + event_id, + outlier=True, + ) + for event_id in missing_auth + ], + consumeErrors=True + ).addErrback(unwrapFirstError) + auth_events.update({a.event_id: a for a in results}) + yield defer.gatherResults( [ self._handle_new_event( @@ -287,22 +307,6 @@ class FederationHandler(BaseHandler): consumeErrors=True, ).addErrback(unwrapFirstError) - yield defer.gatherResults( - [ - self._handle_new_event( - dest, s, - auth_events={ - (auth_events[a_id].type, auth_events[a_id].state_key): - auth_events[a_id] - for a_id, _ in s.auth_events - }, - ) - for s in state_events.values() - if s.event_id not in seen_events - ], - consumeErrors=True, - ).addErrback(unwrapFirstError) - yield defer.gatherResults( [ self._handle_new_event( From 09e23334decf6e01533d2c15f49f418e9e8df6e5 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Jun 2015 11:00:37 +0100 Subject: [PATCH 6/8] Add a timeout --- synapse/handlers/federation.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index b333175349..e8e3173ca2 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -284,6 +284,7 @@ class FederationHandler(BaseHandler): [dest], event_id, outlier=True, + timeout=10000, ) for event_id in missing_auth ], From 22716774d587354229149c752ace3b581013a077 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Jun 2015 14:26:54 +0100 Subject: [PATCH 7/8] Don't about JSON when warning about content tampering --- synapse/federation/federation_base.py | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/synapse/federation/federation_base.py b/synapse/federation/federation_base.py index f0430b2cb1..6799ed05ac 100644 --- a/synapse/federation/federation_base.py +++ b/synapse/federation/federation_base.py @@ -18,8 +18,6 @@ from twisted.internet import defer from synapse.events.utils import prune_event -from syutil.jsonutil import encode_canonical_json - from synapse.crypto.event_signing import check_event_content_hash from synapse.api.errors import SynapseError @@ -120,16 +118,15 @@ class FederationBase(object): ) except SynapseError: logger.warn( - "Signature check failed for %s redacted to %s", - encode_canonical_json(pdu.get_pdu_json()), - encode_canonical_json(redacted_pdu_json), + "Signature check failed for %s, redacting", + pdu.event_id, ) raise if not check_event_content_hash(pdu): logger.warn( - "Event content has been tampered, redacting %s, %s", - pdu.event_id, encode_canonical_json(pdu.get_dict()) + "Event content has been tampered, redacting.", + pdu.event_id, ) defer.returnValue(redacted_event) From d3ded420b11f98fa9b3635099808643992f65e93 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 2 Jun 2015 14:39:15 +0100 Subject: [PATCH 8/8] Rephrase log line --- synapse/federation/federation_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/federation/federation_base.py b/synapse/federation/federation_base.py index 6799ed05ac..299493af91 100644 --- a/synapse/federation/federation_base.py +++ b/synapse/federation/federation_base.py @@ -118,7 +118,7 @@ class FederationBase(object): ) except SynapseError: logger.warn( - "Signature check failed for %s, redacting", + "Signature check failed for %s", pdu.event_id, ) raise