Merge pull request #919 from matrix-org/erikj/auth_fix

Various auth.py fixes.

3 files changed, 35 insertions, 5 deletions

diff --git a/synapse/api/auth.py b/synapse/api/auth.py
index 521a52e001..3d1ce4e09e 100644
--- a/synapse/api/auth.py
+++ b/synapse/api/auth.py
@@ -63,7 +63,7 @@ class Auth(object):
             "user_id = ",
         ])
 
-    def check(self, event, auth_events):
+    def check(self, event, auth_events, do_sig_check=True):
         """ Checks if this event is correctly authed.
 
         Args:
@@ -79,6 +79,13 @@ 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)
+
+            # Check the sender's domain has signed the event
+            if do_sig_check and not event.signatures.get(sender_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
                 # are trusting that this is allowed (at least for now)
@@ -86,6 +93,12 @@ class Auth(object):
                 return True
 
             if event.type == EventTypes.Create:
+                room_id_domain = get_domain_from_id(event.room_id)
+                if room_id_domain != sender_domain:
+                    raise AuthError(
+                        403,
+                        "Creation event's room_id domain does not match sender's"
+                    )
                 # FIXME
                 return True
 
@@ -108,6 +121,17 @@ class Auth(object):
 
             # FIXME: Temp hack
             if event.type == EventTypes.Aliases:
+                if not event.state_key:
+                    raise AuthError(
+                        403,
+                        "Alias event must have non-empty state_key"
+                    )
+                sender_domain = get_domain_from_id(event.sender)
+                if event.state_key != sender_domain:
+                    raise AuthError(
+                        403,
+                        "Alias event's state_key does not match sender's domain"
+                    )
                 return True
 
             logger.debug(
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 351b218247..7622962d46 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -688,7 +688,9 @@ class FederationHandler(BaseHandler):
             logger.warn("Failed to create join %r because %s", event, e)
             raise e
 
-        self.auth.check(event, auth_events=context.current_state)
+        # The remote hasn't signed it yet, obviously. We'll do the full checks
+        # when we get the event back in `on_send_join_request`
+        self.auth.check(event, auth_events=context.current_state, do_sig_check=False)
 
         defer.returnValue(event)
 
@@ -918,7 +920,9 @@ class FederationHandler(BaseHandler):
         )
 
         try:
-            self.auth.check(event, auth_events=context.current_state)
+            # The remote hasn't signed it yet, obviously. We'll do the full checks
+            # when we get the event back in `on_send_leave_request`
+            self.auth.check(event, auth_events=context.current_state, do_sig_check=False)
         except AuthError as e:
             logger.warn("Failed to create new leave %r because %s", event, e)
             raise e
diff --git a/synapse/state.py b/synapse/state.py
index d0f76dc4f5..ef1bc470be 100644
--- a/synapse/state.py
+++ b/synapse/state.py
@@ -379,7 +379,8 @@ class StateHandler(object):
             try:
                 # FIXME: hs.get_auth() is bad style, but we need to do it to
                 # get around circular deps.
-                self.hs.get_auth().check(event, auth_events)
+                # The signatures have already been checked at this point
+                self.hs.get_auth().check(event, auth_events, do_sig_check=False)
                 prev_event = event
             except AuthError:
                 return prev_event
@@ -391,7 +392,8 @@ class StateHandler(object):
             try:
                 # FIXME: hs.get_auth() is bad style, but we need to do it to
                 # get around circular deps.
-                self.hs.get_auth().check(event, auth_events)
+                # The signatures have already been checked at this point
+                self.hs.get_auth().check(event, auth_events, do_sig_check=False)
                 return event
             except AuthError:
                 pass