Improve startup checks for insecure notary configs (#5392)

It's not really a problem to trust notary responses signed by the old key so
long as we are also doing TLS validation.

This commit adds a check to the config parsing code at startup to check that
we do not have the insecure matrix.org key without tls validation, and refuses
to start without it.

This allows us to remove the rather alarming-looking warning which happens at
runtime.
This commit is contained in:
Richard van der Hoff 2019-06-10 10:33:00 +01:00 committed by GitHub
parent 7c455a86bc
commit 88d7182ada
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 24 additions and 11 deletions

1
changelog.d/5392.bugfix Normal file
View File

@ -0,0 +1 @@
Remove redundant warning about key server response validation.

View File

@ -41,6 +41,15 @@ validation or TLS certificate validation. This is likely to be very insecure. If
you are *sure* you want to do this, set 'accept_keys_insecurely' on the you are *sure* you want to do this, set 'accept_keys_insecurely' on the
keyserver configuration.""" keyserver configuration."""
RELYING_ON_MATRIX_KEY_ERROR = """\
Your server is configured to accept key server responses without TLS certificate
validation, and which are only signed by the old (possibly compromised)
matrix.org signing key 'ed25519:auto'. This likely isn't what you want to do,
and you should enable 'federation_verify_certificates' in your configuration.
If you are *sure* you want to do this, set 'accept_keys_insecurely' on the
trusted_key_server configuration."""
logger = logging.getLogger(__name__) logger = logging.getLogger(__name__)
@ -340,10 +349,20 @@ def _parse_key_servers(key_servers, federation_verify_certificates):
result.verify_keys[key_id] = verify_key result.verify_keys[key_id] = verify_key
if ( if (
not verify_keys not federation_verify_certificates and
and not server.get("accept_keys_insecurely") not server.get("accept_keys_insecurely")
and not federation_verify_certificates
): ):
raise ConfigError(INSECURE_NOTARY_ERROR) _assert_keyserver_has_verify_keys(result)
yield result yield result
def _assert_keyserver_has_verify_keys(trusted_key_server):
if not trusted_key_server.verify_keys:
raise ConfigError(INSECURE_NOTARY_ERROR)
# also check that they are not blindly checking the old matrix.org key
if trusted_key_server.server_name == "matrix.org" and any(
key_id == "ed25519:auto" for key_id in trusted_key_server.verify_keys
):
raise ConfigError(RELYING_ON_MATRIX_KEY_ERROR)

View File

@ -750,13 +750,6 @@ class PerspectivesKeyFetcher(BaseV2KeyFetcher):
verify_signed_json(response, perspective_name, perspective_keys[key_id]) verify_signed_json(response, perspective_name, perspective_keys[key_id])
verified = True verified = True
if perspective_name == "matrix.org" and key_id == "ed25519:auto":
logger.warning(
"Trusting trusted_key_server responses signed by the "
"compromised matrix.org signing key 'ed25519:auto'. "
"This is a placebo."
)
if not verified: if not verified:
raise KeyLookupError( raise KeyLookupError(
"Response not signed with a known key: signed with: %r, known keys: %r" "Response not signed with a known key: signed with: %r, known keys: %r"