Merge pull request #4499 from matrix-org/erikj/redactions_eiah

Implement rechecking of redactions for room versions v3

changelog.d/4499.feature

@@ -0,0 +1 @@
+Add support for room version 3

synapse/api/auth.py

@@ -616,7 +616,7 @@ def compute_auth_events(self, event, current_state_ids, for_verification=False):
 
         defer.returnValue(auth_ids)
 
-    def check_redaction(self, event, auth_events):
+    def check_redaction(self, room_version, event, auth_events):
         """Check whether the event sender is allowed to redact the target event.
 
         Returns:
@@ -629,7 +629,7 @@ def check_redaction(self, event, auth_events):
             AuthError if the event sender is definitely not allowed to redact
             the target event.
         """
-        return event_auth.check_redaction(event, auth_events)
+        return event_auth.check_redaction(room_version, event, auth_events)
 
     @defer.inlineCallbacks
     def check_can_change_room_list(self, room_id, user):

synapse/api/constants.py

@@ -104,7 +104,7 @@ class ThirdPartyEntityKind(object):
 class RoomVersions(object):
     V1 = "1"
     V2 = "2"
-    VDH_TEST = "vdh-test-version"
+    V3 = "3"  # Not currently fully supported, so we don't add to known versions below
     STATE_V2_TEST = "state-v2-test"
 
 
@@ -116,7 +116,6 @@ class RoomVersions(object):
 KNOWN_ROOM_VERSIONS = {
     RoomVersions.V1,
     RoomVersions.V2,
-    RoomVersions.VDH_TEST,
     RoomVersions.STATE_V2_TEST,
 }
 

synapse/event_auth.py

@@ -26,6 +26,7 @@
     EventTypes,
     JoinRules,
     Membership,
+    RoomVersions,
 )
 from synapse.api.errors import AuthError, EventSizeError, SynapseError
 from synapse.types import UserID, get_domain_from_id
@@ -177,7 +178,7 @@ def check(room_version, event, auth_events, do_sig_check=True, do_size_check=Tru
         _check_power_levels(event, auth_events)
 
     if event.type == EventTypes.Redaction:
-        check_redaction(event, auth_events)
+        check_redaction(room_version, event, auth_events)
 
     logger.debug("Allowing! %s", event)
 
@@ -431,7 +432,7 @@ def _can_send_event(event, auth_events):
     return True
 
 
-def check_redaction(event, auth_events):
+def check_redaction(room_version, event, auth_events):
     """Check whether the event sender is allowed to redact the target event.
 
     Returns:
@@ -451,10 +452,16 @@ def check_redaction(event, auth_events):
     if user_level >= redact_level:
         return False
 
-    redacter_domain = get_domain_from_id(event.event_id)
-    redactee_domain = get_domain_from_id(event.redacts)
-    if redacter_domain == redactee_domain:
+    if room_version in (RoomVersions.V1, RoomVersions.V2,):
+        redacter_domain = get_domain_from_id(event.event_id)
+        redactee_domain = get_domain_from_id(event.redacts)
+        if redacter_domain == redactee_domain:
+            return True
+    elif room_version == RoomVersions.V3:
+        event.internal_metadata.recheck_redaction = True
         return True
+    else:
+        raise RuntimeError("Unrecognized room version %r" % (room_version,))
 
     raise AuthError(
         403,

synapse/events/__init__.py

@@ -62,6 +62,21 @@ def get_send_on_behalf_of(self):
         """
         return getattr(self, "send_on_behalf_of", None)
 
+    def need_to_check_redaction(self):
+        """Whether the redaction event needs to be rechecked when fetching
+        from the database.
+
+        Starting in room v3 redaction events are accepted up front, and later
+        checked to see if the redacter and redactee's domains match.
+
+        If the sender of the redaction event is allowed to redact any event
+        due to auth rules, then this will always return false.
+
+        Returns:
+            bool
+        """
+        return getattr(self, "recheck_redaction", False)
+
 
 def _event_dict_property(key):
     # We want to be able to use hasattr with the event dict properties.
@@ -328,8 +343,7 @@ def room_version_to_event_format(room_version):
         raise RuntimeError("Unrecognized room version %s" % (room_version,))
 
     if room_version in (
-        RoomVersions.V1, RoomVersions.V2, RoomVersions.VDH_TEST,
-        RoomVersions.STATE_V2_TEST,
+        RoomVersions.V1, RoomVersions.V2, RoomVersions.STATE_V2_TEST,
     ):
         return EventFormatVersions.V1
     else:

synapse/handlers/message.py

@@ -745,7 +745,8 @@ def is_inviter_member_event(e):
             auth_events = {
                 (e.type, e.state_key): e for e in auth_events.values()
             }
-            if self.auth.check_redaction(event, auth_events=auth_events):
+            room_version = yield self.store.get_room_version(event.room_id)
+            if self.auth.check_redaction(room_version, event, auth_events=auth_events):
                 original_event = yield self.store.get_event(
                     event.redacts,
                     check_redacted=False,
@@ -759,6 +760,9 @@ def is_inviter_member_event(e):
                         "You don't have permission to redact events"
                     )
 
+                # We've already checked.
+                event.internal_metadata.recheck_redaction = False
+
         if event.type == EventTypes.Create:
             prev_state_ids = yield context.get_prev_state_ids(self.store)
             if prev_state_ids:

synapse/state/__init__.py

@@ -608,7 +608,7 @@ def resolve_events_with_store(room_version, state_sets, event_map, state_res_sto
             state_sets, event_map, state_res_store.get_events,
         )
     elif room_version in (
-        RoomVersions.VDH_TEST, RoomVersions.STATE_V2_TEST, RoomVersions.V2,
+        RoomVersions.STATE_V2_TEST, RoomVersions.V2,
     ):
         return v2.resolve_events_with_store(
             room_version, state_sets, event_map, state_res_store,

synapse/storage/events_worker.py

@@ -21,13 +21,14 @@
 
 from twisted.internet import defer
 
-from synapse.api.constants import EventFormatVersions
+from synapse.api.constants import EventFormatVersions, EventTypes
 from synapse.api.errors import NotFoundError
 from synapse.events import FrozenEvent, event_type_from_format_version  # noqa: F401
 # these are only included to make the type annotations work
 from synapse.events.snapshot import EventContext  # noqa: F401
 from synapse.events.utils import prune_event
 from synapse.metrics.background_process_metrics import run_as_background_process
+from synapse.types import get_domain_from_id
 from synapse.util.logcontext import (
     LoggingContext,
     PreserveLoggingContext,
@@ -162,7 +163,6 @@ def _get_events(self, event_ids, check_redacted=True,
 
             missing_events = yield self._enqueue_events(
                 missing_events_ids,
-                check_redacted=check_redacted,
                 allow_rejected=allow_rejected,
             )
 
@@ -174,6 +174,29 @@ def _get_events(self, event_ids, check_redacted=True,
             if not entry:
                 continue
 
+            # Starting in room version v3, some redactions need to be rechecked if we
+            # didn't have the redacted event at the time, so we recheck on read
+            # instead.
+            if not allow_rejected and entry.event.type == EventTypes.Redaction:
+                if entry.event.internal_metadata.need_to_check_redaction():
+                    orig = yield self.get_event(
+                        entry.event.redacts,
+                        allow_none=True,
+                        allow_rejected=True,
+                        get_prev_content=False,
+                    )
+                    expected_domain = get_domain_from_id(entry.event.sender)
+                    if orig and get_domain_from_id(orig.sender) == expected_domain:
+                        # This redaction event is allowed. Mark as not needing a
+                        # recheck.
+                        entry.event.internal_metadata.recheck_redaction = False
+                    else:
+                        # We don't have the event that is being redacted, so we
+                        # assume that the event isn't authorized for now. (If we
+                        # later receive the event, then we will always redact
+                        # it anyway, since we have this redaction)
+                        continue
+
             if allow_rejected or not entry.event.rejected_reason:
                 if check_redacted and entry.redacted_event:
                     event = entry.redacted_event
@@ -310,7 +333,7 @@ def fire(evs, exc):
                     self.hs.get_reactor().callFromThread(fire, event_list, e)
 
     @defer.inlineCallbacks
-    def _enqueue_events(self, events, check_redacted=True, allow_rejected=False):
+    def _enqueue_events(self, events, allow_rejected=False):
         """Fetches events from the database using the _event_fetch_list. This
         allows batch and bulk fetching of events - it allows us to fetch events
         without having to create a new transaction for each request for events.
@@ -443,6 +466,19 @@ def _get_event_from_row(self, internal_metadata, js, redacted,
                     # will serialise this field correctly
                     redacted_event.unsigned["redacted_because"] = because
 
+                    # Starting in room version v3, some redactions need to be
+                    # rechecked if we didn't have the redacted event at the
+                    # time, so we recheck on read instead.
+                    if because.internal_metadata.need_to_check_redaction():
+                        expected_domain = get_domain_from_id(original_ev.sender)
+                        if get_domain_from_id(because.sender) == expected_domain:
+                            # This redaction event is allowed. Mark as not needing a
+                            # recheck.
+                            because.internal_metadata.recheck_redaction = False
+                        else:
+                            # Senders don't match, so the event isn't actually redacted
+                            redacted_event = None
+
             cache_entry = _EventCacheEntry(
                 event=original_ev,
                 redacted_event=redacted_event,