From c7b24ac3d0e202a406bc1f972b6c3da52c7e25c4 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Wed, 30 Jan 2019 11:43:33 +0000 Subject: [PATCH] Follow redirects on .well-known (#4520) --- changelog.d/4520.feature | 1 + .../federation/matrix_federation_agent.py | 6 +- .../test_matrix_federation_agent.py | 97 +++++++++++++++++++ 3 files changed, 102 insertions(+), 2 deletions(-) create mode 100644 changelog.d/4520.feature diff --git a/changelog.d/4520.feature b/changelog.d/4520.feature new file mode 100644 index 000000000..bda713adf --- /dev/null +++ b/changelog.d/4520.feature @@ -0,0 +1 @@ +Implement MSC1708 (.well-known routing for server-server federation) \ No newline at end of file diff --git a/synapse/http/federation/matrix_federation_agent.py b/synapse/http/federation/matrix_federation_agent.py index 29804efe1..eeb3665f7 100644 --- a/synapse/http/federation/matrix_federation_agent.py +++ b/synapse/http/federation/matrix_federation_agent.py @@ -23,7 +23,7 @@ from zope.interface import implementer from twisted.internet import defer from twisted.internet.endpoints import HostnameEndpoint, wrapClientTLS -from twisted.web.client import URI, Agent, HTTPConnectionPool, readBody +from twisted.web.client import URI, Agent, HTTPConnectionPool, RedirectAgent, readBody from twisted.web.http import stringToDatetime from twisted.web.http_headers import Headers from twisted.web.iweb import IAgent @@ -93,7 +93,9 @@ class MatrixFederationAgent(object): # the param is called 'contextFactory', but actually passing a # contextfactory is deprecated, and it expects an IPolicyForHTTPS. agent_args['contextFactory'] = _well_known_tls_policy - _well_known_agent = Agent(self._reactor, pool=self._pool, **agent_args) + _well_known_agent = RedirectAgent( + Agent(self._reactor, pool=self._pool, **agent_args), + ) self._well_known_agent = _well_known_agent self._well_known_cache = _well_known_cache diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index fe459ea6e..7b2800021 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -470,6 +470,103 @@ class MatrixFederationAgentTests(TestCase): self.well_known_cache.expire() self.assertNotIn(b"testserv", self.well_known_cache) + def test_get_well_known_redirect(self): + """Test the behaviour when the server name has no port and no SRV record, but + the .well-known has a 300 redirect + """ + self.mock_resolver.resolve_service.side_effect = lambda _: [] + self.reactor.lookups["testserv"] = "1.2.3.4" + self.reactor.lookups["target-server"] = "1::f" + + test_d = self._make_get_request(b"matrix://testserv/foo/bar") + + # Nothing happened yet + self.assertNoResult(test_d) + + self.mock_resolver.resolve_service.assert_called_once_with( + b"_matrix._tcp.testserv", + ) + self.mock_resolver.resolve_service.reset_mock() + + # there should be an attempt to connect on port 443 for the .well-known + clients = self.reactor.tcpClients + self.assertEqual(len(clients), 1) + (host, port, client_factory, _timeout, _bindAddress) = clients.pop() + self.assertEqual(host, '1.2.3.4') + self.assertEqual(port, 443) + + redirect_server = self._make_connection( + client_factory, + expected_sni=b"testserv", + ) + + # send a 302 redirect + self.assertEqual(len(redirect_server.requests), 1) + request = redirect_server.requests[0] + request.redirect(b'https://testserv/even_better_known') + request.finish() + + self.reactor.pump((0.1, )) + + # now there should be another connection + clients = self.reactor.tcpClients + self.assertEqual(len(clients), 1) + (host, port, client_factory, _timeout, _bindAddress) = clients.pop() + self.assertEqual(host, '1.2.3.4') + self.assertEqual(port, 443) + + well_known_server = self._make_connection( + client_factory, + expected_sni=b"testserv", + ) + + self.assertEqual(len(well_known_server.requests), 1, "No request after 302") + request = well_known_server.requests[0] + self.assertEqual(request.method, b'GET') + self.assertEqual(request.path, b'/even_better_known') + request.write(b'{ "m.server": "target-server" }') + request.finish() + + self.reactor.pump((0.1, )) + + # there should be another SRV lookup + self.mock_resolver.resolve_service.assert_called_once_with( + b"_matrix._tcp.target-server", + ) + + # now we should get a connection to the target server + self.assertEqual(len(clients), 1) + (host, port, client_factory, _timeout, _bindAddress) = clients[0] + self.assertEqual(host, '1::f') + self.assertEqual(port, 8448) + + # make a test server, and wire up the client + http_server = self._make_connection( + client_factory, + expected_sni=b'target-server', + ) + + self.assertEqual(len(http_server.requests), 1) + request = http_server.requests[0] + self.assertEqual(request.method, b'GET') + self.assertEqual(request.path, b'/foo/bar') + self.assertEqual( + request.requestHeaders.getRawHeaders(b'host'), + [b'target-server'], + ) + + # finish the request + request.finish() + self.reactor.pump((0.1,)) + self.successResultOf(test_d) + + self.assertEqual(self.well_known_cache[b"testserv"], b"target-server") + + # check the cache expires + self.reactor.pump((25 * 3600,)) + self.well_known_cache.expire() + self.assertNotIn(b"testserv", self.well_known_cache) + def test_get_hostname_srv(self): """ Test the behaviour when there is a single SRV record