From a47636c570c80928af7652d70073496f602511bc Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Tue, 7 Jun 2022 11:53:47 +0100 Subject: Prevent local quarantined media from being claimed by media retention (#12972) --- synapse/rest/admin/media.py | 8 +-- synapse/rest/media/v1/media_repository.py | 22 +++++-- synapse/storage/databases/main/media_repository.py | 68 ++++++++++++++++++++-- 3 files changed, 82 insertions(+), 16 deletions(-) (limited to 'synapse') diff --git a/synapse/rest/admin/media.py b/synapse/rest/admin/media.py index 8ca57bdb28..19d4a008e8 100644 --- a/synapse/rest/admin/media.py +++ b/synapse/rest/admin/media.py @@ -83,7 +83,7 @@ class QuarantineMediaByUser(RestServlet): requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) - logging.info("Quarantining local media by user: %s", user_id) + logging.info("Quarantining media by user: %s", user_id) # Quarantine all media this user has uploaded num_quarantined = await self.store.quarantine_media_ids_by_user( @@ -112,7 +112,7 @@ class QuarantineMediaByID(RestServlet): requester = await self.auth.get_user_by_req(request) await assert_user_is_admin(self.auth, requester.user) - logging.info("Quarantining local media by ID: %s/%s", server_name, media_id) + logging.info("Quarantining media by ID: %s/%s", server_name, media_id) # Quarantine this media id await self.store.quarantine_media_by_id( @@ -140,9 +140,7 @@ class UnquarantineMediaByID(RestServlet): ) -> Tuple[int, JsonDict]: await assert_requester_is_admin(self.auth, request) - logging.info( - "Remove from quarantine local media by ID: %s/%s", server_name, media_id - ) + logging.info("Remove from quarantine media by ID: %s/%s", server_name, media_id) # Remove from quarantine this media id await self.store.quarantine_media_by_id(server_name, media_id, None) diff --git a/synapse/rest/media/v1/media_repository.py b/synapse/rest/media/v1/media_repository.py index a551458a9f..7435fd9130 100644 --- a/synapse/rest/media/v1/media_repository.py +++ b/synapse/rest/media/v1/media_repository.py @@ -919,10 +919,14 @@ class MediaRepository: await self.delete_old_local_media( before_ts=local_media_threshold_timestamp_ms, keep_profiles=True, + delete_quarantined_media=False, + delete_protected_media=False, ) async def delete_old_remote_media(self, before_ts: int) -> Dict[str, int]: - old_media = await self.store.get_remote_media_before(before_ts) + old_media = await self.store.get_remote_media_ids( + before_ts, include_quarantined_media=False + ) deleted = 0 @@ -975,6 +979,8 @@ class MediaRepository: before_ts: int, size_gt: int = 0, keep_profiles: bool = True, + delete_quarantined_media: bool = False, + delete_protected_media: bool = False, ) -> Tuple[List[str], int]: """ Delete local or remote media from this server by size and timestamp. Removes @@ -982,18 +988,22 @@ class MediaRepository: Args: before_ts: Unix timestamp in ms. - Files that were last used before this timestamp will be deleted - size_gt: Size of the media in bytes. Files that are larger will be deleted + Files that were last used before this timestamp will be deleted. + size_gt: Size of the media in bytes. Files that are larger will be deleted. keep_profiles: Switch to delete also files that are still used in image data - (e.g user profile, room avatar) - If false these files will be deleted + (e.g user profile, room avatar). If false these files will be deleted. + delete_quarantined_media: If True, media marked as quarantined will be deleted. + delete_protected_media: If True, media marked as protected will be deleted. + Returns: A tuple of (list of deleted media IDs, total deleted media IDs). """ - old_media = await self.store.get_local_media_before( + old_media = await self.store.get_local_media_ids( before_ts, size_gt, keep_profiles, + include_quarantined_media=delete_quarantined_media, + include_protected_media=delete_protected_media, ) return await self._remove_local_media_from_disk(old_media) diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index deffdc19ce..3c585c555a 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -251,12 +251,36 @@ class MediaRepositoryStore(MediaRepositoryBackgroundUpdateStore): "get_local_media_by_user_paginate_txn", get_local_media_by_user_paginate_txn ) - async def get_local_media_before( + async def get_local_media_ids( self, before_ts: int, size_gt: int, keep_profiles: bool, + include_quarantined_media: bool, + include_protected_media: bool, ) -> List[str]: + """ + Retrieve a list of media IDs from the local media store. + + Args: + before_ts: Only retrieve IDs from media that was either last accessed + (or if never accessed, created) before the given UNIX timestamp in ms. + size_gt: Only retrieve IDs from media that has a size (in bytes) greater than + the given integer. + keep_profiles: If True, exclude media IDs from the results that are used in the + following situations: + * global profile user avatar + * per-room profile user avatar + * room avatar + * a user's avatar in the user directory + include_quarantined_media: If False, exclude media IDs from the results that have + been marked as quarantined. + include_protected_media: If False, exclude media IDs from the results that have + been marked as protected from quarantine. + + Returns: + A list of local media IDs. + """ # to find files that have never been accessed (last_access_ts IS NULL) # compare with `created_ts` @@ -294,12 +318,24 @@ class MediaRepositoryStore(MediaRepositoryBackgroundUpdateStore): ) sql += sql_keep - def _get_local_media_before_txn(txn: LoggingTransaction) -> List[str]: + if include_quarantined_media is False: + # Do not include media that has been quarantined + sql += """ + AND quarantined_by IS NULL + """ + + if include_protected_media is False: + # Do not include media that has been protected from quarantine + sql += """ + AND safe_from_quarantine = false + """ + + def _get_local_media_ids_txn(txn: LoggingTransaction) -> List[str]: txn.execute(sql, (before_ts, before_ts, size_gt)) return [row[0] for row in txn] return await self.db_pool.runInteraction( - "get_local_media_before", _get_local_media_before_txn + "get_local_media_ids", _get_local_media_ids_txn ) async def store_local_media( @@ -599,15 +635,37 @@ class MediaRepositoryStore(MediaRepositoryBackgroundUpdateStore): desc="store_remote_media_thumbnail", ) - async def get_remote_media_before(self, before_ts: int) -> List[Dict[str, str]]: + async def get_remote_media_ids( + self, before_ts: int, include_quarantined_media: bool + ) -> List[Dict[str, str]]: + """ + Retrieve a list of server name, media ID tuples from the remote media cache. + + Args: + before_ts: Only retrieve IDs from media that was either last accessed + (or if never accessed, created) before the given UNIX timestamp in ms. + include_quarantined_media: If False, exclude media IDs from the results that have + been marked as quarantined. + + Returns: + A list of tuples containing: + * The server name of homeserver where the media originates from, + * The ID of the media. + """ sql = ( "SELECT media_origin, media_id, filesystem_id" " FROM remote_media_cache" " WHERE last_access_ts < ?" ) + if include_quarantined_media is False: + # Only include media that has not been quarantined + sql += """ + AND quarantined_by IS NULL + """ + return await self.db_pool.execute( - "get_remote_media_before", self.db_pool.cursor_to_dict, sql, before_ts + "get_remote_media_ids", self.db_pool.cursor_to_dict, sql, before_ts ) async def delete_remote_media(self, media_origin: str, media_id: str) -> None: -- cgit 1.5.1 From c51f5b9592a8f7e44c132e7e63b96f0af34dfef3 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Tue, 7 Jun 2022 12:28:08 +0100 Subject: Prevent breaking old sqlite's when media retention is enabled (#12977) --- changelog.d/12977.feature | 1 + synapse/storage/databases/main/media_repository.py | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) create mode 100644 changelog.d/12977.feature (limited to 'synapse') diff --git a/changelog.d/12977.feature b/changelog.d/12977.feature new file mode 100644 index 0000000000..3c73363d28 --- /dev/null +++ b/changelog.d/12977.feature @@ -0,0 +1 @@ +Add new `media_retention` options to the homeserver config for routinely cleaning up non-recently accessed media. \ No newline at end of file diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index 3c585c555a..d028be16de 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -327,7 +327,7 @@ class MediaRepositoryStore(MediaRepositoryBackgroundUpdateStore): if include_protected_media is False: # Do not include media that has been protected from quarantine sql += """ - AND safe_from_quarantine = false + AND NOT safe_from_quarantine """ def _get_local_media_ids_txn(txn: LoggingTransaction) -> List[str]: -- cgit 1.5.1 From 9dc3293e0b3a5cbf6fcc4a0cef7386b531190882 Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 7 Jun 2022 07:43:35 -0400 Subject: Consolidate the logic of delete_device/delete_devices. (#12970) By always using delete_devices and sometimes passing a list with a single device ID. Previously these methods had gotten out of sync with each other and it seems there's little benefit to the single-device variant. --- changelog.d/12970.misc | 1 + synapse/handlers/device.py | 33 ++----------------------------- synapse/module_api/__init__.py | 2 +- synapse/rest/admin/devices.py | 2 +- synapse/rest/client/devices.py | 4 +++- synapse/rest/client/logout.py | 4 ++-- synapse/storage/databases/main/devices.py | 10 ---------- tests/handlers/test_device.py | 4 ++-- 8 files changed, 12 insertions(+), 48 deletions(-) create mode 100644 changelog.d/12970.misc (limited to 'synapse') diff --git a/changelog.d/12970.misc b/changelog.d/12970.misc new file mode 100644 index 0000000000..8f874aa07b --- /dev/null +++ b/changelog.d/12970.misc @@ -0,0 +1 @@ +Remove the `delete_device` method and always call `delete_devices`. diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index a0cbeedc30..b79c551703 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -397,35 +397,6 @@ class DeviceHandler(DeviceWorkerHandler): for user_id, user_devices in devices.items(): await self.delete_devices(user_id, user_devices) - @trace - async def delete_device(self, user_id: str, device_id: str) -> None: - """Delete the given device - - Args: - user_id: The user to delete the device from. - device_id: The device to delete. - """ - - try: - await self.store.delete_device(user_id, device_id) - except errors.StoreError as e: - if e.code == 404: - # no match - set_tag("error", True) - log_kv( - {"reason": "User doesn't have device id.", "device_id": device_id} - ) - else: - raise - - await self._auth_handler.delete_access_tokens_for_user( - user_id, device_id=device_id - ) - - await self.store.delete_e2e_keys_by_device(user_id=user_id, device_id=device_id) - - await self.notify_device_update(user_id, [device_id]) - @trace async def delete_all_devices_for_user( self, user_id: str, except_device_id: Optional[str] = None @@ -591,7 +562,7 @@ class DeviceHandler(DeviceWorkerHandler): user_id, device_id, device_data ) if old_device_id is not None: - await self.delete_device(user_id, old_device_id) + await self.delete_devices(user_id, [old_device_id]) return device_id async def get_dehydrated_device( @@ -638,7 +609,7 @@ class DeviceHandler(DeviceWorkerHandler): await self.store.update_device(user_id, device_id, old_device["display_name"]) # can't call self.delete_device because that will clobber the # access token so call the storage layer directly - await self.store.delete_device(user_id, old_device_id) + await self.store.delete_devices(user_id, [old_device_id]) await self.store.delete_e2e_keys_by_device( user_id=user_id, device_id=old_device_id ) diff --git a/synapse/module_api/__init__.py b/synapse/module_api/__init__.py index a8ad575fcd..30b2aeffdd 100644 --- a/synapse/module_api/__init__.py +++ b/synapse/module_api/__init__.py @@ -799,7 +799,7 @@ class ModuleApi: if device_id: # delete the device, which will also delete its access tokens yield defer.ensureDeferred( - self._hs.get_device_handler().delete_device(user_id, device_id) + self._hs.get_device_handler().delete_devices(user_id, [device_id]) ) else: # no associated device. Just delete the access token. diff --git a/synapse/rest/admin/devices.py b/synapse/rest/admin/devices.py index cef46ba0dd..d934880102 100644 --- a/synapse/rest/admin/devices.py +++ b/synapse/rest/admin/devices.py @@ -80,7 +80,7 @@ class DeviceRestServlet(RestServlet): if u is None: raise NotFoundError("Unknown user") - await self.device_handler.delete_device(target_user.to_string(), device_id) + await self.device_handler.delete_devices(target_user.to_string(), [device_id]) return HTTPStatus.OK, {} async def on_PUT( diff --git a/synapse/rest/client/devices.py b/synapse/rest/client/devices.py index ad6fd6492b..6fab102437 100644 --- a/synapse/rest/client/devices.py +++ b/synapse/rest/client/devices.py @@ -147,7 +147,9 @@ class DeviceRestServlet(RestServlet): can_skip_ui_auth=True, ) - await self.device_handler.delete_device(requester.user.to_string(), device_id) + await self.device_handler.delete_devices( + requester.user.to_string(), [device_id] + ) return 200, {} async def on_PUT( diff --git a/synapse/rest/client/logout.py b/synapse/rest/client/logout.py index 193a6951b9..23dfa4518f 100644 --- a/synapse/rest/client/logout.py +++ b/synapse/rest/client/logout.py @@ -45,8 +45,8 @@ class LogoutRestServlet(RestServlet): access_token = self.auth.get_access_token_from_request(request) await self._auth_handler.delete_access_token(access_token) else: - await self._device_handler.delete_device( - requester.user.to_string(), requester.device_id + await self._device_handler.delete_devices( + requester.user.to_string(), [requester.device_id] ) return 200, {} diff --git a/synapse/storage/databases/main/devices.py b/synapse/storage/databases/main/devices.py index d900064c07..71e7863dd8 100644 --- a/synapse/storage/databases/main/devices.py +++ b/synapse/storage/databases/main/devices.py @@ -1433,16 +1433,6 @@ class DeviceStore(DeviceWorkerStore, DeviceBackgroundUpdateStore): ) raise StoreError(500, "Problem storing device.") - async def delete_device(self, user_id: str, device_id: str) -> None: - """Delete a device and its device_inbox. - - Args: - user_id: The ID of the user which owns the device - device_id: The ID of the device to delete - """ - - await self.delete_devices(user_id, [device_id]) - async def delete_devices(self, user_id: str, device_ids: List[str]) -> None: """Deletes several devices. diff --git a/tests/handlers/test_device.py b/tests/handlers/test_device.py index 01ea7d2a42..b8b465d35b 100644 --- a/tests/handlers/test_device.py +++ b/tests/handlers/test_device.py @@ -154,7 +154,7 @@ class DeviceTestCase(unittest.HomeserverTestCase): self._record_users() # delete the device - self.get_success(self.handler.delete_device(user1, "abc")) + self.get_success(self.handler.delete_devices(user1, ["abc"])) # check the device was deleted self.get_failure(self.handler.get_device(user1, "abc"), NotFoundError) @@ -179,7 +179,7 @@ class DeviceTestCase(unittest.HomeserverTestCase): ) # delete the device - self.get_success(self.handler.delete_device(user1, "abc")) + self.get_success(self.handler.delete_devices(user1, ["abc"])) # check that the device_inbox was deleted res = self.get_success( -- cgit 1.5.1 From d2fd7f7b5c6adabd1cb323a2107a16bbb3b4506d Mon Sep 17 00:00:00 2001 From: Patrick Cloke Date: Tue, 7 Jun 2022 07:44:31 -0400 Subject: Fix a stale comment in get_room_version_id_txn. (#12969) --- changelog.d/12969.misc | 1 + synapse/storage/databases/main/state.py | 7 +------ 2 files changed, 2 insertions(+), 6 deletions(-) create mode 100644 changelog.d/12969.misc (limited to 'synapse') diff --git a/changelog.d/12969.misc b/changelog.d/12969.misc new file mode 100644 index 0000000000..05de7ce839 --- /dev/null +++ b/changelog.d/12969.misc @@ -0,0 +1 @@ +Fix an inaccurate comment. diff --git a/synapse/storage/databases/main/state.py b/synapse/storage/databases/main/state.py index bdd00273cd..5e6efbd0fc 100644 --- a/synapse/storage/databases/main/state.py +++ b/synapse/storage/databases/main/state.py @@ -127,13 +127,8 @@ class StateGroupWorkerStore(EventsWorkerStore, SQLBaseStore): NotFoundError: if the room is unknown """ - # First we try looking up room version from the database, but for old - # rooms we might not have added the room version to it yet so we fall - # back to previous behaviour and look in current state events. - # # We really should have an entry in the rooms table for every room we - # care about, but let's be a bit paranoid (at least while the background - # update is happening) to avoid breaking existing rooms. + # care about, but let's be a bit paranoid. room_version = self.db_pool.simple_select_one_onecol_txn( txn, table="rooms", -- cgit 1.5.1 From a7e506ddee16919cf543370899c756ec2d6685a8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 7 Jun 2022 16:35:56 +0300 Subject: Reduce amount of state we pull out when attempting to send catchup PDUs. (#12963) * Don't pull out state for catchup * Newsfile * Merge newsfile --- changelog.d/12963.misc | 1 + synapse/federation/sender/per_destination_queue.py | 31 ++++++++++++++-------- 2 files changed, 21 insertions(+), 11 deletions(-) create mode 100644 changelog.d/12963.misc (limited to 'synapse') diff --git a/changelog.d/12963.misc b/changelog.d/12963.misc new file mode 100644 index 0000000000..d57e1aca6b --- /dev/null +++ b/changelog.d/12963.misc @@ -0,0 +1 @@ +Reduce the amount of state we pull from the DB. diff --git a/synapse/federation/sender/per_destination_queue.py b/synapse/federation/sender/per_destination_queue.py index 333ca9a97f..41d8b937af 100644 --- a/synapse/federation/sender/per_destination_queue.py +++ b/synapse/federation/sender/per_destination_queue.py @@ -37,6 +37,7 @@ from synapse.metrics import sent_transactions_counter from synapse.metrics.background_process_metrics import run_as_background_process from synapse.types import ReadReceipt from synapse.util.retryutils import NotRetryingDestination, get_retry_limiter +from synapse.visibility import filter_events_for_server if TYPE_CHECKING: import synapse.server @@ -77,6 +78,7 @@ class PerDestinationQueue: ): self._server_name = hs.hostname self._clock = hs.get_clock() + self._storage_controllers = hs.get_storage_controllers() self._store = hs.get_datastores().main self._transaction_manager = transaction_manager self._instance_name = hs.get_instance_name() @@ -442,6 +444,12 @@ class PerDestinationQueue: "This should not happen." % event_ids ) + logger.info( + "Catching up destination %s with %d PDUs", + self._destination, + len(catchup_pdus), + ) + # We send transactions with events from one room only, as its likely # that the remote will have to do additional processing, which may # take some time. It's better to give it small amounts of work @@ -487,19 +495,20 @@ class PerDestinationQueue: ): continue - # Filter out events where the server is not in the room, - # e.g. it may have left/been kicked. *Ideally* we'd pull - # out the kick and send that, but it's a rare edge case - # so we don't bother for now (the server that sent the - # kick should send it out if its online). - hosts = await self._state.get_hosts_in_room_at_events( - p.room_id, [p.event_id] - ) - if self._destination not in hosts: - continue - new_pdus.append(p) + # Filter out events where the server is not in the room, + # e.g. it may have left/been kicked. *Ideally* we'd pull + # out the kick and send that, but it's a rare edge case + # so we don't bother for now (the server that sent the + # kick should send it out if its online). + new_pdus = await filter_events_for_server( + self._storage_controllers, + self._destination, + new_pdus, + redact=False, + ) + # If we've filtered out all the extremities, fall back to # sending the original event. This should ensure that the # server gets at least some of missed events (especially if -- cgit 1.5.1 From f30bcbd84a651de59777b2a749850f6ca56ce3f0 Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 7 Jun 2022 15:24:11 +0100 Subject: Fix Synapse git info missing in version strings (#12973) --- changelog.d/12973.bugfix | 1 + poetry.lock | 10 +++++----- pyproject.toml | 2 +- synapse/__init__.py | 6 +++--- synapse/_scripts/synapse_port_db.py | 7 ++----- synapse/_scripts/update_synapse_database.py | 5 ++--- synapse/app/_base.py | 4 ++-- synapse/app/admin_cmd.py | 5 ++--- synapse/app/generic_worker.py | 5 ++--- synapse/app/homeserver.py | 6 ++---- synapse/config/logger.py | 4 ++-- synapse/federation/transport/server/federation.py | 4 ++-- synapse/metrics/__init__.py | 4 ++-- synapse/rest/admin/__init__.py | 5 ++--- synapse/util/__init__.py | 6 ++++++ 15 files changed, 36 insertions(+), 38 deletions(-) create mode 100644 changelog.d/12973.bugfix (limited to 'synapse') diff --git a/changelog.d/12973.bugfix b/changelog.d/12973.bugfix new file mode 100644 index 0000000000..1bf45854ff --- /dev/null +++ b/changelog.d/12973.bugfix @@ -0,0 +1 @@ +Fix a bug introduced in Synapse 1.58 where Synapse would not report full version information when installed from a git checkout. This is a best-effort affair and not guaranteed to be stable. diff --git a/poetry.lock b/poetry.lock index 7c561e3182..8a54a939fe 100644 --- a/poetry.lock +++ b/poetry.lock @@ -524,7 +524,7 @@ python-versions = ">=3.7" [[package]] name = "matrix-common" -version = "1.1.0" +version = "1.2.1" description = "Common utilities for Synapse, Sydent and Sygnal" category = "main" optional = false @@ -535,7 +535,7 @@ attrs = "*" importlib-metadata = {version = ">=1.4", markers = "python_version < \"3.8\""} [package.extras] -dev = ["tox", "twisted", "aiounittest", "mypy (==0.910)", "black (==21.9b0)", "flake8 (==4.0.1)", "isort (==5.9.3)"] +dev = ["tox", "twisted", "aiounittest", "mypy (==0.910)", "black (==22.3.0)", "flake8 (==4.0.1)", "isort (==5.9.3)", "build (==0.8.0)", "twine (==4.0.1)"] test = ["tox", "twisted", "aiounittest"] [[package]] @@ -1563,7 +1563,7 @@ url_preview = ["lxml"] [metadata] lock-version = "1.1" python-versions = "^3.7.1" -content-hash = "539e5326f401472d1ffc8325d53d72e544cd70156b3f43f32f1285c4c131f831" +content-hash = "c1bb4dabba1e87517e25ca7bf778e8082fbc960a51d83819aec3a154110a374f" [metadata.files] attrs = [ @@ -2042,8 +2042,8 @@ markupsafe = [ {file = "MarkupSafe-2.1.0.tar.gz", hash = "sha256:80beaf63ddfbc64a0452b841d8036ca0611e049650e20afcb882f5d3c266d65f"}, ] matrix-common = [ - {file = "matrix_common-1.1.0-py3-none-any.whl", hash = "sha256:5d6dfd777503b2f3a031b566e6af25b6e95f9c0818ef57d954c3190fce5eb407"}, - {file = "matrix_common-1.1.0.tar.gz", hash = "sha256:a8238748afc2b37079818367fed5156f355771b07c8ff0a175934f47e0ff3276"}, + {file = "matrix_common-1.2.1-py3-none-any.whl", hash = "sha256:946709c405944a0d4b1d73207b77eb064b6dbfc5d70a69471320b06d8ce98b20"}, + {file = "matrix_common-1.2.1.tar.gz", hash = "sha256:a99dcf02a6bd95b24a5a61b354888a2ac92bf2b4b839c727b8dd9da2cdfa3853"}, ] matrix-synapse-ldap3 = [ {file = "matrix-synapse-ldap3-0.2.0.tar.gz", hash = "sha256:91a0715b43a41ec3033244174fca20846836da98fda711fb01687f7199eecd2e"}, diff --git a/pyproject.toml b/pyproject.toml index ec6e81f254..a6f3169e70 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -150,7 +150,7 @@ typing-extensions = ">=3.10.0.1" cryptography = ">=3.4.7" # ijson 3.1.4 fixes a bug with "." in property names ijson = ">=3.1.4" -matrix-common = "~=1.1.0" +matrix-common = "~=1.2.1" # We need packaging.requirements.Requirement, added in 16.1. packaging = ">=16.1" # At the time of writing, we only use functions from the version `importlib.metadata` diff --git a/synapse/__init__.py b/synapse/__init__.py index 1613941759..b1369aca8f 100644 --- a/synapse/__init__.py +++ b/synapse/__init__.py @@ -20,8 +20,6 @@ import json import os import sys -from matrix_common.versionstring import get_distribution_version_string - # Check that we're not running on an unsupported Python version. if sys.version_info < (3, 7): print("Synapse requires Python 3.7 or above.") @@ -70,7 +68,9 @@ try: except ImportError: pass -__version__ = get_distribution_version_string("matrix-synapse") +import synapse.util + +__version__ = synapse.util.SYNAPSE_VERSION if bool(os.environ.get("SYNAPSE_TEST_PATCH_LOG_CONTEXTS", False)): # We import here so that we don't have to install a bunch of deps when diff --git a/synapse/_scripts/synapse_port_db.py b/synapse/_scripts/synapse_port_db.py index 361b51d2fa..c753dfa7cb 100755 --- a/synapse/_scripts/synapse_port_db.py +++ b/synapse/_scripts/synapse_port_db.py @@ -40,7 +40,6 @@ from typing import ( ) import yaml -from matrix_common.versionstring import get_distribution_version_string from typing_extensions import TypedDict from twisted.internet import defer, reactor as reactor_ @@ -84,7 +83,7 @@ from synapse.storage.databases.state.bg_updates import StateBackgroundUpdateStor from synapse.storage.engines import create_engine from synapse.storage.prepare_database import prepare_database from synapse.types import ISynapseReactor -from synapse.util import Clock +from synapse.util import SYNAPSE_VERSION, Clock # Cast safety: Twisted does some naughty magic which replaces the # twisted.internet.reactor module with a Reactor instance at runtime. @@ -258,9 +257,7 @@ class MockHomeserver: self.clock = Clock(reactor) self.config = config self.hostname = config.server.server_name - self.version_string = "Synapse/" + get_distribution_version_string( - "matrix-synapse" - ) + self.version_string = SYNAPSE_VERSION def get_clock(self) -> Clock: return self.clock diff --git a/synapse/_scripts/update_synapse_database.py b/synapse/_scripts/update_synapse_database.py index c443522c05..b4aeae6dd5 100755 --- a/synapse/_scripts/update_synapse_database.py +++ b/synapse/_scripts/update_synapse_database.py @@ -19,7 +19,6 @@ import sys from typing import cast import yaml -from matrix_common.versionstring import get_distribution_version_string from twisted.internet import defer, reactor as reactor_ @@ -28,6 +27,7 @@ from synapse.metrics.background_process_metrics import run_as_background_process from synapse.server import HomeServer from synapse.storage import DataStore from synapse.types import ISynapseReactor +from synapse.util import SYNAPSE_VERSION # Cast safety: Twisted does some naughty magic which replaces the # twisted.internet.reactor module with a Reactor instance at runtime. @@ -43,8 +43,7 @@ class MockHomeserver(HomeServer): hostname=config.server.server_name, config=config, reactor=reactor, - version_string="Synapse/" - + get_distribution_version_string("matrix-synapse"), + version_string=f"Synapse/{SYNAPSE_VERSION}", ) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index a3446ac6e8..84e389a6cd 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -37,7 +37,6 @@ from typing import ( ) from cryptography.utils import CryptographyDeprecationWarning -from matrix_common.versionstring import get_distribution_version_string from typing_extensions import ParamSpec import twisted @@ -68,6 +67,7 @@ from synapse.metrics import install_gc_manager, register_threadpool from synapse.metrics.background_process_metrics import wrap_as_background_process from synapse.metrics.jemalloc import setup_jemalloc_stats from synapse.types import ISynapseReactor +from synapse.util import SYNAPSE_VERSION from synapse.util.caches.lrucache import setup_expire_lru_cache_entries from synapse.util.daemonize import daemonize_process from synapse.util.gai_resolver import GAIResolver @@ -540,7 +540,7 @@ def setup_sentry(hs: "HomeServer") -> None: sentry_sdk.init( dsn=hs.config.metrics.sentry_dsn, - release=get_distribution_version_string("matrix-synapse"), + release=SYNAPSE_VERSION, ) # We set some default tags that give some context to this instance diff --git a/synapse/app/admin_cmd.py b/synapse/app/admin_cmd.py index 6fedf681f8..561621a285 100644 --- a/synapse/app/admin_cmd.py +++ b/synapse/app/admin_cmd.py @@ -19,8 +19,6 @@ import sys import tempfile from typing import List, Optional -from matrix_common.versionstring import get_distribution_version_string - from twisted.internet import defer, task import synapse @@ -43,6 +41,7 @@ from synapse.replication.slave.storage.registration import SlavedRegistrationSto from synapse.server import HomeServer from synapse.storage.databases.main.room import RoomWorkerStore from synapse.types import StateMap +from synapse.util import SYNAPSE_VERSION from synapse.util.logcontext import LoggingContext logger = logging.getLogger("synapse.app.admin_cmd") @@ -220,7 +219,7 @@ def start(config_options: List[str]) -> None: ss = AdminCmdServer( config.server.server_name, config=config, - version_string="Synapse/" + get_distribution_version_string("matrix-synapse"), + version_string=f"Synapse/{SYNAPSE_VERSION}", ) setup_logging(ss, config, use_worker_options=True) diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 89f8998f0e..4a987fb759 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -16,8 +16,6 @@ import logging import sys from typing import Dict, List, Optional, Tuple -from matrix_common.versionstring import get_distribution_version_string - from twisted.internet import address from twisted.web.resource import Resource @@ -121,6 +119,7 @@ from synapse.storage.databases.main.transactions import TransactionWorkerStore from synapse.storage.databases.main.ui_auth import UIAuthWorkerStore from synapse.storage.databases.main.user_directory import UserDirectoryStore from synapse.types import JsonDict +from synapse.util import SYNAPSE_VERSION from synapse.util.httpresourcetree import create_resource_tree logger = logging.getLogger("synapse.app.generic_worker") @@ -447,7 +446,7 @@ def start(config_options: List[str]) -> None: hs = GenericWorkerServer( config.server.server_name, config=config, - version_string="Synapse/" + get_distribution_version_string("matrix-synapse"), + version_string=f"Synapse/{SYNAPSE_VERSION}", ) setup_logging(hs, config, use_worker_options=True) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 4c6c0658ab..745e704141 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -18,8 +18,6 @@ import os import sys from typing import Dict, Iterable, List -from matrix_common.versionstring import get_distribution_version_string - from twisted.internet.tcp import Port from twisted.web.resource import EncodingResourceWrapper, Resource from twisted.web.server import GzipEncoderFactory @@ -69,7 +67,7 @@ from synapse.rest.synapse.client import build_synapse_client_resource_tree from synapse.rest.well_known import well_known_resource from synapse.server import HomeServer from synapse.storage import DataStore -from synapse.util.check_dependencies import check_requirements +from synapse.util.check_dependencies import VERSION, check_requirements from synapse.util.httpresourcetree import create_resource_tree from synapse.util.module_loader import load_module @@ -371,7 +369,7 @@ def setup(config_options: List[str]) -> SynapseHomeServer: hs = SynapseHomeServer( config.server.server_name, config=config, - version_string="Synapse/" + get_distribution_version_string("matrix-synapse"), + version_string=f"Synapse/{VERSION}", ) synapse.config.logger.setup_logging(hs, config, use_worker_options=False) diff --git a/synapse/config/logger.py b/synapse/config/logger.py index 470b8b4492..82a5b5fa12 100644 --- a/synapse/config/logger.py +++ b/synapse/config/logger.py @@ -22,7 +22,6 @@ from string import Template from typing import TYPE_CHECKING, Any, Dict, Optional import yaml -from matrix_common.versionstring import get_distribution_version_string from zope.interface import implementer from twisted.logger import ( @@ -37,6 +36,7 @@ from synapse.logging.context import LoggingContextFilter from synapse.logging.filter import MetadataFilter from synapse.types import JsonDict +from ..util import SYNAPSE_VERSION from ._base import Config, ConfigError if TYPE_CHECKING: @@ -349,7 +349,7 @@ def setup_logging( logging.warning( "Server %s version %s", sys.argv[0], - get_distribution_version_string("matrix-synapse"), + SYNAPSE_VERSION, ) logging.info("Server hostname: %s", config.server.server_name) logging.info("Instance name: %s", hs.get_instance_name()) diff --git a/synapse/federation/transport/server/federation.py b/synapse/federation/transport/server/federation.py index 7dfb890661..f7884bfbe0 100644 --- a/synapse/federation/transport/server/federation.py +++ b/synapse/federation/transport/server/federation.py @@ -24,7 +24,6 @@ from typing import ( Union, ) -from matrix_common.versionstring import get_distribution_version_string from typing_extensions import Literal from synapse.api.constants import EduTypes @@ -42,6 +41,7 @@ from synapse.http.servlet import ( parse_strings_from_args, ) from synapse.types import JsonDict +from synapse.util import SYNAPSE_VERSION from synapse.util.ratelimitutils import FederationRateLimiter if TYPE_CHECKING: @@ -622,7 +622,7 @@ class FederationVersionServlet(BaseFederationServlet): { "server": { "name": "Synapse", - "version": get_distribution_version_string("matrix-synapse"), + "version": SYNAPSE_VERSION, } }, ) diff --git a/synapse/metrics/__init__.py b/synapse/metrics/__init__.py index fffd83546c..496fce2ecc 100644 --- a/synapse/metrics/__init__.py +++ b/synapse/metrics/__init__.py @@ -35,7 +35,6 @@ from typing import ( ) import attr -from matrix_common.versionstring import get_distribution_version_string from prometheus_client import CollectorRegistry, Counter, Gauge, Histogram, Metric from prometheus_client.core import ( REGISTRY, @@ -54,6 +53,7 @@ from synapse.metrics._exposition import ( ) from synapse.metrics._gc import MIN_TIME_BETWEEN_GCS, install_gc_manager from synapse.metrics._types import Collector +from synapse.util import SYNAPSE_VERSION logger = logging.getLogger(__name__) @@ -419,7 +419,7 @@ build_info = Gauge( ) build_info.labels( " ".join([platform.python_implementation(), platform.python_version()]), - get_distribution_version_string("matrix-synapse"), + SYNAPSE_VERSION, " ".join([platform.system(), platform.release()]), ).set(1) diff --git a/synapse/rest/admin/__init__.py b/synapse/rest/admin/__init__.py index 1aa08f8d95..fa3266720b 100644 --- a/synapse/rest/admin/__init__.py +++ b/synapse/rest/admin/__init__.py @@ -20,8 +20,6 @@ import platform from http import HTTPStatus from typing import TYPE_CHECKING, Optional, Tuple -from matrix_common.versionstring import get_distribution_version_string - from synapse.api.errors import Codes, NotFoundError, SynapseError from synapse.http.server import HttpServer, JsonResource from synapse.http.servlet import RestServlet, parse_json_object_from_request @@ -88,6 +86,7 @@ from synapse.rest.admin.users import ( WhoisRestServlet, ) from synapse.types import JsonDict, RoomStreamToken +from synapse.util import SYNAPSE_VERSION if TYPE_CHECKING: from synapse.server import HomeServer @@ -100,7 +99,7 @@ class VersionServlet(RestServlet): def __init__(self, hs: "HomeServer"): self.res = { - "server_version": get_distribution_version_string("matrix-synapse"), + "server_version": SYNAPSE_VERSION, "python_version": platform.python_version(), } diff --git a/synapse/util/__init__.py b/synapse/util/__init__.py index d8046b7553..6323d452e7 100644 --- a/synapse/util/__init__.py +++ b/synapse/util/__init__.py @@ -19,6 +19,7 @@ from typing import Any, Callable, Dict, Generator, Optional import attr from frozendict import frozendict +from matrix_common.versionstring import get_distribution_version_string from twisted.internet import defer, task from twisted.internet.defer import Deferred @@ -183,3 +184,8 @@ def log_failure( if not consumeErrors: return failure return None + + +# Version string with git info. Computed here once so that we don't invoke git multiple +# times. +SYNAPSE_VERSION = get_distribution_version_string("matrix-synapse", __file__) -- cgit 1.5.1 From b5a3aecf18740fb699f871c8e1d110d847fea6d3 Mon Sep 17 00:00:00 2001 From: Daniel Aloni <74783603+Danieloni1@users.noreply.github.com> Date: Tue, 7 Jun 2022 17:58:48 +0300 Subject: Return the same error message from `/login` when password is incorrect and when account doesn't exist. (#12738) --- changelog.d/12738.misc | 1 + synapse/handlers/auth.py | 8 ++++++-- 2 files changed, 7 insertions(+), 2 deletions(-) create mode 100644 changelog.d/12738.misc (limited to 'synapse') diff --git a/changelog.d/12738.misc b/changelog.d/12738.misc new file mode 100644 index 0000000000..8252223475 --- /dev/null +++ b/changelog.d/12738.misc @@ -0,0 +1 @@ +Report login failures due to unknown third party identifiers in the same way as failures due to invalid passwords. This prevents an attacker from using the error response to determine if the identifier exists. Contributed by Daniel Aloni. \ No newline at end of file diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index fbafbbee6b..6e15028b0a 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -81,6 +81,8 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) +INVALID_USERNAME_OR_PASSWORD = "Invalid username or password" + def convert_client_dict_legacy_fields_to_identifier( submission: JsonDict, @@ -1215,7 +1217,9 @@ class AuthHandler: await self._failed_login_attempts_ratelimiter.can_do_action( None, (medium, address) ) - raise LoginError(403, "", errcode=Codes.FORBIDDEN) + raise LoginError( + 403, msg=INVALID_USERNAME_OR_PASSWORD, errcode=Codes.FORBIDDEN + ) identifier_dict = {"type": "m.id.user", "user": user_id} @@ -1341,7 +1345,7 @@ class AuthHandler: # We raise a 403 here, but note that if we're doing user-interactive # login, it turns all LoginErrors into a 401 anyway. - raise LoginError(403, "Invalid password", errcode=Codes.FORBIDDEN) + raise LoginError(403, msg=INVALID_USERNAME_OR_PASSWORD, errcode=Codes.FORBIDDEN) async def check_password_provider_3pid( self, medium: str, address: str, password: str -- cgit 1.5.1