diff --git a/changelog.d/18092.feature b/changelog.d/18092.feature new file mode 100644 index 00000000000..26371cc810c --- /dev/null +++ b/changelog.d/18092.feature @@ -0,0 +1 @@ +Add the `--no-secrets-in-config` command line option. \ No newline at end of file diff --git a/synapse/config/_base.py b/synapse/config/_base.py index 912346a423d..132ba26af90 100644 --- a/synapse/config/_base.py +++ b/synapse/config/_base.py @@ -589,6 +589,14 @@ def add_arguments_to_parser(cls, config_parser: argparse.ArgumentParser) -> None " Defaults to the directory containing the last config file", ) + config_parser.add_argument( + "--no-secrets-in-config", + dest="secrets_in_config", + action="store_false", + default=True, + help="Reject config options that expect an in-line secret as value.", + ) + cls.invoke_all_static("add_arguments", config_parser) @classmethod @@ -626,7 +634,10 @@ def load_config_with_parser( config_dict = read_config_files(config_files) obj.parse_config_dict( - config_dict, config_dir_path=config_dir_path, data_dir_path=data_dir_path + config_dict, + config_dir_path=config_dir_path, + data_dir_path=data_dir_path, + allow_secrets_in_config=config_args.secrets_in_config, ) obj.invoke_all("read_arguments", config_args) @@ -653,6 +664,13 @@ def load_or_generate_config( help="Specify config file. Can be given multiple times and" " may specify directories containing *.yaml files.", ) + parser.add_argument( + "--no-secrets-in-config", + dest="secrets_in_config", + action="store_false", + default=True, + help="Reject config options that expect an in-line secret as value.", + ) # we nest the mutually-exclusive group inside another group so that the help # text shows them in their own group. @@ -821,14 +839,21 @@ def load_or_generate_config( return None obj.parse_config_dict( - config_dict, config_dir_path=config_dir_path, data_dir_path=data_dir_path + config_dict, + config_dir_path=config_dir_path, + data_dir_path=data_dir_path, + allow_secrets_in_config=config_args.secrets_in_config, ) obj.invoke_all("read_arguments", config_args) return obj def parse_config_dict( - self, config_dict: Dict[str, Any], config_dir_path: str, data_dir_path: str + self, + config_dict: Dict[str, Any], + config_dir_path: str, + data_dir_path: str, + allow_secrets_in_config: bool = True, ) -> None: """Read the information from the config dict into this Config object. @@ -846,6 +871,7 @@ def parse_config_dict( config_dict, config_dir_path=config_dir_path, data_dir_path=data_dir_path, + allow_secrets_in_config=allow_secrets_in_config, ) def generate_missing_files( diff --git a/synapse/config/_base.pyi b/synapse/config/_base.pyi index d9cb0da38bb..55b0e2cbf42 100644 --- a/synapse/config/_base.pyi +++ b/synapse/config/_base.pyi @@ -132,7 +132,11 @@ class RootConfig: @classmethod def invoke_all_static(cls, func_name: str, *args: Any, **kwargs: Any) -> None: ... def parse_config_dict( - self, config_dict: Dict[str, Any], config_dir_path: str, data_dir_path: str + self, + config_dict: Dict[str, Any], + config_dir_path: str, + data_dir_path: str, + allow_secrets_in_config: bool = ..., ) -> None: ... def generate_config( self, diff --git a/synapse/config/captcha.py b/synapse/config/captcha.py index 84897c09c54..57d67abbc3d 100644 --- a/synapse/config/captcha.py +++ b/synapse/config/captcha.py @@ -29,8 +29,15 @@ class CaptchaConfig(Config): section = "captcha" - def read_config(self, config: JsonDict, **kwargs: Any) -> None: + def read_config( + self, config: JsonDict, allow_secrets_in_config: bool, **kwargs: Any + ) -> None: recaptcha_private_key = config.get("recaptcha_private_key") + if recaptcha_private_key and not allow_secrets_in_config: + raise ConfigError( + "Config options that expect an in-line secret as value are disabled", + ("recaptcha_private_key",), + ) if recaptcha_private_key is not None and not isinstance( recaptcha_private_key, str ): @@ -38,6 +45,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.recaptcha_private_key = recaptcha_private_key recaptcha_public_key = config.get("recaptcha_public_key") + if recaptcha_public_key and not allow_secrets_in_config: + raise ConfigError( + "Config options that expect an in-line secret as value are disabled", + ("recaptcha_public_key",), + ) if recaptcha_public_key is not None and not isinstance( recaptcha_public_key, str ): diff --git a/synapse/config/experimental.py b/synapse/config/experimental.py index 3beaeb88693..210e9888c06 100644 --- a/synapse/config/experimental.py +++ b/synapse/config/experimental.py @@ -250,7 +250,9 @@ def admin_token(self) -> Optional[str]: ) return self._admin_token - def check_config_conflicts(self, root: RootConfig) -> None: + def check_config_conflicts( + self, root: RootConfig, allow_secrets_in_config: bool + ) -> None: """Checks for any configuration conflicts with other parts of Synapse. Raises: @@ -260,6 +262,24 @@ def check_config_conflicts(self, root: RootConfig) -> None: if not self.enabled: return + if self.client_secret() and not allow_secrets_in_config: + raise ConfigError( + "Config options that expect an in-line secret as value are disabled", + ("experimental", "msc3861", "client_secret"), + ) + + if self.jwk and not allow_secrets_in_config: + raise ConfigError( + "Config options that expect an in-line secret as value are disabled", + ("experimental", "msc3861", "jwk"), + ) + + if self.admin_token() and not allow_secrets_in_config: + raise ConfigError( + "Config options that expect an in-line secret as value are disabled", + ("experimental", "msc3861", "admin_token"), + ) + if ( root.auth.password_enabled_for_reauth or root.auth.password_enabled_for_login @@ -350,7 +370,9 @@ class ExperimentalConfig(Config): section = "experimental" - def read_config(self, config: JsonDict, **kwargs: Any) -> None: + def read_config( + self, config: JsonDict, allow_secrets_in_config: bool, **kwargs: Any + ) -> None: experimental = config.get("experimental_features") or {} # MSC3026 (busy presence state) @@ -494,7 +516,9 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: ) from exc # Check that none of the other config options conflict with MSC3861 when enabled - self.msc3861.check_config_conflicts(self.root) + self.msc3861.check_config_conflicts( + self.root, allow_secrets_in_config=allow_secrets_in_config + ) self.msc4028_push_encrypted_events = experimental.get( "msc4028_push_encrypted_events", False diff --git a/synapse/config/key.py b/synapse/config/key.py index 01aae09c135..6f99f39e817 100644 --- a/synapse/config/key.py +++ b/synapse/config/key.py @@ -112,7 +112,11 @@ class KeyConfig(Config): section = "key" def read_config( - self, config: JsonDict, config_dir_path: str, **kwargs: Any + self, + config: JsonDict, + config_dir_path: str, + allow_secrets_in_config: bool, + **kwargs: Any, ) -> None: # the signing key can be specified inline or in a separate file if "signing_key" in config: @@ -172,6 +176,11 @@ def read_config( ) macaroon_secret_key = config.get("macaroon_secret_key") + if macaroon_secret_key and not allow_secrets_in_config: + raise ConfigError( + "Config options that expect an in-line secret as value are disabled", + ("macaroon_secret_key",), + ) macaroon_secret_key_path = config.get("macaroon_secret_key_path") if macaroon_secret_key_path: if macaroon_secret_key: @@ -193,6 +202,11 @@ def read_config( # a secret which is used to calculate HMACs for form values, to stop # falsification of values self.form_secret = config.get("form_secret", None) + if self.form_secret and not allow_secrets_in_config: + raise ConfigError( + "Config options that expect an in-line secret as value are disabled", + ("form_secret",), + ) def generate_config_section( self, diff --git a/synapse/config/redis.py b/synapse/config/redis.py index 3f38fa11b0a..948c95eef79 100644 --- a/synapse/config/redis.py +++ b/synapse/config/redis.py @@ -34,7 +34,9 @@ class RedisConfig(Config): section = "redis" - def read_config(self, config: JsonDict, **kwargs: Any) -> None: + def read_config( + self, config: JsonDict, allow_secrets_in_config: bool, **kwargs: Any + ) -> None: redis_config = config.get("redis") or {} self.redis_enabled = redis_config.get("enabled", False) @@ -48,6 +50,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: self.redis_path = redis_config.get("path", None) self.redis_dbid = redis_config.get("dbid", None) self.redis_password = redis_config.get("password") + if self.redis_password and not allow_secrets_in_config: + raise ConfigError( + "Config options that expect an in-line secret as value are disabled", + ("redis", "password"), + ) redis_password_path = redis_config.get("password_path") if redis_password_path: if self.redis_password: diff --git a/synapse/config/registration.py b/synapse/config/registration.py index c7f3e6d35ef..3cf70316560 100644 --- a/synapse/config/registration.py +++ b/synapse/config/registration.py @@ -43,7 +43,9 @@ class RegistrationConfig(Config): section = "registration" - def read_config(self, config: JsonDict, **kwargs: Any) -> None: + def read_config( + self, config: JsonDict, allow_secrets_in_config: bool, **kwargs: Any + ) -> None: self.enable_registration = strtobool( str(config.get("enable_registration", False)) ) @@ -68,6 +70,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # read the shared secret, either inline or from an external file self.registration_shared_secret = config.get("registration_shared_secret") + if self.registration_shared_secret and not allow_secrets_in_config: + raise ConfigError( + "Config options that expect an in-line secret as value are disabled", + ("registration_shared_secret",), + ) registration_shared_secret_path = config.get("registration_shared_secret_path") if registration_shared_secret_path: if self.registration_shared_secret: diff --git a/synapse/config/voip.py b/synapse/config/voip.py index 8614a41dd4a..f33602d975d 100644 --- a/synapse/config/voip.py +++ b/synapse/config/voip.py @@ -34,9 +34,16 @@ class VoipConfig(Config): section = "voip" - def read_config(self, config: JsonDict, **kwargs: Any) -> None: + def read_config( + self, config: JsonDict, allow_secrets_in_config: bool, **kwargs: Any + ) -> None: self.turn_uris = config.get("turn_uris", []) self.turn_shared_secret = config.get("turn_shared_secret") + if self.turn_shared_secret and not allow_secrets_in_config: + raise ConfigError( + "Config options that expect an in-line secret as value are disabled", + ("turn_shared_secret",), + ) turn_shared_secret_path = config.get("turn_shared_secret_path") if turn_shared_secret_path: if self.turn_shared_secret: diff --git a/synapse/config/workers.py b/synapse/config/workers.py index ab896be3077..632cef46ce7 100644 --- a/synapse/config/workers.py +++ b/synapse/config/workers.py @@ -218,7 +218,9 @@ class WorkerConfig(Config): section = "worker" - def read_config(self, config: JsonDict, **kwargs: Any) -> None: + def read_config( + self, config: JsonDict, allow_secrets_in_config: bool, **kwargs: Any + ) -> None: self.worker_app = config.get("worker_app") # Canonicalise worker_app so that master always has None @@ -243,6 +245,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None: # The shared secret used for authentication when connecting to the main synapse. self.worker_replication_secret = config.get("worker_replication_secret", None) + if self.worker_replication_secret and not allow_secrets_in_config: + raise ConfigError( + "Config options that expect an in-line secret as value are disabled", + ("worker_replication_secret",), + ) self.worker_name = config.get("worker_name", self.worker_app) self.instance_name = self.worker_name or MAIN_PROCESS_INSTANCE_NAME diff --git a/tests/config/test_load.py b/tests/config/test_load.py index 220ca23aa73..d7fa4f276a8 100644 --- a/tests/config/test_load.py +++ b/tests/config/test_load.py @@ -21,6 +21,7 @@ # import tempfile from typing import Callable +from unittest import mock import yaml from parameterized import parameterized @@ -31,6 +32,11 @@ from tests.config.utils import ConfigFileTestCase +try: + import authlib +except ImportError: + authlib = None + try: import hiredis except ImportError: @@ -189,3 +195,67 @@ def test_secret_files_existing( config = HomeServerConfig.load_config("", ["-c", self.config_file]) self.assertEqual(get_secret(config), b"53C237") + + @parameterized.expand( + [ + "turn_shared_secret: 53C237", + "registration_shared_secret: 53C237", + "macaroon_secret_key: 53C237", + "recaptcha_private_key: 53C237", + "recaptcha_public_key: ¬53C237", + "form_secret: 53C237", + "worker_replication_secret: 53C237", + *[ + "experimental_features:\n" + " msc3861:\n" + " enabled: true\n" + " client_secret: 53C237" + ] + * (authlib is not None), + *[ + "experimental_features:\n" + " msc3861:\n" + " enabled: true\n" + " client_auth_method: private_key_jwt\n" + ' jwk: {{"mock": "mock"}}' + ] + * (authlib is not None), + *[ + "experimental_features:\n" + " msc3861:\n" + " enabled: true\n" + " admin_token: 53C237\n" + " client_secret_path: {secret_file}" + ] + * (authlib is not None), + *["redis:\n enabled: true\n password: 53C237"] * (hiredis is not None), + ] + ) + def test_no_secrets_in_config(self, config_line: str) -> None: + if authlib is not None: + patcher = mock.patch("authlib.jose.rfc7517.JsonWebKey.import_key") + self.addCleanup(patcher.stop) + patcher.start() + + with tempfile.NamedTemporaryFile(buffering=0) as secret_file: + # Only used for less mocking with admin_token + secret_file.write(b"53C237") + + self.generate_config_and_remove_lines_containing( + ["form_secret", "macaroon_secret_key", "registration_shared_secret"] + ) + # Check strict mode with no offenders. + HomeServerConfig.load_config( + "", ["-c", self.config_file, "--no-secrets-in-config"] + ) + self.add_lines_to_config( + ["", config_line.format(secret_file=secret_file.name)] + ) + # Check strict mode with a single offender. + with self.assertRaises(ConfigError): + HomeServerConfig.load_config( + "", ["-c", self.config_file, "--no-secrets-in-config"] + ) + + # Check lenient mode with a single offender. + HomeServerConfig.load_config("", ["-c", self.config_file]) diff --git a/tests/config/test_workers.py b/tests/config/test_workers.py index 64c0285d010..3a21975b89f 100644 --- a/tests/config/test_workers.py +++ b/tests/config/test_workers.py @@ -47,7 +47,7 @@ def _make_worker_config( "worker_app": worker_app, **extras, } - worker_config.read_config(worker_config_dict) + worker_config.read_config(worker_config_dict, allow_secrets_in_config=True) return worker_config def test_old_configs_master(self) -> None: