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

Rewrite the KeyRing #10035

Merged
merged 17 commits into from
Jun 2, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
21 changes: 3 additions & 18 deletions synapse/crypto/keyring.py
Original file line number Diff line number Diff line change
Expand Up @@ -74,23 +74,19 @@ class VerifyJsonRequest:
minimum_valid_until_ts: time at which we require the signing key to
be valid. (0 implies we don't care)

request_name: The name of the request.

erikjohnston marked this conversation as resolved.
Show resolved Hide resolved
key_ids: The set of key_ids to that could be used to verify the JSON object
"""

server_name = attr.ib(type=str)
get_json_object = attr.ib(type=Callable[[], JsonDict])
minimum_valid_until_ts = attr.ib(type=int)
request_name = attr.ib(type=str)
key_ids = attr.ib(type=List[str])

@staticmethod
def from_json_object(
server_name: str,
json_object: JsonDict,
minimum_valid_until_ms: int,
request_name: str,
):
"""Create a VerifyJsonRequest to verify all signatures on a signed JSON
object for the given server.
Expand All @@ -100,7 +96,6 @@ def from_json_object(
server_name,
lambda: json_object,
minimum_valid_until_ms,
request_name=request_name,
key_ids=key_ids,
)

Expand All @@ -120,7 +115,6 @@ def from_event(
# memory than the Event object itself.
lambda: prune_event_dict(event.room_version, event.get_pdu_json()),
minimum_valid_until_ms,
request_name=event.event_id,
key_ids=key_ids,
)

Expand Down Expand Up @@ -177,7 +171,6 @@ def verify_json_for_server(
server_name: str,
json_object: JsonDict,
validity_time: int,
request_name: str,
) -> defer.Deferred:
"""Verify that a JSON object has been signed by a given server

Expand All @@ -189,9 +182,6 @@ def verify_json_for_server(
validity_time: timestamp at which we require the signing key to
be valid. (0 implies we don't care)

request_name: an identifier for this json object (eg, an event id)
for logging.

Returns:
Deferred[None]: completes if the the object was correctly signed, otherwise
errbacks with an error
Expand All @@ -200,27 +190,23 @@ def verify_json_for_server(
server_name,
json_object,
validity_time,
request_name,
)
return defer.ensureDeferred(self.process_request(request))
erikjohnston marked this conversation as resolved.
Show resolved Hide resolved

def verify_json_objects_for_server(
self, server_and_json: Iterable[Tuple[str, dict, int, str]]
self, server_and_json: Iterable[Tuple[str, dict, int]]
) -> List[defer.Deferred]:
"""Bulk verifies signatures of json objects, bulk fetching keys as
necessary.

Args:
server_and_json:
Iterable of (server_name, json_object, validity_time, request_name)
Iterable of (server_name, json_object, validity_time)
tuples.

validity_time is a timestamp at which the signing key must be
valid.

request_name is an identifier for this json object (eg, an event id)
for logging.

Returns:
List<Deferred[None]>: for each input triplet, a deferred indicating success
or failure to verify each json object's signature for the given
Expand All @@ -235,11 +221,10 @@ def verify_json_objects_for_server(
server_name,
json_object,
validity_time,
request_name,
),
)
)
for server_name, json_object, validity_time, request_name in server_and_json
for server_name, json_object, validity_time in server_and_json
]

def verify_events_for_server(
Expand Down
4 changes: 3 additions & 1 deletion synapse/federation/transport/server.py
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,9 @@ async def authenticate_request(self, request, content):
)

await self.keyring.verify_json_for_server(
origin, json_request, now, "Incoming request"
origin,
json_request,
now,
)

logger.debug("Request from %s", origin)
Expand Down
4 changes: 3 additions & 1 deletion synapse/groups/attestations.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,9 @@ async def verify_attestation(

assert server_name is not None
await self.keyring.verify_json_for_server(
server_name, attestation, now, "Group attestation"
server_name,
attestation,
now,
)

def create_attestation(self, group_id: str, user_id: str) -> JsonDict:
Expand Down
47 changes: 36 additions & 11 deletions tests/crypto/test_keyring.py
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,7 @@ async def first_lookup_fetch(
async def first_lookup():
with LoggingContext("context_11", request=FakeRequest("context_11")):
res_deferreds = kr.verify_json_objects_for_server(
[("server10", json1, 0, "test10"), ("server11", {}, 0, "test11")]
[("server10", json1, 0), ("server11", {}, 0)]
)

# the unsigned json should be rejected pretty quickly
Expand Down Expand Up @@ -147,7 +147,13 @@ async def second_lookup_fetch(
async def second_lookup():
with LoggingContext("context_12", request=FakeRequest("context_12")):
res_deferreds_2 = kr.verify_json_objects_for_server(
[("server10", json1, 0, "test")]
[
(
"server10",
json1,
0,
)
]
)
res_deferreds_2[0].addBoth(self.check_context, None)
second_lookup_state[0] = 1
Expand Down Expand Up @@ -184,11 +190,11 @@ def test_verify_json_for_server(self):
signedjson.sign.sign_json(json1, "server9", key1)

# should fail immediately on an unsigned object
d = _verify_json_for_server(kr, "server9", {}, 0, "test unsigned")
d = _verify_json_for_server(kr, "server9", {}, 0)
self.get_failure(d, SynapseError)

# should succeed on a signed object
d = _verify_json_for_server(kr, "server9", json1, 500, "test signed")
d = _verify_json_for_server(kr, "server9", json1, 500)
# self.assertFalse(d.called)
self.get_success(d)

Expand All @@ -215,14 +221,12 @@ def test_verify_json_for_server_with_null_valid_until_ms(self):
signedjson.sign.sign_json(json1, "server9", key1)

# should fail immediately on an unsigned object
d = _verify_json_for_server(kr, "server9", {}, 0, "test unsigned")
d = _verify_json_for_server(kr, "server9", {}, 0)
self.get_failure(d, SynapseError)

# should fail on a signed object with a non-zero minimum_valid_until_ms,
# as it tries to refetch the keys and fails.
d = _verify_json_for_server(
kr, "server9", json1, 500, "test signed non-zero min"
)
d = _verify_json_for_server(kr, "server9", json1, 500)
self.get_failure(d, SynapseError)

# We expect the keyring tried to refetch the key once.
Expand All @@ -232,7 +236,10 @@ def test_verify_json_for_server_with_null_valid_until_ms(self):

# should succeed on a signed object with a 0 minimum_valid_until_ms
d = _verify_json_for_server(
kr, "server9", json1, 0, "test signed with zero min"
kr,
"server9",
json1,
0,
)
self.get_success(d)

Expand Down Expand Up @@ -260,7 +267,14 @@ async def get_keys(
# the first request should succeed; the second should fail because the key
# has expired
results = kr.verify_json_objects_for_server(
[("server1", json1, 500, "test1"), ("server1", json1, 1500, "test2")]
[
(
"server1",
json1,
500,
),
("server1", json1, 1500),
]
)
self.assertEqual(len(results), 2)
self.get_success(results[0])
Expand Down Expand Up @@ -301,7 +315,18 @@ async def get_keys2(
signedjson.sign.sign_json(json1, "server1", key1)

results = kr.verify_json_objects_for_server(
[("server1", json1, 1200, "test1"), ("server1", json1, 1500, "test2")]
[
(
"server1",
json1,
1200,
),
(
"server1",
json1,
1500,
),
]
)
self.assertEqual(len(results), 2)
self.get_success(results[0])
Expand Down