This repository has been archived by the owner on Apr 26, 2024. It is now read-only.
-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Explicitly log when a homeserver does not have a trusted key server configured #6090
Merged
Merged
Changes from all commits
Commits
Show all changes
10 commits
Select commit
Hold shift + click to select a range
fb27576
Explicitly log when a homeserver does not have the 'trusted_key_serve…
neilisfragile ccd9b8f
add further warning for setting trusted_key_servers
neilisfragile 1759f8f
update sample config
neilisfragile 890bd6b
black
neilisfragile 13f6a87
improve clarity
neilisfragile 5871b6e
Apply suggestions from code review
neilisfragile 5be727f
clarity and refactor
neilisfragile bdc578f
Update sample config
richvdh e2de7bb
Review comments and warning formatting
richvdh 4ab9592
Rename 6090.doc to 6090.feature
richvdh File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Explicitly log when a homeserver does not have the 'trusted_key_servers' config field configured. |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -1061,6 +1061,10 @@ signing_key_path: "CONFDIR/SERVERNAME.signing.key" | |||
# This setting supercedes an older setting named `perspectives`. The old format | ||||
# is still supported for backwards-compatibility, but it is deprecated. | ||||
# | ||||
# 'trusted_key_servers' defaults to matrix.org, but using it will generate a | ||||
# warning on start-up. To suppress this warning, set | ||||
# 'suppress_key_server_warning' to true. | ||||
# | ||||
# Options for each entry in the list include: | ||||
# | ||||
# server_name: the name of the server. required. | ||||
|
@@ -1085,11 +1089,13 @@ signing_key_path: "CONFDIR/SERVERNAME.signing.key" | |||
# "ed25519:auto": "abcdefghijklmnopqrstuvwxyzabcdefghijklmopqr" | ||||
# - server_name: "my_other_trusted_server.example.com" | ||||
# | ||||
# The default configuration is: | ||||
# | ||||
#trusted_key_servers: | ||||
# - server_name: "matrix.org" | ||||
trusted_key_servers: | ||||
- server_name: "matrix.org" | ||||
|
||||
# Uncomment the following to disable the warning that is emitted when the | ||||
# trusted_key_servers include 'matrix.org'. See above. | ||||
# | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
|
||||
#suppress_key_server_warning: true | ||||
|
||||
# The signing keys to use when acting as a trusted key server. If not specified | ||||
# defaults to the server signing key. | ||||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -50,6 +50,33 @@ | |
If you are *sure* you want to do this, set 'accept_keys_insecurely' on the | ||
trusted_key_server configuration.""" | ||
|
||
TRUSTED_KEY_SERVER_NOT_CONFIGURED_WARN = """\ | ||
Synapse requires that a list of trusted key servers are specified in order to | ||
provide signing keys for other servers in the federation. | ||
|
||
This homeserver does not have a trusted key server configured in | ||
homeserver.yaml and will fall back to the default of 'matrix.org'. | ||
|
||
Trusted key servers should be long-lived and stable which makes matrix.org a | ||
good choice for many admins, but some admins may wish to choose another. To | ||
suppress this warning, the admin should set 'trusted_key_servers' in | ||
homeserver.yaml to their desired key server and 'suppress_key_server_warning' | ||
to 'true'. | ||
|
||
In a future release the software-defined default will be removed entirely and | ||
the trusted key server will be defined exclusively by the value of | ||
'trusted_key_servers'. | ||
--------------------------------------------------------------------------------""" | ||
|
||
TRUSTED_KEY_SERVER_CONFIGURED_AS_M_ORG_WARN = """\ | ||
This server is configured to use 'matrix.org' as its trusted key server via the | ||
'trusted_key_servers' config option. 'matrix.org' is a good choice for a key | ||
server since it is long-lived, stable and trusted. However, some admins may | ||
wish to use another server for this purpose. | ||
|
||
To suppress this warning and continue using 'matrix.org', admins should set | ||
'suppress_key_server_warning' to 'true' in homeserver.yaml. | ||
--------------------------------------------------------------------------------""" | ||
|
||
logger = logging.getLogger(__name__) | ||
|
||
|
@@ -85,6 +112,7 @@ def read_config(self, config, config_dir_path, **kwargs): | |
config.get("key_refresh_interval", "1d") | ||
) | ||
|
||
richvdh marked this conversation as resolved.
Show resolved
Hide resolved
|
||
suppress_key_server_warning = config.get("suppress_key_server_warning", False) | ||
key_server_signing_keys_path = config.get("key_server_signing_keys_path") | ||
if key_server_signing_keys_path: | ||
self.key_server_signing_keys = self.read_signing_keys( | ||
|
@@ -95,6 +123,7 @@ def read_config(self, config, config_dir_path, **kwargs): | |
|
||
# if neither trusted_key_servers nor perspectives are given, use the default. | ||
if "perspectives" not in config and "trusted_key_servers" not in config: | ||
logger.warn(TRUSTED_KEY_SERVER_NOT_CONFIGURED_WARN) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. just as a note for future reference: note that logger.warn is deprecated |
||
key_servers = [{"server_name": "matrix.org"}] | ||
else: | ||
key_servers = config.get("trusted_key_servers", []) | ||
|
@@ -108,6 +137,11 @@ def read_config(self, config, config_dir_path, **kwargs): | |
# merge the 'perspectives' config into the 'trusted_key_servers' config. | ||
key_servers.extend(_perspectives_to_key_servers(config)) | ||
|
||
if not suppress_key_server_warning and "matrix.org" in ( | ||
s["server_name"] for s in key_servers | ||
): | ||
logger.warning(TRUSTED_KEY_SERVER_CONFIGURED_AS_M_ORG_WARN) | ||
|
||
# list of TrustedKeyServer objects | ||
self.key_servers = list( | ||
_parse_key_servers(key_servers, self.federation_verify_certificates) | ||
|
@@ -190,6 +224,10 @@ def generate_config_section( | |
# This setting supercedes an older setting named `perspectives`. The old format | ||
# is still supported for backwards-compatibility, but it is deprecated. | ||
# | ||
# 'trusted_key_servers' defaults to matrix.org, but using it will generate a | ||
# warning on start-up. To suppress this warning, set | ||
# 'suppress_key_server_warning' to true. | ||
# | ||
# Options for each entry in the list include: | ||
# | ||
# server_name: the name of the server. required. | ||
|
@@ -214,11 +252,13 @@ def generate_config_section( | |
# "ed25519:auto": "abcdefghijklmnopqrstuvwxyzabcdefghijklmopqr" | ||
# - server_name: "my_other_trusted_server.example.com" | ||
# | ||
# The default configuration is: | ||
# | ||
#trusted_key_servers: | ||
# - server_name: "matrix.org" | ||
trusted_key_servers: | ||
- server_name: "matrix.org" | ||
|
||
# Uncomment the following to disable the warning that is emitted when the | ||
# trusted_key_servers include 'matrix.org'. See above. | ||
# | ||
#suppress_key_server_warning: true | ||
|
||
# The signing keys to use when acting as a trusted key server. If not specified | ||
# defaults to the server signing key. | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.