Merge branch 'develop' of github.com:matrix-org/synapse into matrix-org-hotfixes

27 files changed, 152 insertions, 89 deletions

diff --git a/.codecov.yml b/.codecov.yml
index a05698a39c..ef2e1eabfb 100644
--- a/.codecov.yml
+++ b/.codecov.yml
@@ -1,5 +1,4 @@
-comment:
-  layout: "diff"
+comment: off
 
 coverage:
   status:
diff --git a/changelog.d/5686.feature b/changelog.d/5686.feature
new file mode 100644
index 0000000000..367aa1eca2
--- /dev/null
+++ b/changelog.d/5686.feature
@@ -0,0 +1 @@
+Use `M_USER_DEACTIVATED` instead of `M_UNKNOWN` for errcode when a deactivated user attempts to login.
diff --git a/changelog.d/5787.misc b/changelog.d/5787.misc
new file mode 100644
index 0000000000..ead0b04b62
--- /dev/null
+++ b/changelog.d/5787.misc
@@ -0,0 +1 @@
+Remove DelayedCall debugging from the test suite, as it is no longer required in the vast majority of Synapse's tests.
diff --git a/changelog.d/5790.misc b/changelog.d/5790.misc
new file mode 100644
index 0000000000..3e9e435d7a
--- /dev/null
+++ b/changelog.d/5790.misc
@@ -0,0 +1 @@
+Remove some spurious exceptions from the logs where we failed to talk to a remote server.
diff --git a/changelog.d/5794.misc b/changelog.d/5794.misc
new file mode 100644
index 0000000000..720e0ddcfb
--- /dev/null
+++ b/changelog.d/5794.misc
@@ -0,0 +1 @@
+Improve performance when making `.well-known` requests by sharing the SSL options between requests.
diff --git a/changelog.d/5796.misc b/changelog.d/5796.misc
new file mode 100644
index 0000000000..be520946c7
--- /dev/null
+++ b/changelog.d/5796.misc
@@ -0,0 +1 @@
+Disable codecov GitHub comments on PRs.
diff --git a/changelog.d/5801.misc b/changelog.d/5801.misc
new file mode 100644
index 0000000000..e19854de82
--- /dev/null
+++ b/changelog.d/5801.misc
@@ -0,0 +1 @@
+Don't allow clients to send tombstone events that reference the room it's sent in.
diff --git a/changelog.d/5802.misc b/changelog.d/5802.misc
new file mode 100644
index 0000000000..de31192652
--- /dev/null
+++ b/changelog.d/5802.misc
@@ -0,0 +1 @@
+Deny redactions of events sent in a different room. 
diff --git a/changelog.d/5804.bugfix b/changelog.d/5804.bugfix
new file mode 100644
index 0000000000..75c17b460d
--- /dev/null
+++ b/changelog.d/5804.bugfix
@@ -0,0 +1 @@
+Fix check that tombstone is a state event in push rules.
diff --git a/changelog.d/5805.misc b/changelog.d/5805.misc
new file mode 100644
index 0000000000..352cb3db04
--- /dev/null
+++ b/changelog.d/5805.misc
@@ -0,0 +1 @@
+Deny sending well known state types as non-state events.
diff --git a/changelog.d/5806.bugfix b/changelog.d/5806.bugfix
new file mode 100644
index 0000000000..c5ca0f5629
--- /dev/null
+++ b/changelog.d/5806.bugfix
@@ -0,0 +1 @@
+Fix error when trying to login as a deactivated user when using a worker to handle login.
diff --git a/changelog.d/5808.misc b/changelog.d/5808.misc
new file mode 100644
index 0000000000..cac3fd34d1
--- /dev/null
+++ b/changelog.d/5808.misc
@@ -0,0 +1 @@
+Handle incorrectly encoded query params correctly by returning a 400.
diff --git a/changelog.d/5810.misc b/changelog.d/5810.misc
new file mode 100644
index 0000000000..0a5ccbbb3f
--- /dev/null
+++ b/changelog.d/5810.misc
@@ -0,0 +1 @@
+Return 502 not 500 when failing to reach any remote server.
diff --git a/synapse/api/errors.py b/synapse/api/errors.py
index ad3e262041..cf1ebf1af2 100644
--- a/synapse/api/errors.py
+++ b/synapse/api/errors.py
@@ -61,6 +61,7 @@ class Codes(object):
     INCOMPATIBLE_ROOM_VERSION = "M_INCOMPATIBLE_ROOM_VERSION"
     WRONG_ROOM_KEYS_VERSION = "M_WRONG_ROOM_KEYS_VERSION"
     EXPIRED_ACCOUNT = "ORG_MATRIX_EXPIRED_ACCOUNT"
+    USER_DEACTIVATED = "M_USER_DEACTIVATED"
 
 
 class CodeMessageException(RuntimeError):
@@ -151,7 +152,7 @@ class UserDeactivatedError(SynapseError):
             msg (str): The human-readable error message
         """
         super(UserDeactivatedError, self).__init__(
-            code=http_client.FORBIDDEN, msg=msg, errcode=Codes.UNKNOWN
+            code=http_client.FORBIDDEN, msg=msg, errcode=Codes.USER_DEACTIVATED
         )
 
 
diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py
index 4f48e8e88d..06e63a96b5 100644
--- a/synapse/crypto/context_factory.py
+++ b/synapse/crypto/context_factory.py
@@ -31,6 +31,7 @@ from twisted.internet.ssl import (
     platformTrust,
 )
 from twisted.python.failure import Failure
+from twisted.web.iweb import IPolicyForHTTPS
 
 logger = logging.getLogger(__name__)
 
@@ -74,6 +75,7 @@ class ServerContextFactory(ContextFactory):
         return self._context
 
 
+@implementer(IPolicyForHTTPS)
 class ClientTLSOptionsFactory(object):
     """Factory for Twisted SSLClientConnectionCreators that are used to make connections
     to remote servers for federation.
@@ -146,6 +148,12 @@ class ClientTLSOptionsFactory(object):
             f = Failure()
             tls_protocol.failVerification(f)
 
+    def creatorForNetloc(self, hostname, port):
+        """Implements the IPolicyForHTTPS interace so that this can be passed
+        directly to agents.
+        """
+        return self.get_options(hostname)
+
 
 @implementer(IOpenSSLClientConnectionCreator)
 class SSLClientConnectionCreator(object):
diff --git a/synapse/events/validator.py b/synapse/events/validator.py
index f7ffd1d561..272426e105 100644
--- a/synapse/events/validator.py
+++ b/synapse/events/validator.py
@@ -95,10 +95,10 @@ class EventValidator(object):
 
         elif event.type == EventTypes.Topic:
             self._ensure_strings(event.content, ["topic"])
-
+            self._ensure_state_event(event)
         elif event.type == EventTypes.Name:
             self._ensure_strings(event.content, ["name"])
-
+            self._ensure_state_event(event)
         elif event.type == EventTypes.Member:
             if "membership" not in event.content:
                 raise SynapseError(400, "Content has not membership key")
@@ -106,9 +106,25 @@ class EventValidator(object):
             if event.content["membership"] not in Membership.LIST:
                 raise SynapseError(400, "Invalid membership key")
 
+            self._ensure_state_event(event)
+        elif event.type == EventTypes.Tombstone:
+            if "replacement_room" not in event.content:
+                raise SynapseError(400, "Content has no replacement_room key")
+
+            if event.content["replacement_room"] == event.room_id:
+                raise SynapseError(
+                    400, "Tombstone cannot reference the room it was sent in"
+                )
+
+            self._ensure_state_event(event)
+
     def _ensure_strings(self, d, keys):
         for s in keys:
             if s not in d:
                 raise SynapseError(400, "'%s' not in content" % (s,))
             if not isinstance(d[s], string_types):
                 raise SynapseError(400, "'%s' not a string type" % (s,))
+
+    def _ensure_state_event(self, event):
+        if not event.is_state():
+            raise SynapseError(400, "'%s' must be state events" % (event.type,))
diff --git a/synapse/federation/federation_client.py b/synapse/federation/federation_client.py
index 6e03ce21af..bec3080895 100644
--- a/synapse/federation/federation_client.py
+++ b/synapse/federation/federation_client.py
@@ -511,9 +511,8 @@ class FederationClient(FederationBase):
             The [Deferred] result of callback, if it succeeds
 
         Raises:
-            SynapseError if the chosen remote server returns a 300/400 code.
-
-            RuntimeError if no servers were reachable.
+            SynapseError if the chosen remote server returns a 300/400 code, or
+            no servers were reachable.
         """
         for destination in destinations:
             if destination == self.server_name:
@@ -538,7 +537,7 @@ class FederationClient(FederationBase):
             except Exception:
                 logger.warn("Failed to %s via %s", description, destination, exc_info=1)
 
-        raise RuntimeError("Failed to %s via any server" % (description,))
+        raise SynapseError(502, "Failed to %s via any server" % (description,))
 
     def make_membership_event(
         self, destinations, room_id, user_id, membership, content, params
diff --git a/synapse/handlers/directory.py b/synapse/handlers/directory.py
index 0fd423197c..526379c6f7 100644
--- a/synapse/handlers/directory.py
+++ b/synapse/handlers/directory.py
@@ -278,7 +278,6 @@ class DirectoryHandler(BaseHandler):
             servers = list(servers)
 
         return {"room_id": room_id, "servers": servers}
-        return
 
     @defer.inlineCallbacks
     def on_directory_query(self, args):
diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py
index 10160bfe86..c86903b98b 100644
--- a/synapse/handlers/federation.py
+++ b/synapse/handlers/federation.py
@@ -978,6 +978,9 @@ class FederationHandler(BaseHandler):
                 except NotRetryingDestination as e:
                     logger.info(str(e))
                     continue
+                except RequestSendFailed as e:
+                    logger.info("Falied to get backfill from %s because %s", dom, e)
+                    continue
                 except FederationDeniedError as e:
                     logger.info(e)
                     continue
diff --git a/synapse/handlers/groups_local.py b/synapse/handlers/groups_local.py
index 7b67c8ae0f..46eb9ee88b 100644
--- a/synapse/handlers/groups_local.py
+++ b/synapse/handlers/groups_local.py
@@ -126,9 +126,12 @@ class GroupsLocalHandler(object):
                 group_id, requester_user_id
             )
         else:
-            res = yield self.transport_client.get_group_summary(
-                get_domain_from_id(group_id), group_id, requester_user_id
-            )
+            try:
+                res = yield self.transport_client.get_group_summary(
+                    get_domain_from_id(group_id), group_id, requester_user_id
+                )
+            except RequestSendFailed:
+                raise SynapseError(502, "Failed to contact group server")
 
             group_server_name = get_domain_from_id(group_id)
 
@@ -183,9 +186,12 @@ class GroupsLocalHandler(object):
 
             content["user_profile"] = yield self.profile_handler.get_profile(user_id)
 
-            res = yield self.transport_client.create_group(
-                get_domain_from_id(group_id), group_id, user_id, content
-            )
+            try:
+                res = yield self.transport_client.create_group(
+                    get_domain_from_id(group_id), group_id, user_id, content
+                )
+            except RequestSendFailed:
+                raise SynapseError(502, "Failed to contact group server")
 
             remote_attestation = res["attestation"]
             yield self.attestations.verify_attestation(
@@ -221,9 +227,12 @@ class GroupsLocalHandler(object):
 
         group_server_name = get_domain_from_id(group_id)
 
-        res = yield self.transport_client.get_users_in_group(
-            get_domain_from_id(group_id), group_id, requester_user_id
-        )
+        try:
+            res = yield self.transport_client.get_users_in_group(
+                get_domain_from_id(group_id), group_id, requester_user_id
+            )
+        except RequestSendFailed:
+            raise SynapseError(502, "Failed to contact group server")
 
         chunk = res["chunk"]
         valid_entries = []
@@ -258,9 +267,12 @@ class GroupsLocalHandler(object):
             local_attestation = self.attestations.create_attestation(group_id, user_id)
             content["attestation"] = local_attestation
 
-            res = yield self.transport_client.join_group(
-                get_domain_from_id(group_id), group_id, user_id, content
-            )
+            try:
+                res = yield self.transport_client.join_group(
+                    get_domain_from_id(group_id), group_id, user_id, content
+                )
+            except RequestSendFailed:
+                raise SynapseError(502, "Failed to contact group server")
 
             remote_attestation = res["attestation"]
 
@@ -299,9 +311,12 @@ class GroupsLocalHandler(object):
             local_attestation = self.attestations.create_attestation(group_id, user_id)
             content["attestation"] = local_attestation
 
-            res = yield self.transport_client.accept_group_invite(
-                get_domain_from_id(group_id), group_id, user_id, content
-            )
+            try:
+                res = yield self.transport_client.accept_group_invite(
+                    get_domain_from_id(group_id), group_id, user_id, content
+                )
+            except RequestSendFailed:
+                raise SynapseError(502, "Failed to contact group server")
 
             remote_attestation = res["attestation"]
 
@@ -338,13 +353,16 @@ class GroupsLocalHandler(object):
                 group_id, user_id, requester_user_id, content
             )
         else:
-            res = yield self.transport_client.invite_to_group(
-                get_domain_from_id(group_id),
-                group_id,
-                user_id,
-                requester_user_id,
-                content,
-            )
+            try:
+                res = yield self.transport_client.invite_to_group(
+                    get_domain_from_id(group_id),
+                    group_id,
+                    user_id,
+                    requester_user_id,
+                    content,
+                )
+            except RequestSendFailed:
+                raise SynapseError(502, "Failed to contact group server")
 
         return res
 
@@ -398,13 +416,16 @@ class GroupsLocalHandler(object):
             )
         else:
             content["requester_user_id"] = requester_user_id
-            res = yield self.transport_client.remove_user_from_group(
-                get_domain_from_id(group_id),
-                group_id,
-                requester_user_id,
-                user_id,
-                content,
-            )
+            try:
+                res = yield self.transport_client.remove_user_from_group(
+                    get_domain_from_id(group_id),
+                    group_id,
+                    requester_user_id,
+                    user_id,
+                    content,
+                )
+            except RequestSendFailed:
+                raise SynapseError(502, "Failed to contact group server")
 
         return res
 
@@ -435,9 +456,13 @@ class GroupsLocalHandler(object):
 
             return {"groups": result}
         else:
-            bulk_result = yield self.transport_client.bulk_get_publicised_groups(
-                get_domain_from_id(user_id), [user_id]
-            )
+            try:
+                bulk_result = yield self.transport_client.bulk_get_publicised_groups(
+                    get_domain_from_id(user_id), [user_id]
+                )
+            except RequestSendFailed:
+                raise SynapseError(502, "Failed to contact group server")
+
             result = bulk_result.get("users", {}).get(user_id)
             # TODO: Verify attestations
             return {"groups": result}
diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py
index 77ca34d08a..0bd1e31440 100644
--- a/synapse/handlers/message.py
+++ b/synapse/handlers/message.py
@@ -795,7 +795,6 @@ class EventCreationHandler(object):
                 get_prev_content=False,
                 allow_rejected=False,
                 allow_none=True,
-                check_room_id=event.room_id,
             )
 
             # we can make some additional checks now if we have the original event.
@@ -803,6 +802,9 @@ class EventCreationHandler(object):
                 if original_event.type == EventTypes.Create:
                     raise AuthError(403, "Redacting create events is not permitted")
 
+                if original_event.room_id != event.room_id:
+                    raise SynapseError(400, "Cannot redact event from a different room")
+
             prev_state_ids = yield context.get_prev_state_ids(self.store)
             auth_events_ids = yield self.auth.compute_auth_events(
                 event, prev_state_ids, for_verification=True
diff --git a/synapse/http/federation/matrix_federation_agent.py b/synapse/http/federation/matrix_federation_agent.py
index c03ddb724f..a0d5139839 100644
--- a/synapse/http/federation/matrix_federation_agent.py
+++ b/synapse/http/federation/matrix_federation_agent.py
@@ -64,10 +64,6 @@ class MatrixFederationAgent(object):
         tls_client_options_factory (ClientTLSOptionsFactory|None):
             factory to use for fetching client tls options, or none to disable TLS.
 
-        _well_known_tls_policy (IPolicyForHTTPS|None):
-            TLS policy to use for fetching .well-known files. None to use a default
-            (browser-like) implementation.
-
         _srv_resolver (SrvResolver|None):
             SRVResolver impl to use for looking up SRV records. None to use a default
             implementation.
@@ -81,7 +77,6 @@ class MatrixFederationAgent(object):
         self,
         reactor,
         tls_client_options_factory,
-        _well_known_tls_policy=None,
         _srv_resolver=None,
         _well_known_cache=well_known_cache,
     ):
@@ -98,13 +93,12 @@ class MatrixFederationAgent(object):
         self._pool.maxPersistentPerHost = 5
         self._pool.cachedConnectionTimeout = 2 * 60
 
-        agent_args = {}
-        if _well_known_tls_policy is not None:
-            # the param is called 'contextFactory', but actually passing a
-            # contextfactory is deprecated, and it expects an IPolicyForHTTPS.
-            agent_args["contextFactory"] = _well_known_tls_policy
         _well_known_agent = RedirectAgent(
-            Agent(self._reactor, pool=self._pool, **agent_args)
+            Agent(
+                self._reactor,
+                pool=self._pool,
+                contextFactory=tls_client_options_factory,
+            )
         )
         self._well_known_agent = _well_known_agent
 
diff --git a/synapse/http/servlet.py b/synapse/http/servlet.py
index f0ca7d9aba..fd07bf7b8e 100644
--- a/synapse/http/servlet.py
+++ b/synapse/http/servlet.py
@@ -166,7 +166,12 @@ def parse_string_from_args(
         value = args[name][0]
 
         if encoding:
-            value = value.decode(encoding)
+            try:
+                value = value.decode(encoding)
+            except ValueError:
+                raise SynapseError(
+                    400, "Query parameter %r must be %s" % (name, encoding)
+                )
 
         if allowed_values is not None and value not in allowed_values:
             message = "Query parameter %r must be one of [%s]" % (
diff --git a/synapse/push/baserules.py b/synapse/push/baserules.py
index 134bf805eb..286374d0b5 100644
--- a/synapse/push/baserules.py
+++ b/synapse/push/baserules.py
@@ -245,7 +245,13 @@ BASE_APPEND_OVERRIDE_RULES = [
                 "key": "type",
                 "pattern": "m.room.tombstone",
                 "_id": "_tombstone",
-            }
+            },
+            {
+                "kind": "event_match",
+                "key": "state_key",
+                "pattern": "",
+                "_id": "_tombstone_statekey",
+            },
         ],
         "actions": ["notify", {"set_tweak": "highlight", "value": True}],
     },
diff --git a/synapse/storage/registration.py b/synapse/storage/registration.py
index 999c10a308..55e4e84d71 100644
--- a/synapse/storage/registration.py
+++ b/synapse/storage/registration.py
@@ -569,6 +569,27 @@ class RegistrationWorkerStore(SQLBaseStore):
             desc="get_id_servers_user_bound",
         )
 
+    @cachedInlineCallbacks()
+    def get_user_deactivated_status(self, user_id):
+        """Retrieve the value for the `deactivated` property for the provided user.
+
+        Args:
+            user_id (str): The ID of the user to retrieve the status for.
+
+        Returns:
+            defer.Deferred(bool): The requested value.
+        """
+
+        res = yield self._simple_select_one_onecol(
+            table="users",
+            keyvalues={"name": user_id},
+            retcol="deactivated",
+            desc="get_user_deactivated_status",
+        )
+
+        # Convert the integer into a boolean.
+        return res == 1
+
 
 class RegistrationStore(
     RegistrationWorkerStore, background_updates.BackgroundUpdateStore
@@ -1317,24 +1338,3 @@ class RegistrationStore(
             user_id,
             deactivated,
         )
-
-    @cachedInlineCallbacks()
-    def get_user_deactivated_status(self, user_id):
-        """Retrieve the value for the `deactivated` property for the provided user.
-
-        Args:
-            user_id (str): The ID of the user to retrieve the status for.
-
-        Returns:
-            defer.Deferred(bool): The requested value.
-        """
-
-        res = yield self._simple_select_one_onecol(
-            table="users",
-            keyvalues={"name": user_id},
-            retcol="deactivated",
-            desc="get_user_deactivated_status",
-        )
-
-        # Convert the integer into a boolean.
-        return res == 1
diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py
index b906686b49..4255add097 100644
--- a/tests/http/federation/test_matrix_federation_agent.py
+++ b/tests/http/federation/test_matrix_federation_agent.py
@@ -75,7 +75,6 @@ class MatrixFederationAgentTests(TestCase):
 
         config_dict = default_config("test", parse=False)
         config_dict["federation_custom_ca_list"] = [get_test_ca_cert_file()]
-        # config_dict["trusted_key_servers"] = []
 
         self._config = config = HomeServerConfig()
         config.parse_config_dict(config_dict, "", "")
@@ -83,7 +82,6 @@ class MatrixFederationAgentTests(TestCase):
         self.agent = MatrixFederationAgent(
             reactor=self.reactor,
             tls_client_options_factory=ClientTLSOptionsFactory(config),
-            _well_known_tls_policy=TrustingTLSPolicyForHTTPS(),
             _srv_resolver=self.mock_resolver,
             _well_known_cache=self.well_known_cache,
         )
@@ -691,16 +689,18 @@ class MatrixFederationAgentTests(TestCase):
         not signed by a CA
         """
 
-        # we use the same test server as the other tests, but use an agent
-        # with _well_known_tls_policy left to the default, which will not
-        # trust it (since the presented cert is signed by a test CA)
+        # we use the same test server as the other tests, but use an agent with
+        # the config left to the default, which will not trust it (since the
+        # presented cert is signed by a test CA)
 
         self.mock_resolver.resolve_service.side_effect = lambda _: []
         self.reactor.lookups["testserv"] = "1.2.3.4"
 
+        config = default_config("test", parse=True)
+
         agent = MatrixFederationAgent(
             reactor=self.reactor,
-            tls_client_options_factory=ClientTLSOptionsFactory(self._config),
+            tls_client_options_factory=ClientTLSOptionsFactory(config),
             _srv_resolver=self.mock_resolver,
             _well_known_cache=self.well_known_cache,
         )
diff --git a/tests/unittest.py b/tests/unittest.py
index f5fae21317..561cebc223 100644
--- a/tests/unittest.py
+++ b/tests/unittest.py
@@ -23,8 +23,6 @@ from mock import Mock
 
 from canonicaljson import json
 
-import twisted
-import twisted.logger
 from twisted.internet.defer import Deferred, succeed
 from twisted.python.threadpool import ThreadPool
 from twisted.trial import unittest
@@ -80,10 +78,6 @@ class TestCase(unittest.TestCase):
 
         @around(self)
         def setUp(orig):
-            # enable debugging of delayed calls - this means that we get a
-            # traceback when a unit test exits leaving things on the reactor.
-            twisted.internet.base.DelayedCall.debug = True
-
             # if we're not starting in the sentinel logcontext, then to be honest
             # all future bets are off.
             if LoggingContext.current_context() is not LoggingContext.sentinel: