Config option for verifying federation certificates (MSC 1711)

docs/MSC1711_certificates_FAQ.md

@@ -177,28 +177,6 @@ You can do this with a `.well-known` file as follows:
        on `customer.example.net:8000` it correctly handles HTTP requests with
        Host header set to `customer.example.net:8000`.
 
-## Turning off certificate validation
-
-It is possible to turn off certificate validation for remote servers, but
-note that this must be explicitly enabled and is thus only suitable for
-private federations. This will only disable TLS certificate validation on
-federation endpoints; other requests made to recaptcha, identity services
-etc. will be unaffected.
-
-```
-federation_verify_certificates = false
-```
-
-You can also only disable certificate validation for a specific set of
-homeservers:
-
-```
-federation_certificate_verification_whitelist:
-  - subdomain.my-server.org
-  - example.org
-  - 1.2.3.4
-```
-
 ## Specifying your own Certificate Authorities
 
 If you would like to specify your own list of trusted Certificate

docs/sample_config.yaml

@@ -259,12 +259,20 @@ listeners:
 
 # Whether to verify TLS certificates when sending federation traffic.
 #
+# This currently defaults to `false`, however this will change in
+# Synapse 1.0 when valid federation certificates will be required.
+#
 #federation_verify_certificates: true
 
-# Prevent federation certificate validation on the following whitelist
-# of domains. Only effective if federation_verify_certicates is true.
+# Skip federation certificate verification on the following whitelist
+# of domains.
+#
+# Note that this should only be used within the context of private
+# federation as it will otherwise break things.
+#
+# Only effective if federation_verify_certicates is `true`.
 #
-#federation_certificate_validation_whitelist:
+#federation_certificate_verification_whitelist:
 #  - lon.example.com
 #  - nyc.example.com
 #  - syd.example.com

synapse/config/server.py

@@ -111,15 +111,17 @@ def read_config(self, config):
         self.admin_contact = config.get("admin_contact", None)
 
         # FIXME: federation_domain_whitelist needs sytests
-        self.federation_domain_whitelist = None
         federation_domain_whitelist = config.get(
-            "federation_domain_whitelist", None
+            "federation_domain_whitelist", [],
         )
-        # turn the whitelist into a hash for speed of lookup
-        if federation_domain_whitelist is not None:
+
+        self.federation_domain_whitelist = None
+        if len(federation_domain_whitelist) > 0:
             self.federation_domain_whitelist = {}
-            for domain in federation_domain_whitelist:
-                self.federation_domain_whitelist[domain] = True
+
+        # turn the whitelist into a hash for speed of lookup
+        for domain in federation_domain_whitelist:
+            self.federation_domain_whitelist[domain] = True
 
         if self.public_baseurl is not None:
             if self.public_baseurl[-1] != '/':

synapse/config/tls.py

@@ -77,16 +77,17 @@ def read_config(self, config):
         )
 
         # Whitelist of domains to not verify certificates for
-        self.federation_certificate_verification_whitelist = None
         federation_certificate_verification_whitelist = config.get(
-            "federation_certificate_verification_whitelist", None
+            "federation_certificate_verification_whitelist", [],
         )
 
-        # Store whitelisted domains in a hash for fast lookup
-        if federation_certificate_verification_whitelist is not None:
+        self.federation_certificate_verification_whitelist = None
+        if len(federation_certificate_verification_whitelist) > 0:
             self.federation_certificate_verification_whitelist = {}
-            for domain in federation_certificate_verification_whitelist:
-                self.federation_certificate_verification_whitelist[domain] = True
+
+        # Store whitelisted domains in a hash for fast lookup
+        for domain in federation_certificate_verification_whitelist:
+            self.federation_certificate_verification_whitelist[domain] = True
 
         # List of custom certificate authorities for federation traffic validation
         self.federation_custom_ca_list = config.get(
@@ -95,26 +96,24 @@ def read_config(self, config):
 
         # Read in and parse custom CA certificates
         if self.federation_custom_ca_list is not None:
+            if self.federation_custom_ca_list:
+                raise ConfigError("federation_custom_ca_list specified without "
+                                  "any certificate files")
+
             certs = []
             for ca_file in self.federation_custom_ca_list:
                 logger.debug("Reading custom CA certificate file: %s", ca_file)
-                try:
-                    with open(ca_file, 'rb') as f:
-                        content = f.read()
-                except Exception:
-                    logger.exception("Failed to read custom CA certificate off disk!")
-                    raise
+                content = self.read_file(ca_file)
 
                 # Parse the CA certificates
                 try:
                     cert_base = Certificate.loadPEM(content)
                     certs.append(cert_base)
-                except Exception:
-                    logger.exception("Failed to parse custom CA certificate off disk!")
-                    raise
+                except Exception as e:
+                    raise ConfigError("Error parsing custom CA certificate file %s: %s"
+                                      % (ca_file, e))
 
-            if len(certs) > 0:
-                self.federation_custom_ca_list = trustRootFromCertificates(certs)
+            self.federation_custom_ca_list = trustRootFromCertificates(certs)
 
         # This config option applies to non-federation HTTP clients
         # (e.g. for talking to recaptcha, identity servers, and such)
@@ -146,14 +145,12 @@ def is_disk_cert_valid(self, allow_self_signed=True):
             with open(self.tls_certificate_file, 'rb') as f:
                 cert_pem = f.read()
         except Exception:
-            logger.exception("Failed to read existing certificate off disk!")
-            raise
+            logger.fatal("Failed to read existing certificate off disk")
 
         try:
             tls_certificate = crypto.load_certificate(crypto.FILETYPE_PEM, cert_pem)
         except Exception:
-            logger.exception("Failed to parse existing certificate off disk!")
-            raise
+            logger.fatal("Failed to parse existing certificate off disk")
 
         if not allow_self_signed:
             if tls_certificate.get_subject() == tls_certificate.get_issuer():
@@ -240,12 +237,20 @@ def default_config(self, config_dir_path, server_name, **kwargs):
 
         # Whether to verify TLS certificates when sending federation traffic.
         #
+        # This currently defaults to `false`, however this will change in
+        # Synapse 1.0 when valid federation certificates will be required.
+        #
         #federation_verify_certificates: true
 
-        # Prevent federation certificate validation on the following whitelist
-        # of domains. Only effective if federation_verify_certicates is true.
+        # Skip federation certificate verification on the following whitelist
+        # of domains.
+        #
+        # Note that this should only be used within the context of private
+        # federation as it will otherwise break things.
+        #
+        # Only effective if federation_verify_certicates is `true`.
         #
-        #federation_certificate_validation_whitelist:
+        #federation_certificate_verification_whitelist:
         #  - lon.example.com
         #  - nyc.example.com
         #  - syd.example.com

synapse/crypto/context_factory.py

@@ -18,7 +18,10 @@
 from zope.interface import implementer
 
 from OpenSSL import SSL, crypto
-from twisted.internet._sslverify import OpenSSLCertificateAuthorities, _defaultCurveName
+from twisted.internet._sslverify import (
+    ClientTLSOptions as ClientTLSOptionsVerify,
+    _defaultCurveName,
+)
 from twisted.internet.abstract import isIPAddress, isIPv6Address
 from twisted.internet.interfaces import IOpenSSLClientConnectionCreator
 from twisted.internet.ssl import CertificateOptions, ContextFactory, platformTrust
@@ -128,19 +131,18 @@ class ClientTLSOptionsFactory(object):
 
     def __init__(self, config):
         self._config = config
-
-        self._options_novalidate = CertificateOptions()
+        self._options_noverify = CertificateOptions()
 
         # Check if we're using a custom list of a CA certificates
-        if isinstance(config.federation_custom_ca_list, OpenSSLCertificateAuthorities):
-            self._options_validate = CertificateOptions(
+        if config.federation_custom_ca_list is not None:
+            self._options_verify = CertificateOptions(
                 # Use custom CA trusted root certs
                 trustRoot=config.federation_custom_ca_list,
             )
             return
 
         # If not, verify using those provided by the operating environment
-        self._options_validate = CertificateOptions(
+        self._options_verify = CertificateOptions(
             # Use CA root certs provided by OpenSSL
             trustRoot=platformTrust(),
         )
@@ -149,10 +151,13 @@ def get_options(self, host):
         # Use _makeContext so that we get a fresh OpenSSL CTX each time.
 
         # Check if certificate verification has been enabled
-        if (self._config.federation_verify_certificates and
-                host not in self._config.federation_certificate_validation_whitelist):
-            # Require verification
-            return ClientTLSOptions(host, self._options_validate._makeContext())
+        if (self._config.federation_verify_certificates):
+            # and if the host is whitelisted against it
+            if (self._config.federation_certificate_verification_whitelist and
+                    host in self._config.federation_certificate_verification_whitelist):
+                return ClientTLSOptions(host, self._options_noverify._makeContext())
+
+            return ClientTLSOptionsVerify(host, self._options_verify._makeContext())
 
         # Otherwise don't require verification
-        return ClientTLSOptions(host, self._options_novalidate._makeContext())
+        return ClientTLSOptions(host, self._options_noverify._makeContext())

synapse/federation/transport/server.py

@@ -127,10 +127,8 @@ def authenticate_request(self, request, content):
                 json_request["origin"] = origin
                 json_request["signatures"].setdefault(origin, {})[key] = sig
 
-        if (
-            self.federation_domain_whitelist is not None and
-            origin not in self.federation_domain_whitelist
-        ):
+        if (self.federation_domain_whitelist is not None and
+                origin not in self.federation_domain_whitelist):
             raise FederationDeniedError(origin)
 
         if not json_request["signatures"]:

synapse/http/matrixfederationclient.py

@@ -177,7 +177,6 @@ def __init__(self, hs, tls_client_options_factory):
         self.agent = MatrixFederationAgent(
             hs.get_reactor(),
             tls_client_options_factory,
-            hs.config,
         )
         self.clock = hs.get_clock()
         self._store = hs.get_datastore()
@@ -284,10 +283,8 @@ def _send_request(
         else:
             _sec_timeout = self.default_timeout
 
-        if (
-            self.hs.config.federation_domain_whitelist is not None and
-            request.destination not in self.hs.config.federation_domain_whitelist
-        ):
+        if (self.hs.config.federation_domain_whitelist is not None and
+                request.destination not in self.hs.config.federation_domain_whitelist):
             raise FederationDeniedError(request.destination)
 
         limiter = yield synapse.util.retryutils.get_retry_limiter(

synapse/rest/key/v2/remote_key_resource.py

@@ -139,10 +139,8 @@ def query_keys(self, request, query, query_remote_on_cache_miss=False):
 
         store_queries = []
         for server_name, key_ids in query.items():
-            if (
-                self.federation_domain_whitelist is not None and
-                server_name not in self.federation_domain_whitelist
-            ):
+            if (self.federation_domain_whitelist is not None and
+                    server_name not in self.federation_domain_whitelist):
                 logger.debug("Federation denied with %s", server_name)
                 continue
 

synapse/rest/media/v1/media_repository.py

@@ -231,10 +231,8 @@ def get_remote_media(self, request, server_name, media_id, name):
             Deferred: Resolves once a response has successfully been written
                 to request
         """
-        if (
-            self.federation_domain_whitelist is not None and
-            server_name not in self.federation_domain_whitelist
-        ):
+        if (self.federation_domain_whitelist is not None and
+                server_name not in self.federation_domain_whitelist):
             raise FederationDeniedError(server_name)
 
         self.mark_recently_accessed(server_name, media_id)
@@ -271,10 +269,8 @@ def get_remote_media_info(self, server_name, media_id):
         Returns:
             Deferred[dict]: The media_info of the file
         """
-        if (
-            self.federation_domain_whitelist is not None and
-            server_name not in self.federation_domain_whitelist
-        ):
+        if (self.federation_domain_whitelist is not None and
+                server_name not in self.federation_domain_whitelist):
             raise FederationDeniedError(server_name)
 
         # We linearize here to ensure that we don't try and download remote

tests/http/federation/test_matrix_federation_agent.py

@@ -39,6 +39,7 @@
 from tests.http import ServerTLSContext
 from tests.server import FakeTransport, ThreadedMemoryReactorClock
 from tests.unittest import TestCase
+from tests.utils import default_config
 
 logger = logging.getLogger(__name__)
 
@@ -49,16 +50,11 @@ def setUp(self):
 
         self.mock_resolver = Mock()
 
-        config = Mock()
-        config.federation_custom_ca_list = None
-        config.federation_verify_certificates = False
-        config.federation_certificate_validation_whitelist = []
-
         self.well_known_cache = TTLCache("test_cache", timer=self.reactor.seconds)
 
         self.agent = MatrixFederationAgent(
             reactor=self.reactor,
-            tls_client_options_factory=ClientTLSOptionsFactory(config),
+            tls_client_options_factory=ClientTLSOptionsFactory(default_config("test")),
             _well_known_tls_policy=TrustingTLSPolicyForHTTPS(),
             _srv_resolver=self.mock_resolver,
             _well_known_cache=self.well_known_cache,

tests/utils.py

@@ -137,6 +137,9 @@ def default_config(name):
     config.email_enable_notifs = False
     config.block_non_admin_invites = False
     config.federation_domain_whitelist = None
+    config.federation_certificate_verification_whitelist = None
+    config.federation_custom_ca_list = None
+    config.federation_verify_certificates = False
     config.federation_rc_reject_limit = 10
     config.federation_rc_sleep_limit = 10
     config.federation_rc_sleep_delay = 100