From 8a3175f02243cb32a41e451129a7600a9f0b26b7 Mon Sep 17 00:00:00 2001 From: Andrew Morgan <1342360+anoadragon453@users.noreply.github.com> Date: Mon, 19 Aug 2019 15:22:30 +0100 Subject: [PATCH] Remove trusted_third_party_id_servers functionality (#5875) Part of https://github.com/matrix-org/synapse/pull/5835 Removes the concept of a trusted identity server. The original concept of having the homeserver keep a list of trusted identity servers was to mitigate the danger of having a malicious IS handling password reset or registration emails. Since #5835 gives the homeserver the ability to do both of these things itself, as well as the requirement for it to choose an external, trusted identity server if it so chooses, the homeserver no longer needs to constrain which identity servers are chosen (which was traditionally a choice given to the client). Thus, we can safely the functionality of `trusted_third_party_id_servers`. It does need to stay in the config file for the foreseeable though, as it is currently used by a background job for old 3PIDs, which were bound before Synapse tracked which IS a 3PID was bound to. The identity servers in `trusted_third_party_id_servers` are likely candidates to be where a user registered their 3PID, so this is used during the background update. This background job was added in v0.99.4, so we're catering for those still updating from before v0.99.4. --- changelog.d/5875.misc | 1 + contrib/cmdclient/console.py | 2 ++ docs/sample_config.yaml | 8 +++++++ synapse/config/registration.py | 8 +++++++ synapse/handlers/identity.py | 43 +--------------------------------- 5 files changed, 20 insertions(+), 42 deletions(-) create mode 100644 changelog.d/5875.misc diff --git a/changelog.d/5875.misc b/changelog.d/5875.misc new file mode 100644 index 000000000000..e188c28d2f84 --- /dev/null +++ b/changelog.d/5875.misc @@ -0,0 +1 @@ +Deprecate the `trusted_third_party_id_servers` option. \ No newline at end of file diff --git a/contrib/cmdclient/console.py b/contrib/cmdclient/console.py index af8f39c8c279..05743de68397 100755 --- a/contrib/cmdclient/console.py +++ b/contrib/cmdclient/console.py @@ -37,6 +37,8 @@ CONFIG_JSON = "cmdclient_config.json" +# TODO: The concept of trusted identity servers has been deprecated. This option and checks +# should be removed TRUSTED_ID_SERVERS = ["localhost:8001"] diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 0c6be30e513d..c208f7f4bd93 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -890,6 +890,14 @@ uploads_path: "DATADIR/uploads" # Also defines the ID server which will be called when an account is # deactivated (one will be picked arbitrarily). # +# Note: This option is deprecated. Since v0.99.4, Synapse has tracked which identity +# server a 3PID has been bound to. For 3PIDs bound before then, Synapse runs a +# background migration script, informing itself that the identity server all of its +# 3PIDs have been bound to is likely one of the below. +# +# As of Synapse v1.4.0, all other functionality of this option has been deprecated, and +# it is now solely used for the purposes of the background migration script, and can be +# removed once it has run. #trusted_third_party_id_servers: # - matrix.org # - vector.im diff --git a/synapse/config/registration.py b/synapse/config/registration.py index e2bee3c116b4..df3491568c1f 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -257,6 +257,14 @@ def generate_config_section(self, generate_secrets=False, **kwargs): # Also defines the ID server which will be called when an account is # deactivated (one will be picked arbitrarily). # + # Note: This option is deprecated. Since v0.99.4, Synapse has tracked which identity + # server a 3PID has been bound to. For 3PIDs bound before then, Synapse runs a + # background migration script, informing itself that the identity server all of its + # 3PIDs have been bound to is likely one of the below. + # + # As of Synapse v1.4.0, all other functionality of this option has been deprecated, and + # it is now solely used for the purposes of the background migration script, and can be + # removed once it has run. #trusted_third_party_id_servers: # - matrix.org # - vector.im diff --git a/synapse/handlers/identity.py b/synapse/handlers/identity.py index d199521b5878..f342ad1bfb5f 100644 --- a/synapse/handlers/identity.py +++ b/synapse/handlers/identity.py @@ -23,12 +23,7 @@ from twisted.internet import defer -from synapse.api.errors import ( - CodeMessageException, - Codes, - HttpResponseException, - SynapseError, -) +from synapse.api.errors import CodeMessageException, HttpResponseException, SynapseError from ._base import BaseHandler @@ -42,25 +37,6 @@ def __init__(self, hs): self.http_client = hs.get_simple_http_client() self.federation_http_client = hs.get_http_client() - self.trusted_id_servers = set(hs.config.trusted_third_party_id_servers) - self.trust_any_id_server_just_for_testing_do_not_use = ( - hs.config.use_insecure_ssl_client_just_for_testing_do_not_use - ) - - def _should_trust_id_server(self, id_server): - if id_server not in self.trusted_id_servers: - if self.trust_any_id_server_just_for_testing_do_not_use: - logger.warn( - "Trusting untrustworthy ID server %r even though it isn't" - " in the trusted id list for testing because" - " 'use_insecure_ssl_client_just_for_testing_do_not_use'" - " is set in the config", - id_server, - ) - else: - return False - return True - @defer.inlineCallbacks def threepid_from_creds(self, creds): if "id_server" in creds: @@ -77,13 +53,6 @@ def threepid_from_creds(self, creds): else: raise SynapseError(400, "No client_secret in creds") - if not self._should_trust_id_server(id_server): - logger.warn( - "%s is not a trusted ID server: rejecting 3pid " + "credentials", - id_server, - ) - return None - try: data = yield self.http_client.get_json( "https://%s%s" @@ -230,11 +199,6 @@ def try_unbind_threepid_with_id_server(self, mxid, threepid, id_server): def requestEmailToken( self, id_server, email, client_secret, send_attempt, next_link=None ): - if not self._should_trust_id_server(id_server): - raise SynapseError( - 400, "Untrusted ID server '%s'" % id_server, Codes.SERVER_NOT_TRUSTED - ) - params = { "email": email, "client_secret": client_secret, @@ -259,11 +223,6 @@ def requestEmailToken( def requestMsisdnToken( self, id_server, country, phone_number, client_secret, send_attempt, **kwargs ): - if not self._should_trust_id_server(id_server): - raise SynapseError( - 400, "Untrusted ID server '%s'" % id_server, Codes.SERVER_NOT_TRUSTED - ) - params = { "country": country, "phone_number": phone_number,