From e8d52f2281ff3e87837c2edff0d3805a9c75a9a8 Mon Sep 17 00:00:00 2001 From: Tuomas Ojamies Date: Tue, 11 Oct 2022 07:04:41 +0000 Subject: [PATCH 01/10] Fix missing SSL support in worker endpoints. --- synapse/app/generic_worker.py | 44 +++++++++++++------ .../test_sharded_event_persister.py | 4 +- 2 files changed, 32 insertions(+), 16 deletions(-) diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 2a9f039367b9..bdbe8ba63642 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -270,6 +270,7 @@ class GenericWorkerServer(HomeServer): def _listen_http(self, listener_config: ListenerConfig) -> None: port = listener_config.port bind_addresses = listener_config.bind_addresses + tls = listener_config.tls assert listener_config.http_options is not None @@ -375,22 +376,37 @@ def _listen_http(self, listener_config: ListenerConfig) -> None: root_resource = create_resource_tree(resources, OptionsResource()) - _base.listen_tcp( - bind_addresses, - port, - SynapseSite( - "synapse.access.http.%s" % (site_tag,), - site_tag, - listener_config, - root_resource, - self.version_string, - max_request_body_size=max_request_body_size(self.config), - reactor=self.get_reactor(), - ), + site = SynapseSite( + "synapse.access.%s.%s" % ("https" if tls else "http", site_tag), + site_tag, + listener_config, + root_resource, + self.version_string, + max_request_body_size=max_request_body_size(self.config), reactor=self.get_reactor(), ) - - logger.info("Synapse worker now listening on port %d", port) + # This has same logic as synapse/app/homeserver.py where we choose between + # listen_ssl and listen_tcp based on the tls configuration flag in listener + # config. + if tls: + # refresh_certificate should have been called before this. + assert self.tls_server_context_factory is not None + _base.listen_ssl( + bind_addresses, + port, + site, + self.tls_server_context_factory, + reactor=self.get_reactor(), + ) + logger.info("Synapse worker now listening on port %d (TLS)", port) + else: + _base.listen_tcp( + bind_addresses, + port, + site, + reactor=self.get_reactor(), + ) + logger.info("Synapse worker now listening on port %d", port) def start_listening(self) -> None: for listener in self.config.worker.worker_listeners: diff --git a/tests/replication/test_sharded_event_persister.py b/tests/replication/test_sharded_event_persister.py index 541d3902860c..9b3b76f053a2 100644 --- a/tests/replication/test_sharded_event_persister.py +++ b/tests/replication/test_sharded_event_persister.py @@ -46,8 +46,8 @@ def default_config(self): conf = super().default_config() conf["stream_writers"] = {"events": ["worker1", "worker2"]} conf["instance_map"] = { - "worker1": {"host": "testserv", "port": 1001}, - "worker2": {"host": "testserv", "port": 1002}, + "worker1": {"host": "testserv", "port": 1001, "tls": False}, + "worker2": {"host": "testserv", "port": 1002, "tls": False}, } return conf From ab45ab1e5529e892520f1736aea20c91526a7c35 Mon Sep 17 00:00:00 2001 From: Tuomas Ojamies Date: Tue, 11 Oct 2022 07:11:45 +0000 Subject: [PATCH 02/10] Add changelog --- changelog.d/14128.misc | 1 + 1 file changed, 1 insertion(+) create mode 100644 changelog.d/14128.misc diff --git a/changelog.d/14128.misc b/changelog.d/14128.misc new file mode 100644 index 000000000000..29168ef95540 --- /dev/null +++ b/changelog.d/14128.misc @@ -0,0 +1 @@ +Add TLS support for generic worker endpoints. From 68af173ead5f6fce4c721f0c8edd1b2f3a269783 Mon Sep 17 00:00:00 2001 From: Tuomas Ojamies Date: Thu, 15 Sep 2022 09:28:40 +0200 Subject: [PATCH 03/10] SSL for Replication endpoint --- synapse/config/workers.py | 7 +++++++ synapse/replication/http/_base.py | 10 +++++++++- 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 0fb725dd8fc6..58af4c1b6edf 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -67,6 +67,7 @@ class InstanceLocationConfig: host: str port: int + tls: bool @attr.s @@ -149,6 +150,12 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # The port on the main synapse for HTTP replication endpoint self.worker_replication_http_port = config.get("worker_replication_http_port") + # The tls mode on the main synapse for HTTP replication endpoint. + # For backward compatibility this defaults to False. + self.worker_replication_http_tls = config.get( + "worker_replication_http_tls", False + ) + # The shared secret used for authentication when connecting to the main synapse. self.worker_replication_secret = config.get("worker_replication_secret", None) diff --git a/synapse/replication/http/_base.py b/synapse/replication/http/_base.py index acb0bd18f72e..5e661f8c73c1 100644 --- a/synapse/replication/http/_base.py +++ b/synapse/replication/http/_base.py @@ -184,8 +184,10 @@ def make_client(cls, hs: "HomeServer") -> Callable: client = hs.get_simple_http_client() local_instance_name = hs.get_instance_name() + # The value of these option should match the replication listener settings master_host = hs.config.worker.worker_replication_host master_port = hs.config.worker.worker_replication_http_port + master_tls = hs.config.worker.worker_replication_http_tls instance_map = hs.config.worker.instance_map @@ -205,9 +207,11 @@ async def send_request(*, instance_name: str = "master", **kwargs: Any) -> Any: if instance_name == "master": host = master_host port = master_port + tls = master_tls elif instance_name in instance_map: host = instance_map[instance_name].host port = instance_map[instance_name].port + tls = instance_map[instance_name].tls else: raise Exception( "Instance %r not in 'instance_map' config" % (instance_name,) @@ -238,7 +242,11 @@ async def send_request(*, instance_name: str = "master", **kwargs: Any) -> Any: "Unknown METHOD on %s replication endpoint" % (cls.NAME,) ) - uri = "http://%s:%s/_synapse/replication/%s/%s" % ( + # Here the protocol is hard coded to be http by default or https in case the replication + # port is set to have tls true. + scheme = "https" if tls else "http" + uri = "%s://%s:%s/_synapse/replication/%s/%s" % ( + scheme, host, port, cls.NAME, From a43b3c0ea2bd28b4ffa4be720b78263dd068f01b Mon Sep 17 00:00:00 2001 From: Tuomas Ojamies Date: Wed, 26 Oct 2022 11:13:25 +0000 Subject: [PATCH 04/10] Remove unit test change --- synapse/config/workers.py | 2 +- tests/replication/test_sharded_event_persister.py | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/synapse/config/workers.py b/synapse/config/workers.py index 58af4c1b6edf..88b3168cbc71 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -67,7 +67,7 @@ class InstanceLocationConfig: host: str port: int - tls: bool + tls: bool = False @attr.s diff --git a/tests/replication/test_sharded_event_persister.py b/tests/replication/test_sharded_event_persister.py index 9b3b76f053a2..541d3902860c 100644 --- a/tests/replication/test_sharded_event_persister.py +++ b/tests/replication/test_sharded_event_persister.py @@ -46,8 +46,8 @@ def default_config(self): conf = super().default_config() conf["stream_writers"] = {"events": ["worker1", "worker2"]} conf["instance_map"] = { - "worker1": {"host": "testserv", "port": 1001, "tls": False}, - "worker2": {"host": "testserv", "port": 1002, "tls": False}, + "worker1": {"host": "testserv", "port": 1001}, + "worker2": {"host": "testserv", "port": 1002}, } return conf From 2d14b26cb56e30f78773ca5f8d156c36ac66b108 Mon Sep 17 00:00:00 2001 From: Tuomas Ojamies Date: Wed, 26 Oct 2022 13:59:10 +0200 Subject: [PATCH 05/10] Refactor listener creation to reduce duplicated code --- synapse/app/_base.py | 56 ++++++++++++++++++++++++++++++++++- synapse/app/generic_worker.py | 38 +++--------------------- synapse/app/homeserver.py | 34 +++------------------ 3 files changed, 63 insertions(+), 65 deletions(-) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index a683ebf4cbe9..57c8f36e083e 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -47,6 +47,7 @@ from twisted.logger import LoggingFile, LogLevel from twisted.protocols.tls import TLSMemoryBIOFactory from twisted.python.threadpool import ThreadPool +from twisted.web.resource import Resource import synapse.util.caches from synapse.api.constants import MAX_PDU_SIZE @@ -55,12 +56,13 @@ from synapse.config import ConfigError from synapse.config._base import format_config_error from synapse.config.homeserver import HomeServerConfig -from synapse.config.server import ManholeConfig +from synapse.config.server import ListenerConfig, ManholeConfig from synapse.crypto import context_factory from synapse.events.presence_router import load_legacy_presence_router from synapse.events.spamcheck import load_legacy_spam_checkers from synapse.events.third_party_rules import load_legacy_third_party_event_rules from synapse.handlers.auth import load_legacy_password_auth_providers +from synapse.http.site import SynapseSite from synapse.logging.context import PreserveLoggingContext from synapse.logging.opentracing import init_tracer from synapse.metrics import install_gc_manager, register_threadpool @@ -357,6 +359,58 @@ def listen_tcp( return r # type: ignore[return-value] +def listen_http( + listener_config: ListenerConfig, + root_resource: Resource, + version_string, + max_request_body_size, + context_factory: IOpenSSLContextFactory, + reactor: IReactorSSL = reactor, +): + port = listener_config.port + bind_addresses = listener_config.bind_addresses + tls = listener_config.tls + + assert listener_config.http_options is not None + + site_tag = listener_config.http_options.tag + if site_tag is None: + site_tag = str(port) + + site = SynapseSite( + "synapse.access.%s.%s" % ("https" if tls else "http", site_tag), + site_tag, + listener_config, + root_resource, + version_string, + max_request_body_size=max_request_body_size, + reactor=reactor, + ) + # This has same logic as synapse/app/homeserver.py where we choose between + # listen_ssl and listen_tcp based on the tls configuration flag in listener + # config. + if tls: + # refresh_certificate should have been called before this. + assert context_factory is not None + ports = listen_ssl( + bind_addresses, + port, + site, + context_factory, + reactor=reactor, + ) + logger.info("Synapse worker now listening on port %d (TLS)", port) + else: + ports = listen_tcp( + bind_addresses, + port, + site, + reactor=reactor, + ) + logger.info("Synapse worker now listening on port %d", port) + return ports + + def listen_ssl( bind_addresses: Collection[str], port: int, diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index bdbe8ba63642..7d35659df7ec 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -44,7 +44,7 @@ from synapse.federation.transport.server import TransportLayerServer from synapse.http.server import JsonResource, OptionsResource from synapse.http.servlet import RestServlet, parse_json_object_from_request -from synapse.http.site import SynapseRequest, SynapseSite +from synapse.http.site import SynapseRequest from synapse.logging.context import LoggingContext from synapse.metrics import METRICS_PREFIX, MetricsResource, RegistryProxy from synapse.replication.http import REPLICATION_PREFIX, ReplicationRestResource @@ -268,16 +268,9 @@ class GenericWorkerServer(HomeServer): DATASTORE_CLASS = GenericWorkerSlavedStore # type: ignore def _listen_http(self, listener_config: ListenerConfig) -> None: - port = listener_config.port - bind_addresses = listener_config.bind_addresses - tls = listener_config.tls assert listener_config.http_options is not None - site_tag = listener_config.http_options.tag - if site_tag is None: - site_tag = str(port) - # We always include a health resource. resources: Dict[str, Resource] = {"/health": HealthResource()} @@ -376,37 +369,14 @@ def _listen_http(self, listener_config: ListenerConfig) -> None: root_resource = create_resource_tree(resources, OptionsResource()) - site = SynapseSite( - "synapse.access.%s.%s" % ("https" if tls else "http", site_tag), - site_tag, + _base.listen_http( listener_config, root_resource, self.version_string, - max_request_body_size=max_request_body_size(self.config), + max_request_body_size(self.config), + self.tls_server_context_factory, reactor=self.get_reactor(), ) - # This has same logic as synapse/app/homeserver.py where we choose between - # listen_ssl and listen_tcp based on the tls configuration flag in listener - # config. - if tls: - # refresh_certificate should have been called before this. - assert self.tls_server_context_factory is not None - _base.listen_ssl( - bind_addresses, - port, - site, - self.tls_server_context_factory, - reactor=self.get_reactor(), - ) - logger.info("Synapse worker now listening on port %d (TLS)", port) - else: - _base.listen_tcp( - bind_addresses, - port, - site, - reactor=self.get_reactor(), - ) - logger.info("Synapse worker now listening on port %d", port) def start_listening(self) -> None: for listener in self.config.worker.worker_listeners: diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index de3f08876f99..4f4fee4782f2 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -37,8 +37,7 @@ from synapse.app import _base from synapse.app._base import ( handle_startup_exception, - listen_ssl, - listen_tcp, + listen_http, max_request_body_size, redirect_stdio_to_logs, register_start, @@ -53,7 +52,6 @@ RootOptionsRedirectResource, StaticResource, ) -from synapse.http.site import SynapseSite from synapse.logging.context import LoggingContext from synapse.metrics import METRICS_PREFIX, MetricsResource, RegistryProxy from synapse.replication.http import REPLICATION_PREFIX, ReplicationRestResource @@ -83,8 +81,6 @@ def _listener_http( self, config: HomeServerConfig, listener_config: ListenerConfig ) -> Iterable[Port]: port = listener_config.port - bind_addresses = listener_config.bind_addresses - tls = listener_config.tls # Must exist since this is an HTTP listener. assert listener_config.http_options is not None site_tag = listener_config.http_options.tag @@ -140,37 +136,15 @@ def _listener_http( else: root_resource = OptionsResource() - site = SynapseSite( - "synapse.access.%s.%s" % ("https" if tls else "http", site_tag), - site_tag, + ports = listen_http( listener_config, create_resource_tree(resources, root_resource), self.version_string, - max_request_body_size=max_request_body_size(self.config), + max_request_body_size(self.config), + self.tls_server_context_factory, reactor=self.get_reactor(), ) - if tls: - # refresh_certificate should have been called before this. - assert self.tls_server_context_factory is not None - ports = listen_ssl( - bind_addresses, - port, - site, - self.tls_server_context_factory, - reactor=self.get_reactor(), - ) - logger.info("Synapse now listening on TCP port %d (TLS)", port) - - else: - ports = listen_tcp( - bind_addresses, - port, - site, - reactor=self.get_reactor(), - ) - logger.info("Synapse now listening on TCP port %d", port) - return ports def _configure_named_resource( From ac5184f7dd44ce1c2f5f4b2059e43f1f4651f84b Mon Sep 17 00:00:00 2001 From: Tuomas Ojamies Date: Fri, 28 Oct 2022 08:11:40 +0000 Subject: [PATCH 06/10] Fix the logger message --- synapse/app/_base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 57c8f36e083e..57488c39fb38 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -399,7 +399,7 @@ def listen_http( context_factory, reactor=reactor, ) - logger.info("Synapse worker now listening on port %d (TLS)", port) + logger.info("Synapse now listening on TCP port %d (TLS)", port) else: ports = listen_tcp( bind_addresses, @@ -407,7 +407,7 @@ def listen_http( site, reactor=reactor, ) - logger.info("Synapse worker now listening on port %d", port) + logger.info("Synapse now listening on TCP port %d", port) return ports From 6d6bce38e54e929cada63f9e28cf17cfb01cd8b4 Mon Sep 17 00:00:00 2001 From: Tuomas Ojamies Date: Mon, 31 Oct 2022 18:15:17 +0100 Subject: [PATCH 07/10] Update synapse/app/_base.py Co-authored-by: Patrick Cloke --- synapse/app/_base.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 57488c39fb38..b669f8a2f49e 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -366,7 +366,7 @@ def listen_http( max_request_body_size, context_factory: IOpenSSLContextFactory, reactor: IReactorSSL = reactor, -): +) -> List[Port]: port = listener_config.port bind_addresses = listener_config.bind_addresses tls = listener_config.tls From 415ee8c703967e47a676371ea7f91ca812715d8a Mon Sep 17 00:00:00 2001 From: Tuomas Ojamies Date: Mon, 31 Oct 2022 18:15:35 +0100 Subject: [PATCH 08/10] Update synapse/app/_base.py Co-authored-by: Patrick Cloke --- synapse/app/_base.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index b669f8a2f49e..4b938f7c505b 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -362,8 +362,8 @@ def listen_tcp( def listen_http( listener_config: ListenerConfig, root_resource: Resource, - version_string, - max_request_body_size, + version_string: str, + max_request_body_size: int, context_factory: IOpenSSLContextFactory, reactor: IReactorSSL = reactor, ) -> List[Port]: From 36aa70c65870965195a57fa43cf9edd48cc1e032 Mon Sep 17 00:00:00 2001 From: Tuomas Ojamies Date: Mon, 31 Oct 2022 18:16:01 +0100 Subject: [PATCH 09/10] Update synapse/app/_base.py Co-authored-by: Patrick Cloke --- synapse/app/_base.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 4b938f7c505b..8f5b1a20f517 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -386,9 +386,6 @@ def listen_http( max_request_body_size=max_request_body_size, reactor=reactor, ) - # This has same logic as synapse/app/homeserver.py where we choose between - # listen_ssl and listen_tcp based on the tls configuration flag in listener - # config. if tls: # refresh_certificate should have been called before this. assert context_factory is not None From 4fda2d5e8d3721145b57727c9eb5e9d6414388c5 Mon Sep 17 00:00:00 2001 From: "Olivier Wilkinson (reivilibre)" Date: Tue, 15 Nov 2022 12:52:44 +0000 Subject: [PATCH 10/10] Add config documentation for new TLS option --- .../configuration/config_documentation.md | 20 +++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/docs/usage/configuration/config_documentation.md b/docs/usage/configuration/config_documentation.md index 9a6bd08d016f..f5937dd902ff 100644 --- a/docs/usage/configuration/config_documentation.md +++ b/docs/usage/configuration/config_documentation.md @@ -3893,6 +3893,26 @@ Example configuration: worker_replication_http_port: 9093 ``` --- +### `worker_replication_http_tls` + +Whether TLS should be used for talking to the HTTP replication port on the main +Synapse process. +The main Synapse process defines this with the `tls` option on its [listener](#listeners) that +has the `replication` resource enabled. + +**Please note:** by default, it is not safe to expose replication ports to the +public Internet, even with TLS enabled. +See [`worker_replication_secret`](#worker_replication_secret). + +Defaults to `false`. + +*Added in Synapse 1.72.0.* + +Example configuration: +```yaml +worker_replication_http_tls: true +``` +--- ### `worker_listeners` A worker can handle HTTP requests. To do so, a `worker_listeners` option