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

<!-- Please read
https://element-hq.github.io/synapse/latest/development/contributing_guide.html
before submitting your pull request -->

* [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 <madlittlemods@gmail.com>

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.

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.

changelog.d/19503.feature

@@ -0,0 +1 @@
+Add stable support for [MSC4284](https://github.com/matrix-org/matrix-spec-proposals/pull/4284) Policy Servers.

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"

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

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

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",
-                    )

synapse/types/handlers/policy_server.py

@@ -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:
-# <https://www.gnu.org/licenses/agpl-3.0.html>.
-#
-
-RECOMMENDATION_OK = "ok"
-RECOMMENDATION_SPAM = "spam"

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: