diff options
Diffstat (limited to '')
| -rw-r--r-- | changelog.d/12925.misc | 1 | ||||
| -rw-r--r-- | synapse/federation/federation_base.py | 89 | ||||
| -rw-r--r-- | synapse/federation/federation_client.py | 45 | ||||
| -rw-r--r-- | synapse/federation/federation_server.py | 25 |
4 files changed, 95 insertions, 65 deletions
diff --git a/changelog.d/12925.misc b/changelog.d/12925.misc new file mode 100644 index 0000000000..71ca956dc5 --- /dev/null +++ b/changelog.d/12925.misc @@ -0,0 +1 @@ +Improve the logging when signature checks on events fail. diff --git a/synapse/federation/federation_base.py b/synapse/federation/federation_base.py index 7bc54b9988..a6232e048b 100644 --- a/synapse/federation/federation_base.py +++ b/synapse/federation/federation_base.py @@ -32,6 +32,18 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) +class InvalidEventSignatureError(RuntimeError): + """Raised when the signature on an event is invalid. + + The stringification of this exception is just the error message without reference + to the event id. The event id is available as a property. + """ + + def __init__(self, message: str, event_id: str): + super().__init__(message) + self.event_id = event_id + + class FederationBase: def __init__(self, hs: "HomeServer"): self.hs = hs @@ -59,20 +71,13 @@ class FederationBase: Returns: * the original event if the checks pass * a redacted version of the event (if the signature - matched but the hash did not) + matched but the hash did not). In this case a warning will be logged. Raises: - SynapseError if the signature check failed. + InvalidEventSignatureError if the signature check failed. Nothing + will be logged in this case. """ - try: - await _check_sigs_on_pdu(self.keyring, room_version, pdu) - except SynapseError as e: - logger.warning( - "Signature check failed for %s: %s", - pdu.event_id, - e, - ) - raise + await _check_sigs_on_pdu(self.keyring, room_version, pdu) if not check_event_content_hash(pdu): # let's try to distinguish between failures because the event was @@ -87,7 +92,7 @@ class FederationBase: if set(redacted_event.keys()) == set(pdu.keys()) and set( redacted_event.content.keys() ) == set(pdu.content.keys()): - logger.info( + logger.debug( "Event %s seems to have been redacted; using our redacted copy", pdu.event_id, ) @@ -116,12 +121,13 @@ async def _check_sigs_on_pdu( ) -> None: """Check that the given events are correctly signed - Raise a SynapseError if the event wasn't correctly signed. - Args: keyring: keyring object to do the checks room_version: the room version of the PDUs pdus: the events to be checked + + Raises: + InvalidEventSignatureError if the event wasn't correctly signed. """ # we want to check that the event is signed by: @@ -147,44 +153,38 @@ async def _check_sigs_on_pdu( # First we check that the sender event is signed by the sender's domain # (except if its a 3pid invite, in which case it may be sent by any server) + sender_domain = get_domain_from_id(pdu.sender) if not _is_invite_via_3pid(pdu): try: await keyring.verify_event_for_server( - get_domain_from_id(pdu.sender), + sender_domain, pdu, pdu.origin_server_ts if room_version.enforce_key_validity else 0, ) except Exception as e: - errmsg = "event id %s: unable to verify signature for sender %s: %s" % ( + raise InvalidEventSignatureError( + f"unable to verify signature for sender domain {sender_domain}: {e}", pdu.event_id, - get_domain_from_id(pdu.sender), - e, - ) - raise SynapseError(403, errmsg, Codes.FORBIDDEN) + ) from None # now let's look for events where the sender's domain is different to the # event id's domain (normally only the case for joins/leaves), and add additional # checks. Only do this if the room version has a concept of event ID domain # (ie, the room version uses old-style non-hash event IDs). - if room_version.event_format == EventFormatVersions.V1 and get_domain_from_id( - pdu.event_id - ) != get_domain_from_id(pdu.sender): - try: - await keyring.verify_event_for_server( - get_domain_from_id(pdu.event_id), - pdu, - pdu.origin_server_ts if room_version.enforce_key_validity else 0, - ) - except Exception as e: - errmsg = ( - "event id %s: unable to verify signature for event id domain %s: %s" - % ( - pdu.event_id, - get_domain_from_id(pdu.event_id), - e, + if room_version.event_format == EventFormatVersions.V1: + event_domain = get_domain_from_id(pdu.event_id) + if event_domain != sender_domain: + try: + await keyring.verify_event_for_server( + event_domain, + pdu, + pdu.origin_server_ts if room_version.enforce_key_validity else 0, ) - ) - raise SynapseError(403, errmsg, Codes.FORBIDDEN) + except Exception as e: + raise InvalidEventSignatureError( + f"unable to verify signature for event domain {event_domain}: {e}", + pdu.event_id, + ) from None # If this is a join event for a restricted room it may have been authorised # via a different server from the sending server. Check those signatures. @@ -204,15 +204,10 @@ async def _check_sigs_on_pdu( pdu.origin_server_ts if room_version.enforce_key_validity else 0, ) except Exception as e: - errmsg = ( - "event id %s: unable to verify signature for authorising server %s: %s" - % ( - pdu.event_id, - authorising_server, - e, - ) - ) - raise SynapseError(403, errmsg, Codes.FORBIDDEN) + raise InvalidEventSignatureError( + f"unable to verify signature for authorising serve {authorising_server}: {e}", + pdu.event_id, + ) from None def _is_invite_via_3pid(event: EventBase) -> bool: diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index b60b8983ea..ad475a913b 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -54,7 +54,11 @@ from synapse.api.room_versions import ( RoomVersions, ) from synapse.events import EventBase, builder -from synapse.federation.federation_base import FederationBase, event_from_pdu_json +from synapse.federation.federation_base import ( + FederationBase, + InvalidEventSignatureError, + event_from_pdu_json, +) from synapse.federation.transport.client import SendJoinResponse from synapse.http.types import QueryParams from synapse.types import JsonDict, UserID, get_domain_from_id @@ -319,7 +323,13 @@ class FederationClient(FederationBase): pdu = pdu_list[0] # Check signatures are correct. - signed_pdu = await self._check_sigs_and_hash(room_version, pdu) + try: + signed_pdu = await self._check_sigs_and_hash(room_version, pdu) + except InvalidEventSignatureError as e: + errmsg = f"event id {pdu.event_id}: {e}" + logger.warning("%s", errmsg) + raise SynapseError(403, errmsg, Codes.FORBIDDEN) + return signed_pdu return None @@ -555,20 +565,24 @@ class FederationClient(FederationBase): Returns: The PDU (possibly redacted) if it has valid signatures and hashes. + None if no valid copy could be found. """ - res = None try: - res = await self._check_sigs_and_hash(room_version, pdu) - except SynapseError: - pass - - if not res: - # Check local db. - res = await self.store.get_event( - pdu.event_id, allow_rejected=True, allow_none=True + return await self._check_sigs_and_hash(room_version, pdu) + except InvalidEventSignatureError as e: + logger.warning( + "Signature on retrieved event %s was invalid (%s). " + "Checking local store/orgin server", + pdu.event_id, + e, ) + # Check local db. + res = await self.store.get_event( + pdu.event_id, allow_rejected=True, allow_none=True + ) + pdu_origin = get_domain_from_id(pdu.sender) if not res and pdu_origin != origin: try: @@ -1043,9 +1057,14 @@ class FederationClient(FederationBase): pdu = event_from_pdu_json(pdu_dict, room_version) # Check signatures are correct. - pdu = await self._check_sigs_and_hash(room_version, pdu) + try: + pdu = await self._check_sigs_and_hash(room_version, pdu) + except InvalidEventSignatureError as e: + errmsg = f"event id {pdu.event_id}: {e}" + logger.warning("%s", errmsg) + raise SynapseError(403, errmsg, Codes.FORBIDDEN) - # FIXME: We should handle signature failures more gracefully. + # FIXME: We should handle signature failures more gracefully. return pdu diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 3ecede22d9..12591dc8db 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -48,7 +48,11 @@ from synapse.api.room_versions import KNOWN_ROOM_VERSIONS, RoomVersion from synapse.crypto.event_signing import compute_event_signature from synapse.events import EventBase from synapse.events.snapshot import EventContext -from synapse.federation.federation_base import FederationBase, event_from_pdu_json +from synapse.federation.federation_base import ( + FederationBase, + InvalidEventSignatureError, + event_from_pdu_json, +) from synapse.federation.persistence import TransactionActions from synapse.federation.units import Edu, Transaction from synapse.http.servlet import assert_params_in_dict @@ -631,7 +635,12 @@ class FederationServer(FederationBase): pdu = event_from_pdu_json(content, room_version) origin_host, _ = parse_server_name(origin) await self.check_server_matches_acl(origin_host, pdu.room_id) - pdu = await self._check_sigs_and_hash(room_version, pdu) + try: + pdu = await self._check_sigs_and_hash(room_version, pdu) + except InvalidEventSignatureError as e: + errmsg = f"event id {pdu.event_id}: {e}" + logger.warning("%s", errmsg) + raise SynapseError(403, errmsg, Codes.FORBIDDEN) ret_pdu = await self.handler.on_invite_request(origin, pdu, room_version) time_now = self._clock.time_msec() return {"event": ret_pdu.get_pdu_json(time_now)} @@ -864,7 +873,12 @@ class FederationServer(FederationBase): ) ) - event = await self._check_sigs_and_hash(room_version, event) + try: + event = await self._check_sigs_and_hash(room_version, event) + except InvalidEventSignatureError as e: + errmsg = f"event id {event.event_id}: {e}" + logger.warning("%s", errmsg) + raise SynapseError(403, errmsg, Codes.FORBIDDEN) return await self._federation_event_handler.on_send_membership_event( origin, event @@ -1016,8 +1030,9 @@ class FederationServer(FederationBase): # Check signature. try: pdu = await self._check_sigs_and_hash(room_version, pdu) - except SynapseError as e: - raise FederationError("ERROR", e.code, e.msg, affected=pdu.event_id) + except InvalidEventSignatureError as e: + logger.warning("event id %s: %s", pdu.event_id, e) + raise FederationError("ERROR", 403, str(e), affected=pdu.event_id) if await self._spam_checker.should_drop_federated_event(pdu): logger.warning( |