-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Update the MSC3083 support to verify if joins are from an authorized server #10254
Changes from 45 commits
6a0dd93
cb8aaed
f9bfc19
fd37e76
59de557
2a074d3
d2fdc1b
441a9bb
aab6ae3
111bbcf
6d7e981
80ce8f8
1a8f171
5fbc307
fda81ad
13cfdd7
0da003c
2c6a34c
6b00541
09599a2
f90db62
6997b6a
c71f2d6
83d95a0
9cddd4b
789fdc1
6cf7890
ded8caa
110fb19
84d21d6
858fb10
bca8e73
d8eb84e
9f497a0
fbe0038
b3a4b65
8b2cac2
05e35ce
a588b7b
381cc8e
c82c0ce
0437602
3549b5e
9970af8
5aa985d
8c82dcf
4cb62e8
ba070ad
549ca5b
af2c6a5
bc2677b
6bc22bb
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Update support for [MSC3083](https://github.com/matrix-org/matrix-doc/pull/3083) to consider changes in the MSC around which servers can issue join events. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -106,6 +106,18 @@ def check( | |
if not event.signatures.get(event_id_domain): | ||
raise AuthError(403, "Event not signed by sending server") | ||
|
||
is_invite_via_allow_rule = ( | ||
event.type == EventTypes.Member | ||
and event.membership == Membership.JOIN | ||
and "join_authorised_via_users_server" in event.content | ||
) | ||
if is_invite_via_allow_rule: | ||
authoriser_domain = get_domain_from_id( | ||
event.content["join_authorised_via_users_server"] | ||
) | ||
if not event.signatures.get(authoriser_domain): | ||
raise AuthError(403, "Event not signed by authorising server") | ||
|
||
# Implementation of https://matrix.org/docs/spec/rooms/v1#authorization-rules | ||
# | ||
# 1. If type is m.room.create: | ||
|
@@ -285,7 +297,7 @@ def _is_membership_change_allowed( | |
user_level = get_user_power_level(event.user_id, auth_events) | ||
target_level = get_user_power_level(target_user_id, auth_events) | ||
|
||
# FIXME (erikj): What should we do here as the default? | ||
invite_level = _get_named_level(auth_events, "invite", 0) | ||
ban_level = _get_named_level(auth_events, "ban", 50) | ||
|
||
logger.debug( | ||
|
@@ -336,25 +348,48 @@ def _is_membership_change_allowed( | |
elif target_in_room: # the target is already in the room. | ||
raise AuthError(403, "%s is already in the room." % target_user_id) | ||
else: | ||
invite_level = _get_named_level(auth_events, "invite", 0) | ||
|
||
if user_level < invite_level: | ||
raise AuthError(403, "You don't have permission to invite users") | ||
elif Membership.JOIN == membership: | ||
# Joins are valid iff caller == target and: | ||
# * They are not banned. | ||
# * They are accepting a previously sent invitation. | ||
# * They are already joined (it's a NOOP). | ||
# * The room is public or restricted. | ||
# * The room is public. | ||
# * The room is restricted and the user meets the allows rules. | ||
if event.user_id != target_user_id: | ||
raise AuthError(403, "Cannot force another user to join.") | ||
elif target_banned: | ||
raise AuthError(403, "You are banned from this room") | ||
elif join_rule == JoinRules.PUBLIC or ( | ||
elif join_rule == JoinRules.PUBLIC: | ||
pass | ||
elif ( | ||
room_version.msc3083_join_rules | ||
and join_rule == JoinRules.MSC3083_RESTRICTED | ||
): | ||
pass | ||
# This is the same as public, but the event must contain a reference | ||
# to the server who authorised the join. If the event does not contain | ||
# the proper content it is rejected. | ||
# | ||
# Note that if the caller is in the room or invited, then they do | ||
# not need to meet the allow rules. | ||
if not caller_in_room and not caller_invited: | ||
authorising_user = event.content.get("join_authorised_via_users_server") | ||
|
||
if authorising_user is None: | ||
raise AuthError(403, "Join event is missing authorising user.") | ||
|
||
# The authorising user must be in the room. | ||
key = (EventTypes.Member, authorising_user) | ||
member_event = auth_events.get(key) | ||
_check_joined_room(member_event, authorising_user, event.room_id) | ||
|
||
authorising_user_level = get_user_power_level( | ||
authorising_user, auth_events | ||
) | ||
if authorising_user_level < invite_level: | ||
raise AuthError(403, "Join event authorised by invalid server.") | ||
|
||
elif join_rule == JoinRules.INVITE or ( | ||
room_version.msc2403_knocking and join_rule == JoinRules.KNOCK | ||
): | ||
|
@@ -640,6 +675,66 @@ def get_user_power_level(user_id: str, auth_events: StateMap[EventBase]) -> int: | |
return 0 | ||
|
||
|
||
def get_users_which_can_issue_invite(auth_events: StateMap[EventBase]) -> List[str]: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a bit worried that this is returning a list of potential servers? It feels like a recipe for different implementations to do signature checks on different servers and thus potentially come to different conclusions (though it seems fairly unlikely). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is now only used to find a list of candidate servers to attempt a remote join to. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd be tempted to lift these two functions out of here, as this file is otherwise stuff purely related to the auth rules check from the spec? |
||
""" | ||
Return the list of users which can issue invites. | ||
|
||
This is done by exploring the joined users and comparing their power levels | ||
to the necessyar power level to issue an invite. | ||
|
||
Args: | ||
auth_events: state in force at this point in the room | ||
|
||
Returns: | ||
The users which can issue invites. | ||
""" | ||
invite_level = _get_named_level(auth_events, "invite", 0) | ||
users_default_level = _get_named_level(auth_events, "users_default", 0) | ||
power_level_event = _get_power_level_event(auth_events) | ||
|
||
# Custom power-levels for users. | ||
if power_level_event: | ||
users = power_level_event.content.get("users", {}) | ||
else: | ||
users = {} | ||
|
||
result = [] | ||
|
||
# Check which members are able to invite by ensuring they're joined and have | ||
# the necessary power level. | ||
for (event_type, state_key), event in auth_events.items(): | ||
if event_type != EventTypes.Member: | ||
continue | ||
|
||
if event.membership != Membership.JOIN: | ||
continue | ||
|
||
# Check if the user has a custom power level. | ||
if users.get(state_key, users_default_level) >= invite_level: | ||
result.append(state_key) | ||
|
||
return result | ||
|
||
|
||
def get_servers_from_users(users: List[str]) -> Set[str]: | ||
""" | ||
Resolve a list of users into their servers. | ||
|
||
Args: | ||
users: A list of users. | ||
|
||
Returns: | ||
A set of servers. | ||
""" | ||
servers = set() | ||
for user in users: | ||
try: | ||
servers.add(get_domain_from_id(user)) | ||
except SynapseError: | ||
pass | ||
return servers | ||
|
||
|
||
def _get_named_level(auth_events: StateMap[EventBase], name: str, default: int) -> int: | ||
power_level_event = _get_power_level_event(auth_events) | ||
|
||
|
@@ -760,4 +855,13 @@ def auth_types_for_event(event: Union[EventBase, EventBuilder]) -> Set[Tuple[str | |
) | ||
auth_types.add(key) | ||
|
||
# TODO Should this be limited to only MSC3083 rooms. | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
if membership == Membership.JOIN: | ||
if "join_authorised_via_users_server" in event.content: | ||
key = ( | ||
EventTypes.Member, | ||
event.content["join_authorised_via_users_server"], | ||
) | ||
auth_types.add(key) | ||
|
||
return auth_types |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm pretty sure we need to do this here and in
_check_sigs_on_pdu
(since that checks the signature is valid while this just checks that a signature exists). Anyway I matched what the 3pid code did.