Skip to content

Commit 2159b38

Browse files
authored
Add --no-secrets-in-config command line option (#18092)
Adds the `--no-secrets-in-config` command line option that makes Synapse reject all configurations containing keys with in-line secret values. Currently this rejects - `turn_shared_secret` - `registration_shared_secret` - `macaroon_secret_key` - `recaptcha_private_key` - `recaptcha_public_key` - `experimental_features.msc3861.client_secret` - `experimental_features.msc3861.jwk` - `experimental_features.msc3861.admin_token` - `form_secret` - `redis.password` - `worker_replication_secret` > [!TIP] > Hey, you! Yes, you! 😊 If you think this list is missing an item, please leave a comment below. Thanks :) This PR complements my other PRs[^1] that add the corresponding `_path` variants for this class of config options. It enables admins to enforce a policy of no secrets in configuration files and guards against accident and malice. Because I consider the flag `--no-secrets-in-config` to be security-relevant, I did not add a corresponding `--secrets-in-config` flag; this way, if Synapse command line options are appended at various places, there is no way to weaken the once-set setting with a succeeding flag. [^1]: [#17690](#17690), [#17717](#17717), [#17983](#17983), [#17984](#17984), [#18004](#18004), [#18090](#18090) ### Pull Request Checklist <!-- Please read https://element-hq.github.io/synapse/latest/development/contributing_guide.html before submitting your pull request --> * [x] Pull request is based on the develop branch * [x] Pull request includes a [changelog file](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#changelog). The entry should: - Be a short description of your change which makes sense to users. "Fixed a bug that prevented receiving messages from other servers." instead of "Moved X method from `EventStore` to `EventWorkerStore`.". - Use markdown where necessary, mostly for `code blocks`. - End with either a period (.) or an exclamation mark (!). - Start with a capital letter. - Feel free to credit yourself, by adding a sentence "Contributed by @github_username." or "Contributed by [Your Name]." to the end of the entry. * [x] [Code style](https://element-hq.github.io/synapse/latest/code_style.html) is correct (run the [linters](https://element-hq.github.io/synapse/latest/development/contributing_guide.html#run-the-linters))
1 parent 5121f92 commit 2159b38

12 files changed

Lines changed: 227 additions & 14 deletions

File tree

changelog.d/18092.feature

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
Add the `--no-secrets-in-config` command line option.

synapse/config/_base.py

Lines changed: 29 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -589,6 +589,14 @@ def add_arguments_to_parser(cls, config_parser: argparse.ArgumentParser) -> None
589589
" Defaults to the directory containing the last config file",
590590
)
591591

592+
config_parser.add_argument(
593+
"--no-secrets-in-config",
594+
dest="secrets_in_config",
595+
action="store_false",
596+
default=True,
597+
help="Reject config options that expect an in-line secret as value.",
598+
)
599+
592600
cls.invoke_all_static("add_arguments", config_parser)
593601

594602
@classmethod
@@ -626,7 +634,10 @@ def load_config_with_parser(
626634

627635
config_dict = read_config_files(config_files)
628636
obj.parse_config_dict(
629-
config_dict, config_dir_path=config_dir_path, data_dir_path=data_dir_path
637+
config_dict,
638+
config_dir_path=config_dir_path,
639+
data_dir_path=data_dir_path,
640+
allow_secrets_in_config=config_args.secrets_in_config,
630641
)
631642

632643
obj.invoke_all("read_arguments", config_args)
@@ -653,6 +664,13 @@ def load_or_generate_config(
653664
help="Specify config file. Can be given multiple times and"
654665
" may specify directories containing *.yaml files.",
655666
)
667+
parser.add_argument(
668+
"--no-secrets-in-config",
669+
dest="secrets_in_config",
670+
action="store_false",
671+
default=True,
672+
help="Reject config options that expect an in-line secret as value.",
673+
)
656674

657675
# we nest the mutually-exclusive group inside another group so that the help
658676
# text shows them in their own group.
@@ -821,14 +839,21 @@ def load_or_generate_config(
821839
return None
822840

823841
obj.parse_config_dict(
824-
config_dict, config_dir_path=config_dir_path, data_dir_path=data_dir_path
842+
config_dict,
843+
config_dir_path=config_dir_path,
844+
data_dir_path=data_dir_path,
845+
allow_secrets_in_config=config_args.secrets_in_config,
825846
)
826847
obj.invoke_all("read_arguments", config_args)
827848

828849
return obj
829850

830851
def parse_config_dict(
831-
self, config_dict: Dict[str, Any], config_dir_path: str, data_dir_path: str
852+
self,
853+
config_dict: Dict[str, Any],
854+
config_dir_path: str,
855+
data_dir_path: str,
856+
allow_secrets_in_config: bool = True,
832857
) -> None:
833858
"""Read the information from the config dict into this Config object.
834859
@@ -846,6 +871,7 @@ def parse_config_dict(
846871
config_dict,
847872
config_dir_path=config_dir_path,
848873
data_dir_path=data_dir_path,
874+
allow_secrets_in_config=allow_secrets_in_config,
849875
)
850876

851877
def generate_missing_files(

synapse/config/_base.pyi

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,11 @@ class RootConfig:
132132
@classmethod
133133
def invoke_all_static(cls, func_name: str, *args: Any, **kwargs: Any) -> None: ...
134134
def parse_config_dict(
135-
self, config_dict: Dict[str, Any], config_dir_path: str, data_dir_path: str
135+
self,
136+
config_dict: Dict[str, Any],
137+
config_dir_path: str,
138+
data_dir_path: str,
139+
allow_secrets_in_config: bool = ...,
136140
) -> None: ...
137141
def generate_config(
138142
self,

synapse/config/captcha.py

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -29,15 +29,27 @@
2929
class CaptchaConfig(Config):
3030
section = "captcha"
3131

32-
def read_config(self, config: JsonDict, **kwargs: Any) -> None:
32+
def read_config(
33+
self, config: JsonDict, allow_secrets_in_config: bool, **kwargs: Any
34+
) -> None:
3335
recaptcha_private_key = config.get("recaptcha_private_key")
36+
if recaptcha_private_key and not allow_secrets_in_config:
37+
raise ConfigError(
38+
"Config options that expect an in-line secret as value are disabled",
39+
("recaptcha_private_key",),
40+
)
3441
if recaptcha_private_key is not None and not isinstance(
3542
recaptcha_private_key, str
3643
):
3744
raise ConfigError("recaptcha_private_key must be a string.")
3845
self.recaptcha_private_key = recaptcha_private_key
3946

4047
recaptcha_public_key = config.get("recaptcha_public_key")
48+
if recaptcha_public_key and not allow_secrets_in_config:
49+
raise ConfigError(
50+
"Config options that expect an in-line secret as value are disabled",
51+
("recaptcha_public_key",),
52+
)
4153
if recaptcha_public_key is not None and not isinstance(
4254
recaptcha_public_key, str
4355
):

synapse/config/experimental.py

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -250,7 +250,9 @@ def admin_token(self) -> Optional[str]:
250250
)
251251
return self._admin_token
252252

253-
def check_config_conflicts(self, root: RootConfig) -> None:
253+
def check_config_conflicts(
254+
self, root: RootConfig, allow_secrets_in_config: bool
255+
) -> None:
254256
"""Checks for any configuration conflicts with other parts of Synapse.
255257
256258
Raises:
@@ -260,6 +262,24 @@ def check_config_conflicts(self, root: RootConfig) -> None:
260262
if not self.enabled:
261263
return
262264

265+
if self._client_secret and not allow_secrets_in_config:
266+
raise ConfigError(
267+
"Config options that expect an in-line secret as value are disabled",
268+
("experimental", "msc3861", "client_secret"),
269+
)
270+
271+
if self.jwk and not allow_secrets_in_config:
272+
raise ConfigError(
273+
"Config options that expect an in-line secret as value are disabled",
274+
("experimental", "msc3861", "jwk"),
275+
)
276+
277+
if self._admin_token and not allow_secrets_in_config:
278+
raise ConfigError(
279+
"Config options that expect an in-line secret as value are disabled",
280+
("experimental", "msc3861", "admin_token"),
281+
)
282+
263283
if (
264284
root.auth.password_enabled_for_reauth
265285
or root.auth.password_enabled_for_login
@@ -350,7 +370,9 @@ class ExperimentalConfig(Config):
350370

351371
section = "experimental"
352372

353-
def read_config(self, config: JsonDict, **kwargs: Any) -> None:
373+
def read_config(
374+
self, config: JsonDict, allow_secrets_in_config: bool, **kwargs: Any
375+
) -> None:
354376
experimental = config.get("experimental_features") or {}
355377

356378
# MSC3026 (busy presence state)
@@ -494,7 +516,9 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
494516
) from exc
495517

496518
# Check that none of the other config options conflict with MSC3861 when enabled
497-
self.msc3861.check_config_conflicts(self.root)
519+
self.msc3861.check_config_conflicts(
520+
self.root, allow_secrets_in_config=allow_secrets_in_config
521+
)
498522

499523
self.msc4028_push_encrypted_events = experimental.get(
500524
"msc4028_push_encrypted_events", False

synapse/config/key.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -112,7 +112,11 @@ class KeyConfig(Config):
112112
section = "key"
113113

114114
def read_config(
115-
self, config: JsonDict, config_dir_path: str, **kwargs: Any
115+
self,
116+
config: JsonDict,
117+
config_dir_path: str,
118+
allow_secrets_in_config: bool,
119+
**kwargs: Any,
116120
) -> None:
117121
# the signing key can be specified inline or in a separate file
118122
if "signing_key" in config:
@@ -172,6 +176,11 @@ def read_config(
172176
)
173177

174178
macaroon_secret_key = config.get("macaroon_secret_key")
179+
if macaroon_secret_key and not allow_secrets_in_config:
180+
raise ConfigError(
181+
"Config options that expect an in-line secret as value are disabled",
182+
("macaroon_secret_key",),
183+
)
175184
macaroon_secret_key_path = config.get("macaroon_secret_key_path")
176185
if macaroon_secret_key_path:
177186
if macaroon_secret_key:
@@ -193,6 +202,11 @@ def read_config(
193202
# a secret which is used to calculate HMACs for form values, to stop
194203
# falsification of values
195204
self.form_secret = config.get("form_secret", None)
205+
if self.form_secret and not allow_secrets_in_config:
206+
raise ConfigError(
207+
"Config options that expect an in-line secret as value are disabled",
208+
("form_secret",),
209+
)
196210

197211
def generate_config_section(
198212
self,

synapse/config/redis.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@
3434
class RedisConfig(Config):
3535
section = "redis"
3636

37-
def read_config(self, config: JsonDict, **kwargs: Any) -> None:
37+
def read_config(
38+
self, config: JsonDict, allow_secrets_in_config: bool, **kwargs: Any
39+
) -> None:
3840
redis_config = config.get("redis") or {}
3941
self.redis_enabled = redis_config.get("enabled", False)
4042

@@ -48,6 +50,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
4850
self.redis_path = redis_config.get("path", None)
4951
self.redis_dbid = redis_config.get("dbid", None)
5052
self.redis_password = redis_config.get("password")
53+
if self.redis_password and not allow_secrets_in_config:
54+
raise ConfigError(
55+
"Config options that expect an in-line secret as value are disabled",
56+
("redis", "password"),
57+
)
5158
redis_password_path = redis_config.get("password_path")
5259
if redis_password_path:
5360
if self.redis_password:

synapse/config/registration.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,9 @@
4343
class RegistrationConfig(Config):
4444
section = "registration"
4545

46-
def read_config(self, config: JsonDict, **kwargs: Any) -> None:
46+
def read_config(
47+
self, config: JsonDict, allow_secrets_in_config: bool, **kwargs: Any
48+
) -> None:
4749
self.enable_registration = strtobool(
4850
str(config.get("enable_registration", False))
4951
)
@@ -68,6 +70,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
6870

6971
# read the shared secret, either inline or from an external file
7072
self.registration_shared_secret = config.get("registration_shared_secret")
73+
if self.registration_shared_secret and not allow_secrets_in_config:
74+
raise ConfigError(
75+
"Config options that expect an in-line secret as value are disabled",
76+
("registration_shared_secret",),
77+
)
7178
registration_shared_secret_path = config.get("registration_shared_secret_path")
7279
if registration_shared_secret_path:
7380
if self.registration_shared_secret:

synapse/config/voip.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,9 +34,16 @@
3434
class VoipConfig(Config):
3535
section = "voip"
3636

37-
def read_config(self, config: JsonDict, **kwargs: Any) -> None:
37+
def read_config(
38+
self, config: JsonDict, allow_secrets_in_config: bool, **kwargs: Any
39+
) -> None:
3840
self.turn_uris = config.get("turn_uris", [])
3941
self.turn_shared_secret = config.get("turn_shared_secret")
42+
if self.turn_shared_secret and not allow_secrets_in_config:
43+
raise ConfigError(
44+
"Config options that expect an in-line secret as value are disabled",
45+
("turn_shared_secret",),
46+
)
4047
turn_shared_secret_path = config.get("turn_shared_secret_path")
4148
if turn_shared_secret_path:
4249
if self.turn_shared_secret:

synapse/config/workers.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,9 @@ class WorkerConfig(Config):
218218

219219
section = "worker"
220220

221-
def read_config(self, config: JsonDict, **kwargs: Any) -> None:
221+
def read_config(
222+
self, config: JsonDict, allow_secrets_in_config: bool, **kwargs: Any
223+
) -> None:
222224
self.worker_app = config.get("worker_app")
223225

224226
# Canonicalise worker_app so that master always has None
@@ -243,6 +245,11 @@ def read_config(self, config: JsonDict, **kwargs: Any) -> None:
243245

244246
# The shared secret used for authentication when connecting to the main synapse.
245247
self.worker_replication_secret = config.get("worker_replication_secret", None)
248+
if self.worker_replication_secret and not allow_secrets_in_config:
249+
raise ConfigError(
250+
"Config options that expect an in-line secret as value are disabled",
251+
("worker_replication_secret",),
252+
)
246253

247254
self.worker_name = config.get("worker_name", self.worker_app)
248255
self.instance_name = self.worker_name or MAIN_PROCESS_INSTANCE_NAME

0 commit comments

Comments
 (0)