Update the TLS cipher string and provide configurability for TLS on outgoing federation (#5550)

Author: hawkowl

changelog.d/5550.feature

@@ -0,0 +1 @@
+The minimum TLS version used for outgoing federation requests can now be set with `federation_client_minimum_tls_version`.

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.

docs/sample_config.yaml

@@ -317,6 +317,15 @@ listeners:
 #
 #federation_verify_certificates: false
 
+# The minimum TLS version that will be used for outbound federation requests.
+#
+# 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.
+#
+#federation_client_minimum_tls_version: 1.2
+
 # Skip federation certificate verification on the following whitelist
 # of domains.
 #

scripts/generate_config

@@ -1,4 +1,4 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 
 import argparse
 import shutil

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,27 @@ def read_config(self, config, config_dir_path, **kwargs):
             "federation_verify_certificates", True
         )
 
+        # Minimum TLS version to use for outbound federation traffic
+        self.federation_client_minimum_tls_version = str(
+            config.get("federation_client_minimum_tls_version", 1)
+        )
+
+        if self.federation_client_minimum_tls_version not in ["1", "1.1", "1.2", "1.3"]:
+            raise ConfigError(
+                "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_client_minimum_tls_version == "1.3":
+            if getattr(SSL, "OP_NO_TLSv1_3", None) is None:
+                raise ConfigError(
+                    (
+                        "federation_client_minimum_tls_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 +282,15 @@ def generate_config_section(
         #
         #federation_verify_certificates: false
 
+        # The minimum TLS version that will be used for outbound federation requests.
+        #
+        # 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.
+        #
+        #federation_client_minimum_tls_version: 1.2
+
         # Skip federation certificate verification on the following whitelist
         # of domains.
         #

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": 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_client_minimum_tls_version]
+
+        self._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._no_verify_ssl_context = CertificateOptions().getContext()
+        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)
 
     def get_options(self, host):

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.
@@ -15,7 +16,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 ClientTLSOptionsFactory
 
 from tests.unittest import TestCase
 
@@ -78,3 +82,112 @@ 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_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_client_minimum_tls_version": 1}
+        t = TestConfig()
+        t.read_config(config, config_dir_path="", data_dir_path="")
+        self.assertEqual(t.federation_client_minimum_tls_version, "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_client_minimum_tls_version, "1.1")
+
+        config = {"federation_client_minimum_tls_version": 1.2}
+        t = TestConfig()
+        t.read_config(config, config_dir_path="", data_dir_path="")
+        self.assertEqual(t.federation_client_minimum_tls_version, "1.2")
+
+        # Also test a string version
+        config = {"federation_client_minimum_tls_version": "1"}
+        t = TestConfig()
+        t.read_config(config, config_dir_path="", data_dir_path="")
+        self.assertEqual(t.federation_client_minimum_tls_version, "1")
+
+        config = {"federation_client_minimum_tls_version": "1.2"}
+        t = TestConfig()
+        t.read_config(config, config_dir_path="", data_dir_path="")
+        self.assertEqual(t.federation_client_minimum_tls_version, "1.2")
+
+    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", OP_NO_TLSv1_3)
+            assert not hasattr(SSL, "OP_NO_TLSv1_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_client_minimum_tls_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_client_minimum_tls_version": 1.3}
+        t = TestConfig()
+        t.read_config(config, config_dir_path="", data_dir_path="")
+        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_client_minimum_tls_version": 1.2}
+        t = TestConfig()
+        t.read_config(config, config_dir_path="", data_dir_path="")
+
+        cf = ClientTLSOptionsFactory(t)
+
+        # 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_client_minimum_tls_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)