From 8636ec042b8eaaca96c2db722c0063051aca4567 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 3 Jul 2019 11:45:07 +0100 Subject: Implement restrictions for power levels --- synapse/third_party_rules/access_rules.py | 57 +++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index d13f8de888..f99ea3fc0b 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -80,6 +80,7 @@ class RoomAccessRules(object): """ is_direct = config.get("is_direct") rules_in_initial_state = False + rule = "" # If there's a rules event in the initial state, check if it complies with the # spec for im.vector.room.access_rules and deny the request if not. @@ -123,6 +124,22 @@ class RoomAccessRules(object): } }) + rule = default_rule + + # Check if the creator can override values for the power levels. + allowed = self._is_power_level_content_allowed( + config.get("power_level_content_override", {}), rule, + ) + if not allowed: + raise SynapseError(400, "Invalid power levels content override") + + # Second loop for events we need to know the current rule to process. + for event in config.get("initial_state", []): + if event["type"] == EventTypes.PowerLevels: + allowed = self._is_power_level_content_allowed(event["content"], rule) + if not allowed: + raise SynapseError(400, "Invalid power levels content") + @defer.inlineCallbacks def check_threepid_can_be_invited(self, medium, address, state_events): """Implements synapse.events.ThirdPartyEventRules.check_threepid_can_be_invited @@ -178,6 +195,11 @@ class RoomAccessRules(object): rule = self._get_rule_from_state(state_events) + # Special-case the power levels event because it makes more sense to check it + # here. + if event.type == EventTypes.PowerLevels: + return self._is_power_level_content_allowed(event.content, rule) + if rule == ACCESS_RULE_RESTRICTED: ret = self._apply_restricted(event) elif rule == ACCESS_RULE_UNRESTRICTED: @@ -319,6 +341,41 @@ class RoomAccessRules(object): return True + def _is_power_level_content_allowed(self, content, access_rule): + """Denies a power level events that sets 'users_default' to a non-0 value, and + sets the PL of a user that'd be blacklisted in restricted mode to a non-default + value. + + Args: + content (dict[]): The content of the m.room.power_levels event to check. + access_rule (str): The access rule in place in this room. + Returns: + bool, True if the event can be allowed, False otherwise. + + """ + # Blacklisted servers shouldn't have any restriction in "direct" mode, so always + # accept the event. + if access_rule == ACCESS_RULE_DIRECT: + return True + + # If users_default is explicitly set to a non-0 value, deny the event. + users_default = content.get('users_default', 0) + if users_default: + return False + + users = content.get('users', {}) + for user_id, power_level in users.items(): + server_name = get_domain_from_id(user_id) + # Check the domain against the blacklist. If found, and the PL isn't 0, deny + # the event. + if ( + server_name in self.domains_forbidden_when_restricted + and power_level != 0 + ): + return False + + return True + @staticmethod def _get_rule_from_state(state_events): """Extract the rule to be applied from the given set of state events. -- cgit 1.5.1 From 724ddaddb3f7aa58156b0ca0fbfbb6eec0021f78 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 3 Jul 2019 12:03:22 +0100 Subject: Refactor part of the access rules module Since we're not processing only membership events and 3PID invites anymore, it's nice to know which function is supposed to process what. --- synapse/third_party_rules/access_rules.py | 80 ++++++++++++++++++------------- 1 file changed, 47 insertions(+), 33 deletions(-) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index f99ea3fc0b..d2530645c4 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -92,18 +92,18 @@ class RoomAccessRules(object): # Make sure the event has a valid content. if rule is None: - raise SynapseError(400, "Invalid access rule",) + raise SynapseError(400, "Invalid access rule") # Make sure the rule name is valid. if rule not in VALID_ACCESS_RULES: - raise SynapseError(400, "Invalid access rule", ) + raise SynapseError(400, "Invalid access rule") # Make sure the rule is "direct" if the room is a direct chat. if ( (is_direct and rule != ACCESS_RULE_DIRECT) or (rule == ACCESS_RULE_DIRECT and not is_direct) ): - raise SynapseError(400, "Invalid access rule",) + raise SynapseError(400, "Invalid access rule") # If there's no rules event in the initial state, create one with the default # setting. @@ -189,29 +189,17 @@ class RoomAccessRules(object): Checks the event's type and the current rule and calls the right function to determine whether the event can be allowed. """ - # Special-case the access rules event. if event.type == ACCESS_RULES_TYPE: return self._on_rules_change(event, state_events) + # We need to know the rule to apply when processing the event types below. rule = self._get_rule_from_state(state_events) - # Special-case the power levels event because it makes more sense to check it - # here. if event.type == EventTypes.PowerLevels: return self._is_power_level_content_allowed(event.content, rule) - if rule == ACCESS_RULE_RESTRICTED: - ret = self._apply_restricted(event) - elif rule == ACCESS_RULE_UNRESTRICTED: - ret = self._apply_unrestricted() - elif rule == ACCESS_RULE_DIRECT: - ret = self._apply_direct(event, state_events) - else: - # We currently apply the default (restricted) if we don't know the rule, we - # might want to change that in the future. - ret = self._apply_restricted(event) - - return ret + if event.type == EventTypes.Member or event.type == EventTypes.ThirdPartyInvite: + return self._on_membership_or_invite(event, rule, state_events) def _on_rules_change(self, event, state_events): """Implement the checks and behaviour specified on allowing or forbidding a new @@ -254,35 +242,67 @@ class RoomAccessRules(object): return False - def _apply_restricted(self, event): + def _on_membership_or_invite(self, event, rule, state_events): + """Applies the correct rule for incoming m.room.member and + m.room.third_party_invite events. + + Args: + event (synapse.events.EventBase): The event to check. + rule (str): The name of the rule to apply. + state_events (dict[tuple[event type, state key], EventBase]): The state of the + room before the event was sent. + Returns: + bool, True if the event can be allowed, False otherwise. + """ + if rule == ACCESS_RULE_RESTRICTED: + ret = self._on_membership_or_invite_restricted(event) + elif rule == ACCESS_RULE_UNRESTRICTED: + ret = self._on_membership_or_invite_unrestricted() + elif rule == ACCESS_RULE_DIRECT: + ret = self._on_membership_or_invite_direct(event, state_events) + else: + # We currently apply the default (restricted) if we don't know the rule, we + # might want to change that in the future. + ret = self._on_membership_or_invite_restricted(event) + + return ret + + def _on_membership_or_invite_restricted(self, event): """Implements the checks and behaviour specified for the "restricted" rule. + "restricted" currently means that users can only invite users if their server is + included in a limited list of domains. + Args: event (synapse.events.EventBase): The event to check. Returns: bool, True if the event can be allowed, False otherwise. """ - # "restricted" currently means that users can only invite users if their server is - # included in a limited list of domains. - # We're not filtering on m.room.third_party_member events here because the - # filtering on threepids is done in check_threepid_can_be_invited. - if event.type != EventTypes.Member: + # We're not applying the rules on m.room.third_party_member events here because + # the filtering on threepids is done in check_threepid_can_be_invited, which is + # called before check_event_allowed. + if event.type == EventTypes.ThirdPartyInvite: return True invitee_domain = get_domain_from_id(event.state_key) return invitee_domain not in self.domains_forbidden_when_restricted - def _apply_unrestricted(self): + def _on_membership_or_invite_unrestricted(self): """Implements the checks and behaviour specified for the "unrestricted" rule. + "unrestricted" currently means that every event is allowed. + Returns: bool, True if the event can be allowed, False otherwise. """ - # "unrestricted" currently means that every event is allowed. return True - def _apply_direct(self, event, state_events): + def _on_membership_or_invite_direct(self, event, state_events): """Implements the checks and behaviour specified for the "direct" rule. + "direct" currently means that no member is allowed apart from the two initial + members the room was created for (i.e. the room's creator and their first + invitee). + Args: event (synapse.events.EventBase): The event to check. state_events (dict[tuple[event type, state key], EventBase]): The state of the @@ -290,12 +310,6 @@ class RoomAccessRules(object): Returns: bool, True if the event can be allowed, False otherwise. """ - # "direct" currently means that no member is allowed apart from the two initial - # members the room was created for (i.e. the room's creator and their first - # invitee). - if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: - return True - # Get the room memberships and 3PID invite tokens from the room's state. existing_members, threepid_tokens = self._get_members_and_tokens_from_state( state_events, -- cgit 1.5.1 From aa3ba4193338d7dc1fe3dbbe2743581e7d66cced Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 3 Jul 2019 12:05:56 +0100 Subject: Default return value for events we're not interested in --- synapse/third_party_rules/access_rules.py | 2 ++ 1 file changed, 2 insertions(+) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index d2530645c4..5307a15097 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -201,6 +201,8 @@ class RoomAccessRules(object): if event.type == EventTypes.Member or event.type == EventTypes.ThirdPartyInvite: return self._on_membership_or_invite(event, rule, state_events) + return True + def _on_rules_change(self, event, state_events): """Implement the checks and behaviour specified on allowing or forbidding a new im.vector.room.access_rules event. -- cgit 1.5.1 From d085e0df2a96e2b300ebfa428c95f3827d2c0455 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 3 Jul 2019 15:44:22 +0100 Subject: Change the rule for applying PL restrictions --- synapse/third_party_rules/access_rules.py | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 5307a15097..ba24c701a5 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -33,6 +33,11 @@ VALID_ACCESS_RULES = ( ACCESS_RULE_UNRESTRICTED, ) +# Rules to which we need to apply the power levels restrictions. +RULES_WITH_RESTRICTED_POWER_LEVELS = ( + ACCESS_RULE_UNRESTRICTED, +) + class RoomAccessRules(object): """Implementation of the ThirdPartyEventRules module API that allows federation admins @@ -369,9 +374,8 @@ class RoomAccessRules(object): bool, True if the event can be allowed, False otherwise. """ - # Blacklisted servers shouldn't have any restriction in "direct" mode, so always - # accept the event. - if access_rule == ACCESS_RULE_DIRECT: + # Check if we need to apply the restrictions with the current rule. + if access_rule not in RULES_WITH_RESTRICTED_POWER_LEVELS: return True # If users_default is explicitly set to a non-0 value, deny the event. -- cgit 1.5.1 From 6b83a1826ce1fc811ab98c70069c7c9734680544 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 3 Jul 2019 17:32:52 +0100 Subject: Incorporate review --- synapse/third_party_rules/access_rules.py | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index ba24c701a5..7c97e79c2b 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -34,6 +34,12 @@ VALID_ACCESS_RULES = ( ) # Rules to which we need to apply the power levels restrictions. +# These are all of the rules that don't forbid users to join based on a server blacklist +# (except for the rules targetting direct chats, because both users should be able to be +# room admins in a direct chat created with the "trusted_private_chat" preset). +# The current restrictions forbid users that would be forbidden from joining under more +# restrictive rules from being given a non-default PL, and the default PL from being set +# to a non-0 value. RULES_WITH_RESTRICTED_POWER_LEVELS = ( ACCESS_RULE_UNRESTRICTED, ) @@ -84,15 +90,12 @@ class RoomAccessRules(object): default rule to the initial state. """ is_direct = config.get("is_direct") - rules_in_initial_state = False - rule = "" + rule = None # If there's a rules event in the initial state, check if it complies with the # spec for im.vector.room.access_rules and deny the request if not. for event in config.get("initial_state", []): if event["type"] == ACCESS_RULES_TYPE: - rules_in_initial_state = True - rule = event["content"].get("rule") # Make sure the event has a valid content. @@ -112,7 +115,7 @@ class RoomAccessRules(object): # If there's no rules event in the initial state, create one with the default # setting. - if not rules_in_initial_state: + if not rule: if is_direct: default_rule = ACCESS_RULE_DIRECT else: @@ -363,9 +366,10 @@ class RoomAccessRules(object): return True def _is_power_level_content_allowed(self, content, access_rule): - """Denies a power level events that sets 'users_default' to a non-0 value, and - sets the PL of a user that'd be blacklisted in restricted mode to a non-default - value. + """Check if a given power levels event is permitted under the given access rule. + It shouldn't be allowed if it either changes the default PL to a non-0 value or + gives a non-0 PL to a user that would have been forbidden from joining the room + under a more restrictive access rule. Args: content (dict[]): The content of the m.room.power_levels event to check. -- cgit 1.5.1 From 8b44097771adc414defaceb7ade003b8326c0777 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 3 Jul 2019 18:21:42 +0100 Subject: Update synapse/third_party_rules/access_rules.py Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/third_party_rules/access_rules.py | 1 + 1 file changed, 1 insertion(+) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 7c97e79c2b..c71b7f21bf 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -367,6 +367,7 @@ class RoomAccessRules(object): def _is_power_level_content_allowed(self, content, access_rule): """Check if a given power levels event is permitted under the given access rule. + It shouldn't be allowed if it either changes the default PL to a non-0 value or gives a non-0 PL to a user that would have been forbidden from joining the room under a more restrictive access rule. -- cgit 1.5.1 From 4dd7de17b74ec73770d52b19f3f3ac0598cb0dea Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Wed, 3 Jul 2019 18:26:48 +0100 Subject: Incorporate review --- synapse/third_party_rules/access_rules.py | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) (limited to 'synapse/third_party_rules/access_rules.py') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 7c97e79c2b..5f0629ae2e 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -34,12 +34,16 @@ VALID_ACCESS_RULES = ( ) # Rules to which we need to apply the power levels restrictions. -# These are all of the rules that don't forbid users to join based on a server blacklist -# (except for the rules targetting direct chats, because both users should be able to be -# room admins in a direct chat created with the "trusted_private_chat" preset). -# The current restrictions forbid users that would be forbidden from joining under more -# restrictive rules from being given a non-default PL, and the default PL from being set -# to a non-0 value. +# +# These are all of the rules that neither: +# * forbid users from joining based on a server blacklist (which means that there +# is no need to apply power level restrictions), nor +# * target direct chats (since we allow both users to be room admins in this case). +# +# The power-level restrictions, when they are applied, prevent the following: +# * the default power level for users (users_default) being set to anything other than 0. +# * a non-default power level being assigned to any user which would be forbidden from +# joining a restricted room. RULES_WITH_RESTRICTED_POWER_LEVELS = ( ACCESS_RULE_UNRESTRICTED, ) -- cgit 1.5.1