From e1e26fea3fc19c4de94430748fef563dd4bf0a02 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 17 Mar 2020 15:47:41 +0000 Subject: [PATCH 1/2] Share SSL contexts for non-federation requests --- synapse/crypto/context_factory.py | 60 ++++++++++++------- synapse/http/client.py | 3 - .../federation/matrix_federation_agent.py | 2 +- synapse/server.py | 6 +- .../test_matrix_federation_agent.py | 6 +- 5 files changed, 47 insertions(+), 30 deletions(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 3a16f5ef58..fec197a0d8 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -61,7 +61,7 @@ def getContext(self): @implementer(IPolicyForHTTPS) -class ClientTLSOptionsFactory(object): +class FederationPolicyForHTTPS(object): """Factory for Twisted SSLClientConnectionCreators that are used to make connections to remote servers for federation. @@ -82,10 +82,10 @@ def __init__(self, config): trust_root = platformTrust() self._verify_ssl_context = CertificateOptions(trustRoot=trust_root).getContext() - self._verify_ssl_context.set_info_callback(self._context_info_cb) + self._verify_ssl_context.set_info_callback(_context_info_cb) self._no_verify_ssl_context = CertificateOptions().getContext() - self._no_verify_ssl_context.set_info_callback(self._context_info_cb) + self._no_verify_ssl_context.set_info_callback(_context_info_cb) def get_options(self, host): # Check if certificate verification has been enabled @@ -104,23 +104,6 @@ def get_options(self, host): 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) - def creatorForNetloc(self, hostname, port): """Implements the IPolicyForHTTPS interace so that this can be passed directly to agents. @@ -128,6 +111,43 @@ def creatorForNetloc(self, hostname, port): return self.get_options(hostname) +@implementer(IPolicyForHTTPS) +class RegularPolicyForHTTPS(object): + """Factory for Twisted SSLClientConnectionCreators that are used to make connections + to remote servers, for other than federation. + + Always uses the same OpenSSL context object, which uses the default OpenSSL CA + trust root. + """ + + def __init__(self): + trust_root = platformTrust() + self._ssl_context = CertificateOptions(trustRoot=trust_root).getContext() + self._ssl_context.set_info_callback(_context_info_cb) + + def creatorForNetloc(self, hostname, port): + return SSLClientConnectionCreator(hostname, self._ssl_context, True) + + +def _context_info_cb(ssl_connection, where, ret): + """The 'information callback' for our openssl context objects. + + Note: Once this is set as the info callback on a Context object, the Context should + only be used with the SSLClientConnectionCreator. + """ + # 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. diff --git a/synapse/http/client.py b/synapse/http/client.py index bf8afa703e..896e71cef3 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -246,9 +246,6 @@ def __getattr__(_self, attr): pool.maxPersistentPerHost = max((100 * CACHE_SIZE_FACTOR, 5)) pool.cachedConnectionTimeout = 2 * 60 - # The default context factory in Twisted 14.0.0 (which we require) is - # BrowserLikePolicyForHTTPS which will do regular cert validation - # 'like a browser' self.agent = ProxyAgent( self.reactor, connectTimeout=15, diff --git a/synapse/http/federation/matrix_federation_agent.py b/synapse/http/federation/matrix_federation_agent.py index e0c075e382..f595349a0e 100644 --- a/synapse/http/federation/matrix_federation_agent.py +++ b/synapse/http/federation/matrix_federation_agent.py @@ -61,7 +61,7 @@ class MatrixFederationAgent(object): Args: reactor (IReactor): twisted reactor to use for underlying requests - tls_client_options_factory (ClientTLSOptionsFactory|None): + tls_client_options_factory (FederationPolicyForHTTPS|None): factory to use for fetching client tls options, or none to disable TLS. _srv_resolver (SrvResolver|None): diff --git a/synapse/server.py b/synapse/server.py index 3f3c79498a..0b2e49cb72 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -27,7 +27,6 @@ from twisted.enterprise import adbapi from twisted.mail.smtp import sendmail -from twisted.web.client import BrowserLikePolicyForHTTPS from synapse.api.auth import Auth from synapse.api.filtering import Filtering @@ -35,6 +34,7 @@ from synapse.appservice.api import ApplicationServiceApi from synapse.appservice.scheduler import ApplicationServiceScheduler from synapse.crypto import context_factory +from synapse.crypto.context_factory import RegularPolicyForHTTPS from synapse.crypto.keyring import Keyring from synapse.events.builder import EventBuilderFactory from synapse.events.spamcheck import SpamChecker @@ -302,7 +302,7 @@ def build_http_client_context_factory(self): return ( InsecureInterceptableContextFactory() if self.config.use_insecure_ssl_client_just_for_testing_do_not_use - else BrowserLikePolicyForHTTPS() + else RegularPolicyForHTTPS() ) def build_simple_http_client(self): @@ -412,7 +412,7 @@ def build_pusherpool(self): return PusherPool(self) def build_http_client(self): - tls_client_options_factory = context_factory.ClientTLSOptionsFactory( + tls_client_options_factory = context_factory.FederationPolicyForHTTPS( self.config ) return MatrixFederationHttpClient(self, tls_client_options_factory) diff --git a/tests/http/federation/test_matrix_federation_agent.py b/tests/http/federation/test_matrix_federation_agent.py index 2ca9f8a0cd..16a1d31a64 100644 --- a/tests/http/federation/test_matrix_federation_agent.py +++ b/tests/http/federation/test_matrix_federation_agent.py @@ -30,7 +30,7 @@ from twisted.web.iweb import IPolicyForHTTPS from synapse.config.homeserver import HomeServerConfig -from synapse.crypto.context_factory import ClientTLSOptionsFactory +from synapse.crypto.context_factory import FederationPolicyForHTTPS from synapse.http.federation.matrix_federation_agent import ( MatrixFederationAgent, _cache_period_from_headers, @@ -79,7 +79,7 @@ def setUp(self): self.agent = MatrixFederationAgent( reactor=self.reactor, - tls_client_options_factory=ClientTLSOptionsFactory(config), + tls_client_options_factory=FederationPolicyForHTTPS(config), _srv_resolver=self.mock_resolver, _well_known_cache=self.well_known_cache, ) @@ -703,7 +703,7 @@ def test_get_well_known_unsigned_cert(self): agent = MatrixFederationAgent( reactor=self.reactor, - tls_client_options_factory=ClientTLSOptionsFactory(config), + tls_client_options_factory=FederationPolicyForHTTPS(config), _srv_resolver=self.mock_resolver, _well_known_cache=self.well_known_cache, ) From 7a6dec95b4c936c96489ee9761d9c525fa77b4bb Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Tue, 17 Mar 2020 16:07:50 +0000 Subject: [PATCH 2/2] newsfile --- changelog.d/30.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/30.misc diff --git a/changelog.d/30.misc b/changelog.d/30.misc new file mode 100644 index 0000000000..ae68554be3 --- /dev/null +++ b/changelog.d/30.misc @@ -0,0 +1 @@ +Improve performance when making HTTP requests to sygnal, sydent, etc, by sharing the SSL context object between connections.