Stable support for MSC4284 policy servers

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