From edea4bb5bed609ec011dd1f04256912a1a54e03f Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 21 Jun 2019 15:27:41 +0100 Subject: [PATCH 1/3] Allow configuration of the path used for ACME account keys. Because sticking it in the same place as the config isn't necessarily the right thing to do. --- docs/sample_config.yaml | 7 ++++ synapse/config/tls.py | 16 +++++++-- synapse/handlers/acme.py | 2 +- synapse/handlers/acme_issuing_service.py | 41 +++++++++++++++++++++--- 4 files changed, 59 insertions(+), 7 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index d5cc3e7abc..bb07b02f4e 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -402,6 +402,13 @@ acme: # #domain: matrix.example.com + # file to use for the account key. This will be generated if it doesn't + # exist. + # + # If unspecified, we will use CONFDIR/client.key. + # + account_key_file: DATADIR/acme_account.key + # 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 diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 28be4366d6..9a66e8cc4b 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -33,7 +33,7 @@ logger = logging.getLogger(__name__) class TlsConfig(Config): - def read_config(self, config, **kwargs): + def read_config(self, config, config_dir_path, **kwargs): acme_config = config.get("acme", None) if acme_config is None: @@ -50,6 +50,10 @@ class TlsConfig(Config): self.acme_reprovision_threshold = acme_config.get("reprovision_threshold", 30) self.acme_domain = acme_config.get("domain", config.get("server_name")) + self.acme_account_key_file = self.abspath( + acme_config.get("account_key_file", config_dir_path + "/client.key") + ) + self.tls_certificate_file = self.abspath(config.get("tls_certificate_path")) self.tls_private_key_file = self.abspath(config.get("tls_private_key_path")) @@ -213,11 +217,12 @@ class TlsConfig(Config): if sha256_fingerprint not in sha256_fingerprints: self.tls_fingerprints.append({"sha256": sha256_fingerprint}) - def default_config(self, config_dir_path, server_name, **kwargs): + def default_config(self, config_dir_path, server_name, data_dir_path, **kwargs): base_key_name = os.path.join(config_dir_path, server_name) tls_certificate_path = base_key_name + ".tls.crt" tls_private_key_path = base_key_name + ".tls.key" + default_acme_account_file = os.path.join(data_dir_path, "acme_account.key") # this is to avoid the max line length. Sorrynotsorry proxypassline = ( @@ -343,6 +348,13 @@ class TlsConfig(Config): # #domain: matrix.example.com + # file to use for the account key. This will be generated if it doesn't + # exist. + # + # If unspecified, we will use CONFDIR/client.key. + # + account_key_file: %(default_acme_account_file)s + # 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 diff --git a/synapse/handlers/acme.py b/synapse/handlers/acme.py index a760372203..fbef2f3d38 100644 --- a/synapse/handlers/acme.py +++ b/synapse/handlers/acme.py @@ -47,7 +47,7 @@ class AcmeHandler(object): self._issuer = acme_issuing_service.create_issuing_service( self.reactor, acme_url=self.hs.config.acme_url, - pem_path=self.hs.config.config_dir_path, + account_key_file=self.hs.config.acme_account_key_file, well_known_resource=well_known, ) diff --git a/synapse/handlers/acme_issuing_service.py b/synapse/handlers/acme_issuing_service.py index 70e73d2be0..e1d4224e74 100644 --- a/synapse/handlers/acme_issuing_service.py +++ b/synapse/handlers/acme_issuing_service.py @@ -21,28 +21,34 @@ This file contains the unconditional imports on the acme and cryptography bits t only need (and may only have available) if we are doing ACME, so is designed to be imported conditionally. """ +import logging import attr +from cryptography.hazmat.backends import default_backend +from cryptography.hazmat.primitives import serialization +from josepy import JWKRSA from josepy.jwa import RS256 from txacme.challenges import HTTP01Responder from txacme.client import Client -from txacme.endpoint import load_or_create_client_key from txacme.interfaces import ICertificateStore from txacme.service import AcmeIssuingService +from txacme.util import generate_private_key from zope.interface import implementer from twisted.internet import defer from twisted.python.filepath import FilePath from twisted.python.url import URL +logger = logging.getLogger(__name__) -def create_issuing_service(reactor, acme_url, pem_path, well_known_resource): + +def create_issuing_service(reactor, acme_url, account_key_file, well_known_resource): """Create an ACME issuing service, and attach it to a web Resource Args: reactor: twisted reactor acme_url (str): URL to use to request certificates - pem_path (str): where to store the client key + account_key_file (str): where to store the account key well_known_resource (twisted.web.IResource): web resource for .well-known. we will attach a child resource for "acme-challenge". @@ -61,7 +67,7 @@ def create_issuing_service(reactor, acme_url, pem_path, well_known_resource): lambda: Client.from_url( reactor=reactor, url=URL.from_text(acme_url), - key=load_or_create_client_key(FilePath(pem_path)), + key=load_or_create_client_key(account_key_file), alg=RS256, ) ), @@ -82,3 +88,30 @@ class ErsatzStore(object): def store(self, server_name, pem_objects): self.certs[server_name] = [o.as_bytes() for o in pem_objects] return defer.succeed(None) + + +def load_or_create_client_key(key_file): + """Load the ACME account key from a file, creating it if it does not exist. + + Args: + key_file (str): name of the file to use as the account key + """ + # this is based on txacme.endpoint.load_or_create_client_key, but doesn't + # hardcode the 'client.key' filename + acme_key_file = FilePath(key_file) + if acme_key_file.exists(): + logger.info("Loading ACME account key from '%s'", acme_key_file) + key = serialization.load_pem_private_key( + acme_key_file.getContent(), password=None, backend=default_backend() + ) + else: + logger.info("Saving new ACME account key to '%s'", acme_key_file) + key = generate_private_key("rsa") + acme_key_file.setContent( + key.private_bytes( + encoding=serialization.Encoding.PEM, + format=serialization.PrivateFormat.TraditionalOpenSSL, + encryption_algorithm=serialization.NoEncryption(), + ) + ) + return JWKRSA(key=key) From 367a8263b3afb3ce6b404f355208528014dc6b76 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 21 Jun 2019 17:13:27 +0100 Subject: [PATCH 2/3] Remove unused Config.config_dir_path attribute This is no longer used and only serves to confuse. --- synapse/config/_base.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 6baa315874..21d110c82d 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -414,9 +414,6 @@ class Config(object): Returns: dict """ - # FIXME: get rid of this - self.config_dir_path = config_dir_path - # first we read the config files into a dict specified_config = {} for config_file in config_files: From 5d9c101551f69dbd4bff225b3c528fceabc8e444 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Fri, 21 Jun 2019 15:29:06 +0100 Subject: [PATCH 3/3] changelog --- changelog.d/5516.feature | 1 + changelog.d/5521.feature | 1 + changelog.d/5521.misc | 1 - changelog.d/5522.feature | 1 + changelog.d/5522.misc | 1 - 5 files changed, 3 insertions(+), 2 deletions(-) create mode 100644 changelog.d/5516.feature create mode 100644 changelog.d/5521.feature delete mode 100644 changelog.d/5521.misc create mode 100644 changelog.d/5522.feature delete mode 100644 changelog.d/5522.misc diff --git a/changelog.d/5516.feature b/changelog.d/5516.feature new file mode 100644 index 0000000000..fdf91c35e4 --- /dev/null +++ b/changelog.d/5516.feature @@ -0,0 +1 @@ +Allow configuration of the path used for ACME account keys. diff --git a/changelog.d/5521.feature b/changelog.d/5521.feature new file mode 100644 index 0000000000..fdf91c35e4 --- /dev/null +++ b/changelog.d/5521.feature @@ -0,0 +1 @@ +Allow configuration of the path used for ACME account keys. diff --git a/changelog.d/5521.misc b/changelog.d/5521.misc deleted file mode 100644 index e3a14fdeaf..0000000000 --- a/changelog.d/5521.misc +++ /dev/null @@ -1 +0,0 @@ -Factor acme bits out to a separate file. diff --git a/changelog.d/5522.feature b/changelog.d/5522.feature new file mode 100644 index 0000000000..fdf91c35e4 --- /dev/null +++ b/changelog.d/5522.feature @@ -0,0 +1 @@ +Allow configuration of the path used for ACME account keys. diff --git a/changelog.d/5522.misc b/changelog.d/5522.misc deleted file mode 100644 index 17a7be5c99..0000000000 --- a/changelog.d/5522.misc +++ /dev/null @@ -1 +0,0 @@ -Pass config_dir_path and data_dir_path into Config.read_config.