From 99840b32aa6d5d5ee77892143dd3fdc9c60f5184 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 10 Jul 2024 11:43:29 -0700 Subject: [PATCH 1/8] add column `authenticated` to tables `local_media_repository` and `remote_media_cache` --- synapse/storage/schema/__init__.py | 5 ++++- .../main/delta/86/01_authenticate_media.sql | 16 ++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 synapse/storage/schema/main/delta/86/01_authenticate_media.sql diff --git a/synapse/storage/schema/__init__.py b/synapse/storage/schema/__init__.py index 0dc5d242490..581d00346bf 100644 --- a/synapse/storage/schema/__init__.py +++ b/synapse/storage/schema/__init__.py @@ -19,7 +19,7 @@ # # -SCHEMA_VERSION = 85 # remember to update the list below when updating +SCHEMA_VERSION = 86 # remember to update the list below when updating """Represents the expectations made by the codebase about the database schema This should be incremented whenever the codebase changes its requirements on the @@ -139,6 +139,9 @@ Changes in SCHEMA_VERSION = 85 - Add a column `suspended` to the `users` table + +Changes in SCHEMA_VERSION = 86 + - Add a column `authenticated` to the tables `local_media_repository` and `remote_media_cache` """ diff --git a/synapse/storage/schema/main/delta/86/01_authenticate_media.sql b/synapse/storage/schema/main/delta/86/01_authenticate_media.sql new file mode 100644 index 00000000000..676383fcd2d --- /dev/null +++ b/synapse/storage/schema/main/delta/86/01_authenticate_media.sql @@ -0,0 +1,16 @@ +-- +-- This file is licensed under the Affero General Public License (AGPL) version 3. +-- +-- Copyright (C) 2024 New Vector, Ltd +-- +-- This program is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as +-- published by the Free Software Foundation, either version 3 of the +-- License, or (at your option) any later version. +-- +-- See the GNU Affero General Public License for more details: +-- . + + +ALTER TABLE remote_media_cache ADD COLUMN authenticated BOOLEAN; +ALTER TABLE local_media_repository ADD COLUMN authenticated BOOLEAN; From 7569c77a9888cc11addfe57597a3b4c816084fef Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 10 Jul 2024 11:44:10 -0700 Subject: [PATCH 2/8] add config options to set and enforce authenticated media --- synapse/config/repository.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/config/repository.py b/synapse/config/repository.py index 16454704990..85e6ae4b2b0 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -272,6 +272,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: remote_media_lifetime ) + self.authenticate_new_media = config.get("authenticate_new_media", False) + self.enforce_authenticated_media = config.get( + "enforce_authenticated_media", False + ) + def generate_config_section(self, data_dir_path: str, **kwargs: Any) -> str: assert data_dir_path is not None media_store = os.path.join(data_dir_path, "media_store") From f5ec08109f7e96701e43217cf6c99cfb4ae3e4fa Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 10 Jul 2024 11:44:46 -0700 Subject: [PATCH 3/8] get and set authenticated status when storing/fetching local/remote media --- .../databases/main/media_repository.py | 28 ++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index 6128332af8b..e41ebb3c0ff 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -64,6 +64,7 @@ class LocalMedia: quarantined_by: Optional[str] safe_from_quarantine: bool user_id: Optional[str] + authenticated: Optional[bool] @attr.s(slots=True, frozen=True, auto_attribs=True) @@ -77,6 +78,7 @@ class RemoteMedia: created_ts: int last_access_ts: int quarantined_by: Optional[str] + authenticated: Optional[bool] @attr.s(slots=True, frozen=True, auto_attribs=True) @@ -218,6 +220,7 @@ async def get_local_media(self, media_id: str) -> Optional[LocalMedia]: "last_access_ts", "safe_from_quarantine", "user_id", + "authenticated", ), allow_none=True, desc="get_local_media", @@ -235,6 +238,7 @@ async def get_local_media(self, media_id: str) -> Optional[LocalMedia]: last_access_ts=row[6], safe_from_quarantine=row[7], user_id=row[8], + authenticated=row[9], ) async def get_local_media_by_user_paginate( @@ -290,7 +294,8 @@ def get_local_media_by_user_paginate_txn( last_access_ts, quarantined_by, safe_from_quarantine, - user_id + user_id, + authenticated FROM local_media_repository WHERE user_id = ? ORDER BY {order_by_column} {order}, media_id ASC @@ -314,6 +319,7 @@ def get_local_media_by_user_paginate_txn( quarantined_by=row[7], safe_from_quarantine=bool(row[8]), user_id=row[9], + authenticated=row[10], ) for row in txn ] @@ -417,12 +423,18 @@ async def store_local_media_id( time_now_ms: int, user_id: UserID, ) -> None: + if self.hs.config.media.authenticate_new_media: + authenticated = True + else: + authenticated = False + await self.db_pool.simple_insert( "local_media_repository", { "media_id": media_id, "created_ts": time_now_ms, "user_id": user_id.to_string(), + "authenticated": authenticated, }, desc="store_local_media_id", ) @@ -438,6 +450,11 @@ async def store_local_media( user_id: UserID, url_cache: Optional[str] = None, ) -> None: + if self.hs.config.media.authenticate_new_media: + authenticated = True + else: + authenticated = False + await self.db_pool.simple_insert( "local_media_repository", { @@ -448,6 +465,7 @@ async def store_local_media( "media_length": media_length, "user_id": user_id.to_string(), "url_cache": url_cache, + "authenticated": authenticated, }, desc="store_local_media", ) @@ -638,6 +656,7 @@ async def get_cached_remote_media( "filesystem_id", "last_access_ts", "quarantined_by", + "authenticated", ), allow_none=True, desc="get_cached_remote_media", @@ -654,6 +673,7 @@ async def get_cached_remote_media( filesystem_id=row[4], last_access_ts=row[5], quarantined_by=row[6], + authenticated=row[7], ) async def store_cached_remote_media( @@ -666,6 +686,11 @@ async def store_cached_remote_media( upload_name: Optional[str], filesystem_id: str, ) -> None: + if self.hs.config.media.authenticate_new_media: + authenticated = True + else: + authenticated = False + await self.db_pool.simple_insert( "remote_media_cache", { @@ -677,6 +702,7 @@ async def store_cached_remote_media( "upload_name": upload_name, "filesystem_id": filesystem_id, "last_access_ts": time_now_ms, + "authenticated": authenticated, }, desc="store_cached_remote_media", ) From 87ae2c290628a93fa8484366aaad1938cf1dc857 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 10 Jul 2024 11:45:52 -0700 Subject: [PATCH 4/8] disallow authenticated media over unauthenticated endpoints if config demands it --- synapse/media/media_repository.py | 43 ++++++++++++++++++++- synapse/media/thumbnailer.py | 48 +++++++++++++++++++++--- synapse/rest/media/download_resource.py | 3 +- synapse/rest/media/thumbnail_resource.py | 5 ++- 4 files changed, 91 insertions(+), 8 deletions(-) diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index 87c929eb200..8776a20e7ad 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -430,6 +430,7 @@ async def get_local_media( media_id: str, name: Optional[str], max_timeout_ms: int, + allow_authenticated: bool = True, federation: bool = False, ) -> None: """Responds to requests for local media, if exists, or returns 404. @@ -442,6 +443,7 @@ async def get_local_media( the filename in the Content-Disposition header of the response. max_timeout_ms: the maximum number of milliseconds to wait for the media to be uploaded. + allow_authenticated: whether media marked as authenticated may be served to this request federation: whether the local media being fetched is for a federation request Returns: @@ -451,6 +453,10 @@ async def get_local_media( if not media_info: return + if self.hs.config.media.enforce_authenticated_media and not allow_authenticated: + if media_info.authenticated: + raise NotFoundError + self.mark_recently_accessed(None, media_id) media_type = media_info.media_type @@ -481,6 +487,7 @@ async def get_remote_media( max_timeout_ms: int, ip_address: str, use_federation_endpoint: bool, + allow_authenticated: bool = True, ) -> None: """Respond to requests for remote media. @@ -495,6 +502,8 @@ async def get_remote_media( ip_address: the IP address of the requester use_federation_endpoint: whether to request the remote media over the new federation `/download` endpoint + allow_authenticated: whether media marked as authenticated may be served to this + request Returns: Resolves once a response has successfully been written to request @@ -526,6 +535,7 @@ async def get_remote_media( self.download_ratelimiter, ip_address, use_federation_endpoint, + allow_authenticated, ) # We deliberately stream the file outside the lock @@ -548,6 +558,7 @@ async def get_remote_media_info( max_timeout_ms: int, ip_address: str, use_federation: bool, + allow_authenticated: bool, ) -> RemoteMedia: """Gets the media info associated with the remote file, downloading if necessary. @@ -560,6 +571,8 @@ async def get_remote_media_info( ip_address: IP address of the requester use_federation: if a download is necessary, whether to request the remote file over the federation `/download` endpoint + allow_authenticated: whether media marked as authenticated may be served to this + request Returns: The media info of the file @@ -581,6 +594,7 @@ async def get_remote_media_info( self.download_ratelimiter, ip_address, use_federation, + allow_authenticated, ) # Ensure we actually use the responder so that it releases resources @@ -598,6 +612,7 @@ async def _get_remote_media_impl( download_ratelimiter: Ratelimiter, ip_address: str, use_federation_endpoint: bool, + allow_authenticated: bool, ) -> Tuple[Optional[Responder], RemoteMedia]: """Looks for media in local cache, if not there then attempt to download from remote server. @@ -619,6 +634,15 @@ async def _get_remote_media_impl( """ media_info = await self.store.get_cached_remote_media(server_name, media_id) + if self.hs.config.media.enforce_authenticated_media and not allow_authenticated: + # if it isn't cached then don't fetch it + if not media_info: + raise NotFoundError() + # and if it's authenticated don't serve it + else: + if media_info.authenticated: + raise NotFoundError() + # file_id is the ID we use to track the file locally. If we've already # seen the file then reuse the existing ID, otherwise generate a new # one. @@ -792,6 +816,11 @@ async def _download_remote_file( logger.info("Stored remote media in file %r", fname) + if self.hs.config.media.authenticate_new_media: + authenticated = True + else: + authenticated = False + return RemoteMedia( media_origin=server_name, media_id=media_id, @@ -802,6 +831,7 @@ async def _download_remote_file( filesystem_id=file_id, last_access_ts=time_now_ms, quarantined_by=None, + authenticated=authenticated, ) async def _federation_download_remote_file( @@ -915,6 +945,11 @@ async def _federation_download_remote_file( logger.debug("Stored remote media in file %r", fname) + if self.hs.config.media.authenticate_new_media: + authenticated = True + else: + authenticated = False + return RemoteMedia( media_origin=server_name, media_id=media_id, @@ -925,6 +960,7 @@ async def _federation_download_remote_file( filesystem_id=file_id, last_access_ts=time_now_ms, quarantined_by=None, + authenticated=authenticated, ) def _get_thumbnail_requirements( @@ -1030,7 +1066,12 @@ async def generate_local_exact_thumbnail( t_len = os.path.getsize(output_path) await self.store.store_local_thumbnail( - media_id, t_width, t_height, t_type, t_method, t_len + media_id, + t_width, + t_height, + t_type, + t_method, + t_len, ) return output_path diff --git a/synapse/media/thumbnailer.py b/synapse/media/thumbnailer.py index 413a720e40b..da681ec604a 100644 --- a/synapse/media/thumbnailer.py +++ b/synapse/media/thumbnailer.py @@ -26,7 +26,7 @@ from PIL import Image -from synapse.api.errors import Codes, SynapseError, cs_error +from synapse.api.errors import Codes, NotFoundError, SynapseError, cs_error from synapse.config.repository import THUMBNAIL_SUPPORTED_MEDIA_FORMAT_MAP from synapse.http.server import respond_with_json from synapse.http.site import SynapseRequest @@ -274,6 +274,7 @@ async def respond_local_thumbnail( m_type: str, max_timeout_ms: int, for_federation: bool, + allow_authenticated: bool = True, ) -> None: media_info = await self.media_repo.get_local_media_info( request, media_id, max_timeout_ms @@ -281,6 +282,12 @@ async def respond_local_thumbnail( if not media_info: return + # if the media the thumbnail is generated from is authenticated, don't serve the + # thumbnail over an unauthenticated endpoint + if self.hs.config.media.enforce_authenticated_media and not allow_authenticated: + if media_info.authenticated: + raise NotFoundError + thumbnail_infos = await self.store.get_local_media_thumbnails(media_id) await self._select_and_respond_with_thumbnail( request, @@ -307,14 +314,20 @@ async def select_or_generate_local_thumbnail( desired_type: str, max_timeout_ms: int, for_federation: bool, + allow_authenticated: bool = True, ) -> None: media_info = await self.media_repo.get_local_media_info( request, media_id, max_timeout_ms ) - if not media_info: return + # if the media the thumbnail is generated from is authenticated, don't serve the + # thumbnail over an unauthenticated endpoint + if self.hs.config.media.enforce_authenticated_media and not allow_authenticated: + if media_info.authenticated: + raise NotFoundError + thumbnail_infos = await self.store.get_local_media_thumbnails(media_id) for info in thumbnail_infos: t_w = info.width == desired_width @@ -381,14 +394,27 @@ async def select_or_generate_remote_thumbnail( max_timeout_ms: int, ip_address: str, use_federation: bool, + allow_authenticated: bool = True, ) -> None: media_info = await self.media_repo.get_remote_media_info( - server_name, media_id, max_timeout_ms, ip_address, use_federation + server_name, + media_id, + max_timeout_ms, + ip_address, + use_federation, + allow_authenticated, ) if not media_info: respond_404(request) return + # if the media the thumbnail is generated from is authenticated, don't serve the + # thumbnail over an unauthenticated endpoint + if self.hs.config.media.enforce_authenticated_media and not allow_authenticated: + if media_info.authenticated: + respond_404(request) + return + thumbnail_infos = await self.store.get_remote_media_thumbnails( server_name, media_id ) @@ -446,16 +472,28 @@ async def respond_remote_thumbnail( max_timeout_ms: int, ip_address: str, use_federation: bool, + allow_authenticated: bool = True, ) -> None: # TODO: Don't download the whole remote file # We should proxy the thumbnail from the remote server instead of # downloading the remote file and generating our own thumbnails. media_info = await self.media_repo.get_remote_media_info( - server_name, media_id, max_timeout_ms, ip_address, use_federation + server_name, + media_id, + max_timeout_ms, + ip_address, + use_federation, + allow_authenticated, ) if not media_info: return + # if the media the thumbnail is generated from is authenticated, don't serve the + # thumbnail over an unauthenticated endpoint + if self.hs.config.media.enforce_authenticated_media and not allow_authenticated: + if media_info.authenticated: + raise NotFoundError + thumbnail_infos = await self.store.get_remote_media_thumbnails( server_name, media_id ) @@ -485,8 +523,8 @@ async def _select_and_respond_with_thumbnail( file_id: str, url_cache: bool, for_federation: bool, - server_name: Optional[str] = None, media_info: Optional[LocalMedia] = None, + server_name: Optional[str] = None, ) -> None: """ Respond to a request with an appropriate thumbnail from the previously generated thumbnails. diff --git a/synapse/rest/media/download_resource.py b/synapse/rest/media/download_resource.py index c32c626905f..3c3f703667a 100644 --- a/synapse/rest/media/download_resource.py +++ b/synapse/rest/media/download_resource.py @@ -84,7 +84,7 @@ async def on_GET( if self._is_mine_server_name(server_name): await self.media_repo.get_local_media( - request, media_id, file_name, max_timeout_ms + request, media_id, file_name, max_timeout_ms, allow_authenticated=False ) else: allow_remote = parse_boolean(request, "allow_remote", default=True) @@ -106,4 +106,5 @@ async def on_GET( max_timeout_ms, ip_address, False, + allow_authenticated=False, ) diff --git a/synapse/rest/media/thumbnail_resource.py b/synapse/rest/media/thumbnail_resource.py index 70354aa4394..536fea4c32f 100644 --- a/synapse/rest/media/thumbnail_resource.py +++ b/synapse/rest/media/thumbnail_resource.py @@ -96,6 +96,7 @@ async def on_GET( m_type, max_timeout_ms, False, + allow_authenticated=False, ) else: await self.thumbnail_provider.respond_local_thumbnail( @@ -107,6 +108,7 @@ async def on_GET( m_type, max_timeout_ms, False, + allow_authenticated=False, ) self.media_repo.mark_recently_accessed(None, media_id) else: @@ -134,6 +136,7 @@ async def on_GET( m_type, max_timeout_ms, ip_address, - False, + use_federation=False, + allow_authenticated=False, ) self.media_repo.mark_recently_accessed(server_name, media_id) From 0c787b4c25bc67d6fbf0e4d8854b3f92f33aad2c Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 10 Jul 2024 21:18:24 -0700 Subject: [PATCH 5/8] tests --- tests/rest/client/test_media.py | 167 ++++++++++++++++++++++++++++++++ 1 file changed, 167 insertions(+) diff --git a/tests/rest/client/test_media.py b/tests/rest/client/test_media.py index 7f2caed7d5d..6e671b4ab80 100644 --- a/tests/rest/client/test_media.py +++ b/tests/rest/client/test_media.py @@ -43,6 +43,7 @@ from twisted.test.proto_helpers import AccumulatingProtocol, MemoryReactor from twisted.web.http_headers import Headers from twisted.web.iweb import UNKNOWN_LENGTH, IResponse +from twisted.web.resource import Resource from synapse.api.errors import HttpResponseException from synapse.api.ratelimiting import Ratelimiter @@ -2426,3 +2427,169 @@ def test_same_quality(self, method: str, desired_size: int) -> None: server_name=None, ) ) + + +configs = [ + {"extra_config": {"dynamic_thumbnails": True}}, + {"extra_config": {"dynamic_thumbnails": False}}, +] + + +@parameterized_class(configs) +class AuthenticatedMediaTestCase(unittest.HomeserverTestCase): + extra_config: Dict[str, Any] + servlets = [ + media.register_servlets, + login.register_servlets, + admin.register_servlets, + ] + + def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: + config = self.default_config() + + self.clock = clock + self.storage_path = self.mktemp() + self.media_store_path = self.mktemp() + os.mkdir(self.storage_path) + os.mkdir(self.media_store_path) + config["media_store_path"] = self.media_store_path + config["authenticate_new_media"] = (True,) + config["enforce_authenticated_media"] = True + + provider_config = { + "module": "synapse.media.storage_provider.FileStorageProviderBackend", + "store_local": True, + "store_synchronous": False, + "store_remote": True, + "config": {"directory": self.storage_path}, + } + + config["media_storage_providers"] = [provider_config] + config.update(self.extra_config) + + return self.setup_test_homeserver(config=config) + + def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: + self.repo = hs.get_media_repository() + self.client = hs.get_federation_http_client() + self.store = hs.get_datastores().main + self.user = self.register_user("user", "pass") + self.tok = self.login("user", "pass") + + def create_resource_dict(self) -> Dict[str, Resource]: + resources = super().create_resource_dict() + resources["/_matrix/media"] = self.hs.get_media_repository_resource() + return resources + + def test_authenticated_media(self) -> None: + # upload some local media with authentication on + channel = self.make_request( + "POST", + "_matrix/media/v3/upload?filename=test_png_upload", + SMALL_PNG, + self.tok, + shorthand=False, + content_type=b"image/png", + custom_headers=[("Content-Length", str(67))], + ) + self.assertEqual(channel.code, 200) + res = channel.json_body.get("content_uri") + assert res is not None + uri = res.split("mxc://")[1] + + # request media over authenticated endpoint, should be found + channel2 = self.make_request( + "GET", + f"_matrix/client/v1/media/download/{uri}", + access_token=self.tok, + shorthand=False, + ) + self.assertEqual(channel2.code, 200) + + # request same media over unauthenticated media, should raise 404 not found + channel3 = self.make_request( + "GET", f"_matrix/media/v3/download/{uri}", shorthand=False + ) + self.assertEqual(channel3.code, 404) + + # check thumbnails as well + params = "?width=32&height=32&method=crop" + channel4 = self.make_request( + "GET", + f"/_matrix/client/v1/media/thumbnail/{uri}{params}", + shorthand=False, + access_token=self.tok, + ) + self.assertEqual(channel4.code, 200) + + params = "?width=32&height=32&method=crop" + channel5 = self.make_request( + "GET", + f"/_matrix/media/r0/thumbnail/{uri}{params}", + shorthand=False, + access_token=self.tok, + ) + self.assertEqual(channel5.code, 404) + + # Inject a piece of remote media. + file_id = "abcdefg12345" + file_info = FileInfo(server_name="lonelyIsland", file_id=file_id) + + media_storage = self.hs.get_media_repository().media_storage + + ctx = media_storage.store_into_file(file_info) + (f, fname) = self.get_success(ctx.__aenter__()) + f.write(SMALL_PNG) + self.get_success(ctx.__aexit__(None, None, None)) + + # we write the authenticated status when storing media, so this should pick up + # config and authenticate the media + self.get_success( + self.store.store_cached_remote_media( + origin="lonelyIsland", + media_id="52", + media_type="image/png", + media_length=1, + time_now_ms=self.clock.time_msec(), + upload_name="remote_test.png", + filesystem_id=file_id, + ) + ) + + # ensure we have thumbnails for the non-dynamic code path + if self.extra_config == {"dynamic_thumbnails": False}: + self.get_success( + self.repo._generate_thumbnails( + "lonelyIsland", "52", file_id, "image/png" + ) + ) + + channel6 = self.make_request( + "GET", + "_matrix/client/v1/media/download/lonelyIsland/52", + access_token=self.tok, + shorthand=False, + ) + self.assertEqual(channel6.code, 200) + + channel7 = self.make_request( + "GET", f"_matrix/media/v3/download/{uri}", shorthand=False + ) + self.assertEqual(channel7.code, 404) + + params = "?width=32&height=32&method=crop" + channel8 = self.make_request( + "GET", + f"/_matrix/client/v1/media/thumbnail/lonelyIsland/52{params}", + shorthand=False, + access_token=self.tok, + ) + self.assertEqual(channel8.code, 200) + + channel9 = self.make_request( + "GET", + f"/_matrix/media/r0/thumbnail/lonelyIsland/52{params}", + shorthand=False, + access_token=self.tok, + ) + self.assertEqual(channel9.code, 404) From 11cfc0a849b7e5aa51faf55eecb1ab73cd4c4400 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 10 Jul 2024 21:25:43 -0700 Subject: [PATCH 6/8] newsfragment --- changelog.d/17433.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/17433.feature diff --git a/changelog.d/17433.feature b/changelog.d/17433.feature new file mode 100644 index 00000000000..ac9b5dee694 --- /dev/null +++ b/changelog.d/17433.feature @@ -0,0 +1 @@ +Prepare for authenticated media freeze. \ No newline at end of file From bfee3141a539e11789ee49247e40a581c654ba3a Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Tue, 16 Jul 2024 16:45:14 -0700 Subject: [PATCH 7/8] requested changes --- .../configuration/config_documentation.md | 10 ++++ synapse/_scripts/synapse_port_db.py | 5 +- synapse/config/repository.py | 5 +- synapse/media/media_repository.py | 18 +++----- synapse/media/thumbnailer.py | 14 +++--- .../databases/main/media_repository.py | 6 +-- .../main/delta/86/01_authenticate_media.sql | 5 +- tests/rest/client/test_media.py | 46 ++++++++++++++++++- 8 files changed, 78 insertions(+), 31 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 65b03ad0f82..55ee605dca4 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -1863,6 +1863,16 @@ federation_rr_transactions_per_room_per_second: 40 ## Media Store Config options related to Synapse's media store. +--- +### `enable_authenticated_media` + +When set to true, all subsequent media uploads will be marked as authenticated, and will not be available over legacy +unauthenticated media endpoints (`/_matrix/media/(r0|v3|v1)/download` and `/_matrix/media/(r0|v3|v1)/thumbnail`) - requests for authenticated media over these endpoints will result in a 404. All media, including authenticated media, will be available over the authenticated media endpoints `_matrix/client/v1/media/download` and `_matrix/client/v1/media/thumbnail`. Media uploaded prior to setting this option to true will still be available over the legacy endpoints. + +Example configuration: +```yaml +enable_authenticated_media +``` --- ### `enable_media_repo` diff --git a/synapse/_scripts/synapse_port_db.py b/synapse/_scripts/synapse_port_db.py index 3bb4a34938f..5c6db8118fc 100755 --- a/synapse/_scripts/synapse_port_db.py +++ b/synapse/_scripts/synapse_port_db.py @@ -119,18 +119,19 @@ "e2e_room_keys": ["is_verified"], "event_edges": ["is_state"], "events": ["processed", "outlier", "contains_url"], - "local_media_repository": ["safe_from_quarantine"], + "local_media_repository": ["safe_from_quarantine", "authenticated"], + "per_user_experimental_features": ["enabled"], "presence_list": ["accepted"], "presence_stream": ["currently_active"], "public_room_list_stream": ["visibility"], "pushers": ["enabled"], "redactions": ["have_censored"], + "remote_media_cache": ["authenticated"], "room_stats_state": ["is_federatable"], "rooms": ["is_public", "has_auth_chain_index"], "users": ["shadow_banned", "approved", "locked", "suspended"], "un_partial_stated_event_stream": ["rejection_status_changed"], "users_who_share_rooms": ["share_private"], - "per_user_experimental_features": ["enabled"], } diff --git a/synapse/config/repository.py b/synapse/config/repository.py index 85e6ae4b2b0..977f1140824 100644 --- a/synapse/config/repository.py +++ b/synapse/config/repository.py @@ -272,9 +272,8 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: remote_media_lifetime ) - self.authenticate_new_media = config.get("authenticate_new_media", False) - self.enforce_authenticated_media = config.get( - "enforce_authenticated_media", False + self.enable_authenticated_media = config.get( + "enable_authenticated_media", False ) def generate_config_section(self, data_dir_path: str, **kwargs: Any) -> str: diff --git a/synapse/media/media_repository.py b/synapse/media/media_repository.py index 8776a20e7ad..8bc92305fe5 100644 --- a/synapse/media/media_repository.py +++ b/synapse/media/media_repository.py @@ -453,9 +453,9 @@ async def get_local_media( if not media_info: return - if self.hs.config.media.enforce_authenticated_media and not allow_authenticated: + if self.hs.config.media.enable_authenticated_media and not allow_authenticated: if media_info.authenticated: - raise NotFoundError + raise NotFoundError() self.mark_recently_accessed(None, media_id) @@ -634,14 +634,10 @@ async def _get_remote_media_impl( """ media_info = await self.store.get_cached_remote_media(server_name, media_id) - if self.hs.config.media.enforce_authenticated_media and not allow_authenticated: - # if it isn't cached then don't fetch it - if not media_info: + if self.hs.config.media.enable_authenticated_media and not allow_authenticated: + # if it isn't cached then don't fetch it or if it's authenticated then don't serve it + if not media_info or media_info.authenticated: raise NotFoundError() - # and if it's authenticated don't serve it - else: - if media_info.authenticated: - raise NotFoundError() # file_id is the ID we use to track the file locally. If we've already # seen the file then reuse the existing ID, otherwise generate a new @@ -816,7 +812,7 @@ async def _download_remote_file( logger.info("Stored remote media in file %r", fname) - if self.hs.config.media.authenticate_new_media: + if self.hs.config.media.enable_authenticated_media: authenticated = True else: authenticated = False @@ -945,7 +941,7 @@ async def _federation_download_remote_file( logger.debug("Stored remote media in file %r", fname) - if self.hs.config.media.authenticate_new_media: + if self.hs.config.media.enable_authenticated_media: authenticated = True else: authenticated = False diff --git a/synapse/media/thumbnailer.py b/synapse/media/thumbnailer.py index da681ec604a..ef6aa8ccf54 100644 --- a/synapse/media/thumbnailer.py +++ b/synapse/media/thumbnailer.py @@ -284,9 +284,9 @@ async def respond_local_thumbnail( # if the media the thumbnail is generated from is authenticated, don't serve the # thumbnail over an unauthenticated endpoint - if self.hs.config.media.enforce_authenticated_media and not allow_authenticated: + if self.hs.config.media.enable_authenticated_media and not allow_authenticated: if media_info.authenticated: - raise NotFoundError + raise NotFoundError() thumbnail_infos = await self.store.get_local_media_thumbnails(media_id) await self._select_and_respond_with_thumbnail( @@ -324,9 +324,9 @@ async def select_or_generate_local_thumbnail( # if the media the thumbnail is generated from is authenticated, don't serve the # thumbnail over an unauthenticated endpoint - if self.hs.config.media.enforce_authenticated_media and not allow_authenticated: + if self.hs.config.media.enable_authenticated_media and not allow_authenticated: if media_info.authenticated: - raise NotFoundError + raise NotFoundError() thumbnail_infos = await self.store.get_local_media_thumbnails(media_id) for info in thumbnail_infos: @@ -410,7 +410,7 @@ async def select_or_generate_remote_thumbnail( # if the media the thumbnail is generated from is authenticated, don't serve the # thumbnail over an unauthenticated endpoint - if self.hs.config.media.enforce_authenticated_media and not allow_authenticated: + if self.hs.config.media.enable_authenticated_media and not allow_authenticated: if media_info.authenticated: respond_404(request) return @@ -490,9 +490,9 @@ async def respond_remote_thumbnail( # if the media the thumbnail is generated from is authenticated, don't serve the # thumbnail over an unauthenticated endpoint - if self.hs.config.media.enforce_authenticated_media and not allow_authenticated: + if self.hs.config.media.enable_authenticated_media and not allow_authenticated: if media_info.authenticated: - raise NotFoundError + raise NotFoundError() thumbnail_infos = await self.store.get_remote_media_thumbnails( server_name, media_id diff --git a/synapse/storage/databases/main/media_repository.py b/synapse/storage/databases/main/media_repository.py index e41ebb3c0ff..7617fd3ad49 100644 --- a/synapse/storage/databases/main/media_repository.py +++ b/synapse/storage/databases/main/media_repository.py @@ -423,7 +423,7 @@ async def store_local_media_id( time_now_ms: int, user_id: UserID, ) -> None: - if self.hs.config.media.authenticate_new_media: + if self.hs.config.media.enable_authenticated_media: authenticated = True else: authenticated = False @@ -450,7 +450,7 @@ async def store_local_media( user_id: UserID, url_cache: Optional[str] = None, ) -> None: - if self.hs.config.media.authenticate_new_media: + if self.hs.config.media.enable_authenticated_media: authenticated = True else: authenticated = False @@ -686,7 +686,7 @@ async def store_cached_remote_media( upload_name: Optional[str], filesystem_id: str, ) -> None: - if self.hs.config.media.authenticate_new_media: + if self.hs.config.media.enable_authenticated_media: authenticated = True else: authenticated = False diff --git a/synapse/storage/schema/main/delta/86/01_authenticate_media.sql b/synapse/storage/schema/main/delta/86/01_authenticate_media.sql index 676383fcd2d..c1ac01ae953 100644 --- a/synapse/storage/schema/main/delta/86/01_authenticate_media.sql +++ b/synapse/storage/schema/main/delta/86/01_authenticate_media.sql @@ -11,6 +11,5 @@ -- See the GNU Affero General Public License for more details: -- . - -ALTER TABLE remote_media_cache ADD COLUMN authenticated BOOLEAN; -ALTER TABLE local_media_repository ADD COLUMN authenticated BOOLEAN; +ALTER TABLE remote_media_cache ADD COLUMN authenticated BOOLEAN DEFAULT FALSE NOT NULL; +ALTER TABLE local_media_repository ADD COLUMN authenticated BOOLEAN DEFAULT FALSE NOT NULL; diff --git a/tests/rest/client/test_media.py b/tests/rest/client/test_media.py index 6e671b4ab80..b6e1a177eba 100644 --- a/tests/rest/client/test_media.py +++ b/tests/rest/client/test_media.py @@ -2453,8 +2453,7 @@ def make_homeserver(self, reactor: MemoryReactor, clock: Clock) -> HomeServer: os.mkdir(self.storage_path) os.mkdir(self.media_store_path) config["media_store_path"] = self.media_store_path - config["authenticate_new_media"] = (True,) - config["enforce_authenticated_media"] = True + config["enable_authenticated_media"] = True provider_config = { "module": "synapse.media.storage_provider.FileStorageProviderBackend", @@ -2593,3 +2592,46 @@ def test_authenticated_media(self) -> None: access_token=self.tok, ) self.assertEqual(channel9.code, 404) + + # Inject a piece of local media that isn't authenticated + file_id = "abcdefg123456" + file_info = FileInfo(None, file_id=file_id) + + ctx = media_storage.store_into_file(file_info) + (f, fname) = self.get_success(ctx.__aenter__()) + f.write(SMALL_PNG) + self.get_success(ctx.__aexit__(None, None, None)) + + self.get_success( + self.store.db_pool.simple_insert( + "local_media_repository", + { + "media_id": "abcdefg123456", + "media_type": "image/png", + "created_ts": self.clock.time_msec(), + "upload_name": "test_local", + "media_length": 1, + "user_id": "someone", + "url_cache": None, + "authenticated": False, + }, + desc="store_local_media", + ) + ) + + # check that unauthenticated media is still available over both endpoints + channel9 = self.make_request( + "GET", + "/_matrix/client/v1/media/download/test/abcdefg123456", + shorthand=False, + access_token=self.tok, + ) + self.assertEqual(channel9.code, 200) + + channel10 = self.make_request( + "GET", + "/_matrix/media/r0/download/test/abcdefg123456", + shorthand=False, + access_token=self.tok, + ) + self.assertEqual(channel10.code, 200) From f532149f1caf09c552dc61ff46d0dd81bf21d495 Mon Sep 17 00:00:00 2001 From: "H. Shay" Date: Wed, 17 Jul 2024 11:02:07 -0700 Subject: [PATCH 8/8] request doc changes --- docs/usage/configuration/config_documentation.md | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 55ee605dca4..eca92a3975c 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -1867,11 +1867,13 @@ Config options related to Synapse's media store. ### `enable_authenticated_media` When set to true, all subsequent media uploads will be marked as authenticated, and will not be available over legacy -unauthenticated media endpoints (`/_matrix/media/(r0|v3|v1)/download` and `/_matrix/media/(r0|v3|v1)/thumbnail`) - requests for authenticated media over these endpoints will result in a 404. All media, including authenticated media, will be available over the authenticated media endpoints `_matrix/client/v1/media/download` and `_matrix/client/v1/media/thumbnail`. Media uploaded prior to setting this option to true will still be available over the legacy endpoints. +unauthenticated media endpoints (`/_matrix/media/(r0|v3|v1)/download` and `/_matrix/media/(r0|v3|v1)/thumbnail`) - requests for authenticated media over these endpoints will result in a 404. All media, including authenticated media, will be available over the authenticated media endpoints `_matrix/client/v1/media/download` and `_matrix/client/v1/media/thumbnail`. Media uploaded prior to setting this option to true will still be available over the legacy endpoints. Note if the setting is switched to false +after enabling, media marked as authenticated will be available over legacy endpoints. Defaults to false, but +this will change to true in a future Synapse release. Example configuration: ```yaml -enable_authenticated_media +enable_authenticated_media: true ``` --- ### `enable_media_repo`