From b4a24296a7c886c796571a906e1406ee6e937c03 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 21 Jun 2021 20:12:13 +0100 Subject: [PATCH 1/8] Factor out common parts of FederationHandler.send_join/send_leave/send_knock three copies of basically the same code :/ --- synapse/federation/federation_server.py | 4 +- synapse/handlers/federation.py | 167 ++++++------------ tests/handlers/test_federation.py | 2 +- .../test_federation_sender_shard.py | 2 +- 4 files changed, 55 insertions(+), 120 deletions(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 2b07f1852953..0fae57708713 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -585,7 +585,7 @@ async def on_send_leave_request(self, origin: str, content: JsonDict) -> dict: pdu = await self._check_sigs_and_hash(room_version, pdu) - await self.handler.on_send_leave_request(origin, pdu) + await self.handler.on_send_membership_event(origin, pdu) return {} async def on_make_knock_request( @@ -673,7 +673,7 @@ async def on_send_knock_request( pdu = await self._check_sigs_and_hash(room_version, pdu) # Handle the event, and retrieve the EventContext - event_context = await self.handler.on_send_knock_request(origin, pdu) + event_context = await self.handler.on_send_membership_event(origin, pdu) # Retrieve stripped state events from the room and send them back to the remote # server. This will allow the remote server's clients to display information diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 74d169a2ac97..e4759b3711d2 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1715,73 +1715,12 @@ async def on_send_join_request(self, origin: str, pdu: EventBase) -> JsonDict: """We have received a join event for a room. Fully process it and respond with the current state and auth chains. """ - event = pdu - - logger.debug( - "on_send_join_request from %s: Got event: %s, signatures: %s", - origin, - event.event_id, - event.signatures, - ) - - if get_domain_from_id(event.sender) != origin: - logger.info( - "Got /send_join request for user %r from different origin %s", - event.sender, - origin, - ) - raise SynapseError(403, "User not from origin", Codes.FORBIDDEN) - - event.internal_metadata.outlier = False - # Send this event on behalf of the origin server. - # - # The reasons we have the destination server rather than the origin - # server send it are slightly mysterious: the origin server should have - # all the necessary state once it gets the response to the send_join, - # so it could send the event itself if it wanted to. It may be that - # doing it this way reduces failure modes, or avoids certain attacks - # where a new server selectively tells a subset of the federation that - # it has joined. - # - # The fact is that, as of the current writing, Synapse doesn't send out - # the join event over federation after joining, and changing it now - # would introduce the danger of backwards-compatibility problems. - event.internal_metadata.send_on_behalf_of = origin - - # Calculate the event context. - context = await self.state_handler.compute_event_context(event) - - # Get the state before the new event. + context = await self.on_send_membership_event(origin, pdu) prev_state_ids = await context.get_prev_state_ids() - - # Check if the user is already in the room or invited to the room. - user_id = event.state_key - prev_member_event_id = prev_state_ids.get((EventTypes.Member, user_id), None) - prev_member_event = None - if prev_member_event_id: - prev_member_event = await self.store.get_event(prev_member_event_id) - - # Check if the member should be allowed access via membership in a space. - await self._event_auth_handler.check_restricted_join_rules( - prev_state_ids, - event.room_version, - user_id, - prev_member_event, - ) - - # Persist the event. - await self._auth_and_persist_event(origin, event, context) - - logger.debug( - "on_send_join_request: After _auth_and_persist_event: %s, sigs: %s", - event.event_id, - event.signatures, - ) - state_ids = list(prev_state_ids.values()) - auth_chain = await self.store.get_auth_chain(event.room_id, state_ids) - state = await self.store.get_events(list(prev_state_ids.values())) + auth_chain = await self.store.get_auth_chain(pdu.room_id, state_ids) + state = await self.store.get_events(state_ids) return {"state": list(state.values()), "auth_chain": auth_chain} @@ -1960,44 +1899,6 @@ async def on_make_leave_request( return event - async def on_send_leave_request(self, origin: str, pdu: EventBase) -> None: - """We have received a leave event for a room. Fully process it.""" - event = pdu - - logger.debug( - "on_send_leave_request: Got event: %s, signatures: %s", - event.event_id, - event.signatures, - ) - - if get_domain_from_id(event.sender) != origin: - logger.info( - "Got /send_leave request for user %r from different origin %s", - event.sender, - origin, - ) - raise SynapseError(403, "User not from origin", Codes.FORBIDDEN) - - event.internal_metadata.outlier = False - - # Send this event on behalf of the other server. - # - # The remote server isn't a full participant in the room at this point, so - # may not have an up-to-date list of the other homeservers participating in - # the room, so we send it on their behalf. - event.internal_metadata.send_on_behalf_of = origin - - context = await self.state_handler.compute_event_context(event) - await self._auth_and_persist_event(origin, event, context) - - logger.debug( - "on_send_leave_request: After _auth_and_persist_event: %s, sigs: %s", - event.event_id, - event.signatures, - ) - - return None - @log_function async def on_make_knock_request( self, origin: str, room_id: str, user_id: str @@ -2061,29 +1962,30 @@ async def on_make_knock_request( return event @log_function - async def on_send_knock_request( + async def on_send_membership_event( self, origin: str, event: EventBase ) -> EventContext: """ - We have received a knock event for a room. Verify that event and send it into the room - on the knocking homeserver's behalf. + We have received a join/leave/knock event for a room. + + Verify that event and send it into the room on the remote homeserver's behalf. Args: - origin: The remote homeserver of the knocking user. - event: The knocking member event that has been signed by the remote homeserver. + origin: The homeserver of the remote (joining/invited/knocking) user. + event: The member event that has been signed by the remote homeserver. Returns: The context of the event after inserting it into the room graph. """ logger.debug( - "on_send_knock_request: Got event: %s, signatures: %s", + "on_send_membership_event: Got event: %s, signatures: %s", event.event_id, event.signatures, ) if get_domain_from_id(event.sender) != origin: logger.info( - "Got /send_knock request for user %r from different origin %s", + "Got send_membership request for user %r from different origin %s", event.sender, origin, ) @@ -2100,19 +2002,52 @@ async def on_send_knock_request( context = await self.state_handler.compute_event_context(event) - event_allowed = await self.third_party_event_rules.check_event_allowed( - event, context - ) - if not event_allowed: - logger.info("Sending of knock %s forbidden by third-party rules", event) - raise SynapseError( - 403, "This event is not allowed in this context", Codes.FORBIDDEN + # for joins, we need to check the restrictions of restricted rooms + if event.membership == Membership.JOIN: + await self._check_join_restrictions(context, event) + + # for knock events, we run the third-party event rules. It's not entirely clear + # why we don't do this for other sorts of membership events. + if event.membership == Membership.KNOCK: + event_allowed = await self.third_party_event_rules.check_event_allowed( + event, context ) + if not event_allowed: + logger.info("Sending of knock %s forbidden by third-party rules", event) + raise SynapseError( + 403, "This event is not allowed in this context", Codes.FORBIDDEN + ) await self._auth_and_persist_event(origin, event, context) return context + async def _check_join_restrictions( + self, context: EventContext, event: EventBase + ) -> None: + """Check that restrictions in restricted join rules are matched + + Called when we receive a join event via send_join. + + Raises an auth error if the restrictions are not matched. + """ + prev_state_ids = await context.get_prev_state_ids() + + # Check if the user is already in the room or invited to the room. + user_id = event.state_key + prev_member_event_id = prev_state_ids.get((EventTypes.Member, user_id), None) + prev_member_event = None + if prev_member_event_id: + prev_member_event = await self.store.get_event(prev_member_event_id) + + # Check if the member should be allowed access via membership in a space. + await self._event_auth_handler.check_restricted_join_rules( + prev_state_ids, + event.room_version, + user_id, + prev_member_event, + ) + async def get_state_for_pdu(self, room_id: str, event_id: str) -> List[EventBase]: """Returns the state at the event. i.e. not including said event.""" diff --git a/tests/handlers/test_federation.py b/tests/handlers/test_federation.py index 8796af45edb4..ba8cf44f4626 100644 --- a/tests/handlers/test_federation.py +++ b/tests/handlers/test_federation.py @@ -251,7 +251,7 @@ def _build_and_send_join_event(self, other_server, other_user, room_id): join_event.signatures[other_server] = {"x": "y"} with LoggingContext("send_join"): d = run_in_background( - self.handler.on_send_join_request, other_server, join_event + self.handler.on_send_membership_event, other_server, join_event ) self.get_success(d) diff --git a/tests/replication/test_federation_sender_shard.py b/tests/replication/test_federation_sender_shard.py index 584da5837179..a0c710f85568 100644 --- a/tests/replication/test_federation_sender_shard.py +++ b/tests/replication/test_federation_sender_shard.py @@ -228,7 +228,7 @@ def create_room_with_remote_server(self, user, token, remote_server="other_serve builder.build(prev_event_ids=prev_event_ids, auth_event_ids=None) ) - self.get_success(federation.on_send_join_request(remote_server, join_event)) + self.get_success(federation.on_send_membership_event(remote_server, join_event)) self.replicate() return room From d45ba1a0af60da1c077961733fc9f9a20ccf4d18 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 21 Jun 2021 20:20:46 +0100 Subject: [PATCH 2/8] Inline `FederationHandler.on_send_join_request` this is a bit simpler. --- synapse/federation/federation_server.py | 12 +++++++++--- synapse/handlers/federation.py | 13 ------------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 0fae57708713..e614dd6af312 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -552,11 +552,17 @@ async def on_send_join_request( pdu = await self._check_sigs_and_hash(room_version, pdu) - res_pdus = await self.handler.on_send_join_request(origin, pdu) + context = await self.handler.on_send_membership_event(origin, pdu) + + prev_state_ids = await context.get_prev_state_ids() + state_ids = list(prev_state_ids.values()) + auth_chain = await self.store.get_auth_chain(pdu.room_id, state_ids) + state = await self.store.get_events(state_ids) + time_now = self._clock.time_msec() return { - "state": [p.get_pdu_json(time_now) for p in res_pdus["state"]], - "auth_chain": [p.get_pdu_json(time_now) for p in res_pdus["auth_chain"]], + "state": [p.get_pdu_json(time_now) for p in state.values()], + "auth_chain": [p.get_pdu_json(time_now) for p in auth_chain], } async def on_make_leave_request( diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index e4759b3711d2..5a3e78879df3 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1711,19 +1711,6 @@ async def on_make_join_request( return event - async def on_send_join_request(self, origin: str, pdu: EventBase) -> JsonDict: - """We have received a join event for a room. Fully process it and - respond with the current state and auth chains. - """ - context = await self.on_send_membership_event(origin, pdu) - prev_state_ids = await context.get_prev_state_ids() - state_ids = list(prev_state_ids.values()) - - auth_chain = await self.store.get_auth_chain(pdu.room_id, state_ids) - state = await self.store.get_events(state_ids) - - return {"state": list(state.values()), "auth_chain": auth_chain} - async def on_invite_request( self, origin: str, event: EventBase, room_version: RoomVersion ) -> EventBase: From 8ef8652f6b2064c0e0e40fe76547a89ac90580aa Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 21 Jun 2021 21:24:05 +0100 Subject: [PATCH 3/8] Factor out common parts of FederationServer.on_send_join and on_send_leave --- synapse/federation/federation_server.py | 47 ++++++++++--------------- 1 file changed, 19 insertions(+), 28 deletions(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index e614dd6af312..e1526d28f488 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -46,6 +46,7 @@ ) from synapse.api.room_versions import KNOWN_ROOM_VERSIONS 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.persistence import TransactionActions from synapse.federation.units import Edu, Transaction @@ -539,24 +540,11 @@ async def on_invite_request( async def on_send_join_request( self, origin: str, content: JsonDict ) -> Dict[str, Any]: - logger.debug("on_send_join_request: content: %s", content) - - assert_params_in_dict(content, ["room_id"]) - room_version = await self.store.get_room_version(content["room_id"]) - 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) - - logger.debug("on_send_join_request: pdu sigs: %s", pdu.signatures) - - pdu = await self._check_sigs_and_hash(room_version, pdu) - - context = await self.handler.on_send_membership_event(origin, pdu) + context = await self._on_send_membership_event(origin, content) prev_state_ids = await context.get_prev_state_ids() state_ids = list(prev_state_ids.values()) - auth_chain = await self.store.get_auth_chain(pdu.room_id, state_ids) + auth_chain = await self.store.get_auth_chain(content["room_id"], state_ids) state = await self.store.get_events(state_ids) time_now = self._clock.time_msec() @@ -579,19 +567,7 @@ async def on_make_leave_request( async def on_send_leave_request(self, origin: str, content: JsonDict) -> dict: logger.debug("on_send_leave_request: content: %s", content) - - assert_params_in_dict(content, ["room_id"]) - room_version = await self.store.get_room_version(content["room_id"]) - 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) - - logger.debug("on_send_leave_request: pdu sigs: %s", pdu.signatures) - - pdu = await self._check_sigs_and_hash(room_version, pdu) - - await self.handler.on_send_membership_event(origin, pdu) + await self._on_send_membership_event(origin, content) return {} async def on_make_knock_request( @@ -691,6 +667,21 @@ async def on_send_knock_request( ) return {"knock_state_events": stripped_room_state} + async def _on_send_membership_event( + self, origin: str, content: JsonDict + ) -> EventContext: + assert_params_in_dict(content, ["room_id"]) + room_version = await self.store.get_room_version(content["room_id"]) + event = event_from_pdu_json(content, room_version) + + origin_host, _ = parse_server_name(origin) + await self.check_server_matches_acl(origin_host, event.room_id) + + logger.debug("_on_send_membership_event: pdu sigs: %s", event.signatures) + + event = await self._check_sigs_and_hash(room_version, event) + return await self.handler.on_send_membership_event(origin, event) + async def on_event_auth( self, origin: str, room_id: str, event_id: str ) -> Tuple[int, Dict[str, Any]]: From d52353ec211a70c643957ffa522da9d2c8074386 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 21 Jun 2021 21:45:50 +0100 Subject: [PATCH 4/8] More validation for `send_join` and `send_leave` ... make sure it's actually a join/leave event. --- synapse/federation/federation_server.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index e1526d28f488..dc4497b0db94 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -34,7 +34,7 @@ from twisted.internet.abstract import isIPAddress from twisted.python import failure -from synapse.api.constants import EduTypes, EventTypes +from synapse.api.constants import EduTypes, EventTypes, Membership from synapse.api.errors import ( AuthError, Codes, @@ -540,7 +540,7 @@ async def on_invite_request( async def on_send_join_request( self, origin: str, content: JsonDict ) -> Dict[str, Any]: - context = await self._on_send_membership_event(origin, content) + context = await self._on_send_membership_event(origin, content, Membership.JOIN) prev_state_ids = await context.get_prev_state_ids() state_ids = list(prev_state_ids.values()) @@ -567,7 +567,7 @@ async def on_make_leave_request( async def on_send_leave_request(self, origin: str, content: JsonDict) -> dict: logger.debug("on_send_leave_request: content: %s", content) - await self._on_send_membership_event(origin, content) + await self._on_send_membership_event(origin, content, Membership.LEAVE) return {} async def on_make_knock_request( @@ -668,12 +668,18 @@ async def on_send_knock_request( return {"knock_state_events": stripped_room_state} async def _on_send_membership_event( - self, origin: str, content: JsonDict + self, origin: str, content: JsonDict, membership_type: str ) -> EventContext: assert_params_in_dict(content, ["room_id"]) room_version = await self.store.get_room_version(content["room_id"]) event = event_from_pdu_json(content, room_version) + if event.type != EventTypes.Member or not event.is_state(): + raise SynapseError(400, "Not an m.room.member event", Codes.BAD_JSON) + + if event.content.get("membership") != membership_type: + raise SynapseError(400, "Not a %s event" % membership_type, Codes.BAD_JSON) + origin_host, _ = parse_server_name(origin) await self.check_server_matches_acl(origin_host, event.room_id) From 54eae70648832031f68993ccde0f22d5c9552ebc Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 21 Jun 2021 21:56:11 +0100 Subject: [PATCH 5/8] Also validate the room id matches --- synapse/federation/federation_server.py | 42 ++++++++++++++++++++----- synapse/federation/transport/server.py | 12 +++---- 2 files changed, 41 insertions(+), 13 deletions(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index dc4497b0db94..a7454c3905d6 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -538,13 +538,15 @@ async def on_invite_request( return {"event": ret_pdu.get_pdu_json(time_now)} async def on_send_join_request( - self, origin: str, content: JsonDict + self, origin: str, content: JsonDict, room_id: str ) -> Dict[str, Any]: - context = await self._on_send_membership_event(origin, content, Membership.JOIN) + context = await self._on_send_membership_event( + origin, content, Membership.JOIN, room_id + ) prev_state_ids = await context.get_prev_state_ids() state_ids = list(prev_state_ids.values()) - auth_chain = await self.store.get_auth_chain(content["room_id"], state_ids) + auth_chain = await self.store.get_auth_chain(room_id, state_ids) state = await self.store.get_events(state_ids) time_now = self._clock.time_msec() @@ -565,9 +567,11 @@ async def on_make_leave_request( time_now = self._clock.time_msec() return {"event": pdu.get_pdu_json(time_now), "room_version": room_version} - async def on_send_leave_request(self, origin: str, content: JsonDict) -> dict: + async def on_send_leave_request( + self, origin: str, content: JsonDict, room_id: str + ) -> dict: logger.debug("on_send_leave_request: content: %s", content) - await self._on_send_membership_event(origin, content, Membership.LEAVE) + await self._on_send_membership_event(origin, content, Membership.LEAVE, room_id) return {} async def on_make_knock_request( @@ -668,10 +672,34 @@ async def on_send_knock_request( return {"knock_state_events": stripped_room_state} async def _on_send_membership_event( - self, origin: str, content: JsonDict, membership_type: str + self, origin: str, content: JsonDict, membership_type: str, room_id: str ) -> EventContext: + """Handle an on_send_{join,leave} request + + Args: + origin: The (authenticated) requesting server + content: The body of the send_* request - a complete membership event + membership_type: The expected membership type (join or leave, depending + on the endpoint) + room_id: The room_id from the request, to be validated against the room_id + in the event + + Returns: + The context of the event after inserting it into the room graph. + + Raises: + SynapseError if there is a problem with the request, including things like + the room_id not matching or the event not being authorized. + """ assert_params_in_dict(content, ["room_id"]) - room_version = await self.store.get_room_version(content["room_id"]) + if content["room_id"] != room_id: + raise SynapseError( + 400, + "Room ID in body does not match that in request path", + Codes.BAD_JSON, + ) + + room_version = await self.store.get_room_version(room_id) event = event_from_pdu_json(content, room_version) if event.type != EventTypes.Member or not event.is_state(): diff --git a/synapse/federation/transport/server.py b/synapse/federation/transport/server.py index bed47f8abd5d..676fbd37508c 100644 --- a/synapse/federation/transport/server.py +++ b/synapse/federation/transport/server.py @@ -553,7 +553,7 @@ class FederationV1SendLeaveServlet(BaseFederationServerServlet): PATH = "/send_leave/(?P[^/]*)/(?P[^/]*)" async def on_PUT(self, origin, content, query, room_id, event_id): - content = await self.handler.on_send_leave_request(origin, content) + content = await self.handler.on_send_leave_request(origin, content, room_id) return 200, (200, content) @@ -563,7 +563,7 @@ class FederationV2SendLeaveServlet(BaseFederationServerServlet): PREFIX = FEDERATION_V2_PREFIX async def on_PUT(self, origin, content, query, room_id, event_id): - content = await self.handler.on_send_leave_request(origin, content) + content = await self.handler.on_send_leave_request(origin, content, room_id) return 200, content @@ -602,9 +602,9 @@ class FederationV1SendJoinServlet(BaseFederationServerServlet): PATH = "/send_join/(?P[^/]*)/(?P[^/]*)" async def on_PUT(self, origin, content, query, room_id, event_id): - # TODO(paul): assert that room_id/event_id parsed from path actually + # TODO(paul): assert that event_id parsed from path actually # match those given in content - content = await self.handler.on_send_join_request(origin, content) + content = await self.handler.on_send_join_request(origin, content, room_id) return 200, (200, content) @@ -614,9 +614,9 @@ class FederationV2SendJoinServlet(BaseFederationServerServlet): PREFIX = FEDERATION_V2_PREFIX async def on_PUT(self, origin, content, query, room_id, event_id): - # TODO(paul): assert that room_id/event_id parsed from path actually + # TODO(paul): assert that event_id parsed from path actually # match those given in content - content = await self.handler.on_send_join_request(origin, content) + content = await self.handler.on_send_join_request(origin, content, room_id) return 200, content From 904db5b6d27941254caa68f319b1fd92db89e391 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 21 Jun 2021 22:04:21 +0100 Subject: [PATCH 6/8] Use `_on_send_membership_event` to handle knock events --- synapse/federation/federation_server.py | 40 ++++++++++--------------- 1 file changed, 16 insertions(+), 24 deletions(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index a7454c3905d6..341965047a33 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -637,29 +637,9 @@ async def on_send_knock_request( Returns: The stripped room state. """ - logger.debug("on_send_knock_request: content: %s", content) - - room_version = await self.store.get_room_version(room_id) - - # Check that this room supports knocking as defined by its room version - if not room_version.msc2403_knocking: - raise SynapseError( - 403, - "This room version does not support knocking", - errcode=Codes.FORBIDDEN, - ) - - 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) - - logger.debug("on_send_knock_request: pdu sigs: %s", pdu.signatures) - - pdu = await self._check_sigs_and_hash(room_version, pdu) - - # Handle the event, and retrieve the EventContext - event_context = await self.handler.on_send_membership_event(origin, pdu) + event_context = await self._on_send_membership_event( + origin, content, Membership.KNOCK, room_id + ) # Retrieve stripped state events from the room and send them back to the remote # server. This will allow the remote server's clients to display information @@ -674,7 +654,10 @@ async def on_send_knock_request( async def _on_send_membership_event( self, origin: str, content: JsonDict, membership_type: str, room_id: str ) -> EventContext: - """Handle an on_send_{join,leave} request + """Handle an on_send_{join,leave,knock} request + + Does some preliminary validation before passing the request on to the + federation handler. Args: origin: The (authenticated) requesting server @@ -700,6 +683,14 @@ async def _on_send_membership_event( ) room_version = await self.store.get_room_version(room_id) + + if membership_type == Membership.KNOCK and not room_version.msc2403_knocking: + raise SynapseError( + 403, + "This room version does not support knocking", + errcode=Codes.FORBIDDEN, + ) + event = event_from_pdu_json(content, room_version) if event.type != EventTypes.Member or not event.is_state(): @@ -714,6 +705,7 @@ async def _on_send_membership_event( logger.debug("_on_send_membership_event: pdu sigs: %s", event.signatures) event = await self._check_sigs_and_hash(room_version, event) + return await self.handler.on_send_membership_event(origin, event) async def on_event_auth( From c25da76102c326e9de4c04c09ca695f45021ff75 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 21 Jun 2021 22:11:45 +0100 Subject: [PATCH 7/8] changelog --- changelog.d/10225.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/10225.feature diff --git a/changelog.d/10225.feature b/changelog.d/10225.feature new file mode 100644 index 000000000000..d16f66ffe9ee --- /dev/null +++ b/changelog.d/10225.feature @@ -0,0 +1 @@ +Improve validation on federation `send_{join,leave,knock}` endpoints. From 09aa2a15b9298f92918e728b6db82027c362511e Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Wed, 23 Jun 2021 16:45:20 +0100 Subject: [PATCH 8/8] Check that state_key matches sender --- synapse/handlers/federation.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 5a3e78879df3..12f3d853422a 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1978,6 +1978,9 @@ async def on_send_membership_event( ) raise SynapseError(403, "User not from origin", Codes.FORBIDDEN) + if event.sender != event.state_key: + raise SynapseError(400, "state_key and sender must match", Codes.BAD_JSON) + event.internal_metadata.outlier = False # Send this event on behalf of the other server.