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

Implement MSC2730: verifiable forwarded events #8078

Closed
wants to merge 10 commits into from
1 change: 1 addition & 0 deletions changelog.d/8078.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Implement [MSC2730](https://github.com/matrix-org/matrix-doc/pull/2730): Verifiable forwarded events.
49 changes: 48 additions & 1 deletion synapse/handlers/federation.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
from synapse.events import EventBase
from synapse.events.snapshot import EventContext
from synapse.events.validator import EventValidator
from synapse.federation.federation_base import FederationBase, event_from_pdu_json
from synapse.handlers._base import BaseHandler
from synapse.logging.context import (
make_deferred_yieldable,
Expand Down Expand Up @@ -108,7 +109,7 @@ class _NewEventInfo:
auth_events = attr.ib(type=Optional[MutableStateMap[EventBase]], default=None)


class FederationHandler(BaseHandler):
class FederationHandler(BaseHandler, FederationBase):
Copy link
Member

Choose a reason for hiding this comment

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

ftr, FederationBase is something I wish would go away. Those utility functions need to be brought in by composition, not inheritance.

"""Handles events that originated from federation.
Responsible for:
a) handling received Pdus before handing them on as Events to the rest
Expand Down Expand Up @@ -683,6 +684,41 @@ async def _get_events_from_store_or_dest(

return fetched_events

_forwarded_key = "net.maunium.msc2730.forwarded"
Copy link
Member

Choose a reason for hiding this comment

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

constants go in UPPER_CASE at the top of the file.


async def _validate_forwarded_event(
self, event: EventBase
) -> Tuple[bool, Optional[str]]:
Copy link
Member

Choose a reason for hiding this comment

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

why not just return the event id if it's valid, and None if it's not?

Copy link
Member

Choose a reason for hiding this comment

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

(alternatively: what does it mean if valid is False, but an event id is returned? Some docstring would help here).

try:
source_evt_dict = {**event.content[self._forwarded_key]}
room_version_identifier = source_evt_dict["unsigned"]["room_version"]
source_evt_dict["type"] = event.type
source_evt_dict["content"] = {**event.content}
del source_evt_dict["unsigned"]
del source_evt_dict["content"][self._forwarded_key]
except (KeyError, TypeError):
return False, None

room_version = KNOWN_ROOM_VERSIONS.get(room_version_identifier)
if not room_version:
# We don't support the source room version, so we can't verify the event :(
return False, None

try:
source_evt = event_from_pdu_json(source_evt_dict, room_version)
except SynapseError:
return False, None
Comment on lines +709 to +710
Copy link
Member

Choose a reason for hiding this comment

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

as a general rule, eating exceptions like this without giving any clue about what the exception was leads to hard-to-debug failures. I'd recommend logging something before returning.


try:
checked_evt = await self._check_sigs_and_hash(room_version, source_evt)
Copy link
Member

Choose a reason for hiding this comment

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

I wonder if there is some horrible attack where you can claim that an event is for a different room version than it is, and hence get it to pass the hash checks when it shouldn't...

# _check_sigs_and_hash returns a redacted event if hash validation failed and
# a SynapseError if signature validation failed. In both those cases, we want to
# mark the forward as invalid.
valid = not checked_evt.internal_metadata.is_redacted()
except SynapseError:
valid = False
return valid, source_evt.event_id

async def _process_received_pdu(
self, origin: str, event: EventBase, state: Optional[Iterable[EventBase]],
):
Expand All @@ -703,6 +739,17 @@ async def _process_received_pdu(

logger.debug("[%s %s] Processing event: %s", room_id, event_id, event)

if (
not event.is_state()
and not event.redacts
and self._forwarded_key in event.content
):
valid, forwarded_event_id = await self._validate_forwarded_event(event)
event.unsigned[self._forwarded_key] = {
"valid": valid,
"event_id": forwarded_event_id,
}

try:
await self._handle_new_event(origin, event, state=state)
except AuthError as e:
Expand Down
2 changes: 2 additions & 0 deletions synapse/rest/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@
capabilities,
devices,
filter,
forward_event,
groups,
keys,
notifications,
Expand Down Expand Up @@ -109,6 +110,7 @@ def register_servlets(client_resource, hs):
tags.register_servlets(hs, client_resource)
account_data.register_servlets(hs, client_resource)
report_event.register_servlets(hs, client_resource)
forward_event.register_servlets(hs, client_resource)
openid.register_servlets(hs, client_resource)
notifications.register_servlets(hs, client_resource)
devices.register_servlets(hs, client_resource)
Expand Down
134 changes: 134 additions & 0 deletions synapse/rest/client/v2_alpha/forward_event.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
# -*- coding: utf-8 -*-
# Copyright 2020 Tulir Asokan <[email protected]>
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.


import logging

from synapse.api.errors import AuthError, Codes, SynapseError
from synapse.http.servlet import RestServlet, parse_integer
from synapse.logging.opentracing import set_tag

from ._base import client_patterns

logger = logging.getLogger(__name__)


class RoomEventForwardServlet(RestServlet):
"""
PUT /net.maunium.msc2730/rooms/{room_id}/event/{event_id}/forward/{target_room_id}/{txn_id}
"""

PATTERNS = client_patterns(
(
"/net.maunium.msc2730/rooms/(?P<room_id>[^/]*)/event/(?P<event_id>[^/]*)"
"/forward/(?P<target_room_id>[^/]*)/(?P<txn_id>.*)"
),
releases=(), # This is an unstable feature
)

_data_key = "net.maunium.msc2730.forwarded"
_err_not_forwardable = "NET.MAUNIUM.MSC2730_NOT_FORWARDABLE"

def __init__(self, hs):
super().__init__()
self.event_creation_handler = hs.get_event_creation_handler()
self.event_handler = hs.get_event_handler()
self.store = hs.get_datastore()
self.auth = hs.get_auth()

async def on_PUT(self, request, room_id, event_id, target_room_id, txn_id):
requester = await self.auth.get_user_by_req(request, allow_guest=True)

try:
event = await self.event_handler.get_event(
requester.user, room_id, event_id
)
except AuthError:
event = None
if not event:
raise SynapseError(404, "Event not found.", errcode=Codes.NOT_FOUND)

if event.is_state():
raise SynapseError(
401,
"State events cannot be forwarded.",
errcode=self._err_not_forwardable,
)
elif event.redacts:
raise SynapseError(
401,
"Redaction events cannot be forwarded.",
errcode=self._err_not_forwardable,
)
elif event.internal_metadata.is_redacted():
raise SynapseError(
401,
"Redacted events cannot be forwarded.",
errcode=self._err_not_forwardable,
)

event_id = event.event_id
event_dict = event.get_dict()

content = event_dict.pop("content")
unsigned = event_dict.pop("unsigned", {})
event_type = event_dict.pop("type")
has_forward_meta = self._data_key in content
try:
is_valid_forward = has_forward_meta and unsigned[self._data_key]["valid"]
except (KeyError, TypeError):
is_valid_forward = False

if has_forward_meta:
if not is_valid_forward:
raise SynapseError(
401,
"Event contains invalid forward metadata.",
errcode=self._err_not_forwardable,
)
# Pass through the old event ID to the new unsigned data
event_id = unsigned[self._data_key]["event_id"]
elif not has_forward_meta:
content[self._data_key] = event_dict
Copy link
Member

Choose a reason for hiding this comment

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

careful: I think this modifies the original event, stored in the cache. You need to copy content before modifying it.

room_version = await self.store.get_room_version(event.room_id)
content[self._data_key]["unsigned"] = {
"room_version": room_version.identifier,
# TODO add sender profile info here
}

forward_event_dict = {
"type": event_type,
"content": content,
"room_id": target_room_id,
"sender": requester.user.to_string(),
"unsigned": {self._data_key: {"valid": True, "event_id": event_id}},
}

if b"ts" in request.args and requester.app_service:
forward_event_dict["origin_server_ts"] = parse_integer(request, "ts", 0)

(
forwarded_event,
_,
) = await self.event_creation_handler.create_and_send_nonmember_event(
requester, forward_event_dict, txn_id=txn_id
)

set_tag("event_id", forwarded_event.event_id)
return 200, {"event_id": forwarded_event.event_id}


def register_servlets(hs, http_server):
RoomEventForwardServlet(hs).register(http_server)
2 changes: 2 additions & 0 deletions synapse/rest/client/versions.py
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,8 @@ def on_GET(self, request):
"org.matrix.e2e_cross_signing": True,
# Implements additional endpoints as described in MSC2432
"org.matrix.msc2432": True,
# Implements endpoint and forwarded event validation as described in MSC2730
"net.maunium.msc2730": True,
# Implements additional endpoints as described in MSC2666
"uk.half-shot.msc2666": True,
# Whether new rooms will be set to encrypted or not (based on presets).
Expand Down