Return errors from send_join etc if the event is rejected (#10243)

Rather than persisting rejected events via `send_join` and friends, raise a 403 if someone tries to pull a fast one.
This commit is contained in:
Richard van der Hoff 2021-06-24 16:00:08 +01:00 committed by GitHub
parent 6e8fb42be7
commit 8165ba48b1
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 41 additions and 10 deletions

View file

@ -0,0 +1 @@
Improve validation on federation `send_{join,leave,knock}` endpoints.

View file

@ -1953,16 +1953,31 @@ class FederationHandler(BaseHandler):
self, origin: str, event: EventBase self, origin: str, event: EventBase
) -> EventContext: ) -> EventContext:
""" """
We have received a join/leave/knock event for a room. We have received a join/leave/knock event for a room via send_join/leave/knock.
Verify that event and send it into the room on the remote homeserver's behalf. Verify that event and send it into the room on the remote homeserver's behalf.
This is quite similar to on_receive_pdu, with the following principal
differences:
* only membership events are permitted (and only events with
sender==state_key -- ie, no kicks or bans)
* *We* send out the event on behalf of the remote server.
* We enforce the membership restrictions of restricted rooms.
* Rejected events result in an exception rather than being stored.
There are also other differences, however it is not clear if these are by
design or omission. In particular, we do not attempt to backfill any missing
prev_events.
Args: Args:
origin: The homeserver of the remote (joining/invited/knocking) user. origin: The homeserver of the remote (joining/invited/knocking) user.
event: The member event that has been signed by the remote homeserver. event: The member event that has been signed by the remote homeserver.
Returns: Returns:
The context of the event after inserting it into the room graph. The context of the event after inserting it into the room graph.
Raises:
SynapseError if the event is not accepted into the room
""" """
logger.debug( logger.debug(
"on_send_membership_event: Got event: %s, signatures: %s", "on_send_membership_event: Got event: %s, signatures: %s",
@ -1981,7 +1996,7 @@ class FederationHandler(BaseHandler):
if event.sender != event.state_key: if event.sender != event.state_key:
raise SynapseError(400, "state_key and sender must match", Codes.BAD_JSON) raise SynapseError(400, "state_key and sender must match", Codes.BAD_JSON)
event.internal_metadata.outlier = False assert not event.internal_metadata.outlier
# Send this event on behalf of the other server. # Send this event on behalf of the other server.
# #
@ -1991,6 +2006,11 @@ class FederationHandler(BaseHandler):
event.internal_metadata.send_on_behalf_of = origin event.internal_metadata.send_on_behalf_of = origin
context = await self.state_handler.compute_event_context(event) context = await self.state_handler.compute_event_context(event)
context = await self._check_event_auth(origin, event, context)
if context.rejected:
raise SynapseError(
403, f"{event.membership} event was rejected", Codes.FORBIDDEN
)
# for joins, we need to check the restrictions of restricted rooms # for joins, we need to check the restrictions of restricted rooms
if event.membership == Membership.JOIN: if event.membership == Membership.JOIN:
@ -2008,8 +2028,8 @@ class FederationHandler(BaseHandler):
403, "This event is not allowed in this context", Codes.FORBIDDEN 403, "This event is not allowed in this context", Codes.FORBIDDEN
) )
await self._auth_and_persist_event(origin, event, context) # all looks good, we can persist the event.
await self._run_push_actions_and_persist_event(event, context)
return context return context
async def _check_join_restrictions( async def _check_join_restrictions(
@ -2179,6 +2199,18 @@ class FederationHandler(BaseHandler):
backfilled=backfilled, backfilled=backfilled,
) )
await self._run_push_actions_and_persist_event(event, context, backfilled)
async def _run_push_actions_and_persist_event(
self, event: EventBase, context: EventContext, backfilled: bool = False
):
"""Run the push actions for a received event, and persist it.
Args:
event: The event itself.
context: The event context.
backfilled: True if the event was backfilled.
"""
try: try:
if ( if (
not event.internal_metadata.is_outlier() not event.internal_metadata.is_outlier()
@ -2492,9 +2524,9 @@ class FederationHandler(BaseHandler):
origin: str, origin: str,
event: EventBase, event: EventBase,
context: EventContext, context: EventContext,
state: Optional[Iterable[EventBase]], state: Optional[Iterable[EventBase]] = None,
auth_events: Optional[MutableStateMap[EventBase]], auth_events: Optional[MutableStateMap[EventBase]] = None,
backfilled: bool, backfilled: bool = False,
) -> EventContext: ) -> EventContext:
""" """
Checks whether an event should be rejected (for failing auth checks). Checks whether an event should be rejected (for failing auth checks).

View file

@ -205,9 +205,7 @@ class FederationKnockingTestCase(
# Have this homeserver skip event auth checks. This is necessary due to # Have this homeserver skip event auth checks. This is necessary due to
# event auth checks ensuring that events were signed by the sender's homeserver. # event auth checks ensuring that events were signed by the sender's homeserver.
async def _check_event_auth( async def _check_event_auth(origin, event, context, *args, **kwargs):
origin, event, context, state, auth_events, backfilled
):
return context return context
homeserver.get_federation_handler()._check_event_auth = _check_event_auth homeserver.get_federation_handler()._check_event_auth = _check_event_auth