Enable authenticated media by default (#17889)

Co-authored-by: Olivier 'reivilibre <[email protected]>

changelog.d/17889.feature

@@ -0,0 +1 @@
+Enforce authenticated media by default. Administrators can revert this by configuring `enable_authenticated_media` to `false`. In a future release of Synapse, this option will be removed and become always-on.

docs/upgrade.md

@@ -128,6 +128,29 @@ removing the experimental support for it in this release.
 The `experimental_features.msc3886_endpoint` configuration option has
 been removed.
 
+## Authenticated media is now enforced by default
+
+The [`enable_authenticated_media`] configuration option now defaults to true.
+
+This means that clients and remote (federated) homeservers now need to use
+the authenticated media endpoints in order to download media from your
+homeserver.
+
+As an exception, existing media that was stored on the server prior to
+this option changing to `true` will still be accessible over the
+unauthenticated endpoints.
+
+The matrix.org homeserver has already been running with this option enabled
+since September 2024, so most common clients and homeservers should already
+be compatible.
+
+With that said, administrators who wish to disable this feature for broader
+compatibility can still do so by manually configuring
+`enable_authenticated_media: False`.
+
+[`enable_authenticated_media`]: usage/configuration/config_documentation.md#enable_authenticated_media
+
+
 # Upgrading to v1.119.0
 
 ## Minimum supported Python version

docs/usage/configuration/config_documentation.md

@@ -1887,8 +1887,7 @@ Config options related to Synapse's media store.
 
 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. 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.
+after enabling, media marked as authenticated will be available over legacy endpoints. Defaults to true (previously false). In a future release of Synapse, this option will be removed and become always-on.
 
 In all cases, authenticated requests to download media will succeed, but for unauthenticated requests, this
 case-by-case breakdown describes whether media downloads are permitted:
@@ -1910,9 +1909,11 @@ will perpetually be available over the legacy, unauthenticated endpoint, even af
 This is for backwards compatibility with older clients and homeservers that do not yet support requesting authenticated media;
 those older clients or homeservers will not be cut off from media they can already see.
 
+_Changed in Synapse 1.120:_ This option now defaults to `True` when not set, whereas before this version it defaulted to `False`.
+
 Example configuration:
 ```yaml
-enable_authenticated_media: true
+enable_authenticated_media: false
 ```
 ---
 ### `enable_media_repo`

synapse/config/repository.py

@@ -272,9 +272,7 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
                 remote_media_lifetime
             )
 
-        self.enable_authenticated_media = config.get(
-            "enable_authenticated_media", False
-        )
+        self.enable_authenticated_media = config.get("enable_authenticated_media", True)
 
     def generate_config_section(self, data_dir_path: str, **kwargs: Any) -> str:
         assert data_dir_path is not None

tests/media/test_media_storage.py

@@ -419,6 +419,11 @@ def _req(
 
         return channel
 
+    @unittest.override_config(
+        {
+            "enable_authenticated_media": False,
+        }
+    )
     def test_handle_missing_content_type(self) -> None:
         channel = self._req(
             b"attachment; filename=out" + self.test_image.extension,
@@ -430,6 +435,11 @@ def test_handle_missing_content_type(self) -> None:
             headers.getRawHeaders(b"Content-Type"), [b"application/octet-stream"]
         )
 
+    @unittest.override_config(
+        {
+            "enable_authenticated_media": False,
+        }
+    )
     def test_disposition_filename_ascii(self) -> None:
         """
         If the filename is filename=<ascii> then Synapse will decode it as an
@@ -450,6 +460,11 @@ def test_disposition_filename_ascii(self) -> None:
             ],
         )
 
+    @unittest.override_config(
+        {
+            "enable_authenticated_media": False,
+        }
+    )
     def test_disposition_filenamestar_utf8escaped(self) -> None:
         """
         If the filename is filename=*utf8''<utf8 escaped> then Synapse will
@@ -475,6 +490,11 @@ def test_disposition_filenamestar_utf8escaped(self) -> None:
             ],
         )
 
+    @unittest.override_config(
+        {
+            "enable_authenticated_media": False,
+        }
+    )
     def test_disposition_none(self) -> None:
         """
         If there is no filename, Content-Disposition should only
@@ -491,6 +511,11 @@ def test_disposition_none(self) -> None:
             [b"inline" if self.test_image.is_inline else b"attachment"],
         )
 
+    @unittest.override_config(
+        {
+            "enable_authenticated_media": False,
+        }
+    )
     def test_thumbnail_crop(self) -> None:
         """Test that a cropped remote thumbnail is available."""
         self._test_thumbnail(
@@ -500,6 +525,11 @@ def test_thumbnail_crop(self) -> None:
             unable_to_thumbnail=self.test_image.unable_to_thumbnail,
         )
 
+    @unittest.override_config(
+        {
+            "enable_authenticated_media": False,
+        }
+    )
     def test_thumbnail_scale(self) -> None:
         """Test that a scaled remote thumbnail is available."""
         self._test_thumbnail(
@@ -509,6 +539,11 @@ def test_thumbnail_scale(self) -> None:
             unable_to_thumbnail=self.test_image.unable_to_thumbnail,
         )
 
+    @unittest.override_config(
+        {
+            "enable_authenticated_media": False,
+        }
+    )
     def test_invalid_type(self) -> None:
         """An invalid thumbnail type is never available."""
         self._test_thumbnail(
@@ -519,7 +554,10 @@ def test_invalid_type(self) -> None:
         )
 
     @unittest.override_config(
-        {"thumbnail_sizes": [{"width": 32, "height": 32, "method": "scale"}]}
+        {
+            "thumbnail_sizes": [{"width": 32, "height": 32, "method": "scale"}],
+            "enable_authenticated_media": False,
+        },
     )
     def test_no_thumbnail_crop(self) -> None:
         """
@@ -533,7 +571,10 @@ def test_no_thumbnail_crop(self) -> None:
         )
 
     @unittest.override_config(
-        {"thumbnail_sizes": [{"width": 32, "height": 32, "method": "crop"}]}
+        {
+            "thumbnail_sizes": [{"width": 32, "height": 32, "method": "crop"}],
+            "enable_authenticated_media": False,
+        }
     )
     def test_no_thumbnail_scale(self) -> None:
         """
@@ -546,6 +587,11 @@ def test_no_thumbnail_scale(self) -> None:
             unable_to_thumbnail=self.test_image.unable_to_thumbnail,
         )
 
+    @unittest.override_config(
+        {
+            "enable_authenticated_media": False,
+        }
+    )
     def test_thumbnail_repeated_thumbnail(self) -> None:
         """Test that fetching the same thumbnail works, and deleting the on disk
         thumbnail regenerates it.
@@ -720,6 +766,11 @@ def test_same_quality(self, method: str, desired_size: int) -> None:
             )
         )
 
+    @unittest.override_config(
+        {
+            "enable_authenticated_media": False,
+        }
+    )
     def test_x_robots_tag_header(self) -> None:
         """
         Tests that the `X-Robots-Tag` header is present, which informs web crawlers
@@ -733,6 +784,11 @@ def test_x_robots_tag_header(self) -> None:
             [b"noindex, nofollow, noarchive, noimageindex"],
         )
 
+    @unittest.override_config(
+        {
+            "enable_authenticated_media": False,
+        }
+    )
     def test_cross_origin_resource_policy_header(self) -> None:
         """
         Test that the Cross-Origin-Resource-Policy header is set to "cross-origin"
@@ -747,6 +803,11 @@ def test_cross_origin_resource_policy_header(self) -> None:
             [b"cross-origin"],
         )
 
+    @unittest.override_config(
+        {
+            "enable_authenticated_media": False,
+        }
+    )
     def test_unknown_v3_endpoint(self) -> None:
         """
         If the v3 endpoint fails, try the r0 one.
@@ -985,6 +1046,11 @@ def read_body_with_max_size_50MiB(*args: Any, **kwargs: Any) -> Deferred:
         d.callback(52428800)
         return d
 
+    @override_config(
+        {
+            "enable_authenticated_media": False,
+        }
+    )
     @patch(
         "synapse.http.matrixfederationclient.read_body_with_max_size",
         read_body_with_max_size_30MiB,
@@ -1060,6 +1126,7 @@ async def _send_request(*args: Any, **kwargs: Any) -> IResponse:
         {
             "remote_media_download_per_second": "50M",
             "remote_media_download_burst_count": "50M",
+            "enable_authenticated_media": False,
         }
     )
     @patch(
@@ -1119,7 +1186,12 @@ async def _send_request(*args: Any, **kwargs: Any) -> IResponse:
         )
         assert channel.code == 200
 
-    @override_config({"remote_media_download_burst_count": "87M"})
+    @override_config(
+        {
+            "remote_media_download_burst_count": "87M",
+            "enable_authenticated_media": False,
+        }
+    )
     @patch(
         "synapse.http.matrixfederationclient.read_body_with_max_size",
         read_body_with_max_size_30MiB,
@@ -1159,7 +1231,7 @@ async def _send_request(*args: Any, **kwargs: Any) -> IResponse:
         )
         assert channel2.code == 429
 
-    @override_config({"max_upload_size": "29M"})
+    @override_config({"max_upload_size": "29M", "enable_authenticated_media": False})
     @patch(
         "synapse.http.matrixfederationclient.read_body_with_max_size",
         read_body_with_max_size_30MiB,

tests/replication/test_multi_media_repo.py

@@ -40,6 +40,7 @@
 from tests.replication._base import BaseMultiWorkerStreamTestCase
 from tests.server import FakeChannel, FakeTransport, make_request
 from tests.test_utils import SMALL_PNG
+from tests.unittest import override_config
 
 logger = logging.getLogger(__name__)
 
@@ -148,6 +149,7 @@ def _get_media_req(
 
         return channel, request
 
+    @override_config({"enable_authenticated_media": False})
     def test_basic(self) -> None:
         """Test basic fetching of remote media from a single worker."""
         hs1 = self.make_worker_hs("synapse.app.generic_worker")
@@ -164,6 +166,7 @@ def test_basic(self) -> None:
         self.assertEqual(channel.code, 200)
         self.assertEqual(channel.result["body"], b"Hello!")
 
+    @override_config({"enable_authenticated_media": False})
     def test_download_simple_file_race(self) -> None:
         """Test that fetching remote media from two different processes at the
         same time works.
@@ -203,6 +206,7 @@ def test_download_simple_file_race(self) -> None:
         # We expect only one new file to have been persisted.
         self.assertEqual(start_count + 1, self._count_remote_media())
 
+    @override_config({"enable_authenticated_media": False})
     def test_download_image_race(self) -> None:
         """Test that fetching remote *images* from two different processes at
         the same time works.

tests/rest/admin/test_admin.py

@@ -30,7 +30,7 @@
 import synapse.rest.admin
 from synapse.http.server import JsonResource
 from synapse.rest.admin import VersionServlet
-from synapse.rest.client import login, room
+from synapse.rest.client import login, media, room
 from synapse.server import HomeServer
 from synapse.util import Clock
 
@@ -60,6 +60,7 @@ class QuarantineMediaTestCase(unittest.HomeserverTestCase):
         synapse.rest.admin.register_servlets,
         synapse.rest.admin.register_servlets_for_media_repo,
         login.register_servlets,
+        media.register_servlets,
         room.register_servlets,
     ]
 
@@ -74,7 +75,7 @@ def _ensure_quarantined(
         """Ensure a piece of media is quarantined when trying to access it."""
         channel = self.make_request(
             "GET",
-            f"/_matrix/media/v3/download/{server_and_media_id}",
+            f"/_matrix/client/v1/media/download/{server_and_media_id}",
             shorthand=False,
             access_token=admin_user_tok,
         )
@@ -131,7 +132,7 @@ def test_quarantine_media_by_id(self) -> None:
         # Attempt to access the media
         channel = self.make_request(
             "GET",
-            f"/_matrix/media/v3/download/{server_name_and_media_id}",
+            f"/_matrix/client/v1/media/download/{server_name_and_media_id}",
             shorthand=False,
             access_token=non_admin_user_tok,
         )
@@ -295,7 +296,7 @@ def test_cannot_quarantine_safe_media(self) -> None:
         # Attempt to access each piece of media
         channel = self.make_request(
             "GET",
-            f"/_matrix/media/v3/download/{server_and_media_id_2}",
+            f"/_matrix/client/v1/media/download/{server_and_media_id_2}",
             shorthand=False,
             access_token=non_admin_user_tok,
         )

tests/rest/admin/test_media.py

@@ -36,6 +36,7 @@
 
 from tests import unittest
 from tests.test_utils import SMALL_PNG
+from tests.unittest import override_config
 
 VALID_TIMESTAMP = 1609459200000  # 2021-01-01 in milliseconds
 INVALID_TIMESTAMP_IN_S = 1893456000  # 2030-01-01 in seconds
@@ -126,6 +127,7 @@ def test_media_is_not_local(self) -> None:
         self.assertEqual(400, channel.code, msg=channel.json_body)
         self.assertEqual("Can only delete local media", channel.json_body["error"])
 
+    @override_config({"enable_authenticated_media": False})
     def test_delete_media(self) -> None:
         """
         Tests that delete a media is successfully
@@ -371,6 +373,7 @@ def test_delete_media_never_accessed(self, use_legacy_url: bool) -> None:
 
         self._access_media(server_and_media_id, False)
 
+    @override_config({"enable_authenticated_media": False})
     def test_keep_media_by_date(self) -> None:
         """
         Tests that media is not deleted if it is newer than `before_ts`
@@ -408,6 +411,7 @@ def test_keep_media_by_date(self) -> None:
 
         self._access_media(server_and_media_id, False)
 
+    @override_config({"enable_authenticated_media": False})
     def test_keep_media_by_size(self) -> None:
         """
         Tests that media is not deleted if its size is smaller than or equal
@@ -443,6 +447,7 @@ def test_keep_media_by_size(self) -> None:
 
         self._access_media(server_and_media_id, False)
 
+    @override_config({"enable_authenticated_media": False})
     def test_keep_media_by_user_avatar(self) -> None:
         """
         Tests that we do not delete media if is used as a user avatar
@@ -487,6 +492,7 @@ def test_keep_media_by_user_avatar(self) -> None:
 
         self._access_media(server_and_media_id, False)
 
+    @override_config({"enable_authenticated_media": False})
     def test_keep_media_by_room_avatar(self) -> None:
         """
         Tests that we do not delete media if it is used as a room avatar