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

Ability to blacklist ip ranges for federation traffic #5043

Merged
merged 30 commits into from
May 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
1b8532b
tests fail
anoadragon453 Apr 10, 2019
4501489
tests pass
anoadragon453 Apr 10, 2019
0200c86
lint
anoadragon453 Apr 10, 2019
9f1f03f
lint and changelog
anoadragon453 Apr 10, 2019
6631485
actually add changelog
anoadragon453 Apr 10, 2019
25c99dc
sample config
anoadragon453 Apr 10, 2019
9795344
Don't raise an exception if coming from federation
anoadragon453 Apr 10, 2019
1b3989b
lint
anoadragon453 Apr 10, 2019
0e2f8ca
Add some notes
anoadragon453 Apr 10, 2019
6479cd5
Use an empty list as default
anoadragon453 Apr 30, 2019
3f4f931
Merge branch 'develop' into anoa/blacklist_ip_ranges
anoadragon453 Apr 30, 2019
968ddca
Testing
anoadragon453 May 2, 2019
e1feb45
We can't throw exceptions in an IResolutionReceiver
richvdh May 2, 2019
152d7a8
Remove different behaviour for fed vs. nonfed
anoadragon453 May 2, 2019
6592691
Import at the top
anoadragon453 May 2, 2019
517794e
isort locally didn't have a problem >:(
anoadragon453 May 3, 2019
15d1802
lint
anoadragon453 May 3, 2019
131b9c0
yield deferred
anoadragon453 May 3, 2019
13f430c
Same behavior for no result and result blacklisted
anoadragon453 May 3, 2019
e2bc9af
lint
anoadragon453 May 3, 2019
ec67848
Remove yield
anoadragon453 May 3, 2019
43ffe47
Enable federation blacklisting by default
anoadragon453 May 8, 2019
aee810a
Fix tests and various small review issues
anoadragon453 May 8, 2019
a30a778
Update tests
anoadragon453 May 8, 2019
ede582f
lint
anoadragon453 May 8, 2019
4ba420f
always blacklist 0.0.0.0, ::
anoadragon453 May 10, 2019
358777d
lower pump value
anoadragon453 May 10, 2019
7f15dd7
lint
anoadragon453 May 10, 2019
6b29f7e
regen config
anoadragon453 May 10, 2019
e0715d0
Merge branch 'develop' into anoa/blacklist_ip_ranges
anoadragon453 May 10, 2019
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/5043.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add ability to blacklist IP ranges for the federation client.
18 changes: 18 additions & 0 deletions docs/sample_config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,24 @@ pid_file: DATADIR/homeserver.pid
# - 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.
#
# (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:
- '127.0.0.0/8'
- '10.0.0.0/8'
- '172.16.0.0/12'
- '192.168.0.0/16'
- '100.64.0.0/10'
- '169.254.0.0/16'
- '::1/128'
- 'fe80::/64'
- 'fc00::/7'

# List of ports that Synapse should listen on, their purpose and their
# configuration.
#
Expand Down
38 changes: 38 additions & 0 deletions synapse/config/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,8 @@
import logging
import os.path

from netaddr import IPSet

from synapse.http.endpoint import parse_and_validate_server_name
from synapse.python_dependencies import DependencyException, check_requirements

Expand Down Expand Up @@ -137,6 +139,24 @@ def read_config(self, config):
for domain in federation_domain_whitelist:
self.federation_domain_whitelist[domain] = True

self.federation_ip_range_blacklist = config.get(
Copy link
Member

Choose a reason for hiding this comment

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

we ought to follow the example of #5134 and include 0.0.0.0 and ::, whether they were explicitly listed or not.

"federation_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", "::"])
Copy link
Member

Choose a reason for hiding this comment

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

this could really have done with being outside the try/catch, but nm

except Exception as e:
raise ConfigError(
"Invalid range(s) provided in "
"federation_ip_range_blacklist: %s" % e
)

if self.public_baseurl is not None:
if self.public_baseurl[-1] != '/':
self.public_baseurl += '/'
Expand Down Expand Up @@ -386,6 +406,24 @@ def default_config(self, server_name, data_dir_path, **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.
#
# (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:
- '127.0.0.0/8'
- '10.0.0.0/8'
- '172.16.0.0/12'
- '192.168.0.0/16'
- '100.64.0.0/10'
- '169.254.0.0/16'
- '::1/128'
- 'fe80::/64'
- 'fc00::/7'

# List of ports that Synapse should listen on, their purpose and their
# configuration.
#
Expand Down
6 changes: 2 additions & 4 deletions synapse/http/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -165,7 +165,8 @@ def request(self, method, uri, headers=None, bodyProducer=None):
ip_address, self._ip_whitelist, self._ip_blacklist
):
logger.info(
"Blocking access to %s because of blacklist" % (ip_address,)
"Blocking access to %s due to blacklist" %
(ip_address,)
)
e = SynapseError(403, "IP address blocked by IP blacklist entry")
return defer.fail(Failure(e))
Expand Down Expand Up @@ -263,9 +264,6 @@ def request(self, method, uri, data=None, headers=None):
uri (str): URI to query.
data (bytes): Data to send in the request body, if applicable.
headers (t.w.http_headers.Headers): Request headers.

Raises:
SynapseError: If the IP is blacklisted.
"""
# A small wrapper around self.agent.request() so we can easily attach
# counters to it
Expand Down
48 changes: 38 additions & 10 deletions synapse/http/matrixfederationclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,11 @@
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, protocol
from twisted.internet.error import DNSLookupError
from twisted.internet.interfaces import IReactorPluggableNameResolver
from twisted.internet.task import _EPSILON, Cooperator
from twisted.web._newclient import ResponseDone
from twisted.web.http_headers import Headers
Expand All @@ -44,6 +46,7 @@
SynapseError,
)
from synapse.http import QuieterFileBodyProducer
from synapse.http.client import BlacklistingAgentWrapper, IPBlacklistingResolver
from synapse.http.federation.matrix_federation_agent import MatrixFederationAgent
from synapse.util.async_helpers import timeout_deferred
from synapse.util.logcontext import make_deferred_yieldable
Expand Down Expand Up @@ -172,19 +175,44 @@ def __init__(self, hs, tls_client_options_factory):
self.hs = hs
self.signing_key = hs.config.signing_key[0]
self.server_name = hs.hostname
reactor = hs.get_reactor()

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,
)

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

self.reactor = Reactor()

self.agent = MatrixFederationAgent(
hs.get_reactor(),
self.reactor,
tls_client_options_factory,
)

# Use a BlacklistingAgentWrapper to prevent circumventing the IP
# blacklist via IP literals in server names
self.agent = BlacklistingAgentWrapper(
self.agent, self.reactor,
ip_blacklist=hs.config.federation_ip_range_blacklist,
)

self.clock = hs.get_clock()
self._store = hs.get_datastore()
self.version_string_bytes = hs.version_string.encode('ascii')
self.default_timeout = 60

def schedule(x):
reactor.callLater(_EPSILON, x)
self.reactor.callLater(_EPSILON, x)

self._cooperator = Cooperator(scheduler=schedule)

Expand Down Expand Up @@ -370,7 +398,7 @@ def _send_request(
request_deferred = timeout_deferred(
request_deferred,
timeout=_sec_timeout,
reactor=self.hs.get_reactor(),
reactor=self.reactor,
)

response = yield request_deferred
Expand All @@ -397,7 +425,7 @@ def _send_request(
d = timeout_deferred(
d,
timeout=_sec_timeout,
reactor=self.hs.get_reactor(),
reactor=self.reactor,
)

try:
Expand Down Expand Up @@ -586,7 +614,7 @@ def put_json(self, destination, path, args={}, data={},
)

body = yield _handle_json_response(
self.hs.get_reactor(), self.default_timeout, request, response,
self.reactor, self.default_timeout, request, response,
)

defer.returnValue(body)
Expand Down Expand Up @@ -645,7 +673,7 @@ def post_json(self, destination, path, data={}, long_retries=False,
_sec_timeout = self.default_timeout

body = yield _handle_json_response(
self.hs.get_reactor(), _sec_timeout, request, response,
self.reactor, _sec_timeout, request, response,
)
defer.returnValue(body)

Expand Down Expand Up @@ -704,7 +732,7 @@ def get_json(self, destination, path, args=None, retry_on_dns_fail=True,
)

body = yield _handle_json_response(
self.hs.get_reactor(), self.default_timeout, request, response,
self.reactor, self.default_timeout, request, response,
)

defer.returnValue(body)
Expand Down Expand Up @@ -753,7 +781,7 @@ def delete_json(self, destination, path, long_retries=False,
)

body = yield _handle_json_response(
self.hs.get_reactor(), self.default_timeout, request, response,
self.reactor, self.default_timeout, request, response,
)
defer.returnValue(body)

Expand Down Expand Up @@ -801,7 +829,7 @@ def get_file(self, destination, path, output_stream, args={},

try:
d = _readBodyToFile(response, output_stream, max_size)
d.addTimeout(self.default_timeout, self.hs.get_reactor())
d.addTimeout(self.default_timeout, self.reactor)
length = yield make_deferred_yieldable(d)
except Exception as e:
logger.warn(
Expand Down
71 changes: 71 additions & 0 deletions tests/http/test_fedclient.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

from mock import Mock

from netaddr import IPSet

from twisted.internet import defer
from twisted.internet.defer import TimeoutError
from twisted.internet.error import ConnectingCancelledError, DNSLookupError
Expand Down Expand Up @@ -209,6 +211,75 @@ def test_client_connect_no_response(self):
self.assertIsInstance(f.value, RequestSendFailed)
self.assertIsInstance(f.value.inner_exception, ResponseNeverReceived)

def test_client_ip_range_blacklist(self):
"""Ensure that Synapse does not try to connect to blacklisted IPs"""

# Set up the ip_range blacklist
self.hs.config.federation_ip_range_blacklist = IPSet([
"127.0.0.0/8",
"fe80::/64",
])
self.reactor.lookups["internal"] = "127.0.0.1"
self.reactor.lookups["internalv6"] = "fe80:0:0:0:0:8a2e:370:7337"
self.reactor.lookups["fine"] = "10.20.30.40"
cl = MatrixFederationHttpClient(self.hs, None)

# Try making a GET request to a blacklisted IPv4 address
# ------------------------------------------------------
# Make the request
d = cl.get_json("internal:8008", "foo/bar", timeout=10000)

# Nothing happened yet
self.assertNoResult(d)

self.pump(1)

# Check that it was unable to resolve the address
clients = self.reactor.tcpClients
self.assertEqual(len(clients), 0)

f = self.failureResultOf(d)
self.assertIsInstance(f.value, RequestSendFailed)
self.assertIsInstance(f.value.inner_exception, DNSLookupError)

# Try making a POST request to a blacklisted IPv6 address
# -------------------------------------------------------
# Make the request
d = cl.post_json("internalv6:8008", "foo/bar", timeout=10000)

# Nothing has happened yet
self.assertNoResult(d)

# Move the reactor forwards
self.pump(1)

# Check that it was unable to resolve the address
clients = self.reactor.tcpClients
self.assertEqual(len(clients), 0)

# Check that it was due to a blacklisted DNS lookup
f = self.failureResultOf(d, RequestSendFailed)
self.assertIsInstance(f.value.inner_exception, DNSLookupError)

# Try making a GET request to a non-blacklisted IPv4 address
# ----------------------------------------------------------
# Make the request
d = cl.post_json("fine:8008", "foo/bar", timeout=10000)

# Nothing has happened yet
self.assertNoResult(d)

# Move the reactor forwards
self.pump(1)

# Check that it was able to resolve the address
clients = self.reactor.tcpClients
self.assertNotEqual(len(clients), 0)

# Connection will still fail as this IP address does not resolve to anything
f = self.failureResultOf(d, RequestSendFailed)
self.assertIsInstance(f.value.inner_exception, ConnectingCancelledError)

def test_client_gets_headers(self):
"""
Once the client gets the headers, _request returns successfully.
Expand Down