Strip unauthorized fields from unsigned object in events received over federation

changelog.d/11530.misc

@@ -0,0 +1 @@
+Add functionality to strip the unsignedData object of unauthorized fields in events arriving over federation.

synapse/handlers/federation_event.py

@@ -52,7 +52,7 @@
     check_auth_rules_for_event,
     validate_event_for_room_version,
 )
-from synapse.events import EventBase
+from synapse.events import EventBase, make_event_from_dict
 from synapse.events.snapshot import EventContext
 from synapse.federation.federation_client import InvalidResponseError
 from synapse.logging.context import nested_logging_context, run_in_background
@@ -181,6 +181,11 @@ async def on_receive_pdu(self, origin: str, pdu: EventBase) -> None:
             logger.warning("Received event failed sanity checks")
             raise FederationError("ERROR", err.code, err.msg, affected=pdu.event_id)
 
+        # Strip any unauthorized unsigned values if they exist
+        event_dict = pdu.get_dict()
+        if pdu.unsigned and event_dict["unsigned"] != {}:
+            pdu = self.strip_unsigned_values(pdu, event_dict)
+
         # If we are currently in the process of joining this room, then we
         # queue up events for later processing.
         if room_id in self.room_queues:
@@ -325,6 +330,11 @@ async def on_send_membership_event(
 
         assert not event.internal_metadata.outlier
 
+        # Strip any unauthorized unsigned values if they exist
+        event_dict = event.get_dict()
+        if event.unsigned and event_dict["unsigned"] != {}:
+            event = self.strip_unsigned_values(event, event_dict)
+
         # Send this event on behalf of the other server.
         #
         # The remote server isn't a full participant in the room at this point, so
@@ -1939,3 +1949,33 @@ def _sanity_check_event(self, ev: EventBase) -> None:
                 len(ev.auth_event_ids()),
             )
             raise SynapseError(HTTPStatus.BAD_REQUEST, "Too many auth_events")
+
+    def strip_unsigned_values(self, pdu: EventBase, event_dict: Dict) -> EventBase:
+        """
+        Strip any unsigned values unless specifically allowed, as defined by the whitelist.
+
+        pdu: the event to strip values from
+        event_dict: a dict of values derived from the pdu
+        """
+        unsigned = event_dict.get("unsigned")
+
+        if pdu.type == EventTypes.Member:
+            whitelist = ["knock_room_state", "invite_room_state", "age"]
+        else:
+            whitelist = ["age"]
+
+        filtered_unsigned = {}
+
+        # Unsigned should never be None by the time we are here but mypy doesn't know that
+        assert unsigned is not None
+        for k, v in unsigned.items():
+            if k in whitelist:
+                filtered_unsigned[k] = v
+
+        event_dict["unsigned"] = filtered_unsigned
+
+        stripped_event = make_event_from_dict(
+            event_dict, pdu.room_version, pdu.internal_metadata.get_dict()
+        )
+
+        return stripped_event

tests/test_federation.py

@@ -276,3 +276,111 @@ def test_cross_signing_keys_retry(self):
             "ed25519:" + remote_self_signing_key in self_signing_key["keys"].keys(),
         )
         self.assertTrue(remote_self_signing_key in self_signing_key["keys"].values())
+
+
+class StripUnsignedFromEventsTestCase(MessageAcceptTests):
+    def setUp(self):
+        super().setUp()
+
+    def strip_unauthorized_unsigned_values(self):
+        most_recent = self.get_success(
+            self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
+        )[0]
+        federation_event_handler = self.homeserver.get_federation_event_handler()
+        event1 = make_event_from_dict(
+            {
+                "room_id": self.room_id,
+                "sender": "@baduser:test.serv",
+                "state_key": "@baduser:test.serv",
+                "event_id": "$event1:test.serv",
+                "depth": 1000,
+                "origin_server_ts": 1,
+                "type": "m.room.member",
+                "origin": "test.servx",
+                "content": {"membership": "join"},
+                "auth_events": [],
+                "prev_state": [(most_recent, {})],
+                "prev_events": [(most_recent, {})],
+                "unsigned": {"malicious garbage": "hackz", "more warez": "more hackz"},
+            }
+        )
+        self.get_success(federation_event_handler.on_receive_pdu("test.serv", event1))
+
+        event = self.get_success(self.store.get_event("$event1:test.serv"))
+        event_dict = event.get_dict()
+        # Make sure unauthorized fields are stripped from unsigned
+        self.assertNotIn("hackz", event_dict["unsigned"])
+
+    def strip_event_maintains_allowed_fields(self):
+        most_recent = self.get_success(
+            self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
+        )[0]
+        federation_event_handler = self.homeserver.get_federation_event_handler()
+        event2 = make_event_from_dict(
+            {
+                "room_id": self.room_id,
+                "sender": "@baduser:test.serv",
+                "state_key": "@baduser:test.serv",
+                "event_id": "$event2:test.serv",
+                "depth": 1000,
+                "origin_server_ts": 1,
+                "type": "m.room.member",
+                "origin": "test.servx",
+                "auth_events": [],
+                "prev_state": [(most_recent, {})],
+                "prev_events": [(most_recent, {})],
+                "content": {"membership": "join"},
+                "unsigned": {
+                    "malicious garbage": "hackz",
+                    "more warez": "more hackz",
+                    "age": 14,
+                    "invite_room_state": [],
+                },
+            }
+        )
+        self.get_success(
+            federation_event_handler.on_send_membership_event("test.serv", event2)
+        )
+
+        event = self.get_success(self.store.get_event("$event2:test.serv"))
+        event_dict = event.get_dict()
+        self.assertIn("age", event_dict["unsigned"])
+        self.assertNotIn("hackz", event_dict["unsigned"])
+        # Invite_room_state is allowed in events of type m.room.member
+        self.assertIn("invite_room_state", event_dict["unsigned"])
+
+    def strip_event_removes_fields_based_on_event_type(self):
+        most_recent = self.get_success(
+            self.homeserver.get_datastore().get_latest_event_ids_in_room(self.room_id)
+        )[0]
+        federation_event_handler = self.homeserver.get_federation_event_handler()
+        event3 = make_event_from_dict(
+            {
+                "room_id": self.room_id,
+                "sender": "@baduser:test.serv",
+                "state_key": "@baduser:test.serv",
+                "event_id": "$event3:test.serv",
+                "depth": 1000,
+                "origin_server_ts": 1,
+                "type": "m.room.power_levels",
+                "origin": "test.servx",
+                "content": {},
+                "auth_events": [],
+                "prev_state": [(most_recent, {})],
+                "prev_events": [(most_recent, {})],
+                "unsigned": {
+                    "malicious garbage": "hackz",
+                    "more warez": "more hackz",
+                    "age": 14,
+                    "invite_room_state": [],
+                },
+            }
+        )
+        self.get_success(federation_event_handler.on_receive_pdu("test.serv", event3))
+
+        event = self.get_success(self.store.get_event("$event3:test.serv"))
+        event_dict = event.get_dict()
+        self.assertIn("age", event_dict["unsigned"])
+        # Invite_room_state field is only associated with event type m.room.member
+        self.assertNotIn("invite_room_state", event_dict["unsigned"])
+        self.assertNotIn("more warez", event_dict["unsigned"])