From 1168cbd54db059cefdf968077f6b14163de6c04c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 22 Sep 2016 10:56:53 +0100 Subject: [PATCH 1/4] Allow invites via 3pid to bypass sender sig check When a server sends a third party invite another server may be the one that the inviting user registers with. In this case it is that remote server that will issue an actual invitation, and wants to do it "in the name of" the original invitee. However, the new proper invite will not be signed by the original server, and thus other servers would reject the invite if it was seen as coming from the original user. To fix this, a special case has been added to the auth rules whereby another server can send an invite "in the name of" another server's user, so long as that user had previously issued a third party invite that is now being accepted. --- synapse/api/auth.py | 17 ++++++++++++++++- synapse/handlers/federation.py | 12 ++++++------ 2 files changed, 22 insertions(+), 7 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 98a50f094..d60c1b15a 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -72,7 +72,7 @@ class Auth(object): auth_events = { (e.type, e.state_key): e for e in auth_events.values() } - self.check(event, auth_events=auth_events, do_sig_check=False) + self.check(event, auth_events=auth_events, do_sig_check=do_sig_check) def check(self, event, auth_events, do_sig_check=True): """ Checks if this event is correctly authed. @@ -92,9 +92,21 @@ class Auth(object): raise AuthError(500, "Event has no room_id: %s" % event) sender_domain = get_domain_from_id(event.sender) + event_id_domain = get_domain_from_id(event.event_id) + + is_invite_via_3pid = ( + event.type == EventTypes.Member + and event.membership == Membership.INVITE + and "third_party_invite" in event.content + ) # Check the sender's domain has signed the event if do_sig_check and not event.signatures.get(sender_domain): + if not is_invite_via_3pid: + raise AuthError(403, "Event not signed by sender's server") + + # Check the event_id's domain has signed the event + if do_sig_check and not event.signatures.get(event_id_domain): raise AuthError(403, "Event not signed by sending server") if auth_events is None: @@ -491,6 +503,9 @@ class Auth(object): if not invite_event: return False + if invite_event.sender != event.sender: + return False + if event.user_id != invite_event.user_id: return False diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index f7cb3c1bb..a393263e1 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1922,15 +1922,15 @@ class FederationHandler(BaseHandler): original_invite = yield self.store.get_event( original_invite_id, allow_none=True ) - if not original_invite: + if original_invite: + display_name = original_invite.content["display_name"] + event_dict["content"]["third_party_invite"]["display_name"] = display_name + else: logger.info( - "Could not find invite event for third_party_invite - " - "discarding: %s" % (event_dict,) + "Could not find invite event for third_party_invite: %r", + event_dict ) - return - display_name = original_invite.content["display_name"] - event_dict["content"]["third_party_invite"]["display_name"] = display_name builder = self.event_builder_factory.new(event_dict) EventValidator().validate_new(builder) message_handler = self.hs.get_handlers().message_handler From a61e4522b51a51261865b24238a42c0cdd6fb6c8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 22 Sep 2016 11:08:12 +0100 Subject: [PATCH 2/4] Shuffle things around to make unit tests work --- synapse/api/auth.py | 29 +++++++++++++++-------------- synapse/types.py | 2 +- 2 files changed, 16 insertions(+), 15 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index d60c1b15a..377bfcc48 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -91,23 +91,24 @@ class Auth(object): if not hasattr(event, "room_id"): raise AuthError(500, "Event has no room_id: %s" % event) - sender_domain = get_domain_from_id(event.sender) - event_id_domain = get_domain_from_id(event.event_id) + if do_sig_check: + sender_domain = get_domain_from_id(event.sender) + event_id_domain = get_domain_from_id(event.event_id) - is_invite_via_3pid = ( - event.type == EventTypes.Member - and event.membership == Membership.INVITE - and "third_party_invite" in event.content - ) + is_invite_via_3pid = ( + event.type == EventTypes.Member + and event.membership == Membership.INVITE + and "third_party_invite" in event.content + ) - # Check the sender's domain has signed the event - if do_sig_check and not event.signatures.get(sender_domain): - if not is_invite_via_3pid: - raise AuthError(403, "Event not signed by sender's server") + # Check the sender's domain has signed the event + if not event.signatures.get(sender_domain): + if not is_invite_via_3pid: + raise AuthError(403, "Event not signed by sender's server") - # Check the event_id's domain has signed the event - if do_sig_check and not event.signatures.get(event_id_domain): - raise AuthError(403, "Event not signed by sending server") + # Check the event_id's domain has signed the event + if not event.signatures.get(event_id_domain): + raise AuthError(403, "Event not signed by sending server") if auth_events is None: # Oh, we don't know what the state of the room was, so we diff --git a/synapse/types.py b/synapse/types.py index 9d64e8c4d..1694af125 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -56,7 +56,7 @@ def get_domain_from_id(string): try: return string.split(":", 1)[1] except IndexError: - raise SynapseError(400, "Invalid ID: %r", string) + raise SynapseError(400, "Invalid ID: %r" % (string,)) class DomainSpecificString( From 2e9ee3096907573773d3f0e4ff22dd014b8253c8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 22 Sep 2016 11:59:46 +0100 Subject: [PATCH 3/4] Add comments --- synapse/api/auth.py | 3 +++ synapse/handlers/federation.py | 3 +++ 2 files changed, 6 insertions(+) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 377bfcc48..5bd250992 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -103,6 +103,9 @@ class Auth(object): # Check the sender's domain has signed the event if not event.signatures.get(sender_domain): + # We allow invites via 3pid to have a sender from a differnt + # HS, as the sender must match the sender of the original + # 3pid invite. This is checked further down. if not is_invite_via_3pid: raise AuthError(403, "Event not signed by sender's server") diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index a393263e1..2d801bad4 100644 --- a/synapse/handlers/federation.py +++ b/synapse/handlers/federation.py @@ -1930,6 +1930,9 @@ class FederationHandler(BaseHandler): "Could not find invite event for third_party_invite: %r", event_dict ) + # We don't discard here as this is not the appropriate place to do + # auth checks. If we need the invite and don't have it then the + # auth check code will explode appropriately. builder = self.event_builder_factory.new(event_dict) EventValidator().validate_new(builder) From f96020550ff7f03d1106f23ea049cb1a1b925574 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 22 Sep 2016 12:54:22 +0100 Subject: [PATCH 4/4] Update comments --- synapse/api/auth.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/api/auth.py b/synapse/api/auth.py index 5bd250992..e75fd518b 100644 --- a/synapse/api/auth.py +++ b/synapse/api/auth.py @@ -103,9 +103,10 @@ class Auth(object): # Check the sender's domain has signed the event if not event.signatures.get(sender_domain): - # We allow invites via 3pid to have a sender from a differnt + # We allow invites via 3pid to have a sender from a different # HS, as the sender must match the sender of the original - # 3pid invite. This is checked further down. + # 3pid invite. This is checked further down with the + # other dedicated membership checks. if not is_invite_via_3pid: raise AuthError(403, "Event not signed by sender's server")