Infer no_tls from presence of TLS listeners

Rather than have to specify `no_tls` explicitly, infer whether we need to load
the TLS keys etc from whether we have any TLS-enabled listeners.

10 files changed, 27 insertions, 20 deletions

diff --git a/changelog.d/4613.feature b/changelog.d/4613.feature
new file mode 100644
index 0000000000..098f906af2
--- /dev/null
+++ b/changelog.d/4613.feature
@@ -0,0 +1 @@
+There is no longer any need to specify `no_tls`: it is inferred from the absence of TLS listeners
diff --git a/changelog.d/4615.feature b/changelog.d/4615.feature
new file mode 100644
index 0000000000..098f906af2
--- /dev/null
+++ b/changelog.d/4615.feature
@@ -0,0 +1 @@
+There is no longer any need to specify `no_tls`: it is inferred from the absence of TLS listeners
diff --git a/changelog.d/4615.misc b/changelog.d/4615.misc
deleted file mode 100644
index c7266fcfc7..0000000000
--- a/changelog.d/4615.misc
+++ /dev/null
@@ -1 +0,0 @@
-Logging improvements around TLS certs
diff --git a/changelog.d/4617.feature b/changelog.d/4617.feature
new file mode 100644
index 0000000000..098f906af2
--- /dev/null
+++ b/changelog.d/4617.feature
@@ -0,0 +1 @@
+There is no longer any need to specify `no_tls`: it is inferred from the absence of TLS listeners
diff --git a/changelog.d/4617.misc b/changelog.d/4617.misc
deleted file mode 100644
index 6d751865c9..0000000000
--- a/changelog.d/4617.misc
+++ /dev/null
@@ -1 +0,0 @@
-Don't create server contexts when TLS is disabled
diff --git a/synapse/app/_base.py b/synapse/app/_base.py
index 6b3cb61ae9..50fd17c0be 100644
--- a/synapse/app/_base.py
+++ b/synapse/app/_base.py
@@ -215,7 +215,7 @@ def refresh_certificate(hs):
     """
     hs.config.read_certificate_from_disk()
 
-    if hs.config.no_tls:
+    if not hs.config.has_tls_listener():
         # nothing else to do here
         return
 
diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py
index b4476bf16e..dbd98d394f 100755
--- a/synapse/app/homeserver.py
+++ b/synapse/app/homeserver.py
@@ -90,11 +90,6 @@ class SynapseHomeServer(HomeServer):
         tls = listener_config.get("tls", False)
         site_tag = listener_config.get("tag", port)
 
-        if tls and config.no_tls:
-            raise ConfigError(
-                "Listener on port %i has TLS enabled, but no_tls is set" % (port,),
-            )
-
         resources = {}
         for res in listener_config["resources"]:
             for name in res["names"]:
diff --git a/synapse/config/homeserver.py b/synapse/config/homeserver.py
index 5aad062c36..727fdc54d8 100644
--- a/synapse/config/homeserver.py
+++ b/synapse/config/homeserver.py
@@ -42,7 +42,7 @@ from .voip import VoipConfig
 from .workers import WorkerConfig
 
 
-class HomeServerConfig(TlsConfig, ServerConfig, DatabaseConfig, LoggingConfig,
+class HomeServerConfig(ServerConfig, TlsConfig, DatabaseConfig, LoggingConfig,
                        RatelimitConfig, ContentRepositoryConfig, CaptchaConfig,
                        VoipConfig, RegistrationConfig, MetricsConfig, ApiConfig,
                        AppServiceConfig, KeyConfig, SAML2Config, CasConfig,
diff --git a/synapse/config/server.py b/synapse/config/server.py
index eed9d7c81e..767897c419 100644
--- a/synapse/config/server.py
+++ b/synapse/config/server.py
@@ -126,14 +126,22 @@ class ServerConfig(Config):
                 self.public_baseurl += '/'
         self.start_pushers = config.get("start_pushers", True)
 
-        self.listeners = config.get("listeners", [])
-
-        for listener in self.listeners:
+        self.listeners = []
+        for listener in config.get("listeners", []):
             if not isinstance(listener.get("port", None), int):
                 raise ConfigError(
                     "Listener configuration is lacking a valid 'port' option"
                 )
 
+            if listener.setdefault("tls", False):
+                # no_tls is not really supported any more, but let's grandfather it in
+                # here.
+                if config.get("no_tls", False):
+                    logger.info(
+                        "Ignoring TLS-enabled listener on port %i due to no_tls"
+                    )
+                    continue
+
             bind_address = listener.pop("bind_address", None)
             bind_addresses = listener.setdefault("bind_addresses", [])
 
@@ -145,6 +153,8 @@ class ServerConfig(Config):
             if not bind_addresses:
                 bind_addresses.extend(DEFAULT_BIND_ADDRESSES)
 
+            self.listeners.append(listener)
+
         if not self.web_client_location:
             _warn_if_webclient_configured(self.listeners)
 
@@ -152,6 +162,9 @@ class ServerConfig(Config):
 
         bind_port = config.get("bind_port")
         if bind_port:
+            if config.get("no_tls", False):
+                raise ConfigError("no_tls is incompatible with bind_port")
+
             self.listeners = []
             bind_host = config.get("bind_host", "")
             gzip_responses = config.get("gzip_responses", True)
@@ -198,6 +211,7 @@ class ServerConfig(Config):
                 "port": manhole,
                 "bind_addresses": ["127.0.0.1"],
                 "type": "manhole",
+                "tls": False,
             })
 
         metrics_port = config.get("metrics_port")
@@ -223,6 +237,9 @@ class ServerConfig(Config):
 
         _check_resource_config(self.listeners)
 
+    def has_tls_listener(self):
+        return any(l["tls"] for l in self.listeners)
+
     def default_config(self, server_name, data_dir_path, **kwargs):
         _, bind_port = parse_and_validate_server_name(server_name)
         if bind_port is not None:
diff --git a/synapse/config/tls.py b/synapse/config/tls.py
index 76d2add4fe..e37a41eff4 100644
--- a/synapse/config/tls.py
+++ b/synapse/config/tls.py
@@ -51,7 +51,6 @@ class TlsConfig(Config):
             self._original_tls_fingerprints = []
 
         self.tls_fingerprints = list(self._original_tls_fingerprints)
-        self.no_tls = config.get("no_tls", False)
 
         # This config option applies to non-federation HTTP clients
         # (e.g. for talking to recaptcha, identity servers, and such)
@@ -141,6 +140,8 @@ class TlsConfig(Config):
 
         return (
             """\
+        ## TLS ##
+
         # PEM-encoded X509 certificate for TLS.
         # This certificate, as of Synapse 1.0, will need to be a valid and verifiable
         # certificate, signed by a recognised Certificate Authority.
@@ -201,13 +202,6 @@ class TlsConfig(Config):
             #
             # reprovision_threshold: 30
 
-        # If your server runs behind a reverse-proxy which terminates TLS connections
-        # (for both client and federation connections), it may be useful to disable
-        # All TLS support for incoming connections. Setting no_tls to True will
-        # do so (and avoid the need to give synapse a TLS private key).
-        #
-        # no_tls: True
-
         # 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