From 6e895366ea7f194cd48fae08a9909ee01a9fadae Mon Sep 17 00:00:00 2001 From: Azrenbeth <77782548+Azrenbeth@users.noreply.github.com> Date: Mon, 6 Sep 2021 16:08:03 +0100 Subject: [PATCH] Add config option to use non-default manhole password and keys (#10643) --- changelog.d/10643.feature | 1 + docs/manhole.md | 29 ++++++++++-- docs/sample_config.yaml | 18 ++++++++ synapse/app/_base.py | 10 +++- synapse/app/generic_worker.py | 5 +- synapse/app/homeserver.py | 5 +- synapse/config/server.py | 87 ++++++++++++++++++++++++++++++++++- synapse/util/manhole.py | 15 ++++-- tests/config/test_server.py | 8 ++-- 9 files changed, 161 insertions(+), 17 deletions(-) create mode 100644 changelog.d/10643.feature diff --git a/changelog.d/10643.feature b/changelog.d/10643.feature new file mode 100644 index 0000000000..bd63a3d258 --- /dev/null +++ b/changelog.d/10643.feature @@ -0,0 +1 @@ +Add config option to use non-default manhole password and keys. \ No newline at end of file diff --git a/docs/manhole.md b/docs/manhole.md index db92df88dc..715ed840f2 100644 --- a/docs/manhole.md +++ b/docs/manhole.md @@ -11,7 +11,7 @@ Note that this will give administrative access to synapse to **all users** with shell access to the server. It should therefore **not** be enabled in environments where untrusted users have shell access. -*** +## Configuring the manhole To enable it, first uncomment the `manhole` listener configuration in `homeserver.yaml`. The configuration is slightly different if you're using docker. @@ -52,16 +52,37 @@ listeners: type: manhole ``` -#### Accessing synapse manhole +### Security settings + +The following config options are available: + +- `username` - The username for the manhole (defaults to `matrix`) +- `password` - The password for the manhole (defaults to `rabbithole`) +- `ssh_priv_key` - The path to a private SSH key (defaults to a hardcoded value) +- `ssh_pub_key` - The path to a public SSH key (defaults to a hardcoded value) + +For example: + +```yaml +manhole_settings: + username: manhole + password: mypassword + ssh_priv_key: "/home/synapse/manhole_keys/id_rsa" + ssh_pub_key: "/home/synapse/manhole_keys/id_rsa.pub" +``` + + +## Accessing synapse manhole Then restart synapse, and point an ssh client at port 9000 on localhost, using -the username `matrix`: +the username and password configured in `homeserver.yaml` - with the default +configuration, this would be: ```bash ssh -p9000 matrix@localhost ``` -The password is `rabbithole`. +Then enter the password when prompted (the default is `rabbithole`). This gives a Python REPL in which `hs` gives access to the `synapse.server.HomeServer` object - which in turn gives access to many other diff --git a/docs/sample_config.yaml b/docs/sample_config.yaml index e155b978d8..e15a832220 100644 --- a/docs/sample_config.yaml +++ b/docs/sample_config.yaml @@ -335,6 +335,24 @@ listeners: # bind_addresses: ['::1', '127.0.0.1'] # type: manhole +# Connection settings for the manhole +# +manhole_settings: + # The username for the manhole. This defaults to 'matrix'. + # + #username: manhole + + # The password for the manhole. This defaults to 'rabbithole'. + # + #password: mypassword + + # The private and public SSH key pair used to encrypt the manhole traffic. + # If these are left unset, then hardcoded and non-secret keys are used, + # which could allow traffic to be intercepted if sent over a public network. + # + #ssh_priv_key_path: CONFDIR/id_rsa + #ssh_pub_key_path: CONFDIR/id_rsa.pub + # Forward extremities can build up in a room due to networking delays between # homeservers. Once this happens in a large room, calculation of the state of # that room can become quite expensive. To mitigate this, once the number of diff --git a/synapse/app/_base.py b/synapse/app/_base.py index 6fc14930d1..89bda00090 100644 --- a/synapse/app/_base.py +++ b/synapse/app/_base.py @@ -37,6 +37,7 @@ from synapse.api.constants import MAX_PDU_SIZE from synapse.app import check_bind_error from synapse.app.phone_stats_home import start_phone_stats_home from synapse.config.homeserver import HomeServerConfig +from synapse.config.server import 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 @@ -230,7 +231,12 @@ def listen_metrics(bind_addresses, port): start_http_server(port, addr=host, registry=RegistryProxy) -def listen_manhole(bind_addresses: Iterable[str], port: int, manhole_globals: dict): +def listen_manhole( + bind_addresses: Iterable[str], + port: int, + manhole_settings: ManholeConfig, + manhole_globals: dict, +): # twisted.conch.manhole 21.1.0 uses "int_from_bytes", which produces a confusing # warning. It's fixed by https://github.com/twisted/twisted/pull/1522), so # suppress the warning for now. @@ -245,7 +251,7 @@ def listen_manhole(bind_addresses: Iterable[str], port: int, manhole_globals: di listen_tcp( bind_addresses, port, - manhole(username="matrix", password="rabbithole", globals=manhole_globals), + manhole(settings=manhole_settings, globals=manhole_globals), ) diff --git a/synapse/app/generic_worker.py b/synapse/app/generic_worker.py index 9b71dd75e6..2eb8d5a79c 100644 --- a/synapse/app/generic_worker.py +++ b/synapse/app/generic_worker.py @@ -395,7 +395,10 @@ class GenericWorkerServer(HomeServer): self._listen_http(listener) elif listener.type == "manhole": _base.listen_manhole( - listener.bind_addresses, listener.port, manhole_globals={"hs": self} + listener.bind_addresses, + listener.port, + manhole_settings=self.config.server.manhole_settings, + manhole_globals={"hs": self}, ) elif listener.type == "metrics": if not self.config.enable_metrics: diff --git a/synapse/app/homeserver.py b/synapse/app/homeserver.py index 7dae163c1a..708db86f5d 100644 --- a/synapse/app/homeserver.py +++ b/synapse/app/homeserver.py @@ -291,7 +291,10 @@ class SynapseHomeServer(HomeServer): ) elif listener.type == "manhole": _base.listen_manhole( - listener.bind_addresses, listener.port, manhole_globals={"hs": self} + listener.bind_addresses, + listener.port, + manhole_settings=self.config.server.manhole_settings, + manhole_globals={"hs": self}, ) elif listener.type == "replication": services = listen_tcp( diff --git a/synapse/config/server.py b/synapse/config/server.py index d2c900f50c..7b9109a592 100644 --- a/synapse/config/server.py +++ b/synapse/config/server.py @@ -25,11 +25,14 @@ import attr import yaml from netaddr import AddrFormatError, IPNetwork, IPSet +from twisted.conch.ssh.keys import Key + from synapse.api.room_versions import KNOWN_ROOM_VERSIONS from synapse.util.module_loader import load_module from synapse.util.stringutils import parse_and_validate_server_name from ._base import Config, ConfigError +from ._util import validate_config logger = logging.Logger(__name__) @@ -216,6 +219,16 @@ class ListenerConfig: http_options = attr.ib(type=Optional[HttpListenerConfig], default=None) +@attr.s(frozen=True) +class ManholeConfig: + """Object describing the configuration of the manhole""" + + username = attr.ib(type=str, validator=attr.validators.instance_of(str)) + password = attr.ib(type=str, validator=attr.validators.instance_of(str)) + priv_key = attr.ib(type=Optional[Key]) + pub_key = attr.ib(type=Optional[Key]) + + class ServerConfig(Config): section = "server" @@ -649,6 +662,41 @@ class ServerConfig(Config): ) ) + manhole_settings = config.get("manhole_settings") or {} + validate_config( + _MANHOLE_SETTINGS_SCHEMA, manhole_settings, ("manhole_settings",) + ) + + manhole_username = manhole_settings.get("username", "matrix") + manhole_password = manhole_settings.get("password", "rabbithole") + manhole_priv_key_path = manhole_settings.get("ssh_priv_key_path") + manhole_pub_key_path = manhole_settings.get("ssh_pub_key_path") + + manhole_priv_key = None + if manhole_priv_key_path is not None: + try: + manhole_priv_key = Key.fromFile(manhole_priv_key_path) + except Exception as e: + raise ConfigError( + f"Failed to read manhole private key file {manhole_priv_key_path}" + ) from e + + manhole_pub_key = None + if manhole_pub_key_path is not None: + try: + manhole_pub_key = Key.fromFile(manhole_pub_key_path) + except Exception as e: + raise ConfigError( + f"Failed to read manhole public key file {manhole_pub_key_path}" + ) from e + + self.manhole_settings = ManholeConfig( + username=manhole_username, + password=manhole_password, + priv_key=manhole_priv_key, + pub_key=manhole_pub_key, + ) + metrics_port = config.get("metrics_port") if metrics_port: logger.warning(METRICS_PORT_WARNING) @@ -715,7 +763,7 @@ class ServerConfig(Config): if not isinstance(templates_config, dict): raise ConfigError("The 'templates' section must be a dictionary") - self.custom_template_directory = templates_config.get( + self.custom_template_directory: Optional[str] = templates_config.get( "custom_template_directory" ) if self.custom_template_directory is not None and not isinstance( @@ -727,7 +775,13 @@ class ServerConfig(Config): return any(listener.tls for listener in self.listeners) def generate_config_section( - self, server_name, data_dir_path, open_private_ports, listeners, **kwargs + self, + server_name, + data_dir_path, + open_private_ports, + listeners, + config_dir_path, + **kwargs, ): ip_range_blacklist = "\n".join( " # - '%s'" % ip for ip in DEFAULT_IP_RANGE_BLACKLIST @@ -1068,6 +1122,24 @@ class ServerConfig(Config): # bind_addresses: ['::1', '127.0.0.1'] # type: manhole + # Connection settings for the manhole + # + manhole_settings: + # The username for the manhole. This defaults to 'matrix'. + # + #username: manhole + + # The password for the manhole. This defaults to 'rabbithole'. + # + #password: mypassword + + # The private and public SSH key pair used to encrypt the manhole traffic. + # If these are left unset, then hardcoded and non-secret keys are used, + # which could allow traffic to be intercepted if sent over a public network. + # + #ssh_priv_key_path: %(config_dir_path)s/id_rsa + #ssh_pub_key_path: %(config_dir_path)s/id_rsa.pub + # Forward extremities can build up in a room due to networking delays between # homeservers. Once this happens in a large room, calculation of the state of # that room can become quite expensive. To mitigate this, once the number of @@ -1436,3 +1508,14 @@ def _warn_if_webclient_configured(listeners: Iterable[ListenerConfig]) -> None: if name == "webclient": logger.warning(NO_MORE_WEB_CLIENT_WARNING) return + + +_MANHOLE_SETTINGS_SCHEMA = { + "type": "object", + "properties": { + "username": {"type": "string"}, + "password": {"type": "string"}, + "ssh_priv_key_path": {"type": "string"}, + "ssh_pub_key_path": {"type": "string"}, + }, +} diff --git a/synapse/util/manhole.py b/synapse/util/manhole.py index 522daa323d..cfb5b94ca9 100644 --- a/synapse/util/manhole.py +++ b/synapse/util/manhole.py @@ -61,7 +61,7 @@ EddTrx3TNpr1D5m/f+6mnXWrc8u9y1+GNx9yz889xMjIBTBI9KqaaOs= -----END RSA PRIVATE KEY-----""" -def manhole(username, password, globals): +def manhole(settings, globals): """Starts a ssh listener with password authentication using the given username and password. Clients connecting to the ssh listener will find themselves in a colored python shell with @@ -75,6 +75,15 @@ def manhole(username, password, globals): Returns: twisted.internet.protocol.Factory: A factory to pass to ``listenTCP`` """ + username = settings.username + password = settings.password + priv_key = settings.priv_key + if priv_key is None: + priv_key = Key.fromString(PRIVATE_KEY) + pub_key = settings.pub_key + if pub_key is None: + pub_key = Key.fromString(PUBLIC_KEY) + if not isinstance(password, bytes): password = password.encode("ascii") @@ -86,8 +95,8 @@ def manhole(username, password, globals): ) factory = manhole_ssh.ConchFactory(portal.Portal(rlm, [checker])) - factory.publicKeys[b"ssh-rsa"] = Key.fromString(PUBLIC_KEY) - factory.privateKeys[b"ssh-rsa"] = Key.fromString(PRIVATE_KEY) + factory.privateKeys[b"ssh-rsa"] = priv_key + factory.publicKeys[b"ssh-rsa"] = pub_key return factory diff --git a/tests/config/test_server.py b/tests/config/test_server.py index 6f2b9e997d..b6f21294ba 100644 --- a/tests/config/test_server.py +++ b/tests/config/test_server.py @@ -35,7 +35,7 @@ class ServerConfigTestCase(unittest.TestCase): 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 + "che.org", "/data_dir_path", False, None, config_dir_path="CONFDIR" ) ) @@ -55,7 +55,7 @@ class ServerConfigTestCase(unittest.TestCase): 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 + "che.org", "/data_dir_path", True, None, config_dir_path="CONFDIR" ) ) @@ -89,7 +89,7 @@ class ServerConfigTestCase(unittest.TestCase): conf = yaml.safe_load( ServerConfig().generate_config_section( - "this.one.listens", "/data_dir_path", True, listeners + "this.one.listens", "/data_dir_path", True, listeners, "CONFDIR" ) ) @@ -123,7 +123,7 @@ class ServerConfigTestCase(unittest.TestCase): conf = yaml.safe_load( ServerConfig().generate_config_section( - "this.one.listens", "/data_dir_path", True, listeners + "this.one.listens", "/data_dir_path", True, listeners, "CONFDIR" ) )