Addressed changes

3 files changed, 27 insertions, 44 deletions

diff --git a/docs/MSC1711_certificates_FAQ.md b/docs/MSC1711_certificates_FAQ.md
index 8549f126a4..ebfb20f5c8 100644
--- a/docs/MSC1711_certificates_FAQ.md
+++ b/docs/MSC1711_certificates_FAQ.md
@@ -177,20 +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`.
 
-## Specifying your own Certificate Authorities
-
-If you would like to specify your own list of trusted Certificate
-Authorities, you can do so with the following option. **Note that this list
-will replace any certificates provided by your operating environment.**
-
-```
-federation_custom_ca_list:
-  - myCA1.pem
-  - myCA2.pem
-```
-
-Certificate files must be provided in PEM format.
-
 ## FAQ
 
 ### Synapse 0.99.0 has just been released, what do I need to do right now?
diff --git a/synapse/config/tls.py b/synapse/config/tls.py
index d157e310e4..3fc6cf9a3f 100644
--- a/synapse/config/tls.py
+++ b/synapse/config/tls.py
@@ -90,18 +90,19 @@ class TlsConfig(Config):
             self.federation_certificate_verification_whitelist[domain] = True
 
         # List of custom certificate authorities for federation traffic validation
-        self.federation_custom_ca_list = config.get(
+        custom_ca_list = config.get(
             "federation_custom_ca_list", None,
         )
 
         # Read in and parse custom CA certificates
-        if self.federation_custom_ca_list is not None:
-            if self.federation_custom_ca_list:
+        self.federation_ca_trust_root = None
+        if custom_ca_list is not None:
+            if len(custom_ca_list) == 0:
                 raise ConfigError("federation_custom_ca_list specified without "
                                   "any certificate files")
 
             certs = []
-            for ca_file in self.federation_custom_ca_list:
+            for ca_file in custom_ca_list:
                 logger.debug("Reading custom CA certificate file: %s", ca_file)
                 content = self.read_file(ca_file)
 
@@ -113,7 +114,7 @@ class TlsConfig(Config):
                     raise ConfigError("Error parsing custom CA certificate file %s: %s"
                                       % (ca_file, e))
 
-            self.federation_custom_ca_list = trustRootFromCertificates(certs)
+            self.federation_ca_trust_root = trustRootFromCertificates(certs)
 
         # This config option applies to non-federation HTTP clients
         # (e.g. for talking to recaptcha, identity servers, and such)
@@ -144,13 +145,15 @@ class TlsConfig(Config):
         try:
             with open(self.tls_certificate_file, 'rb') as f:
                 cert_pem = f.read()
-        except Exception:
-            logger.fatal("Failed to read existing certificate off disk")
+        except Exception as e:
+            raise ConfigError("Failed to read existing certificate file %s: %s"
+                              % (self.tls_certificate_file, e))
 
         try:
             tls_certificate = crypto.load_certificate(crypto.FILETYPE_PEM, cert_pem)
-        except Exception:
-            logger.fatal("Failed to parse existing certificate off disk")
+        except Exception as e:
+            raise ConfigError("Failed to parse existing certificate file %s: %s"
+                              % (self.tls_certificate_file, e))
 
         if not allow_self_signed:
             if tls_certificate.get_subject() == tls_certificate.get_issuer():
@@ -245,8 +248,8 @@ class TlsConfig(Config):
         # 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.
+        # This setting should only normally be used within a private network of
+        # homeservers.
         #
         # Only effective if federation_verify_certicates is `true`.
         #
@@ -257,6 +260,9 @@ class TlsConfig(Config):
 
         # List of custom certificate authorities for federation traffic.
         #
+        # This setting should only normally be used within a private network of
+        # homeservers.
+        #
         # Note that this list will replace those that are provided by your
         # operating environment. Certificates must be in PEM format.
         #
diff --git a/synapse/crypto/context_factory.py b/synapse/crypto/context_factory.py
index 1ee87cdd13..8a3fea043b 100644
--- a/synapse/crypto/context_factory.py
+++ b/synapse/crypto/context_factory.py
@@ -18,10 +18,7 @@ import logging
 from zope.interface import implementer
 
 from OpenSSL import SSL, crypto
-from twisted.internet._sslverify import (
-    ClientTLSOptions as ClientTLSOptionsVerify,
-    _defaultCurveName,
-)
+from twisted.internet._sslverify import ClientTLSOptions, _defaultCurveName
 from twisted.internet.abstract import isIPAddress, isIPv6Address
 from twisted.internet.interfaces import IOpenSSLClientConnectionCreator
 from twisted.internet.ssl import CertificateOptions, ContextFactory, platformTrust
@@ -93,7 +90,7 @@ def _tolerateErrors(wrapped):
 
 
 @implementer(IOpenSSLClientConnectionCreator)
-class ClientTLSOptions(object):
+class ClientTLSOptionsNoVerify(object):
     """
     Client creator for TLS without certificate identity verification. This is a
     copy of twisted.internet._sslverify.ClientTLSOptions with the identity
@@ -134,18 +131,12 @@ class ClientTLSOptionsFactory(object):
         self._options_noverify = CertificateOptions()
 
         # Check if we're using a custom list of a CA certificates
-        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_verify = CertificateOptions(
+        trust_root = config.federation_ca_trust_root
+        if trust_root is None:
             # Use CA root certs provided by OpenSSL
-            trustRoot=platformTrust(),
-        )
+            trust_root = platformTrust()
+
+        self._options_verify = CertificateOptions(trustRoot=trust_root)
 
     def get_options(self, host):
         # Use _makeContext so that we get a fresh OpenSSL CTX each time.
@@ -155,9 +146,9 @@ class ClientTLSOptionsFactory(object):
             # 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 ClientTLSOptionsNoVerify(host, self._options_noverify._makeContext())
 
-            return ClientTLSOptionsVerify(host, self._options_verify._makeContext())
+            return ClientTLSOptions(host, self._options_verify._makeContext())
 
         # Otherwise don't require verification
-        return ClientTLSOptions(host, self._options_noverify._makeContext())
+        return ClientTLSOptionsNoVerify(host, self._options_noverify._makeContext())