Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Implement rechecking of redactions for room versions v3 #4499

Merged
merged 15 commits into from
Jan 29, 2019
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/4499.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add support for room version 3
4 changes: 2 additions & 2 deletions synapse/api/auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand All @@ -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):
Expand Down
3 changes: 1 addition & 2 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -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"


Expand All @@ -116,7 +116,6 @@ class RoomVersions(object):
KNOWN_ROOM_VERSIONS = {
RoomVersions.V1,
RoomVersions.V2,
RoomVersions.VDH_TEST,
RoomVersions.STATE_V2_TEST,
}

Expand Down
24 changes: 18 additions & 6 deletions synapse/event_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -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)

Expand Down Expand Up @@ -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:
Expand All @@ -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,):
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,
Expand Down
15 changes: 15 additions & 0 deletions synapse/events/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,21 @@ def get_send_on_behalf_of(self):
"""
return getattr(self, "send_on_behalf_of", None)

def need_to_check_redaction(self):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can has docstring please - what does this mean?

"""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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"is allowed to redact any event"

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.
Expand Down
2 changes: 0 additions & 2 deletions synapse/events/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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(
Expand Down
6 changes: 5 additions & 1 deletion synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion synapse/state/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
42 changes: 39 additions & 3 deletions synapse/storage/events_worker.py
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
)

Expand All @@ -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
Expand Down Expand Up @@ -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.
Expand Down Expand Up @@ -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 is actually redacted
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
redacted_event = None

cache_entry = _EventCacheEntry(
event=original_ev,
redacted_event=redacted_event,
Expand Down