Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Commit

Permalink
Fix error when loading cert if tls is disabled (#4618)
Browse files Browse the repository at this point in the history
If TLS is disabled, it should not be an error if no cert is given.

Fixes #4554.
  • Loading branch information
richvdh authored Feb 12, 2019
1 parent 46b8a79 commit 32b781b
Show file tree
Hide file tree
Showing 4 changed files with 47 additions and 18 deletions.
1 change: 1 addition & 0 deletions changelog.d/4618.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix failure to start when not TLS certificate was given even if TLS was disabled.
5 changes: 3 additions & 2 deletions synapse/app/_base.py
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
57 changes: 42 additions & 15 deletions synapse/config/tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@

from OpenSSL import crypto

from synapse.config._base import Config
from synapse.config._base import Config, ConfigError

logger = logging.getLogger(__name__)

Expand All @@ -45,6 +45,19 @@ def read_config(self, 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:
Expand Down Expand Up @@ -105,26 +118,40 @@ def is_disk_cert_valid(self, allow_self_signed=True):
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)
Expand Down
2 changes: 1 addition & 1 deletion tests/config/test_tls.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def test_warn_self_signed(self):

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)
Expand Down

0 comments on commit 32b781b

Please sign in to comment.