From 7a3ec5b022185e805d82c9b1e441597ef9d65425 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 29 Jan 2019 11:55:18 +0000 Subject: [PATCH 01/13] Add RoomVersions.V3 constant, without enabling it We add the constant, but don't add it to the known room versions. This lets us start adding V3 logic, but the servers will never join or create V3 rooms --- synapse/api/constants.py | 1 + 1 file changed, 1 insertion(+) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 51ee078bc372..2c3b1f8c591c 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -104,6 +104,7 @@ class ThirdPartyEntityKind(object): class RoomVersions(object): V1 = "1" V2 = "2" + V3 = "3" # Not currently fully supported, so VDH_TEST = "vdh-test-version" STATE_V2_TEST = "state-v2-test" From 7709d2bd167e27493b134e938410c307f8c10396 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 28 Jan 2019 21:09:45 +0000 Subject: [PATCH 02/13] Implement rechecking of redactions --- synapse/api/auth.py | 4 ++-- synapse/event_auth.py | 24 ++++++++++++++++++------ synapse/events/__init__.py | 3 +++ synapse/handlers/message.py | 6 +++++- synapse/storage/events_worker.py | 26 +++++++++++++++++++++++++- 5 files changed, 53 insertions(+), 10 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 7b213e54c890..963e0e7d6035 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -627,7 +627,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: @@ -640,7 +640,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): diff --git a/synapse/event_auth.py b/synapse/event_auth.py index 9adedbbb0294..a95d142f0c09 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -20,7 +20,13 @@ from signedjson.sign import SignatureVerifyException, verify_signed_json from unpaddedbase64 import decode_base64 -from synapse.api.constants import KNOWN_ROOM_VERSIONS, EventTypes, JoinRules, Membership +from synapse.api.constants import ( + KNOWN_ROOM_VERSIONS, + EventTypes, + JoinRules, + Membership, + RoomVersions, +) from synapse.api.errors import AuthError, EventSizeError, SynapseError from synapse.types import UserID, get_domain_from_id @@ -168,7 +174,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) @@ -422,7 +428,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: @@ -442,10 +448,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, RoomVersions.VDH_TEST): + 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, diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index 3fe52aaa453c..70d3c0fbd933 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -63,6 +63,9 @@ def get_send_on_behalf_of(self): """ return getattr(self, "send_on_behalf_of", None) + def need_to_check_redaction(self): + return getattr(self, "recheck_redaction", False) + def _event_dict_property(key): # We want to be able to use hasattr with the event dict properties. diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index 05d1370c1851..0cfced43d58e 100644 --- a/synapse/handlers/message.py +++ b/synapse/handlers/message.py @@ -767,7 +767,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, @@ -781,6 +782,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: diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index 0a0ca58fc446..9ce19430e8bb 100644 --- a/synapse/storage/events_worker.py +++ b/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, @@ -174,6 +175,29 @@ def _get_events(self, event_ids, check_redacted=True, if not entry: continue + # Some redactions in room version v3 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.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 From 7d1024d5746ab8be82997ae6117aa51255af67ce Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 29 Jan 2019 11:58:11 +0000 Subject: [PATCH 03/13] Newsfile --- changelog.d/4499.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/4499.feature diff --git a/changelog.d/4499.feature b/changelog.d/4499.feature new file mode 100644 index 000000000000..9538c64f088d --- /dev/null +++ b/changelog.d/4499.feature @@ -0,0 +1 @@ +Add support for room version 3 From b82a76c3845be27f91c53b6770d8ccbad675ca4f Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 29 Jan 2019 13:50:59 +0000 Subject: [PATCH 04/13] Finish comment... --- synapse/api/constants.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 2c3b1f8c591c..042a640f0a3c 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -104,7 +104,7 @@ class ThirdPartyEntityKind(object): class RoomVersions(object): V1 = "1" V2 = "2" - V3 = "3" # Not currently fully supported, so + V3 = "3" # Not currently fully supported, so we don't add to known versions below VDH_TEST = "vdh-test-version" STATE_V2_TEST = "state-v2-test" From 82165eeb052ce69f078cdf3371127cc89c702424 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 29 Jan 2019 21:14:39 +0000 Subject: [PATCH 05/13] Update synapse/storage/events_worker.py Co-Authored-By: erikjohnston --- synapse/storage/events_worker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index 9ce19430e8bb..9dcb27b65de2 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -175,7 +175,7 @@ def _get_events(self, event_ids, check_redacted=True, if not entry: continue - # Some redactions in room version v3 need to be rechecked if we + # 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: From 38590a4870a2774c500e9ff2478b2d420221f1d0 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 29 Jan 2019 21:22:47 +0000 Subject: [PATCH 06/13] Add docstring --- synapse/events/__init__.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index 70d3c0fbd933..527aec8c694a 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -64,6 +64,18 @@ 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 due to auth + rules, then this will always return false. + + Returns: + bool + """ return getattr(self, "recheck_redaction", False) From 47e2dd1994c27f7d91b092954fcd9ed79ca9f822 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 29 Jan 2019 21:24:34 +0000 Subject: [PATCH 07/13] Drop vdh support --- synapse/api/constants.py | 2 -- synapse/event_auth.py | 2 +- synapse/events/builder.py | 2 -- synapse/state/__init__.py | 2 +- 4 files changed, 2 insertions(+), 6 deletions(-) diff --git a/synapse/api/constants.py b/synapse/api/constants.py index 042a640f0a3c..302e1e2f1f0c 100644 --- a/synapse/api/constants.py +++ b/synapse/api/constants.py @@ -105,7 +105,6 @@ class RoomVersions(object): V1 = "1" V2 = "2" V3 = "3" # Not currently fully supported, so we don't add to known versions below - VDH_TEST = "vdh-test-version" STATE_V2_TEST = "state-v2-test" @@ -117,7 +116,6 @@ class RoomVersions(object): KNOWN_ROOM_VERSIONS = { RoomVersions.V1, RoomVersions.V2, - RoomVersions.VDH_TEST, RoomVersions.STATE_V2_TEST, } diff --git a/synapse/event_auth.py b/synapse/event_auth.py index a95d142f0c09..df30c2cea7f5 100644 --- a/synapse/event_auth.py +++ b/synapse/event_auth.py @@ -448,7 +448,7 @@ def check_redaction(room_version, event, auth_events): if user_level >= redact_level: return False - if room_version in (RoomVersions.V1, RoomVersions.V2, RoomVersions.VDH_TEST): + 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: diff --git a/synapse/events/builder.py b/synapse/events/builder.py index 7e633710957c..9ca405b56bce 100644 --- a/synapse/events/builder.py +++ b/synapse/events/builder.py @@ -38,7 +38,6 @@ def get_event_builder(room_version, key_values={}, internal_metadata_dict={}): if room_version in { RoomVersions.V1, RoomVersions.V2, - RoomVersions.VDH_TEST, RoomVersions.STATE_V2_TEST, }: return EventBuilder(key_values, internal_metadata_dict) @@ -101,7 +100,6 @@ def new(self, room_version, key_values={}): if room_version not in { RoomVersions.V1, RoomVersions.V2, - RoomVersions.VDH_TEST, RoomVersions.STATE_V2_TEST, }: raise Exception( diff --git a/synapse/state/__init__.py b/synapse/state/__init__.py index 2fca51d0b2bf..125635b01a7f 100644 --- a/synapse/state/__init__.py +++ b/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, From b5d510ad64ad3ed4e40a61c389af875058a73258 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 29 Jan 2019 21:45:28 +0000 Subject: [PATCH 08/13] Remove unused arg --- synapse/storage/events_worker.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index 9dcb27b65de2..df8169b91e17 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -163,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, ) @@ -334,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. From 6d23ec21116d5312e26315c0c75f3761398daf00 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 29 Jan 2019 21:45:53 +0000 Subject: [PATCH 09/13] Fix typo --- synapse/storage/events_worker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index df8169b91e17..0ff80fa7285d 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -189,7 +189,7 @@ def _get_events(self, event_ids, check_redacted=True, if orig and get_domain_from_id(orig.sender) == expected_domain: # This redaction event is allowed. Mark as not needing a # recheck. - entry.event.recheck_redaction = False + 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 From 4db252c0738c590ed5fa0b53c61f9dd9da1cb33e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 29 Jan 2019 21:46:17 +0000 Subject: [PATCH 10/13] Check redaction state when event is pulled out of the database --- synapse/storage/events_worker.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index 0ff80fa7285d..1d75e309b3f7 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -466,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 is actually redacted + redacted_event = None + cache_entry = _EventCacheEntry( event=original_ev, redacted_event=redacted_event, From c21b7cbc0978080beb9837c3027b15a094670f91 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 29 Jan 2019 21:53:48 +0000 Subject: [PATCH 11/13] Update synapse/storage/events_worker.py --- synapse/storage/events_worker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/storage/events_worker.py b/synapse/storage/events_worker.py index 1d75e309b3f7..ebe1429acbcb 100644 --- a/synapse/storage/events_worker.py +++ b/synapse/storage/events_worker.py @@ -476,7 +476,7 @@ def _get_event_from_row(self, internal_metadata, js, redacted, # recheck. because.internal_metadata.recheck_redaction = False else: - # Senders don't match, so the event is actually redacted + # Senders don't match, so the event isn't actually redacted redacted_event = None cache_entry = _EventCacheEntry( From f46a818ce5a4ccc8977780d5cf5e20d107233bf9 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 29 Jan 2019 22:02:54 +0000 Subject: [PATCH 12/13] kill vdh test some more --- synapse/events/__init__.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index 44b2b41f1874..d380686e992f 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -343,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: From afeea319df0ae84f74d506e4395a58fbe17b6480 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 29 Jan 2019 22:55:29 +0000 Subject: [PATCH 13/13] Fixup comment --- synapse/events/__init__.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/events/__init__.py b/synapse/events/__init__.py index d380686e992f..697cf58582e4 100644 --- a/synapse/events/__init__.py +++ b/synapse/events/__init__.py @@ -69,8 +69,8 @@ def need_to_check_redaction(self): 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 due to auth - rules, then this will always return false. + 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