Don't fail an entire request if one of the returned events fails a signature check. If an event does fail a signature check, look in the local database and request it from the originator.

synapse/federation/federation_client.py

@@ -224,17 +224,17 @@ class FederationClient(object):
             for p in result.get("auth_chain", [])
         ]
 
-        for i, pdu in enumerate(pdus):
+        signed_pdus = yield self._check_sigs_and_hash_and_fetch(
-            pdus[i] = yield self._check_sigs_and_hash(pdu)
+            pdus, outlier=True
+        )
 
-            # FIXME: We should handle signature failures more gracefully.
+        signed_auth = yield self._check_sigs_and_hash_and_fetch(
+            auth_chain, outlier=True
+        )
 
-        for i, pdu in enumerate(auth_chain):
+        signed_auth.sort(key=lambda e: e.depth)
-            auth_chain[i] = yield self._check_sigs_and_hash(pdu)
 
-            # FIXME: We should handle signature failures more gracefully.
+        defer.returnValue((signed_pdus, signed_auth))
-
-        defer.returnValue((pdus, auth_chain))
 
     @defer.inlineCallbacks
     @log_function
@@ -248,14 +248,13 @@ class FederationClient(object):
             for p in res["auth_chain"]
         ]
 
-        for i, pdu in enumerate(auth_chain):
+        signed_auth = yield self._check_sigs_and_hash_and_fetch(
-            auth_chain[i] = yield self._check_sigs_and_hash(pdu)
+            auth_chain, outlier=True
+        )
 
-            # FIXME: We should handle signature failures more gracefully.
+        signed_auth.sort(key=lambda e: e.depth)
 
-        auth_chain.sort(key=lambda e: e.depth)
+        defer.returnValue(signed_auth)
-
-        defer.returnValue(auth_chain)
 
     @defer.inlineCallbacks
     def make_join(self, destination, room_id, user_id):
@@ -291,21 +290,19 @@ class FederationClient(object):
             for p in content.get("auth_chain", [])
         ]
 
-        for i, pdu in enumerate(state):
+        signed_state = yield self._check_sigs_and_hash_and_fetch(
-            state[i] = yield self._check_sigs_and_hash(pdu)
+            state, outlier=True
+        )
 
-            # FIXME: We should handle signature failures more gracefully.
+        signed_auth = yield self._check_sigs_and_hash_and_fetch(
-
+            auth_chain, outlier=True
-        for i, pdu in enumerate(auth_chain):
+        )
-            auth_chain[i] = yield self._check_sigs_and_hash(pdu)
-
-            # FIXME: We should handle signature failures more gracefully.
 
         auth_chain.sort(key=lambda e: e.depth)
 
         defer.returnValue({
-            "state": state,
+            "state": signed_state,
-            "auth_chain": auth_chain,
+            "auth_chain": signed_auth,
         })
 
     @defer.inlineCallbacks
@@ -353,12 +350,18 @@ class FederationClient(object):
         )
 
         auth_chain = [
-            (yield self._check_sigs_and_hash(self.event_from_pdu_json(e)))
+            self.event_from_pdu_json(e)
             for e in content["auth_chain"]
         ]
 
+        signed_auth = yield self._check_sigs_and_hash_and_fetch(
+            auth_chain, outlier=True
+        )
+
+        signed_auth.sort(key=lambda e: e.depth)
+
         ret = {
-            "auth_chain": auth_chain,
+            "auth_chain": signed_auth,
             "rejects": content.get("rejects", []),
             "missing": content.get("missing", []),
         }
@@ -374,6 +377,58 @@ class FederationClient(object):
 
         return event
 
+    @defer.inlineCallbacks
+    def _check_sigs_and_hash_and_fetch(self, pdus, outlier=False):
+        """Takes a list of PDUs and checks the signatures and hashs of each
+        one. If a PDU fails its signature check then we check if we have it in
+        the database and if not then request if from the originating server of
+        that PDU.
+
+        If a PDU fails its content hash check then it is redacted.
+
+        The given list of PDUs are not modified, instead the function returns
+        a new list.
+
+        Args:
+            pdu (list)
+            outlier (bool)
+
+        Returns:
+            Deferred : A list of PDUs that have valid signatures and hashes.
+        """
+        signed_pdus = []
+        for pdu in pdus:
+            try:
+                new_pdu = yield self._check_sigs_and_hash(pdu)
+                signed_pdus.append(new_pdu)
+            except SynapseError:
+                # FIXME: We should handle signature failures more gracefully.
+
+                # Check local db.
+                new_pdu = yield self.store.get_event(
+                    pdu.event_id,
+                    allow_rejected=True
+                )
+                if new_pdu:
+                    signed_pdus.append(new_pdu)
+                    continue
+
+                # Check pdu.origin
+                new_pdu = yield self.get_pdu(
+                    destinations=[pdu.origin],
+                    event_id=pdu.event_id,
+                    outlier=outlier,
+                )
+
+                if new_pdu:
+                    signed_pdus.append(new_pdu)
+                    continue
+
+                logger.warn("Failed to find copy of %s with valid signature")
+
+        defer.returnValue(signed_pdus)
+
+
     @defer.inlineCallbacks
     def _check_sigs_and_hash(self, pdu):
         """Throws a SynapseError if the PDU does not have the correct

synapse/storage/__init__.py

@@ -128,16 +128,21 @@ class DataStore(RoomMemberStore, RoomStore,
             pass
 
     @defer.inlineCallbacks
-    def get_event(self, event_id, allow_none=False):
+    def get_event(self, event_id, check_redacted=True,
-        events = yield self._get_events([event_id])
+                  get_prev_content=False, allow_rejected=False,
+                  allow_none=False):
+        event = yield self.runInteraction(
+            "get_event", self._get_event_txn,
+            event_id,
+            check_redacted=check_redacted,
+            get_prev_content=get_prev_content,
+            allow_rejected=allow_rejected,
+        )
 
-        if not events:
+        if not event and not allow_none:
-            if allow_none:
-                defer.returnValue(None)
-            else:
             raise RuntimeError("Could not find event %s" % (event_id,))
 
-        defer.returnValue(events[0])
+        defer.returnValue(event)
 
     @log_function
     def _persist_event_txn(self, txn, event, context, backfilled,