Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve event validation for call invites (m.call.invite) #16908

Merged
merged 4 commits into from
Mar 19, 2024
Merged
Show file tree
Hide file tree
Changes from all 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/16908.misc
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Improve event validation (#16908).
2 changes: 2 additions & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -130,6 +130,8 @@ class EventTypes:

Reaction: Final = "m.reaction"

CallInvite: Final = "m.call.invite"


class ToDeviceEventTypes:
RoomKeyRequest: Final = "m.room_key_request"
Expand Down
13 changes: 13 additions & 0 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@
EventTypes,
GuestAccess,
HistoryVisibility,
JoinRules,
Membership,
RelationTypes,
UserTypes,
Expand Down Expand Up @@ -1325,6 +1326,18 @@ async def create_new_client_event(

self.validator.validate_new(event, self.config)
await self._validate_event_relation(event)

if event.type == EventTypes.CallInvite:
room_id = event.room_id
room_info = await self.store.get_room_with_stats(room_id)
assert room_info is not None

if room_info.join_rules == JoinRules.PUBLIC:
raise SynapseError(
403,
"Call invites are not allowed in public rooms.",
Codes.FORBIDDEN,
)
logger.debug("Created event %s", event.event_id)

return event, context
Expand Down
12 changes: 11 additions & 1 deletion synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
AccountDataTypes,
EventContentFields,
EventTypes,
JoinRules,
Membership,
)
from synapse.api.filtering import FilterCollection
Expand Down Expand Up @@ -675,13 +676,22 @@ async def _load_filtered_recents(
)
)

loaded_recents = await filter_events_for_client(
filtered_recents = await filter_events_for_client(
self._storage_controllers,
sync_config.user.to_string(),
loaded_recents,
always_include_ids=current_state_ids,
)

loaded_recents = []
for event in filtered_recents:
if event.type == EventTypes.CallInvite:
room_info = await self.store.get_room_with_stats(event.room_id)
assert room_info is not None
if room_info.join_rules == JoinRules.PUBLIC:
continue
loaded_recents.append(event)
Comment on lines +686 to +693
Copy link
Contributor

@MadLittleMods MadLittleMods Jun 24, 2024

Choose a reason for hiding this comment

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

Could we move this logic to filter_events_for_client(...) so it works everywhere?

I assume this restriction only really matters for /sync because those events are considered "live" and will ring on the client but in the case of a limited gappy sync in a busy room, a client paginating /messages to catch-up may consider a call invite depending on a max age to ring.

It also means the logic would be consolidated to be shared with Sliding Sync which is how I am stumbling upon this now.


Or can we soft_fail these events?

Copy link
Contributor

Choose a reason for hiding this comment

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

Tracking this with #17359


log_kv({"loaded_recents_after_client_filtering": len(loaded_recents)})

loaded_recents.extend(recents)
Expand Down
40 changes: 40 additions & 0 deletions tests/handlers/test_message.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
from twisted.test.proto_helpers import MemoryReactor

from synapse.api.constants import EventTypes
from synapse.api.errors import SynapseError
from synapse.events import EventBase
from synapse.events.snapshot import EventContext, UnpersistedEventContextBase
from synapse.rest import admin
Expand Down Expand Up @@ -51,11 +52,15 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
persistence = self.hs.get_storage_controllers().persistence
assert persistence is not None
self._persist_event_storage_controller = persistence
self.store = self.hs.get_datastores().main

self.user_id = self.register_user("tester", "foobar")
device_id = "dev-1"
access_token = self.login("tester", "foobar", device_id=device_id)
self.room_id = self.helper.create_room_as(self.user_id, tok=access_token)
self.private_room_id = self.helper.create_room_as(
self.user_id, tok=access_token, extra_content={"preset": "private_chat"}
)

self.requester = create_requester(self.user_id, device_id=device_id)

Expand Down Expand Up @@ -285,6 +290,41 @@ def test_when_empty_prev_events_allowed_reject_event_with_empty_prev_events_and_
AssertionError,
)

def test_call_invite_event_creation_fails_in_public_room(self) -> None:
# get prev_events for room
prev_events = self.get_success(
self.store.get_prev_events_for_room(self.room_id)
)

# the invite in a public room should fail
self.get_failure(
self.handler.create_event(
self.requester,
{
"type": EventTypes.CallInvite,
"room_id": self.room_id,
"sender": self.requester.user.to_string(),
},
prev_event_ids=prev_events,
auth_event_ids=prev_events,
),
SynapseError,
)

# but a call invite in a private room should succeed
self.get_success(
self.handler.create_event(
self.requester,
{
"type": EventTypes.CallInvite,
"room_id": self.private_room_id,
"sender": self.requester.user.to_string(),
},
prev_event_ids=prev_events,
auth_event_ids=prev_events,
)
)


class ServerAclValidationTestCase(unittest.HomeserverTestCase):
servlets = [
Expand Down
115 changes: 113 additions & 2 deletions tests/handlers/test_sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,18 @@
# [This file includes modifications made by New Vector Limited]
#
#
from typing import Optional
from typing import Collection, List, Optional
from unittest.mock import AsyncMock, Mock, patch

from twisted.test.proto_helpers import MemoryReactor

from synapse.api.constants import EventTypes, JoinRules
from synapse.api.errors import Codes, ResourceLimitError
from synapse.api.filtering import Filtering
from synapse.api.room_versions import RoomVersions
from synapse.api.room_versions import RoomVersion, RoomVersions
from synapse.events import EventBase
from synapse.events.snapshot import EventContext
from synapse.federation.federation_base import event_from_pdu_json
from synapse.handlers.sync import SyncConfig, SyncResult
from synapse.rest import admin
from synapse.rest.client import knock, login, room
Expand Down Expand Up @@ -285,6 +288,114 @@ def test_ban_wins_race_with_join(self) -> None:
)
self.assertEqual(eve_initial_sync_after_join.joined, [])

def test_call_invite_in_public_room_not_returned(self) -> None:
user = self.register_user("alice", "password")
tok = self.login(user, "password")
room_id = self.helper.create_room_as(user, is_public=True, tok=tok)
self.handler = self.hs.get_federation_handler()
federation_event_handler = self.hs.get_federation_event_handler()

async def _check_event_auth(
origin: Optional[str], event: EventBase, context: EventContext
) -> None:
pass

federation_event_handler._check_event_auth = _check_event_auth # type: ignore[method-assign]
self.client = self.hs.get_federation_client()

async def _check_sigs_and_hash_for_pulled_events_and_fetch(
dest: str, pdus: Collection[EventBase], room_version: RoomVersion
) -> List[EventBase]:
return list(pdus)

self.client._check_sigs_and_hash_for_pulled_events_and_fetch = _check_sigs_and_hash_for_pulled_events_and_fetch # type: ignore[assignment]

prev_events = self.get_success(self.store.get_prev_events_for_room(room_id))

# create a call invite event
call_event = event_from_pdu_json(
{
"type": EventTypes.CallInvite,
"content": {},
"room_id": room_id,
"sender": user,
"depth": 32,
"prev_events": prev_events,
"auth_events": prev_events,
"origin_server_ts": self.clock.time_msec(),
},
RoomVersions.V10,
)

self.assertEqual(
self.get_success(
federation_event_handler.on_receive_pdu("test.serv", call_event)
),
None,
)

# check that it is in DB
recent_event = self.get_success(self.store.get_prev_events_for_room(room_id))
self.assertIn(call_event.event_id, recent_event)

# but that it does not come down /sync in public room
sync_result: SyncResult = self.get_success(
self.sync_handler.wait_for_sync_for_user(
create_requester(user), generate_sync_config(user)
)
)
event_ids = []
for event in sync_result.joined[0].timeline.events:
event_ids.append(event.event_id)
self.assertNotIn(call_event.event_id, event_ids)

# it will come down in a private room, though
user2 = self.register_user("bob", "password")
tok2 = self.login(user2, "password")
private_room_id = self.helper.create_room_as(
user2, is_public=False, tok=tok2, extra_content={"preset": "private_chat"}
)

priv_prev_events = self.get_success(
self.store.get_prev_events_for_room(private_room_id)
)
private_call_event = event_from_pdu_json(
{
"type": EventTypes.CallInvite,
"content": {},
"room_id": private_room_id,
"sender": user,
"depth": 32,
"prev_events": priv_prev_events,
"auth_events": priv_prev_events,
"origin_server_ts": self.clock.time_msec(),
},
RoomVersions.V10,
)

self.assertEqual(
self.get_success(
federation_event_handler.on_receive_pdu("test.serv", private_call_event)
),
None,
)

recent_events = self.get_success(
self.store.get_prev_events_for_room(private_room_id)
)
self.assertIn(private_call_event.event_id, recent_events)

private_sync_result: SyncResult = self.get_success(
self.sync_handler.wait_for_sync_for_user(
create_requester(user2), generate_sync_config(user2)
)
)
priv_event_ids = []
for event in private_sync_result.joined[0].timeline.events:
priv_event_ids.append(event.event_id)

self.assertIn(private_call_event.event_id, priv_event_ids)


_request_key = 0

Expand Down
Loading