From 001ee16a31ee8417e00241ab14edf9c449bc2630 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Wed, 21 Aug 2019 16:11:54 +0100 Subject: [PATCH 01/23] Started templating the output --- synapse/config/database.py | 4 +- synapse/config/server.py | 74 ++++++++++++++++++++++++-------- synapse/config/tls.py | 31 ++++++++++--- synapse_topology/model/config.py | 38 ++++++++++++++++ 4 files changed, 123 insertions(+), 24 deletions(-) create mode 100644 synapse_topology/model/config.py diff --git a/synapse/config/database.py b/synapse/config/database.py index 746a6cd1f42a..2f95e746ed28 100644 --- a/synapse/config/database.py +++ b/synapse/config/database.py @@ -38,7 +38,7 @@ def read_config(self, config, **kwargs): self.set_databasepath(config.get("database_path")) - def generate_config_section(self, data_dir_path, **kwargs): + def generate_config_section(self, data_dir_path, database="sqlite3", **kwargs): database_path = os.path.join(data_dir_path, "homeserver.db") return ( """\ @@ -46,7 +46,7 @@ def generate_config_section(self, data_dir_path, **kwargs): database: # The database engine name - name: "sqlite3" + name: "%(database)s" # Arguments to pass to the engine args: # Path to the database diff --git a/synapse/config/server.py b/synapse/config/server.py index 15449695d19d..11004792b513 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -17,6 +17,8 @@ import logging import os.path +import yaml +import re import attr from netaddr import IPSet @@ -24,6 +26,7 @@ from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.http.endpoint import parse_and_validate_server_name from synapse.python_dependencies import DependencyException, check_requirements +from textwrap import indent from ._base import Config, ConfigError @@ -352,7 +355,7 @@ def has_tls_listener(self): return any(l["tls"] for l in self.listeners) def generate_config_section( - self, server_name, data_dir_path, open_private_ports, **kwargs + self, server_name, data_dir_path, open_private_ports, listeners=None, **kwargs ): _, bind_port = parse_and_validate_server_name(server_name) if bind_port is not None: @@ -366,11 +369,58 @@ def generate_config_section( # Bring DEFAULT_ROOM_VERSION into the local-scope for use in the # default config string default_room_version = DEFAULT_ROOM_VERSION + secure_listeners = [] + unsecure_listeners = [] + if listeners: + print(listeners) + for listener in listeners: + if listener["tls"]: + secure_listeners.append(listener) + else: + if open_private_ports and not listener["bind_addresses"]: + listener["bind_addresses"] = ["::1", "127.0.0.1"] + unsecure_listeners.append(listener) + + secure_http_bindings = indent( + yaml.dump(secure_listeners), " " * 10 + ).lstrip() + + unsecure_http_bindings = indent( + yaml.dump(unsecure_listeners), " " * 10 + ).lstrip() + + if not unsecure_listeners: + unsecure_http_bindings = ( + """- port: %(unsecure_port)s + tls: false + type: http + x_forwarded: true + + resources: + - names: [client, federation] + compress: false""" + % locals() + ) + if not open_private_ports: + unsecure_http_bindings += ( + "\n bind_addresses: ['::1', '127.0.0.1']" + ) + if listeners: + # comment out this block + unsecure_http_bindings = "#" + re.sub( + "\n {10}", + lambda match: match.group(0) + "#", + unsecure_http_bindings, + ) - unsecure_http_binding = "port: %i\n tls: false" % (unsecure_port,) - if not open_private_ports: - unsecure_http_binding += ( - "\n bind_addresses: ['::1', '127.0.0.1']" + if not secure_listeners: + secure_http_bindings = ( + """#- port: %(bind_port)s + # type: http + # tls: true + # resources: + # - names: [client, federation]""" + % locals() ) return ( @@ -556,11 +606,7 @@ def generate_config_section( # will also need to give Synapse a TLS key and certificate: see the TLS section # below.) # - #- port: %(bind_port)s - # type: http - # tls: true - # resources: - # - names: [client, federation] + %(secure_http_bindings)s # Unsecure HTTP listener: for when matrix traffic passes through a reverse proxy # that unwraps TLS. @@ -568,13 +614,7 @@ def generate_config_section( # If you plan to use a reverse proxy, please see # https://github.com/matrix-org/synapse/blob/master/docs/reverse_proxy.rst. # - - %(unsecure_http_binding)s - type: http - x_forwarded: true - - resources: - - names: [client, federation] - compress: false + %(unsecure_http_bindings)s # example additional_resources: # diff --git a/synapse/config/tls.py b/synapse/config/tls.py index ca508a224fa7..5baa308eabda 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -239,12 +239,33 @@ def read_certificate_from_disk(self, require_cert_and_key): self.tls_fingerprints.append({"sha256": sha256_fingerprint}) def generate_config_section( - self, config_dir_path, server_name, data_dir_path, **kwargs + self, + config_dir_path, + server_name, + data_dir_path, + tls_certificate_path=None, + tls_private_key_path=None, + **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" + if tls_certificate_path: + tls_certificate_path = ( + 'tls_certificate_path: "' + tls_certificate_path + '"' + ) + else: + tls_certificate_path = ( + '#tls_certificate_path: "' + base_key_name + ".tls.crt" + '"' + ) + if tls_private_key_path: + tls_private_key_path = ( + 'tls_private_key_path: "' + tls_private_key_path + '"' + ) + else: + tls_private_key_path = ( + '#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 @@ -269,11 +290,11 @@ def generate_config_section( # instance, if using certbot, use `fullchain.pem` as your certificate, # not `cert.pem`). # - #tls_certificate_path: "%(tls_certificate_path)s" + %(tls_certificate_path)s # PEM-encoded private key for TLS # - #tls_private_key_path: "%(tls_private_key_path)s" + %(tls_private_key_path)s # Whether to verify TLS server certificates for outbound federation requests. # diff --git a/synapse_topology/model/config.py b/synapse_topology/model/config.py new file mode 100644 index 000000000000..fcc5b48e41c0 --- /dev/null +++ b/synapse_topology/model/config.py @@ -0,0 +1,38 @@ +from synapse.config.database import DatabaseConfig +from synapse.config.server import ServerConfig +from model import get_config_dir, get_data_dir, set_config_dir + + +def create_config(conf): + server = ServerConfig().generate_config_section( + conf["server_name"], get_data_dir(), False, conf["listeners"] + ) + database = DatabaseConfig().generate_config_section(get_data_dir(), **conf) + + return "\n\n".join([server, database]) + + +set_config_dir("/exampledir/") +print( + create_config( + { + "server_name": "banterserver", + "database": "sqlcrap", + "listeners": [ + { + "port": 8448, + "resources": [{"names": ["federation"]}], + "tls": True, + "type": "http", + }, + { + "port": 443, + "resources": [{"names": ["client"]}], + "tls": False, + "type": "http", + }, + ], + } + ) +) + From ce5e5b226fc40bfc777e431572c1dec331f48665 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Wed, 21 Aug 2019 16:49:18 +0100 Subject: [PATCH 02/23] Tls config --- synapse/config/tls.py | 45 ++++++++++++++++---------------- synapse_topology/model/config.py | 15 ++++++++--- 2 files changed, 35 insertions(+), 25 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 5baa308eabda..4b40a24ffd75 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -245,27 +245,28 @@ def generate_config_section( data_dir_path, tls_certificate_path=None, tls_private_key_path=None, + acme_domain=None, **kwargs ): base_key_name = os.path.join(config_dir_path, server_name) - if tls_certificate_path: - tls_certificate_path = ( - 'tls_certificate_path: "' + tls_certificate_path + '"' - ) - else: - tls_certificate_path = ( - '#tls_certificate_path: "' + base_key_name + ".tls.crt" + '"' - ) - if tls_private_key_path: - tls_private_key_path = ( - 'tls_private_key_path: "' + tls_private_key_path + '"' - ) - else: - tls_private_key_path = ( - '#tls_private_key_path: "' + base_key_name + ".tls.key" + '"' + if bool(tls_certificate_path) ^ bool(tls_private_key_path): + raise ConfigError( + "Please specify both a cert path and a key path or neither." ) + tls_enabled = ( + "" if tls_certificate_path and tls_private_key_path or acme_domain else "#" + ) + + if not tls_certificate_path: + tls_certificate_path = base_key_name + ".tls.crt" + if not tls_private_key_path: + tls_private_key_path = base_key_name + ".tls.key" + + acme_enabled = "" if acme_domain else "#" + acme_domain = "matrix.example.com" + default_acme_account_file = os.path.join(data_dir_path, "acme_account.key") # this is to avoid the max line length. Sorrynotsorry @@ -290,11 +291,11 @@ def generate_config_section( # instance, if using certbot, use `fullchain.pem` as your certificate, # not `cert.pem`). # - %(tls_certificate_path)s + %(tls_enabled)stls_certificate_path: %(tls_certificate_path)s # PEM-encoded private key for TLS # - %(tls_private_key_path)s + %(tls_enabled)stls_private_key_path: %(tls_private_key_path)s # Whether to verify TLS server certificates for outbound federation requests. # @@ -364,7 +365,7 @@ def generate_config_section( # ACME support is disabled by default. Uncomment the following line # (and tls_certificate_path and tls_private_key_path above) to enable it. # - #enabled: true + %(acme_enabled)senabled: true # Endpoint to use to request certificates. If you only want to test, # use Let's Encrypt's staging url: @@ -375,17 +376,17 @@ def generate_config_section( # Port number to listen on for the HTTP-01 challenge. Change this if # you are forwarding connections through Apache/Nginx/etc. # - #port: 80 + %(acme_enabled)sport: 80 # Local addresses to listen on for incoming connections. # Again, you may want to change this if you are forwarding connections # through Apache/Nginx/etc. # - #bind_addresses: ['::', '0.0.0.0'] + %(acme_enabled)sbind_addresses: ['::', '0.0.0.0'] # How many days remaining on a certificate before it is renewed. # - #reprovision_threshold: 30 + %(acme_enabled)sreprovision_threshold: 30 # The domain that the certificate should be for. Normally this # should be the same as your Matrix domain (i.e., 'server_name'), but, @@ -399,7 +400,7 @@ def generate_config_section( # # If not set, defaults to your 'server_name'. # - #domain: matrix.example.com + %(acme_enabled)sdomain: %(acme_domain)s # file to use for the account key. This will be generated if it doesn't # exist. diff --git a/synapse_topology/model/config.py b/synapse_topology/model/config.py index fcc5b48e41c0..bf860930e001 100644 --- a/synapse_topology/model/config.py +++ b/synapse_topology/model/config.py @@ -1,15 +1,21 @@ from synapse.config.database import DatabaseConfig from synapse.config.server import ServerConfig +from synapse.config.tls import TlsConfig +from synapse.config.logger import LoggingConfig from model import get_config_dir, get_data_dir, set_config_dir def create_config(conf): + server_name = conf["server_name"] + del conf["server_name"] server = ServerConfig().generate_config_section( - conf["server_name"], get_data_dir(), False, conf["listeners"] + server_name, get_data_dir(), False, **conf ) database = DatabaseConfig().generate_config_section(get_data_dir(), **conf) - - return "\n\n".join([server, database]) + tls = TlsConfig().generate_config_section( + get_config_dir(), server_name, get_data_dir(), **conf + ) + return "\n\n".join([server, database, tls]) set_config_dir("/exampledir/") @@ -32,6 +38,9 @@ def create_config(conf): "type": "http", }, ], + "tls_certificate_path": "asdfasfdasdfadf", + "tls_private_key_path": "asdfasfdha;kdfjhafd", + "acme_domain": "asdfaklhsadfkj", } ) ) From 2b15b7bed351d356a13b336d66b818a11d2c46fa Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 22 Aug 2019 16:25:34 +0100 Subject: [PATCH 03/23] Pass configu options into sections --- synapse/config/_base.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 6ce5cd07fb90..1f0d9a369f38 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -220,6 +220,11 @@ def generate_config( generate_secrets=generate_secrets, report_stats=report_stats, open_private_ports=open_private_ports, + listeners=listeners, + database=database, + tls_certificate_path=tls_certificate_path, + tls_private_key_path=tls_private_key_path, + acme_domain=acme_domain, ) ) From 0000e909b76b034ac699ed1e3bf04780cb891c66 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 22 Aug 2019 14:41:09 +0100 Subject: [PATCH 04/23] Move args to generate_config --- synapse/config/_base.py | 5 +++++ synapse/config/database.py | 4 +++- synapse/config/server.py | 2 +- synapse/config/tls.py | 6 +++--- synapse_topology/model/config.py | 1 + 5 files changed, 13 insertions(+), 5 deletions(-) diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 1f0d9a369f38..0b7f37669ea4 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -181,6 +181,11 @@ def generate_config( generate_secrets=False, report_stats=None, open_private_ports=False, + listeners=None, + database=None, + tls_certificate_path=None, + tls_private_key_path=None, + acme_domain=None, ): """Build a default configuration file diff --git a/synapse/config/database.py b/synapse/config/database.py index 2f95e746ed28..a56a578d6ad5 100644 --- a/synapse/config/database.py +++ b/synapse/config/database.py @@ -38,7 +38,9 @@ def read_config(self, config, **kwargs): self.set_databasepath(config.get("database_path")) - def generate_config_section(self, data_dir_path, database="sqlite3", **kwargs): + def generate_config_section(self, data_dir_path, database, **kwargs): + if not database: + database = "sqlite3" database_path = os.path.join(data_dir_path, "homeserver.db") return ( """\ diff --git a/synapse/config/server.py b/synapse/config/server.py index 11004792b513..6fe8edd49876 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -355,7 +355,7 @@ def has_tls_listener(self): return any(l["tls"] for l in self.listeners) def generate_config_section( - self, server_name, data_dir_path, open_private_ports, listeners=None, **kwargs + self, server_name, data_dir_path, open_private_ports, listeners, **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 4b40a24ffd75..3369acd58bf3 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -243,9 +243,9 @@ def generate_config_section( config_dir_path, server_name, data_dir_path, - tls_certificate_path=None, - tls_private_key_path=None, - acme_domain=None, + tls_certificate_path, + tls_private_key_path, + acme_domain, **kwargs ): base_key_name = os.path.join(config_dir_path, server_name) diff --git a/synapse_topology/model/config.py b/synapse_topology/model/config.py index bf860930e001..13ae050ed679 100644 --- a/synapse_topology/model/config.py +++ b/synapse_topology/model/config.py @@ -8,6 +8,7 @@ def create_config(conf): server_name = conf["server_name"] del conf["server_name"] + server = ServerConfig().generate_config_section( server_name, get_data_dir(), False, **conf ) From 4ec743fdf29a0b36063bc91be915d34818953faf Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 22 Aug 2019 16:42:16 +0100 Subject: [PATCH 05/23] Erant debug print --- synapse/config/server.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index 6fe8edd49876..e9f6d2bf4961 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -372,7 +372,6 @@ def generate_config_section( secure_listeners = [] unsecure_listeners = [] if listeners: - print(listeners) for listener in listeners: if listener["tls"]: secure_listeners.append(listener) From 84c89a7bc68f7ae237dc04514801a4425cb0cd17 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 22 Aug 2019 16:52:16 +0100 Subject: [PATCH 06/23] Do something sensible when opening private ports --- synapse/config/server.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index e9f6d2bf4961..27879a784c4c 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -371,13 +371,22 @@ def generate_config_section( default_room_version = DEFAULT_ROOM_VERSION secure_listeners = [] unsecure_listeners = [] + private_addresses = ["::1", "127.0.0.1"] if listeners: for listener in listeners: if listener["tls"]: secure_listeners.append(listener) else: - if open_private_ports and not listener["bind_addresses"]: - listener["bind_addresses"] = ["::1", "127.0.0.1"] + # The open_private_ports option kind of conflicts with the + # idea of passing in your own listener config however + # the local addresses need to be bound if open_private_ports is + # specified. + if open_private_ports: + bind_addresses = listener.setdefault("bind_addresses", []) + for address in private_addresses: + if address not in bind_addresses: + bind_addresses.append(address) + unsecure_listeners.append(listener) secure_http_bindings = indent( From 48e2e2e34c6a45ab9d58b5a7251d81baf959950f Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 22 Aug 2019 16:57:21 +0100 Subject: [PATCH 07/23] Comment explaining tls config defaults --- synapse/config/tls.py | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 3369acd58bf3..e5bd5103e579 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -248,6 +248,10 @@ def generate_config_section( acme_domain, **kwargs ): + """If the acme_domain is specified acme will be enabled. + If the TLS paths are not specified the default will be certs in the + config directory""" + base_key_name = os.path.join(config_dir_path, server_name) if bool(tls_certificate_path) ^ bool(tls_private_key_path): From 54d8db1a65df005f34cf0d378b6c9c5031b98d18 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 22 Aug 2019 16:59:12 +0100 Subject: [PATCH 08/23] Not relevent to this pr --- synapse_topology/model/config.py | 48 -------------------------------- 1 file changed, 48 deletions(-) delete mode 100644 synapse_topology/model/config.py diff --git a/synapse_topology/model/config.py b/synapse_topology/model/config.py deleted file mode 100644 index 13ae050ed679..000000000000 --- a/synapse_topology/model/config.py +++ /dev/null @@ -1,48 +0,0 @@ -from synapse.config.database import DatabaseConfig -from synapse.config.server import ServerConfig -from synapse.config.tls import TlsConfig -from synapse.config.logger import LoggingConfig -from model import get_config_dir, get_data_dir, set_config_dir - - -def create_config(conf): - server_name = conf["server_name"] - del conf["server_name"] - - server = ServerConfig().generate_config_section( - server_name, get_data_dir(), False, **conf - ) - database = DatabaseConfig().generate_config_section(get_data_dir(), **conf) - tls = TlsConfig().generate_config_section( - get_config_dir(), server_name, get_data_dir(), **conf - ) - return "\n\n".join([server, database, tls]) - - -set_config_dir("/exampledir/") -print( - create_config( - { - "server_name": "banterserver", - "database": "sqlcrap", - "listeners": [ - { - "port": 8448, - "resources": [{"names": ["federation"]}], - "tls": True, - "type": "http", - }, - { - "port": 443, - "resources": [{"names": ["client"]}], - "tls": False, - "type": "http", - }, - ], - "tls_certificate_path": "asdfasfdasdfadf", - "tls_private_key_path": "asdfasfdha;kdfjhafd", - "acme_domain": "asdfaklhsadfkj", - } - ) -) - From 213913809daada5287cb9dba0451e61212ec82e6 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 22 Aug 2019 17:19:21 +0100 Subject: [PATCH 09/23] newsfile --- changelog.d/5900.feature | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/5900.feature diff --git a/changelog.d/5900.feature b/changelog.d/5900.feature new file mode 100644 index 000000000000..b62d88a76bb1 --- /dev/null +++ b/changelog.d/5900.feature @@ -0,0 +1 @@ +Add support for config templating. From 0627c9f7f938e34ba224038af13a9bc53efae80f Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 22 Aug 2019 18:03:41 +0100 Subject: [PATCH 10/23] isort --- synapse/config/server.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index 27879a784c4c..bba16c6a2e47 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -17,16 +17,16 @@ import logging import os.path -import yaml import re +from textwrap import indent import attr +import yaml from netaddr import IPSet from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.http.endpoint import parse_and_validate_server_name from synapse.python_dependencies import DependencyException, check_requirements -from textwrap import indent from ._base import Config, ConfigError From bd2a1f45fe3ad7a882f4d4548508199fe42926b8 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 22 Aug 2019 18:17:44 +0100 Subject: [PATCH 11/23] Surround paths in quotes. --- synapse/config/tls.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index e5bd5103e579..943e94e92978 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -295,11 +295,11 @@ def generate_config_section( # instance, if using certbot, use `fullchain.pem` as your certificate, # not `cert.pem`). # - %(tls_enabled)stls_certificate_path: %(tls_certificate_path)s + %(tls_enabled)stls_certificate_path: "%(tls_certificate_path)s" # PEM-encoded private key for TLS # - %(tls_enabled)stls_private_key_path: %(tls_private_key_path)s + %(tls_enabled)stls_private_key_path: "%(tls_private_key_path)s" # Whether to verify TLS server certificates for outbound federation requests. # From fa1f322cb75282ddf1899c7dc93788101d226f23 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Thu, 22 Aug 2019 18:18:07 +0100 Subject: [PATCH 12/23] Nicer listener formatting --- docs/sample_config.yaml | 2 +- synapse/config/server.py | 16 ++++++++++------ 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 0c6be30e513d..de0931e690aa 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -205,9 +205,9 @@ listeners: # - port: 8008 tls: false - bind_addresses: ['::1', '127.0.0.1'] type: http x_forwarded: true + bind_addresses: ['::1', '127.0.0.1'] resources: - names: [client, federation] diff --git a/synapse/config/server.py b/synapse/config/server.py index bba16c6a2e47..e4e1db827710 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -402,17 +402,21 @@ def generate_config_section( """- port: %(unsecure_port)s tls: false type: http - x_forwarded: true - - resources: - - names: [client, federation] - compress: false""" + x_forwarded: true""" % locals() ) + if not open_private_ports: unsecure_http_bindings += ( "\n bind_addresses: ['::1', '127.0.0.1']" ) + + unsecure_http_bindings += """ + + resources: + - names: [client, federation] + compress: false""" + if listeners: # comment out this block unsecure_http_bindings = "#" + re.sub( @@ -427,7 +431,7 @@ def generate_config_section( # type: http # tls: true # resources: - # - names: [client, federation]""" + # - names: [client, federation]""" % locals() ) From 67e7c63a0d3bafc2039bec046b5cb93b92371b94 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 23 Aug 2019 11:38:02 +0100 Subject: [PATCH 13/23] docstrings --- synapse/config/_base.py | 27 +++++++++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 0b7f37669ea4..397c25bc8d72 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -212,6 +212,33 @@ def generate_config( open_private_ports (bool): True to leave private ports (such as the non-TLS HTTP listener) open to the internet. + listeners (list(dict)|None): A list of descriptions of the listeners + synapse should start with each of which specifies a port, a list of resources, + tls (bool) and type (str). For example: + [{ + "port": 8448, + "resources": [{"names": ["federation"]}], + "tls": True, + "type": "http", + }, + { + "port": 443, + "resources": [{"names": ["client"]}], + "tls": False, + "type": "http", + }], + + + database (str|None): The database type to configure, either `psycog2` + or `sqlite3`. + + tls_certificate_path (str|None): The path to the tls certificate. + + tls_private_key_path (str|None): The path to the tls private key. + + acme_domain (str|None): The domain acme will try to validate. If + specified acme will be enabled. + Returns: str: the yaml config file """ From e7450cdaa0cbb3bac8c4c4d19a3f5ee8911f8ee4 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 23 Aug 2019 11:38:54 +0100 Subject: [PATCH 14/23] Formatting --- synapse/config/_base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 397c25bc8d72..3b72899cc46e 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -213,8 +213,8 @@ def generate_config( HTTP listener) open to the internet. listeners (list(dict)|None): A list of descriptions of the listeners - synapse should start with each of which specifies a port, a list of resources, - tls (bool) and type (str). For example: + synapse should start with each of which specifies a port (str), a list of + resources (list(str)), tls (bool) and type (str). For example: [{ "port": 8448, "resources": [{"names": ["federation"]}], From 4a41acda52eccd44b01946e6d997be3b7024d4a6 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 23 Aug 2019 11:43:40 +0100 Subject: [PATCH 15/23] Imagine a system composed entirely of x, y, z etc and the basic operations.. Wait George, why XOR? Why not just neq? George: Eh, I didn't think of that.. Co-Authored-By: Erik Johnston --- synapse/config/tls.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 943e94e92978..0699f15d0d7b 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -254,7 +254,7 @@ def generate_config_section( base_key_name = os.path.join(config_dir_path, server_name) - if bool(tls_certificate_path) ^ bool(tls_private_key_path): + if bool(tls_certificate_path) != bool(tls_private_key_path): raise ConfigError( "Please specify both a cert path and a key path or neither." ) From ec656ce9f8bef46d3e45b1a12348d198194dd5ea Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 23 Aug 2019 11:47:09 +0100 Subject: [PATCH 16/23] Cleaner acme enable/disable --- docs/sample_config.yaml | 10 +++++----- synapse/config/tls.py | 12 ++++++------ 2 files changed, 11 insertions(+), 11 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index de0931e690aa..965545968523 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -395,7 +395,7 @@ acme: # ACME support is disabled by default. Uncomment the following line # (and tls_certificate_path and tls_private_key_path above) to enable it. # - #enabled: true + enabled: False # Endpoint to use to request certificates. If you only want to test, # use Let's Encrypt's staging url: @@ -406,17 +406,17 @@ acme: # Port number to listen on for the HTTP-01 challenge. Change this if # you are forwarding connections through Apache/Nginx/etc. # - #port: 80 + port: 80 # Local addresses to listen on for incoming connections. # Again, you may want to change this if you are forwarding connections # through Apache/Nginx/etc. # - #bind_addresses: ['::', '0.0.0.0'] + bind_addresses: ['::', '0.0.0.0'] # How many days remaining on a certificate before it is renewed. # - #reprovision_threshold: 30 + reprovision_threshold: 30 # The domain that the certificate should be for. Normally this # should be the same as your Matrix domain (i.e., 'server_name'), but, @@ -430,7 +430,7 @@ acme: # # If not set, defaults to your 'server_name'. # - #domain: matrix.example.com + domain: matrix.example.com # file to use for the account key. This will be generated if it doesn't # exist. diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 0699f15d0d7b..7cebe1097060 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -268,7 +268,7 @@ def generate_config_section( if not tls_private_key_path: tls_private_key_path = base_key_name + ".tls.key" - acme_enabled = "" if acme_domain else "#" + acme_enabled = bool(acme_domain) acme_domain = "matrix.example.com" default_acme_account_file = os.path.join(data_dir_path, "acme_account.key") @@ -369,7 +369,7 @@ def generate_config_section( # ACME support is disabled by default. Uncomment the following line # (and tls_certificate_path and tls_private_key_path above) to enable it. # - %(acme_enabled)senabled: true + enabled: %(acme_enabled)s # Endpoint to use to request certificates. If you only want to test, # use Let's Encrypt's staging url: @@ -380,17 +380,17 @@ def generate_config_section( # Port number to listen on for the HTTP-01 challenge. Change this if # you are forwarding connections through Apache/Nginx/etc. # - %(acme_enabled)sport: 80 + port: 80 # Local addresses to listen on for incoming connections. # Again, you may want to change this if you are forwarding connections # through Apache/Nginx/etc. # - %(acme_enabled)sbind_addresses: ['::', '0.0.0.0'] + bind_addresses: ['::', '0.0.0.0'] # How many days remaining on a certificate before it is renewed. # - %(acme_enabled)sreprovision_threshold: 30 + reprovision_threshold: 30 # The domain that the certificate should be for. Normally this # should be the same as your Matrix domain (i.e., 'server_name'), but, @@ -404,7 +404,7 @@ def generate_config_section( # # If not set, defaults to your 'server_name'. # - %(acme_enabled)sdomain: %(acme_domain)s + domain: %(acme_domain)s # file to use for the account key. This will be generated if it doesn't # exist. From 3a9ab4853d9e56fb513add7ecb14af6476e22ff2 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 23 Aug 2019 11:48:46 +0100 Subject: [PATCH 17/23] Update comment --- docs/sample_config.yaml | 4 ++-- synapse/config/tls.py | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index 965545968523..da6b0f8ff534 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -392,8 +392,8 @@ listeners: # permission to listen on port 80. # acme: - # ACME support is disabled by default. Uncomment the following line - # (and tls_certificate_path and tls_private_key_path above) to enable it. + # ACME support is disabled by default. Set this to `true` and uncomment + # tls_certificate_path and tls_private_key_path above to enable it. # enabled: False diff --git a/synapse/config/tls.py b/synapse/config/tls.py index 7cebe1097060..c0148aa95c2f 100644 --- a/synapse/config/tls.py +++ b/synapse/config/tls.py @@ -366,8 +366,8 @@ def generate_config_section( # permission to listen on port 80. # acme: - # ACME support is disabled by default. Uncomment the following line - # (and tls_certificate_path and tls_private_key_path above) to enable it. + # ACME support is disabled by default. Set this to `true` and uncomment + # tls_certificate_path and tls_private_key_path above to enable it. # enabled: %(acme_enabled)s From 4f7450297d1dc4b46fd99288eb9ba4998deb19ca Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 23 Aug 2019 13:55:46 +0100 Subject: [PATCH 18/23] Ports should be bound if they are *not* open. --- synapse/config/server.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index e4e1db827710..61f3004d9257 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -381,7 +381,7 @@ def generate_config_section( # idea of passing in your own listener config however # the local addresses need to be bound if open_private_ports is # specified. - if open_private_ports: + if not open_private_ports: bind_addresses = listener.setdefault("bind_addresses", []) for address in private_addresses: if address not in bind_addresses: From 4d31a54847fbc22ae0b1382c422822206d2b9799 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 23 Aug 2019 14:35:02 +0100 Subject: [PATCH 19/23] Do something similar to what read_config does with openports. --- synapse/config/server.py | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/synapse/config/server.py b/synapse/config/server.py index 61f3004d9257..2abdef0971a6 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -377,15 +377,13 @@ def generate_config_section( if listener["tls"]: secure_listeners.append(listener) else: - # The open_private_ports option kind of conflicts with the - # idea of passing in your own listener config however - # the local addresses need to be bound if open_private_ports is - # specified. + # If we don't want open ports we need to bind the listeners + # to some address other than 0.0.0.0. Here we chose to use + # localhost. + # If the addresses are already bound we won't overwrite them + # however. if not open_private_ports: - bind_addresses = listener.setdefault("bind_addresses", []) - for address in private_addresses: - if address not in bind_addresses: - bind_addresses.append(address) + listener.setdefault("bind_addresses", private_addresses) unsecure_listeners.append(listener) From da080f89bab654020345ed1cd44f83beb521baa1 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Fri, 23 Aug 2019 14:40:12 +0100 Subject: [PATCH 20/23] Template tests --- tests/config/test_server.py | 101 +++++++++++++++++++++++++++++++++++- tests/config/test_tls.py | 44 ++++++++++++++++ 2 files changed, 144 insertions(+), 1 deletion(-) diff --git a/tests/config/test_server.py b/tests/config/test_server.py index 1ca5ea54ca6e..a10d01712049 100644 --- a/tests/config/test_server.py +++ b/tests/config/test_server.py @@ -13,7 +13,9 @@ # See the License for the specific language governing permissions and # limitations under the License. -from synapse.config.server import is_threepid_reserved +import yaml + +from synapse.config.server import ServerConfig, is_threepid_reserved from tests import unittest @@ -29,3 +31,100 @@ def test_is_threepid_reserved(self): self.assertTrue(is_threepid_reserved(config, user1)) self.assertFalse(is_threepid_reserved(config, user3)) self.assertFalse(is_threepid_reserved(config, user1_msisdn)) + + def test_unsecure_listener_no_listeners_open_private_ports_false(self): + conf = yaml.safe_load( + ServerConfig().generate_config_section( + "che.org", "/data_dir_path", False, None + ) + ) + + expected_listeners = [ + { + "port": 8008, + "tls": False, + "type": "http", + "x_forwarded": True, + "bind_addresses": ["::1", "127.0.0.1"], + "resources": [{"names": ["client", "federation"], "compress": False}], + } + ] + + self.assertEqual(conf["listeners"], expected_listeners) + + def test_unsecure_listener_no_listeners_open_private_ports_true(self): + conf = yaml.safe_load( + ServerConfig().generate_config_section( + "che.org", "/data_dir_path", True, None + ) + ) + + expected_listeners = [ + { + "port": 8008, + "tls": False, + "type": "http", + "x_forwarded": True, + "resources": [{"names": ["client", "federation"], "compress": False}], + } + ] + + self.assertEqual(conf["listeners"], expected_listeners) + + def test_listeners_set_correctly_open_private_ports_false(self): + listeners = [ + { + "port": 8448, + "resources": [{"names": ["federation"]}], + "tls": True, + "type": "http", + }, + { + "port": 443, + "resources": [{"names": ["client"]}], + "tls": False, + "type": "http", + }, + ] + + conf = yaml.safe_load( + ServerConfig().generate_config_section( + "this.one.listens", "/data_dir_path", True, listeners + ) + ) + + self.assertEqual(conf["listeners"], listeners) + + def test_listeners_set_correctly_open_private_ports_true(self): + listeners = [ + { + "port": 8448, + "resources": [{"names": ["federation"]}], + "tls": True, + "type": "http", + }, + { + "port": 443, + "resources": [{"names": ["client"]}], + "tls": False, + "type": "http", + }, + { + "port": 1243, + "resources": [{"names": ["client"]}], + "tls": False, + "type": "http", + "bind_addresses": ["this_one_is_bound"], + }, + ] + + expected_listeners = listeners.copy() + expected_listeners[1]["bind_addresses"] = ["::1", "127.0.0.1"] + + conf = yaml.safe_load( + ServerConfig().generate_config_section( + "this.one.listens", "/data_dir_path", True, listeners + ) + ) + + self.assertEqual(conf["listeners"], expected_listeners) diff --git a/tests/config/test_tls.py b/tests/config/test_tls.py index 4f8a87a3dfde..8e0c4b9533a5 100644 --- a/tests/config/test_tls.py +++ b/tests/config/test_tls.py @@ -16,6 +16,8 @@ import os +import yaml + from OpenSSL import SSL from synapse.config.tls import ConfigError, TlsConfig @@ -191,3 +193,45 @@ def test_tls_client_minimum_set_passed_through_1_0(self): self.assertEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1, 0) self.assertEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1_1, 0) self.assertEqual(cf._verify_ssl._options & SSL.OP_NO_TLSv1_2, 0) + + def test_acme_disabled_in_generated_config_no_acme_domain_provied(self): + """ + Checks acme is disabled by default. + """ + conf = TestConfig() + conf.read_config( + yaml.safe_load( + TestConfig().generate_config_section( + "/config_dir_path", + "my_super_secure_server", + "/data_dir_path", + "/tls_cert_path", + "tls_private_key", + None, # This is the acme_domain + ) + ), + "/config_dir_path", + ) + + self.assertFalse(conf.acme_enabled) + + def test_acme_enabled_in_generated_config_domain_provided(self): + """ + Checks acme is enabled if the acme_domain arg is set to some string. + """ + conf = TestConfig() + conf.read_config( + yaml.safe_load( + TestConfig().generate_config_section( + "/config_dir_path", + "my_super_secure_server", + "/data_dir_path", + "/tls_cert_path", + "tls_private_key", + "my_supe_secure_server", # This is the acme_domain + ) + ), + "/config_dir_path", + ) + + self.assertTrue(conf.acme_enabled) From 1c0558950d336278029275e3c99da48f6fb6e836 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Tue, 27 Aug 2019 18:20:38 +0100 Subject: [PATCH 21/23] Pass database args opaquely. --- synapse/config/database.py | 30 ++++++++++++-------- tests/config/test_database.py | 52 +++++++++++++++++++++++++++++++++++ 2 files changed, 71 insertions(+), 11 deletions(-) create mode 100644 tests/config/test_database.py diff --git a/synapse/config/database.py b/synapse/config/database.py index a56a578d6ad5..7fe85c20e81d 100644 --- a/synapse/config/database.py +++ b/synapse/config/database.py @@ -13,6 +13,9 @@ # See the License for the specific language governing permissions and # limitations under the License. import os +from textwrap import indent + +import yaml from ._base import Config @@ -38,22 +41,27 @@ def read_config(self, config, **kwargs): self.set_databasepath(config.get("database_path")) - def generate_config_section(self, data_dir_path, database, **kwargs): - if not database: - database = "sqlite3" - database_path = os.path.join(data_dir_path, "homeserver.db") + def generate_config_section(self, data_dir_path, database_conf, **kwargs): + if not database_conf: + database_path = os.path.join(data_dir_path, "homeserver.db") + database_conf = ( + """ # The database engine name + name: "sqlite3" + # Arguments to pass to the engine + args: + # Path to the database + database: "%(database_path)s" """ + % locals() + ) + else: + database_conf = indent(yaml.dump(database_conf), " " * 10).lstrip() + return ( """\ ## Database ## database: - # The database engine name - name: "%(database)s" - # Arguments to pass to the engine - args: - # Path to the database - database: "%(database_path)s" - + %(database_conf)s # Number of events to cache in memory. # #event_cache_size: 10K diff --git a/tests/config/test_database.py b/tests/config/test_database.py new file mode 100644 index 000000000000..151d3006acb6 --- /dev/null +++ b/tests/config/test_database.py @@ -0,0 +1,52 @@ +# -*- 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 yaml + +from synapse.config.database import DatabaseConfig + +from tests import unittest + + +class DatabaseConfigTestCase(unittest.TestCase): + def test_database_configured_correctly_no_database_conf_param(self): + conf = yaml.safe_load( + DatabaseConfig().generate_config_section("/data_dir_path", None) + ) + + expected_database_conf = { + "name": "sqlite3", + "args": {"database": "/data_dir_path/homeserver.db"}, + } + + self.assertEqual(conf["database"], expected_database_conf) + + def test_database_configured_correctly_database_conf_param(self): + + database_conf = { + "name": "my super fast datastore", + "args": { + "user": "matrix", + "password": "synapse_database_password", + "host": "synapse_database_host", + "database": "matrix", + }, + } + + conf = yaml.safe_load( + DatabaseConfig().generate_config_section("/data_dir_path", database_conf) + ) + + self.assertEqual(conf["database"], database_conf) From 861019deb313c09b4e10845d475f7f6c1a656fec Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Tue, 27 Aug 2019 18:24:16 +0100 Subject: [PATCH 22/23] Properly integrate --- synapse/config/_base.py | 4 ++-- synapse/config/database.py | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 3b72899cc46e..31f65309784c 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -182,7 +182,7 @@ def generate_config( report_stats=None, open_private_ports=False, listeners=None, - database=None, + database_conf=None, tls_certificate_path=None, tls_private_key_path=None, acme_domain=None, @@ -253,7 +253,7 @@ def generate_config( report_stats=report_stats, open_private_ports=open_private_ports, listeners=listeners, - database=database, + database_conf=database_conf, tls_certificate_path=tls_certificate_path, tls_private_key_path=tls_private_key_path, acme_domain=acme_domain, diff --git a/synapse/config/database.py b/synapse/config/database.py index 7fe85c20e81d..650e8552275b 100644 --- a/synapse/config/database.py +++ b/synapse/config/database.py @@ -62,6 +62,7 @@ def generate_config_section(self, data_dir_path, database_conf, **kwargs): database: %(database_conf)s + # Number of events to cache in memory. # #event_cache_size: 10K From e4951118e1140ae23578535b4cd47e80b6e8efe6 Mon Sep 17 00:00:00 2001 From: Jorik Schellekens Date: Tue, 27 Aug 2019 18:40:03 +0100 Subject: [PATCH 23/23] Don't change the sample config --- synapse/config/database.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/config/database.py b/synapse/config/database.py index 650e8552275b..118aafbd4ae8 100644 --- a/synapse/config/database.py +++ b/synapse/config/database.py @@ -45,12 +45,13 @@ def generate_config_section(self, data_dir_path, database_conf, **kwargs): if not database_conf: database_path = os.path.join(data_dir_path, "homeserver.db") database_conf = ( - """ # The database engine name + """# The database engine name name: "sqlite3" # Arguments to pass to the engine args: # Path to the database - database: "%(database_path)s" """ + database: "%(database_path)s" + """ % locals() ) else: @@ -62,7 +63,6 @@ def generate_config_section(self, data_dir_path, database_conf, **kwargs): database: %(database_conf)s - # Number of events to cache in memory. # #event_cache_size: 10K