summary refs log tree commit diff
diff options
context:
space:
mode:
authorShay <hillerys@element.io>2022-07-20 11:17:26 -0700
committerGitHub <noreply@github.com>2022-07-20 11:17:26 -0700
commita1b62af2afc4a5439b7276a02f9fd981fbfd06a4 (patch)
treecb8c9181d954c4757e27f277348c83ce0d942439
parentMerge remote-tracking branch 'origin/master' into develop (diff)
downloadsynapse-a1b62af2afc4a5439b7276a02f9fd981fbfd06a4.tar.xz
Validate federation destinations and log an error if server name is invalid. (#13318)
-rw-r--r--changelog.d/13318.misc1
-rw-r--r--synapse/http/matrixfederationclient.py9
-rw-r--r--tests/federation/test_federation_client.py4
3 files changed, 12 insertions, 2 deletions
diff --git a/changelog.d/13318.misc b/changelog.d/13318.misc
new file mode 100644
index 0000000000..f5cd26b862
--- /dev/null
+++ b/changelog.d/13318.misc
@@ -0,0 +1 @@
+Validate federation destinations and log an error if a destination is invalid.
diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py
index c63d068f74..3c35b1d2c7 100644
--- a/synapse/http/matrixfederationclient.py
+++ b/synapse/http/matrixfederationclient.py
@@ -79,6 +79,7 @@ from synapse.types import JsonDict
 from synapse.util import json_decoder
 from synapse.util.async_helpers import AwakenableSleeper, timeout_deferred
 from synapse.util.metrics import Measure
+from synapse.util.stringutils import parse_and_validate_server_name
 
 if TYPE_CHECKING:
     from synapse.server import HomeServer
@@ -479,6 +480,14 @@ class MatrixFederationHttpClient:
             RequestSendFailed: If there were problems connecting to the
                 remote, due to e.g. DNS failures, connection timeouts etc.
         """
+        # Validate server name and log if it is an invalid destination, this is
+        # partially to help track down code paths where we haven't validated before here
+        try:
+            parse_and_validate_server_name(request.destination)
+        except ValueError:
+            logger.exception(f"Invalid destination: {request.destination}.")
+            raise FederationDeniedError(request.destination)
+
         if timeout:
             _sec_timeout = timeout / 1000
         else:
diff --git a/tests/federation/test_federation_client.py b/tests/federation/test_federation_client.py
index d2bda07198..cf6b130e4f 100644
--- a/tests/federation/test_federation_client.py
+++ b/tests/federation/test_federation_client.py
@@ -102,7 +102,7 @@ class FederationClientTest(FederatingHomeserverTestCase):
         # now fire off the request
         state_resp, auth_resp = self.get_success(
             self.hs.get_federation_client().get_room_state(
-                "yet_another_server",
+                "yet.another.server",
                 test_room_id,
                 "event_id",
                 RoomVersions.V9,
@@ -112,7 +112,7 @@ class FederationClientTest(FederatingHomeserverTestCase):
         # 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://yet.another.server/_matrix/federation/v1/state/%21room_id?event_id=event_id",
             headers=mock.ANY,
             bodyProducer=None,
         )