diff --git a/changelog.d/18686.feature b/changelog.d/18686.feature new file mode 100644 index 0000000000..648529d6d4 --- /dev/null +++ b/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`. diff --git a/docs/setup/forward_proxy.md b/docs/setup/forward_proxy.md index f02c7b5fc5..eab8bb9951 100644 --- a/docs/setup/forward_proxy.md +++ b/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 diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index fc838a1f0e..f1ab996810 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/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. diff --git a/schema/synapse-config.schema.yaml b/schema/synapse-config.schema.yaml index 65cc12d17c..45807a81f1 100644 --- a/schema/synapse-config.schema.yaml +++ b/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: >- diff --git a/synapse/config/repository.py b/synapse/config/repository.py index e6a5064c16..efdc505659 100644 --- a/synapse/config/repository.py +++ b/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. diff --git a/synapse/config/server.py b/synapse/config/server.py index 6893450989..e15bceb296 100644 --- a/synapse/config/server.py +++ b/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 ) diff --git a/synapse/http/client.py b/synapse/http/client.py index 928bfb228a..dcaaafe45d 100644 --- a/synapse/http/client.py +++ b/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: diff --git a/synapse/http/federation/matrix_federation_agent.py b/synapse/http/federation/matrix_federation_agent.py index 15609a799f..6ebadf0dbf 100644 --- a/synapse/http/federation/matrix_federation_agent.py +++ b/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 diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 0013b97723..31bde39c71 100644 --- a/synapse/http/matrixfederationclient.py +++ b/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, ) diff --git a/synapse/http/proxyagent.py b/synapse/http/proxyagent.py index 6217f9b0b2..ab413990c5 100644 --- a/synapse/http/proxyagent.py +++ b/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 ( diff --git a/tests/handlers/test_typing.py b/tests/handlers/test_typing.py index 1126b6f183..dd3116c929 100644 --- a/tests/handlers/test_typing.py +++ b/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 diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index a1243b053d..12428e64a9 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/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", diff --git a/tests/http/test_proxyagent.py b/tests/http/test_proxyagent.py index 2ef8a95c45..5bc5d18d81 100644 --- a/tests/http/test_proxyagent.py +++ b/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) diff --git a/tests/replication/test_federation_sender_shard.py b/tests/replication/test_federation_sender_shard.py index 5b7ed95a23..10970963c7 100644 --- a/tests/replication/test_federation_sender_shard.py +++ b/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: