Share SSL contexts for non-federation requests (#7094)
Extends #5794 etc to the SimpleHttpClient so that it also applies to non-federation requests. Fixes #7092.
This commit is contained in:
Richard van der Hoff
GitHub
parent
5e477c1deb
commit
c37db0211e
|
|
@ -0,0 +1 @@
|
|||
Improve performance when making HTTPS requests to sygnal, sydent, etc, by sharing the SSL context object between connections.
|
||||
|
|
@ -75,7 +75,7 @@ class ServerContextFactory(ContextFactory):
|
|||
|
||||
|
||||
@implementer(IPolicyForHTTPS)
|
||||
class ClientTLSOptionsFactory(object):
|
||||
class FederationPolicyForHTTPS(object):
|
||||
"""Factory for Twisted SSLClientConnectionCreators that are used to make connections
|
||||
to remote servers for federation.
|
||||
|
||||
|
|
@ -103,15 +103,15 @@ class ClientTLSOptionsFactory(object):
|
|||
# let us do).
|
||||
minTLS = _TLS_VERSION_MAP[config.federation_client_minimum_tls_version]
|
||||
|
||||
self._verify_ssl = CertificateOptions(
|
||||
_verify_ssl = CertificateOptions(
|
||||
trustRoot=trust_root, insecurelyLowerMinimumTo=minTLS
|
||||
)
|
||||
self._verify_ssl_context = self._verify_ssl.getContext()
|
||||
self._verify_ssl_context.set_info_callback(self._context_info_cb)
|
||||
self._verify_ssl_context = _verify_ssl.getContext()
|
||||
self._verify_ssl_context.set_info_callback(_context_info_cb)
|
||||
|
||||
self._no_verify_ssl = CertificateOptions(insecurelyLowerMinimumTo=minTLS)
|
||||
self._no_verify_ssl_context = self._no_verify_ssl.getContext()
|
||||
self._no_verify_ssl_context.set_info_callback(self._context_info_cb)
|
||||
_no_verify_ssl = CertificateOptions(insecurelyLowerMinimumTo=minTLS)
|
||||
self._no_verify_ssl_context = _no_verify_ssl.getContext()
|
||||
self._no_verify_ssl_context.set_info_callback(_context_info_cb)
|
||||
|
||||
def get_options(self, host: bytes):
|
||||
|
||||
|
|
@ -136,23 +136,6 @@ class ClientTLSOptionsFactory(object):
|
|||
|
||||
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.
|
||||
|
|
@ -160,6 +143,43 @@ class ClientTLSOptionsFactory(object):
|
|||
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.
|
||||
|
|
|
|||
|
|
@ -244,9 +244,6 @@ class SimpleHttpClient(object):
|
|||
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,
|
||||
|
|
|
|||
|
|
@ -45,7 +45,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):
|
||||
|
|
|
|||
|
|
@ -26,7 +26,6 @@ import logging
|
|||
import os
|
||||
|
||||
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.config.homeserver import HomeServerConfig
|
||||
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
|
||||
|
|
@ -310,7 +310,7 @@ class HomeServer(object):
|
|||
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):
|
||||
|
|
@ -420,7 +420,7 @@ class HomeServer(object):
|
|||
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)
|
||||
|
|
|
|||
|
|
@ -23,7 +23,7 @@ from OpenSSL import SSL
|
|||
|
||||
from synapse.config._base import Config, RootConfig
|
||||
from synapse.config.tls import ConfigError, TlsConfig
|
||||
from synapse.crypto.context_factory import ClientTLSOptionsFactory
|
||||
from synapse.crypto.context_factory import FederationPolicyForHTTPS
|
||||
|
||||
from tests.unittest import TestCase
|
||||
|
||||
|
|
@ -180,12 +180,13 @@ s4niecZKPBizL6aucT59CsunNmmb5Glq8rlAcU+1ZTZZzGYqVYhF6axB9Qg=
|
|||
t = TestConfig()
|
||||
t.read_config(config, config_dir_path="", data_dir_path="")
|
||||
|
||||
cf = ClientTLSOptionsFactory(t)
|
||||
cf = FederationPolicyForHTTPS(t)
|
||||
options = _get_ssl_context_options(cf._verify_ssl_context)
|
||||
|
||||
# The context has had NO_TLSv1_1 and NO_TLSv1_0 set, but not NO_TLSv1_2
|
||||
self.assertNotEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1, 0)
|
||||
self.assertNotEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1_1, 0)
|
||||
self.assertEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1_2, 0)
|
||||
self.assertNotEqual(options & SSL.OP_NO_TLSv1, 0)
|
||||
self.assertNotEqual(options & SSL.OP_NO_TLSv1_1, 0)
|
||||
self.assertEqual(options & SSL.OP_NO_TLSv1_2, 0)
|
||||
|
||||
def test_tls_client_minimum_set_passed_through_1_0(self):
|
||||
"""
|
||||
|
|
@ -195,12 +196,13 @@ s4niecZKPBizL6aucT59CsunNmmb5Glq8rlAcU+1ZTZZzGYqVYhF6axB9Qg=
|
|||
t = TestConfig()
|
||||
t.read_config(config, config_dir_path="", data_dir_path="")
|
||||
|
||||
cf = ClientTLSOptionsFactory(t)
|
||||
cf = FederationPolicyForHTTPS(t)
|
||||
options = _get_ssl_context_options(cf._verify_ssl_context)
|
||||
|
||||
# The context has not had any of the NO_TLS set.
|
||||
self.assertEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1, 0)
|
||||
self.assertEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1_1, 0)
|
||||
self.assertEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1_2, 0)
|
||||
self.assertEqual(options & SSL.OP_NO_TLSv1, 0)
|
||||
self.assertEqual(options & SSL.OP_NO_TLSv1_1, 0)
|
||||
self.assertEqual(options & SSL.OP_NO_TLSv1_2, 0)
|
||||
|
||||
def test_acme_disabled_in_generated_config_no_acme_domain_provied(self):
|
||||
"""
|
||||
|
|
@ -273,7 +275,7 @@ s4niecZKPBizL6aucT59CsunNmmb5Glq8rlAcU+1ZTZZzGYqVYhF6axB9Qg=
|
|||
t = TestConfig()
|
||||
t.read_config(config, config_dir_path="", data_dir_path="")
|
||||
|
||||
cf = ClientTLSOptionsFactory(t)
|
||||
cf = FederationPolicyForHTTPS(t)
|
||||
|
||||
# Not in the whitelist
|
||||
opts = cf.get_options(b"notexample.com")
|
||||
|
|
@ -282,3 +284,10 @@ s4niecZKPBizL6aucT59CsunNmmb5Glq8rlAcU+1ZTZZzGYqVYhF6axB9Qg=
|
|||
# Caught by the wildcard
|
||||
opts = cf.get_options(idna.encode("テスト.ドメイン.テスト"))
|
||||
self.assertFalse(opts._verifier._verify_certs)
|
||||
|
||||
|
||||
def _get_ssl_context_options(ssl_context: SSL.Context) -> int:
|
||||
"""get the options bits from an openssl context object"""
|
||||
# the OpenSSL.SSL.Context wrapper doesn't expose get_options, so we have to
|
||||
# use the low-level interface
|
||||
return SSL._lib.SSL_CTX_get_options(ssl_context._context)
|
||||
|
|
|
|||
|
|
@ -31,7 +31,7 @@ from twisted.web.http_headers import Headers
|
|||
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
|
||||
from synapse.http.federation.srv_resolver import Server
|
||||
from synapse.http.federation.well_known_resolver import (
|
||||
|
|
@ -79,7 +79,7 @@ class MatrixFederationAgentTests(unittest.TestCase):
|
|||
self._config = config = HomeServerConfig()
|
||||
config.parse_config_dict(config_dict, "", "")
|
||||
|
||||
self.tls_factory = ClientTLSOptionsFactory(config)
|
||||
self.tls_factory = FederationPolicyForHTTPS(config)
|
||||
|
||||
self.well_known_cache = TTLCache("test_cache", timer=self.reactor.seconds)
|
||||
self.had_well_known_cache = TTLCache("test_cache", timer=self.reactor.seconds)
|
||||
|
|
@ -715,7 +715,7 @@ class MatrixFederationAgentTests(unittest.TestCase):
|
|||
config = default_config("test", parse=True)
|
||||
|
||||
# Build a new agent and WellKnownResolver with a different tls factory
|
||||
tls_factory = ClientTLSOptionsFactory(config)
|
||||
tls_factory = FederationPolicyForHTTPS(config)
|
||||
agent = MatrixFederationAgent(
|
||||
reactor=self.reactor,
|
||||
tls_client_options_factory=tls_factory,
|
||||
|
|
|
|||
Loading…
Reference in New Issue