diff options
30 files changed, 175 insertions, 90 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/5793.misc b/changelog.d/5793.misc new file mode 100644 index 0000000000..5e15dfd5fa --- /dev/null +++ b/changelog.d/5793.misc @@ -0,0 +1 @@ +Reduce database IO usage by optimising queries for current membership. 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 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 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/synapse/storage/schema/delta/56/current_state_events_membership.sql b/synapse/storage/schema/delta/56/current_state_events_membership.sql new file mode 100644 index 0000000000..473018676f --- /dev/null +++ b/synapse/storage/schema/delta/56/current_state_events_membership.sql @@ -0,0 +1,22 @@ +/* Copyright 2019 The Matrix.org Foundation C.I.C. + * + * 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. + */ + +-- We add membership to current state so that we don't need to join against +-- room_memberships, which can be surprisingly costly (we do such queries +-- very frequently). +-- This will be null for non-membership events and the content.membership key +-- for membership events. (Will also be null for membership events until the +-- background update job has finished). +ALTER TABLE current_state_events ADD membership TEXT; diff --git a/synapse/storage/schema/delta/56/current_state_events_membership_mk2.sql b/synapse/storage/schema/delta/56/current_state_events_membership_mk2.sql index b2e08cd85d..3133d42d4a 100644 --- a/synapse/storage/schema/delta/56/current_state_events_membership_mk2.sql +++ b/synapse/storage/schema/delta/56/current_state_events_membership_mk2.sql @@ -19,7 +19,6 @@ -- This will be null for non-membership events and the content.membership key -- for membership events. (Will also be null for membership events until the -- background update job has finished). -ALTER TABLE current_state_events ADD membership TEXT; INSERT INTO background_updates (update_name, progress_json) VALUES ('current_state_events_membership', '{}'); 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: |