Fix MatrixFederationAgentTests

Conflicts:
	synapse/http/federation/matrix_federation_agent.py
	synapse/http/matrixfederationclient.py
	tests/handlers/test_typing.py
	tests/replication/test_federation_sender_shard.py

changelog.d/18686.feature

@@ -0,0 +1 @@
+Add ability to configure forward/outbound proxy via homeserver config instead of environment variables. See `http_proxy`, `https_proxy`, `no_proxy_hosts`.

docs/setup/forward_proxy.md

@@ -7,8 +7,23 @@ proxy is supported, not SOCKS proxy or anything else.
 
 ## Configure
 
-The `http_proxy`, `https_proxy`, `no_proxy` environment variables are used to
-specify proxy settings. The environment variable is not case sensitive.
+The proxy settings can be configured in the homeserver configuration file via
+[`http_proxy`](../usage/configuration/config_documentation.md#http_proxy),
+[`https_proxy`](../usage/configuration/config_documentation.md#https_proxy), and
+[`no_proxy_hosts`](../usage/configuration/config_documentation.md#no_proxy_hosts).
+
+`homeserver.yaml` example:
+```yaml
+http_proxy: http://USERNAME:PASSWORD@10.0.1.1:8080/
+https_proxy: http://USERNAME:PASSWORD@proxy.example.com:8080/
+no_proxy_hosts:
+  - master.hostname.example.com
+  - 10.1.0.0/16
+  - 172.30.0.0/16
+```
+
+The proxy settings can also be configured via the `http_proxy`, `https_proxy`,
+`no_proxy` environment variables. The environment variable is not case sensitive.
 - `http_proxy`: Proxy server to use for HTTP requests.
 - `https_proxy`: Proxy server to use for HTTPS requests.
 - `no_proxy`: Comma-separated list of hosts, IP addresses, or IP ranges in CIDR
@@ -44,7 +59,7 @@ The proxy will be **used** for:
 - phone-home stats
 - recaptcha validation
 - CAS auth validation
-- OpenID Connect
+- OpenID Connect (OIDC)
 - Outbound federation
 - Federation (checking public key revocation)
 - Fetching public keys of other servers
@@ -53,7 +68,7 @@ The proxy will be **used** for:
 It will **not be used** for:
 
 - Application Services
-- Identity servers
+- Matrix Identity servers
 - In worker configurations
   - connections between workers
   - connections from workers to Redis

docs/usage/configuration/config_documentation.md

@@ -610,6 +610,39 @@ manhole_settings:
   ssh_pub_key_path: CONFDIR/id_rsa.pub
 ```
 ---
+### `http_proxy`
+
+*(string|null)* Proxy server to use for HTTP requests.
+For more details, see the [forward proxy documentation](../../setup/forward_proxy.md). There is no default for this option.
+
+Example configuration:
+```yaml
+http_proxy: http://USERNAME:PASSWORD@10.0.1.1:8080/
+```
+---
+### `https_proxy`
+
+*(string|null)* Proxy server to use for HTTPS requests.
+For more details, see the [forward proxy documentation](../../setup/forward_proxy.md). There is no default for this option.
+
+Example configuration:
+```yaml
+https_proxy: http://USERNAME:PASSWORD@proxy.example.com:8080/
+```
+---
+### `no_proxy_hosts`
+
+*(array)* List of hosts, IP addresses, or IP ranges in CIDR format which should not use the proxy. Synapse will directly connect to these hosts.
+For more details, see the [forward proxy documentation](../../setup/forward_proxy.md). There is no default for this option.
+
+Example configuration:
+```yaml
+no_proxy_hosts:
+- master.hostname.example.com
+- 10.1.0.0/16
+- 172.30.0.0/16
+```
+---
 ### `dummy_events_threshold`
 
 *(integer)* Forward extremities can build up in a room due to networking delays between homeservers. Once this happens in a large room, calculation of the state of that room can become quite expensive. To mitigate this, once the number of forward extremities reaches a given threshold, Synapse will send an `org.matrix.dummy_event` event, which will reduce the forward extremities in the room.

schema/synapse-config.schema.yaml

@@ -629,6 +629,33 @@ properties:
         password: mypassword
         ssh_priv_key_path: CONFDIR/id_rsa
         ssh_pub_key_path: CONFDIR/id_rsa.pub
+  http_proxy:
+    type: ["string", "null"]
+    description: >-
+      Proxy server to use for HTTP requests.
+
+      For more details, see the [forward proxy documentation](../../setup/forward_proxy.md).
+    examples:
+      - "http://USERNAME:PASSWORD@10.0.1.1:8080/"
+  https_proxy:
+    type: ["string", "null"]
+    description: >-
+      Proxy server to use for HTTPS requests.
+
+      For more details, see the [forward proxy documentation](../../setup/forward_proxy.md).
+    examples:
+      - "http://USERNAME:PASSWORD@proxy.example.com:8080/"
+  no_proxy_hosts:
+    type: array
+    description: >-
+      List of hosts, IP addresses, or IP ranges in CIDR format which should not use the
+      proxy. Synapse will directly connect to these hosts.
+
+      For more details, see the [forward proxy documentation](../../setup/forward_proxy.md).
+    examples:
+      - - master.hostname.example.com
+        - 10.1.0.0/16
+        - 172.30.0.0/16
   dummy_events_threshold:
     type: integer
     description: >-

synapse/config/repository.py

@@ -22,11 +22,10 @@
 import logging
 import os
 from typing import Any, Dict, List, Tuple
-from urllib.request import getproxies_environment
 
 import attr
 
-from synapse.config.server import generate_ip_set
+from synapse.config.server import generate_ip_set, parse_proxy_config
 from synapse.types import JsonDict
 from synapse.util.check_dependencies import check_requirements
 from synapse.util.module_loader import load_module
@@ -61,7 +60,7 @@ THUMBNAIL_SUPPORTED_MEDIA_FORMAT_MAP = {
     "image/png": "png",
 }
 
-HTTP_PROXY_SET_WARNING = """\
+URL_PREVIEW_BLACKLIST_IGNORED_BECAUSE_HTTP_PROXY_SET_WARNING = """\
 The Synapse config url_preview_ip_range_blacklist will be ignored as an HTTP(s) proxy is configured."""
 
 
@@ -234,17 +233,25 @@ class ContentRepositoryConfig(Config):
         if self.url_preview_enabled:
             check_requirements("url-preview")
 
-            proxy_env = getproxies_environment()
-            if "url_preview_ip_range_blacklist" not in config:
-                if "http" not in proxy_env or "https" not in proxy_env:
+            proxy_config = parse_proxy_config(config)
+            is_proxy_configured = (
+                proxy_config.http_proxy is not None
+                or proxy_config.https_proxy is not None
+            )
+            if "url_preview_ip_range_blacklist" in config:
+                if is_proxy_configured:
+                    logger.warning(
+                        "".join(
+                            URL_PREVIEW_BLACKLIST_IGNORED_BECAUSE_HTTP_PROXY_SET_WARNING
+                        )
+                    )
+            else:
+                if not is_proxy_configured:
                     raise ConfigError(
                         "For security, you must specify an explicit target IP address "
                         "blacklist in url_preview_ip_range_blacklist for url previewing "
                         "to work"
                     )
-            else:
-                if "http" in proxy_env or "https" in proxy_env:
-                    logger.warning("".join(HTTP_PROXY_SET_WARNING))
 
             # we always block '0.0.0.0' and '::', which are supposed to be
             # unroutable addresses.

synapse/config/server.py

@@ -25,11 +25,13 @@ import logging
 import os.path
 import urllib.parse
 from textwrap import indent
-from typing import Any, Dict, Iterable, List, Optional, Set, Tuple, Union
+from typing import Any, Dict, Iterable, List, Optional, Set, Tuple, TypedDict, Union
+from urllib.request import getproxies_environment
 
 import attr
 import yaml
 from netaddr import AddrFormatError, IPNetwork, IPSet
+from typing_extensions import TypeGuard
 
 from twisted.conch.ssh.keys import Key
 
@@ -43,6 +45,21 @@ from ._util import validate_config
 
 logger = logging.getLogger(__name__)
 
+
+# Directly from the mypy docs:
+# https://typing.python.org/en/latest/spec/narrowing.html#typeguard
+def is_str_list(val: Any, allow_empty: bool) -> TypeGuard[list[str]]:
+    """
+    Type-narrow a value to a list of strings (compatible with mypy).
+    """
+    if not isinstance(val, list):
+        return False
+
+    if len(val) == 0:
+        return allow_empty
+    return all(isinstance(x, str) for x in val)
+
+
 DIRECT_TCP_ERROR = """
 Using direct TCP replication for workers is no longer supported.
 
@@ -291,6 +308,102 @@ class LimitRemoteRoomsConfig:
     )
 
 
+class ProxyConfigDictionary(TypedDict):
+    """
+    Dictionary of proxy settings suitable for interacting with `urllib.request` API's
+    """
+
+    http: Optional[str]
+    """
+    Proxy server to use for HTTP requests.
+    """
+    https: Optional[str]
+    """
+    Proxy server to use for HTTPS requests.
+    """
+    no: str
+    """
+    Comma-separated list of hosts, IP addresses, or IP ranges in CIDR format which
+    should not use the proxy.
+
+    Empty string means no hosts should be excluded from the proxy.
+    """
+
+
+@attr.s(slots=True, frozen=True, auto_attribs=True)
+class ProxyConfig:
+    """
+    Synapse configuration for HTTP proxy settings.
+    """
+
+    http_proxy: Optional[str]
+    """
+    Proxy server to use for HTTP requests.
+    """
+    https_proxy: Optional[str]
+    """
+    Proxy server to use for HTTPS requests.
+    """
+    no_proxy_hosts: Optional[List[str]]
+    """
+    List of hosts, IP addresses, or IP ranges in CIDR format which should not use the
+    proxy. Synapse will directly connect to these hosts.
+    """
+
+    def get_proxies_dictionary(self) -> ProxyConfigDictionary:
+        """
+        Returns a dictionary of proxy settings suitable for interacting with
+        `urllib.request` API's (e.g. `urllib.request.proxy_bypass_environment`)
+
+        The keys are `"http"`, `"https"`, and `"no"`.
+        """
+        return ProxyConfigDictionary(
+            http=self.http_proxy,
+            https=self.https_proxy,
+            no=",".join(self.no_proxy_hosts) if self.no_proxy_hosts else "",
+        )
+
+
+def parse_proxy_config(config: JsonDict) -> ProxyConfig:
+    """
+    Figure out forward proxy config for outgoing HTTP requests.
+
+    Prefer values from the given config over the environment variables (`http_proxy`,
+    `https_proxy`, `no_proxy`, not case-sensitive).
+
+    Args:
+        config: The top-level homeserver configuration dictionary.
+    """
+    proxies_from_env = getproxies_environment()
+    http_proxy = config.get("http_proxy", proxies_from_env.get("http"))
+    if http_proxy is not None and not isinstance(http_proxy, str):
+        raise ConfigError("'http_proxy' must be a string", ("http_proxy",))
+
+    https_proxy = config.get("https_proxy", proxies_from_env.get("https"))
+    if https_proxy is not None and not isinstance(https_proxy, str):
+        raise ConfigError("'https_proxy' must be a string", ("https_proxy",))
+
+    # List of hosts which should not use the proxy. Synapse will directly connect to
+    # these hosts.
+    no_proxy_hosts = config.get("no_proxy_hosts")
+    # The `no_proxy` environment variable should be a comma-separated list of hosts,
+    # IP addresses, or IP ranges in CIDR format
+    no_proxy_from_env = proxies_from_env.get("no")
+    if no_proxy_hosts is None and no_proxy_from_env is not None:
+        no_proxy_hosts = no_proxy_from_env.split(",")
+
+    if no_proxy_hosts is not None and not is_str_list(no_proxy_hosts, allow_empty=True):
+        raise ConfigError(
+            "'no_proxy_hosts' must be a list of strings", ("no_proxy_hosts",)
+        )
+
+    return ProxyConfig(
+        http_proxy=http_proxy,
+        https_proxy=https_proxy,
+        no_proxy_hosts=no_proxy_hosts,
+    )
+
+
 class ServerConfig(Config):
     section = "server"
 
@@ -718,6 +831,17 @@ class ServerConfig(Config):
                 )
             )
 
+        # Figure out forward proxy config for outgoing HTTP requests.
+        #
+        # Prefer values from the file config over the environment variables
+        self.proxy_config = parse_proxy_config(config)
+        logger.debug(
+            "Using proxy settings: http_proxy=%s, https_proxy=%s, no_proxy=%s",
+            self.proxy_config.http_proxy,
+            self.proxy_config.https_proxy,
+            self.proxy_config.no_proxy_hosts,
+        )
+
         self.cleanup_extremities_with_dummy_events = config.get(
             "cleanup_extremities_with_dummy_events", True
         )

synapse/http/client.py

@@ -821,12 +821,12 @@ class SimpleHttpClient(BaseHttpClient):
         pool.cachedConnectionTimeout = 2 * 60
 
         self.agent: IAgent = ProxyAgent(
-            self.reactor,
-            hs.get_reactor(),
+            reactor=self.reactor,
+            proxy_reactor=hs.get_reactor(),
             connectTimeout=15,
             contextFactory=self.hs.get_http_client_context_factory(),
             pool=pool,
-            use_proxy=use_proxy,
+            proxy_config=hs.config.server.proxy_config,
         )
 
         if self._ip_blocklist:

synapse/http/federation/matrix_federation_agent.py

@@ -21,7 +21,6 @@ import logging
 import urllib.parse
 from typing import Any, Generator, List, Optional
 from urllib.request import (  # type: ignore[attr-defined]
-    getproxies_environment,
     proxy_bypass_environment,
 )
 
@@ -40,6 +39,7 @@ from twisted.web.client import URI, Agent, HTTPConnectionPool
 from twisted.web.http_headers import Headers
 from twisted.web.iweb import IAgent, IAgentEndpointFactory, IBodyProducer, IResponse
 
+from synapse.config.server import ProxyConfig
 from synapse.crypto.context_factory import FederationPolicyForHTTPS
 from synapse.http import proxyagent
 from synapse.http.client import BlocklistingAgentWrapper, BlocklistingReactorWrapper
@@ -77,6 +77,8 @@ class MatrixFederationAgent:
 
         ip_blocklist: Disallowed IP addresses.
 
+        proxy_config: Proxy configuration to use for this agent.
+
         proxy_reactor: twisted reactor to use for connections to the proxy server
            reactor might have some blocking applied (i.e. for DNS queries),
            but we need unblocked access to the proxy.
@@ -92,12 +94,14 @@ class MatrixFederationAgent:
 
     def __init__(
         self,
+        *,
         server_name: str,
         reactor: ISynapseReactor,
         tls_client_options_factory: Optional[FederationPolicyForHTTPS],
         user_agent: bytes,
         ip_allowlist: Optional[IPSet],
         ip_blocklist: IPSet,
+        proxy_config: Optional[ProxyConfig] = None,
         _srv_resolver: Optional[SrvResolver] = None,
         _well_known_resolver: Optional[WellKnownResolver] = None,
     ):
@@ -129,10 +133,11 @@ class MatrixFederationAgent:
         self._agent = Agent.usingEndpointFactory(
             reactor,
             MatrixHostnameEndpointFactory(
-                reactor,
-                proxy_reactor,
-                tls_client_options_factory,
-                _srv_resolver,
+                reactor=reactor,
+                proxy_reactor=proxy_reactor,
+                tls_client_options_factory=tls_client_options_factory,
+                srv_resolver=_srv_resolver,
+                proxy_config=proxy_config,
             ),
             pool=self._pool,
         )
@@ -144,11 +149,11 @@ class MatrixFederationAgent:
                 reactor=reactor,
                 agent=BlocklistingAgentWrapper(
                     ProxyAgent(
-                        reactor,
-                        proxy_reactor,
+                        reactor=reactor,
+                        proxy_reactor=proxy_reactor,
                         pool=self._pool,
                         contextFactory=tls_client_options_factory,
-                        use_proxy=True,
+                        proxy_config=proxy_config,
                     ),
                     ip_blocklist=ip_blocklist,
                 ),
@@ -246,14 +251,17 @@ class MatrixHostnameEndpointFactory:
 
     def __init__(
         self,
+        *,
         reactor: IReactorCore,
         proxy_reactor: IReactorCore,
         tls_client_options_factory: Optional[FederationPolicyForHTTPS],
         srv_resolver: Optional[SrvResolver],
+        proxy_config: Optional[ProxyConfig],
     ):
         self._reactor = reactor
         self._proxy_reactor = proxy_reactor
         self._tls_client_options_factory = tls_client_options_factory
+        self._proxy_config = proxy_config
 
         if srv_resolver is None:
             srv_resolver = SrvResolver()
@@ -262,11 +270,12 @@ class MatrixHostnameEndpointFactory:
 
     def endpointForURI(self, parsed_uri: URI) -> "MatrixHostnameEndpoint":
         return MatrixHostnameEndpoint(
-            self._reactor,
-            self._proxy_reactor,
-            self._tls_client_options_factory,
-            self._srv_resolver,
-            parsed_uri,
+            reactor=self._reactor,
+            proxy_reactor=self._proxy_reactor,
+            tls_client_options_factory=self._tls_client_options_factory,
+            srv_resolver=self._srv_resolver,
+            proxy_config=self._proxy_config,
+            parsed_uri=parsed_uri,
         )
 
 
@@ -283,6 +292,7 @@ class MatrixHostnameEndpoint:
         tls_client_options_factory:
             factory to use for fetching client tls options, or none to disable TLS.
         srv_resolver: The SRV resolver to use
+        proxy_config: Proxy configuration to use for this agent.
         parsed_uri: The parsed URI that we're wanting to connect to.
 
     Raises:
@@ -292,26 +302,28 @@ class MatrixHostnameEndpoint:
 
     def __init__(
         self,
+        *,
         reactor: IReactorCore,
         proxy_reactor: IReactorCore,
         tls_client_options_factory: Optional[FederationPolicyForHTTPS],
         srv_resolver: SrvResolver,
+        proxy_config: Optional[ProxyConfig],
         parsed_uri: URI,
     ):
         self._reactor = reactor
         self._parsed_uri = parsed_uri
+        self.proxy_config = proxy_config
 
         # http_proxy is not needed because federation is always over TLS
-        proxies = getproxies_environment()
-        https_proxy = proxies["https"].encode() if "https" in proxies else None
-        self.no_proxy = proxies["no"] if "no" in proxies else None
 
         # endpoint and credentials to use to connect to the outbound https proxy, if any.
         (
             self._https_proxy_endpoint,
             self._https_proxy_creds,
         ) = proxyagent.http_proxy_endpoint(
-            https_proxy,
+            self.proxy_config.https_proxy.encode()
+            if self.proxy_config and self.proxy_config.https_proxy
+            else None,
             proxy_reactor,
             tls_client_options_factory,
         )
@@ -348,10 +360,10 @@ class MatrixHostnameEndpoint:
             port = server.port
 
             should_skip_proxy = False
-            if self.no_proxy is not None:
+            if self.proxy_config is not None:
                 should_skip_proxy = proxy_bypass_environment(
                     host.decode(),
-                    proxies={"no": self.no_proxy},
+                    proxies=self.proxy_config.get_proxies_dictionary(),
                 )
 
             endpoint: IStreamClientEndpoint

synapse/http/matrixfederationclient.py

@@ -423,6 +423,7 @@ class MatrixFederationHttpClient:
                 user_agent=user_agent.encode("ascii"),
                 ip_allowlist=hs.config.server.federation_ip_range_allowlist,
                 ip_blocklist=hs.config.server.federation_ip_range_blocklist,
+                proxy_config=hs.config.server.proxy_config,
             )
         else:
             proxy_authorization_secret = hs.config.worker.worker_replication_secret
@@ -437,9 +438,9 @@ class MatrixFederationHttpClient:
             # locations
             federation_proxy_locations = outbound_federation_restricted_to.locations
             federation_agent = ProxyAgent(
-                self.reactor,
-                self.reactor,
-                tls_client_options_factory,
+                reactor=self.reactor,
+                proxy_reactor=self.reactor,
+                contextFactory=tls_client_options_factory,
                 federation_proxy_locations=federation_proxy_locations,
                 federation_proxy_credentials=federation_proxy_credentials,
             )

synapse/http/proxyagent.py

@@ -24,7 +24,6 @@ import re
 from typing import Any, Collection, Dict, List, Optional, Sequence, Tuple, Union, cast
 from urllib.parse import urlparse
 from urllib.request import (  # type: ignore[attr-defined]
-    getproxies_environment,
     proxy_bypass_environment,
 )
 
@@ -54,6 +53,7 @@ from twisted.web.error import SchemeNotSupported
 from twisted.web.http_headers import Headers
 from twisted.web.iweb import IAgent, IBodyProducer, IPolicyForHTTPS, IResponse
 
+from synapse.config.server import ProxyConfig
 from synapse.config.workers import (
     InstanceLocationConfig,
     InstanceTcpLocationConfig,
@@ -99,8 +99,7 @@ class ProxyAgent(_AgentBase):
         pool: connection pool to be used. If None, a
             non-persistent pool instance will be created.
 
-        use_proxy: Whether proxy settings should be discovered and used
-            from conventional environment variables.
+        proxy_config: Proxy configuration to use for this agent.
 
         federation_proxy_locations: An optional list of locations to proxy outbound federation
             traffic through (only requests that use the `matrix-federation://` scheme
@@ -118,13 +117,14 @@ class ProxyAgent(_AgentBase):
 
     def __init__(
         self,
+        *,
         reactor: IReactorCore,
         proxy_reactor: Optional[IReactorCore] = None,
         contextFactory: Optional[IPolicyForHTTPS] = None,
         connectTimeout: Optional[float] = None,
         bindAddress: Optional[bytes] = None,
         pool: Optional[HTTPConnectionPool] = None,
-        use_proxy: bool = False,
+        proxy_config: Optional[ProxyConfig] = None,
         federation_proxy_locations: Collection[InstanceLocationConfig] = (),
         federation_proxy_credentials: Optional[ProxyCredentials] = None,
     ):
@@ -145,31 +145,33 @@ class ProxyAgent(_AgentBase):
         if bindAddress is not None:
             self._endpoint_kwargs["bindAddress"] = bindAddress
 
-        http_proxy = None
-        https_proxy = None
-        no_proxy = None
-        if use_proxy:
-            proxies = getproxies_environment()
-            http_proxy = proxies["http"].encode() if "http" in proxies else None
-            https_proxy = proxies["https"].encode() if "https" in proxies else None
-            no_proxy = proxies["no"] if "no" in proxies else None
+        self.proxy_config = proxy_config
+        if self.proxy_config is not None:
             logger.debug(
                 "Using proxy settings: http_proxy=%s, https_proxy=%s, no_proxy=%s",
-                http_proxy,
-                https_proxy,
-                no_proxy,
+                self.proxy_config.http_proxy,
+                self.proxy_config.https_proxy,
+                self.proxy_config.no_proxy_hosts,
             )
 
         self.http_proxy_endpoint, self.http_proxy_creds = http_proxy_endpoint(
-            http_proxy, self.proxy_reactor, contextFactory, **self._endpoint_kwargs
+            self.proxy_config.http_proxy.encode()
+            if self.proxy_config and self.proxy_config.http_proxy
+            else None,
+            self.proxy_reactor,
+            contextFactory,
+            **self._endpoint_kwargs,
         )
 
         self.https_proxy_endpoint, self.https_proxy_creds = http_proxy_endpoint(
-            https_proxy, self.proxy_reactor, contextFactory, **self._endpoint_kwargs
+            self.proxy_config.https_proxy.encode()
+            if self.proxy_config and self.proxy_config.https_proxy
+            else None,
+            self.proxy_reactor,
+            contextFactory,
+            **self._endpoint_kwargs,
         )
 
-        self.no_proxy = no_proxy
-
         self._policy_for_https = contextFactory
         self._reactor = cast(IReactorTime, reactor)
 
@@ -268,10 +270,10 @@ class ProxyAgent(_AgentBase):
         request_path = parsed_uri.originForm
 
         should_skip_proxy = False
-        if self.no_proxy is not None:
+        if self.proxy_config is not None:
             should_skip_proxy = proxy_bypass_environment(
                 parsed_uri.host.decode(),
-                proxies={"no": self.no_proxy},
+                proxies=self.proxy_config.get_proxies_dictionary(),
             )
 
         if (

tests/handlers/test_typing.py

@@ -92,6 +92,7 @@ class TypingNotificationsTestCase(unittest.HomeserverTestCase):
             user_agent=b"SynapseInTrialTest/0.0.0",
             ip_allowlist=None,
             ip_blocklist=IPSet(),
+            proxy_config=None,
         )
 
         # the tests assume that we are starting at unix time 1000

tests/http/federation/test_matrix_federation_agent.py

@@ -45,6 +45,7 @@ from twisted.web.http_headers import Headers
 from twisted.web.iweb import IPolicyForHTTPS, IResponse
 
 from synapse.config.homeserver import HomeServerConfig
+from synapse.config.server import parse_proxy_config
 from synapse.crypto.context_factory import FederationPolicyForHTTPS
 from synapse.http.federation.matrix_federation_agent import MatrixFederationAgent
 from synapse.http.federation.srv_resolver import Server, SrvResolver
@@ -280,6 +281,7 @@ class MatrixFederationAgentTests(unittest.TestCase):
             user_agent=b"test-agent",  # Note that this is unused since _well_known_resolver is provided.
             ip_allowlist=IPSet(),
             ip_blocklist=IPSet(),
+            proxy_config=parse_proxy_config({}),
             _srv_resolver=self.mock_resolver,
             _well_known_resolver=self.well_known_resolver,
         )
@@ -1023,6 +1025,7 @@ class MatrixFederationAgentTests(unittest.TestCase):
             user_agent=b"test-agent",  # This is unused since _well_known_resolver is passed below.
             ip_allowlist=IPSet(),
             ip_blocklist=IPSet(),
+            proxy_config=None,
             _srv_resolver=self.mock_resolver,
             _well_known_resolver=WellKnownResolver(
                 server_name="OUR_STUB_HOMESERVER_NAME",

tests/http/test_proxyagent.py

@@ -39,6 +39,7 @@ from twisted.internet.protocol import Factory, Protocol
 from twisted.protocols.tls import TLSMemoryBIOProtocol
 from twisted.web.http import HTTPChannel
 
+from synapse.config.server import ProxyConfig, parse_proxy_config
 from synapse.http.client import BlocklistingReactorWrapper
 from synapse.http.connectproxyclient import BasicProxyCredentials
 from synapse.http.proxyagent import ProxyAgent, parse_proxy
@@ -241,7 +242,7 @@ class TestBasicProxyCredentials(TestCase):
         )
 
 
-class MatrixFederationAgentTests(TestCase):
+class ProxyAgentTests(TestCase):
     def setUp(self) -> None:
         self.reactor = ThreadedMemoryReactorClock()
 
@@ -379,27 +380,40 @@ class MatrixFederationAgentTests(TestCase):
         self.assertEqual(body, b"result")
 
     def test_http_request(self) -> None:
-        agent = ProxyAgent(self.reactor)
+        agent = ProxyAgent(reactor=self.reactor)
         self._test_request_direct_connection(agent, b"http", b"test.com", b"")
 
     def test_https_request(self) -> None:
-        agent = ProxyAgent(self.reactor, contextFactory=get_test_https_policy())
+        agent = ProxyAgent(reactor=self.reactor, contextFactory=get_test_https_policy())
         self._test_request_direct_connection(agent, b"https", b"test.com", b"abc")
 
-    def test_http_request_use_proxy_empty_environment(self) -> None:
-        agent = ProxyAgent(self.reactor, use_proxy=True)
+    def test_http_request_proxy_config_empty_environment(self) -> None:
+        agent = ProxyAgent(
+            reactor=self.reactor,
+            proxy_config=parse_proxy_config({}),
+        )
         self._test_request_direct_connection(agent, b"http", b"test.com", b"")
 
     @patch.dict(os.environ, {"http_proxy": "proxy.com:8888", "NO_PROXY": "test.com"})
     def test_http_request_via_uppercase_no_proxy(self) -> None:
-        agent = ProxyAgent(self.reactor, use_proxy=True)
+        """
+        Ensure hosts listed in the NO_PROXY environment variable are not sent via the
+        proxy.
+        """
+        agent = ProxyAgent(
+            reactor=self.reactor,
+            proxy_config=parse_proxy_config({}),
+        )
         self._test_request_direct_connection(agent, b"http", b"test.com", b"")
 
     @patch.dict(
         os.environ, {"http_proxy": "proxy.com:8888", "no_proxy": "test.com,unused.com"}
     )
     def test_http_request_via_no_proxy(self) -> None:
-        agent = ProxyAgent(self.reactor, use_proxy=True)
+        agent = ProxyAgent(
+            reactor=self.reactor,
+            proxy_config=parse_proxy_config({}),
+        )
         self._test_request_direct_connection(agent, b"http", b"test.com", b"")
 
     @patch.dict(
@@ -407,23 +421,26 @@ class MatrixFederationAgentTests(TestCase):
     )
     def test_https_request_via_no_proxy(self) -> None:
         agent = ProxyAgent(
-            self.reactor,
+            reactor=self.reactor,
             contextFactory=get_test_https_policy(),
-            use_proxy=True,
+            proxy_config=parse_proxy_config({}),
         )
         self._test_request_direct_connection(agent, b"https", b"test.com", b"abc")
 
     @patch.dict(os.environ, {"http_proxy": "proxy.com:8888", "no_proxy": "*"})
     def test_http_request_via_no_proxy_star(self) -> None:
-        agent = ProxyAgent(self.reactor, use_proxy=True)
+        agent = ProxyAgent(
+            reactor=self.reactor,
+            proxy_config=parse_proxy_config({}),
+        )
         self._test_request_direct_connection(agent, b"http", b"test.com", b"")
 
     @patch.dict(os.environ, {"https_proxy": "proxy.com", "no_proxy": "*"})
     def test_https_request_via_no_proxy_star(self) -> None:
         agent = ProxyAgent(
-            self.reactor,
+            reactor=self.reactor,
             contextFactory=get_test_https_policy(),
-            use_proxy=True,
+            proxy_config=parse_proxy_config({}),
         )
         self._test_request_direct_connection(agent, b"https", b"test.com", b"abc")
 
@@ -433,9 +450,72 @@ class MatrixFederationAgentTests(TestCase):
         Tests that requests can be made through a proxy.
         """
         self._do_http_request_via_proxy(
-            expect_proxy_ssl=False, expected_auth_credentials=None
+            proxy_config=parse_proxy_config({}),
+            expect_proxy_ssl=False,
+            expected_auth_credentials=None,
         )
 
+    def test_given_http_proxy_config(self) -> None:
+        self._do_http_request_via_proxy(
+            proxy_config=parse_proxy_config({"http_proxy": "proxy.com:8888"}),
+            expect_proxy_ssl=False,
+            expected_auth_credentials=None,
+        )
+
+    def test_given_https_proxy_config(self) -> None:
+        self._do_https_request_via_proxy(
+            proxy_config=parse_proxy_config({"https_proxy": "proxy.com"}),
+            expect_proxy_ssl=False,
+            expected_auth_credentials=None,
+        )
+
+    def test_given_no_proxy_hosts_config(self) -> None:
+        agent = ProxyAgent(
+            reactor=self.reactor,
+            proxy_config=parse_proxy_config(
+                {"http_proxy": "proxy.com:8888", "no_proxy_hosts": ["test.com"]}
+            ),
+        )
+        self._test_request_direct_connection(agent, b"http", b"test.com", b"")
+
+    @patch.dict(
+        os.environ,
+        {"http_proxy": "unused.com", "no_proxy": "unused.com"},
+    )
+    def test_given_http_proxy_config_overrides_environment_config(self) -> None:
+        """Tests that the given `http_proxy` in file config overrides the environment config."""
+        self._do_http_request_via_proxy(
+            proxy_config=parse_proxy_config({"http_proxy": "proxy.com:8888"}),
+            expect_proxy_ssl=False,
+            expected_auth_credentials=None,
+        )
+
+    @patch.dict(
+        os.environ,
+        {"https_proxy": "unused.com", "no_proxy": "unused.com"},
+    )
+    def test_given_https_proxy_config_overrides_environment_config(self) -> None:
+        """Tests that the given `https_proxy` in file config overrides the environment config."""
+        self._do_https_request_via_proxy(
+            proxy_config=parse_proxy_config({"https_proxy": "proxy.com"}),
+            expect_proxy_ssl=False,
+            expected_auth_credentials=None,
+        )
+
+    @patch.dict(
+        os.environ,
+        {"https_proxy": "unused.com", "no_proxy": "unused.com"},
+    )
+    def test_given_no_proxy_config_overrides_environment_config(self) -> None:
+        """Tests that the given `no_proxy_hosts` in file config overrides the `no_proxy` environment config."""
+        agent = ProxyAgent(
+            reactor=self.reactor,
+            proxy_config=parse_proxy_config(
+                {"http_proxy": "proxy.com:8888", "no_proxy_hosts": ["test.com"]}
+            ),
+        )
+        self._test_request_direct_connection(agent, b"http", b"test.com", b"")
+
     @patch.dict(
         os.environ,
         {"http_proxy": "bob:pinkponies@proxy.com:8888", "no_proxy": "unused.com"},
@@ -445,7 +525,9 @@ class MatrixFederationAgentTests(TestCase):
         Tests that authenticated requests can be made through a proxy.
         """
         self._do_http_request_via_proxy(
-            expect_proxy_ssl=False, expected_auth_credentials=b"bob:pinkponies"
+            proxy_config=parse_proxy_config({}),
+            expect_proxy_ssl=False,
+            expected_auth_credentials=b"bob:pinkponies",
         )
 
     @patch.dict(
@@ -453,7 +535,9 @@ class MatrixFederationAgentTests(TestCase):
     )
     def test_http_request_via_https_proxy(self) -> None:
         self._do_http_request_via_proxy(
-            expect_proxy_ssl=True, expected_auth_credentials=None
+            proxy_config=parse_proxy_config({}),
+            expect_proxy_ssl=True,
+            expected_auth_credentials=None,
         )
 
     @patch.dict(
@@ -465,14 +549,18 @@ class MatrixFederationAgentTests(TestCase):
     )
     def test_http_request_via_https_proxy_with_auth(self) -> None:
         self._do_http_request_via_proxy(
-            expect_proxy_ssl=True, expected_auth_credentials=b"bob:pinkponies"
+            proxy_config=parse_proxy_config({}),
+            expect_proxy_ssl=True,
+            expected_auth_credentials=b"bob:pinkponies",
         )
 
     @patch.dict(os.environ, {"https_proxy": "proxy.com", "no_proxy": "unused.com"})
     def test_https_request_via_proxy(self) -> None:
         """Tests that TLS-encrypted requests can be made through a proxy"""
         self._do_https_request_via_proxy(
-            expect_proxy_ssl=False, expected_auth_credentials=None
+            proxy_config=parse_proxy_config({}),
+            expect_proxy_ssl=False,
+            expected_auth_credentials=None,
         )
 
     @patch.dict(
@@ -482,7 +570,9 @@ class MatrixFederationAgentTests(TestCase):
     def test_https_request_via_proxy_with_auth(self) -> None:
         """Tests that authenticated, TLS-encrypted requests can be made through a proxy"""
         self._do_https_request_via_proxy(
-            expect_proxy_ssl=False, expected_auth_credentials=b"bob:pinkponies"
+            proxy_config=parse_proxy_config({}),
+            expect_proxy_ssl=False,
+            expected_auth_credentials=b"bob:pinkponies",
         )
 
     @patch.dict(
@@ -491,7 +581,9 @@ class MatrixFederationAgentTests(TestCase):
     def test_https_request_via_https_proxy(self) -> None:
         """Tests that TLS-encrypted requests can be made through a proxy"""
         self._do_https_request_via_proxy(
-            expect_proxy_ssl=True, expected_auth_credentials=None
+            proxy_config=parse_proxy_config({}),
+            expect_proxy_ssl=True,
+            expected_auth_credentials=None,
         )
 
     @patch.dict(
@@ -501,11 +593,14 @@ class MatrixFederationAgentTests(TestCase):
     def test_https_request_via_https_proxy_with_auth(self) -> None:
         """Tests that authenticated, TLS-encrypted requests can be made through a proxy"""
         self._do_https_request_via_proxy(
-            expect_proxy_ssl=True, expected_auth_credentials=b"bob:pinkponies"
+            proxy_config=parse_proxy_config({}),
+            expect_proxy_ssl=True,
+            expected_auth_credentials=b"bob:pinkponies",
         )
 
     def _do_http_request_via_proxy(
         self,
+        proxy_config: ProxyConfig,
         expect_proxy_ssl: bool = False,
         expected_auth_credentials: Optional[bytes] = None,
     ) -> None:
@@ -517,10 +612,15 @@ class MatrixFederationAgentTests(TestCase):
         """
         if expect_proxy_ssl:
             agent = ProxyAgent(
-                self.reactor, use_proxy=True, contextFactory=get_test_https_policy()
+                reactor=self.reactor,
+                proxy_config=proxy_config,
+                contextFactory=get_test_https_policy(),
             )
         else:
-            agent = ProxyAgent(self.reactor, use_proxy=True)
+            agent = ProxyAgent(
+                reactor=self.reactor,
+                proxy_config=proxy_config,
+            )
 
         self.reactor.lookups["proxy.com"] = "1.2.3.5"
         d = agent.request(b"GET", b"http://test.com")
@@ -580,6 +680,7 @@ class MatrixFederationAgentTests(TestCase):
 
     def _do_https_request_via_proxy(
         self,
+        proxy_config: ProxyConfig,
         expect_proxy_ssl: bool = False,
         expected_auth_credentials: Optional[bytes] = None,
     ) -> None:
@@ -590,9 +691,9 @@ class MatrixFederationAgentTests(TestCase):
             expected_auth_credentials: credentials to authenticate at proxy
         """
         agent = ProxyAgent(
-            self.reactor,
+            reactor=self.reactor,
             contextFactory=get_test_https_policy(),
-            use_proxy=True,
+            proxy_config=proxy_config,
         )
 
         self.reactor.lookups["proxy.com"] = "1.2.3.5"
@@ -713,11 +814,11 @@ class MatrixFederationAgentTests(TestCase):
     def test_http_request_via_proxy_with_blocklist(self) -> None:
         # The blocklist includes the configured proxy IP.
         agent = ProxyAgent(
-            BlocklistingReactorWrapper(
+            reactor=BlocklistingReactorWrapper(
                 self.reactor, ip_allowlist=None, ip_blocklist=IPSet(["1.0.0.0/8"])
             ),
-            self.reactor,
-            use_proxy=True,
+            proxy_reactor=self.reactor,
+            proxy_config=parse_proxy_config({}),
         )
 
         self.reactor.lookups["proxy.com"] = "1.2.3.5"
@@ -759,12 +860,12 @@ class MatrixFederationAgentTests(TestCase):
     def test_https_request_via_uppercase_proxy_with_blocklist(self) -> None:
         # The blocklist includes the configured proxy IP.
         agent = ProxyAgent(
-            BlocklistingReactorWrapper(
+            reactor=BlocklistingReactorWrapper(
                 self.reactor, ip_allowlist=None, ip_blocklist=IPSet(["1.0.0.0/8"])
             ),
-            self.reactor,
+            proxy_reactor=self.reactor,
             contextFactory=get_test_https_policy(),
-            use_proxy=True,
+            proxy_config=parse_proxy_config({}),
         )
 
         self.reactor.lookups["proxy.com"] = "1.2.3.5"
@@ -852,7 +953,10 @@ class MatrixFederationAgentTests(TestCase):
 
     @patch.dict(os.environ, {"http_proxy": "proxy.com:8888"})
     def test_proxy_with_no_scheme(self) -> None:
-        http_proxy_agent = ProxyAgent(self.reactor, use_proxy=True)
+        http_proxy_agent = ProxyAgent(
+            reactor=self.reactor,
+            proxy_config=parse_proxy_config({}),
+        )
         proxy_ep = checked_cast(HostnameEndpoint, http_proxy_agent.http_proxy_endpoint)
         self.assertEqual(proxy_ep._hostText, "proxy.com")
         self.assertEqual(proxy_ep._port, 8888)
@@ -860,18 +964,27 @@ class MatrixFederationAgentTests(TestCase):
     @patch.dict(os.environ, {"http_proxy": "socks://proxy.com:8888"})
     def test_proxy_with_unsupported_scheme(self) -> None:
         with self.assertRaises(ValueError):
-            ProxyAgent(self.reactor, use_proxy=True)
+            ProxyAgent(
+                reactor=self.reactor,
+                proxy_config=parse_proxy_config({}),
+            )
 
     @patch.dict(os.environ, {"http_proxy": "http://proxy.com:8888"})
     def test_proxy_with_http_scheme(self) -> None:
-        http_proxy_agent = ProxyAgent(self.reactor, use_proxy=True)
+        http_proxy_agent = ProxyAgent(
+            reactor=self.reactor,
+            proxy_config=parse_proxy_config({}),
+        )
         proxy_ep = checked_cast(HostnameEndpoint, http_proxy_agent.http_proxy_endpoint)
         self.assertEqual(proxy_ep._hostText, "proxy.com")
         self.assertEqual(proxy_ep._port, 8888)
 
     @patch.dict(os.environ, {"http_proxy": "https://proxy.com:8888"})
     def test_proxy_with_https_scheme(self) -> None:
-        https_proxy_agent = ProxyAgent(self.reactor, use_proxy=True)
+        https_proxy_agent = ProxyAgent(
+            reactor=self.reactor,
+            proxy_config=parse_proxy_config({}),
+        )
         proxy_ep = checked_cast(_WrapperEndpoint, https_proxy_agent.http_proxy_endpoint)
         self.assertEqual(proxy_ep._wrappedEndpoint._hostText, "proxy.com")
         self.assertEqual(proxy_ep._wrappedEndpoint._port, 8888)

tests/replication/test_federation_sender_shard.py

@@ -74,6 +74,7 @@ class FederationSenderTestCase(BaseMultiWorkerStreamTestCase):
             user_agent=b"SynapseInTrialTest/0.0.0",
             ip_allowlist=None,
             ip_blocklist=IPSet(),
+            proxy_config=None,
         )
 
     def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None: