From 30c4f168a6f80559bf583b533c21d1c60a87a9ae Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Tue, 25 Jun 2019 15:39:56 +1000 Subject: [PATCH 1/9] add the option for client config and bump the server config --- synapse/config/_base.py | 16 +++++ synapse/config/tls.py | 33 ++++++++++- synapse/crypto/context_factory.py | 39 ++++++++++-- tests/config/test_tls.py | 99 ++++++++++++++++++++++++++++++- 4 files changed, 179 insertions(+), 8 deletions(-) diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 965478d8d59e..daea536dd7d2 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -82,6 +82,22 @@ def parse_duration(value): size = sizes[suffix] return int(value) * size + @staticmethod + def parse_float(value, config_name): + """ + Parse a value to a float, raising a configuration error if it is not. + """ + if isinstance(value, (int, float)): + return value + + if isinstance(value, str): + try: + return float(value) + except ValueError: + raise ConfigError("%s value must be a number" % (config_name)) + else: + raise ConfigError("%s value must be a number" % (config_name)) + @staticmethod def abspath(file_path): return os.path.abspath(file_path) if file_path else file_path diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 8fcf80141863..fe1270d1d15c 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -23,7 +23,7 @@ from unpaddedbase64 import encode_base64 -from OpenSSL import crypto +from OpenSSL import SSL, crypto from twisted.internet._sslverify import Certificate, trustRootFromCertificates from synapse.config._base import Config, ConfigError @@ -81,6 +81,28 @@ def read_config(self, config, config_dir_path, **kwargs): "federation_verify_certificates", True ) + # Minimum TLS version to use for outbound federation traffic + self.federation_minimum_tls_client_version = self.parse_float( + config.get("federation_minimum_tls_client_version", 1.0), + "federation_minimum_tls_client_version", + ) + + if self.federation_minimum_tls_client_version not in [1.0, 1.1, 1.2, 1.3]: + raise ConfigError( + "federation_minimum_tls_client_version must be one of: 1.0, 1.1, 1.2, 1.3" + ) + + # Prevent people shooting themselves in the foot here by setting it to + # the biggest number blindly + if self.federation_minimum_tls_client_version == 1.3: + if getattr(SSL, "OP_NO_TLSv1_3", None) is None: + raise ConfigError( + ( + "federation_minimum_tls_client_version cannot be 1.3, " + "your OpenSSL does not support it" + ) + ) + # Whitelist of domains to not verify certificates for fed_whitelist_entries = config.get( "federation_certificate_verification_whitelist", [] @@ -261,6 +283,15 @@ def generate_config_section( # #federation_verify_certificates: false + # The minimum TLS version that will be used for outbound federation requests. + # + # Defaults to `1.0`. Configurable to `1.0`, `1.1`, `1.2`, or `1.3`. Note + # that setting this value higher than `1.2` will prevent federation to most + # of the public Matrix network, only configure it to `1.3` if you have an + # entirely private federation setup and you can ensure TLS 1.3 support. + # + #federation_minimum_tls_client_version: 1.2 + # Skip federation certificate verification on the following whitelist # of domains. # diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 2bc5cc38073e..927505fc578f 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -24,12 +24,25 @@ 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 +from twisted.internet.ssl import ( + CertificateOptions, + ContextFactory, + TLSVersion, + platformTrust, +) from twisted.python.failure import Failure logger = logging.getLogger(__name__) +_TLS_VERSION_MAP = { + 1.0: TLSVersion.TLSv1_0, + 1.1: TLSVersion.TLSv1_1, + 1.2: TLSVersion.TLSv1_2, + 1.3: TLSVersion.TLSv1_3, +} + + class ServerContextFactory(ContextFactory): """Factory for PyOpenSSL SSL contexts that are used to handle incoming connections.""" @@ -43,16 +56,18 @@ def configure_context(context, config): try: _ecCurve = crypto.get_elliptic_curve(_defaultCurveName) context.set_tmp_ecdh(_ecCurve) - except Exception: logger.exception("Failed to enable elliptic curve for TLS") - context.set_options(SSL.OP_NO_SSLv2 | SSL.OP_NO_SSLv3) + + context.set_options( + SSL.OP_NO_SSLv2 | SSL.OP_NO_SSLv3 | SSL.OP_NO_TLSv1 | SSL.OP_NO_TLSv1_1 + ) context.use_certificate_chain_file(config.tls_certificate_file) context.use_privatekey(config.tls_private_key) # https://hynek.me/articles/hardening-your-web-servers-ssl-ciphers/ context.set_cipher_list( - "ECDH+AESGCM:ECDH+CHACHA20:ECDH+AES256:ECDH+AES128:!aNULL:!SHA1" + "ECDH+AESGCM:ECDH+CHACHA20:ECDH+AES256:ECDH+AES128:!aNULL:!SHA1:!AESCCM" ) def getContext(self): @@ -79,10 +94,22 @@ def __init__(self, config): # Use CA root certs provided by OpenSSL trust_root = platformTrust() - self._verify_ssl_context = CertificateOptions(trustRoot=trust_root).getContext() + # "insecurelyLowerMinimumTo" is the argument that will go lower than + # Twisted's default, which is why it is marked as "insecure" (since + # Twisted's defaults are reasonably secure). But, since Twisted is + # moving to TLS 1.2 by default, we want to respect the config option if + # it is set to 1.0 (which the alternate option, raiseMinimumTo, will not + # let us do). + minTLS = _TLS_VERSION_MAP[config.federation_minimum_tls_client_version] + + self._verify_ssl_context = CertificateOptions( + trustRoot=trust_root, insecurelyLowerMinimumTo=minTLS + ).getContext() self._verify_ssl_context.set_info_callback(self._context_info_cb) - self._no_verify_ssl_context = CertificateOptions().getContext() + self._no_verify_ssl_context = CertificateOptions( + insecurelyLowerMinimumTo=minTLS + ).getContext() self._no_verify_ssl_context.set_info_callback(self._context_info_cb) def get_options(self, host): diff --git a/tests/config/test_tls.py b/tests/config/test_tls.py index a5d88d644ac7..76e63315f6cb 100644 --- a/tests/config/test_tls.py +++ b/tests/config/test_tls.py @@ -15,7 +15,10 @@ import os -from synapse.config.tls import TlsConfig +from OpenSSL import SSL + +from synapse.config.tls import ConfigError, TlsConfig +from synapse.crypto.context_factory import ContextFactory from tests.unittest import TestCase @@ -78,3 +81,97 @@ def test_warn_self_signed(self): "or use Synapse's ACME support to provision one." ), ) + + def test_tls_client_minimum_default(self): + """ + The default client TLS version is 1.0. + """ + config = {} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + + self.assertEqual(t.federation_minimum_tls_client_version, 1.0) + + def test_tls_client_minimum_set(self): + """ + The default client TLS version can be set to 1.0, 1.1, and 1.2. + """ + config = {"federation_minimum_tls_client_version": 1.0} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual(t.federation_minimum_tls_client_version, 1.0) + + config = {"federation_minimum_tls_client_version": 1.1} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual(t.federation_minimum_tls_client_version, 1.1) + + config = {"federation_minimum_tls_client_version": 1.2} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual(t.federation_minimum_tls_client_version, 1.2) + + # Also test a string version + config = {"federation_minimum_tls_client_version": "1.0"} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual(t.federation_minimum_tls_client_version, 1.0) + + # Also just 1 for 1.0 + config = {"federation_minimum_tls_client_version": "1"} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual(t.federation_minimum_tls_client_version, 1.0) + + def test_tls_client_minimum_1_point_3_missing(self): + """ + If TLS 1.3 support is missing and it's configured, it will raise a + ConfigError. + """ + # thanks i hate it + if hasattr(SSL, "OP_NO_TLSv1_3"): + OP_NO_TLSv1_3 = SSL.OP_NO_TLSv1_3 + delattr(SSL, "OP_NO_TLSv1_3") + self.addCleanup(setattr, SSL, "SSL.OP_NO_TLSv1_3", SSL.OP_NO_TLSv1_3) + assert not hasattr(SSL, "OP_NO_TLSv1_3") + + config = {"federation_minimum_tls_client_version": 1.3} + t = TestConfig() + with self.assertRaises(ConfigError) as e: + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual( + e.exception.args[0], + ( + "federation_minimum_tls_client_version cannot be 1.3, " + "your OpenSSL does not support it" + ), + ) + + def test_tls_client_minimum_1_point_3_exists(self): + """ + If TLS 1.3 support exists and it's configured, it will be settable. + """ + # thanks i hate it, still + if not hasattr(SSL, "OP_NO_TLSv1_3"): + SSL.OP_NO_TLSv1_3 = 0x00 + self.addCleanup(lambda: delattr(SSL, "OP_NO_TLSv1_3")) + assert hasattr(SSL, "OP_NO_TLSv1_3") + + config = {"federation_minimum_tls_client_version": 1.3} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + self.assertEqual(t.federation_minimum_tls_client_version, 1.3) + + def test_tls_client_minimum_set_passed_through(self): + """ + The configured TLS version is correctly configured by the ContextFactory. + """ + config = {"federation_minimum_tls_client_version": 1.2} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + + cf = ContextFactory(t) + + # The context has had NO_TLSv1_1 set, but not NO_TLSv1_2 + self.assertNotEqual(cf._verify_ssl_context._options & SSL.OP_NO_TLSv1_1, 0) + self.assertEqual(cf._verify_ssl_context._options & SSL.OP_NO_TLSv1_2, 0) From 2b41967a65dd527f6af8752fa5c3844a0f84afd1 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Tue, 25 Jun 2019 15:46:24 +1000 Subject: [PATCH 2/9] changelog --- changelog.d/5550.feature | 1 + changelog.d/5550.misc | 1 + 2 files changed, 2 insertions(+) create mode 100644 changelog.d/5550.feature create mode 100644 changelog.d/5550.misc diff --git a/changelog.d/5550.feature b/changelog.d/5550.feature new file mode 100644 index 000000000000..2fbdb18fe748 --- /dev/null +++ b/changelog.d/5550.feature @@ -0,0 +1 @@ +The minimum TLS version used for outgoing federation requests can now be set with `federation_minimum_tls_client_version`. diff --git a/changelog.d/5550.misc b/changelog.d/5550.misc new file mode 100644 index 000000000000..ad5693338e17 --- /dev/null +++ b/changelog.d/5550.misc @@ -0,0 +1 @@ +Synapse will now only allow TLS v1.2 connections when serving federation, if it terminates TLS. As Synapse's allowed ciphers were only able to be used in TLSv1.2 before, this does not change behaviour. From 5f1ab1ba383b48fc25bf5aab2dfaeb38765fbc0c Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Tue, 25 Jun 2019 16:02:03 +1000 Subject: [PATCH 3/9] fixes --- docs/sample_config.yaml | 57 +++++++++++++++---------------- synapse/crypto/context_factory.py | 10 +++--- tests/config/test_tls.py | 11 +++--- 3 files changed, 40 insertions(+), 38 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index da10788e96c3..4d7e6f3eb5ac 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -23,6 +23,29 @@ server_name: "SERVERNAME" # pid_file: DATADIR/homeserver.pid +# CPU affinity mask. Setting this restricts the CPUs on which the +# process will be scheduled. It is represented as a bitmask, with the +# lowest order bit corresponding to the first logical CPU and the +# highest order bit corresponding to the last logical CPU. Not all CPUs +# may exist on a given system but a mask may specify more CPUs than are +# present. +# +# For example: +# 0x00000001 is processor #0, +# 0x00000003 is processors #0 and #1, +# 0xFFFFFFFF is all processors (#0 through #31). +# +# Pinning a Python process to a single CPU is desirable, because Python +# is inherently single-threaded due to the GIL, and can suffer a +# 30-40% slowdown due to cache blow-out and thread context switching +# if the scheduler happens to schedule the underlying threads across +# different cores. See +# https://www.mirantis.com/blog/improve-performance-python-programs-restricting-single-cpu/. +# +# This setting requires the affinity package to be installed! +# +#cpu_affinity: 0xFFFFFFFF + # The path to the web client which will be served at /_matrix/client/ # if 'webclient' is configured under the 'listeners' configuration. # @@ -54,15 +77,11 @@ pid_file: DATADIR/homeserver.pid # #require_auth_for_profile_requests: true -# If set to 'false', requires authentication to access the server's public rooms -# directory through the client API. Defaults to 'true'. -# -#allow_public_rooms_without_auth: false - -# If set to 'false', forbids any other homeserver to fetch the server's public -# rooms directory via federation. Defaults to 'true'. +# If set to 'true', requires authentication to access the server's +# public rooms directory through the client API, and forbids any other +# homeserver to fetch it via federation. Defaults to 'false'. # -#allow_public_rooms_over_federation: false +#restrict_public_rooms_to_local_users: true # The default room version for newly created rooms. # @@ -213,7 +232,7 @@ listeners: - names: [client, federation] compress: false - # example additional_resources: + # example additonal_resources: # #additional_resources: # "/_matrix/my/custom/endpoint": @@ -406,13 +425,6 @@ acme: # #domain: matrix.example.com - # file to use for the account key. This will be generated if it doesn't - # exist. - # - # If unspecified, we will use CONFDIR/client.key. - # - account_key_file: DATADIR/acme_account.key - # List of allowed TLS fingerprints for this server to publish along # with the signing keys for this server. Other matrix servers that # make HTTPS requests to this server will check that the TLS @@ -1339,16 +1351,3 @@ password_config: # alias: "*" # room_id: "*" # action: allow - - -# Server admins can define a Python module that implements extra rules for -# allowing or denying incoming events. In order to work, this module needs to -# override the methods defined in synapse/events/third_party_rules.py. -# -# This feature is designed to be used in closed federations only, where each -# participating server enforces the same rules. -# -#third_party_event_rules: -# module: "my_custom_project.SuperRulesSet" -# config: -# example_option: 'things' diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 927505fc578f..bb0f19fbd1fd 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -102,14 +102,16 @@ def __init__(self, config): # let us do). minTLS = _TLS_VERSION_MAP[config.federation_minimum_tls_client_version] - self._verify_ssl_context = CertificateOptions( + self._verify_ssl = CertificateOptions( trustRoot=trust_root, insecurelyLowerMinimumTo=minTLS - ).getContext() + ) + self._verify_ssl_context = self._verify_ssl.getContext() self._verify_ssl_context.set_info_callback(self._context_info_cb) - self._no_verify_ssl_context = CertificateOptions( + self._no_verify_ssl = CertificateOptions( insecurelyLowerMinimumTo=minTLS - ).getContext() + ) + self._no_verify_ssl_context = self._no_verify_ssl.getContext() self._no_verify_ssl_context.set_info_callback(self._context_info_cb) def get_options(self, host): diff --git a/tests/config/test_tls.py b/tests/config/test_tls.py index 76e63315f6cb..481a3c9eea94 100644 --- a/tests/config/test_tls.py +++ b/tests/config/test_tls.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2019 New Vector Ltd +# Copyright 2019 Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -18,7 +19,7 @@ from OpenSSL import SSL from synapse.config.tls import ConfigError, TlsConfig -from synapse.crypto.context_factory import ContextFactory +from synapse.crypto.context_factory import ClientTLSOptionsFactory from tests.unittest import TestCase @@ -132,7 +133,7 @@ def test_tls_client_minimum_1_point_3_missing(self): if hasattr(SSL, "OP_NO_TLSv1_3"): OP_NO_TLSv1_3 = SSL.OP_NO_TLSv1_3 delattr(SSL, "OP_NO_TLSv1_3") - self.addCleanup(setattr, SSL, "SSL.OP_NO_TLSv1_3", SSL.OP_NO_TLSv1_3) + self.addCleanup(setattr, SSL, "SSL.OP_NO_TLSv1_3", OP_NO_TLSv1_3) assert not hasattr(SSL, "OP_NO_TLSv1_3") config = {"federation_minimum_tls_client_version": 1.3} @@ -170,8 +171,8 @@ def test_tls_client_minimum_set_passed_through(self): t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - cf = ContextFactory(t) + cf = ClientTLSOptionsFactory(t) # The context has had NO_TLSv1_1 set, but not NO_TLSv1_2 - self.assertNotEqual(cf._verify_ssl_context._options & SSL.OP_NO_TLSv1_1, 0) - self.assertEqual(cf._verify_ssl_context._options & SSL.OP_NO_TLSv1_2, 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) From 319004361ff37af10c45e3d13914a29f92ad7501 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Tue, 25 Jun 2019 16:05:46 +1000 Subject: [PATCH 4/9] fix, and make it use python3 for building config --- docs/sample_config.yaml | 66 ++++++++++++++++++++++++----------------- scripts/generate_config | 2 +- 2 files changed, 39 insertions(+), 29 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 4d7e6f3eb5ac..b856e6b611a8 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -23,29 +23,6 @@ server_name: "SERVERNAME" # pid_file: DATADIR/homeserver.pid -# CPU affinity mask. Setting this restricts the CPUs on which the -# process will be scheduled. It is represented as a bitmask, with the -# lowest order bit corresponding to the first logical CPU and the -# highest order bit corresponding to the last logical CPU. Not all CPUs -# may exist on a given system but a mask may specify more CPUs than are -# present. -# -# For example: -# 0x00000001 is processor #0, -# 0x00000003 is processors #0 and #1, -# 0xFFFFFFFF is all processors (#0 through #31). -# -# Pinning a Python process to a single CPU is desirable, because Python -# is inherently single-threaded due to the GIL, and can suffer a -# 30-40% slowdown due to cache blow-out and thread context switching -# if the scheduler happens to schedule the underlying threads across -# different cores. See -# https://www.mirantis.com/blog/improve-performance-python-programs-restricting-single-cpu/. -# -# This setting requires the affinity package to be installed! -# -#cpu_affinity: 0xFFFFFFFF - # The path to the web client which will be served at /_matrix/client/ # if 'webclient' is configured under the 'listeners' configuration. # @@ -77,11 +54,15 @@ pid_file: DATADIR/homeserver.pid # #require_auth_for_profile_requests: true -# If set to 'true', requires authentication to access the server's -# public rooms directory through the client API, and forbids any other -# homeserver to fetch it via federation. Defaults to 'false'. +# If set to 'false', requires authentication to access the server's public rooms +# directory through the client API. Defaults to 'true'. # -#restrict_public_rooms_to_local_users: true +#allow_public_rooms_without_auth: false + +# If set to 'false', forbids any other homeserver to fetch the server's public +# rooms directory via federation. Defaults to 'true'. +# +#allow_public_rooms_over_federation: false # The default room version for newly created rooms. # @@ -232,7 +213,7 @@ listeners: - names: [client, federation] compress: false - # example additonal_resources: + # example additional_resources: # #additional_resources: # "/_matrix/my/custom/endpoint": @@ -336,6 +317,15 @@ listeners: # #federation_verify_certificates: false +# The minimum TLS version that will be used for outbound federation requests. +# +# Defaults to `1.0`. Configurable to `1.0`, `1.1`, `1.2`, or `1.3`. Note +# that setting this value higher than `1.2` will prevent federation to most +# of the public Matrix network, only configure it to `1.3` if you have an +# entirely private federation setup and you can ensure TLS 1.3 support. +# +#federation_minimum_tls_client_version: 1.2 + # Skip federation certificate verification on the following whitelist # of domains. # @@ -425,6 +415,13 @@ acme: # #domain: matrix.example.com + # file to use for the account key. This will be generated if it doesn't + # exist. + # + # If unspecified, we will use CONFDIR/client.key. + # + account_key_file: DATADIR/acme_account.key + # List of allowed TLS fingerprints for this server to publish along # with the signing keys for this server. Other matrix servers that # make HTTPS requests to this server will check that the TLS @@ -1351,3 +1348,16 @@ password_config: # alias: "*" # room_id: "*" # action: allow + + +# Server admins can define a Python module that implements extra rules for +# allowing or denying incoming events. In order to work, this module needs to +# override the methods defined in synapse/events/third_party_rules.py. +# +# This feature is designed to be used in closed federations only, where each +# participating server enforces the same rules. +# +#third_party_event_rules: +# module: "my_custom_project.SuperRulesSet" +# config: +# example_option: 'things' diff --git a/scripts/generate_config b/scripts/generate_config index 93b64069922f..771cbf8d95ad 100755 --- a/scripts/generate_config +++ b/scripts/generate_config @@ -1,4 +1,4 @@ -#!/usr/bin/env python +#!/usr/bin/env python3 import argparse import shutil From fd71127074659101199e1b949075c4444a4f9533 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Tue, 25 Jun 2019 16:06:07 +1000 Subject: [PATCH 5/9] fix, as well --- synapse/crypto/context_factory.py | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index bb0f19fbd1fd..e623f7d07ad5 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -108,9 +108,7 @@ def __init__(self, config): self._verify_ssl_context = self._verify_ssl.getContext() self._verify_ssl_context.set_info_callback(self._context_info_cb) - self._no_verify_ssl = CertificateOptions( - insecurelyLowerMinimumTo=minTLS - ) + 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) From c50c06e13f323457f622b94e81b8b54e4cceaa4c Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Wed, 26 Jun 2019 15:01:51 +1000 Subject: [PATCH 6/9] fixes --- synapse/config/_base.py | 16 ------------ synapse/config/tls.py | 13 +++++----- synapse/crypto/context_factory.py | 8 +++--- tests/config/test_tls.py | 41 +++++++++++++++++++++---------- 4 files changed, 38 insertions(+), 40 deletions(-) diff --git a/synapse/config/_base.py b/synapse/config/_base.py index daea536dd7d2..965478d8d59e 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -82,22 +82,6 @@ def parse_duration(value): size = sizes[suffix] return int(value) * size - @staticmethod - def parse_float(value, config_name): - """ - Parse a value to a float, raising a configuration error if it is not. - """ - if isinstance(value, (int, float)): - return value - - if isinstance(value, str): - try: - return float(value) - except ValueError: - raise ConfigError("%s value must be a number" % (config_name)) - else: - raise ConfigError("%s value must be a number" % (config_name)) - @staticmethod def abspath(file_path): return os.path.abspath(file_path) if file_path else file_path diff --git a/synapse/config/tls.py b/synapse/config/tls.py index fe1270d1d15c..818f7231c6b1 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -82,19 +82,18 @@ def read_config(self, config, config_dir_path, **kwargs): ) # Minimum TLS version to use for outbound federation traffic - self.federation_minimum_tls_client_version = self.parse_float( - config.get("federation_minimum_tls_client_version", 1.0), - "federation_minimum_tls_client_version", + self.federation_minimum_tls_client_version = str( + config.get("federation_minimum_tls_client_version", 1) ) - if self.federation_minimum_tls_client_version not in [1.0, 1.1, 1.2, 1.3]: + if self.federation_minimum_tls_client_version not in ["1", "1.1", "1.2", "1.3"]: raise ConfigError( - "federation_minimum_tls_client_version must be one of: 1.0, 1.1, 1.2, 1.3" + "federation_minimum_tls_client_version must be one of: 1, 1.1, 1.2, 1.3" ) # Prevent people shooting themselves in the foot here by setting it to # the biggest number blindly - if self.federation_minimum_tls_client_version == 1.3: + if self.federation_minimum_tls_client_version == "1.3": if getattr(SSL, "OP_NO_TLSv1_3", None) is None: raise ConfigError( ( @@ -285,7 +284,7 @@ def generate_config_section( # The minimum TLS version that will be used for outbound federation requests. # - # Defaults to `1.0`. Configurable to `1.0`, `1.1`, `1.2`, or `1.3`. Note + # Defaults to `1`. Configurable to `1`, `1.1`, `1.2`, or `1.3`. Note # that setting this value higher than `1.2` will prevent federation to most # of the public Matrix network, only configure it to `1.3` if you have an # entirely private federation setup and you can ensure TLS 1.3 support. diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index e623f7d07ad5..2097dd3800b4 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -36,10 +36,10 @@ _TLS_VERSION_MAP = { - 1.0: TLSVersion.TLSv1_0, - 1.1: TLSVersion.TLSv1_1, - 1.2: TLSVersion.TLSv1_2, - 1.3: TLSVersion.TLSv1_3, + "1": TLSVersion.TLSv1_0, + "1.1": TLSVersion.TLSv1_1, + "1.2": TLSVersion.TLSv1_2, + "1.3": TLSVersion.TLSv1_3, } diff --git a/tests/config/test_tls.py b/tests/config/test_tls.py index 481a3c9eea94..3c51b386d565 100644 --- a/tests/config/test_tls.py +++ b/tests/config/test_tls.py @@ -91,38 +91,37 @@ def test_tls_client_minimum_default(self): t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, 1.0) + self.assertEqual(t.federation_minimum_tls_client_version, "1") def test_tls_client_minimum_set(self): """ The default client TLS version can be set to 1.0, 1.1, and 1.2. """ - config = {"federation_minimum_tls_client_version": 1.0} + config = {"federation_minimum_tls_client_version": 1} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, 1.0) + self.assertEqual(t.federation_minimum_tls_client_version, "1") config = {"federation_minimum_tls_client_version": 1.1} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, 1.1) + self.assertEqual(t.federation_minimum_tls_client_version, "1.1") config = {"federation_minimum_tls_client_version": 1.2} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, 1.2) + self.assertEqual(t.federation_minimum_tls_client_version, "1.2") # Also test a string version - config = {"federation_minimum_tls_client_version": "1.0"} + config = {"federation_minimum_tls_client_version": "1"} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, 1.0) + self.assertEqual(t.federation_minimum_tls_client_version, "1") - # Also just 1 for 1.0 - config = {"federation_minimum_tls_client_version": "1"} + config = {"federation_minimum_tls_client_version": "1.2"} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, 1.0) + self.assertEqual(t.federation_minimum_tls_client_version, "1.2") def test_tls_client_minimum_1_point_3_missing(self): """ @@ -161,9 +160,9 @@ def test_tls_client_minimum_1_point_3_exists(self): config = {"federation_minimum_tls_client_version": 1.3} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, 1.3) + self.assertEqual(t.federation_minimum_tls_client_version, "1.3") - def test_tls_client_minimum_set_passed_through(self): + def test_tls_client_minimum_set_passed_through_1_2(self): """ The configured TLS version is correctly configured by the ContextFactory. """ @@ -173,6 +172,22 @@ def test_tls_client_minimum_set_passed_through(self): cf = ClientTLSOptionsFactory(t) - # The context has had NO_TLSv1_1 set, but not NO_TLSv1_2 + # 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) + + def test_tls_client_minimum_set_passed_through_1_0(self): + """ + The configured TLS version is correctly configured by the ContextFactory. + """ + config = {"federation_minimum_tls_client_version": 1} + t = TestConfig() + t.read_config(config, config_dir_path="", data_dir_path="") + + cf = ClientTLSOptionsFactory(t) + + # 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) From 1872924676ea1d4f643ef691b4d06fd3da6de2cc Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Wed, 26 Jun 2019 15:04:55 +1000 Subject: [PATCH 7/9] fixes --- docs/sample_config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index b856e6b611a8..376cf58d9370 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -319,7 +319,7 @@ listeners: # The minimum TLS version that will be used for outbound federation requests. # -# Defaults to `1.0`. Configurable to `1.0`, `1.1`, `1.2`, or `1.3`. Note +# Defaults to `1`. Configurable to `1`, `1.1`, `1.2`, or `1.3`. Note # that setting this value higher than `1.2` will prevent federation to most # of the public Matrix network, only configure it to `1.3` if you have an # entirely private federation setup and you can ensure TLS 1.3 support. From ed2dd72367f5e1acc293daef5b3905e38f6ae443 Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Thu, 27 Jun 2019 19:07:05 +1000 Subject: [PATCH 8/9] Update synapse/config/tls.py Co-Authored-By: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> --- synapse/config/tls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 818f7231c6b1..0f1c42ce9e72 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -286,7 +286,7 @@ def generate_config_section( # # Defaults to `1`. Configurable to `1`, `1.1`, `1.2`, or `1.3`. Note # that setting this value higher than `1.2` will prevent federation to most - # of the public Matrix network, only configure it to `1.3` if you have an + # of the public Matrix network: only configure it to `1.3` if you have an # entirely private federation setup and you can ensure TLS 1.3 support. # #federation_minimum_tls_client_version: 1.2 From e34140316e810981b6c6794e450efaca44bf37d5 Mon Sep 17 00:00:00 2001 From: "Amber H. Brown" Date: Fri, 28 Jun 2019 17:55:21 +1000 Subject: [PATCH 9/9] change config name to be better --- changelog.d/5550.feature | 2 +- docs/sample_config.yaml | 4 ++-- synapse/config/tls.py | 14 ++++++------- synapse/crypto/context_factory.py | 2 +- tests/config/test_tls.py | 34 +++++++++++++++---------------- 5 files changed, 28 insertions(+), 28 deletions(-) diff --git a/changelog.d/5550.feature b/changelog.d/5550.feature index 2fbdb18fe748..79ecedf3b8bb 100644 --- a/changelog.d/5550.feature +++ b/changelog.d/5550.feature @@ -1 +1 @@ -The minimum TLS version used for outgoing federation requests can now be set with `federation_minimum_tls_client_version`. +The minimum TLS version used for outgoing federation requests can now be set with `federation_client_minimum_tls_version`. diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 376cf58d9370..53d4ffacb93c 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -321,10 +321,10 @@ listeners: # # Defaults to `1`. Configurable to `1`, `1.1`, `1.2`, or `1.3`. Note # that setting this value higher than `1.2` will prevent federation to most -# of the public Matrix network, only configure it to `1.3` if you have an +# of the public Matrix network: only configure it to `1.3` if you have an # entirely private federation setup and you can ensure TLS 1.3 support. # -#federation_minimum_tls_client_version: 1.2 +#federation_client_minimum_tls_version: 1.2 # Skip federation certificate verification on the following whitelist # of domains. diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 0f1c42ce9e72..ca508a224fa7 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -82,22 +82,22 @@ def read_config(self, config, config_dir_path, **kwargs): ) # Minimum TLS version to use for outbound federation traffic - self.federation_minimum_tls_client_version = str( - config.get("federation_minimum_tls_client_version", 1) + self.federation_client_minimum_tls_version = str( + config.get("federation_client_minimum_tls_version", 1) ) - if self.federation_minimum_tls_client_version not in ["1", "1.1", "1.2", "1.3"]: + if self.federation_client_minimum_tls_version not in ["1", "1.1", "1.2", "1.3"]: raise ConfigError( - "federation_minimum_tls_client_version must be one of: 1, 1.1, 1.2, 1.3" + "federation_client_minimum_tls_version must be one of: 1, 1.1, 1.2, 1.3" ) # Prevent people shooting themselves in the foot here by setting it to # the biggest number blindly - if self.federation_minimum_tls_client_version == "1.3": + if self.federation_client_minimum_tls_version == "1.3": if getattr(SSL, "OP_NO_TLSv1_3", None) is None: raise ConfigError( ( - "federation_minimum_tls_client_version cannot be 1.3, " + "federation_client_minimum_tls_version cannot be 1.3, " "your OpenSSL does not support it" ) ) @@ -289,7 +289,7 @@ def generate_config_section( # of the public Matrix network: only configure it to `1.3` if you have an # entirely private federation setup and you can ensure TLS 1.3 support. # - #federation_minimum_tls_client_version: 1.2 + #federation_client_minimum_tls_version: 1.2 # Skip federation certificate verification on the following whitelist # of domains. diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py index 2097dd3800b4..4f48e8e88d51 100644 --- a/synapse/crypto/context_factory.py +++ b/synapse/crypto/context_factory.py @@ -100,7 +100,7 @@ def __init__(self, config): # moving to TLS 1.2 by default, we want to respect the config option if # it is set to 1.0 (which the alternate option, raiseMinimumTo, will not # let us do). - minTLS = _TLS_VERSION_MAP[config.federation_minimum_tls_client_version] + minTLS = _TLS_VERSION_MAP[config.federation_client_minimum_tls_version] self._verify_ssl = CertificateOptions( trustRoot=trust_root, insecurelyLowerMinimumTo=minTLS diff --git a/tests/config/test_tls.py b/tests/config/test_tls.py index 3c51b386d565..4f8a87a3dfde 100644 --- a/tests/config/test_tls.py +++ b/tests/config/test_tls.py @@ -91,37 +91,37 @@ def test_tls_client_minimum_default(self): t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, "1") + self.assertEqual(t.federation_client_minimum_tls_version, "1") def test_tls_client_minimum_set(self): """ The default client TLS version can be set to 1.0, 1.1, and 1.2. """ - config = {"federation_minimum_tls_client_version": 1} + config = {"federation_client_minimum_tls_version": 1} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, "1") + self.assertEqual(t.federation_client_minimum_tls_version, "1") - config = {"federation_minimum_tls_client_version": 1.1} + config = {"federation_client_minimum_tls_version": 1.1} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, "1.1") + self.assertEqual(t.federation_client_minimum_tls_version, "1.1") - config = {"federation_minimum_tls_client_version": 1.2} + config = {"federation_client_minimum_tls_version": 1.2} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, "1.2") + self.assertEqual(t.federation_client_minimum_tls_version, "1.2") # Also test a string version - config = {"federation_minimum_tls_client_version": "1"} + config = {"federation_client_minimum_tls_version": "1"} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, "1") + self.assertEqual(t.federation_client_minimum_tls_version, "1") - config = {"federation_minimum_tls_client_version": "1.2"} + config = {"federation_client_minimum_tls_version": "1.2"} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, "1.2") + self.assertEqual(t.federation_client_minimum_tls_version, "1.2") def test_tls_client_minimum_1_point_3_missing(self): """ @@ -135,14 +135,14 @@ def test_tls_client_minimum_1_point_3_missing(self): self.addCleanup(setattr, SSL, "SSL.OP_NO_TLSv1_3", OP_NO_TLSv1_3) assert not hasattr(SSL, "OP_NO_TLSv1_3") - config = {"federation_minimum_tls_client_version": 1.3} + config = {"federation_client_minimum_tls_version": 1.3} t = TestConfig() with self.assertRaises(ConfigError) as e: t.read_config(config, config_dir_path="", data_dir_path="") self.assertEqual( e.exception.args[0], ( - "federation_minimum_tls_client_version cannot be 1.3, " + "federation_client_minimum_tls_version cannot be 1.3, " "your OpenSSL does not support it" ), ) @@ -157,16 +157,16 @@ def test_tls_client_minimum_1_point_3_exists(self): self.addCleanup(lambda: delattr(SSL, "OP_NO_TLSv1_3")) assert hasattr(SSL, "OP_NO_TLSv1_3") - config = {"federation_minimum_tls_client_version": 1.3} + config = {"federation_client_minimum_tls_version": 1.3} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") - self.assertEqual(t.federation_minimum_tls_client_version, "1.3") + self.assertEqual(t.federation_client_minimum_tls_version, "1.3") def test_tls_client_minimum_set_passed_through_1_2(self): """ The configured TLS version is correctly configured by the ContextFactory. """ - config = {"federation_minimum_tls_client_version": 1.2} + config = {"federation_client_minimum_tls_version": 1.2} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="") @@ -181,7 +181,7 @@ def test_tls_client_minimum_set_passed_through_1_0(self): """ The configured TLS version is correctly configured by the ContextFactory. """ - config = {"federation_minimum_tls_client_version": 1} + config = {"federation_client_minimum_tls_version": 1} t = TestConfig() t.read_config(config, config_dir_path="", data_dir_path="")