diff options
Diffstat (limited to 'synapse')
| -rw-r--r-- | synapse/__init__.py | 2 | ||||
| -rw-r--r-- | synapse/config/key.py | 27 | ||||
| -rw-r--r-- | synapse/crypto/context_factory.py | 180 | ||||
| -rw-r--r-- | synapse/crypto/keyring.py | 7 | ||||
| -rw-r--r-- | synapse/python_dependencies.py | 5 |
5 files changed, 134 insertions, 87 deletions
diff --git a/synapse/__init__.py b/synapse/__init__.py index 77a4cfc3a5..8dc07fe73c 100644 --- a/synapse/__init__.py +++ b/synapse/__init__.py @@ -27,4 +27,4 @@ try: except ImportError: pass -__version__ = "1.0.0rc1" +__version__ = "1.0.0rc2" diff --git a/synapse/config/key.py b/synapse/config/key.py index aba7092ccd..424875feae 100644 --- a/synapse/config/key.py +++ b/synapse/config/key.py @@ -41,6 +41,15 @@ validation or TLS certificate validation. This is likely to be very insecure. If you are *sure* you want to do this, set 'accept_keys_insecurely' on the keyserver configuration.""" +RELYING_ON_MATRIX_KEY_ERROR = """\ +Your server is configured to accept key server responses without TLS certificate +validation, and which are only signed by the old (possibly compromised) +matrix.org signing key 'ed25519:auto'. This likely isn't what you want to do, +and you should enable 'federation_verify_certificates' in your configuration. + +If you are *sure* you want to do this, set 'accept_keys_insecurely' on the +trusted_key_server configuration.""" + logger = logging.getLogger(__name__) @@ -340,10 +349,20 @@ def _parse_key_servers(key_servers, federation_verify_certificates): result.verify_keys[key_id] = verify_key if ( - not verify_keys - and not server.get("accept_keys_insecurely") - and not federation_verify_certificates + not federation_verify_certificates and + not server.get("accept_keys_insecurely") ): - raise ConfigError(INSECURE_NOTARY_ERROR) + _assert_keyserver_has_verify_keys(result) yield result + + +def _assert_keyserver_has_verify_keys(trusted_key_server): + if not trusted_key_server.verify_keys: + raise ConfigError(INSECURE_NOTARY_ERROR) + + # also check that they are not blindly checking the old matrix.org key + if trusted_key_server.server_name == "matrix.org" and any( + key_id == "ed25519:auto" for key_id in trusted_key_server.verify_keys + ): + raise ConfigError(RELYING_ON_MATRIX_KEY_ERROR) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 59ea087e66..2bc5cc3807 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -15,10 +15,13 @@ import logging +import idna +from service_identity import VerificationError +from service_identity.pyopenssl import verify_hostname, verify_ip_address from zope.interface import implementer from OpenSSL import SSL, crypto -from twisted.internet._sslverify import ClientTLSOptions, _defaultCurveName +from twisted.internet._sslverify import _defaultCurveName from twisted.internet.abstract import isIPAddress, isIPv6Address from twisted.internet.interfaces import IOpenSSLClientConnectionCreator from twisted.internet.ssl import CertificateOptions, ContextFactory, platformTrust @@ -56,79 +59,19 @@ class ServerContextFactory(ContextFactory): return self._context -def _idnaBytes(text): - """ - Convert some text typed by a human into some ASCII bytes. This is a - copy of twisted.internet._idna._idnaBytes. For documentation, see the - twisted documentation. - """ - try: - import idna - except ImportError: - return text.encode("idna") - else: - return idna.encode(text) - - -def _tolerateErrors(wrapped): - """ - Wrap up an info_callback for pyOpenSSL so that if something goes wrong - the error is immediately logged and the connection is dropped if possible. - This is a copy of twisted.internet._sslverify._tolerateErrors. For - documentation, see the twisted documentation. - """ - - def infoCallback(connection, where, ret): - try: - return wrapped(connection, where, ret) - except: # noqa: E722, taken from the twisted implementation - f = Failure() - logger.exception("Error during info_callback") - connection.get_app_data().failVerification(f) - - return infoCallback +class ClientTLSOptionsFactory(object): + """Factory for Twisted SSLClientConnectionCreators that are used to make connections + to remote servers for federation. + Uses one of two OpenSSL context objects for all connections, depending on whether + we should do SSL certificate verification. -@implementer(IOpenSSLClientConnectionCreator) -class ClientTLSOptionsNoVerify(object): - """ - Client creator for TLS without certificate identity verification. This is a - copy of twisted.internet._sslverify.ClientTLSOptions with the identity - verification left out. For documentation, see the twisted documentation. + get_options decides whether we should do SSL certificate verification and + constructs an SSLClientConnectionCreator factory accordingly. """ - def __init__(self, hostname, ctx): - self._ctx = ctx - - if isIPAddress(hostname) or isIPv6Address(hostname): - self._hostnameBytes = hostname.encode('ascii') - self._sendSNI = False - else: - self._hostnameBytes = _idnaBytes(hostname) - self._sendSNI = True - - ctx.set_info_callback(_tolerateErrors(self._identityVerifyingInfoCallback)) - - def clientConnectionForTLS(self, tlsProtocol): - context = self._ctx - connection = SSL.Connection(context, None) - connection.set_app_data(tlsProtocol) - return connection - - def _identityVerifyingInfoCallback(self, connection, where, ret): - # Literal IPv4 and IPv6 addresses are not permitted - # as host names according to the RFCs - if where & SSL.SSL_CB_HANDSHAKE_START and self._sendSNI: - connection.set_tlsext_host_name(self._hostnameBytes) - - -class ClientTLSOptionsFactory(object): - """Factory for Twisted ClientTLSOptions that are used to make connections - to remote servers for federation.""" - def __init__(self, config): self._config = config - self._options_noverify = CertificateOptions() # Check if we're using a custom list of a CA certificates trust_root = config.federation_ca_trust_root @@ -136,11 +79,13 @@ class ClientTLSOptionsFactory(object): # Use CA root certs provided by OpenSSL trust_root = platformTrust() - self._options_verify = CertificateOptions(trustRoot=trust_root) + self._verify_ssl_context = CertificateOptions(trustRoot=trust_root).getContext() + self._verify_ssl_context.set_info_callback(self._context_info_cb) - def get_options(self, host): - # Use _makeContext so that we get a fresh OpenSSL CTX each time. + self._no_verify_ssl_context = CertificateOptions().getContext() + self._no_verify_ssl_context.set_info_callback(self._context_info_cb) + def get_options(self, host): # Check if certificate verification has been enabled should_verify = self._config.federation_verify_certificates @@ -151,6 +96,93 @@ class ClientTLSOptionsFactory(object): should_verify = False break - if should_verify: - return ClientTLSOptions(host, self._options_verify._makeContext()) - return ClientTLSOptionsNoVerify(host, self._options_noverify._makeContext()) + ssl_context = ( + self._verify_ssl_context if should_verify else self._no_verify_ssl_context + ) + + return SSLClientConnectionCreator(host, ssl_context, should_verify) + + @staticmethod + def _context_info_cb(ssl_connection, where, ret): + """The 'information callback' for our openssl context object.""" + # we assume that the app_data on the connection object has been set to + # a TLSMemoryBIOProtocol object. (This is done by SSLClientConnectionCreator) + tls_protocol = ssl_connection.get_app_data() + try: + # ... we further assume that SSLClientConnectionCreator has set the + # '_synapse_tls_verifier' attribute to a ConnectionVerifier object. + tls_protocol._synapse_tls_verifier.verify_context_info_cb( + ssl_connection, where + ) + except: # noqa: E722, taken from the twisted implementation + logger.exception("Error during info_callback") + f = Failure() + tls_protocol.failVerification(f) + + +@implementer(IOpenSSLClientConnectionCreator) +class SSLClientConnectionCreator(object): + """Creates openssl connection objects for client connections. + + Replaces twisted.internet.ssl.ClientTLSOptions + """ + + def __init__(self, hostname, ctx, verify_certs): + self._ctx = ctx + self._verifier = ConnectionVerifier(hostname, verify_certs) + + def clientConnectionForTLS(self, tls_protocol): + context = self._ctx + connection = SSL.Connection(context, None) + + # as per twisted.internet.ssl.ClientTLSOptions, we set the application + # data to our TLSMemoryBIOProtocol... + connection.set_app_data(tls_protocol) + + # ... and we also gut-wrench a '_synapse_tls_verifier' attribute into the + # tls_protocol so that the SSL context's info callback has something to + # call to do the cert verification. + setattr(tls_protocol, "_synapse_tls_verifier", self._verifier) + return connection + + +class ConnectionVerifier(object): + """Set the SNI, and do cert verification + + This is a thing which is attached to the TLSMemoryBIOProtocol, and is called by + the ssl context's info callback. + """ + + # This code is based on twisted.internet.ssl.ClientTLSOptions. + + def __init__(self, hostname, verify_certs): + self._verify_certs = verify_certs + + if isIPAddress(hostname) or isIPv6Address(hostname): + self._hostnameBytes = hostname.encode("ascii") + self._is_ip_address = True + else: + # twisted's ClientTLSOptions falls back to the stdlib impl here if + # idna is not installed, but points out that lacks support for + # IDNA2008 (http://bugs.python.org/issue17305). + # + # We can rely on having idna. + self._hostnameBytes = idna.encode(hostname) + self._is_ip_address = False + + self._hostnameASCII = self._hostnameBytes.decode("ascii") + + def verify_context_info_cb(self, ssl_connection, where): + if where & SSL.SSL_CB_HANDSHAKE_START and not self._is_ip_address: + ssl_connection.set_tlsext_host_name(self._hostnameBytes) + + if where & SSL.SSL_CB_HANDSHAKE_DONE and self._verify_certs: + try: + if self._is_ip_address: + verify_ip_address(ssl_connection, self._hostnameASCII) + else: + verify_hostname(ssl_connection, self._hostnameASCII) + except VerificationError: + f = Failure() + tls_protocol = ssl_connection.get_app_data() + tls_protocol.failVerification(f) diff --git a/synapse/crypto/keyring.py b/synapse/crypto/keyring.py index 96964b0d50..6f603f1961 100644 --- a/synapse/crypto/keyring.py +++ b/synapse/crypto/keyring.py @@ -750,13 +750,6 @@ class PerspectivesKeyFetcher(BaseV2KeyFetcher): verify_signed_json(response, perspective_name, perspective_keys[key_id]) verified = True - if perspective_name == "matrix.org" and key_id == "ed25519:auto": - logger.warning( - "Trusting trusted_key_server responses signed by the " - "compromised matrix.org signing key 'ed25519:auto'. " - "This is a placebo." - ) - if not verified: raise KeyLookupError( "Response not signed with a known key: signed with: %r, known keys: %r" diff --git a/synapse/python_dependencies.py b/synapse/python_dependencies.py index c78f2cb15e..6efd81f204 100644 --- a/synapse/python_dependencies.py +++ b/synapse/python_dependencies.py @@ -44,7 +44,10 @@ REQUIREMENTS = [ "canonicaljson>=1.1.3", "signedjson>=1.0.0", "pynacl>=1.2.1", - "service_identity>=16.0.0", + "idna>=2", + + # validating SSL certs for IP addresses requires service_identity 18.1. + "service_identity>=18.1.0", # our logcontext handling relies on the ability to cancel inlineCallbacks # (https://twistedmatrix.com/trac/ticket/4632) which landed in Twisted 18.7. |