dandri/synapse
1
0
Fork 0
mirror of https://github.com/element-hq/synapse.git synced 2026-09-25 11:13:48 +00:00
Code Issues Packages Projects Releases Wiki Activity

Restrict what errors we forward to clients from federation and policy servers

Browse Source 
Fixes: https://github.com/element-hq/synapse/security/advisories/GHSA-95fh-hv8c-chvq
Fixes: https://github.com/matrix-org/internal-config/issues/1721

https://github.com/element-hq/synapse-private/pull/158 is the end goal, I think

-----

Reviewed-on: https://github.com/element-hq/synapse-private/pull/159
This commit is contained in:
Olivier 'reivilibre
2026-07-28 13:58:06 +01:00
committed by Olivier 'reivilibre
parent 44216bf2b6
commit c3adee3509
4 changed files with 285 additions and 7 deletions
Download Patch File Download Diff File Expand all files Collapse all files
synapse/api/errors.py
+66 -4
View File
@@ -823,20 +823,24 @@ class HttpResponseException(CodeMessageException):
super().__init__(code, msg)
self.response = response
def to_synapse_error(self) -> SynapseError:
"""Make a SynapseError based on an HTTPResponseException
def unsafe_to_verbatim_synapse_error(self) -> SynapseError:
"""Make a SynapseError directly based on a TRUSTED HTTPResponseException.
This is useful when a proxied request has failed, and we need to
decide how to map the failure onto a matrix error to send back to the
client.
An attempt is made to parse the body of the http response as a matrix
An attempt is made to parse the body of the HTTP response as a Matrix
error. If that succeeds, the errcode and error message from the body
are used as the errcode and error message in the new synapse error.
are copied verbatim into the new Synapse error.
Otherwise, the errcode is set to M_UNKNOWN, and the error message is
set to the reason code from the HTTP response.
Safety:
This must ONLY be used on errors from TRUSTED sources,
such as other Synapse workers.
Returns:
The error converted to a SynapseError.
"""
@@ -851,10 +855,68 @@ class HttpResponseException(CodeMessageException):
j = {}
errcode = j.pop("errcode", Codes.UNKNOWN)
if not isinstance(errcode, str):
errcode = Codes.UNKNOWN
errmsg = j.pop("error", self.msg)
if not isinstance(errmsg, str):
errmsg = self.msg
return ProxiedRequestError(self.code, errmsg, errcode, j)
def to_synapse_error(self) -> SynapseError:
"""Make a SynapseError directly based on a TRUSTED HTTPResponseException.
This is useful when a proxied request has failed, and we need to
decide how to map the failure onto a matrix error to send back to the
client.
An attempt is made to parse the body of the HTTP response as a Matrix
error. If that succeeds, the errcode and error message from the body
are copied verbatim into the new Synapse error, unless it's of
a forbidden type.
Otherwise, the errcode is set to M_UNKNOWN, and the error message is
set to the reason code from the HTTP response.
Safety:
This is the correct method to use when forwarding errors
from upstream requests (e.g. federation, policy servers).
FIXME: restrict forwarded errors further
Returns:
The error converted to a SynapseError.
"""
# try to parse the body as json, to get better errcode/msg, but
# default to M_UNKNOWN with the HTTP status as the error text
try:
j = json_decoder.decode(self.response.decode("utf-8"))
except ValueError:
j = {}
if not isinstance(j, dict):
j = {}
status = self.code
errcode = j.pop("errcode", Codes.UNKNOWN)
if not isinstance(errcode, str):
errcode = Codes.UNKNOWN
errmsg = j.pop("error", self.msg)
if not isinstance(errmsg, str):
errmsg = self.msg
if errcode == Codes.UNKNOWN_TOKEN:
# We must not relay this error code back down to clients,
# because clients interpret this code to mean that they
# have been logged out.
# See: https://github.com/element-hq/synapse/security/advisories/GHSA-95fh-hv8c-chvq
errcode = Codes.UNKNOWN
if status == HTTPStatus.UNAUTHORIZED:
status = HTTPStatus.BAD_REQUEST
return ProxiedRequestError(status, errmsg, errcode, j)
class HomeServerNotSetupException(Exception):
"""
synapse/replication/http/_base.py
+6 -1
View File
@@ -344,7 +344,12 @@ class ReplicationEndpoint(metaclass=abc.ABCMeta):
code=e.code,
**{SERVER_NAME_LABEL: server_name},
).inc()
raise e.to_synapse_error()
# This error is coming from another worker, so we trust it to be safe
# to relay to clients directly.
# In fact, we rely relaying verbatim at the very least to tell
# clients when they are rate-limited,
# but most likely other things too.
raise e.unsafe_to_verbatim_synapse_error()
except Exception as e:
_outgoing_request_counter.labels(
name=cls.NAME,
tests/handlers/test_room_policy.py
+95 -1
View File
@@ -12,17 +12,22 @@
# <https://www.gnu.org/licenses/agpl-3.0.html>.
#
#
from http import HTTPStatus
from unittest import mock
import signedjson
from parameterized import parameterized
from signedjson.key import encode_verify_key_base64, get_verify_key
from twisted.internet import defer
from twisted.internet.testing import MemoryReactor
from twisted.web.client import Agent
from synapse.api.constants import EventTypes
from synapse.api.errors import HttpResponseException, SynapseError
from synapse.crypto.event_signing import compute_event_signature
from synapse.events import EventBase
from synapse.federation.transport.client import TransportLayerClient
from synapse.handlers.room_policy import POLICY_SERVER_KEY_ID
from synapse.rest import admin
from synapse.rest.client import filter, login, room, sync
@@ -31,7 +36,7 @@ from synapse.types import JsonDict, UserID
from synapse.util.clock import Clock
from tests import unittest
from tests.test_utils import event_injection
from tests.test_utils import FakeResponse, event_injection
from tests.test_utils.event_builders import make_test_event
@@ -546,3 +551,92 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase):
if ev["event_id"] == event_id:
return ev
return None
def _mock_policy_server_response_with_http_error(
self,
status: HTTPStatus,
error_body: JsonDict,
) -> None:
"""
Make the policy server reply to its `/sign` endpoint with an error.
Args:
status: the HTTP status to return
error_body: the JSON error body to return
"""
def request(
method: bytes,
uri: bytes,
headers: object = None,
bodyProducer: object = None,
) -> "defer.Deferred":
# For our test, we don't expect any other outbound request
assert b"/_matrix/policy/v1/sign" in uri, (
f"unexpected outbound request to {uri!r}"
)
return defer.succeed(
FakeResponse.json(
code=status,
payload=error_body,
)
)
fake_agent = mock.create_autospec(Agent, spec_set=True)
fake_agent.request.side_effect = request
self.handler._federation_client.transport_layer = TransportLayerClient(self.hs)
self.hs.get_federation_http_client().agent = fake_agent
@parameterized.expand(
(
(
HTTPStatus.IM_A_TEAPOT,
{"errcode": "M_FORBIDDEN", "error": "No coffee here"},
HTTPStatus.IM_A_TEAPOT,
{"errcode": "M_FORBIDDEN", "error": "No coffee here"},
),
# This case is https://github.com/element-hq/synapse/security/advisories/GHSA-95fh-hv8c-chvq
# The error is rewritten for safety.
(
HTTPStatus.UNAUTHORIZED,
{"errcode": "M_UNKNOWN_TOKEN", "error": "unknown token"},
HTTPStatus.BAD_REQUEST,
{
"errcode": "M_UNKNOWN",
"error": "unknown token",
},
),
)
)
def test_policy_server_error_bubbling_to_client(
self,
policy_server_error_status: HTTPStatus,
policy_server_error_body: JsonDict,
expected_client_facing_error_status: HTTPStatus,
expected_client_facing_error_body: JsonDict,
) -> None:
"""
Tests how errors from the policy server are forwarded back to clients.
Regression test for https://github.com/element-hq/synapse/security/advisories/GHSA-95fh-hv8c-chvq
"""
verify_key_str = encode_verify_key_base64(get_verify_key(self.signing_key))
self._add_policy_server_to_room(public_key=verify_key_str)
# Mock the policy server (at the HTTP level) to return
# the configured error
self._mock_policy_server_response_with_http_error(
policy_server_error_status,
policy_server_error_body,
)
response_body = self.helper.send_event(
self.room_id,
"m.room.message",
{"body": "honk", "msgtype": "m.text"},
tok=self.creator_token,
expect_code=expected_client_facing_error_status,
)
self.assertEqual(response_body, expected_client_facing_error_body)
tests/rest/client/test_rooms.py
+118 -1
View File
@@ -26,12 +26,14 @@
import json
from http import HTTPStatus
from typing import Any, Iterable, Literal
from unittest.mock import AsyncMock, Mock, call, patch
from unittest.mock import AsyncMock, Mock, call, create_autospec, patch
from urllib import parse as urlparse
from parameterized import param, parameterized
from twisted.internet import defer
from twisted.internet.testing import MemoryReactor
from twisted.web.client import Agent
import synapse.rest.admin
from synapse.api.constants import (
@@ -67,6 +69,7 @@ from synapse.util.stringutils import random_string
from tests import unittest
from tests.http.server._base import make_request_with_cancellation_test
from tests.storage.test_stream import PaginationTestCase
from tests.test_utils import FakeResponse
from tests.test_utils.event_injection import (
create_event,
inject_event,
@@ -5764,3 +5767,117 @@ class MSC4293RedactOnBanKickTestCase(unittest.FederatingHomeserverTestCase):
expect_redaction=True,
reason="being disruptive",
)
class CreateRoomRemoteInviteTestCase(unittest.FederatingHomeserverTestCase):
"""
Tests error propagation from remote invites during /createRoom.
Regression test for https://github.com/element-hq/synapse/security/advisories/GHSA-95fh-hv8c-chvq.
"""
servlets = [
room.register_servlets,
login.register_servlets,
register.register_servlets,
admin.register_servlets,
]
hijack_auth = False
def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
self.user_id = self.register_user("creator", "test")
self.token = self.login("creator", "test")
def _mock_remote_invite_http_error(
self,
status: int,
error_body: JsonDict,
) -> None:
"""
Make the remote homeserver reply to its `/invite` endpoint with an error.
Args:
status: the HTTP status to return
error_body: the JSON error body to return
"""
federation_http_client = self.hs.get_federation_http_client()
fake_agent = create_autospec(Agent, spec_set=True)
def request(
method: bytes,
uri: bytes,
headers: object = None,
bodyProducer: object = None,
) -> "defer.Deferred":
# For our test, we don't expect any other outbound request
assert b"/invite/" in uri, f"unexpected outbound request to {uri!r}"
return defer.succeed(
FakeResponse.json(
code=status,
payload=error_body,
)
)
fake_agent.request.side_effect = request
federation_http_client.agent = fake_agent
@parameterized.expand(
(
(
HTTPStatus.IM_A_TEAPOT,
{
"errcode": "M_FORBIDDEN",
"error": "You can't invite this user",
},
HTTPStatus.IM_A_TEAPOT,
{
"errcode": "M_FORBIDDEN",
"error": "You can't invite this user",
},
),
# This case is https://github.com/element-hq/synapse/security/advisories/GHSA-95fh-hv8c-chvq
# The error is rewritten for safety.
(
HTTPStatus.UNAUTHORIZED,
{"errcode": "M_UNKNOWN_TOKEN", "error": "unknown token"},
HTTPStatus.BAD_REQUEST,
{
"errcode": "M_UNKNOWN",
"error": "unknown token",
},
),
)
)
def test_remote_invite_bubbles_errors(
self,
policy_server_error_status: HTTPStatus,
policy_server_error_body: JsonDict,
expected_client_facing_error_status: HTTPStatus,
expected_client_facing_error_body: JsonDict,
) -> None:
"""
Test that, when creating a room involving a remote invite,
when the remote homeserver returns an error, we bubble it
to the client carefully.
Regression test for https://github.com/element-hq/synapse/security/advisories/GHSA-95fh-hv8c-chvq
"""
# Mock the remote homeserver (at the HTTP level) to return the configured error
self._mock_remote_invite_http_error(
policy_server_error_status,
policy_server_error_body,
)
channel = self.make_request(
"POST",
"/createRoom",
{"invite": ["@alice:" + self.OTHER_SERVER_NAME]},
access_token=self.token,
)
self.assertEqual(
channel.code, expected_client_facing_error_status, channel.result
)
self.assertEqual(channel.json_body, expected_client_facing_error_body)