From 40d699b1d4d7855ffb892723eac90cd34f22aa6f Mon Sep 17 00:00:00 2001 From: Travis Ralston Date: Fri, 20 Mar 2026 13:34:26 -0600 Subject: [PATCH] Stable support for MSC4284 policy servers (#19503) Fixes https://github.com/element-hq/synapse/issues/19494 MSC4284 policy servers This: * removes the old `/check` (recommendation) support because it's from an older design. Policy servers should have updated to `/sign` by now. We also remove optionality around the policy server's public key because it was only optional to support `/check`. * supports the stable `m.room.policy` state event and `/sign` endpoints, falling back to unstable if required. Note the changes between unstable and stable: * Stable `/sign` uses errors instead of an empty signatures block to indicate refusal. * Stable `m.room.policy` nests the public key in an object with explicit key algorithm (always ed25519 for now) * does *not* introduce tests that the above fallback to unstable works. If it breaks, we're not going to be sad about an early transition. Tests can be added upon request, though. * fixes a bug where the policy server was asked to sign policy server state events (the events were correctly skipped in `is_event_allowed`, but `ask_policy_server_to_sign_event` didn't do the same). * fixes a bug where the original event sender's signature can be deleted if the sending server is the same as the policy server. * proxies Matrix-shaped errors from the policy server to the Client-Server API as `SynapseError`s (a new capability of the stable API). Membership event handling (from the issue) is expected to be a different PR due to the size of changes involved (tracked by https://github.com/element-hq/synapse/issues/19587). ### Pull Request Checklist * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters)) --------- Co-authored-by: turt2live <1190097+turt2live@users.noreply.github.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com> Co-authored-by: Eric Eastwood --- changelog.d/19503.bugfix.1 | 1 + changelog.d/19503.bugfix.2 | 1 + changelog.d/19503.feature | 1 + synapse/api/constants.py | 2 + synapse/federation/federation_client.py | 75 +------ synapse/federation/transport/client.py | 57 +++--- synapse/handlers/room_policy.py | 251 ++++++++++++++++-------- synapse/types/handlers/policy_server.py | 16 -- tests/handlers/test_room_policy.py | 96 ++++----- 9 files changed, 250 insertions(+), 250 deletions(-) create mode 100644 changelog.d/19503.bugfix.1 create mode 100644 changelog.d/19503.bugfix.2 create mode 100644 changelog.d/19503.feature delete mode 100644 synapse/types/handlers/policy_server.py diff --git a/changelog.d/19503.bugfix.1 b/changelog.d/19503.bugfix.1 new file mode 100644 index 0000000000..45e3ad603b --- /dev/null +++ b/changelog.d/19503.bugfix.1 @@ -0,0 +1 @@ +Fix [MSC4284](https://github.com/matrix-org/matrix-spec-proposals/pull/4284) Policy Servers implementation to skip signing `org.matrix.msc4284.policy` and `m.room.policy` state events. \ No newline at end of file diff --git a/changelog.d/19503.bugfix.2 b/changelog.d/19503.bugfix.2 new file mode 100644 index 0000000000..8a423ba785 --- /dev/null +++ b/changelog.d/19503.bugfix.2 @@ -0,0 +1 @@ +Correctly apply [MSC4284](https://github.com/matrix-org/matrix-spec-proposals/pull/4284) Policy Server signatures to events when the sender and policy server have the same server name. \ No newline at end of file diff --git a/changelog.d/19503.feature b/changelog.d/19503.feature new file mode 100644 index 0000000000..20db8aeaae --- /dev/null +++ b/changelog.d/19503.feature @@ -0,0 +1 @@ +Add stable support for [MSC4284](https://github.com/matrix-org/matrix-spec-proposals/pull/4284) Policy Servers. diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 21139a3867..eb9e6cc39b 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -158,6 +158,8 @@ class EventTypes: PollStart: Final = "m.poll.start" + RoomPolicy: Final = "m.room.policy" + class ToDeviceEventTypes: RoomKeyRequest: Final = "m.room_key_request" diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py index ba738ad65e..55151ca549 100644 --- a/synapse/federation/federation_client.py +++ b/synapse/federation/federation_client.py @@ -72,7 +72,6 @@ from synapse.http.types import QueryParams from synapse.logging.opentracing import SynapseTags, log_kv, set_tag, tag_args, trace from synapse.metrics import SERVER_NAME_LABEL from synapse.types import JsonDict, StrCollection, UserID, get_domain_from_id -from synapse.types.handlers.policy_server import RECOMMENDATION_OK, RECOMMENDATION_SPAM from synapse.util.async_helpers import concurrently_execute from synapse.util.caches.expiringcache import ExpiringCache from synapse.util.duration import Duration @@ -438,72 +437,16 @@ class FederationClient(FederationBase): return None - @trace - @tag_args - async def get_pdu_policy_recommendation( - self, destination: str, pdu: EventBase, timeout: int | None = None - ) -> str: - """Requests that the destination server (typically a policy server) - check the event and return its recommendation on how to handle the - event. - - If the policy server could not be contacted or the policy server - returned an unknown recommendation, this returns an OK recommendation. - This type fixing behaviour is done because the typical caller will be - in a critical call path and would generally interpret a `None` or similar - response as "weird value; don't care; move on without taking action". We - just frontload that logic here. - - - Args: - destination: The remote homeserver to ask (a policy server) - pdu: The event to check - timeout: How long to try (in ms) the destination for before - giving up. None indicates no timeout. - - Returns: - The policy recommendation, or RECOMMENDATION_OK if the policy server was - uncontactable or returned an unknown recommendation. - """ - - logger.debug( - "get_pdu_policy_recommendation for event_id=%s from %s", - pdu.event_id, - destination, - ) - - try: - res = await self.transport_layer.get_policy_recommendation_for_pdu( - destination, pdu, timeout=timeout - ) - recommendation = res.get("recommendation") - if not isinstance(recommendation, str): - raise InvalidResponseError("recommendation is not a string") - if recommendation not in (RECOMMENDATION_OK, RECOMMENDATION_SPAM): - logger.warning( - "get_pdu_policy_recommendation: unknown recommendation: %s", - recommendation, - ) - return RECOMMENDATION_OK - return recommendation - except Exception as e: - logger.warning( - "get_pdu_policy_recommendation: server %s responded with error, assuming OK recommendation: %s", - destination, - e, - ) - return RECOMMENDATION_OK - @trace @tag_args async def ask_policy_server_to_sign_event( self, destination: str, pdu: EventBase, timeout: int | None = None - ) -> JsonDict | None: + ) -> JsonDict: """Requests that the destination server (typically a policy server) sign the event as not spam. If the policy server could not be contacted or the policy server - returned an error, this returns no signature. + returned an error, that error is raised. Args: destination: The remote homeserver to ask (a policy server) @@ -519,17 +462,9 @@ class FederationClient(FederationBase): pdu.event_id, destination, ) - try: - return await self.transport_layer.ask_policy_server_to_sign_event( - destination, pdu, timeout=timeout - ) - except Exception as e: - logger.warning( - "ask_policy_server_to_sign_event: server %s responded with error: %s", - destination, - e, - ) - return None + return await self.transport_layer.ask_policy_server_to_sign_event( + destination, pdu, timeout=timeout + ) @trace @tag_args diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py index 35d3c30c69..5d5212ef96 100644 --- a/synapse/federation/transport/client.py +++ b/synapse/federation/transport/client.py @@ -47,6 +47,7 @@ from synapse.api.urls import ( ) from synapse.events import EventBase, make_event_from_dict from synapse.federation.units import Transaction +from synapse.http.client import is_unknown_endpoint from synapse.http.matrixfederationclient import ByteParser, LegacyJsonSendParser from synapse.http.types import QueryParams from synapse.types import JsonDict, UserID @@ -141,33 +142,6 @@ class TransportLayerClient: destination, path=path, timeout=timeout, try_trailing_slash_on_400=True ) - async def get_policy_recommendation_for_pdu( - self, destination: str, event: EventBase, timeout: int | None = None - ) -> JsonDict: - """Requests the policy recommendation for the given pdu from the given policy server. - - Args: - destination: The host name of the remote homeserver checking the event. - event: The event to check. - timeout: How long to try (in ms) the destination for before giving up. - None indicates no timeout. - - Returns: - The full recommendation object from the remote server. - """ - logger.debug( - "get_policy_recommendation_for_pdu dest=%s, event_id=%s", - destination, - event.event_id, - ) - return await self.client.post_json( - destination=destination, - path=f"/_matrix/policy/unstable/org.matrix.msc4284/event/{event.event_id}/check", - data=event.get_pdu_json(), - ignore_backoff=True, - timeout=timeout, - ) - async def ask_policy_server_to_sign_event( self, destination: str, event: EventBase, timeout: int | None = None ) -> JsonDict: @@ -186,13 +160,28 @@ class TransportLayerClient: The signature from the policy server, structured in the same was as the 'signatures' JSON in the event e.g { "$policy_server_via_domain" : { "ed25519:policy_server": "signature_base64" }} """ - return await self.client.post_json( - destination=destination, - path="/_matrix/policy/unstable/org.matrix.msc4284/sign", - data=event.get_pdu_json(), - ignore_backoff=True, - timeout=timeout, - ) + # Try stable first, then fall back to unstable if unsupported. All other errors + # are just errors. + try: + return await self.client.post_json( + destination=destination, + path="/_matrix/policy/v1/sign", + data=event.get_pdu_json(), + ignore_backoff=True, + timeout=timeout, + ) + except HttpResponseException as ex: + if is_unknown_endpoint(ex): + # TODO: Remove unstable MSC4284 support + # https://github.com/element-hq/synapse/issues/19502 + return await self.client.post_json( + destination=destination, + path="/_matrix/policy/unstable/org.matrix.msc4284/sign", + data=event.get_pdu_json(), + ignore_backoff=True, + timeout=timeout, + ) + raise async def backfill( self, destination: str, room_id: str, event_tuples: Collection[str], limit: int diff --git a/synapse/handlers/room_policy.py b/synapse/handlers/room_policy.py index 0663a36714..57163c4344 100644 --- a/synapse/handlers/room_policy.py +++ b/synapse/handlers/room_policy.py @@ -17,13 +17,14 @@ import logging from typing import TYPE_CHECKING +import attr from signedjson.key import decode_verify_key_bytes from unpaddedbase64 import decode_base64 -from synapse.api.errors import SynapseError +from synapse.api.constants import EventTypes +from synapse.api.errors import Codes, HttpResponseException, SynapseError from synapse.crypto.keyring import VerifyJsonRequest from synapse.events import EventBase -from synapse.types.handlers.policy_server import RECOMMENDATION_OK from synapse.util.stringutils import parse_and_validate_server_name if TYPE_CHECKING: @@ -31,10 +32,18 @@ if TYPE_CHECKING: logger = logging.getLogger(__name__) -POLICY_SERVER_EVENT_TYPE = "org.matrix.msc4284.policy" POLICY_SERVER_KEY_ID = "ed25519:policy_server" +@attr.s(slots=True, auto_attribs=True) +class PolicyServerInfo: + # name of the server. + server_name: str + + # the unpadded base64-encoded Ed25519 public key of the server. + public_key: str + + class RoomPolicyHandler: def __init__(self, hs: "HomeServer"): self._hs = hs @@ -43,6 +52,86 @@ class RoomPolicyHandler: self._event_auth_handler = hs.get_event_auth_handler() self._federation_client = hs.get_federation_client() + def _is_policy_server_state_event(self, event: EventBase) -> bool: + state_key = event.get_state_key() + if state_key is not None and state_key == "": + # TODO: Remove unstable MSC4284 support + # https://github.com/element-hq/synapse/issues/19502 + # Note: we can probably drop this whole function when we remove unstable support + return event.type in [EventTypes.RoomPolicy, "org.matrix.msc4284.policy"] + return False + + async def _get_policy_server(self, room_id: str) -> PolicyServerInfo | None: + """Get the policy server's name and Ed25519 public key for the room, if set. + + If the `m.room.policy` state event is invalid, then a policy server is not set. It + can be invalid if: + - The room doesn't have an `m.room.policy` state event with empty state key. + - The policy state event is missing the `via` or `public_keys` field. + - The policy state event's public keys is missing an `ed25519` key. + - The via server is not a valid server name. + - The via server is not in the room. + - The via server is Synapse itself. + + TODO: Remove unstable MSC4284 support - https://github.com/element-hq/synapse/issues/19502 + This function also checks for the unstable `org.matrix.msc4284.policy` state event. + + Args: + room_id: The room ID to get the policy server for. + + Returns: + A tuple of policy server name and its Ed25519 public key (unpadded base64). + Both values will be None if no policy server is configured or the configration + is invalid. + """ + policy_event = await self._storage_controllers.state.get_current_state_event( + room_id, EventTypes.RoomPolicy, "" + ) + public_key = None + if not policy_event: + # TODO: Remove unstable MSC4284 support + # https://github.com/element-hq/synapse/issues/19502 + policy_event = ( + await self._storage_controllers.state.get_current_state_event( + room_id, "org.matrix.msc4284.policy", "" + ) + ) + if not policy_event: + return None # neither stable or unstable configured + + # Unstable configured, grab its public key + public_key = policy_event.content.get("public_key", None) + else: + # Stable configured, grab its public key + public_keys = policy_event.content.get("public_keys") + if isinstance(public_keys, dict): + ed25519_key = public_keys.get("ed25519") + if isinstance(ed25519_key, str): + public_key = ed25519_key + + if public_key is None or not isinstance(public_key, str): + return None # no public key means no policy server + + policy_server = policy_event.content.get("via", "") + if policy_server is None or not isinstance(policy_server, str): + return None # no policy server + + if policy_server == self._hs.hostname: + return None # Synapse itself can't be a policy server (currently) + + try: + parse_and_validate_server_name(policy_server) + except ValueError: + return None # invalid policy server + + is_in_room = await self._event_auth_handler.is_host_in_room( + room_id, policy_server + ) + if not is_in_room: + return None # policy server not in room + + return PolicyServerInfo(policy_server, public_key) + async def is_event_allowed(self, event: EventBase) -> bool: """Check if the given event is allowed in the room by the policy server. @@ -62,58 +151,32 @@ class RoomPolicyHandler: Returns: bool: True if the event is allowed in the room, False otherwise. """ - if event.type == POLICY_SERVER_EVENT_TYPE and event.state_key is not None: + if self._is_policy_server_state_event(event): return True # always allow policy server change events - policy_event = await self._storage_controllers.state.get_current_state_event( - event.room_id, POLICY_SERVER_EVENT_TYPE, "" + policy_server = await self._get_policy_server(event.room_id) + if policy_server is None: + return True # no policy server configured, so allow + + # Check if the event has been signed with the public key in the policy server + # state event. If it is, the event is valid according to the policy server and + # we don't need to request a fresh signature. + valid = await self._verify_policy_server_signature( + event, policy_server.server_name, policy_server.public_key ) - if not policy_event: - return True # no policy server == default allow - - policy_server = policy_event.content.get("via", "") - if policy_server is None or not isinstance(policy_server, str): - return True # no policy server == default allow - - if policy_server == self._hs.hostname: - return True # Synapse itself can't be a policy server (currently) + if valid: + return True # valid signature == allow + # We couldn't save the HTTP hit, so do that hit. try: - parse_and_validate_server_name(policy_server) - except ValueError: - return True # invalid policy server == default allow - - is_in_room = await self._event_auth_handler.is_host_in_room( - event.room_id, policy_server - ) - if not is_in_room: - return True # policy server not in room == default allow - - # Check if the event has been signed with the public key in the policy server state event. - # If it is, we can save an HTTP hit. - # We actually want to get the policy server state event BEFORE THE EVENT rather than - # the current state value, else changing the public key will cause all of these checks to fail. - # However, if we are checking outlier events (which we will due to is_event_allowed being called - # near the edges at _check_sigs_and_hash) we won't know the state before the event, so the - # only safe option is to use the current state - public_key = policy_event.content.get("public_key", None) - if public_key is not None and isinstance(public_key, str): - valid = await self._verify_policy_server_signature( - event, policy_server, public_key - ) - if valid: - return True - # fallthrough to hit /check manually - - # At this point, the server appears valid and is in the room, so ask it to check - # the event. - recommendation = await self._federation_client.get_pdu_policy_recommendation( - policy_server, event - ) - if recommendation != RECOMMENDATION_OK: + await self.ask_policy_server_to_sign_event(event, verify=True) + except Exception as ex: + # We probably caught either a refusal to sign, an invalid signature, or + # some other transient or network error. These are all rejection cases. + logger.warning("Failed to get a signature from the policy server: %s", ex) return False - return True # default allow + return True # passed all verifications and checks, so allow async def _verify_policy_server_signature( self, event: EventBase, policy_server: str, public_key: str @@ -140,47 +203,71 @@ class RoomPolicyHandler: """Ask the policy server to sign this event. The signature is added to the event signatures block. Does nothing if there is no policy server state event in the room. If the policy server - refuses to sign the event (as it's marked as spam) does nothing. + refuses to sign the event (as it's marked as spam), an error is raised. Args: - event: The event to sign - verify: If True, verify that the signature is correctly signed by the public_key in the - policy server state event. + event: The event to sign. + verify: If True, verify that the signature is correctly signed by the policy server's + defined public key. Raises: - if verify=True and the policy server signed the event with an invalid signature. Does - not raise if the policy server refuses to sign the event. + When the policy server refuses to sign the event, or when verify is True and the + signature is invalid. """ - policy_event = await self._storage_controllers.state.get_current_state_event( - event.room_id, POLICY_SERVER_EVENT_TYPE, "" - ) - if not policy_event: + if self._is_policy_server_state_event(event): + # per spec, policy servers aren't asked to sign `m.room.policy` state events + # with empty state keys return - policy_server = policy_event.content.get("via", None) - if policy_server is None or not isinstance(policy_server, str): - return - # Only ask to sign events if the policy state event has a public_key (so they can be subsequently verified) - public_key = policy_event.content.get("public_key", None) - if public_key is None or not isinstance(public_key, str): + + policy_server = await self._get_policy_server(event.room_id) + if policy_server is None: return # Ask the policy server to sign this event. # We set a smallish timeout here as we don't want to block event sending too long. - signature = await self._federation_client.ask_policy_server_to_sign_event( - policy_server, - event, - timeout=3000, - ) - if ( - # the policy server returns {} if it refuses to sign the event. - signature and len(signature) > 0 - ): - event.signatures.update(signature) - if verify: - is_valid = await self._verify_policy_server_signature( - event, policy_server, public_key + try: + signature = await self._federation_client.ask_policy_server_to_sign_event( + policy_server.server_name, + event, + timeout=3000, + ) + # TODO: We can *probably* remove this when we remove unstable MSC4284 support. + # The server *should* be returning either a signature or an error, but there could + # also be implementation bugs. Whoever reads this when removing unstable MSC4284 + # stuff, make a decision on whether to remove this bit. + # https://github.com/element-hq/synapse/issues/19502 + if not signature or len(signature) == 0: + raise SynapseError( + 403, + "This event has been rejected as probable spam by the policy server", + Codes.FORBIDDEN, + ) + + # Note: if the policy server and event sender are the same server, the sender + # might not have added policy server signatures to the event for whatever reason. + # When this happens, we don't want to obliterate the event's existing signatures + # because the event will fail authorization. This is why we add defaults rather + # than simply `update` the signatures on the event. + # + # This situation can happen if the homeserver and policy server parts are + # logically the same server, but run by different software. For example, Synapse + # will not ask "itself" for a policy server signature, even if its server name + # is the designated policy server, so it could send an event outwards that other + # servers need to manually fetch signatures for. This is the code that allows + # those events to continue working (because they're legally sent, even if missing + # the policy server signature). + event.signatures.setdefault(policy_server.server_name, {}).update( + signature.get(policy_server.server_name, {}) + ) + except HttpResponseException as ex: + # re-wrap HTTP errors as `SynapseError` so they can be proxied to clients directly + raise ex.to_synapse_error() from ex + + if verify: + is_valid = await self._verify_policy_server_signature( + event, policy_server.server_name, policy_server.public_key + ) + if not is_valid: + raise SynapseError( + 500, + f"policy server {policy_server.server_name} failed to sign event correctly", ) - if not is_valid: - raise SynapseError( - 500, - f"policy server {policy_server} failed to sign event correctly", - ) diff --git a/synapse/types/handlers/policy_server.py b/synapse/types/handlers/policy_server.py deleted file mode 100644 index bfc09dabf4..0000000000 --- a/synapse/types/handlers/policy_server.py +++ /dev/null @@ -1,16 +0,0 @@ -# -# This file is licensed under the Affero General Public License (AGPL) version 3. -# -# Copyright (C) 2025 New Vector, Ltd -# -# This program is free software: you can redistribute it and/or modify -# it under the terms of the GNU Affero General Public License as -# published by the Free Software Foundation, either version 3 of the -# License, or (at your option) any later version. -# -# See the GNU Affero General Public License for more details: -# . -# - -RECOMMENDATION_OK = "ok" -RECOMMENDATION_SPAM = "spam" diff --git a/tests/handlers/test_room_policy.py b/tests/handlers/test_room_policy.py index ff212ab06e..9adf0e38c2 100644 --- a/tests/handlers/test_room_policy.py +++ b/tests/handlers/test_room_policy.py @@ -19,7 +19,8 @@ from signedjson.key import encode_verify_key_base64, get_verify_key from twisted.internet.testing import MemoryReactor -from synapse.api.errors import SynapseError +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, make_event_from_dict from synapse.handlers.room_policy import POLICY_SERVER_KEY_ID @@ -27,7 +28,6 @@ from synapse.rest import admin from synapse.rest.client import filter, login, room, sync from synapse.server import HomeServer from synapse.types import JsonDict, UserID -from synapse.types.handlers.policy_server import RECOMMENDATION_OK, RECOMMENDATION_SPAM from synapse.util.clock import Clock from tests import unittest @@ -49,13 +49,9 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): # mock out the federation transport client self.mock_federation_transport_client = mock.Mock( spec=[ - "get_policy_recommendation_for_pdu", "ask_policy_server_to_sign_event", ] ) - self.mock_federation_transport_client.get_policy_recommendation_for_pdu = ( - mock.AsyncMock() - ) self.mock_federation_transport_client.ask_policy_server_to_sign_event = ( mock.AsyncMock() ) @@ -106,25 +102,6 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): }, ) - # Prepare the policy server mock to decide spam vs not spam on those events - self.call_count = 0 - - async def get_policy_recommendation_for_pdu( - destination: str, - pdu: EventBase, - timeout: int | None = None, - ) -> JsonDict: - self.call_count += 1 - self.assertEqual(destination, self.OTHER_SERVER_NAME) - if pdu.event_id == self.spammy_event.event_id: - return {"recommendation": RECOMMENDATION_SPAM} - elif pdu.event_id == self.not_spammy_event.event_id: - return {"recommendation": RECOMMENDATION_OK} - else: - self.fail("Unexpected event ID") - - self.mock_federation_transport_client.get_policy_recommendation_for_pdu.side_effect = get_policy_recommendation_for_pdu - # Mock policy server actions on signing events async def policy_server_signs_event( destination: str, pdu: EventBase, timeout: int | None = None @@ -157,7 +134,9 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): async def policy_server_event_sign_error( destination: str, pdu: EventBase, timeout: int | None = None ) -> JsonDict | None: - return None + raise HttpResponseException( + 500, "Internal Server Error", b'{"errcode": "M_UNKNOWN"}' + ) self.policy_server_signs_event = policy_server_signs_event self.policy_server_refuses_to_sign_event = policy_server_refuses_to_sign_event @@ -174,14 +153,16 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): self.hs, self.room_id, policy_user_id, "join" ) ) - content = { + content: JsonDict = { "via": self.OTHER_SERVER_NAME, } if public_key is not None: - content["public_key"] = public_key + content["public_keys"] = { + "ed25519": public_key, + } self.helper.send_state( self.room_id, - "org.matrix.msc4284.policy", + EventTypes.RoomPolicy, content, tok=self.creator_token, state_key="", @@ -192,12 +173,11 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): # case where a room doesn't use a policy server. ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, True) - self.assertEqual(self.call_count, 0) def test_empty_policy_event_set(self) -> None: self.helper.send_state( self.room_id, - "org.matrix.msc4284.policy", + EventTypes.RoomPolicy, { # empty content (no `via`) }, @@ -207,12 +187,11 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, True) - self.assertEqual(self.call_count, 0) def test_nonstring_policy_event_set(self) -> None: self.helper.send_state( self.room_id, - "org.matrix.msc4284.policy", + EventTypes.RoomPolicy, { "via": 42, # should be a server name }, @@ -222,12 +201,11 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, True) - self.assertEqual(self.call_count, 0) def test_self_policy_event_set(self) -> None: self.helper.send_state( self.room_id, - "org.matrix.msc4284.policy", + EventTypes.RoomPolicy, { # We ignore events when the policy server is ourselves (for now?) "via": (UserID.from_string(self.creator)).domain, @@ -238,12 +216,11 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, True) - self.assertEqual(self.call_count, 0) def test_invalid_server_policy_event_set(self) -> None: self.helper.send_state( self.room_id, - "org.matrix.msc4284.policy", + EventTypes.RoomPolicy, { "via": "|this| is *not* a (valid) server name.com", }, @@ -253,12 +230,11 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, True) - self.assertEqual(self.call_count, 0) def test_not_in_room_policy_event_set(self) -> None: self.helper.send_state( self.room_id, - "org.matrix.msc4284.policy", + EventTypes.RoomPolicy, { "via": f"x.{self.OTHER_SERVER_NAME}", }, @@ -268,14 +244,30 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, True) - self.assertEqual(self.call_count, 0) + + def test_missing_public_key_event_set(self) -> None: + """ + Tests that a missing public key in the `m.room.policy` state event (an invalid + configuration) is treated as though there is no policy server configured, thus + allowing all events. + """ + self._add_policy_server_to_room() # no public_key + + ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) + self.assertEqual(ok, True) def test_spammy_event_is_spam(self) -> None: - self._add_policy_server_to_room() + verify_key_str = encode_verify_key_base64(get_verify_key(self.signing_key)) + self._add_policy_server_to_room(public_key=verify_key_str) + + # Explicitly configure the policy server mock to refuse to sign the event. + self.mock_federation_transport_client.ask_policy_server_to_sign_event.return_value = False ok = self.get_success(self.handler.is_event_allowed(self.spammy_event)) self.assertEqual(ok, False) - self.assertEqual(self.call_count, 1) + + # Ensure we actually contacted the policy server once for this event. + self.mock_federation_transport_client.ask_policy_server_to_sign_event.assert_awaited_once() def test_signed_event_is_not_spam(self) -> None: verify_key_str = encode_verify_key_base64(get_verify_key(self.signing_key)) @@ -306,8 +298,6 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): ok = self.get_success(self.handler.is_event_allowed(event)) self.assertEqual(ok, True) - # Make sure we did not make an HTTP hit to get_policy_recommendation_for_pdu - self.assertEqual(self.call_count, 0) def test_ask_policy_server_to_sign_event_ok(self) -> None: verify_key_str = encode_verify_key_base64(get_verify_key(self.signing_key)) @@ -348,8 +338,15 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): }, ) self.mock_federation_transport_client.ask_policy_server_to_sign_event.side_effect = self.policy_server_refuses_to_sign_event - self.get_success( - self.handler.ask_policy_server_to_sign_event(event, verify=True) + fail = self.get_failure( + self.handler.ask_policy_server_to_sign_event(event, verify=True), + SynapseError, + ) + self.assertIsInstance(fail.value, SynapseError) + self.assertEqual(fail.value.code, 403) + self.assertEqual( + fail.value.msg, + "This event has been rejected as probable spam by the policy server", ) self.assertEqual(len(event.signatures), 0) @@ -370,9 +367,12 @@ class RoomPolicyTestCase(unittest.FederatingHomeserverTestCase): }, ) self.mock_federation_transport_client.ask_policy_server_to_sign_event.side_effect = self.policy_server_event_sign_error - self.get_success( - self.handler.ask_policy_server_to_sign_event(event, verify=True) + fail = self.get_failure( + self.handler.ask_policy_server_to_sign_event(event, verify=True), + SynapseError, ) + self.assertIsInstance(fail.value, SynapseError) + self.assertEqual(fail.value.code, 500) self.assertEqual(len(event.signatures), 0) def test_ask_policy_server_to_sign_event_wrong_sig(self) -> None: