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

Apply the federation_ip_range_blacklist to push and key revocation requests #8821

Merged
merged 16 commits into from
Dec 2, 2020
Merged
Show file tree
Hide file tree
Changes from 13 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.
14 changes: 8 additions & 6 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -642,17 +642,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 outbound requests for federation, identity servers, push servers, and for
# checking key validitity for third-party invite events
Copy link
Member

Choose a reason for hiding this comment

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

this sentence no verb

#
# (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.
Copy link
Member Author

Choose a reason for hiding this comment

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

I debated how much to mention federation_ip_range_blacklist at all. I suspect we'll want to add something to the upgrade notes. The current implementation actually lets you provide two different blacklists (one for push/key validity and one for federation/identity servers). I'm unsure if we should make that a feature (and document it) or if that's an implementation detail and federation_ip_range_blacklist is deprecated. 😄

Copy link
Member

Choose a reason for hiding this comment

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

I vote for the latter :)

#
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 ot 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 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 @@ -845,7 +845,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 @@ -35,7 +35,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 @@ -140,7 +140,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
6 changes: 3 additions & 3 deletions synapse/handlers/identity.py
Original file line number Diff line number Diff line change
Expand Up @@ -46,13 +46,13 @@ class IdentityHandler(BaseHandler):
def __init__(self, hs):
super().__init__(hs)

# An HTTP client for contacting trusted URLs.
self.http_client = SimpleHttpClient(hs)
# 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

async def threepid_from_creds(
Expand Down
46 changes: 32 additions & 14 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 @@ -292,22 +321,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 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
2 changes: 1 addition & 1 deletion synapse/push/httppusher.py
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ def __init__(self, hs, pusherdict):
if "url" not in self.data:
raise PusherConfigException("'url' required in data for HTTP pusher")
self.url = self.data["url"]
self.http_client = hs.get_proxied_http_client()
self.http_client = hs.get_proxied_blacklisted_http_client()
self.data_minus_url = {}
self.data_minus_url.update(self.data)
del self.data_minus_url["url"]
Expand Down
2 changes: 1 addition & 1 deletion synapse/rest/media/v1/media_repository.py
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ class MediaRepository:
def __init__(self, hs):
self.hs = hs
self.auth = hs.get_auth()
self.client = hs.get_http_client()
self.client = hs.get_federation_http_client()
self.clock = hs.get_clock()
self.server_name = hs.hostname
self.store = hs.get_datastore()
Expand Down
Loading