Merge pull request #4613 from matrix-org/rav/deprecate_no_tls

Infer no_tls from presence of TLS listeners
This commit is contained in:
Erik Johnston 2019-02-12 09:59:53 +00:00 committed by GitHub
commit 8a2e316413
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 82 additions and 44 deletions

1
changelog.d/4613.feature Normal file
View File

@ -0,0 +1 @@
There is no longer any need to specify `no_tls`: it is inferred from the absence of TLS listeners

1
changelog.d/4615.feature Normal file
View File

@ -0,0 +1 @@
There is no longer any need to specify `no_tls`: it is inferred from the absence of TLS listeners

1
changelog.d/4616.misc Normal file
View File

@ -0,0 +1 @@
Fail cleanly if listener config lacks a 'port'

1
changelog.d/4617.feature Normal file
View File

@ -0,0 +1 @@
There is no longer any need to specify `no_tls`: it is inferred from the absence of TLS listeners

View File

@ -213,13 +213,16 @@ def refresh_certificate(hs):
Refresh the TLS certificates that Synapse is using by re-reading them from Refresh the TLS certificates that Synapse is using by re-reading them from
disk and updating the TLS context factories to use them. disk and updating the TLS context factories to use them.
""" """
logging.info("Loading certificate from disk...")
hs.config.read_certificate_from_disk() hs.config.read_certificate_from_disk()
if not hs.config.has_tls_listener():
# nothing else to do here
return
hs.tls_server_context_factory = context_factory.ServerContextFactory(hs.config) hs.tls_server_context_factory = context_factory.ServerContextFactory(hs.config)
logging.info("Certificate loaded.")
if hs._listening_services: if hs._listening_services:
logging.info("Updating context factories...") logger.info("Updating context factories...")
for i in hs._listening_services: for i in hs._listening_services:
# When you listenSSL, it doesn't make an SSL port but a TCP one with # When you listenSSL, it doesn't make an SSL port but a TCP one with
# a TLS wrapping factory around the factory you actually want to get # a TLS wrapping factory around the factory you actually want to get
@ -234,7 +237,7 @@ def refresh_certificate(hs):
False, False,
i.factory.wrappedFactory i.factory.wrappedFactory
) )
logging.info("Context factories updated.") logger.info("Context factories updated.")
def start(hs, listeners=None): def start(hs, listeners=None):

View File

@ -90,11 +90,6 @@ class SynapseHomeServer(HomeServer):
tls = listener_config.get("tls", False) tls = listener_config.get("tls", False)
site_tag = listener_config.get("tag", port) 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 = {} resources = {}
for res in listener_config["resources"]: for res in listener_config["resources"]:
for name in res["names"]: for name in res["names"]:

View File

@ -42,7 +42,7 @@ from .voip import VoipConfig
from .workers import WorkerConfig from .workers import WorkerConfig
class HomeServerConfig(TlsConfig, ServerConfig, DatabaseConfig, LoggingConfig, class HomeServerConfig(ServerConfig, TlsConfig, DatabaseConfig, LoggingConfig,
RatelimitConfig, ContentRepositoryConfig, CaptchaConfig, RatelimitConfig, ContentRepositoryConfig, CaptchaConfig,
VoipConfig, RegistrationConfig, MetricsConfig, ApiConfig, VoipConfig, RegistrationConfig, MetricsConfig, ApiConfig,
AppServiceConfig, KeyConfig, SAML2Config, CasConfig, AppServiceConfig, KeyConfig, SAML2Config, CasConfig,

View File

@ -126,9 +126,22 @@ class ServerConfig(Config):
self.public_baseurl += '/' self.public_baseurl += '/'
self.start_pushers = config.get("start_pushers", True) self.start_pushers = config.get("start_pushers", True)
self.listeners = config.get("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
for listener in self.listeners:
bind_address = listener.pop("bind_address", None) bind_address = listener.pop("bind_address", None)
bind_addresses = listener.setdefault("bind_addresses", []) bind_addresses = listener.setdefault("bind_addresses", [])
@ -140,6 +153,8 @@ class ServerConfig(Config):
if not bind_addresses: if not bind_addresses:
bind_addresses.extend(DEFAULT_BIND_ADDRESSES) bind_addresses.extend(DEFAULT_BIND_ADDRESSES)
self.listeners.append(listener)
if not self.web_client_location: if not self.web_client_location:
_warn_if_webclient_configured(self.listeners) _warn_if_webclient_configured(self.listeners)
@ -147,6 +162,9 @@ class ServerConfig(Config):
bind_port = config.get("bind_port") bind_port = config.get("bind_port")
if bind_port: if bind_port:
if config.get("no_tls", False):
raise ConfigError("no_tls is incompatible with bind_port")
self.listeners = [] self.listeners = []
bind_host = config.get("bind_host", "") bind_host = config.get("bind_host", "")
gzip_responses = config.get("gzip_responses", True) gzip_responses = config.get("gzip_responses", True)
@ -193,6 +211,7 @@ class ServerConfig(Config):
"port": manhole, "port": manhole,
"bind_addresses": ["127.0.0.1"], "bind_addresses": ["127.0.0.1"],
"type": "manhole", "type": "manhole",
"tls": False,
}) })
metrics_port = config.get("metrics_port") metrics_port = config.get("metrics_port")
@ -218,6 +237,9 @@ class ServerConfig(Config):
_check_resource_config(self.listeners) _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): def default_config(self, server_name, data_dir_path, **kwargs):
_, bind_port = parse_and_validate_server_name(server_name) _, bind_port = parse_and_validate_server_name(server_name)
if bind_port is not None: if bind_port is not None:

View File

@ -25,7 +25,7 @@ from OpenSSL import crypto
from synapse.config._base import Config from synapse.config._base import Config
logger = logging.getLogger() logger = logging.getLogger(__name__)
class TlsConfig(Config): class TlsConfig(Config):
@ -51,7 +51,6 @@ class TlsConfig(Config):
self._original_tls_fingerprints = [] self._original_tls_fingerprints = []
self.tls_fingerprints = list(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 # This config option applies to non-federation HTTP clients
# (e.g. for talking to recaptcha, identity servers, and such) # (e.g. for talking to recaptcha, identity servers, and such)
@ -110,20 +109,10 @@ class TlsConfig(Config):
""" """
Read the certificates from disk. Read the certificates from disk.
""" """
self.tls_certificate = self.read_tls_certificate(self.tls_certificate_file) self.tls_certificate = self.read_tls_certificate()
# Check if it is self-signed, and issue a warning if so. if self.has_tls_listener():
if self.tls_certificate.get_issuer() == self.tls_certificate.get_subject(): self.tls_private_key = self.read_tls_private_key()
warnings.warn(
(
"Self-signed TLS certificates will not be accepted by Synapse 1.0. "
"Please either provide a valid certificate, or use Synapse's ACME "
"support to provision one."
)
)
if not self.no_tls:
self.tls_private_key = self.read_tls_private_key(self.tls_private_key_file)
self.tls_fingerprints = list(self._original_tls_fingerprints) self.tls_fingerprints = list(self._original_tls_fingerprints)
@ -151,6 +140,8 @@ class TlsConfig(Config):
return ( return (
"""\ """\
## TLS ##
# PEM-encoded X509 certificate for TLS. # PEM-encoded X509 certificate for TLS.
# This certificate, as of Synapse 1.0, will need to be a valid and verifiable # This certificate, as of Synapse 1.0, will need to be a valid and verifiable
# certificate, signed by a recognised Certificate Authority. # certificate, signed by a recognised Certificate Authority.
@ -211,13 +202,6 @@ class TlsConfig(Config):
# #
# reprovision_threshold: 30 # 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 # List of allowed TLS fingerprints for this server to publish along
# with the signing keys for this server. Other matrix servers that # with the signing keys for this server. Other matrix servers that
# make HTTPS requests to this server will check that the TLS # make HTTPS requests to this server will check that the TLS
@ -250,10 +234,38 @@ class TlsConfig(Config):
% locals() % locals()
) )
def read_tls_certificate(self, cert_path): def read_tls_certificate(self):
cert_pem = self.read_file(cert_path, "tls_certificate") """Reads the TLS certificate from the configured file, and returns it
return crypto.load_certificate(crypto.FILETYPE_PEM, cert_pem)
def read_tls_private_key(self, private_key_path): Also checks if it is self-signed, and warns if so
private_key_pem = self.read_file(private_key_path, "tls_private_key")
Returns:
OpenSSL.crypto.X509: the certificate
"""
cert_path = self.tls_certificate_file
logger.info("Loading TLS certificate from %s", cert_path)
cert_pem = self.read_file(cert_path, "tls_certificate_path")
cert = crypto.load_certificate(crypto.FILETYPE_PEM, cert_pem)
# Check if it is self-signed, and issue a warning if so.
if cert.get_issuer() == cert.get_subject():
warnings.warn(
(
"Self-signed TLS certificates will not be accepted by Synapse 1.0. "
"Please either provide a valid certificate, or use Synapse's ACME "
"support to provision one."
)
)
return cert
def read_tls_private_key(self):
"""Reads the TLS private key from the configured file, and returns it
Returns:
OpenSSL.crypto.PKey: the private key
"""
private_key_path = self.tls_private_key_file
logger.info("Loading TLS key from %s", private_key_path)
private_key_pem = self.read_file(private_key_path, "tls_private_key_path")
return crypto.load_privatekey(crypto.FILETYPE_PEM, private_key_pem) return crypto.load_privatekey(crypto.FILETYPE_PEM, private_key_pem)

View File

@ -43,8 +43,6 @@ class ServerContextFactory(ContextFactory):
logger.exception("Failed to enable elliptic curve for TLS") 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)
context.use_certificate_chain_file(config.tls_certificate_file) context.use_certificate_chain_file(config.tls_certificate_file)
if not config.no_tls:
context.use_privatekey(config.tls_private_key) context.use_privatekey(config.tls_private_key)
# https://hynek.me/articles/hardening-your-web-servers-ssl-ciphers/ # https://hynek.me/articles/hardening-your-web-servers-ssl-ciphers/

View File

@ -20,6 +20,11 @@ from synapse.config.tls import TlsConfig
from tests.unittest import TestCase from tests.unittest import TestCase
class TestConfig(TlsConfig):
def has_tls_listener(self):
return False
class TLSConfigTests(TestCase): class TLSConfigTests(TestCase):
def test_warn_self_signed(self): def test_warn_self_signed(self):
@ -55,11 +60,10 @@ s4niecZKPBizL6aucT59CsunNmmb5Glq8rlAcU+1ZTZZzGYqVYhF6axB9Qg=
config = { config = {
"tls_certificate_path": os.path.join(config_dir, "cert.pem"), "tls_certificate_path": os.path.join(config_dir, "cert.pem"),
"no_tls": True,
"tls_fingerprints": [] "tls_fingerprints": []
} }
t = TlsConfig() t = TestConfig()
t.read_config(config) t.read_config(config)
t.read_certificate_from_disk() t.read_certificate_from_disk()