From aecae8f3973803dcdbe93bcc1c5b9022f2c38ddd Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Mon, 29 Jul 2019 17:21:57 +0100 Subject: [PATCH 01/24] Correctly handle errors doing requests to group servers --- synapse/handlers/groups_local.py | 89 ++++++++++++++++++++------------ 1 file changed, 57 insertions(+), 32 deletions(-) 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} From b4d5ff0af73d40f8daad631b33e38e8d7c472bc1 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 30 Jul 2019 13:19:22 +0100 Subject: [PATCH 02/24] Don't log as exception when failing durig backfill --- synapse/handlers/federation.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/synapse/handlers/federation.py b/synapse/handlers/federation.py index 89b37dbc1c..c70f12092a 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 From f92d05e25487fdca22961847611e598006d17252 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 30 Jul 2019 13:42:54 +0100 Subject: [PATCH 03/24] Newsfile --- changelog.d/5790.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5790.misc 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. From a9bcae9f50a14dc020634c532732c1a86b696d92 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 30 Jul 2019 17:42:56 +0100 Subject: [PATCH 04/24] Share SSL options for well-known requests --- synapse/crypto/context_factory.py | 8 ++++++++ .../http/federation/matrix_federation_agent.py | 16 +++++----------- .../federation/test_matrix_federation_agent.py | 12 ++++++------ 3 files changed, 19 insertions(+), 17 deletions(-) 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/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/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, ) From 3b7a35a59a29c2700171546cd052dd8d506f3330 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 31 Jul 2019 10:31:00 +0100 Subject: [PATCH 05/24] Newsfile --- changelog.d/5794.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5794.misc 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. From 6be336c0d8b1203744ae745add847f01f21e1246 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 31 Jul 2019 10:56:02 +0100 Subject: [PATCH 06/24] Disable codecov reports to GH comments. The double posting is really annoying, and I don't think anyone is actually reading them. The commit statuses should give a good summary and will link to a full report. --- .codecov.yml | 3 +-- 1 file changed, 1 insertion(+), 2 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: From fe2f2fc530e3ea832d30a6964322a2b8d3691e55 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 31 Jul 2019 10:59:39 +0100 Subject: [PATCH 07/24] Newsfile --- changelog.d/5796.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5796.misc 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. From 8f15832950148280ed5a9cf28b74971abfd09e4f Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Wed, 31 Jul 2019 20:39:22 +1000 Subject: [PATCH 08/24] Remove DelayedCall debugging from test runs (#5787) --- changelog.d/5787.misc | 1 + tests/unittest.py | 6 ------ 2 files changed, 1 insertion(+), 6 deletions(-) create mode 100644 changelog.d/5787.misc 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/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: From 58a755cdc36d94fa25c086576748bf44d978cf5e Mon Sep 17 00:00:00 2001 From: Andrew Morgan Date: Wed, 31 Jul 2019 13:24:51 +0100 Subject: [PATCH 09/24] Remove duplicate return statement --- synapse/handlers/directory.py | 1 - 1 file changed, 1 deletion(-) 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): From 72167fb39474517da836170b79d03726f78e35e2 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Wed, 31 Jul 2019 15:19:06 +0100 Subject: [PATCH 10/24] Change user deactivated errcode to USER_DEACTIVATED and use it (#5686) This is intended as an amendment to #5674 as using M_UNKNOWN as the errcode makes it hard for clients to differentiate between an invalid password and a deactivated user (the problem we were trying to solve in the first place). M_UNKNOWN was originally chosen as it was presumed than an MSC would have to be carried out to add a new code, but as Synapse often is the testing bed for new MSC implementations, it makes sense to try it out first in the wild and then add it into the spec if it is successful. Thus this PR return a new M_USER_DEACTIVATED code when a deactivated user attempts to login. --- changelog.d/5686.feature | 1 + synapse/api/errors.py | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) create mode 100644 changelog.d/5686.feature 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/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 ) From f31d4cb7a2e90b337f60ef06a3d31c0be9ad667c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 31 Jul 2019 15:52:27 +0100 Subject: [PATCH 11/24] Don't allow clients to send tombstones that reference the same room --- synapse/events/validator.py | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/synapse/events/validator.py b/synapse/events/validator.py index f7ffd1d561..29f99361c0 100644 --- a/synapse/events/validator.py +++ b/synapse/events/validator.py @@ -106,6 +106,13 @@ class EventValidator(object): if event.content["membership"] not in Membership.LIST: raise SynapseError(400, "Invalid membership key") + 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 itself") + def _ensure_strings(self, d, keys): for s in keys: if s not in d: From 02735e140f4b1e36ae29be15511a7c08cd74364e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 31 Jul 2019 15:53:52 +0100 Subject: [PATCH 12/24] Newsfile --- changelog.d/5801.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5801.misc diff --git a/changelog.d/5801.misc b/changelog.d/5801.misc new file mode 100644 index 0000000000..e6ecb475d9 --- /dev/null +++ b/changelog.d/5801.misc @@ -0,0 +1 @@ +Don't allow clients to send tombstone events that reference the room its sent in. From cf89266b980b62a6d8547f8e1ae9394359a05fc8 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 31 Jul 2019 16:03:14 +0100 Subject: [PATCH 13/24] Deny redaction of events in a different room. We already correctly filter out such redactions, but we should also deny them over the CS API. --- synapse/handlers/message.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/message.py b/synapse/handlers/message.py index e951c39fa7..a5e23c4caf 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 From 0eefb76fa1a4348a50843097a92ead108cec398c Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 31 Jul 2019 16:13:57 +0100 Subject: [PATCH 14/24] Newsfile --- changelog.d/5802.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5802.misc 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. From 2e697d30134c2d1b8a8d98775aa72657186deb76 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 31 Jul 2019 16:22:38 +0100 Subject: [PATCH 15/24] Explicitly check that tombstone is a state event before notifying. --- synapse/push/baserules.py | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) 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}], }, From c5288e9984d87c1d1257094f0493da15a9e08962 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 31 Jul 2019 16:28:40 +0100 Subject: [PATCH 16/24] Newsfile --- changelog.d/5804.bugfix | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5804.bugfix 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. From dc4d74e44adbd8fc79bbaa7ac44b430a11454173 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 31 Jul 2019 16:36:20 +0100 Subject: [PATCH 17/24] Validate well-known state events are state events. Lets disallow sending things like memberships, topics etc as non-state events. --- synapse/events/validator.py | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/synapse/events/validator.py b/synapse/events/validator.py index 29f99361c0..0cf2b9ba42 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,6 +106,7 @@ 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") @@ -113,9 +114,15 @@ class EventValidator(object): if event.content["replacement_room"] == event.room_id: raise SynapseError(400, "Tombstone cannot reference itself") + 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,)) From e5a0224837544142a2d78cae1c68c9c8023e1c32 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 31 Jul 2019 16:39:42 +0100 Subject: [PATCH 18/24] Newsfile --- changelog.d/5805.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5805.misc 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. From 76a58fdcced5d152efee48f69b6ab658e0e6cbc5 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 1 Aug 2019 13:14:25 +0100 Subject: [PATCH 19/24] Fix spelling. Co-Authored-By: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> --- changelog.d/5801.misc | 2 +- synapse/events/validator.py | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/changelog.d/5801.misc b/changelog.d/5801.misc index e6ecb475d9..e19854de82 100644 --- a/changelog.d/5801.misc +++ b/changelog.d/5801.misc @@ -1 +1 @@ -Don't allow clients to send tombstone events that reference the room its sent in. +Don't allow clients to send tombstone events that reference the room it's sent in. diff --git a/synapse/events/validator.py b/synapse/events/validator.py index 29f99361c0..6374dd067d 100644 --- a/synapse/events/validator.py +++ b/synapse/events/validator.py @@ -111,7 +111,9 @@ class EventValidator(object): raise SynapseError(400, "Content has no replacement_room key") if event.content["replacement_room"] == event.room_id: - raise SynapseError(400, "Tombstone cannot reference itself") + raise SynapseError( + 400, "Tombstone cannot reference the room it was sent in" + ) def _ensure_strings(self, d, keys): for s in keys: From d2e3d5b9db346c88b31ff5eef2793c5cf82f698e Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 1 Aug 2019 13:23:00 +0100 Subject: [PATCH 20/24] Handle incorrectly encoded query params correctly --- synapse/http/servlet.py | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) 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]" % ( From da378af44517d96d461431c21400e44b00edb285 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 1 Aug 2019 13:24:00 +0100 Subject: [PATCH 21/24] Newsfile --- changelog.d/5808.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5808.misc 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. From a8f40a8302fb1b9c95287d87a56440bb9b201435 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 1 Aug 2019 13:47:31 +0100 Subject: [PATCH 22/24] Return 502 not 500 when failing to reach any remote server. --- synapse/federation/federation_client.py | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) 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 From 93fd3cbc7a130c31c320fb6c4b4db1477ea827d1 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 1 Aug 2019 13:48:52 +0100 Subject: [PATCH 23/24] Newsfile --- changelog.d/5810.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5810.misc 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. From 5d018d23f01afe995c435d1d2951a0e7f5b5feb1 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Thu, 1 Aug 2019 13:54:56 +0100 Subject: [PATCH 24/24] Have ClientReaderSlavedStore inherit RegistrationStore (#5806) Fixes #5803 --- changelog.d/5806.bugfix | 1 + synapse/storage/registration.py | 42 ++++++++++++++++----------------- 2 files changed, 22 insertions(+), 21 deletions(-) create mode 100644 changelog.d/5806.bugfix 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/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