From a9bcae9f50a14dc020634c532732c1a86b696d92 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 30 Jul 2019 17:42:56 +0100 Subject: Share SSL options for well-known requests --- synapse/crypto/context_factory.py | 8 ++++++++ 1 file changed, 8 insertions(+) (limited to 'synapse/crypto') 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): -- cgit 1.4.1 From 5906be858900e134d99dd94f0ca9e8bd1db14c05 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 20 Aug 2019 15:27:08 +0100 Subject: Add config option for keys to use to sign keys This allows servers to separate keys that are used to sign remote keys when acting as a notary server. --- docs/sample_config.yaml | 8 ++++++++ synapse/config/key.py | 35 +++++++++++++++++++++++++++++++---- synapse/crypto/keyring.py | 12 +++++++----- 3 files changed, 46 insertions(+), 9 deletions(-) (limited to 'synapse/crypto') diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 0c6be30e51..c96eb0cf2d 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1027,6 +1027,14 @@ signing_key_path: "CONFDIR/SERVERNAME.signing.key" # #trusted_key_servers: # - server_name: "matrix.org" +# + +# The additional signing keys to use when acting as a trusted key server, on +# top of the normal signing keys. +# +# Can contain multiple keys, one per line. +# +#key_server_signing_keys_path: "key_server_signing_keys.key" # Enable SAML2 for registration and login. Uses pysaml2. diff --git a/synapse/config/key.py b/synapse/config/key.py index fe8386985c..f1a1efcb7f 100644 --- a/synapse/config/key.py +++ b/synapse/config/key.py @@ -76,7 +76,7 @@ class KeyConfig(Config): config_dir_path, config["server_name"] + ".signing.key" ) - self.signing_key = self.read_signing_key(signing_key_path) + self.signing_key = self.read_signing_keys(signing_key_path, "signing_key") self.old_signing_keys = self.read_old_signing_keys( config.get("old_signing_keys", {}) @@ -85,6 +85,15 @@ class KeyConfig(Config): config.get("key_refresh_interval", "1d") ) + self.key_server_signing_keys = list(self.signing_key) + key_server_signing_keys_path = config.get("key_server_signing_keys_path") + if key_server_signing_keys_path: + self.key_server_signing_keys.extend( + self.read_signing_keys( + key_server_signing_keys_path, "key_server_signing_keys_path" + ) + ) + # if neither trusted_key_servers nor perspectives are given, use the default. if "perspectives" not in config and "trusted_key_servers" not in config: key_servers = [{"server_name": "matrix.org"}] @@ -210,16 +219,34 @@ class KeyConfig(Config): # #trusted_key_servers: # - server_name: "matrix.org" + # + + # The additional signing keys to use when acting as a trusted key server, on + # top of the normal signing keys. + # + # Can contain multiple keys, one per line. + # + #key_server_signing_keys_path: "key_server_signing_keys.key" """ % locals() ) - def read_signing_key(self, signing_key_path): - signing_keys = self.read_file(signing_key_path, "signing_key") + def read_signing_keys(self, signing_key_path, name): + """Read the signing keys in the given path. + + Args: + signing_key_path (str) + name (str): Associated config key name + + Returns: + list[SigningKey] + """ + + signing_keys = self.read_file(signing_key_path, name) try: return read_signing_keys(signing_keys.splitlines(True)) except Exception as e: - raise ConfigError("Error reading signing_key: %s" % (str(e))) + raise ConfigError("Error reading %s: %s" % (name, str(e))) def read_old_signing_keys(self, old_signing_keys): keys = {} diff --git a/synapse/crypto/keyring.py b/synapse/crypto/keyring.py index 6c3e885e72..a3b55e349e 100644 --- a/synapse/crypto/keyring.py +++ b/synapse/crypto/keyring.py @@ -540,11 +540,13 @@ class BaseV2KeyFetcher(object): verify_key=verify_key, valid_until_ts=key_data["expired_ts"] ) - # re-sign the json with our own key, so that it is ready if we are asked to - # give it out as a notary server - signed_key_json = sign_json( - response_json, self.config.server_name, self.config.signing_key[0] - ) + # re-sign the json with our own keys, so that it is ready if we are + # asked to give it out as a notary server + signed_key_json = response_json + for signing_key in self.config.key_server_signing_keys: + signed_key_json = sign_json( + signed_key_json, self.config.server_name, signing_key + ) signed_key_json_bytes = encode_canonical_json(signed_key_json) -- cgit 1.4.1 From 97cbc96093dcd878bc823f34d71437a08786a3e4 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 21 Aug 2019 10:39:45 +0100 Subject: Only sign when we respond to remote key requests --- synapse/crypto/keyring.py | 11 +---------- synapse/rest/key/v2/remote_key_resource.py | 28 +++++++++++++++------------- 2 files changed, 16 insertions(+), 23 deletions(-) (limited to 'synapse/crypto') diff --git a/synapse/crypto/keyring.py b/synapse/crypto/keyring.py index a3b55e349e..abeb0ac26e 100644 --- a/synapse/crypto/keyring.py +++ b/synapse/crypto/keyring.py @@ -30,7 +30,6 @@ from signedjson.key import ( from signedjson.sign import ( SignatureVerifyException, encode_canonical_json, - sign_json, signature_ids, verify_signed_json, ) @@ -540,15 +539,7 @@ class BaseV2KeyFetcher(object): verify_key=verify_key, valid_until_ts=key_data["expired_ts"] ) - # re-sign the json with our own keys, so that it is ready if we are - # asked to give it out as a notary server - signed_key_json = response_json - for signing_key in self.config.key_server_signing_keys: - signed_key_json = sign_json( - signed_key_json, self.config.server_name, signing_key - ) - - signed_key_json_bytes = encode_canonical_json(signed_key_json) + signed_key_json_bytes = encode_canonical_json(response_json) yield make_deferred_yieldable( defer.gatherResults( diff --git a/synapse/rest/key/v2/remote_key_resource.py b/synapse/rest/key/v2/remote_key_resource.py index 031a316693..f3398c9523 100644 --- a/synapse/rest/key/v2/remote_key_resource.py +++ b/synapse/rest/key/v2/remote_key_resource.py @@ -13,7 +13,9 @@ # limitations under the License. import logging -from io import BytesIO + +from canonicaljson import json +from signedjson.sign import sign_json from twisted.internet import defer @@ -95,6 +97,7 @@ class RemoteKey(DirectServeResource): self.store = hs.get_datastore() self.clock = hs.get_clock() self.federation_domain_whitelist = hs.config.federation_domain_whitelist + self.config = hs.config @wrap_json_request_handler async def _async_render_GET(self, request): @@ -214,15 +217,14 @@ class RemoteKey(DirectServeResource): yield self.fetcher.get_keys(cache_misses) yield self.query_keys(request, query, query_remote_on_cache_miss=False) else: - result_io = BytesIO() - result_io.write(b'{"server_keys":') - sep = b"[" - for json_bytes in json_results: - result_io.write(sep) - result_io.write(json_bytes) - sep = b"," - if sep == b"[": - result_io.write(sep) - result_io.write(b"]}") - - respond_with_json_bytes(request, 200, result_io.getvalue()) + signed_keys = [] + for key_json in json_results: + key_json = json.loads(key_json) + for signing_key in self.config.key_server_signing_keys: + key_json = sign_json(key_json, self.config.server_name, signing_key) + + signed_keys.append(key_json) + + results = {"server_keys": signed_keys} + + respond_with_json_bytes(request, 200, json.dumps(results).encode("utf-8")) -- cgit 1.4.1 From ef1c524bb381545761fdd1ad2a61db1693ddbd3d Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Thu, 22 Aug 2019 10:42:06 +0100 Subject: Improve error msg when key-fetch fails (#5896) There's no point doing a raise_from here, because the exception is always logged at warn with no stacktrace in the caller. Instead, let's try to give better messages to reduce confusion. In particular, this means that we won't log 'Failed to connect to remote server' when we don't even attempt to connect to the remote server due to blacklisting. --- changelog.d/5896.misc | 1 + synapse/crypto/keyring.py | 12 +++++++----- 2 files changed, 8 insertions(+), 5 deletions(-) create mode 100644 changelog.d/5896.misc (limited to 'synapse/crypto') diff --git a/changelog.d/5896.misc b/changelog.d/5896.misc new file mode 100644 index 0000000000..ed47c747bd --- /dev/null +++ b/changelog.d/5896.misc @@ -0,0 +1 @@ +Improve the logging when we have an error when fetching signing keys. diff --git a/synapse/crypto/keyring.py b/synapse/crypto/keyring.py index 6c3e885e72..654accc843 100644 --- a/synapse/crypto/keyring.py +++ b/synapse/crypto/keyring.py @@ -18,7 +18,6 @@ import logging from collections import defaultdict import six -from six import raise_from from six.moves import urllib import attr @@ -657,9 +656,10 @@ class PerspectivesKeyFetcher(BaseV2KeyFetcher): }, ) except (NotRetryingDestination, RequestSendFailed) as e: - raise_from(KeyLookupError("Failed to connect to remote server"), e) + # these both have str() representations which we can't really improve upon + raise KeyLookupError(str(e)) except HttpResponseException as e: - raise_from(KeyLookupError("Remote server returned an error"), e) + raise KeyLookupError("Remote server returned an error: %s" % (e,)) keys = {} added_keys = [] @@ -821,9 +821,11 @@ class ServerKeyFetcher(BaseV2KeyFetcher): timeout=10000, ) except (NotRetryingDestination, RequestSendFailed) as e: - raise_from(KeyLookupError("Failed to connect to remote server"), e) + # these both have str() representations which we can't really improve + # upon + raise KeyLookupError(str(e)) except HttpResponseException as e: - raise_from(KeyLookupError("Remote server returned an error"), e) + raise KeyLookupError("Remote server returned an error: %s" % (e,)) if response["server_name"] != server_name: raise KeyLookupError( -- cgit 1.4.1 From 7af5a63063aa69888ab59ee997cc3d1459d25af4 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Fri, 23 Aug 2019 14:52:11 +0100 Subject: Fixup review comments --- docs/sample_config.yaml | 4 ++-- synapse/crypto/keyring.py | 4 ++-- synapse/rest/key/v2/remote_key_resource.py | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) (limited to 'synapse/crypto') diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index c96eb0cf2d..ae1cafc5f3 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -1029,8 +1029,8 @@ signing_key_path: "CONFDIR/SERVERNAME.signing.key" # - server_name: "matrix.org" # -# The additional signing keys to use when acting as a trusted key server, on -# top of the normal signing keys. +# The signing keys to use when acting as a trusted key server. If not specified +# defaults to the server signing key. # # Can contain multiple keys, one per line. # diff --git a/synapse/crypto/keyring.py b/synapse/crypto/keyring.py index abeb0ac26e..2d7434fb2f 100644 --- a/synapse/crypto/keyring.py +++ b/synapse/crypto/keyring.py @@ -539,7 +539,7 @@ class BaseV2KeyFetcher(object): verify_key=verify_key, valid_until_ts=key_data["expired_ts"] ) - signed_key_json_bytes = encode_canonical_json(response_json) + key_json_bytes = encode_canonical_json(response_json) yield make_deferred_yieldable( defer.gatherResults( @@ -551,7 +551,7 @@ class BaseV2KeyFetcher(object): from_server=from_server, ts_now_ms=time_added_ms, ts_expires_ms=ts_valid_until_ms, - key_json_bytes=signed_key_json_bytes, + key_json_bytes=key_json_bytes, ) for key_id in verify_keys ], diff --git a/synapse/rest/key/v2/remote_key_resource.py b/synapse/rest/key/v2/remote_key_resource.py index f3398c9523..55580bc59e 100644 --- a/synapse/rest/key/v2/remote_key_resource.py +++ b/synapse/rest/key/v2/remote_key_resource.py @@ -14,7 +14,7 @@ import logging -from canonicaljson import json +from canonicaljson import encode_canonical_json, json from signedjson.sign import sign_json from twisted.internet import defer @@ -227,4 +227,4 @@ class RemoteKey(DirectServeResource): results = {"server_keys": signed_keys} - respond_with_json_bytes(request, 200, json.dumps(results).encode("utf-8")) + respond_with_json_bytes(request, 200, encode_canonical_json(results)) -- cgit 1.4.1