From d428b463465a24a9922bbc759198398377b76d0a Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Thu, 31 Jan 2019 23:13:44 +0000 Subject: [PATCH] Update federation routing logic to check .well-known before SRV --- changelog.d/4539.misc | 1 + .../federation/matrix_federation_agent.py | 10 ++-- .../test_matrix_federation_agent.py | 51 ++++++++----------- 3 files changed, 27 insertions(+), 35 deletions(-) create mode 100644 changelog.d/4539.misc diff --git a/changelog.d/4539.misc b/changelog.d/4539.misc new file mode 100644 index 0000000000..b222c8d23f --- /dev/null +++ b/changelog.d/4539.misc @@ -0,0 +1 @@ +Update federation routing logic to check .well-known before SRV diff --git a/synapse/http/federation/matrix_federation_agent.py b/synapse/http/federation/matrix_federation_agent.py index 26649e70be..57dcab4727 100644 --- a/synapse/http/federation/matrix_federation_agent.py +++ b/synapse/http/federation/matrix_federation_agent.py @@ -210,11 +210,7 @@ class MatrixFederationAgent(object): target_port=parsed_uri.port, )) - # try a SRV lookup - service_name = b"_matrix._tcp.%s" % (parsed_uri.host,) - server_list = yield self._srv_resolver.resolve_service(service_name) - - if not server_list and lookup_well_known: + if lookup_well_known: # try a .well-known lookup well_known_server = yield self._get_well_known(parsed_uri.host) @@ -250,6 +246,10 @@ class MatrixFederationAgent(object): res = yield self._route_matrix_uri(new_uri, lookup_well_known=False) defer.returnValue(res) + # try a SRV lookup + service_name = b"_matrix._tcp.%s" % (parsed_uri.host,) + server_list = yield self._srv_resolver.resolve_service(service_name) + if not server_list: target_host = parsed_uri.host port = 8448 diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index 7b2800021f..369e63ecc6 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -358,9 +358,8 @@ class MatrixFederationAgentTests(TestCase): # Nothing happened yet self.assertNoResult(test_d) - self.mock_resolver.resolve_service.assert_called_once_with( - b"_matrix._tcp.testserv", - ) + # No SRV record lookup yet + self.mock_resolver.resolve_service.assert_not_called() # there should be an attempt to connect on port 443 for the .well-known clients = self.reactor.tcpClients @@ -376,6 +375,11 @@ class MatrixFederationAgentTests(TestCase): # .well-known request fails. self.reactor.pump((0.4,)) + # now there should be a SRV lookup + self.mock_resolver.resolve_service.assert_called_once_with( + b"_matrix._tcp.testserv", + ) + # we should fall back to a direct connection self.assertEqual(len(clients), 2) (host, port, client_factory, _timeout, _bindAddress) = clients[1] @@ -403,8 +407,7 @@ class MatrixFederationAgentTests(TestCase): self.successResultOf(test_d) def test_get_well_known(self): - """Test the behaviour when the server name has no port and no SRV record, but - the .well-known redirects elsewhere + """Test the behaviour when the .well-known redirects elsewhere """ self.mock_resolver.resolve_service.side_effect = lambda _: [] @@ -416,11 +419,6 @@ class MatrixFederationAgentTests(TestCase): # 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) @@ -432,7 +430,7 @@ class MatrixFederationAgentTests(TestCase): client_factory, expected_sni=b"testserv", target_server=b"target-server", ) - # there should be another SRV lookup + # there should be a SRV lookup self.mock_resolver.resolve_service.assert_called_once_with( b"_matrix._tcp.target-server", ) @@ -483,11 +481,6 @@ class MatrixFederationAgentTests(TestCase): # 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) @@ -529,7 +522,7 @@ class MatrixFederationAgentTests(TestCase): self.reactor.pump((0.1, )) - # there should be another SRV lookup + # there should be a SRV lookup self.mock_resolver.resolve_service.assert_called_once_with( b"_matrix._tcp.target-server", ) @@ -581,6 +574,7 @@ class MatrixFederationAgentTests(TestCase): # Nothing happened yet self.assertNoResult(test_d) + # the request for a .well-known will have failed with a DNS lookup error. self.mock_resolver.resolve_service.assert_called_once_with( b"_matrix._tcp.testserv", ) @@ -613,11 +607,9 @@ class MatrixFederationAgentTests(TestCase): self.successResultOf(test_d) def test_get_well_known_srv(self): - """Test the behaviour when the server name has no port and no SRV record, but - the .well-known redirects to a place where there is a SRV. + """Test the behaviour when the .well-known redirects to a place where there + is a SRV. """ - - self.mock_resolver.resolve_service.side_effect = lambda _: [] self.reactor.lookups["testserv"] = "1.2.3.4" self.reactor.lookups["srvtarget"] = "5.6.7.8" @@ -626,11 +618,6 @@ class MatrixFederationAgentTests(TestCase): # 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) @@ -646,7 +633,7 @@ class MatrixFederationAgentTests(TestCase): client_factory, expected_sni=b"testserv", target_server=b"target-server", ) - # there should be another SRV lookup + # there should be a SRV lookup self.mock_resolver.resolve_service.assert_called_once_with( b"_matrix._tcp.target-server", ) @@ -691,9 +678,8 @@ class MatrixFederationAgentTests(TestCase): # Nothing happened yet self.assertNoResult(test_d) - self.mock_resolver.resolve_service.assert_called_once_with( - b"_matrix._tcp.xn--bcher-kva.com", - ) + # No SRV record lookup yet + self.mock_resolver.resolve_service.assert_not_called() # there should be an attempt to connect on port 443 for the .well-known clients = self.reactor.tcpClients @@ -709,6 +695,11 @@ class MatrixFederationAgentTests(TestCase): # .well-known request fails. self.reactor.pump((0.4,)) + # now there should have been a SRV lookup + self.mock_resolver.resolve_service.assert_called_once_with( + b"_matrix._tcp.xn--bcher-kva.com", + ) + # We should fall back to port 8448 clients = self.reactor.tcpClients self.assertEqual(len(clients), 2)