Require download.base_homeserver_url, remove per-origin media fetching

README.md

@@ -58,11 +58,11 @@ deployment instructions) is the configuration format:
 
 * the `server` section is renamed `web`
 * `scan.tempDirectory` is renamed `scan.temp_directory`
-* `scan.baseUrl` is renamed `download.base_homeserver_url` (and becomes optional)
+* `scan.baseUrl` is renamed `download.base_homeserver_url` and is now required. All media is
+  downloaded via the homeserver at this URL, regardless of which homeserver it originated from.
 * `scan.doNotCacheExitCodes` is renamed `result_cache.exit_codes_to_ignore`
-* `scan.directDownload` is removed. Direct download always happens when `download.base_homeserver_url`
-  is absent from the configuration file, and setting a value for it will always cause files to be
-  downloaded from the server configured.
+* `scan.directDownload` is removed. Files are always downloaded from the homeserver configured
+  in `download.base_homeserver_url`.
 * `proxy` is renamed `download.proxy`
 * `middleware.encryptedBody.pickleKey` is renamed `crypto.pickle_key`
 * `middleware.encryptedBody.picklePath` is renamed `crypto.pickle_path`

config.sample.yaml

@@ -87,18 +87,13 @@ result_cache:
 
 
 # Configuration for downloading files.
-# When downloading files directly from their respective homeservers (which is the default
-# behaviour), the homeservers' default URLs are determined using .well-known discovery
-# (defaults to using the homeserver's domain if not available).
-# See https://spec.matrix.org/latest/client-server-api/#server-discovery for more info.
-# Settings in this section (apart from `base_homeserver_url`) apply to .well-known
-# discovery requests as well as file download ones.
 download:
-    # If provided, all files are downloaded using the homeserver at this URL. If this
-    # setting is provided, .well-known discovery is not used to determine the base URL
-    # to use.
-    # Optional, defaults to downloading files directly from their respective homeservers.
-    base_homeserver_url: "https://matrix.org"
+    # All files are downloaded via the homeserver at this URL, regardless of which
+    # homeserver they originated from. The content scanner never contacts a media's
+    # origin homeserver, so that it never sends an Authorization header
+    # (and thus an access token) to a server other than this one.
+    # It is expected that there is one instance of content scanner per one homeserver.
+    base_homeserver_url: "https://matrix-client.matrix.org"
 
     # HTTP(S) proxy to use when sending requests.
     # Optional, defaults to no proxy.

src/matrix_content_scanner/config.py

@@ -58,7 +58,7 @@ def _parse_size(size: Optional[Union[str, float]]) -> Optional[float]:
 # Schema to validate the raw configuration dictionary against.
 _config_schema = {
     "type": "object",
-    "required": ["web", "scan", "crypto"],
+    "required": ["web", "scan", "crypto", "download"],
     "additionalProperties": False,
     "properties": {
         "web": {
@@ -84,6 +84,7 @@ def _parse_size(size: Optional[Union[str, float]]) -> Optional[float]:
         },
         "download": {
             "type": "object",
+            "required": ["base_homeserver_url"],
             "additionalProperties": False,
             "properties": {
                 "base_homeserver_url": {"type": "string"},
@@ -153,7 +154,7 @@ class ResultCacheConfig:
 class DownloadConfig:
     """Configuration for downloading files."""
 
-    base_homeserver_url: Optional[str] = None
+    base_homeserver_url: str
     proxy: Optional[str] = None
     additional_headers: Optional[Dict[str, str]] = None
     headers_to_forward: Optional[List[str]] = None

src/matrix_content_scanner/scanner/file_downloader.py

@@ -6,16 +6,13 @@
 import logging
 import urllib.parse
 from http import HTTPStatus
-from typing import TYPE_CHECKING, Dict, Optional, Tuple
+from typing import TYPE_CHECKING, Optional, Tuple
 
 import aiohttp
 from multidict import CIMultiDict, CIMultiDictProxy, MultiMapping
 
 from matrix_content_scanner.utils.constants import ErrCode
-from matrix_content_scanner.utils.errors import (
-    ContentScannerRestError,
-    WellKnownDiscoveryError,
-)
+from matrix_content_scanner.utils.errors import ContentScannerRestError
 from matrix_content_scanner.utils.types import MediaDescription
 
 if TYPE_CHECKING:
@@ -38,7 +35,6 @@ class FileDownloader:
 
     def __init__(self, mcs: "MatrixContentScanner"):
         self._base_url = mcs.config.download.base_homeserver_url
-        self._well_known_cache: Dict[str, Optional[str]] = {}
         self._proxy_url = mcs.config.download.proxy
         self._additional_headers = (
             mcs.config.download.additional_headers
@@ -149,37 +145,17 @@ async def _build_https_url(
                 this is either "v3" or "r0".
 
         Returns:
-            An https URL to use. If `base_homeserver_url` is set in the config, this
-            will be used as the base of the URL.
+            A URL to use, always based on `base_homeserver_url`.
         """
         server_name, media_id = media_path.split("/")
 
-        # Figure out what base URL to use. If one is specified in the configuration file,
-        # use it, otherwise try to discover one using .well-known. If that fails, use the
-        # server name with an HTTPS scheme.
-        if self._base_url is not None:
-            base_url = self._base_url
-        else:
-            base_url = None
-
-            try:
-                base_url = await self._discover_via_well_known(server_name)
-            except WellKnownDiscoveryError as e:
-                # We don't catch ContentScannerRestErrors here because if one makes its
-                # way up here then it likely means that trying to reach https://server_name
-                # failed, in which case we're unlikely to be able to reach it again when
-                # downloading the file, so we let the error escalate.
-                logger.info("Failed to discover server via well-known: %s", e)
-
-            if base_url is None:
-                # base_url might be None if either .well-known discovery failed, or we
-                # didn't find a .well-known file.
-                base_url = "https://" + server_name
-
-        # Build the full URL.
+        # Build the full URL, always against the configured base homeserver URL. We
+        # never contact the media's origin homeserver directly, so that we never send
+        # an Authorization header to a server other than the one we've been configured
+        # to trust.
         path_prefix = prefix % endpoint_version
         url = "%s/%s/%s/%s" % (
-            base_url,
+            self._base_url,
             path_prefix,
             urllib.parse.quote(server_name),
             urllib.parse.quote(media_id),
@@ -279,88 +255,6 @@ async def _get_file_content(
             response_headers=headers,
         )
 
-    async def _discover_via_well_known(self, domain: str) -> Optional[str]:
-        """Try to discover the base URL for the given domain via .well-known client
-        discovery.
-
-        Args:
-            domain: The domain to discover the base URL for.
-
-        Returns:
-            The base URL to use, or None if no .well-known client file exist for this
-            domain.
-
-        Raises:
-            WellKnownDiscoveryError if an error happened during the discovery attempt.
-        """
-        # Check if we already have a result cached, and if so return with it straight
-        # away.
-        if domain in self._well_known_cache:
-            logger.info("Fetching .well-known discovery result from cache")
-            return self._well_known_cache[domain]
-
-        # Attempt to download the .well-known file.
-        try:
-            url = f"https://{domain}/.well-known/matrix/client"
-            code, body, _ = await self._get(url)
-        except ContentScannerRestError:
-            raise WellKnownDiscoveryError(f"Failed to reach web server at {domain}")
-
-        if code != 200:
-            if code == 404:
-                # If the response status is 404, then the homeserver hasn't set up
-                # .well-known discovery, in which case we tell the caller that there's
-                # no base URL to use rather than raising an error.
-                # The difference is that we want to cache this result here, but we don't
-                # want to do that when the discovery fails due to an incorrectly set up
-                # file or an unavailable homeserver, which might be fixed later on.
-                logger.info(
-                    ".well-known discover has not been set up for this homeserver"
-                )
-                self._well_known_cache[domain] = None
-                return None
-
-            raise WellKnownDiscoveryError(
-                f"Server responded with non-200 status {code}"
-            )
-
-        # Try to parse the JSON content.
-        try:
-            parsed_body = json.loads(body)
-        except json.decoder.JSONDecodeError as e:
-            raise WellKnownDiscoveryError(e)
-
-        # Check if the parsed content has a base URL in the right place.
-        try:
-            base_url: str = parsed_body["m.homeserver"]["base_url"]
-        except (KeyError, TypeError):
-            # We might get a KeyError if we're trying to reach a key that doesn't exist,
-            # and we might get a TypeError if parsed_body or parsed_body["m.homeserver"]
-            # isn't a dictionary.
-            raise WellKnownDiscoveryError("Response did not include a usable URL")
-
-        # Remove the trailing slash if there is one.
-        if base_url.endswith("/"):
-            base_url = base_url[:-1]
-
-        # Check if the base URL is one for a working homeserver.
-        url = base_url + "/_matrix/client/versions"
-        try:
-            code, _, _ = await self._get(url)
-        except ContentScannerRestError:
-            raise WellKnownDiscoveryError(
-                "Base URL does not seem to point to a working homeserver"
-            )
-
-        if code != 200:
-            raise WellKnownDiscoveryError(
-                "Base URL does not seem to point to a working homeserver"
-            )
-
-        # Cache and return the result.
-        self._well_known_cache[domain] = base_url
-        return base_url
-
     async def _get(
         self,
         url: str,

src/matrix_content_scanner/utils/errors.py

@@ -53,7 +53,3 @@ def __init__(self, info: Optional[str]) -> None:
 
 class ConfigError(Exception):
     """An error indicating an issue with the configuration file."""
-
-
-class WellKnownDiscoveryError(Exception):
-    """An error indicating a failure when attempting a .well-known discovery."""