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

Do checks on aliases for incoming m.room.aliases events #5128

Merged
merged 14 commits into from
May 8, 2019
1 change: 1 addition & 0 deletions changelog.d/5128.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Add some missing limitations to room alias creation.
3 changes: 3 additions & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,9 @@
# the "depth" field on events is limited to 2**63 - 1
MAX_DEPTH = 2**63 - 1

# the maximum length for a room alias is 255 characters
MAX_ALIAS_LENGTH = 255


class Membership(object):

Expand Down
8 changes: 6 additions & 2 deletions synapse/events/snapshot.py
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,9 @@ def get_current_state_ids(self, store):

Returns:
Deferred[dict[(str, str), str]|None]: Returns None if state_group
is None, which happens when the associated event is an outlier.
is None, which happens when the associated event is an outlier.
Maps a (type, state_key) to the event ID of the state event matching
this tuple.
"""

if not self._fetching_state_deferred:
Expand All @@ -205,7 +207,9 @@ def get_prev_state_ids(self, store):

Returns:
Deferred[dict[(str, str), str]|None]: Returns None if state_group
is None, which happens when the associated event is an outlier.
is None, which happens when the associated event is an outlier.
Maps a (type, state_key) to the event ID of the state event matching
this tuple.
"""

if not self._fetching_state_deferred:
Expand Down
15 changes: 13 additions & 2 deletions synapse/events/validator.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,8 @@

from six import string_types

from synapse.api.constants import EventTypes, Membership
from synapse.api.errors import SynapseError
from synapse.api.constants import MAX_ALIAS_LENGTH, EventTypes, Membership
from synapse.api.errors import Codes, SynapseError
from synapse.api.room_versions import EventFormatVersions
from synapse.types import EventID, RoomID, UserID

Expand Down Expand Up @@ -56,6 +56,17 @@ def validate_new(self, event):
if not isinstance(getattr(event, s), string_types):
raise SynapseError(400, "'%s' not a string type" % (s,))

if event.type == EventTypes.Aliases:
if "aliases" in event.content:
for alias in event.content["aliases"]:
if len(alias) > MAX_ALIAS_LENGTH:
raise SynapseError(
400,
("Can't create aliases longer than"
" %d characters" % (MAX_ALIAS_LENGTH,)),
Codes.INVALID_PARAM,
)

def validate_builder(self, event):
"""Validates that the builder/event has roughly the right format. Only
checks values that we expect a proto event to have, rather than all the
Expand Down
7 changes: 3 additions & 4 deletions synapse/handlers/directory.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@

from twisted.internet import defer

from synapse.api.constants import EventTypes
from synapse.api.constants import MAX_ALIAS_LENGTH, EventTypes
from synapse.api.errors import (
AuthError,
CodeMessageException,
Expand All @@ -36,7 +36,6 @@


class DirectoryHandler(BaseHandler):
MAX_ALIAS_LENGTH = 255

def __init__(self, hs):
super(DirectoryHandler, self).__init__(hs)
Expand Down Expand Up @@ -105,10 +104,10 @@ def create_association(self, requester, room_alias, room_id, servers=None,

user_id = requester.user.to_string()

if len(room_alias.to_string()) > self.MAX_ALIAS_LENGTH:
if len(room_alias.to_string()) > MAX_ALIAS_LENGTH:
raise SynapseError(
400,
"Can't create aliases longer than %s characters" % self.MAX_ALIAS_LENGTH,
"Can't create aliases longer than %s characters" % MAX_ALIAS_LENGTH,
Codes.INVALID_PARAM,
)

Expand Down
30 changes: 30 additions & 0 deletions synapse/handlers/message.py
Original file line number Diff line number Diff line change
Expand Up @@ -228,6 +228,7 @@ def __init__(self, hs):
self.ratelimiter = hs.get_ratelimiter()
self.notifier = hs.get_notifier()
self.config = hs.config
self.require_membership_for_aliases = hs.config.require_membership_for_aliases

self.send_event_to_master = ReplicationSendEventRestServlet.make_client(hs)

Expand Down Expand Up @@ -336,6 +337,35 @@ def create_event(self, requester, event_dict, token_id=None, txn_id=None,
prev_events_and_hashes=prev_events_and_hashes,
)

# In an ideal world we wouldn't need the second part of this condition. However,
# this behaviour isn't spec'd yet, meaning we should be able to deactivate this
# behaviour. Another reason is that this code is also evaluated each time a new
# m.room.aliases event is created, which includes hitting a /directory route.
# Therefore not including this condition here would render the similar one in
# synapse.handlers.directory pointless.
if builder.type == EventTypes.Aliases and self.require_membership_for_aliases:
# Ideally we'd do the membership check in event_auth.check(), which
# describes a spec'd algorithm for authenticating events received over
# federation as well as those created locally. As of room v3, aliases events
# can be created by users that are not in the room, therefore we have to
# tolerate them in event_auth.check().
prev_state_ids = yield context.get_prev_state_ids(self.store)
prev_event_id = prev_state_ids.get((EventTypes.Member, event.sender))
prev_event = yield self.store.get_event(prev_event_id, allow_none=True)
if not prev_event or prev_event.membership != Membership.JOIN:
logger.warning(
("Attempt to send `m.room.aliases` in room %s by user %s but"
" membership is %s"),
event.room_id,
event.sender,
prev_event.membership if prev_event else None,
)

raise AuthError(
babolivier marked this conversation as resolved.
Show resolved Hide resolved
403,
"You must be in the room to create an alias for it",
)

self.validator.validate_new(event)

defer.returnValue((event, context))
Expand Down
169 changes: 169 additions & 0 deletions tests/rest/client/v1/test_directory.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,169 @@
# -*- coding: utf-8 -*-
# Copyright 2019 New Vector Ltd
#
# 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 json

from synapse.rest.admin import register_servlets
from synapse.rest.client.v1 import directory, login, room
from synapse.types import RoomAlias
from synapse.util.stringutils import random_string

from tests import unittest


class DirectoryTestCase(unittest.HomeserverTestCase):

servlets = [
register_servlets,
directory.register_servlets,
login.register_servlets,
room.register_servlets,
]

def make_homeserver(self, reactor, clock):
config = self.default_config()
config.require_membership_for_aliases = True

self.hs = self.setup_test_homeserver(config=config)

return self.hs

def prepare(self, reactor, clock, homeserver):
self.room_owner = self.register_user("room_owner", "test")
self.room_owner_tok = self.login("room_owner", "test")

self.room_id = self.helper.create_room_as(
self.room_owner, tok=self.room_owner_tok,
)

self.user = self.register_user("user", "test")
self.user_tok = self.login("user", "test")

def test_state_event_not_in_room(self):
self.ensure_user_left_room()
self.set_alias_via_state_event(403)

def test_directory_endpoint_not_in_room(self):
self.ensure_user_left_room()
self.set_alias_via_directory(403)

def test_state_event_in_room_too_long(self):
self.ensure_user_joined_room()
self.set_alias_via_state_event(400, alias_length=256)

def test_directory_in_room_too_long(self):
self.ensure_user_joined_room()
self.set_alias_via_directory(400, alias_length=256)

def test_state_event_in_room(self):
self.ensure_user_joined_room()
self.set_alias_via_state_event(200)

def test_directory_in_room(self):
self.ensure_user_joined_room()
self.set_alias_via_directory(200)

def test_room_creation_too_long(self):
url = "/_matrix/client/r0/createRoom"

# We use deliberately a localpart under the length threshold so
# that we can make sure that the check is done on the whole alias.
data = {
"room_alias_name": random_string(256 - len(self.hs.hostname)),
}
request_data = json.dumps(data)
request, channel = self.make_request(
"POST", url, request_data, access_token=self.user_tok,
)
self.render(request)
self.assertEqual(channel.code, 400, channel.result)

def test_room_creation(self):
url = "/_matrix/client/r0/createRoom"

# Check with an alias of allowed length. There should already be
# a test that ensures it works in test_register.py, but let's be
# as cautious as possible here.
data = {
"room_alias_name": random_string(5),
}
request_data = json.dumps(data)
request, channel = self.make_request(
"POST", url, request_data, access_token=self.user_tok,
)
self.render(request)
self.assertEqual(channel.code, 200, channel.result)

def set_alias_via_state_event(self, expected_code, alias_length=5):
url = ("/_matrix/client/r0/rooms/%s/state/m.room.aliases/%s"
% (self.room_id, self.hs.hostname))

data = {
"aliases": [
self.random_alias(alias_length),
],
}
request_data = json.dumps(data)

request, channel = self.make_request(
"PUT", url, request_data, access_token=self.user_tok,
)
self.render(request)
self.assertEqual(channel.code, expected_code, channel.result)

def set_alias_via_directory(self, expected_code, alias_length=5):
url = "/_matrix/client/r0/directory/room/%s" % self.random_alias(alias_length)
data = {
"room_id": self.room_id,
}
request_data = json.dumps(data)

request, channel = self.make_request(
"PUT", url, request_data, access_token=self.user_tok,
)
self.render(request)
self.assertEqual(channel.code, expected_code, channel.result)

def random_alias(self, length):
return RoomAlias(
random_string(length),
self.hs.hostname,
).to_string()

def ensure_user_left_room(self):
self.ensure_membership("leave")

def ensure_user_joined_room(self):
self.ensure_membership("join")

def ensure_membership(self, membership):
try:
if membership == "leave":
self.helper.leave(
room=self.room_id,
user=self.user,
tok=self.user_tok,
)
if membership == "join":
self.helper.join(
room=self.room_id,
user=self.user,
tok=self.user_tok,
)
except AssertionError:
# We don't care whether the leave request didn't return a 200 (e.g.
# if the user isn't already in the room), because we only want to
# make sure the user isn't in the room.
pass