diff options
| -rw-r--r-- | changelog.d/4544.misc | 1 | ||||
| -rw-r--r-- | synapse/http/federation/matrix_federation_agent.py | 25 | ||||
| -rw-r--r-- | tests/http/federation/test_matrix_federation_agent.py | 81 |
3 files changed, 77 insertions, 30 deletions
diff --git a/changelog.d/4544.misc b/changelog.d/4544.misc new file mode 100644 index 0000000000..b29fc8575c --- /dev/null +++ b/changelog.d/4544.misc @@ -0,0 +1 @@ +Treat an invalid .well-known file the same as an absent one \ 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 50baa8bf0a..c4312a3653 100644 --- a/synapse/http/federation/matrix_federation_agent.py +++ b/synapse/http/federation/matrix_federation_agent.py @@ -47,9 +47,6 @@ WELL_KNOWN_INVALID_CACHE_PERIOD = 1 * 3600 # cap for .well-known cache period WELL_KNOWN_MAX_CACHE_PERIOD = 48 * 3600 -# magic value to mark an invalid well-known -INVALID_WELL_KNOWN = object() - logger = logging.getLogger(__name__) well_known_cache = TTLCache('well-known') @@ -108,8 +105,7 @@ class MatrixFederationAgent(object): # our cache of .well-known lookup results, mapping from server name # to delegated name. The values can be: # `bytes`: a valid server-name - # `None`: there is no .well-known here - # INVALID_WELL_KNWOWN: the .well-known here is invalid + # `None`: there is no (valid) .well-known here self._well_known_cache = _well_known_cache @defer.inlineCallbacks @@ -302,9 +298,6 @@ class MatrixFederationAgent(object): if cache_period > 0: self._well_known_cache.set(server_name, result, cache_period) - if result == INVALID_WELL_KNOWN: - raise Exception("invalid .well-known on this server") - defer.returnValue(result) @defer.inlineCallbacks @@ -331,16 +324,7 @@ class MatrixFederationAgent(object): body = yield make_deferred_yieldable(readBody(response)) if response.code != 200: raise Exception("Non-200 response %s" % (response.code, )) - except Exception as e: - logger.info("Error fetching %s: %s", uri_str, e) - - # add some randomness to the TTL to avoid a stampeding herd every hour - # after startup - cache_period = WELL_KNOWN_INVALID_CACHE_PERIOD - cache_period += random.uniform(0, WELL_KNOWN_DEFAULT_CACHE_PERIOD_JITTER) - defer.returnValue((None, cache_period)) - try: parsed_body = json.loads(body.decode('utf-8')) logger.info("Response from .well-known: %s", parsed_body) if not isinstance(parsed_body, dict): @@ -348,10 +332,13 @@ class MatrixFederationAgent(object): if "m.server" not in parsed_body: raise Exception("Missing key 'm.server'") except Exception as e: - logger.info("invalid .well-known response from %s: %s", uri_str, e) + logger.info("Error fetching %s: %s", uri_str, e) + + # add some randomness to the TTL to avoid a stampeding herd every hour + # after startup cache_period = WELL_KNOWN_INVALID_CACHE_PERIOD cache_period += random.uniform(0, WELL_KNOWN_DEFAULT_CACHE_PERIOD_JITTER) - defer.returnValue((INVALID_WELL_KNOWN, cache_period)) + defer.returnValue((None, cache_period)) result = parsed_body["m.server"].encode("ascii") diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index 369e63ecc6..dcf184d3cf 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -124,7 +124,7 @@ class MatrixFederationAgentTests(TestCase): _check_logcontext(context) def _handle_well_known_connection( - self, client_factory, expected_sni, target_server, response_headers={}, + self, client_factory, expected_sni, content, response_headers={}, ): """Handle an outgoing HTTPs connection: wire it up to a server, check that the request is for a .well-known, and send the response. @@ -132,8 +132,7 @@ class MatrixFederationAgentTests(TestCase): Args: client_factory (IProtocolFactory): outgoing connection expected_sni (bytes): SNI that we expect the outgoing connection to send - target_server (bytes): target server that we should redirect to in the - .well-known response. + content (bytes): content to send back as the .well-known Returns: HTTPChannel: server impl """ @@ -145,10 +144,10 @@ class MatrixFederationAgentTests(TestCase): # check the .well-known request and send a response self.assertEqual(len(well_known_server.requests), 1) request = well_known_server.requests[0] - self._send_well_known_response(request, target_server, headers=response_headers) + self._send_well_known_response(request, content, headers=response_headers) return well_known_server - def _send_well_known_response(self, request, target_server, headers={}): + def _send_well_known_response(self, request, content, headers={}): """Check that an incoming request looks like a valid .well-known request, and send back the response. """ @@ -161,7 +160,7 @@ class MatrixFederationAgentTests(TestCase): # send back a response for k, v in headers.items(): request.setHeader(k, v) - request.write(b'{ "m.server": "%s" }' % (target_server,)) + request.write(content) request.finish() self.reactor.pump((0.1, )) @@ -407,7 +406,7 @@ class MatrixFederationAgentTests(TestCase): self.successResultOf(test_d) def test_get_well_known(self): - """Test the behaviour when the .well-known redirects elsewhere + """Test the behaviour when the .well-known delegates elsewhere """ self.mock_resolver.resolve_service.side_effect = lambda _: [] @@ -427,7 +426,8 @@ class MatrixFederationAgentTests(TestCase): self.assertEqual(port, 443) self._handle_well_known_connection( - client_factory, expected_sni=b"testserv", target_server=b"target-server", + client_factory, expected_sni=b"testserv", + content=b'{ "m.server": "target-server" }', ) # there should be a SRV lookup @@ -560,6 +560,64 @@ class MatrixFederationAgentTests(TestCase): self.well_known_cache.expire() self.assertNotIn(b"testserv", self.well_known_cache) + def test_get_invalid_well_known(self): + """ + Test the behaviour when the server name has an *invalid* well-known (and no SRV) + """ + + self.mock_resolver.resolve_service.side_effect = lambda _: [] + self.reactor.lookups["testserv"] = "1.2.3.4" + + test_d = self._make_get_request(b"matrix://testserv/foo/bar") + + # Nothing happened yet + self.assertNoResult(test_d) + + # 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 + self.assertEqual(len(clients), 1) + (host, port, client_factory, _timeout, _bindAddress) = clients.pop() + self.assertEqual(host, '1.2.3.4') + self.assertEqual(port, 443) + + self._handle_well_known_connection( + client_factory, expected_sni=b"testserv", content=b'NOT JSON', + ) + + # 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), 1) + (host, port, client_factory, _timeout, _bindAddress) = clients.pop() + self.assertEqual(host, '1.2.3.4') + self.assertEqual(port, 8448) + + # make a test server, and wire up the client + http_server = self._make_connection( + client_factory, + expected_sni=b'testserv', + ) + + 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'testserv'], + ) + + # finish the request + request.finish() + self.reactor.pump((0.1,)) + self.successResultOf(test_d) + def test_get_hostname_srv(self): """ Test the behaviour when there is a single SRV record @@ -630,7 +688,8 @@ class MatrixFederationAgentTests(TestCase): ] self._handle_well_known_connection( - client_factory, expected_sni=b"testserv", target_server=b"target-server", + client_factory, expected_sni=b"testserv", + content=b'{ "m.server": "target-server" }', ) # there should be a SRV lookup @@ -797,7 +856,7 @@ class MatrixFederationAgentTests(TestCase): client_factory, expected_sni=b"testserv", response_headers={b'Cache-Control': b'max-age=10'}, - target_server=b"target-server", + content=b'{ "m.server": "target-server" }', ) r = self.successResultOf(fetch_d) @@ -825,7 +884,7 @@ class MatrixFederationAgentTests(TestCase): self._handle_well_known_connection( client_factory, expected_sni=b"testserv", - target_server=b"other-server", + content=b'{ "m.server": "other-server" }', ) r = self.successResultOf(fetch_d) |