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

Port "Add support for no_proxy and case insensitive env variables" from mainline to dinsic #93

Merged
merged 4 commits into from
Mar 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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/8821.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Apply the `federation_ip_range_blacklist` to push and key revocation requests.
1 change: 1 addition & 0 deletions changelog.d/9084.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Don't blacklist connections to the configured proxy. Contributed by @Bubu.
1 change: 1 addition & 0 deletions changelog.d/9372.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
The `no_proxy` and `NO_PROXY` environment variables are now respected in proxied HTTP clients with the lowercase form taking precedence if both are present. Additionally, the lowercase `https_proxy` environment variable is now respected in proxied HTTP clients on top of existing support for the uppercase `HTTPS_PROXY` form and takes precedence if both are present. Contributed by Timothy Leung.
14 changes: 8 additions & 6 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -710,17 +710,19 @@ acme:
# - nyc.example.com
# - syd.example.com

# Prevent federation requests from being sent to the following
# blacklist IP address CIDR ranges. If this option is not specified, or
# specified with an empty list, no ip range blacklist will be enforced.
# Prevent outgoing requests from being sent to the following blacklisted IP address
# CIDR ranges. If this option is not specified, or specified with an empty list,
# no IP range blacklist will be enforced.
#
# As of Synapse v1.4.0 this option also affects any outbound requests to identity
# servers provided by user input.
# The blacklist applies to the outbound requests for federation, identity servers,
# push servers, and for checking key validitity for third-party invite events.
#
# (0.0.0.0 and :: are always blacklisted, whether or not they are explicitly
# listed here, since they correspond to unroutable addresses.)
#
federation_ip_range_blacklist:
# This option replaces federation_ip_range_blacklist in Synapse v1.24.0.
#
ip_range_blacklist:
- '127.0.0.0/8'
- '10.0.0.0/8'
- '172.16.0.0/12'
Expand Down
1 change: 0 additions & 1 deletion synapse/app/generic_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -266,7 +266,6 @@ def __init__(self, hs):
super().__init__(hs)
self.hs = hs
self.is_mine_id = hs.is_mine_id
self.http_client = hs.get_simple_http_client()

self._presence_enabled = hs.config.use_presence

Expand Down
40 changes: 25 additions & 15 deletions synapse/config/federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,30 @@ def read_config(self, config, **kwargs):
for domain in federation_domain_whitelist:
self.federation_domain_whitelist[domain] = True

self.federation_ip_range_blacklist = config.get(
"federation_ip_range_blacklist", []
)
ip_range_blacklist = config.get("ip_range_blacklist", [])

# Attempt to create an IPSet from the given ranges
try:
self.federation_ip_range_blacklist = IPSet(
self.federation_ip_range_blacklist
)

# Always blacklist 0.0.0.0, ::
self.federation_ip_range_blacklist.update(["0.0.0.0", "::"])
self.ip_range_blacklist = IPSet(ip_range_blacklist)
except Exception as e:
raise ConfigError("Invalid range(s) provided in ip_range_blacklist: %s" % e)
# Always blacklist 0.0.0.0, ::
self.ip_range_blacklist.update(["0.0.0.0", "::"])

# The federation_ip_range_blacklist is used for backwards-compatibility
# and only applies to federation and identity servers. If it is not given,
# default to ip_range_blacklist.
federation_ip_range_blacklist = config.get(
"federation_ip_range_blacklist", ip_range_blacklist
)
try:
self.federation_ip_range_blacklist = IPSet(federation_ip_range_blacklist)
except Exception as e:
raise ConfigError(
"Invalid range(s) provided in federation_ip_range_blacklist: %s" % e
)
# Always blacklist 0.0.0.0, ::
self.federation_ip_range_blacklist.update(["0.0.0.0", "::"])

federation_metrics_domains = config.get("federation_metrics_domains") or []
validate_config(
Expand All @@ -76,17 +84,19 @@ def generate_config_section(self, config_dir_path, server_name, **kwargs):
# - nyc.example.com
# - syd.example.com

# Prevent federation requests from being sent to the following
# blacklist IP address CIDR ranges. If this option is not specified, or
# specified with an empty list, no ip range blacklist will be enforced.
# Prevent outgoing requests from being sent to the following blacklisted IP address
# CIDR ranges. If this option is not specified, or specified with an empty list,
# no IP range blacklist will be enforced.
#
# As of Synapse v1.4.0 this option also affects any outbound requests to identity
# servers provided by user input.
# The blacklist applies to the outbound requests for federation, identity servers,
# push servers, and for checking key validitity for third-party invite events.
#
# (0.0.0.0 and :: are always blacklisted, whether or not they are explicitly
# listed here, since they correspond to unroutable addresses.)
#
federation_ip_range_blacklist:
# This option replaces federation_ip_range_blacklist in Synapse v1.24.0.
#
ip_range_blacklist:
- '127.0.0.0/8'
- '10.0.0.0/8'
- '172.16.0.0/12'
Expand Down
4 changes: 2 additions & 2 deletions synapse/crypto/keyring.py
Original file line number Diff line number Diff line change
Expand Up @@ -578,7 +578,7 @@ class PerspectivesKeyFetcher(BaseV2KeyFetcher):
def __init__(self, hs):
super().__init__(hs)
self.clock = hs.get_clock()
self.client = hs.get_http_client()
self.client = hs.get_federation_http_client()
self.key_servers = self.config.key_servers

async def get_keys(self, keys_to_fetch):
Expand Down Expand Up @@ -748,7 +748,7 @@ class ServerKeyFetcher(BaseV2KeyFetcher):
def __init__(self, hs):
super().__init__(hs)
self.clock = hs.get_clock()
self.client = hs.get_http_client()
self.client = hs.get_federation_http_client()

async def get_keys(self, keys_to_fetch):
"""
Expand Down
1 change: 0 additions & 1 deletion synapse/federation/federation_server.py
Original file line number Diff line number Diff line change
Expand Up @@ -916,7 +916,6 @@ class FederationHandlerRegistry:

def __init__(self, hs: "HomeServer"):
self.config = hs.config
self.http_client = hs.get_simple_http_client()
self.clock = hs.get_clock()
self._instance_name = hs.get_instance_name()

Expand Down
2 changes: 1 addition & 1 deletion synapse/federation/transport/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ class TransportLayerClient:

def __init__(self, hs):
self.server_name = hs.hostname
self.client = hs.get_http_client()
self.client = hs.get_federation_http_client()

@log_function
def get_room_state_ids(self, destination, room_id, event_id):
Expand Down
2 changes: 1 addition & 1 deletion synapse/handlers/federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ def __init__(self, hs: "HomeServer"):
self._message_handler = hs.get_message_handler()
self._server_notices_mxid = hs.config.server_notices_mxid
self.config = hs.config
self.http_client = hs.get_simple_http_client()
self.http_client = hs.get_proxied_blacklisted_http_client()
self._instance_name = hs.get_instance_name()
self._replication = hs.get_replication_data_handler()

Expand Down
8 changes: 4 additions & 4 deletions synapse/handlers/identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,14 +46,14 @@ class IdentityHandler(BaseHandler):
def __init__(self, hs):
super().__init__(hs)

self.hs = hs
# An HTTP client for contacting trusted URLs.
self.http_client = hs.get_simple_http_client()
# We create a blacklisting instance of SimpleHttpClient for contacting identity
# servers specified by clients
# An HTTP client for contacting identity servers specified by clients.
self.blacklisting_http_client = SimpleHttpClient(
hs, ip_blacklist=hs.config.federation_ip_range_blacklist
)
self.federation_http_client = hs.get_http_client()
self.federation_http_client = hs.get_federation_http_client()
self.hs = hs

self.trusted_id_servers = set(hs.config.trusted_third_party_id_servers)
self.trust_any_id_server_just_for_testing_do_not_use = (
Expand Down
57 changes: 37 additions & 20 deletions synapse/http/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ def _scheduler(x):
return _scheduler


class IPBlacklistingResolver:
class _IPBlacklistingResolver:
"""
A proxy for reactor.nameResolver which only produces non-blacklisted IP
addresses, preventing DNS rebinding attacks on URL preview.
Expand Down Expand Up @@ -199,6 +199,35 @@ def resolutionComplete() -> None:
return r


@implementer(IReactorPluggableNameResolver)
class BlacklistingReactorWrapper:
"""
A Reactor wrapper which will prevent DNS resolution to blacklisted IP
addresses, to prevent DNS rebinding.
"""

def __init__(
self,
reactor: IReactorPluggableNameResolver,
ip_whitelist: Optional[IPSet],
ip_blacklist: IPSet,
):
self._reactor = reactor

# We need to use a DNS resolver which filters out blacklisted IP
# addresses, to prevent DNS rebinding.
self._nameResolver = _IPBlacklistingResolver(
self._reactor, ip_whitelist, ip_blacklist
)

def __getattr__(self, attr: str) -> Any:
# Passthrough to the real reactor except for the DNS resolver.
if attr == "nameResolver":
return self._nameResolver
else:
return getattr(self._reactor, attr)


class BlacklistingAgentWrapper(Agent):
"""
An Agent wrapper which will prevent access to IP addresses being accessed
Expand Down Expand Up @@ -260,8 +289,7 @@ def __init__(
treq_args: Dict[str, Any] = {},
ip_whitelist: Optional[IPSet] = None,
ip_blacklist: Optional[IPSet] = None,
http_proxy: Optional[bytes] = None,
https_proxy: Optional[bytes] = None,
use_proxy: bool = False,
):
"""
Args:
Expand All @@ -271,8 +299,8 @@ def __init__(
we may not request.
ip_whitelist: The whitelisted IP addresses, that we can
request if it were otherwise caught in a blacklist.
http_proxy: proxy server to use for http connections. host[:port]
https_proxy: proxy server to use for https connections. host[:port]
use_proxy: Whether proxy settings should be discovered and used
from conventional environment variables.
"""
self.hs = hs

Expand All @@ -292,22 +320,11 @@ def __init__(
self.user_agent = self.user_agent.encode("ascii")

if self._ip_blacklist:
real_reactor = hs.get_reactor()
# If we have an IP blacklist, we need to use a DNS resolver which
# filters out blacklisted IP addresses, to prevent DNS rebinding.
nameResolver = IPBlacklistingResolver(
real_reactor, self._ip_whitelist, self._ip_blacklist
self.reactor = BlacklistingReactorWrapper(
hs.get_reactor(), self._ip_whitelist, self._ip_blacklist
)

@implementer(IReactorPluggableNameResolver)
class Reactor:
def __getattr__(_self, attr):
if attr == "nameResolver":
return nameResolver
else:
return getattr(real_reactor, attr)

self.reactor = Reactor()
else:
self.reactor = hs.get_reactor()

Expand All @@ -323,11 +340,11 @@ def __getattr__(_self, attr):

self.agent = ProxyAgent(
self.reactor,
hs.get_reactor(),
connectTimeout=15,
contextFactory=self.hs.get_http_client_context_factory(),
pool=pool,
http_proxy=http_proxy,
https_proxy=https_proxy,
use_proxy=use_proxy,
)

if self._ip_blacklist:
Expand Down
16 changes: 12 additions & 4 deletions synapse/http/federation/matrix_federation_agent.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import urllib.parse
from typing import List, Optional

from netaddr import AddrFormatError, IPAddress
from netaddr import AddrFormatError, IPAddress, IPSet
from zope.interface import implementer

from twisted.internet import defer
Expand All @@ -31,6 +31,7 @@
from twisted.web.iweb import IAgent, IAgentEndpointFactory, IBodyProducer

from synapse.crypto.context_factory import FederationPolicyForHTTPS
from synapse.http.client import BlacklistingAgentWrapper
from synapse.http.federation.srv_resolver import Server, SrvResolver
from synapse.http.federation.well_known_resolver import WellKnownResolver
from synapse.logging.context import make_deferred_yieldable, run_in_background
Expand Down Expand Up @@ -70,6 +71,7 @@ def __init__(
reactor: IReactorCore,
tls_client_options_factory: Optional[FederationPolicyForHTTPS],
user_agent: bytes,
ip_blacklist: IPSet,
_srv_resolver: Optional[SrvResolver] = None,
_well_known_resolver: Optional[WellKnownResolver] = None,
):
Expand All @@ -90,12 +92,18 @@ def __init__(
self.user_agent = user_agent

if _well_known_resolver is None:
# Note that the name resolver has already been wrapped in a
# IPBlacklistingResolver by MatrixFederationHttpClient.
_well_known_resolver = WellKnownResolver(
self._reactor,
agent=Agent(
agent=BlacklistingAgentWrapper(
Agent(
self._reactor,
pool=self._pool,
contextFactory=tls_client_options_factory,
),
self._reactor,
pool=self._pool,
contextFactory=tls_client_options_factory,
ip_blacklist=ip_blacklist,
),
user_agent=self.user_agent,
)
Expand Down
26 changes: 8 additions & 18 deletions synapse/http/matrixfederationclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -26,11 +26,10 @@
from canonicaljson import encode_canonical_json
from prometheus_client import Counter
from signedjson.sign import sign_json
from zope.interface import implementer

from twisted.internet import defer
from twisted.internet.error import DNSLookupError
from twisted.internet.interfaces import IReactorPluggableNameResolver, IReactorTime
from twisted.internet.interfaces import IReactorTime
from twisted.internet.task import _EPSILON, Cooperator
from twisted.web.http_headers import Headers
from twisted.web.iweb import IBodyProducer, IResponse
Expand All @@ -45,7 +44,7 @@
from synapse.http import QuieterFileBodyProducer
from synapse.http.client import (
BlacklistingAgentWrapper,
IPBlacklistingResolver,
BlacklistingReactorWrapper,
encode_query_args,
readBodyToFile,
)
Expand Down Expand Up @@ -221,31 +220,22 @@ def __init__(self, hs, tls_client_options_factory):
self.signing_key = hs.signing_key
self.server_name = hs.hostname

real_reactor = hs.get_reactor()

# We need to use a DNS resolver which filters out blacklisted IP
# addresses, to prevent DNS rebinding.
nameResolver = IPBlacklistingResolver(
real_reactor, None, hs.config.federation_ip_range_blacklist
self.reactor = BlacklistingReactorWrapper(
hs.get_reactor(), None, hs.config.federation_ip_range_blacklist
)

@implementer(IReactorPluggableNameResolver)
class Reactor:
def __getattr__(_self, attr):
if attr == "nameResolver":
return nameResolver
else:
return getattr(real_reactor, attr)

self.reactor = Reactor()

user_agent = hs.version_string
if hs.config.user_agent_suffix:
user_agent = "%s %s" % (user_agent, hs.config.user_agent_suffix)
user_agent = user_agent.encode("ascii")

self.agent = MatrixFederationAgent(
self.reactor, tls_client_options_factory, user_agent
self.reactor,
tls_client_options_factory,
user_agent,
hs.config.federation_ip_range_blacklist,
)

# Use a BlacklistingAgentWrapper to prevent circumventing the IP
Expand Down
Loading