From b800834351f3f15c9de4996b18149a7e8cae0c34 Mon Sep 17 00:00:00 2001 From: Vincent Breitmoser Date: Sat, 9 Jun 2018 22:50:29 +0200 Subject: [PATCH 01/10] add note that the affinity package is required for the cpu_affinity setting --- synapse/config/server.py | 2 ++ 1 file changed, 2 insertions(+) diff --git a/synapse/config/server.py b/synapse/config/server.py index 968ecd9ea0..1b8aac9b75 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -201,6 +201,8 @@ class ServerConfig(Config): # different cores. See # https://www.mirantis.com/blog/improve-performance-python-programs-restricting-single-cpu/. # + # This setting requires the affinity package to be installed! + # # cpu_affinity: 0xFFFFFFFF # Whether to serve a web client from the HTTP/HTTPS root resource. From 7417951117fb096d433b620264f2c034500e41d3 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 24 Jul 2018 13:46:39 +0100 Subject: [PATCH 02/10] Update ISSUE_TEMPLATE.md request backticks for logs --- .github/ISSUE_TEMPLATE.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.github/ISSUE_TEMPLATE.md b/.github/ISSUE_TEMPLATE.md index d2050a3e44..21acb3202a 100644 --- a/.github/ISSUE_TEMPLATE.md +++ b/.github/ISSUE_TEMPLATE.md @@ -27,8 +27,9 @@ Describe here the problem that you are experiencing, or the feature you are requ Describe how what happens differs from what you expected. -If you can identify any relevant log snippets from _homeserver.log_, please include -those here (please be careful to remove any personal or private data): + ### Version information From 0a65450d044fb580d789013dcdac48b10c930761 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 2 Aug 2018 11:53:52 +0100 Subject: [PATCH 03/10] Validation for events/rooms in fed requests When we get a federation request which refers to an event id, make sure that said event is in the room the caller claims it is in. (patch supplied by @turt2live) --- synapse/federation/federation_server.py | 1 + synapse/handlers/federation.py | 35 ++++++++++++++++++++++++- synapse/storage/event_federation.py | 29 ++++++++++++++++++++ 3 files changed, 64 insertions(+), 1 deletion(-) diff --git a/synapse/federation/federation_server.py b/synapse/federation/federation_server.py index 48f26db67c..10e71c78ce 100644 --- a/synapse/federation/federation_server.py +++ b/synapse/federation/federation_server.py @@ -425,6 +425,7 @@ class FederationServer(FederationBase): ret = yield self.handler.on_query_auth( origin, event_id, + room_id, signed_auth, content.get("rejects", []), content.get("missing", []), diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 20fb46fc89..12eeb7c4cd 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1349,6 +1349,9 @@ class FederationHandler(BaseHandler): def get_state_for_pdu(self, room_id, event_id): """Returns the state at the event. i.e. not including said event. """ + + yield self._verify_events_in_room([event_id], room_id) + state_groups = yield self.store.get_state_groups( room_id, [event_id] ) @@ -1391,6 +1394,9 @@ class FederationHandler(BaseHandler): def get_state_ids_for_pdu(self, room_id, event_id): """Returns the state at the event. i.e. not including said event. """ + + yield self._verify_events_in_room([event_id], room_id) + state_groups = yield self.store.get_state_groups_ids( room_id, [event_id] ) @@ -1420,6 +1426,8 @@ class FederationHandler(BaseHandler): if not in_room: raise AuthError(403, "Host not in room.") + yield self._verify_events_in_room(pdu_list, room_id) + events = yield self.store.get_backfill_events( room_id, pdu_list, @@ -1706,8 +1714,17 @@ class FederationHandler(BaseHandler): defer.returnValue(context) @defer.inlineCallbacks - def on_query_auth(self, origin, event_id, remote_auth_chain, rejects, + def on_query_auth(self, origin, event_id, room_id, remote_auth_chain, rejects, missing): + in_room = yield self.auth.check_host_in_room( + room_id, + origin + ) + if not in_room: + raise AuthError(403, "Host not in room.") + + yield self._verify_events_in_room([event_id], room_id) + # Just go through and process each event in `remote_auth_chain`. We # don't want to fall into the trap of `missing` being wrong. for e in remote_auth_chain: @@ -2368,3 +2385,19 @@ class FederationHandler(BaseHandler): ) if "valid" not in response or not response["valid"]: raise AuthError(403, "Third party certificate was invalid") + + @defer.inlineCallbacks + def _verify_events_in_room(self, pdu_ids, room_id): + """Checks whether the given PDU IDs are in the given room or not. + + Args: + pdu_ids (list): list of PDU IDs + room_id (str): the room ID that the PDUs should be in + + Raises: + AuthError: if one or more of the PDUs does not belong to the + given room. + """ + room_ids = yield self.store.get_room_ids_for_events(pdu_ids) + if len(room_ids) != 1 or room_ids[0] != room_id: + raise AuthError(403, "Events must belong to the given room") diff --git a/synapse/storage/event_federation.py b/synapse/storage/event_federation.py index 8d366d1b91..e860fe1a1e 100644 --- a/synapse/storage/event_federation.py +++ b/synapse/storage/event_federation.py @@ -295,6 +295,35 @@ class EventFederationWorkerStore(EventsWorkerStore, SignatureWorkerStore, get_forward_extremeties_for_room_txn ) + def get_room_ids_for_events(self, event_ids): + """Get a list of room IDs for which the given events belong. + + Args: + event_ids (list): the events to look up the room of + + Returns: + list, the room IDs for the events + """ + return self.runInteraction( + "get_room_ids_for_events", + self._get_room_ids_for_events, event_ids + ) + + def _get_room_ids_for_events(self, txn, event_ids): + logger.debug("_get_room_ids_for_events: %s", repr(event_ids)) + + base_sql = ( + "SELECT DISTINCT room_id FROM events" + " WHERE event_id IN (%s)" + ) + + txn.execute( + base_sql % (",".join(["?"] * len(event_ids)),), + event_ids + ) + + return [r[0] for r in txn] + def get_backfill_events(self, room_id, event_list, limit): """Get a list of Events for a given topic that occurred before (and including) the events in event_list. Return a list of max size `limit` From 14fa9d4d92eaa242d44a2823bbd9908be2f02d81 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 2 Aug 2018 13:23:48 +0100 Subject: [PATCH 04/10] Avoid extra db lookups Since we're about to look up the events themselves anyway, we can skip the extra db queries here. --- synapse/handlers/federation.py | 38 +++++++++-------------------- synapse/storage/event_federation.py | 30 +---------------------- synapse/storage/events_worker.py | 20 ++++++++++----- 3 files changed, 26 insertions(+), 62 deletions(-) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 12eeb7c4cd..60391d07c4 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1350,7 +1350,9 @@ class FederationHandler(BaseHandler): """Returns the state at the event. i.e. not including said event. """ - yield self._verify_events_in_room([event_id], room_id) + event = yield self.store.get_event( + event_id, allow_none=False, check_room_id=room_id, + ) state_groups = yield self.store.get_state_groups( room_id, [event_id] @@ -1362,8 +1364,7 @@ class FederationHandler(BaseHandler): (e.type, e.state_key): e for e in state } - event = yield self.store.get_event(event_id) - if event and event.is_state(): + if event.is_state(): # Get previous state if "replaces_state" in event.unsigned: prev_id = event.unsigned["replaces_state"] @@ -1394,8 +1395,9 @@ class FederationHandler(BaseHandler): def get_state_ids_for_pdu(self, room_id, event_id): """Returns the state at the event. i.e. not including said event. """ - - yield self._verify_events_in_room([event_id], room_id) + event = yield self.store.get_event( + event_id, allow_none=False, check_room_id=room_id, + ) state_groups = yield self.store.get_state_groups_ids( room_id, [event_id] @@ -1405,8 +1407,7 @@ class FederationHandler(BaseHandler): _, state = state_groups.items().pop() results = state - event = yield self.store.get_event(event_id) - if event and event.is_state(): + if event.is_state(): # Get previous state if "replaces_state" in event.unsigned: prev_id = event.unsigned["replaces_state"] @@ -1426,8 +1427,6 @@ class FederationHandler(BaseHandler): if not in_room: raise AuthError(403, "Host not in room.") - yield self._verify_events_in_room(pdu_list, room_id) - events = yield self.store.get_backfill_events( room_id, pdu_list, @@ -1723,7 +1722,9 @@ class FederationHandler(BaseHandler): if not in_room: raise AuthError(403, "Host not in room.") - yield self._verify_events_in_room([event_id], room_id) + event = yield self.store.get_event( + event_id, allow_none=False, check_room_id=room_id + ) # Just go through and process each event in `remote_auth_chain`. We # don't want to fall into the trap of `missing` being wrong. @@ -1734,7 +1735,6 @@ class FederationHandler(BaseHandler): pass # Now get the current auth_chain for the event. - event = yield self.store.get_event(event_id) local_auth_chain = yield self.store.get_auth_chain( [auth_id for auth_id, _ in event.auth_events], include_given=True @@ -2385,19 +2385,3 @@ class FederationHandler(BaseHandler): ) if "valid" not in response or not response["valid"]: raise AuthError(403, "Third party certificate was invalid") - - @defer.inlineCallbacks - def _verify_events_in_room(self, pdu_ids, room_id): - """Checks whether the given PDU IDs are in the given room or not. - - Args: - pdu_ids (list): list of PDU IDs - room_id (str): the room ID that the PDUs should be in - - Raises: - AuthError: if one or more of the PDUs does not belong to the - given room. - """ - room_ids = yield self.store.get_room_ids_for_events(pdu_ids) - if len(room_ids) != 1 or room_ids[0] != room_id: - raise AuthError(403, "Events must belong to the given room") diff --git a/synapse/storage/event_federation.py b/synapse/storage/event_federation.py index e860fe1a1e..7cd77c1c29 100644 --- a/synapse/storage/event_federation.py +++ b/synapse/storage/event_federation.py @@ -295,35 +295,6 @@ class EventFederationWorkerStore(EventsWorkerStore, SignatureWorkerStore, get_forward_extremeties_for_room_txn ) - def get_room_ids_for_events(self, event_ids): - """Get a list of room IDs for which the given events belong. - - Args: - event_ids (list): the events to look up the room of - - Returns: - list, the room IDs for the events - """ - return self.runInteraction( - "get_room_ids_for_events", - self._get_room_ids_for_events, event_ids - ) - - def _get_room_ids_for_events(self, txn, event_ids): - logger.debug("_get_room_ids_for_events: %s", repr(event_ids)) - - base_sql = ( - "SELECT DISTINCT room_id FROM events" - " WHERE event_id IN (%s)" - ) - - txn.execute( - base_sql % (",".join(["?"] * len(event_ids)),), - event_ids - ) - - return [r[0] for r in txn] - def get_backfill_events(self, room_id, event_list, limit): """Get a list of Events for a given topic that occurred before (and including) the events in event_list. Return a list of max size `limit` @@ -372,6 +343,7 @@ class EventFederationWorkerStore(EventsWorkerStore, SignatureWorkerStore, table="events", keyvalues={ "event_id": event_id, + "room_id": room_id, }, retcol="depth", allow_none=True, diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index 67433606c6..6b8a8e908f 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -19,7 +19,7 @@ from canonicaljson import json from twisted.internet import defer -from synapse.api.errors import SynapseError +from synapse.api.errors import NotFoundError # these are only included to make the type annotations work from synapse.events import EventBase # noqa: F401 from synapse.events import FrozenEvent @@ -76,7 +76,7 @@ class EventsWorkerStore(SQLBaseStore): @defer.inlineCallbacks def get_event(self, event_id, check_redacted=True, get_prev_content=False, allow_rejected=False, - allow_none=False): + allow_none=False, check_room_id=None): """Get an event from the database by event_id. Args: @@ -87,7 +87,9 @@ class EventsWorkerStore(SQLBaseStore): include the previous states content in the unsigned field. allow_rejected (bool): If True return rejected events. allow_none (bool): If True, return None if no event found, if - False throw an exception. + False throw a NotFoundError + check_room_id (str|None): if not None, check the room of the found event. + If there is a mismatch, behave as per allow_none. Returns: Deferred : A FrozenEvent. @@ -99,10 +101,16 @@ class EventsWorkerStore(SQLBaseStore): allow_rejected=allow_rejected, ) - if not events and not allow_none: - raise SynapseError(404, "Could not find event %s" % (event_id,)) + event = events[0] if events else None - defer.returnValue(events[0] if events else None) + if event is not None and check_room_id is not None: + if event.room_id != check_room_id: + event = None + + if event is None and not allow_none: + raise NotFoundError("Could not find event %s" % (event_id,)) + + defer.returnValue(event) @defer.inlineCallbacks def get_events(self, event_ids, check_redacted=True, From a0134042922b7d1a128150e1be0d7213c1e9e138 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 2 Aug 2018 14:00:29 +0100 Subject: [PATCH 05/10] changelog --- changelog.d/3641.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/3641.bugfix diff --git a/changelog.d/3641.bugfix b/changelog.d/3641.bugfix new file mode 100644 index 0000000000..02181975c0 --- /dev/null +++ b/changelog.d/3641.bugfix @@ -0,0 +1 @@ +Fix a potential event disclosure issue \ No newline at end of file From 0bf5ec0db700f189ba36360ea8424d9761658905 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 2 Aug 2018 15:03:27 +0100 Subject: [PATCH 06/10] Check room visibility for /event/ requests Make sure that the user has permission to view the requeseted event for /event/{eventId} and /room/{roomId}/event/{eventId} requests. Also check that the event is in the given room for /room/{roomId}/event/{eventId}, for sanity. --- synapse/handlers/events.py | 25 +++++++++++++++++++++---- synapse/rest/client/v1/events.py | 2 +- synapse/rest/client/v1/room.py | 2 +- 3 files changed, 23 insertions(+), 6 deletions(-) diff --git a/synapse/handlers/events.py b/synapse/handlers/events.py index c3f2d7feff..f772e62c28 100644 --- a/synapse/handlers/events.py +++ b/synapse/handlers/events.py @@ -19,10 +19,12 @@ import random from twisted.internet import defer from synapse.api.constants import EventTypes, Membership +from synapse.api.errors import AuthError from synapse.events import EventBase from synapse.events.utils import serialize_event from synapse.types import UserID from synapse.util.logutils import log_function +from synapse.visibility import filter_events_for_client from ._base import BaseHandler @@ -129,11 +131,13 @@ class EventStreamHandler(BaseHandler): class EventHandler(BaseHandler): @defer.inlineCallbacks - def get_event(self, user, event_id): + def get_event(self, user, room_id, event_id): """Retrieve a single specified event. Args: user (synapse.types.UserID): The user requesting the event + room_id (str|None): The expected room id. We'll return None if the + event's room does not match. event_id (str): The event ID to obtain. Returns: dict: An event, or None if there is no event matching this ID. @@ -142,13 +146,26 @@ class EventHandler(BaseHandler): AuthError if the user does not have the rights to inspect this event. """ - event = yield self.store.get_event(event_id) + event = yield self.store.get_event(event_id, check_room_id=room_id) if not event: defer.returnValue(None) return - if hasattr(event, "room_id"): - yield self.auth.check_joined_room(event.room_id, user.to_string()) + users = yield self.store.get_users_in_room(event.room_id) + is_peeking = user.to_string() not in users + + filtered = yield filter_events_for_client( + self.store, + user.to_string(), + [event], + is_peeking=is_peeking + ) + + if not filtered: + raise AuthError( + 403, + "You don't have permission to access that event." + ) defer.returnValue(event) diff --git a/synapse/rest/client/v1/events.py b/synapse/rest/client/v1/events.py index b70c9c2806..0f3a2e8b51 100644 --- a/synapse/rest/client/v1/events.py +++ b/synapse/rest/client/v1/events.py @@ -88,7 +88,7 @@ class EventRestServlet(ClientV1RestServlet): @defer.inlineCallbacks def on_GET(self, request, event_id): requester = yield self.auth.get_user_by_req(request) - event = yield self.event_handler.get_event(requester.user, event_id) + event = yield self.event_handler.get_event(requester.user, None, event_id) time_now = self.clock.time_msec() if event: diff --git a/synapse/rest/client/v1/room.py b/synapse/rest/client/v1/room.py index 3d62447854..2a679ac830 100644 --- a/synapse/rest/client/v1/room.py +++ b/synapse/rest/client/v1/room.py @@ -508,7 +508,7 @@ class RoomEventServlet(ClientV1RestServlet): @defer.inlineCallbacks def on_GET(self, request, room_id, event_id): requester = yield self.auth.get_user_by_req(request) - event = yield self.event_handler.get_event(requester.user, event_id) + event = yield self.event_handler.get_event(requester.user, room_id, event_id) time_now = self.clock.time_msec() if event: From 8cefc690c9206c8f0936b4cd6a5b0823d9c7beb6 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 2 Aug 2018 15:11:19 +0100 Subject: [PATCH 07/10] changelogs --- changelog.d/3641.bugfix | 2 +- changelog.d/3642.bugfix | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/3642.bugfix diff --git a/changelog.d/3641.bugfix b/changelog.d/3641.bugfix index 02181975c0..149349baa2 100644 --- a/changelog.d/3641.bugfix +++ b/changelog.d/3641.bugfix @@ -1 +1 @@ -Fix a potential event disclosure issue \ No newline at end of file +Fix a potential issue where servers could request events for rooms they have not joined. diff --git a/changelog.d/3642.bugfix b/changelog.d/3642.bugfix new file mode 100644 index 0000000000..e2e9b209de --- /dev/null +++ b/changelog.d/3642.bugfix @@ -0,0 +1 @@ +Fix a potential issue where users could see events in private joins before they joined From 14a4e7d5a4e3c90195a907aa9677c058f2932cdb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 2 Aug 2018 15:31:04 +0100 Subject: [PATCH 08/10] Prepare 0.33.1 --- CHANGES.rst | 10 ++++++++++ changelog.d/3641.bugfix | 1 - changelog.d/3642.bugfix | 1 - synapse/__init__.py | 2 +- 4 files changed, 11 insertions(+), 3 deletions(-) delete mode 100644 changelog.d/3641.bugfix delete mode 100644 changelog.d/3642.bugfix diff --git a/CHANGES.rst b/CHANGES.rst index da6e0eb195..dff195be6e 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,3 +1,13 @@ +Synapse 0.33.1 (2018-08-02) +=========================== + +Bugfixes +-------- + +- Fix a potential issue where servers could request events for rooms they have not joined. (`#3641 `_) +- Fix a potential issue where users could see events in private joins before they joined (`#3642 `_) + + Synapse 0.33.0 (2018-07-19) =========================== diff --git a/changelog.d/3641.bugfix b/changelog.d/3641.bugfix deleted file mode 100644 index 149349baa2..0000000000 --- a/changelog.d/3641.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix a potential issue where servers could request events for rooms they have not joined. diff --git a/changelog.d/3642.bugfix b/changelog.d/3642.bugfix deleted file mode 100644 index e2e9b209de..0000000000 --- a/changelog.d/3642.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix a potential issue where users could see events in private joins before they joined diff --git a/synapse/__init__.py b/synapse/__init__.py index 5c0f2f83aa..1810cb6fcd 100644 --- a/synapse/__init__.py +++ b/synapse/__init__.py @@ -17,4 +17,4 @@ """ This is a reference implementation of a Matrix home server. """ -__version__ = "0.33.0" +__version__ = "0.33.1" From db1f33fb363e827d0b9225a9544cb512bbfa2886 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 2 Aug 2018 15:33:53 +0100 Subject: [PATCH 09/10] fix changelog typos --- CHANGES.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index dff195be6e..60be8e7b5d 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -5,7 +5,7 @@ Bugfixes -------- - Fix a potential issue where servers could request events for rooms they have not joined. (`#3641 `_) -- Fix a potential issue where users could see events in private joins before they joined (`#3642 `_) +- Fix a potential issue where users could see events in private rooms before they joined. (`#3642 `_) Synapse 0.33.0 (2018-07-19) From c2a83349f026c964302c6ad50a402c4cd664367f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 2 Aug 2018 15:35:42 +0100 Subject: [PATCH 10/10] changelog: this is a security release --- CHANGES.rst | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index 60be8e7b5d..a1cc88fe21 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -1,8 +1,8 @@ Synapse 0.33.1 (2018-08-02) =========================== -Bugfixes --------- +SECURITY FIXES +-------------- - Fix a potential issue where servers could request events for rooms they have not joined. (`#3641 `_) - Fix a potential issue where users could see events in private rooms before they joined. (`#3642 `_)