Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Improve logging when signature checks fail #12925

Merged
merged 4 commits into from May 31, 2022
Merged

Improve logging when signature checks fail #12925

Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e8ba5dc
Raise a dedicated `InvalidEventSignatureError` from `_check_sigs_on_pdu`
richvdh May 27, 2022
d696b92
Downgrade logging about redactions to DEBUG
richvdh May 27, 2022
ae44ee8
Raise `InvalidEventSignatureError` from `_check_sigs_and_hash`
richvdh May 27, 2022
dc05746
changelog
richvdh May 31, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
83 changes: 41 additions & 42 deletions synapse/federation/federation_base.py
View file
Open in desktop
Expand Up @@ -33,6 +33,18 @@
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
Expand Down Expand Up @@ -67,13 +79,10 @@ async def _check_sigs_and_hash(
"""
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
except InvalidEventSignatureError as e:
errmsg = f"event id {pdu.event_id}: {e}"
logger.warning("%s", errmsg)
raise SynapseError(403, errmsg, Codes.FORBIDDEN)
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

_check_sigs_on_pdu now raises an InvalidEventSignatureError where previously it raised a SynapseError, so modulo slight wording tweaks to the log line and returned error, this should be identical, functionally.


if not check_event_content_hash(pdu):
# let's try to distinguish between failures because the event was
Expand Down Expand Up @@ -117,12 +126,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:
Expand All @@ -148,44 +158,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.
Expand All @@ -205,15 +209,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:
Expand Down