Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

support federation queries through http connect proxy #10475

Merged
merged 32 commits into from
Aug 11, 2021
Merged
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
32 commits
Select commit Hold shift + click to select a range
85081fa
support federation queries through http connect proxy
Bubu Jan 11, 2021
7115567
HTTPConnectProxyEndpoint now want a headers parameter
Bubu Apr 26, 2021
ef8792a
make mypy happy
Bubu Apr 27, 2021
781da36
also make black happy :)
Bubu Apr 27, 2021
196af98
Merge remote-tracking branch 'synapse/develop' into federation_throug…
dklimpel Jul 24, 2021
86b3623
add type hints and comments to tests
dklimpel Jul 24, 2021
3c7c3b6
change logic of test to the same like in `test_proxyagent`
dklimpel Jul 24, 2021
cee7f65
Add http connections to `_make_connection` for http proxies
dklimpel Jul 24, 2021
e8cb087
add tests for proxy
dklimpel Jul 24, 2021
73c5ff5
add test for bypass proxy
dklimpel Jul 24, 2021
4cd4a48
put creation of `MatrixFederationAgent` in own function
dklimpel Jul 24, 2021
1595da4
comments and type hints to federation_agent
dklimpel Jul 24, 2021
5acd7ce
move proxy handling to `MatrixHostnameEndpoint` similar to `ProxyAgent`
dklimpel Jul 24, 2021
3342bda
newsfile
dklimpel Jul 24, 2021
0bf5ac8
fix imports
dklimpel Jul 24, 2021
b81eaaf
add upgrade notes
dklimpel Jul 26, 2021
08b19a6
Merge remote-tracking branch 'synapse/develop' into
dklimpel Jul 27, 2021
70a8b11
fix merge error and update tests
dklimpel Jul 27, 2021
b68a14d
fix lint
dklimpel Jul 27, 2021
b766a0f
Merge branch 'matrix-org:develop' into federation_through_proxy
dklimpel Aug 5, 2021
eab6c74
update docs
dklimpel Aug 5, 2021
73a7380
Apply suggestions from code review
dklimpel Aug 7, 2021
007ea8a
add to internal attributes underscore prefixes
dklimpel Aug 7, 2021
258fab8
rename param `ssl` to `expect_proxy_ssl`
dklimpel Aug 7, 2021
08823be
remove `_make_agent()` from `setUp` in tests
dklimpel Aug 7, 2021
67522c3
rename in test `auth_credentials` to `expected_auth_credentials`
dklimpel Aug 7, 2021
512166b
remove default TLS policy
dklimpel Aug 7, 2021
ab2067d
bring `ProxyCredentials` to `HTTPConnectProxyEndpoint`
dklimpel Aug 7, 2021
e8fd305
move build `BlacklistingReactorWrapper` into `MatrixFederationAgent`
dklimpel Aug 7, 2021
d0933e5
lint
dklimpel Aug 7, 2021
c953242
remove not needed reactor
dklimpel Aug 9, 2021
2720c88
Merge branch 'matrix-org:develop' into federation_through_proxy
dklimpel Aug 9, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/10475.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for sending federation requests through a proxy. Contributed by @Bubu and @dklimpel.
24 changes: 24 additions & 0 deletions docs/upgrade.md
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,30 @@ process, for example:
```


# Upgrading to v1.xx.0

## Add support for routing outbound HTTP requests via a proxy for federation

Since Synapse 1.6.0 (2019-11-26) you can set a proxy for outbound HTTP requests via
http_proxy/https_proxy environment variables. This proxy was set for:
- push
- url previews
- phone-home stats
- recaptcha validation
- CAS auth validation
- OpenID Connect

In this version we have added support for outbound requests for:
- federation
- download remote media
- fetch keys from other servers
- Identity servers

These requests use the same proxy configuration. If you have a proxy configuration we
recommend to verify the configuration. It may be necessary to adjust the `no_proxy`
environment variable.


# Upgrading to v1.39.0

## Deprecation of the current third-party rules module interface
Expand Down
91 changes: 84 additions & 7 deletions synapse/http/federation/matrix_federation_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@
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,
)

from netaddr import AddrFormatError, IPAddress, IPSet
from zope.interface import implementer
Expand All @@ -25,14 +29,17 @@
IReactorCore,
IStreamClientEndpoint,
)
from twisted.web.client import URI, Agent, HTTPConnectionPool
from twisted.web.client import URI, Agent, BrowserLikePolicyForHTTPS, HTTPConnectionPool
from twisted.web.http_headers import Headers
from twisted.web.iweb import IAgent, IAgentEndpointFactory, IBodyProducer, IResponse

from synapse.crypto.context_factory import FederationPolicyForHTTPS
from synapse.http import proxyagent
from synapse.http.client import BlacklistingAgentWrapper
from synapse.http.connectproxyclient import HTTPConnectProxyEndpoint
from synapse.http.federation.srv_resolver import Server, SrvResolver
from synapse.http.federation.well_known_resolver import WellKnownResolver
from synapse.http.proxyagent import ProxyAgent
from synapse.logging.context import make_deferred_yieldable, run_in_background
from synapse.types import ISynapseReactor
from synapse.util import Clock
Expand All @@ -57,6 +64,12 @@ class MatrixFederationAgent:
user_agent:
The user agent header to use for federation requests.

ip_blacklist: Disallowed IP addresses.

proxy_reactor: twisted reactor to use for connections to the proxy server
reactor might have some blacklisting applied (i.e. for DNS queries),
but we need unblocked access to the proxy.

_srv_resolver:
SrvResolver implementation to use for looking up SRV records. None
to use a default implementation.
Expand All @@ -72,6 +85,7 @@ def __init__(
tls_client_options_factory: Optional[FederationPolicyForHTTPS],
user_agent: bytes,
ip_blacklist: IPSet,
proxy_reactor: Optional[ISynapseReactor] = None,
_srv_resolver: Optional[SrvResolver] = None,
_well_known_resolver: Optional[WellKnownResolver] = None,
):
Expand All @@ -82,10 +96,18 @@ def __init__(
self._pool.maxPersistentPerHost = 5
self._pool.cachedConnectionTimeout = 2 * 60

if proxy_reactor is None:
self.proxy_reactor = reactor
else:
self.proxy_reactor = proxy_reactor

self._agent = Agent.usingEndpointFactory(
self._reactor,
MatrixHostnameEndpointFactory(
reactor, tls_client_options_factory, _srv_resolver
reactor,
self.proxy_reactor,
tls_client_options_factory,
_srv_resolver,
),
pool=self._pool,
)
Expand All @@ -97,10 +119,12 @@ def __init__(
_well_known_resolver = WellKnownResolver(
self._reactor,
agent=BlacklistingAgentWrapper(
Agent(
ProxyAgent(
self._reactor,
self.proxy_reactor,
pool=self._pool,
contextFactory=tls_client_options_factory,
use_proxy=True,
),
ip_blacklist=ip_blacklist,
),
Expand Down Expand Up @@ -200,20 +224,23 @@ class MatrixHostnameEndpointFactory:
def __init__(
self,
reactor: IReactorCore,
proxy_reactor: IReactorCore,
tls_client_options_factory: Optional[FederationPolicyForHTTPS],
srv_resolver: Optional[SrvResolver],
):
self._reactor = reactor
self._proxy_reactor = proxy_reactor
self._tls_client_options_factory = tls_client_options_factory

if srv_resolver is None:
srv_resolver = SrvResolver()

self._srv_resolver = srv_resolver

def endpointForURI(self, parsed_uri):
def endpointForURI(self, parsed_uri: URI):
return MatrixHostnameEndpoint(
self._reactor,
self._proxy_reactor,
self._tls_client_options_factory,
self._srv_resolver,
parsed_uri,
Expand All @@ -227,6 +254,9 @@ class MatrixHostnameEndpoint:

Args:
reactor: twisted reactor to use for underlying requests
proxy_reactor: twisted reactor to use for connections to the proxy server
reactor might have some blacklisting applied (i.e. for DNS queries),
but we need unblocked access to the proxy.
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
tls_client_options_factory:
factory to use for fetching client tls options, or none to disable TLS.
srv_resolver: The SRV resolver to use
Expand All @@ -236,14 +266,28 @@ class MatrixHostnameEndpoint:
def __init__(
self,
reactor: IReactorCore,
proxy_reactor: IReactorCore,
tls_client_options_factory: Optional[FederationPolicyForHTTPS],
srv_resolver: SrvResolver,
parsed_uri: URI,
):
self._reactor = reactor

self._parsed_uri = parsed_uri

# 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

(
self.https_proxy_endpoint,
self.https_proxy_creds,
) = proxyagent.http_proxy_endpoint(
dklimpel marked this conversation as resolved.
Show resolved Hide resolved
https_proxy,
proxy_reactor,
tls_client_options_factory or BrowserLikePolicyForHTTPS(),
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure if BrowserLikePolicyForHTTPS is the best default policy.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we need a default policy here. tls_client_options_factory=None is supposed to disable TLS, not fall back to a default. I would make the tls_options_factory parameter to _http_proxy_endpoint Optional, and raise an Exception if the scheme is https but there is no tls factory.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Im not sure what is the best error to raise. ValueError, ConfigError, RuntimeError or anything else?

)

# set up the TLS connection params
#
# XXX disabling TLS is really only supported here for the benefit of the
Expand Down Expand Up @@ -273,9 +317,42 @@ async def _do_connect(self, protocol_factory: IProtocolFactory) -> None:
host = server.host
port = server.port

should_skip_proxy = False
if self.no_proxy is not None:
should_skip_proxy = proxy_bypass_environment(
host.decode(),
proxies={"no": self.no_proxy},
)

endpoint: IStreamClientEndpoint
try:
logger.debug("Connecting to %s:%i", host.decode("ascii"), port)
endpoint = HostnameEndpoint(self._reactor, host, port)
if self.https_proxy_endpoint and not should_skip_proxy:
logger.debug(
"Connecting to %s:%i via %s",
host.decode("ascii"),
port,
self.https_proxy_endpoint,
)
connect_headers = Headers()
# Determine whether we need to set Proxy-Authorization headers
if self.https_proxy_creds:
# Set a Proxy-Authorization header
connect_headers.addRawHeader(
b"Proxy-Authorization",
self.https_proxy_creds.as_proxy_authorization_value(),
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we just move this into HTTPConnectProxyEndpoint, to save doing it each time we construct one?

(in other words: make HTTPConnectProxyEndpoint take an Optional[ProxyCredentials] parameter instead of a custom headers parameter)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change was larger.
I had to move ProxyCredentials from proxyagent to connectproxyclient. The reason was a circular import.
I have replaced headers parameter because it was introduced in #9657 only for proxy connections and is not needed anymore.


endpoint = HTTPConnectProxyEndpoint(
self._reactor,
self.https_proxy_endpoint,
host,
port,
headers=connect_headers,
)
else:
logger.debug("Connecting to %s:%i", host.decode("ascii"), port)
# not using a proxy
endpoint = HostnameEndpoint(self._reactor, host, port)
if self._tls_options:
endpoint = wrapClientTLS(self._tls_options, endpoint)
result = await make_deferred_yieldable(
Expand Down
1 change: 1 addition & 0 deletions synapse/http/matrixfederationclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -343,6 +343,7 @@ def __init__(self, hs, tls_client_options_factory):
tls_client_options_factory,
user_agent,
hs.config.federation_ip_range_blacklist,
proxy_reactor=hs.get_reactor(),
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

per #9306 (comment):

I suggest you move the code at lines 330-334 which builds a BlacklistingReactorWrapper into MatrixFederationAgent. There is no need for MatrixFederationHttpClient.reactor to be a BlacklistingReactorWrapper.

)

# Use a BlacklistingAgentWrapper to prevent circumventing the IP
Expand Down
6 changes: 3 additions & 3 deletions synapse/http/proxyagent.py
Original file line number Diff line number Diff line change
Expand Up @@ -131,11 +131,11 @@ def __init__(
https_proxy = proxies["https"].encode() if "https" in proxies else None
no_proxy = proxies["no"] if "no" in proxies else None

self.http_proxy_endpoint, self.http_proxy_creds = _http_proxy_endpoint(
self.http_proxy_endpoint, self.http_proxy_creds = http_proxy_endpoint(
http_proxy, self.proxy_reactor, contextFactory, **self._endpoint_kwargs
)

self.https_proxy_endpoint, self.https_proxy_creds = _http_proxy_endpoint(
self.https_proxy_endpoint, self.https_proxy_creds = http_proxy_endpoint(
https_proxy, self.proxy_reactor, contextFactory, **self._endpoint_kwargs
)

Expand Down Expand Up @@ -268,7 +268,7 @@ def request(
)


def _http_proxy_endpoint(
def http_proxy_endpoint(
proxy: Optional[bytes],
reactor: IReactorCore,
tls_options_factory: IPolicyForHTTPS,
Expand Down
Loading