Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Validate federation destinations and log an error if server name is invalid. #13318

Merged
merged 6 commits into from
Jul 20, 2022
Merged

Validate federation destinations and log an error if server name is invalid. #13318

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
8739c0d
validate destinations
H-Shay Jul 16, 2022
7f0c89d
update test to pass destination validation
H-Shay Jul 16, 2022
b468a3c
newsfragment + typo
H-Shay Jul 18, 2022
79745f4
random line changes fix
H-Shay Jul 18, 2022
a85c648
suggested changes
H-Shay Jul 20, 2022
30d5503
error -> exception
H-Shay Jul 20, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/13318.misc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Validate federation destinations and log an error if a destination is invalid.
9 changes: 9 additions & 0 deletions synapse/http/matrixfederationclient.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@
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
Expand Down Expand Up @@ -479,6 +480,14 @@ async def _send_request(
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)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure if this was the proper error but it seemed the closest out of all of the options, open to suggestions if it's not right!

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems reasonable! RequestSendFailed could be an alternative, since we can't connect to malformed server names.


if timeout:
_sec_timeout = timeout / 1000
else:
Expand Down
4 changes: 2 additions & 2 deletions tests/federation/test_federation_client.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ def test_get_room_state(self):
# 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,
Expand All @@ -112,7 +112,7 @@ def test_get_room_state(self):
# 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,
)
Expand Down