From 110608d1d6f5aec1610284e65a07a64cfda0978a Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 14 Jun 2019 14:43:12 +0100 Subject: Base --- synapse/tchap/__init__.py | 14 ++++++++++++++ synapse/tchap/event_rules.py | 31 +++++++++++++++++++++++++++++++ 2 files changed, 45 insertions(+) create mode 100644 synapse/tchap/__init__.py create mode 100644 synapse/tchap/event_rules.py (limited to 'synapse') diff --git a/synapse/tchap/__init__.py b/synapse/tchap/__init__.py new file mode 100644 index 0000000000..1453d04571 --- /dev/null +++ b/synapse/tchap/__init__.py @@ -0,0 +1,14 @@ +# -*- 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. diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py new file mode 100644 index 0000000000..f82883c642 --- /dev/null +++ b/synapse/tchap/event_rules.py @@ -0,0 +1,31 @@ +# -*- 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. + +from twisted.internet import defer + + +class TchapEventRules(object): + + def __init__(self, config): + # We don't have a config yet. + pass + + @staticmethod + def parse_config(config): + return config + + @defer.inlineCallbacks + def check_event_allowed(self, event, context): + return True -- cgit 1.5.1 From bd8448ccb26e54291cc754f29a0fca88fa9b0515 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Fri, 14 Jun 2019 17:26:37 +0100 Subject: Backbone --- synapse/tchap/event_rules.py | 11 +++++++++++ 1 file changed, 11 insertions(+) (limited to 'synapse') diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index f82883c642..e8acad7577 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -18,6 +18,8 @@ from twisted.internet import defer class TchapEventRules(object): + ACESS_RULES_TYPE = "im.vector.room.access_rules" + def __init__(self, config): # We don't have a config yet. pass @@ -29,3 +31,12 @@ class TchapEventRules(object): @defer.inlineCallbacks def check_event_allowed(self, event, context): return True + + def _apply_restricted(self): + pass + + def _apply_unrestricted(self): + pass + + def _apply_direct(self): + pass -- cgit 1.5.1 From 8b2f655589e992c7e60a0846e005cb380c723f96 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 15:13:27 +0100 Subject: Implement rules for direct --- synapse/tchap/event_rules.py | 112 ++++++++++++++++++++++++++++++++++++++----- 1 file changed, 100 insertions(+), 12 deletions(-) (limited to 'synapse') diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index e8acad7577..fecfbefede 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -15,28 +15,116 @@ from twisted.internet import defer +from synapse.api.constants import EventTypes, Membership +from synapse.config._base import ConfigError -class TchapEventRules(object): +ACESS_RULES_TYPE = "im.vector.room.access_rules" +ACCESS_RULE_RESTRICTED = "restricted" +ACCESS_RULE_UNRESTRICTED = "unrestricted" +ACCESS_RULE_DIRECT = "direct" - ACESS_RULES_TYPE = "im.vector.room.access_rules" +class TchapEventRules(object): def __init__(self, config): - # We don't have a config yet. - pass + self.id_server = config["id_server"] @staticmethod def parse_config(config): - return config + if "id_server" in config: + return config + else: + raise ConfigError("No IS for event rules TchapEventRules") + + def check_event_allowed(self, event, state_events): + # TODO: add rules for sending the access rules event + access_rules = state_events.get((ACESS_RULES_TYPE, "")) + if access_rules is None: + rule = ACCESS_RULE_RESTRICTED + else: + rule = access_rules.content.get("rule") + + if rule == ACCESS_RULE_RESTRICTED: + ret = self._apply_restricted(event, state_events) + elif rule == ACCESS_RULE_UNRESTRICTED: + ret = self._apply_unrestricted(event, state_events) + 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, state_events) + + return ret @defer.inlineCallbacks - def check_event_allowed(self, event, context): + def _apply_restricted(self, event, state_events): return True - def _apply_restricted(self): - pass + def _apply_unrestricted(self, event, state_events): + # "unrestricted" currently means that every event is allowed. + return True + + def _apply_direct(self, event, state_events): + # "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). + # TODO: figure out how to know if the room was created with "is_direct". + + if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: + return True + + # Get the m.room.member and m.room.third_party_invite events from the room's + # state. + member_events = [] + threepid_invite_events = [] + for key, event in state_events.items(): + if key[0] == EventTypes.Member: + member_events.append(event) + if key[0] == EventTypes.ThirdPartyInvite: + threepid_invite_events.append(event) - def _apply_unrestricted(self): - pass + # There should never be more than one 3PID invite in the room state: + if len(threepid_invite_events) == 1 and event.type == EventTypes.ThirdPartyInvite: + # If we already have a 3PID invite in flight, don't accept another one. + return False + + if len(member_events) == 2: + # If the user was within the two initial user of the room, Synapse would have + # looked it up successfully and thus sent a m.room.member here instead of + # m.room.third_party_invite. + if event.type == EventTypes.ThirdPartyInvite: + return False + + # We can only have m.room.member events here. The rule in this case is to only + # allow the event if its target is one of the initial two members in the room, + # i.e. the state key of one of the two m.room.member states in the room. + target = event.state_key + for e in member_events: + if e.state_key == target: + return True + + return False + + # We're alone in the room (and always have been) and there's one 3PID invite in + # flight. + if len(member_events) == 1 and len(threepid_invite_events) == 1: + # We can only have m.room.member events here. In this case, we can only allow + # the event if it's either a m.room.member from the joined user (we can assume + # that the only m.room.member event is a join otherwise we wouldn't be able to + # send an event to the room) or an a m.room.member with the "invite" + # membership which target is the invited user. + target = event.state_key + is_from_threepid_invite = self._is_invite_from_threepid( + event, threepid_invite_events[0], + + ) + if is_from_threepid_invite or target == member_events[0].state_key: + return True + + return False + + return True - def _apply_direct(self): - pass + def _is_invite_from_threepid(self, invite, threepid_invite): + token = invite.content.get("third_party_signed", {}).get("token", "") + return token == threepid_invite.state_key -- cgit 1.5.1 From 53aff08ebe3d1ba07ee7a1a117fc3d33e14cc644 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 17:57:59 +0100 Subject: Implement restricted rules and room creation hook --- synapse/tchap/event_rules.py | 54 +++++++++++++++++++++++++++++++++++--------- 1 file changed, 43 insertions(+), 11 deletions(-) (limited to 'synapse') diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index fecfbefede..471083fb1a 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -17,8 +17,9 @@ from twisted.internet import defer from synapse.api.constants import EventTypes, Membership from synapse.config._base import ConfigError +from synapse.rulecheck.domain_rule_checker import DomainRuleChecker -ACESS_RULES_TYPE = "im.vector.room.access_rules" +ACCESS_RULES_TYPE = "im.vector.room.access_rules" ACCESS_RULE_RESTRICTED = "restricted" ACCESS_RULE_UNRESTRICTED = "unrestricted" ACCESS_RULE_DIRECT = "direct" @@ -27,6 +28,9 @@ ACCESS_RULE_DIRECT = "direct" class TchapEventRules(object): def __init__(self, config): self.id_server = config["id_server"] + self.domains_forbidden_when_restricted = config.get( + "domains_forbidden_when_restricted", [], + ) @staticmethod def parse_config(config): @@ -35,18 +39,45 @@ class TchapEventRules(object): else: raise ConfigError("No IS for event rules TchapEventRules") + def on_create_room(self, requester, config, is_requester_admin): + for event in config.get("initial_state", []): + if event["type"] == ACCESS_RULES_TYPE: + # If there's already a rules event in the initial state, check if it + # breaks the rules for "direct", and if not don't do anything else. + if ( + not config.get("is_direct") + or event["content"]["rule"] != ACCESS_RULE_DIRECT + ): + return + + # Append an access rules event to be sent once every other event in initial_state + # has been sent. If "is_direct" exists and is set to True, the rule needs to be + # "direct", and "restricted" otherwise. + if config.get("is_direct"): + default_rule = ACCESS_RULE_DIRECT + else: + default_rule = ACCESS_RULE_RESTRICTED + + config["initial_state"].append({ + "type": ACCESS_RULES_TYPE, + "state_key": "", + "content": { + "rule": default_rule, + } + }) + def check_event_allowed(self, event, state_events): # TODO: add rules for sending the access rules event - access_rules = state_events.get((ACESS_RULES_TYPE, "")) + access_rules = state_events.get((ACCESS_RULES_TYPE, "")) if access_rules is None: rule = ACCESS_RULE_RESTRICTED else: rule = access_rules.content.get("rule") if rule == ACCESS_RULE_RESTRICTED: - ret = self._apply_restricted(event, state_events) + ret = self._apply_restricted(event) elif rule == ACCESS_RULE_UNRESTRICTED: - ret = self._apply_unrestricted(event, state_events) + ret = self._apply_unrestricted() elif rule == ACCESS_RULE_DIRECT: ret = self._apply_direct(event, state_events) else: @@ -56,11 +87,13 @@ class TchapEventRules(object): return ret - @defer.inlineCallbacks - def _apply_restricted(self, event, state_events): - return True + def _apply_restricted(self, event): + # "restricted" currently means that users can only invite users if their server is + # included in a limited list of domains. + invitee_domain = DomainRuleChecker._get_domain_from_id(event.state_key) + return invitee_domain not in self.domains_forbidden_when_restricted - def _apply_unrestricted(self, event, state_events): + def _apply_unrestricted(self): # "unrestricted" currently means that every event is allowed. return True @@ -68,8 +101,6 @@ class TchapEventRules(object): # "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). - # TODO: figure out how to know if the room was created with "is_direct". - if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: return True @@ -125,6 +156,7 @@ class TchapEventRules(object): return True - def _is_invite_from_threepid(self, invite, threepid_invite): + @staticmethod + def _is_invite_from_threepid(invite, threepid_invite): token = invite.content.get("third_party_signed", {}).get("token", "") return token == threepid_invite.state_key -- cgit 1.5.1 From 112a48a5aa53c49794de988066f846ba4ae5fec1 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 18:53:13 +0100 Subject: Implement 3PID invite hook --- synapse/tchap/event_rules.py | 56 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 49 insertions(+), 7 deletions(-) (limited to 'synapse') diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index 471083fb1a..624336dab3 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -13,9 +13,11 @@ # See the License for the specific language governing permissions and # limitations under the License. +import email.utils + from twisted.internet import defer -from synapse.api.constants import EventTypes, Membership +from synapse.api.constants import EventTypes from synapse.config._base import ConfigError from synapse.rulecheck.domain_rule_checker import DomainRuleChecker @@ -26,7 +28,8 @@ ACCESS_RULE_DIRECT = "direct" class TchapEventRules(object): - def __init__(self, config): + def __init__(self, config, http_client): + self.http_client = http_client self.id_server = config["id_server"] self.domains_forbidden_when_restricted = config.get( "domains_forbidden_when_restricted", [], @@ -66,13 +69,43 @@ class TchapEventRules(object): } }) + @defer.inlineCallbacks + def check_threepid_can_be_invited(self, medium, address, state_events): + rule = self._get_rule_from_state(state_events) + + if medium != "email": + defer.returnValue(False) + + if rule != ACCESS_RULE_RESTRICTED: + # Only "restricted" requires filtering 3PID invites. + defer.returnValue(True) + + parsed_address = email.utils.parseaddr(address)[1] + if parsed_address != address: + # Avoid reproducing the security issue described here: + # https://matrix.org/blog/2019/04/18/security-update-sydent-1-0-2 + # It's probably not worth it but let's just be overly safe here. + defer.returnValue(False) + + res = yield self.http_client.get_json( + "https://%s/_matrix/identity/api/v1/info" % (self.id_server,), + { + "medium": medium, + "address": address, + } + ) + + # Look for a domain that's not forbidden from being invited. + if not res.get("hs"): + defer.returnValue(False) + if res.get("hs") in self.domains_forbidden_when_restricted: + defer.returnValue(False) + + defer.returnValue(True) + def check_event_allowed(self, event, state_events): # TODO: add rules for sending the access rules event - access_rules = state_events.get((ACCESS_RULES_TYPE, "")) - if access_rules is None: - rule = ACCESS_RULE_RESTRICTED - else: - rule = access_rules.content.get("rule") + rule = self._get_rule_from_state(state_events) if rule == ACCESS_RULE_RESTRICTED: ret = self._apply_restricted(event) @@ -156,6 +189,15 @@ class TchapEventRules(object): return True + @staticmethod + def _get_rule_from_state(state_events): + access_rules = state_events.get((ACCESS_RULES_TYPE, "")) + if access_rules is None: + rule = ACCESS_RULE_RESTRICTED + else: + rule = access_rules.content.get("rule") + return rule + @staticmethod def _is_invite_from_threepid(invite, threepid_invite): token = invite.content.get("third_party_signed", {}).get("token", "") -- cgit 1.5.1 From 00736c8935b544112f61b5cdc283569ad76136a3 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 19:21:09 +0100 Subject: Improve doc --- synapse/tchap/event_rules.py | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) (limited to 'synapse') diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index 624336dab3..7a2bec8072 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -87,6 +87,7 @@ class TchapEventRules(object): # It's probably not worth it but let's just be overly safe here. defer.returnValue(False) + # Get the HS this address belongs to from the identity server. res = yield self.http_client.get_json( "https://%s/_matrix/identity/api/v1/info" % (self.id_server,), { @@ -147,7 +148,11 @@ class TchapEventRules(object): if key[0] == EventTypes.ThirdPartyInvite: threepid_invite_events.append(event) - # There should never be more than one 3PID invite in the room state: + # There should never be more than one 3PID invite in the room state: if the second + # original user came and left, and we're inviting them using their email address, + # given we know they have a Matrix account binded to the address (so they could + # join the first time), Synapse will successfully look it up before attempting to + # store an invite on the IS. if len(threepid_invite_events) == 1 and event.type == EventTypes.ThirdPartyInvite: # If we already have a 3PID invite in flight, don't accept another one. return False @@ -175,8 +180,8 @@ class TchapEventRules(object): # We can only have m.room.member events here. In this case, we can only allow # the event if it's either a m.room.member from the joined user (we can assume # that the only m.room.member event is a join otherwise we wouldn't be able to - # send an event to the room) or an a m.room.member with the "invite" - # membership which target is the invited user. + # send an event to the room) or an an invite event which target is the invited + # user. target = event.state_key is_from_threepid_invite = self._is_invite_from_threepid( event, threepid_invite_events[0], -- cgit 1.5.1 From 073dd7778e0f9fdfdfcce53ef4c9d8a96cffeb63 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 19:21:58 +0100 Subject: Implement rules change --- synapse/tchap/event_rules.py | 41 ++++++++++++++++++++++++++++++++++++++++- 1 file changed, 40 insertions(+), 1 deletion(-) (limited to 'synapse') diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index 7a2bec8072..18d6d3bd6f 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -105,7 +105,10 @@ class TchapEventRules(object): defer.returnValue(True) def check_event_allowed(self, event, state_events): - # TODO: add rules for sending the access rules event + # Special-case the access rules event. + if event.type == ACCESS_RULES_TYPE: + return self._on_rules_change(event, state_events) + rule = self._get_rule_from_state(state_events) if rule == ACCESS_RULE_RESTRICTED: @@ -121,6 +124,42 @@ class TchapEventRules(object): return ret + def _on_rules_change(self, event, state_events): + new_rule = event.content.get("rule") + + # Check for invalid values. + if ( + new_rule != ACCESS_RULE_DIRECT + and new_rule != ACCESS_RULE_RESTRICTED + and new_rule != ACCESS_RULE_UNRESTRICTED + ): + return False + + # Make sure we don't apply "direct" if the room has more than two members. + if new_rule == ACCESS_RULE_DIRECT: + member_events_count = 0 + for key, event in state_events.items(): + if key[0] == EventTypes.Member: + member_events_count += 1 + + if member_events_count > 2: + return False + + prev_rules_event = state_events.get((ACCESS_RULES_TYPE, "")) + + # Now that we know the new rule doesn't break the "direct" case, we can allow any + # new rule in rooms that had none before. + if prev_rules_event is None: + return True + + prev_rule = prev_rules_event.content.get("rule") + + # Currently, we can only go from "restricted" to "unrestricted". + if prev_rule == ACCESS_RULE_RESTRICTED and new_rule == ACCESS_RULE_UNRESTRICTED: + return True + + return False + def _apply_restricted(self, event): # "restricted" currently means that users can only invite users if their server is # included in a limited list of domains. -- cgit 1.5.1 From 19a4298a51a99f4a920693824bdba8f4758de805 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 19:22:31 +0100 Subject: Fix function call --- synapse/tchap/event_rules.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'synapse') diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py index 18d6d3bd6f..a724b22790 100644 --- a/synapse/tchap/event_rules.py +++ b/synapse/tchap/event_rules.py @@ -120,7 +120,7 @@ class TchapEventRules(object): 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, state_events) + ret = self._apply_restricted(event) return ret -- cgit 1.5.1 From 1cd0ecc1f22be02bfb11794cb472d333fd82d238 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 19:29:42 +0100 Subject: Rename into RoomAccessRules --- synapse/tchap/__init__.py | 14 -- synapse/tchap/event_rules.py | 248 ------------------------------ synapse/third_party_rules/__init__.py | 14 ++ synapse/third_party_rules/access_rules.py | 248 ++++++++++++++++++++++++++++++ 4 files changed, 262 insertions(+), 262 deletions(-) delete mode 100644 synapse/tchap/__init__.py delete mode 100644 synapse/tchap/event_rules.py create mode 100644 synapse/third_party_rules/__init__.py create mode 100644 synapse/third_party_rules/access_rules.py (limited to 'synapse') diff --git a/synapse/tchap/__init__.py b/synapse/tchap/__init__.py deleted file mode 100644 index 1453d04571..0000000000 --- a/synapse/tchap/__init__.py +++ /dev/null @@ -1,14 +0,0 @@ -# -*- 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. diff --git a/synapse/tchap/event_rules.py b/synapse/tchap/event_rules.py deleted file mode 100644 index a724b22790..0000000000 --- a/synapse/tchap/event_rules.py +++ /dev/null @@ -1,248 +0,0 @@ -# -*- 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 email.utils - -from twisted.internet import defer - -from synapse.api.constants import EventTypes -from synapse.config._base import ConfigError -from synapse.rulecheck.domain_rule_checker import DomainRuleChecker - -ACCESS_RULES_TYPE = "im.vector.room.access_rules" -ACCESS_RULE_RESTRICTED = "restricted" -ACCESS_RULE_UNRESTRICTED = "unrestricted" -ACCESS_RULE_DIRECT = "direct" - - -class TchapEventRules(object): - def __init__(self, config, http_client): - self.http_client = http_client - self.id_server = config["id_server"] - self.domains_forbidden_when_restricted = config.get( - "domains_forbidden_when_restricted", [], - ) - - @staticmethod - def parse_config(config): - if "id_server" in config: - return config - else: - raise ConfigError("No IS for event rules TchapEventRules") - - def on_create_room(self, requester, config, is_requester_admin): - for event in config.get("initial_state", []): - if event["type"] == ACCESS_RULES_TYPE: - # If there's already a rules event in the initial state, check if it - # breaks the rules for "direct", and if not don't do anything else. - if ( - not config.get("is_direct") - or event["content"]["rule"] != ACCESS_RULE_DIRECT - ): - return - - # Append an access rules event to be sent once every other event in initial_state - # has been sent. If "is_direct" exists and is set to True, the rule needs to be - # "direct", and "restricted" otherwise. - if config.get("is_direct"): - default_rule = ACCESS_RULE_DIRECT - else: - default_rule = ACCESS_RULE_RESTRICTED - - config["initial_state"].append({ - "type": ACCESS_RULES_TYPE, - "state_key": "", - "content": { - "rule": default_rule, - } - }) - - @defer.inlineCallbacks - def check_threepid_can_be_invited(self, medium, address, state_events): - rule = self._get_rule_from_state(state_events) - - if medium != "email": - defer.returnValue(False) - - if rule != ACCESS_RULE_RESTRICTED: - # Only "restricted" requires filtering 3PID invites. - defer.returnValue(True) - - parsed_address = email.utils.parseaddr(address)[1] - if parsed_address != address: - # Avoid reproducing the security issue described here: - # https://matrix.org/blog/2019/04/18/security-update-sydent-1-0-2 - # It's probably not worth it but let's just be overly safe here. - defer.returnValue(False) - - # Get the HS this address belongs to from the identity server. - res = yield self.http_client.get_json( - "https://%s/_matrix/identity/api/v1/info" % (self.id_server,), - { - "medium": medium, - "address": address, - } - ) - - # Look for a domain that's not forbidden from being invited. - if not res.get("hs"): - defer.returnValue(False) - if res.get("hs") in self.domains_forbidden_when_restricted: - defer.returnValue(False) - - defer.returnValue(True) - - def check_event_allowed(self, event, state_events): - # Special-case the access rules event. - if event.type == ACCESS_RULES_TYPE: - return self._on_rules_change(event, state_events) - - rule = self._get_rule_from_state(state_events) - - 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 - - def _on_rules_change(self, event, state_events): - new_rule = event.content.get("rule") - - # Check for invalid values. - if ( - new_rule != ACCESS_RULE_DIRECT - and new_rule != ACCESS_RULE_RESTRICTED - and new_rule != ACCESS_RULE_UNRESTRICTED - ): - return False - - # Make sure we don't apply "direct" if the room has more than two members. - if new_rule == ACCESS_RULE_DIRECT: - member_events_count = 0 - for key, event in state_events.items(): - if key[0] == EventTypes.Member: - member_events_count += 1 - - if member_events_count > 2: - return False - - prev_rules_event = state_events.get((ACCESS_RULES_TYPE, "")) - - # Now that we know the new rule doesn't break the "direct" case, we can allow any - # new rule in rooms that had none before. - if prev_rules_event is None: - return True - - prev_rule = prev_rules_event.content.get("rule") - - # Currently, we can only go from "restricted" to "unrestricted". - if prev_rule == ACCESS_RULE_RESTRICTED and new_rule == ACCESS_RULE_UNRESTRICTED: - return True - - return False - - def _apply_restricted(self, event): - # "restricted" currently means that users can only invite users if their server is - # included in a limited list of domains. - invitee_domain = DomainRuleChecker._get_domain_from_id(event.state_key) - return invitee_domain not in self.domains_forbidden_when_restricted - - def _apply_unrestricted(self): - # "unrestricted" currently means that every event is allowed. - return True - - def _apply_direct(self, event, state_events): - # "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 m.room.member and m.room.third_party_invite events from the room's - # state. - member_events = [] - threepid_invite_events = [] - for key, event in state_events.items(): - if key[0] == EventTypes.Member: - member_events.append(event) - if key[0] == EventTypes.ThirdPartyInvite: - threepid_invite_events.append(event) - - # There should never be more than one 3PID invite in the room state: if the second - # original user came and left, and we're inviting them using their email address, - # given we know they have a Matrix account binded to the address (so they could - # join the first time), Synapse will successfully look it up before attempting to - # store an invite on the IS. - if len(threepid_invite_events) == 1 and event.type == EventTypes.ThirdPartyInvite: - # If we already have a 3PID invite in flight, don't accept another one. - return False - - if len(member_events) == 2: - # If the user was within the two initial user of the room, Synapse would have - # looked it up successfully and thus sent a m.room.member here instead of - # m.room.third_party_invite. - if event.type == EventTypes.ThirdPartyInvite: - return False - - # We can only have m.room.member events here. The rule in this case is to only - # allow the event if its target is one of the initial two members in the room, - # i.e. the state key of one of the two m.room.member states in the room. - target = event.state_key - for e in member_events: - if e.state_key == target: - return True - - return False - - # We're alone in the room (and always have been) and there's one 3PID invite in - # flight. - if len(member_events) == 1 and len(threepid_invite_events) == 1: - # We can only have m.room.member events here. In this case, we can only allow - # the event if it's either a m.room.member from the joined user (we can assume - # that the only m.room.member event is a join otherwise we wouldn't be able to - # send an event to the room) or an an invite event which target is the invited - # user. - target = event.state_key - is_from_threepid_invite = self._is_invite_from_threepid( - event, threepid_invite_events[0], - - ) - if is_from_threepid_invite or target == member_events[0].state_key: - return True - - return False - - return True - - @staticmethod - def _get_rule_from_state(state_events): - access_rules = state_events.get((ACCESS_RULES_TYPE, "")) - if access_rules is None: - rule = ACCESS_RULE_RESTRICTED - else: - rule = access_rules.content.get("rule") - return rule - - @staticmethod - def _is_invite_from_threepid(invite, threepid_invite): - token = invite.content.get("third_party_signed", {}).get("token", "") - return token == threepid_invite.state_key diff --git a/synapse/third_party_rules/__init__.py b/synapse/third_party_rules/__init__.py new file mode 100644 index 0000000000..1453d04571 --- /dev/null +++ b/synapse/third_party_rules/__init__.py @@ -0,0 +1,14 @@ +# -*- 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. diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py new file mode 100644 index 0000000000..1f03138752 --- /dev/null +++ b/synapse/third_party_rules/access_rules.py @@ -0,0 +1,248 @@ +# -*- 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 email.utils + +from twisted.internet import defer + +from synapse.api.constants import EventTypes +from synapse.config._base import ConfigError +from synapse.rulecheck.domain_rule_checker import DomainRuleChecker + +ACCESS_RULES_TYPE = "im.vector.room.access_rules" +ACCESS_RULE_RESTRICTED = "restricted" +ACCESS_RULE_UNRESTRICTED = "unrestricted" +ACCESS_RULE_DIRECT = "direct" + + +class RoomAccessRules(object): + def __init__(self, config, http_client): + self.http_client = http_client + self.id_server = config["id_server"] + self.domains_forbidden_when_restricted = config.get( + "domains_forbidden_when_restricted", [], + ) + + @staticmethod + def parse_config(config): + if "id_server" in config: + return config + else: + raise ConfigError("No IS for event rules TchapEventRules") + + def on_create_room(self, requester, config, is_requester_admin): + for event in config.get("initial_state", []): + if event["type"] == ACCESS_RULES_TYPE: + # If there's already a rules event in the initial state, check if it + # breaks the rules for "direct", and if not don't do anything else. + if ( + not config.get("is_direct") + or event["content"]["rule"] != ACCESS_RULE_DIRECT + ): + return + + # Append an access rules event to be sent once every other event in initial_state + # has been sent. If "is_direct" exists and is set to True, the rule needs to be + # "direct", and "restricted" otherwise. + if config.get("is_direct"): + default_rule = ACCESS_RULE_DIRECT + else: + default_rule = ACCESS_RULE_RESTRICTED + + config["initial_state"].append({ + "type": ACCESS_RULES_TYPE, + "state_key": "", + "content": { + "rule": default_rule, + } + }) + + @defer.inlineCallbacks + def check_threepid_can_be_invited(self, medium, address, state_events): + rule = self._get_rule_from_state(state_events) + + if medium != "email": + defer.returnValue(False) + + if rule != ACCESS_RULE_RESTRICTED: + # Only "restricted" requires filtering 3PID invites. + defer.returnValue(True) + + parsed_address = email.utils.parseaddr(address)[1] + if parsed_address != address: + # Avoid reproducing the security issue described here: + # https://matrix.org/blog/2019/04/18/security-update-sydent-1-0-2 + # It's probably not worth it but let's just be overly safe here. + defer.returnValue(False) + + # Get the HS this address belongs to from the identity server. + res = yield self.http_client.get_json( + "https://%s/_matrix/identity/api/v1/info" % (self.id_server,), + { + "medium": medium, + "address": address, + } + ) + + # Look for a domain that's not forbidden from being invited. + if not res.get("hs"): + defer.returnValue(False) + if res.get("hs") in self.domains_forbidden_when_restricted: + defer.returnValue(False) + + defer.returnValue(True) + + def check_event_allowed(self, event, state_events): + # Special-case the access rules event. + if event.type == ACCESS_RULES_TYPE: + return self._on_rules_change(event, state_events) + + rule = self._get_rule_from_state(state_events) + + 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 + + def _on_rules_change(self, event, state_events): + new_rule = event.content.get("rule") + + # Check for invalid values. + if ( + new_rule != ACCESS_RULE_DIRECT + and new_rule != ACCESS_RULE_RESTRICTED + and new_rule != ACCESS_RULE_UNRESTRICTED + ): + return False + + # Make sure we don't apply "direct" if the room has more than two members. + if new_rule == ACCESS_RULE_DIRECT: + member_events_count = 0 + for key, event in state_events.items(): + if key[0] == EventTypes.Member: + member_events_count += 1 + + if member_events_count > 2: + return False + + prev_rules_event = state_events.get((ACCESS_RULES_TYPE, "")) + + # Now that we know the new rule doesn't break the "direct" case, we can allow any + # new rule in rooms that had none before. + if prev_rules_event is None: + return True + + prev_rule = prev_rules_event.content.get("rule") + + # Currently, we can only go from "restricted" to "unrestricted". + if prev_rule == ACCESS_RULE_RESTRICTED and new_rule == ACCESS_RULE_UNRESTRICTED: + return True + + return False + + def _apply_restricted(self, event): + # "restricted" currently means that users can only invite users if their server is + # included in a limited list of domains. + invitee_domain = DomainRuleChecker._get_domain_from_id(event.state_key) + return invitee_domain not in self.domains_forbidden_when_restricted + + def _apply_unrestricted(self): + # "unrestricted" currently means that every event is allowed. + return True + + def _apply_direct(self, event, state_events): + # "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 m.room.member and m.room.third_party_invite events from the room's + # state. + member_events = [] + threepid_invite_events = [] + for key, event in state_events.items(): + if key[0] == EventTypes.Member: + member_events.append(event) + if key[0] == EventTypes.ThirdPartyInvite: + threepid_invite_events.append(event) + + # There should never be more than one 3PID invite in the room state: if the second + # original user came and left, and we're inviting them using their email address, + # given we know they have a Matrix account binded to the address (so they could + # join the first time), Synapse will successfully look it up before attempting to + # store an invite on the IS. + if len(threepid_invite_events) == 1 and event.type == EventTypes.ThirdPartyInvite: + # If we already have a 3PID invite in flight, don't accept another one. + return False + + if len(member_events) == 2: + # If the user was within the two initial user of the room, Synapse would have + # looked it up successfully and thus sent a m.room.member here instead of + # m.room.third_party_invite. + if event.type == EventTypes.ThirdPartyInvite: + return False + + # We can only have m.room.member events here. The rule in this case is to only + # allow the event if its target is one of the initial two members in the room, + # i.e. the state key of one of the two m.room.member states in the room. + target = event.state_key + for e in member_events: + if e.state_key == target: + return True + + return False + + # We're alone in the room (and always have been) and there's one 3PID invite in + # flight. + if len(member_events) == 1 and len(threepid_invite_events) == 1: + # We can only have m.room.member events here. In this case, we can only allow + # the event if it's either a m.room.member from the joined user (we can assume + # that the only m.room.member event is a join otherwise we wouldn't be able to + # send an event to the room) or an an invite event which target is the invited + # user. + target = event.state_key + is_from_threepid_invite = self._is_invite_from_threepid( + event, threepid_invite_events[0], + + ) + if is_from_threepid_invite or target == member_events[0].state_key: + return True + + return False + + return True + + @staticmethod + def _get_rule_from_state(state_events): + access_rules = state_events.get((ACCESS_RULES_TYPE, "")) + if access_rules is None: + rule = ACCESS_RULE_RESTRICTED + else: + rule = access_rules.content.get("rule") + return rule + + @staticmethod + def _is_invite_from_threepid(invite, threepid_invite): + token = invite.content.get("third_party_signed", {}).get("token", "") + return token == threepid_invite.state_key -- cgit 1.5.1 From 8aea2c3be12d8f96d96fc53cb4880d56ce4c8dc4 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Mon, 17 Jun 2019 20:09:53 +0100 Subject: Docstrings --- synapse/third_party_rules/access_rules.py | 181 +++++++++++++++++++++++++----- 1 file changed, 154 insertions(+), 27 deletions(-) (limited to 'synapse') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 1f03138752..a8b3ed9458 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -28,9 +28,31 @@ ACCESS_RULE_DIRECT = "direct" class RoomAccessRules(object): + """Implementation of the ThirdPartyEventRules module API that allows federation admins + to define custom rules for specific events and actions. + Implements the custom behaviour for the "im.vector.room.access_rules" state event. + + Takes a config in the format: + + third_party_event_rules: + module: third_party_rules.RoomAccessRules + config: + # List of domains (server names) that can't be invited to rooms if the + # "restricted" rule is set. Defaults to an empty list. + domains_forbidden_when_restricted: [] + + # Identity server to use when checking the HS an email address belongs to + # using the /info endpoint. Required. + id_server: "vector.im" + + Don't forget to consider if you can invite users from your own domain. + """ + def __init__(self, config, http_client): self.http_client = http_client + self.id_server = config["id_server"] + self.domains_forbidden_when_restricted = config.get( "domains_forbidden_when_restricted", [], ) @@ -43,34 +65,77 @@ class RoomAccessRules(object): raise ConfigError("No IS for event rules TchapEventRules") def on_create_room(self, requester, config, is_requester_admin): + """Implements synapse.events.ThirdPartyEventRules.on_create_room + + Checks if a im.vector.room.access_rules event is being set during room creation. + If yes, make sure the event is correct. Otherwise, append an event with the + default rule to the initial state. + """ + is_direct = config.get("is_direct") + rules_in_initial_state = False + + # If there's a rules event in the initial state, check if it complies with the + # spec for im.vector.room.access_rules and fix it if not. for event in config.get("initial_state", []): if event["type"] == ACCESS_RULES_TYPE: - # If there's already a rules event in the initial state, check if it - # breaks the rules for "direct", and if not don't do anything else. - if ( - not config.get("is_direct") - or event["content"]["rule"] != ACCESS_RULE_DIRECT - ): - return - - # Append an access rules event to be sent once every other event in initial_state - # has been sent. If "is_direct" exists and is set to True, the rule needs to be - # "direct", and "restricted" otherwise. - if config.get("is_direct"): - default_rule = ACCESS_RULE_DIRECT - else: - default_rule = ACCESS_RULE_RESTRICTED + rules_in_initial_state = True + + rule = event["content"].get("rule") + + # Make sure the event has a valid content. + if rule is None: + event["content"] = { + "rule": self._on_create_room_default_rule(is_direct) + } + + # Make sure the rule name is valid. + if not self._is_rule_name_valid(rule): + event["content"]["rule"] = self._on_create_room_default_rule( + is_direct, + ) + + # Make sure the rule is "direct" if the room is a direct chat. + if is_direct and rule != ACCESS_RULE_DIRECT: + event["content"]["rule"] = ACCESS_RULE_DIRECT + + # Make sure the rule is not "direct" if the room isn't a direct chat. + if rule == ACCESS_RULE_DIRECT and not is_direct: + event["content"]["rule"] = ACCESS_RULE_RESTRICTED + + # If there's no rules event in the initial state, create one with the default + # setting. + if not rules_in_initial_state: + config["initial_state"].append({ + "type": ACCESS_RULES_TYPE, + "state_key": "", + "content": { + "rule": self._on_create_room_default_rule(is_direct), + } + }) - config["initial_state"].append({ - "type": ACCESS_RULES_TYPE, - "state_key": "", - "content": { - "rule": default_rule, - } - }) + @staticmethod + def _on_create_room_default_rule(is_direct): + """Returns the default rule to set. + + Args: + is_direct (bool): Is the room created with "is_direct" set to True. + + Returns: + str, the name of the rule tu use as the default. + """ + if is_direct: + return ACCESS_RULE_DIRECT + else: + return ACCESS_RULE_RESTRICTED @defer.inlineCallbacks def check_threepid_can_be_invited(self, medium, address, state_events): + """Implements synapse.events.ThirdPartyEventRules.check_threepid_can_be_invited + + Check if a threepid can be invited to the room via a 3PID invite given the current + rules and the threepid's address, by retrieving the HS it's mapped to from the + configured identity server, and checking if we can invite users from it. + """ rule = self._get_rule_from_state(state_events) if medium != "email": @@ -105,6 +170,11 @@ class RoomAccessRules(object): defer.returnValue(True) def check_event_allowed(self, event, state_events): + """Implements synapse.events.ThirdPartyEventRules.check_event_allowed + + 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) @@ -125,14 +195,20 @@ class RoomAccessRules(object): return ret 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. + + Args: + event (synapse.events.EventBase): The event to check. + 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. + """ new_rule = event.content.get("rule") # Check for invalid values. - if ( - new_rule != ACCESS_RULE_DIRECT - and new_rule != ACCESS_RULE_RESTRICTED - and new_rule != ACCESS_RULE_UNRESTRICTED - ): + if not self._is_rule_name_valid(new_rule): return False # Make sure we don't apply "direct" if the room has more than two members. @@ -161,16 +237,37 @@ class RoomAccessRules(object): return False def _apply_restricted(self, event): + """Implements the checks and behaviour specified for the "restricted" rule. + + 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. invitee_domain = DomainRuleChecker._get_domain_from_id(event.state_key) return invitee_domain not in self.domains_forbidden_when_restricted def _apply_unrestricted(self): + """Implements the checks and behaviour specified for the "unrestricted" rule. + + 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): + """Implements the checks and behaviour specified for the "direct" rule. + + Args: + event (synapse.events.EventBase): The event to check. + 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. + """ # "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). @@ -235,6 +332,14 @@ class RoomAccessRules(object): @staticmethod def _get_rule_from_state(state_events): + """Extract the rule to be applied from the given set of state events. + + Args: + state_events (dict[tuple[event type, state key], EventBase]): The set of state + events + Returns: + str, the name of the rule (either "direct", "restricted" or "unrestricted") + """ access_rules = state_events.get((ACCESS_RULES_TYPE, "")) if access_rules is None: rule = ACCESS_RULE_RESTRICTED @@ -244,5 +349,27 @@ class RoomAccessRules(object): @staticmethod def _is_invite_from_threepid(invite, threepid_invite): + """Checks whether the given invite follows the given 3PID invite. + + Args: + invite (EventBase): The m.room.member event with "invite" membership. + threepid_invite (EventBase): The m.room.third_party_invite event. + """ token = invite.content.get("third_party_signed", {}).get("token", "") return token == threepid_invite.state_key + + @staticmethod + def _is_rule_name_valid(rule): + """Returns whether the given rule name is within the allowed values ("direct", + "restricted" or "unrestricted"). + + Args: + rule (str): The name of the rule. + Returns: + bool, True if the name is valid, False otherwise. + """ + return ( + rule == ACCESS_RULE_DIRECT + or rule == ACCESS_RULE_RESTRICTED + or rule == ACCESS_RULE_UNRESTRICTED + ) -- cgit 1.5.1 From f7339d42ee572f892c12501896a477c8f927b94e Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 11:28:33 +0100 Subject: Fixes --- synapse/third_party_rules/access_rules.py | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'synapse') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index a8b3ed9458..07a1649584 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -105,6 +105,9 @@ 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 config.get("initial_state"): + config["initial_state"] = [] + config["initial_state"].append({ "type": ACCESS_RULES_TYPE, "state_key": "", @@ -246,6 +249,8 @@ class RoomAccessRules(object): """ # "restricted" currently means that users can only invite users if their server is # included in a limited list of domains. + if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: + return True invitee_domain = DomainRuleChecker._get_domain_from_id(event.state_key) return invitee_domain not in self.domains_forbidden_when_restricted -- cgit 1.5.1 From d36a876d2d4571321e8a989c5bfb73a3b2e1dc1f Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 14:53:33 +0100 Subject: Incorporate review --- synapse/third_party_rules/access_rules.py | 147 ++++++++++++++---------------- 1 file changed, 69 insertions(+), 78 deletions(-) (limited to 'synapse') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 07a1649584..096e23e149 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -18,14 +18,21 @@ import email.utils from twisted.internet import defer from synapse.api.constants import EventTypes +from synapse.api.errors import SynapseError from synapse.config._base import ConfigError -from synapse.rulecheck.domain_rule_checker import DomainRuleChecker +from synapse.types import get_domain_from_id ACCESS_RULES_TYPE = "im.vector.room.access_rules" ACCESS_RULE_RESTRICTED = "restricted" ACCESS_RULE_UNRESTRICTED = "unrestricted" ACCESS_RULE_DIRECT = "direct" +VALID_ACCESS_RULES = ( + ACCESS_RULE_DIRECT, + ACCESS_RULE_RESTRICTED, + ACCESS_RULE_UNRESTRICTED, +) + class RoomAccessRules(object): """Implementation of the ThirdPartyEventRules module API that allows federation admins @@ -75,7 +82,7 @@ class RoomAccessRules(object): rules_in_initial_state = False # If there's a rules event in the initial state, check if it complies with the - # spec for im.vector.room.access_rules and fix it if not. + # 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 @@ -84,27 +91,27 @@ class RoomAccessRules(object): # Make sure the event has a valid content. if rule is None: - event["content"] = { - "rule": self._on_create_room_default_rule(is_direct) - } + raise SynapseError(400, "Invalid access rule",) # Make sure the rule name is valid. - if not self._is_rule_name_valid(rule): - event["content"]["rule"] = self._on_create_room_default_rule( - is_direct, - ) + if rule not in VALID_ACCESS_RULES: + 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: - event["content"]["rule"] = ACCESS_RULE_DIRECT - - # Make sure the rule is not "direct" if the room isn't a direct chat. - if rule == ACCESS_RULE_DIRECT and not is_direct: - event["content"]["rule"] = ACCESS_RULE_RESTRICTED + if ( + (is_direct and rule != ACCESS_RULE_DIRECT) + or (rule == ACCESS_RULE_DIRECT and not is_direct) + ): + raise SynapseError(400, "Invalid access rule",) # If there's no rules event in the initial state, create one with the default # setting. if not rules_in_initial_state: + if is_direct: + default_rule = ACCESS_RULE_DIRECT + else: + default_rule = ACCESS_RULE_RESTRICTED + if not config.get("initial_state"): config["initial_state"] = [] @@ -112,25 +119,10 @@ class RoomAccessRules(object): "type": ACCESS_RULES_TYPE, "state_key": "", "content": { - "rule": self._on_create_room_default_rule(is_direct), + "rule": default_rule, } }) - @staticmethod - def _on_create_room_default_rule(is_direct): - """Returns the default rule to set. - - Args: - is_direct (bool): Is the room created with "is_direct" set to True. - - Returns: - str, the name of the rule tu use as the default. - """ - if is_direct: - return ACCESS_RULE_DIRECT - else: - return ACCESS_RULE_RESTRICTED - @defer.inlineCallbacks def check_threepid_can_be_invited(self, medium, address, state_events): """Implements synapse.events.ThirdPartyEventRules.check_threepid_can_be_invited @@ -145,7 +137,9 @@ class RoomAccessRules(object): defer.returnValue(False) if rule != ACCESS_RULE_RESTRICTED: - # Only "restricted" requires filtering 3PID invites. + # Only "restricted" requires filtering 3PID invites. We don't need to do + # anything for "direct" here, because only "restricted" requires filtering + # based on the HS the address is mapped to. defer.returnValue(True) parsed_address = email.utils.parseaddr(address)[1] @@ -211,17 +205,16 @@ class RoomAccessRules(object): new_rule = event.content.get("rule") # Check for invalid values. - if not self._is_rule_name_valid(new_rule): + if new_rule not in VALID_ACCESS_RULES: return False # Make sure we don't apply "direct" if the room has more than two members. if new_rule == ACCESS_RULE_DIRECT: - member_events_count = 0 - for key, event in state_events.items(): - if key[0] == EventTypes.Member: - member_events_count += 1 + existing_members, threepid_tokens = self._get_members_and_tokens_from_state( + state_events, + ) - if member_events_count > 2: + if len(existing_members) > 2 or len(threepid_tokens) > 1: return False prev_rules_event = state_events.get((ACCESS_RULES_TYPE, "")) @@ -251,7 +244,7 @@ class RoomAccessRules(object): # included in a limited list of domains. if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: return True - invitee_domain = DomainRuleChecker._get_domain_from_id(event.state_key) + invitee_domain = get_domain_from_id(event.state_key) return invitee_domain not in self.domains_forbidden_when_restricted def _apply_unrestricted(self): @@ -279,26 +272,21 @@ class RoomAccessRules(object): if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: return True - # Get the m.room.member and m.room.third_party_invite events from the room's - # state. - member_events = [] - threepid_invite_events = [] - for key, event in state_events.items(): - if key[0] == EventTypes.Member: - member_events.append(event) - if key[0] == EventTypes.ThirdPartyInvite: - threepid_invite_events.append(event) + # 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, + ) # There should never be more than one 3PID invite in the room state: if the second # original user came and left, and we're inviting them using their email address, # given we know they have a Matrix account binded to the address (so they could # join the first time), Synapse will successfully look it up before attempting to # store an invite on the IS. - if len(threepid_invite_events) == 1 and event.type == EventTypes.ThirdPartyInvite: + if len(threepid_tokens) == 1 and event.type == EventTypes.ThirdPartyInvite: # If we already have a 3PID invite in flight, don't accept another one. return False - if len(member_events) == 2: + if len(existing_members) == 2: # If the user was within the two initial user of the room, Synapse would have # looked it up successfully and thus sent a m.room.member here instead of # m.room.third_party_invite. @@ -308,16 +296,11 @@ class RoomAccessRules(object): # We can only have m.room.member events here. The rule in this case is to only # allow the event if its target is one of the initial two members in the room, # i.e. the state key of one of the two m.room.member states in the room. - target = event.state_key - for e in member_events: - if e.state_key == target: - return True - - return False + return event.state_key in existing_members # We're alone in the room (and always have been) and there's one 3PID invite in # flight. - if len(member_events) == 1 and len(threepid_invite_events) == 1: + if len(existing_members) == 1 and len(threepid_tokens) == 1: # We can only have m.room.member events here. In this case, we can only allow # the event if it's either a m.room.member from the joined user (we can assume # that the only m.room.member event is a join otherwise we wouldn't be able to @@ -325,10 +308,9 @@ class RoomAccessRules(object): # user. target = event.state_key is_from_threepid_invite = self._is_invite_from_threepid( - event, threepid_invite_events[0], - + event, threepid_tokens[0], ) - if is_from_threepid_invite or target == member_events[0].state_key: + if is_from_threepid_invite or target == existing_members[0]: return True return False @@ -341,7 +323,7 @@ class RoomAccessRules(object): Args: state_events (dict[tuple[event type, state key], EventBase]): The set of state - events + events. Returns: str, the name of the rule (either "direct", "restricted" or "unrestricted") """ @@ -353,28 +335,37 @@ class RoomAccessRules(object): return rule @staticmethod - def _is_invite_from_threepid(invite, threepid_invite): - """Checks whether the given invite follows the given 3PID invite. + def _get_members_and_tokens_from_state(state_events): + """Retrieves from a list of state events the list of users that have a + m.room.member event in the room, and the tokens of 3PID invites in the room. Args: - invite (EventBase): The m.room.member event with "invite" membership. - threepid_invite (EventBase): The m.room.third_party_invite event. + state_events (dict[tuple[event type, state key], EventBase]): The set of state + events. + Returns: + existing_members (list[str]): List of targets of the m.room.member events in + the state. + threepid_invite_tokens (list[str]): List of tokens of the 3PID invites in the + state. """ - token = invite.content.get("third_party_signed", {}).get("token", "") - return token == threepid_invite.state_key + existing_members = [] + threepid_invite_tokens = [] + for key, event in state_events.items(): + if key[0] == EventTypes.Member: + existing_members.append(event.state_key) + if key[0] == EventTypes.ThirdPartyInvite: + threepid_invite_tokens.append(event.state_key) + + return existing_members, threepid_invite_tokens + @staticmethod - def _is_rule_name_valid(rule): - """Returns whether the given rule name is within the allowed values ("direct", - "restricted" or "unrestricted"). + def _is_invite_from_threepid(invite, threepid_invite_token): + """Checks whether the given invite follows the given 3PID invite. Args: - rule (str): The name of the rule. - Returns: - bool, True if the name is valid, False otherwise. + invite (EventBase): The m.room.member event with "invite" membership. + threepid_invite_token (str): The state key from the 3PID invite. """ - return ( - rule == ACCESS_RULE_DIRECT - or rule == ACCESS_RULE_RESTRICTED - or rule == ACCESS_RULE_UNRESTRICTED - ) + token = invite.content.get("third_party_signed", {}).get("token", "") + return token == threepid_invite_token -- cgit 1.5.1 From cefc5542fb6ad0cd3f4b03f0d720d0cd91eb1a52 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 14:59:05 +0100 Subject: Lint --- synapse/third_party_rules/access_rules.py | 1 - 1 file changed, 1 deletion(-) (limited to 'synapse') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index 096e23e149..fbd829cdd2 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -358,7 +358,6 @@ class RoomAccessRules(object): return existing_members, threepid_invite_tokens - @staticmethod def _is_invite_from_threepid(invite, threepid_invite_token): """Checks whether the given invite follows the given 3PID invite. -- cgit 1.5.1 From 71572761240b18645b16adb0bd607d2092865ba0 Mon Sep 17 00:00:00 2001 From: Brendan Abolivier Date: Tue, 18 Jun 2019 17:43:34 +0100 Subject: Don't process 3PIDs in _apply_restricted --- synapse/third_party_rules/access_rules.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'synapse') diff --git a/synapse/third_party_rules/access_rules.py b/synapse/third_party_rules/access_rules.py index fbd829cdd2..d13f8de888 100644 --- a/synapse/third_party_rules/access_rules.py +++ b/synapse/third_party_rules/access_rules.py @@ -242,7 +242,9 @@ class RoomAccessRules(object): """ # "restricted" currently means that users can only invite users if their server is # included in a limited list of domains. - if event.type != EventTypes.Member and event.type != EventTypes.ThirdPartyInvite: + # 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: return True invitee_domain = get_domain_from_id(event.state_key) return invitee_domain not in self.domains_forbidden_when_restricted -- cgit 1.5.1