Stable support for MSC4284 policy servers (#19503)

Fixes #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
#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>

Author: 3 people

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.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 @@ async def _record_failure_callback(
 
         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 @@ async def ask_policy_server_to_sign_event(
             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.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 @@ async def get_event(
             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 @@ async def ask_policy_server_to_sign_event(
             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