From e0bb2681340752f2716f1f386139ca37c53f26cd Mon Sep 17 00:00:00 2001 From: David Robertson Date: Tue, 29 Mar 2022 22:37:50 +0100 Subject: [PATCH] Fix typechecker problems exposed by signedjson 1.1.2 (#12326) --- changelog.d/12326.misc | 1 + mypy.ini | 3 +++ synapse/config/key.py | 13 ++++++++----- synapse/crypto/keyring.py | 2 +- synapse/events/builder.py | 2 +- synapse/rest/key/v2/local_key_resource.py | 12 ++++++------ synapse/rest/key/v2/remote_key_resource.py | 8 ++++---- tests/crypto/test_event_signing.py | 6 +++--- tests/rest/key/v2/test_remote_key_resource.py | 16 +++++++++++----- 9 files changed, 38 insertions(+), 25 deletions(-) create mode 100644 changelog.d/12326.misc diff --git a/changelog.d/12326.misc b/changelog.d/12326.misc new file mode 100644 index 000000000..2d2a00e57 --- /dev/null +++ b/changelog.d/12326.misc @@ -0,0 +1 @@ +Fix typechecker problems exposed by signedjson 1.1.2. diff --git a/mypy.ini b/mypy.ini index 4781ce56f..cfe7e77c2 100644 --- a/mypy.ini +++ b/mypy.ini @@ -273,6 +273,9 @@ ignore_missing_imports = True [mypy-ijson.*] ignore_missing_imports = True +[mypy-importlib_metadata.*] +ignore_missing_imports = True + [mypy-jaeger_client.*] ignore_missing_imports = True diff --git a/synapse/config/key.py b/synapse/config/key.py index ee83c6c06..f5377e7d9 100644 --- a/synapse/config/key.py +++ b/synapse/config/key.py @@ -16,7 +16,7 @@ import hashlib import logging import os -from typing import Any, Dict, Iterator, List, Optional +from typing import TYPE_CHECKING, Any, Dict, Iterator, List, Optional import attr import jsonschema @@ -38,6 +38,9 @@ from synapse.util.stringutils import random_string, random_string_with_symbols from ._base import Config, ConfigError +if TYPE_CHECKING: + from signedjson.key import VerifyKeyWithExpiry + INSECURE_NOTARY_ERROR = """\ Your server is configured to accept key server responses without signature validation or TLS certificate validation. This is likely to be very insecure. If @@ -300,7 +303,7 @@ class KeyConfig(Config): def read_old_signing_keys( self, old_signing_keys: Optional[JsonDict] - ) -> Dict[str, VerifyKey]: + ) -> Dict[str, "VerifyKeyWithExpiry"]: if old_signing_keys is None: return {} keys = {} @@ -308,8 +311,8 @@ class KeyConfig(Config): if is_signing_algorithm_supported(key_id): key_base64 = key_data["key"] key_bytes = decode_base64(key_base64) - verify_key = decode_verify_key_bytes(key_id, key_bytes) - verify_key.expired_ts = key_data["expired_ts"] + verify_key: "VerifyKeyWithExpiry" = decode_verify_key_bytes(key_id, key_bytes) # type: ignore[assignment] + verify_key.expired = key_data["expired_ts"] keys[key_id] = verify_key else: raise ConfigError( @@ -422,7 +425,7 @@ def _parse_key_servers( server_name = server["server_name"] result = TrustedKeyServer(server_name=server_name) - verify_keys = server.get("verify_keys") + verify_keys: Optional[Dict[str, str]] = server.get("verify_keys") if verify_keys is not None: result.verify_keys = {} for key_id, key_base64 in verify_keys.items(): diff --git a/synapse/crypto/keyring.py b/synapse/crypto/keyring.py index 6cf384f6a..c88afb298 100644 --- a/synapse/crypto/keyring.py +++ b/synapse/crypto/keyring.py @@ -176,7 +176,7 @@ class Keyring: self._local_verify_keys: Dict[str, FetchKeyResult] = {} for key_id, key in hs.config.key.old_signing_keys.items(): self._local_verify_keys[key_id] = FetchKeyResult( - verify_key=key, valid_until_ts=key.expired_ts + verify_key=key, valid_until_ts=key.expired ) vk = get_verify_key(hs.signing_key) diff --git a/synapse/events/builder.py b/synapse/events/builder.py index 1ea1bb7d3..98c203ada 100644 --- a/synapse/events/builder.py +++ b/synapse/events/builder.py @@ -15,7 +15,7 @@ import logging from typing import TYPE_CHECKING, Any, Dict, List, Optional, Tuple, Union import attr -from nacl.signing import SigningKey +from signedjson.types import SigningKey from synapse.api.constants import MAX_DEPTH from synapse.api.room_versions import ( diff --git a/synapse/rest/key/v2/local_key_resource.py b/synapse/rest/key/v2/local_key_resource.py index b9bfbea21..0c9f042c8 100644 --- a/synapse/rest/key/v2/local_key_resource.py +++ b/synapse/rest/key/v2/local_key_resource.py @@ -76,17 +76,17 @@ class LocalKey(Resource): def response_json_object(self) -> JsonDict: verify_keys = {} - for key in self.config.key.signing_key: - verify_key_bytes = key.verify_key.encode() - key_id = "%s:%s" % (key.alg, key.version) + for signing_key in self.config.key.signing_key: + verify_key_bytes = signing_key.verify_key.encode() + key_id = "%s:%s" % (signing_key.alg, signing_key.version) verify_keys[key_id] = {"key": encode_base64(verify_key_bytes)} old_verify_keys = {} - for key_id, key in self.config.key.old_signing_keys.items(): - verify_key_bytes = key.encode() + for key_id, old_signing_key in self.config.key.old_signing_keys.items(): + verify_key_bytes = old_signing_key.encode() old_verify_keys[key_id] = { "key": encode_base64(verify_key_bytes), - "expired_ts": key.expired_ts, + "expired_ts": old_signing_key.expired, } json_object = { diff --git a/synapse/rest/key/v2/remote_key_resource.py b/synapse/rest/key/v2/remote_key_resource.py index 3525d6ae5..f59715758 100644 --- a/synapse/rest/key/v2/remote_key_resource.py +++ b/synapse/rest/key/v2/remote_key_resource.py @@ -13,7 +13,7 @@ # limitations under the License. import logging -from typing import TYPE_CHECKING, Dict +from typing import TYPE_CHECKING, Dict, Set from signedjson.sign import sign_json @@ -149,7 +149,7 @@ class RemoteKey(DirectServeJsonResource): cached = await self.store.get_server_keys_json(store_queries) - json_results = set() + json_results: Set[bytes] = set() time_now_ms = self.clock.time_msec() @@ -234,8 +234,8 @@ class RemoteKey(DirectServeJsonResource): await self.query_keys(request, query, query_remote_on_cache_miss=False) else: signed_keys = [] - for key_json in json_results: - key_json = json_decoder.decode(key_json.decode("utf-8")) + for key_json_raw in json_results: + key_json = json_decoder.decode(key_json_raw.decode("utf-8")) for signing_key in self.config.key.key_server_signing_keys: key_json = sign_json( key_json, self.config.server.server_name, signing_key diff --git a/tests/crypto/test_event_signing.py b/tests/crypto/test_event_signing.py index 694020fbe..06e0545a4 100644 --- a/tests/crypto/test_event_signing.py +++ b/tests/crypto/test_event_signing.py @@ -28,8 +28,8 @@ from tests import unittest SIGNING_KEY_SEED = decode_base64("YJDBA9Xnr2sVqXD9Vj7XVUnmFZcZrlw8Md7kMW+3XA1") KEY_ALG = "ed25519" -KEY_VER = 1 -KEY_NAME = "%s:%d" % (KEY_ALG, KEY_VER) +KEY_VER = "1" +KEY_NAME = "%s:%s" % (KEY_ALG, KEY_VER) HOSTNAME = "domain" @@ -39,7 +39,7 @@ class EventSigningTestCase(unittest.TestCase): # NB: `signedjson` expects `nacl.signing.SigningKey` instances which have been # monkeypatched to include new `alg` and `version` attributes. This is captured # by the `signedjson.types.SigningKey` protocol. - self.signing_key: signedjson.types.SigningKey = nacl.signing.SigningKey( + self.signing_key: signedjson.types.SigningKey = nacl.signing.SigningKey( # type: ignore[assignment] SIGNING_KEY_SEED ) self.signing_key.alg = KEY_ALG diff --git a/tests/rest/key/v2/test_remote_key_resource.py b/tests/rest/key/v2/test_remote_key_resource.py index 978c252f8..ac0ac06b7 100644 --- a/tests/rest/key/v2/test_remote_key_resource.py +++ b/tests/rest/key/v2/test_remote_key_resource.py @@ -76,7 +76,7 @@ class BaseRemoteKeyResourceTestCase(unittest.HomeserverTestCase): "verify_keys": { key_id: { "key": signedjson.key.encode_verify_key_base64( - signing_key.verify_key + signedjson.key.get_verify_key(signing_key) ) } }, @@ -175,7 +175,7 @@ class EndToEndPerspectivesTests(BaseRemoteKeyResourceTestCase): % ( self.hs_signing_key.version, ): signedjson.key.encode_verify_key_base64( - self.hs_signing_key.verify_key + signedjson.key.get_verify_key(self.hs_signing_key) ) }, } @@ -229,7 +229,9 @@ class EndToEndPerspectivesTests(BaseRemoteKeyResourceTestCase): assert isinstance(keyres, FetchKeyResult) self.assertEqual( signedjson.key.encode_verify_key_base64(keyres.verify_key), - signedjson.key.encode_verify_key_base64(testkey.verify_key), + signedjson.key.encode_verify_key_base64( + signedjson.key.get_verify_key(testkey) + ), ) def test_get_notary_key(self) -> None: @@ -251,7 +253,9 @@ class EndToEndPerspectivesTests(BaseRemoteKeyResourceTestCase): assert isinstance(keyres, FetchKeyResult) self.assertEqual( signedjson.key.encode_verify_key_base64(keyres.verify_key), - signedjson.key.encode_verify_key_base64(testkey.verify_key), + signedjson.key.encode_verify_key_base64( + signedjson.key.get_verify_key(testkey) + ), ) def test_get_notary_keyserver_key(self) -> None: @@ -268,5 +272,7 @@ class EndToEndPerspectivesTests(BaseRemoteKeyResourceTestCase): assert isinstance(keyres, FetchKeyResult) self.assertEqual( signedjson.key.encode_verify_key_base64(keyres.verify_key), - signedjson.key.encode_verify_key_base64(self.hs_signing_key.verify_key), + signedjson.key.encode_verify_key_base64( + signedjson.key.get_verify_key(self.hs_signing_key) + ), )