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

7 files changed, 190 insertions, 9 deletions

diff --git a/changelog.d/5550.feature b/changelog.d/5550.feature
new file mode 100644
index 0000000000..79ecedf3b8
--- /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_client_minimum_tls_version`.
diff --git a/changelog.d/5550.misc b/changelog.d/5550.misc
new file mode 100644
index 0000000000..ad5693338e
--- /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.
diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml
index a01e1152f7..bf9cd88b15 100644
--- a/docs/sample_config.yaml
+++ b/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.
 #
diff --git a/scripts/generate_config b/scripts/generate_config
index 93b6406992..771cbf8d95 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
diff --git a/synapse/config/tls.py b/synapse/config/tls.py
index 8fcf801418..ca508a224f 100644
--- a/synapse/config/tls.py
+++ b/synapse/config/tls.py
@@ -23,7 +23,7 @@ import six
 
 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 @@ class TlsConfig(Config):
             "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 @@ class TlsConfig(Config):
         #
         #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.
         #
diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py
index 2bc5cc3807..4f48e8e88d 100644
--- a/synapse/crypto/context_factory.py
+++ b/synapse/crypto/context_factory.py
@@ -24,12 +24,25 @@ from OpenSSL import SSL, crypto
 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 @@ class ServerContextFactory(ContextFactory):
         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 @@ class ClientTLSOptionsFactory(object):
             # 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):
diff --git a/tests/config/test_tls.py b/tests/config/test_tls.py
index a5d88d644a..4f8a87a3df 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.
@@ -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 @@ s4niecZKPBizL6aucT59CsunNmmb5Glq8rlAcU+1ZTZZzGYqVYhF6axB9Qg=
                 "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)