Add --no-secrets-in-config command line option

changelog.d/18092.feature

@@ -0,0 +1 @@
+Add the `--no-secrets-in-config` command line option.

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(

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,

synapse/config/captcha.py

@@ -29,15 +29,27 @@
 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
         ):
             raise ConfigError("recaptcha_private_key must be a string.")
         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
         ):

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

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,

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:

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:

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:

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

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])

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: