Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
#
|
2023-11-21 15:29:58 -05:00
|
|
|
# This file is licensed under the Affero General Public License (AGPL) version 3.
|
|
|
|
#
|
2024-01-23 11:26:48 +00:00
|
|
|
# Copyright 2021 The Matrix.org Foundation C.I.C.
|
|
|
|
# Copyright 2016 OpenMarket Ltd
|
2023-11-21 15:29:58 -05:00
|
|
|
# Copyright (C) 2023 New Vector, Ltd
|
|
|
|
#
|
|
|
|
# This program is free software: you can redistribute it and/or modify
|
|
|
|
# it under the terms of the GNU Affero General Public License as
|
|
|
|
# published by the Free Software Foundation, either version 3 of the
|
|
|
|
# License, or (at your option) any later version.
|
|
|
|
#
|
|
|
|
# See the GNU Affero General Public License for more details:
|
|
|
|
# <https://www.gnu.org/licenses/agpl-3.0.html>.
|
|
|
|
#
|
|
|
|
# Originally licensed under the Apache License, Version 2.0:
|
|
|
|
# <http://www.apache.org/licenses/LICENSE-2.0>.
|
|
|
|
#
|
|
|
|
# [This file includes modifications made by New Vector Limited]
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
#
|
|
|
|
#
|
2024-10-07 10:46:51 +02:00
|
|
|
import tempfile
|
|
|
|
from typing import Callable
|
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](https://github.com/element-hq/synapse/pull/17690),
[#17717](https://github.com/element-hq/synapse/pull/17717),
[#17983](https://github.com/element-hq/synapse/pull/17983),
[#17984](https://github.com/element-hq/synapse/pull/17984),
[#18004](https://github.com/element-hq/synapse/pull/18004),
[#18090](https://github.com/element-hq/synapse/pull/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))
2025-02-25 17:26:01 +01:00
|
|
|
from unittest import mock
|
2024-10-07 10:46:51 +02:00
|
|
|
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
import yaml
|
2024-10-07 10:46:51 +02:00
|
|
|
from parameterized import parameterized
|
2018-07-09 16:09:20 +10:00
|
|
|
|
2021-04-06 13:48:22 +01:00
|
|
|
from synapse.config import ConfigError
|
2024-10-07 10:46:51 +02:00
|
|
|
from synapse.config._base import RootConfig
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
from synapse.config.homeserver import HomeServerConfig
|
2018-07-09 16:09:20 +10:00
|
|
|
|
2021-10-22 13:00:52 +03:00
|
|
|
from tests.config.utils import ConfigFileTestCase
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
|
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](https://github.com/element-hq/synapse/pull/17690),
[#17717](https://github.com/element-hq/synapse/pull/17717),
[#17983](https://github.com/element-hq/synapse/pull/17983),
[#17984](https://github.com/element-hq/synapse/pull/17984),
[#18004](https://github.com/element-hq/synapse/pull/18004),
[#18090](https://github.com/element-hq/synapse/pull/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))
2025-02-25 17:26:01 +01:00
|
|
|
try:
|
|
|
|
import authlib
|
|
|
|
except ImportError:
|
|
|
|
authlib = None
|
|
|
|
|
2024-10-07 10:46:51 +02:00
|
|
|
try:
|
|
|
|
import hiredis
|
|
|
|
except ImportError:
|
|
|
|
hiredis = None # type: ignore
|
|
|
|
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
|
2021-10-22 13:00:52 +03:00
|
|
|
class ConfigLoadingFileTestCase(ConfigFileTestCase):
|
2022-12-16 08:53:28 -05:00
|
|
|
def test_load_fails_if_server_name_missing(self) -> None:
|
2024-12-17 01:01:33 +01:00
|
|
|
self.generate_config_and_remove_lines_containing(["server_name"])
|
2021-04-06 13:48:22 +01:00
|
|
|
with self.assertRaises(ConfigError):
|
2021-10-22 13:00:52 +03:00
|
|
|
HomeServerConfig.load_config("", ["-c", self.config_file])
|
2021-04-06 13:48:22 +01:00
|
|
|
with self.assertRaises(ConfigError):
|
2021-10-22 13:00:52 +03:00
|
|
|
HomeServerConfig.load_or_generate_config("", ["-c", self.config_file])
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
|
2022-12-16 08:53:28 -05:00
|
|
|
def test_generates_and_loads_macaroon_secret_key(self) -> None:
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
self.generate_config()
|
|
|
|
|
2021-10-22 13:00:52 +03:00
|
|
|
with open(self.config_file) as f:
|
2019-03-22 10:20:17 +00:00
|
|
|
raw = yaml.safe_load(f)
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
self.assertIn("macaroon_secret_key", raw)
|
|
|
|
|
2021-10-22 13:00:52 +03:00
|
|
|
config = HomeServerConfig.load_config("", ["-c", self.config_file])
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
self.assertTrue(
|
2021-10-06 10:47:41 -04:00
|
|
|
hasattr(config.key, "macaroon_secret_key"),
|
2018-08-10 23:54:09 +10:00
|
|
|
"Want config to have attr macaroon_secret_key",
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
)
|
2021-09-23 12:03:01 -04:00
|
|
|
if len(config.key.macaroon_secret_key) < 5:
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
self.fail(
|
|
|
|
"Want macaroon secret key to be string of at least length 5,"
|
2021-09-23 12:03:01 -04:00
|
|
|
"was: %r" % (config.key.macaroon_secret_key,)
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
)
|
|
|
|
|
2021-11-23 10:21:19 -05:00
|
|
|
config2 = HomeServerConfig.load_or_generate_config("", ["-c", self.config_file])
|
|
|
|
assert config2 is not None
|
2016-06-09 18:50:38 +01:00
|
|
|
self.assertTrue(
|
2021-11-23 10:21:19 -05:00
|
|
|
hasattr(config2.key, "macaroon_secret_key"),
|
2018-08-10 23:54:09 +10:00
|
|
|
"Want config to have attr macaroon_secret_key",
|
2016-06-09 18:50:38 +01:00
|
|
|
)
|
2021-11-23 10:21:19 -05:00
|
|
|
if len(config2.key.macaroon_secret_key) < 5:
|
2016-06-09 18:50:38 +01:00
|
|
|
self.fail(
|
|
|
|
"Want macaroon secret key to be string of at least length 5,"
|
2021-11-23 10:21:19 -05:00
|
|
|
"was: %r" % (config2.key.macaroon_secret_key,)
|
2016-06-09 18:50:38 +01:00
|
|
|
)
|
|
|
|
|
2022-12-16 08:53:28 -05:00
|
|
|
def test_load_succeeds_if_macaroon_secret_key_missing(self) -> None:
|
2024-12-17 01:01:33 +01:00
|
|
|
self.generate_config_and_remove_lines_containing(["macaroon"])
|
2021-10-22 13:00:52 +03:00
|
|
|
config1 = HomeServerConfig.load_config("", ["-c", self.config_file])
|
|
|
|
config2 = HomeServerConfig.load_config("", ["-c", self.config_file])
|
|
|
|
config3 = HomeServerConfig.load_or_generate_config("", ["-c", self.config_file])
|
2021-11-23 10:21:19 -05:00
|
|
|
assert config1 is not None
|
|
|
|
assert config2 is not None
|
|
|
|
assert config3 is not None
|
2021-10-06 10:47:41 -04:00
|
|
|
self.assertEqual(
|
|
|
|
config1.key.macaroon_secret_key, config2.key.macaroon_secret_key
|
|
|
|
)
|
|
|
|
self.assertEqual(
|
|
|
|
config1.key.macaroon_secret_key, config3.key.macaroon_secret_key
|
|
|
|
)
|
Error if macaroon key is missing from config
Currently we store all access tokens in the DB, and fall back to that
check if we can't validate the macaroon, so our fallback works here, but
for guests, their macaroons don't get persisted, so we don't get to
find them in the database. Each restart, we generate a new ephemeral
key, so guests lose access after each server restart.
I tried to fix up the config stuff to be less insane, but gave up, so
instead I bolt on yet another piece of custom one-off insanity.
Also, add some basic tests for config generation and loading.
2016-02-05 01:58:23 +00:00
|
|
|
|
2022-12-16 08:53:28 -05:00
|
|
|
def test_disable_registration(self) -> None:
|
2016-02-22 16:01:29 +00:00
|
|
|
self.generate_config()
|
2018-08-10 23:54:09 +10:00
|
|
|
self.add_lines_to_config(
|
|
|
|
["enable_registration: true", "disable_registration: true"]
|
|
|
|
)
|
2016-02-22 16:01:29 +00:00
|
|
|
# Check that disable_registration clobbers enable_registration.
|
2021-10-22 13:00:52 +03:00
|
|
|
config = HomeServerConfig.load_config("", ["-c", self.config_file])
|
2021-10-04 07:18:54 -04:00
|
|
|
self.assertFalse(config.registration.enable_registration)
|
2016-02-22 16:01:29 +00:00
|
|
|
|
2021-11-23 10:21:19 -05:00
|
|
|
config2 = HomeServerConfig.load_or_generate_config("", ["-c", self.config_file])
|
|
|
|
assert config2 is not None
|
|
|
|
self.assertFalse(config2.registration.enable_registration)
|
2016-06-09 18:50:38 +01:00
|
|
|
|
2016-02-22 16:01:29 +00:00
|
|
|
# Check that either config value is clobbered by the command line.
|
2021-11-23 10:21:19 -05:00
|
|
|
config3 = HomeServerConfig.load_or_generate_config(
|
2021-10-22 13:00:52 +03:00
|
|
|
"", ["-c", self.config_file, "--enable-registration"]
|
2018-08-10 23:54:09 +10:00
|
|
|
)
|
2021-11-23 10:21:19 -05:00
|
|
|
assert config3 is not None
|
|
|
|
self.assertTrue(config3.registration.enable_registration)
|
2016-02-22 16:01:29 +00:00
|
|
|
|
2022-12-16 08:53:28 -05:00
|
|
|
def test_stats_enabled(self) -> None:
|
2024-12-17 01:01:33 +01:00
|
|
|
self.generate_config_and_remove_lines_containing(["enable_metrics"])
|
2019-09-07 01:45:51 +10:00
|
|
|
self.add_lines_to_config(["enable_metrics: true"])
|
|
|
|
|
|
|
|
# The default Metrics Flags are off by default.
|
2021-10-22 13:00:52 +03:00
|
|
|
config = HomeServerConfig.load_config("", ["-c", self.config_file])
|
2021-09-23 12:03:01 -04:00
|
|
|
self.assertFalse(config.metrics.metrics_flags.known_servers)
|
2021-11-18 10:56:32 -08:00
|
|
|
|
2022-12-16 08:53:28 -05:00
|
|
|
def test_depreciated_identity_server_flag_throws_error(self) -> None:
|
2021-11-18 10:56:32 -08:00
|
|
|
self.generate_config()
|
|
|
|
# Needed to ensure that actual key/value pair added below don't end up on a line with a comment
|
|
|
|
self.add_lines_to_config([" "])
|
|
|
|
# Check that presence of "trust_identity_server_for_password" throws config error
|
|
|
|
self.add_lines_to_config(["trust_identity_server_for_password_resets: true"])
|
|
|
|
with self.assertRaises(ConfigError):
|
|
|
|
HomeServerConfig.load_config("", ["-c", self.config_file])
|
2024-10-07 10:46:51 +02:00
|
|
|
|
|
|
|
@parameterized.expand(
|
|
|
|
[
|
|
|
|
"turn_shared_secret_path: /does/not/exist",
|
|
|
|
"registration_shared_secret_path: /does/not/exist",
|
2024-12-17 01:01:33 +01:00
|
|
|
"macaroon_secret_key_path: /does/not/exist",
|
2025-02-25 22:35:06 +01:00
|
|
|
"form_secret_path: /does/not/exist",
|
2025-02-26 16:55:10 +01:00
|
|
|
"worker_replication_secret_path: /does/not/exist",
|
2025-02-04 19:45:33 +01:00
|
|
|
"experimental_features:\n msc3861:\n client_secret_path: /does/not/exist",
|
|
|
|
"experimental_features:\n msc3861:\n admin_token_path: /does/not/exist",
|
2024-10-07 10:46:51 +02:00
|
|
|
*["redis:\n enabled: true\n password_path: /does/not/exist"]
|
|
|
|
* (hiredis is not None),
|
|
|
|
]
|
|
|
|
)
|
|
|
|
def test_secret_files_missing(self, config_str: str) -> None:
|
|
|
|
self.generate_config()
|
|
|
|
self.add_lines_to_config(["", config_str])
|
|
|
|
|
|
|
|
with self.assertRaises(ConfigError):
|
|
|
|
HomeServerConfig.load_config("", ["-c", self.config_file])
|
|
|
|
|
|
|
|
@parameterized.expand(
|
|
|
|
[
|
|
|
|
(
|
|
|
|
"turn_shared_secret_path: {}",
|
2024-12-17 01:01:33 +01:00
|
|
|
lambda c: c.voip.turn_shared_secret.encode("utf-8"),
|
2024-10-07 10:46:51 +02:00
|
|
|
),
|
|
|
|
(
|
|
|
|
"registration_shared_secret_path: {}",
|
2024-12-17 01:01:33 +01:00
|
|
|
lambda c: c.registration.registration_shared_secret.encode("utf-8"),
|
|
|
|
),
|
|
|
|
(
|
|
|
|
"macaroon_secret_key_path: {}",
|
|
|
|
lambda c: c.key.macaroon_secret_key,
|
2024-10-07 10:46:51 +02:00
|
|
|
),
|
2025-02-25 22:35:06 +01:00
|
|
|
(
|
|
|
|
"form_secret_path: {}",
|
|
|
|
lambda c: c.key.form_secret.encode("utf-8"),
|
|
|
|
),
|
2025-02-26 16:55:10 +01:00
|
|
|
(
|
|
|
|
"worker_replication_secret_path: {}",
|
|
|
|
lambda c: c.worker.worker_replication_secret.encode("utf-8"),
|
|
|
|
),
|
2025-02-04 19:45:33 +01:00
|
|
|
(
|
|
|
|
"experimental_features:\n msc3861:\n client_secret_path: {}",
|
|
|
|
lambda c: c.experimental.msc3861.client_secret().encode("utf-8"),
|
|
|
|
),
|
|
|
|
(
|
|
|
|
"experimental_features:\n msc3861:\n admin_token_path: {}",
|
|
|
|
lambda c: c.experimental.msc3861.admin_token().encode("utf-8"),
|
|
|
|
),
|
2024-10-07 10:46:51 +02:00
|
|
|
*[
|
|
|
|
(
|
|
|
|
"redis:\n enabled: true\n password_path: {}",
|
2024-12-17 01:01:33 +01:00
|
|
|
lambda c: c.redis.redis_password.encode("utf-8"),
|
2024-10-07 10:46:51 +02:00
|
|
|
)
|
|
|
|
]
|
|
|
|
* (hiredis is not None),
|
|
|
|
]
|
|
|
|
)
|
|
|
|
def test_secret_files_existing(
|
|
|
|
self, config_line: str, get_secret: Callable[[RootConfig], str]
|
|
|
|
) -> None:
|
2024-12-17 01:01:33 +01:00
|
|
|
self.generate_config_and_remove_lines_containing(
|
2025-02-25 22:35:06 +01:00
|
|
|
["form_secret", "macaroon_secret_key", "registration_shared_secret"]
|
2024-12-17 01:01:33 +01:00
|
|
|
)
|
2024-10-07 10:46:51 +02:00
|
|
|
with tempfile.NamedTemporaryFile(buffering=0) as secret_file:
|
|
|
|
secret_file.write(b"53C237")
|
|
|
|
|
|
|
|
self.add_lines_to_config(["", config_line.format(secret_file.name)])
|
|
|
|
config = HomeServerConfig.load_config("", ["-c", self.config_file])
|
|
|
|
|
2024-12-17 01:01:33 +01:00
|
|
|
self.assertEqual(get_secret(config), b"53C237")
|
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](https://github.com/element-hq/synapse/pull/17690),
[#17717](https://github.com/element-hq/synapse/pull/17717),
[#17983](https://github.com/element-hq/synapse/pull/17983),
[#17984](https://github.com/element-hq/synapse/pull/17984),
[#18004](https://github.com/element-hq/synapse/pull/18004),
[#18090](https://github.com/element-hq/synapse/pull/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))
2025-02-25 17:26:01 +01:00
|
|
|
|
|
|
|
@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])
|
|
|
|
|
|
|
|
def test_no_secrets_in_config_but_in_files(self) -> None:
|
|
|
|
with tempfile.NamedTemporaryFile(buffering=0) as secret_file:
|
|
|
|
secret_file.write(b"53C237")
|
|
|
|
|
|
|
|
self.generate_config_and_remove_lines_containing(
|
|
|
|
["form_secret", "macaroon_secret_key", "registration_shared_secret"]
|
|
|
|
)
|
|
|
|
self.add_lines_to_config(
|
|
|
|
[
|
|
|
|
"",
|
|
|
|
f"turn_shared_secret_path: {secret_file.name}",
|
|
|
|
f"registration_shared_secret_path: {secret_file.name}",
|
|
|
|
f"macaroon_secret_key_path: {secret_file.name}",
|
|
|
|
f"recaptcha_private_key_path: {secret_file.name}",
|
|
|
|
f"recaptcha_public_key_path: {secret_file.name}",
|
|
|
|
f"form_secret_path: {secret_file.name}",
|
|
|
|
f"worker_replication_secret_path: {secret_file.name}",
|
|
|
|
*[
|
|
|
|
"experimental_features:\n"
|
|
|
|
" msc3861:\n"
|
|
|
|
" enabled: true\n"
|
|
|
|
f" admin_token_path: {secret_file.name}\n"
|
|
|
|
f" client_secret_path: {secret_file.name}\n"
|
|
|
|
# f" jwk_path: {secret_file.name}"
|
|
|
|
]
|
|
|
|
* (authlib is not None),
|
|
|
|
*[f"redis:\n enabled: true\n password_path: {secret_file.name}"]
|
|
|
|
* (hiredis is not None),
|
|
|
|
]
|
|
|
|
)
|
|
|
|
HomeServerConfig.load_config(
|
|
|
|
"", ["-c", self.config_file, "--no-secrets-in-config"]
|
|
|
|
)
|