From 48563d6e2baf1d6bd9b02efd974a423596833555 Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 20 Jun 2023 02:15:57 -0500 Subject: [PATCH 1/3] Switch from `matrix://` to `matrix-federation://` scheme for internal Synapse routing of outbound federation traffic `matrix://` is a registered specced scheme nowadays and doesn't make sense for our internal to Synapse use case anymore. ([discussion] (https://github.com/matrix-org/synapse/pull/15773#discussion_r1227598679)) --- contrib/lnav/synapse-log-format.json | 2 +- scripts-dev/federation_client.py | 4 +- .../federation/matrix_federation_agent.py | 12 +++--- synapse/http/matrixfederationclient.py | 9 ++++- tests/federation/test_federation_client.py | 4 +- .../test_matrix_federation_agent.py | 38 ++++++++++--------- 6 files changed, 41 insertions(+), 28 deletions(-) diff --git a/contrib/lnav/synapse-log-format.json b/contrib/lnav/synapse-log-format.json index ad7017ee5ec2..649cd623e8b4 100644 --- a/contrib/lnav/synapse-log-format.json +++ b/contrib/lnav/synapse-log-format.json @@ -29,7 +29,7 @@ "level": "error" }, { - "line": "my-matrix-server-federation-sender-1 | 2023-01-25 20:56:20,995 - synapse.http.matrixfederationclient - 709 - WARNING - federation_transaction_transmission_loop-3 - {PUT-O-3} [example.com] Request failed: PUT matrix://example.com/_matrix/federation/v1/send/1674680155797: HttpResponseException('403: Forbidden')", + "line": "my-matrix-server-federation-sender-1 | 2023-01-25 20:56:20,995 - synapse.http.matrixfederationclient - 709 - WARNING - federation_transaction_transmission_loop-3 - {PUT-O-3} [example.com] Request failed: PUT matrix-federation://example.com/_matrix/federation/v1/send/1674680155797: HttpResponseException('403: Forbidden')", "level": "warning" }, { diff --git a/scripts-dev/federation_client.py b/scripts-dev/federation_client.py index b1d5e2e61667..63f0b25ddd7f 100755 --- a/scripts-dev/federation_client.py +++ b/scripts-dev/federation_client.py @@ -136,11 +136,11 @@ def request( authorization_headers.append(header) print("Authorization: %s" % header, file=sys.stderr) - dest = "matrix://%s%s" % (destination, path) + dest = "matrix-federation://%s%s" % (destination, path) print("Requesting %s" % dest, file=sys.stderr) s = requests.Session() - s.mount("matrix://", MatrixConnectionAdapter()) + s.mount("matrix-federation://", MatrixConnectionAdapter()) headers: Dict[str, str] = { "Authorization": authorization_headers[0], diff --git a/synapse/http/federation/matrix_federation_agent.py b/synapse/http/federation/matrix_federation_agent.py index 7e8cf316822d..9f8b68bedf16 100644 --- a/synapse/http/federation/matrix_federation_agent.py +++ b/synapse/http/federation/matrix_federation_agent.py @@ -51,8 +51,10 @@ @implementer(IAgent) class MatrixFederationAgent: """An Agent-like thing which provides a `request` method which correctly - handles resolving matrix server names when using matrix://. Handles standard - https URIs as normal. + handles resolving matrix server names when using `matrix-federation://`. Handles + standard https URIs as normal. The `matrix-federation://` scheme is internal to + Synapse and we purposely want to avoid colliding with the `matrix://` URL scheme + which is now specced. Doesn't implement any retries. (Those are done in MatrixFederationHttpClient.) @@ -167,14 +169,14 @@ def request( # There must be a valid hostname. assert parsed_uri.hostname - # If this is a matrix:// URI check if the server has delegated matrix + # If this is a matrix-federation:// URI check if the server has delegated matrix # traffic using well-known delegation. # # We have to do this here and not in the endpoint as we need to rewrite # the host header with the delegated server name. delegated_server = None if ( - parsed_uri.scheme == b"matrix" + parsed_uri.scheme == b"matrix-federation" and not _is_ip_literal(parsed_uri.hostname) and not parsed_uri.port ): @@ -250,7 +252,7 @@ def endpointForURI(self, parsed_uri: URI) -> "MatrixHostnameEndpoint": @implementer(IStreamClientEndpoint) class MatrixHostnameEndpoint: - """An endpoint that resolves matrix:// URLs using Matrix server name + """An endpoint that resolves matrix-federation:// URLs using Matrix server name resolution (i.e. via SRV). Does not check for well-known delegation. Args: diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index abb5ae581521..fc0101808d0f 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -174,7 +174,14 @@ def __attrs_post_init__(self) -> None: # The object is frozen so we can pre-compute this. uri = urllib.parse.urlunparse( - (b"matrix", destination_bytes, path_bytes, None, query_bytes, b"") + ( + b"matrix-federation", + destination_bytes, + path_bytes, + None, + query_bytes, + b"", + ) ) object.__setattr__(self, "uri", uri) diff --git a/tests/federation/test_federation_client.py b/tests/federation/test_federation_client.py index 91694e4fcada..a45ab8368347 100644 --- a/tests/federation/test_federation_client.py +++ b/tests/federation/test_federation_client.py @@ -124,7 +124,7 @@ def test_get_room_state(self) -> None: # check the right call got made to the agent self._mock_agent.request.assert_called_once_with( b"GET", - b"matrix://yet.another.server/_matrix/federation/v1/state/%21room_id?event_id=event_id", + b"matrix-federation://yet.another.server/_matrix/federation/v1/state/%21room_id?event_id=event_id", headers=mock.ANY, bodyProducer=None, ) @@ -232,7 +232,7 @@ def _get_pdu_once(self) -> EventBase: # check the right call got made to the agent self._mock_agent.request.assert_called_once_with( b"GET", - b"matrix://yet.another.server/_matrix/federation/v1/event/event_id", + b"matrix-federation://yet.another.server/_matrix/federation/v1/event/event_id", headers=mock.ANY, bodyProducer=None, ) diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index 105b4caefa3d..aed2a4c07a46 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -292,7 +292,7 @@ def _do_get(self) -> None: self.agent = self._make_agent() self.reactor.lookups["testserv"] = "1.2.3.4" - test_d = self._make_get_request(b"matrix://testserv:8448/foo/bar") + test_d = self._make_get_request(b"matrix-federation://testserv:8448/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -393,7 +393,7 @@ def _do_get_via_proxy( self.reactor.lookups["testserv"] = "1.2.3.4" self.reactor.lookups["proxy.com"] = "9.9.9.9" - test_d = self._make_get_request(b"matrix://testserv:8448/foo/bar") + test_d = self._make_get_request(b"matrix-federation://testserv:8448/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -532,7 +532,7 @@ def test_get_ip_address(self) -> None: # there will be a getaddrinfo on the IP self.reactor.lookups["1.2.3.4"] = "1.2.3.4" - test_d = self._make_get_request(b"matrix://1.2.3.4/foo/bar") + test_d = self._make_get_request(b"matrix-federation://1.2.3.4/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -568,7 +568,7 @@ def test_get_ipv6_address(self) -> None: # there will be a getaddrinfo on the IP self.reactor.lookups["::1"] = "::1" - test_d = self._make_get_request(b"matrix://[::1]/foo/bar") + test_d = self._make_get_request(b"matrix-federation://[::1]/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -604,7 +604,7 @@ def test_get_ipv6_address_with_port(self) -> None: # there will be a getaddrinfo on the IP self.reactor.lookups["::1"] = "::1" - test_d = self._make_get_request(b"matrix://[::1]:80/foo/bar") + test_d = self._make_get_request(b"matrix-federation://[::1]:80/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -639,7 +639,7 @@ def test_get_hostname_bad_cert(self) -> None: self.mock_resolver.resolve_service.side_effect = generate_resolve_service([]) self.reactor.lookups["testserv1"] = "1.2.3.4" - test_d = self._make_get_request(b"matrix://testserv1/foo/bar") + test_d = self._make_get_request(b"matrix-federation://testserv1/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -693,7 +693,7 @@ def test_get_ip_address_bad_cert(self) -> None: # there will be a getaddrinfo on the IP self.reactor.lookups["1.2.3.5"] = "1.2.3.5" - test_d = self._make_get_request(b"matrix://1.2.3.5/foo/bar") + test_d = self._make_get_request(b"matrix-federation://1.2.3.5/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -725,7 +725,7 @@ def test_get_no_srv_no_well_known(self) -> None: self.mock_resolver.resolve_service.side_effect = generate_resolve_service([]) self.reactor.lookups["testserv"] = "1.2.3.4" - test_d = self._make_get_request(b"matrix://testserv/foo/bar") + test_d = self._make_get_request(b"matrix-federation://testserv/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -780,7 +780,7 @@ def test_get_well_known(self) -> None: 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") + test_d = self._make_get_request(b"matrix-federation://testserv/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -844,7 +844,7 @@ def test_get_well_known_redirect(self) -> None: 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") + test_d = self._make_get_request(b"matrix-federation://testserv/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -933,7 +933,7 @@ def test_get_invalid_well_known(self) -> None: self.mock_resolver.resolve_service.side_effect = generate_resolve_service([]) self.reactor.lookups["testserv"] = "1.2.3.4" - test_d = self._make_get_request(b"matrix://testserv/foo/bar") + test_d = self._make_get_request(b"matrix-federation://testserv/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -1009,7 +1009,7 @@ def test_get_well_known_unsigned_cert(self) -> None: ), ) - test_d = agent.request(b"GET", b"matrix://testserv/foo/bar") + test_d = agent.request(b"GET", b"matrix-federation://testserv/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -1042,7 +1042,7 @@ def test_get_hostname_srv(self) -> None: ) self.reactor.lookups["srvtarget"] = "1.2.3.4" - test_d = self._make_get_request(b"matrix://testserv/foo/bar") + test_d = self._make_get_request(b"matrix-federation://testserv/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -1082,7 +1082,7 @@ def test_get_well_known_srv(self) -> None: self.reactor.lookups["testserv"] = "1.2.3.4" self.reactor.lookups["srvtarget"] = "5.6.7.8" - test_d = self._make_get_request(b"matrix://testserv/foo/bar") + test_d = self._make_get_request(b"matrix-federation://testserv/foo/bar") # Nothing happened yet self.assertNoResult(test_d) @@ -1143,7 +1143,9 @@ def test_idna_servername(self) -> None: self.reactor.lookups["xn--bcher-kva.com"] = "1.2.3.4" # this is idna for bücher.com - test_d = self._make_get_request(b"matrix://xn--bcher-kva.com/foo/bar") + test_d = self._make_get_request( + b"matrix-federation://xn--bcher-kva.com/foo/bar" + ) # Nothing happened yet self.assertNoResult(test_d) @@ -1204,7 +1206,9 @@ def test_idna_srv_target(self) -> None: ) self.reactor.lookups["xn--trget-3qa.com"] = "1.2.3.4" - test_d = self._make_get_request(b"matrix://xn--bcher-kva.com/foo/bar") + test_d = self._make_get_request( + b"matrix-federation://xn--bcher-kva.com/foo/bar" + ) # Nothing happened yet self.assertNoResult(test_d) @@ -1411,7 +1415,7 @@ def test_srv_fallbacks(self) -> None: ) self.reactor.lookups["target.com"] = "1.2.3.4" - test_d = self._make_get_request(b"matrix://testserv/foo/bar") + test_d = self._make_get_request(b"matrix-federation://testserv/foo/bar") # Nothing happened yet self.assertNoResult(test_d) From 170d6f63803d680517083fde0d148c15daa5be1d Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 20 Jun 2023 02:19:24 -0500 Subject: [PATCH 2/3] Add changelog --- changelog.d/15806.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/15806.misc diff --git a/changelog.d/15806.misc b/changelog.d/15806.misc new file mode 100644 index 000000000000..80d0eb2f8f46 --- /dev/null +++ b/changelog.d/15806.misc @@ -0,0 +1 @@ +Switch from `matrix://` to `matrix-federation://` scheme for internal Synapse routing of outbound federation traffic. From 78cf4a08391b58f8f27fb879a67fc0dfd462aead Mon Sep 17 00:00:00 2001 From: Eric Eastwood Date: Tue, 20 Jun 2023 02:49:39 -0500 Subject: [PATCH 3/3] Fix missing spot --- synapse/http/federation/matrix_federation_agent.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/http/federation/matrix_federation_agent.py b/synapse/http/federation/matrix_federation_agent.py index 9f8b68bedf16..91a24efcd019 100644 --- a/synapse/http/federation/matrix_federation_agent.py +++ b/synapse/http/federation/matrix_federation_agent.py @@ -381,7 +381,7 @@ async def _resolve_server(self) -> List[Server]: connect to. """ - if self._parsed_uri.scheme != b"matrix": + if self._parsed_uri.scheme != b"matrix-federation": return [Server(host=self._parsed_uri.host, port=self._parsed_uri.port)] # Note: We don't do well-known lookup as that needs to have happened