Skip to content
This repository has been archived by the owner on Apr 26, 2024. It is now read-only.

Fix typechecker problems exposed by signedjson 1.1.2 #12326

Merged
merged 9 commits into from
Mar 29, 2022
Merged
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 0 additions & 3 deletions mypy.ini
Original file line number Diff line number Diff line change
Expand Up @@ -321,9 +321,6 @@ ignore_missing_imports = True
[mypy-service_identity.*]
ignore_missing_imports = True

[mypy-signedjson.*]
ignore_missing_imports = True

[mypy-treq.*]
ignore_missing_imports = True

Expand Down
9 changes: 5 additions & 4 deletions synapse/config/key.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
NACL_ED25519,
SigningKey,
VerifyKey,
VerifyKeyWithExpiry,
decode_signing_key_base64,
decode_verify_key_bytes,
generate_signing_key,
Expand Down Expand Up @@ -300,16 +301,16 @@ def read_signing_keys(self, signing_key_path: str, name: str) -> List[SigningKey

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 = {}
for key_id, key_data in old_signing_keys.items():
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(
Expand Down Expand Up @@ -422,7 +423,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():
Expand Down
2 changes: 1 addition & 1 deletion synapse/crypto/keyring.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,7 +176,7 @@ def __init__(
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)
Expand Down
2 changes: 1 addition & 1 deletion synapse/events/builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
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 (
Expand Down
12 changes: 6 additions & 6 deletions synapse/rest/key/v2/local_key_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,17 +76,17 @@ def update_response_body(self, time_now_msec: int) -> None:

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 = {
Expand Down
8 changes: 4 additions & 4 deletions synapse/rest/key/v2/remote_key_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -149,7 +149,7 @@ async def query_keys(

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()

Expand Down Expand Up @@ -234,8 +234,8 @@ async def query_keys(
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
Expand Down
6 changes: 3 additions & 3 deletions tests/crypto/test_event_signing.py
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,8 @@
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"

Expand All @@ -39,7 +39,7 @@ def setUp(self):
# 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
Expand Down
16 changes: 11 additions & 5 deletions tests/rest/key/v2/test_remote_key_resource.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ async def get_json(
"verify_keys": {
key_id: {
"key": signedjson.key.encode_verify_key_base64(
signing_key.verify_key
signedjson.key.get_verify_key(signing_key)
)
}
},
Expand Down Expand Up @@ -175,7 +175,7 @@ def prepare(self, reactor: MemoryReactor, clock: Clock, hs: HomeServer) -> None:
% (
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)
)
},
}
Expand Down Expand Up @@ -229,7 +229,9 @@ def test_get_key(self) -> None:
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:
Expand All @@ -251,7 +253,9 @@ def test_get_notary_key(self) -> None:
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:
Expand All @@ -268,5 +272,7 @@ def test_get_notary_keyserver_key(self) -> None:
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)
),
)