From 6bd43746367028849b0942bf7f8050a85175dffc Mon Sep 17 00:00:00 2001 From: Amber Brown Date: Tue, 29 Jan 2019 14:09:10 +0000 Subject: Do not generate self-signed TLS certificates by default. (#4509) --- tests/config/test_tls.py | 75 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 75 insertions(+) create mode 100644 tests/config/test_tls.py (limited to 'tests/config/test_tls.py') diff --git a/tests/config/test_tls.py b/tests/config/test_tls.py new file mode 100644 index 0000000000..4ccaf35603 --- /dev/null +++ b/tests/config/test_tls.py @@ -0,0 +1,75 @@ +# -*- coding: utf-8 -*- +# Copyright 2019 New Vector Ltd +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. + +import os + +from synapse.config.tls import TlsConfig + +from tests.unittest import TestCase + + +class TLSConfigTests(TestCase): + + def test_warn_self_signed(self): + """ + Synapse will give a warning when it loads a self-signed certificate. + """ + config_dir = self.mktemp() + os.mkdir(config_dir) + with open(os.path.join(config_dir, "cert.pem"), 'w') as f: + f.write("""-----BEGIN CERTIFICATE----- +MIID6DCCAtACAws9CjANBgkqhkiG9w0BAQUFADCBtzELMAkGA1UEBhMCVFIxDzAN +BgNVBAgMBsOHb3J1bTEUMBIGA1UEBwwLQmHFn21ha8OnxLExEjAQBgNVBAMMCWxv +Y2FsaG9zdDEcMBoGA1UECgwTVHdpc3RlZCBNYXRyaXggTGFiczEkMCIGA1UECwwb +QXV0b21hdGVkIFRlc3RpbmcgQXV0aG9yaXR5MSkwJwYJKoZIhvcNAQkBFhpzZWN1 +cml0eUB0d2lzdGVkbWF0cml4LmNvbTAgFw0xNzA3MTIxNDAxNTNaGA8yMTE3MDYx +ODE0MDE1M1owgbcxCzAJBgNVBAYTAlRSMQ8wDQYDVQQIDAbDh29ydW0xFDASBgNV +BAcMC0JhxZ9tYWvDp8SxMRIwEAYDVQQDDAlsb2NhbGhvc3QxHDAaBgNVBAoME1R3 +aXN0ZWQgTWF0cml4IExhYnMxJDAiBgNVBAsMG0F1dG9tYXRlZCBUZXN0aW5nIEF1 +dGhvcml0eTEpMCcGCSqGSIb3DQEJARYac2VjdXJpdHlAdHdpc3RlZG1hdHJpeC5j +b20wggEiMA0GCSqGSIb3DQEBAQUAA4IBDwAwggEKAoIBAQDwT6kbqtMUI0sMkx4h +I+L780dA59KfksZCqJGmOsMD6hte9EguasfkZzvCF3dk3NhwCjFSOvKx6rCwiteo +WtYkVfo+rSuVNmt7bEsOUDtuTcaxTzIFB+yHOYwAaoz3zQkyVW0c4pzioiLCGCmf +FLdiDBQGGp74tb+7a0V6kC3vMLFoM3L6QWq5uYRB5+xLzlPJ734ltyvfZHL3Us6p +cUbK+3WTWvb4ER0W2RqArAj6Bc/ERQKIAPFEiZi9bIYTwvBH27OKHRz+KoY/G8zY ++l+WZoJqDhupRAQAuh7O7V/y6bSP+KNxJRie9QkZvw1PSaGSXtGJI3WWdO12/Ulg +epJpAgMBAAEwDQYJKoZIhvcNAQEFBQADggEBAJXEq5P9xwvP9aDkXIqzcD0L8sf8 +ewlhlxTQdeqt2Nace0Yk18lIo2oj1t86Y8jNbpAnZJeI813Rr5M7FbHCXoRc/SZG +I8OtG1xGwcok53lyDuuUUDexnK4O5BkjKiVlNPg4HPim5Kuj2hRNFfNt/F2BVIlj +iZupikC5MT1LQaRwidkSNxCku1TfAyueiBwhLnFwTmIGNnhuDCutEVAD9kFmcJN2 +SznugAcPk4doX2+rL+ila+ThqgPzIkwTUHtnmjI0TI6xsDUlXz5S3UyudrE2Qsfz +s4niecZKPBizL6aucT59CsunNmmb5Glq8rlAcU+1ZTZZzGYqVYhF6axB9Qg= +-----END CERTIFICATE-----""") + + config = { + "tls_certificate_path": os.path.join(config_dir, "cert.pem"), + "no_tls": True, + "tls_fingerprints": [] + } + + t = TlsConfig() + t.read_config(config) + t.read_certificate_from_disk() + + warnings = self.flushWarnings() + self.assertEqual(len(warnings), 1) + self.assertEqual( + warnings[0]["message"], + ( + "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." + ) + ) -- cgit 1.5.1 From 0ca290865350212e1834730c918973162a3067f4 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 11 Feb 2019 22:01:27 +0000 Subject: fix tests --- synapse/config/tls.py | 2 +- tests/config/test_tls.py | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) (limited to 'tests/config/test_tls.py') diff --git a/synapse/config/tls.py b/synapse/config/tls.py index e37a41eff4..86e6eb80db 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -111,7 +111,7 @@ class TlsConfig(Config): """ self.tls_certificate = self.read_tls_certificate() - if not self.no_tls: + if self.has_tls_listener(): self.tls_private_key = self.read_tls_private_key() self.tls_fingerprints = list(self._original_tls_fingerprints) diff --git a/tests/config/test_tls.py b/tests/config/test_tls.py index 4ccaf35603..d8fd18a9cb 100644 --- a/tests/config/test_tls.py +++ b/tests/config/test_tls.py @@ -20,6 +20,11 @@ from synapse.config.tls import TlsConfig from tests.unittest import TestCase +class TestConfig(TlsConfig): + def has_tls_listener(self): + return False + + class TLSConfigTests(TestCase): def test_warn_self_signed(self): @@ -55,11 +60,10 @@ s4niecZKPBizL6aucT59CsunNmmb5Glq8rlAcU+1ZTZZzGYqVYhF6axB9Qg= config = { "tls_certificate_path": os.path.join(config_dir, "cert.pem"), - "no_tls": True, "tls_fingerprints": [] } - t = TlsConfig() + t = TestConfig() t.read_config(config) t.read_certificate_from_disk() -- cgit 1.5.1 From 32b781bfe20034052d71558c0ed9b0df511a1f77 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff <1389908+richvdh@users.noreply.github.com> Date: Tue, 12 Feb 2019 10:51:31 +0000 Subject: Fix error when loading cert if tls is disabled (#4618) If TLS is disabled, it should not be an error if no cert is given. Fixes #4554. --- changelog.d/4618.bugfix | 1 + synapse/app/_base.py | 5 +++-- synapse/config/tls.py | 57 +++++++++++++++++++++++++++++++++++------------- tests/config/test_tls.py | 2 +- 4 files changed, 47 insertions(+), 18 deletions(-) create mode 100644 changelog.d/4618.bugfix (limited to 'tests/config/test_tls.py') diff --git a/changelog.d/4618.bugfix b/changelog.d/4618.bugfix new file mode 100644 index 0000000000..22115a020e --- /dev/null +++ b/changelog.d/4618.bugfix @@ -0,0 +1 @@ +Fix failure to start when not TLS certificate was given even if TLS was disabled. diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 50fd17c0be..5b0ca312e2 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -213,12 +213,13 @@ def refresh_certificate(hs): Refresh the TLS certificates that Synapse is using by re-reading them from disk and updating the TLS context factories to use them. """ - hs.config.read_certificate_from_disk() if not hs.config.has_tls_listener(): - # nothing else to do here + # attempt to reload the certs for the good of the tls_fingerprints + hs.config.read_certificate_from_disk(require_cert_and_key=False) return + hs.config.read_certificate_from_disk(require_cert_and_key=True) hs.tls_server_context_factory = context_factory.ServerContextFactory(hs.config) if hs._listening_services: diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 86e6eb80db..57f117a14d 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -23,7 +23,7 @@ from unpaddedbase64 import encode_base64 from OpenSSL import crypto -from synapse.config._base import Config +from synapse.config._base import Config, ConfigError logger = logging.getLogger(__name__) @@ -45,6 +45,19 @@ class TlsConfig(Config): self.tls_certificate_file = self.abspath(config.get("tls_certificate_path")) self.tls_private_key_file = self.abspath(config.get("tls_private_key_path")) + + if self.has_tls_listener(): + if not self.tls_certificate_file: + raise ConfigError( + "tls_certificate_path must be specified if TLS-enabled listeners are " + "configured." + ) + if not self.tls_private_key_file: + raise ConfigError( + "tls_certificate_path must be specified if TLS-enabled listeners are " + "configured." + ) + self._original_tls_fingerprints = config.get("tls_fingerprints", []) if self._original_tls_fingerprints is None: @@ -105,26 +118,40 @@ class TlsConfig(Config): days_remaining = (expires_on - now).days return days_remaining - def read_certificate_from_disk(self): - """ - Read the certificates from disk. + def read_certificate_from_disk(self, require_cert_and_key): """ - self.tls_certificate = self.read_tls_certificate() + Read the certificates and private key from disk. - if self.has_tls_listener(): + Args: + require_cert_and_key (bool): set to True to throw an error if the certificate + and key file are not given + """ + if require_cert_and_key: self.tls_private_key = self.read_tls_private_key() + self.tls_certificate = self.read_tls_certificate() + elif self.tls_certificate_file: + # we only need the certificate for the tls_fingerprints. Reload it if we + # can, but it's not a fatal error if we can't. + try: + self.tls_certificate = self.read_tls_certificate() + except Exception as e: + logger.info( + "Unable to read TLS certificate (%s). Ignoring as no " + "tls listeners enabled.", e, + ) self.tls_fingerprints = list(self._original_tls_fingerprints) - # Check that our own certificate is included in the list of fingerprints - # and include it if it is not. - x509_certificate_bytes = crypto.dump_certificate( - crypto.FILETYPE_ASN1, self.tls_certificate - ) - sha256_fingerprint = encode_base64(sha256(x509_certificate_bytes).digest()) - sha256_fingerprints = set(f["sha256"] for f in self.tls_fingerprints) - if sha256_fingerprint not in sha256_fingerprints: - self.tls_fingerprints.append({u"sha256": sha256_fingerprint}) + if self.tls_certificate: + # Check that our own certificate is included in the list of fingerprints + # and include it if it is not. + x509_certificate_bytes = crypto.dump_certificate( + crypto.FILETYPE_ASN1, self.tls_certificate + ) + sha256_fingerprint = encode_base64(sha256(x509_certificate_bytes).digest()) + sha256_fingerprints = set(f["sha256"] for f in self.tls_fingerprints) + if sha256_fingerprint not in sha256_fingerprints: + self.tls_fingerprints.append({u"sha256": sha256_fingerprint}) def default_config(self, config_dir_path, server_name, **kwargs): base_key_name = os.path.join(config_dir_path, server_name) diff --git a/tests/config/test_tls.py b/tests/config/test_tls.py index d8fd18a9cb..c260d3359f 100644 --- a/tests/config/test_tls.py +++ b/tests/config/test_tls.py @@ -65,7 +65,7 @@ s4niecZKPBizL6aucT59CsunNmmb5Glq8rlAcU+1ZTZZzGYqVYhF6axB9Qg= t = TestConfig() t.read_config(config) - t.read_certificate_from_disk() + t.read_certificate_from_disk(require_cert_and_key=False) warnings = self.flushWarnings() self.assertEqual(len(warnings), 1) -- cgit 1.5.1