From 81a93ddcc8798568276582ed9c7a63bc64dc5bc0 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Wed, 9 Sep 2015 12:02:07 +0100 Subject: [PATCH 1/5] Allow configuration to ignore invalid SSL certs This will be useful for sytest, and sytest only, hence the aggressive config key name. --- synapse/app/homeserver.py | 8 ++++---- synapse/config/tls.py | 4 ++++ synapse/crypto/keyring.py | 4 ++-- synapse/handlers/auth.py | 3 +-- synapse/http/client.py | 25 +++++++++++++++++++++++-- synapse/http/matrixfederationclient.py | 4 ++-- synapse/server.py | 14 ++++++++++++++ 7 files changed, 50 insertions(+), 12 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index ffc629914..ba76ee362 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -15,6 +15,7 @@ # limitations under the License. import sys + sys.dont_write_bytecode = True from synapse.python_dependencies import check_requirements, DEPENDENCY_LINKS @@ -221,7 +222,7 @@ class SynapseHomeServer(HomeServer): listener_config, root_resource, ), - self.tls_context_factory, + self.tls_server_context_factory, interface=bind_address ) else: @@ -365,7 +366,6 @@ def setup(config_options): Args: config_options_options: The options passed to Synapse. Usually `sys.argv[1:]`. - should_run (bool): Whether to start the reactor. Returns: HomeServer @@ -388,7 +388,7 @@ def setup(config_options): events.USE_FROZEN_DICTS = config.use_frozen_dicts - tls_context_factory = context_factory.ServerContextFactory(config) + tls_server_context_factory = context_factory.ServerContextFactory(config) database_engine = create_engine(config.database_config["name"]) config.database_config["args"]["cp_openfun"] = database_engine.on_new_connection @@ -396,7 +396,7 @@ def setup(config_options): hs = SynapseHomeServer( config.server_name, db_config=config.database_config, - tls_context_factory=tls_context_factory, + tls_server_context_factory=tls_server_context_factory, config=config, content_addr=config.content_addr, version_string=version_string, diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 4751d39bc..472cf7ac4 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -42,6 +42,10 @@ class TlsConfig(Config): config.get("tls_dh_params_path"), "tls_dh_params" ) + self.use_insecure_ssl_client = config.get( + "i_really_want_to_ignore_ssl_certs_when_i_am_an_http_client_even_" + "though_it_is_woefully_insecure_because_i_hate_my_users", False) + def default_config(self, config_dir_path, server_name): base_key_name = os.path.join(config_dir_path, server_name) diff --git a/synapse/crypto/keyring.py b/synapse/crypto/keyring.py index a692cdbe5..e98a625fe 100644 --- a/synapse/crypto/keyring.py +++ b/synapse/crypto/keyring.py @@ -463,7 +463,7 @@ class Keyring(object): continue (response, tls_certificate) = yield fetch_server_key( - server_name, self.hs.tls_context_factory, + server_name, self.hs.tls_server_context_factory, path=(b"/_matrix/key/v2/server/%s" % ( urllib.quote(requested_key_id), )).encode("ascii"), @@ -597,7 +597,7 @@ class Keyring(object): # Try to fetch the key from the remote server. (response, tls_certificate) = yield fetch_server_key( - server_name, self.hs.tls_context_factory + server_name, self.hs.tls_server_context_factory ) # Check the response. diff --git a/synapse/handlers/auth.py b/synapse/handlers/auth.py index 59f687e0f..793b3fcd8 100644 --- a/synapse/handlers/auth.py +++ b/synapse/handlers/auth.py @@ -19,7 +19,6 @@ from ._base import BaseHandler from synapse.api.constants import LoginType from synapse.types import UserID from synapse.api.errors import LoginError, Codes -from synapse.http.client import SimpleHttpClient from synapse.util.async import run_on_reactor from twisted.web.client import PartialDownloadError @@ -187,7 +186,7 @@ class AuthHandler(BaseHandler): # TODO: get this from the homeserver rather than creating a new one for # each request try: - client = SimpleHttpClient(self.hs) + client = self.hs.get_simple_http_client() resp_body = yield client.post_urlencoded_get_json( self.hs.config.recaptcha_siteverify_api, args={ diff --git a/synapse/http/client.py b/synapse/http/client.py index 4b8fd3d3a..da77c8b0a 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -12,6 +12,8 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +from OpenSSL import SSL +from OpenSSL.SSL import VERIFY_NONE from synapse.api.errors import CodeMessageException from synapse.util.logcontext import preserve_context_over_fn @@ -19,7 +21,7 @@ import synapse.metrics from canonicaljson import encode_canonical_json -from twisted.internet import defer, reactor +from twisted.internet import defer, reactor, ssl from twisted.web.client import ( Agent, readBody, FileBodyProducer, PartialDownloadError, HTTPConnectionPool, @@ -59,7 +61,12 @@ class SimpleHttpClient(object): # 'like a browser' pool = HTTPConnectionPool(reactor) pool.maxPersistentPerHost = 10 - self.agent = Agent(reactor, pool=pool) + self.agent = Agent( + reactor, + pool=pool, + connectTimeout=15, + contextFactory=hs.get_http_client_context_factory() + ) self.version_string = hs.version_string def request(self, method, uri, *args, **kwargs): @@ -252,3 +259,17 @@ def _print_ex(e): _print_ex(ex) else: logger.exception(e) + + +class WoefullyInsecureContextFactory(ssl.ContextFactory): + """ + Factory for PyOpenSSL SSL contexts which does absolutely no certificate verification. + + Do not use this unless you really, really hate your users.""" + + def __init__(self): + self._context = SSL.Context(SSL.SSLv23_METHOD) + self._context.set_verify(VERIFY_NONE, lambda *_: None) + + def getContext(self, hostname, port): + return self._context diff --git a/synapse/http/matrixfederationclient.py b/synapse/http/matrixfederationclient.py index 1c9e55278..b50a0c445 100644 --- a/synapse/http/matrixfederationclient.py +++ b/synapse/http/matrixfederationclient.py @@ -57,14 +57,14 @@ incoming_responses_counter = metrics.register_counter( class MatrixFederationEndpointFactory(object): def __init__(self, hs): - self.tls_context_factory = hs.tls_context_factory + self.tls_server_context_factory = hs.tls_server_context_factory def endpointForURI(self, uri): destination = uri.netloc return matrix_federation_endpoint( reactor, destination, timeout=10, - ssl_context_factory=self.tls_context_factory + ssl_context_factory=self.tls_server_context_factory ) diff --git a/synapse/server.py b/synapse/server.py index 4d1fb1cbf..656e534df 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -19,7 +19,9 @@ # partial one for unit test mocking. # Imports required for the default HomeServer() implementation +from twisted.web.client import BrowserLikePolicyForHTTPS from synapse.federation import initialize_http_replication +from synapse.http.client import SimpleHttpClient, WoefullyInsecureContextFactory from synapse.notifier import Notifier from synapse.api.auth import Auth from synapse.handlers import Handlers @@ -87,6 +89,8 @@ class BaseHomeServer(object): 'pusherpool', 'event_builder_factory', 'filtering', + 'http_client_context_factory', + 'simple_http_client', ] def __init__(self, hostname, **kwargs): @@ -174,6 +178,16 @@ class HomeServer(BaseHomeServer): def build_auth(self): return Auth(self) + def build_http_client_context_factory(self): + config = self.get_config() + return ( + WoefullyInsecureContextFactory() if config.use_insecure_ssl_client + else BrowserLikePolicyForHTTPS() + ) + + def build_simple_http_client(self): + return SimpleHttpClient(self) + def build_v1auth(self): orf = Auth(self) # Matrix spec makes no reference to what HTTP status code is returned, From 6485f03d91a5f96da28f9dcc8e9ebc3adb213f6f Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Wed, 9 Sep 2015 13:05:00 +0100 Subject: [PATCH 2/5] Fix random formatting --- synapse/app/homeserver.py | 1 - synapse/http/client.py | 3 ++- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index ba76ee362..8e60304e2 100755 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -15,7 +15,6 @@ # limitations under the License. import sys - sys.dont_write_bytecode = True from synapse.python_dependencies import check_requirements, DEPENDENCY_LINKS diff --git a/synapse/http/client.py b/synapse/http/client.py index da77c8b0a..815a83872 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -265,7 +265,8 @@ class WoefullyInsecureContextFactory(ssl.ContextFactory): """ Factory for PyOpenSSL SSL contexts which does absolutely no certificate verification. - Do not use this unless you really, really hate your users.""" + Do not use this unless you really, really hate your users. + """ def __init__(self): self._context = SSL.Context(SSL.SSLv23_METHOD) From ddfe30ba835da4357670f2a2a39386b8b8e65b60 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Wed, 9 Sep 2015 13:26:23 +0100 Subject: [PATCH 3/5] Better document the intent of the insecure SSL setting --- synapse/config/tls.py | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 472cf7ac4..35ff13f4b 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -42,9 +42,13 @@ class TlsConfig(Config): config.get("tls_dh_params_path"), "tls_dh_params" ) + # This config option applies to non-federation HTTP clients + # (e.g. for talking to recaptcha, identity servers, and such) + # It should never be used in production, and is intended for + # use only when running tests. self.use_insecure_ssl_client = config.get( - "i_really_want_to_ignore_ssl_certs_when_i_am_an_http_client_even_" - "though_it_is_woefully_insecure_because_i_hate_my_users", False) + "i_really_want_to_ignore_ssl_certs_when_i_am_an_https_client_even_" + "though_it_is_woefully_insecure_because_i_am_testing_i_promise", False) def default_config(self, config_dir_path, server_name): base_key_name = os.path.join(config_dir_path, server_name) From 3bcbabc9fb5446e74a675352e22963d528189957 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Tue, 15 Sep 2015 15:46:22 +0100 Subject: [PATCH 4/5] Rename context factory Mjark is officially no fun. --- synapse/http/client.py | 6 +++--- synapse/server.py | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/synapse/http/client.py b/synapse/http/client.py index 815a83872..0933388c0 100644 --- a/synapse/http/client.py +++ b/synapse/http/client.py @@ -261,11 +261,11 @@ def _print_ex(e): logger.exception(e) -class WoefullyInsecureContextFactory(ssl.ContextFactory): +class InsecureInterceptableContextFactory(ssl.ContextFactory): """ - Factory for PyOpenSSL SSL contexts which does absolutely no certificate verification. + Factory for PyOpenSSL SSL contexts which accepts any certificate for any domain. - Do not use this unless you really, really hate your users. + Do not use this since it allows an attacker to intercept your communications. """ def __init__(self): diff --git a/synapse/server.py b/synapse/server.py index 656e534df..d96c5a573 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -21,7 +21,7 @@ # Imports required for the default HomeServer() implementation from twisted.web.client import BrowserLikePolicyForHTTPS from synapse.federation import initialize_http_replication -from synapse.http.client import SimpleHttpClient, WoefullyInsecureContextFactory +from synapse.http.client import SimpleHttpClient, InsecureInterceptableContextFactory from synapse.notifier import Notifier from synapse.api.auth import Auth from synapse.handlers import Handlers @@ -181,7 +181,7 @@ class HomeServer(BaseHomeServer): def build_http_client_context_factory(self): config = self.get_config() return ( - WoefullyInsecureContextFactory() if config.use_insecure_ssl_client + InsecureInterceptableContextFactory() if config.use_insecure_ssl_client else BrowserLikePolicyForHTTPS() ) From d4af08a167cb5351110036c35bdfc267242d8131 Mon Sep 17 00:00:00 2001 From: Daniel Wagner-Hall Date: Tue, 15 Sep 2015 15:50:13 +0100 Subject: [PATCH 5/5] Use shorter config key name --- synapse/config/tls.py | 6 +++--- synapse/server.py | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 35ff13f4b..e6023a718 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -46,9 +46,9 @@ class TlsConfig(Config): # (e.g. for talking to recaptcha, identity servers, and such) # It should never be used in production, and is intended for # use only when running tests. - self.use_insecure_ssl_client = config.get( - "i_really_want_to_ignore_ssl_certs_when_i_am_an_https_client_even_" - "though_it_is_woefully_insecure_because_i_am_testing_i_promise", False) + self.use_insecure_ssl_client_just_for_testing_do_not_use = config.get( + "use_insecure_ssl_client_just_for_testing_do_not_use" + ) def default_config(self, config_dir_path, server_name): base_key_name = os.path.join(config_dir_path, server_name) diff --git a/synapse/server.py b/synapse/server.py index d96c5a573..8424798b1 100644 --- a/synapse/server.py +++ b/synapse/server.py @@ -181,7 +181,8 @@ class HomeServer(BaseHomeServer): def build_http_client_context_factory(self): config = self.get_config() return ( - InsecureInterceptableContextFactory() if config.use_insecure_ssl_client + InsecureInterceptableContextFactory() + if config.use_insecure_ssl_client_just_for_testing_do_not_use else BrowserLikePolicyForHTTPS() )