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

Fetch verify key locally rather than trying to do so over federation if origin and host are the same. #11129

Merged
merged 10 commits into from
Oct 28, 2021
1 change: 1 addition & 0 deletions changelog.d/11129.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix long-standing bug where verification requests could fail in certain cases if whitelist was in place but did not include your own homeserver.
38 changes: 38 additions & 0 deletions synapse/crypto/keyring.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
from signedjson.key import (
decode_verify_key_bytes,
encode_verify_key_base64,
get_verify_key,
is_signing_algorithm_supported,
)
from signedjson.sign import (
Expand Down Expand Up @@ -177,6 +178,8 @@ def __init__(
clock=hs.get_clock(),
process_batch_callback=self._inner_fetch_key_requests,
)
self.verify_key = get_verify_key(hs.signing_key)
self.hostname = hs.hostname

async def verify_json_for_server(
self,
Expand All @@ -196,6 +199,7 @@ async 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 = VerifyJsonRequest.from_json_object(
server_name,
json_object,
Expand Down Expand Up @@ -262,6 +266,10 @@ async def process_request(self, verify_request: VerifyJsonRequest) -> None:
Codes.UNAUTHORIZED,
)

# If we are the originating server don't fetch verify key for self over federation
if verify_request.server_name == self.hostname:
return await self.process_request_locally(verify_request)

# Add the keys we need to verify to the queue for retrieval. We queue
# up requests for the same server so we don't end up with many in flight
# requests for the same keys.
Expand Down Expand Up @@ -322,6 +330,36 @@ async def process_request(self, verify_request: VerifyJsonRequest) -> None:
Codes.UNAUTHORIZED,
)

async def process_request_locally(self, verify_request: VerifyJsonRequest) -> None:
clokep marked this conversation as resolved.
Show resolved Hide resolved
verify_key = self.verify_key
json_object = verify_request.get_json_object()
try:
verify_signed_json(
json_object,
verify_request.server_name,
verify_key,
)
except SignatureVerifyException as e:
logger.debug(
"Error verifying signature for %s:%s:%s with key %s: %s",
verify_request.server_name,
verify_key.alg,
verify_key.version,
encode_verify_key_base64(verify_key),
str(e),
)
raise SynapseError(
401,
"Invalid signature for server %s with key %s:%s: %s"
% (
verify_request.server_name,
verify_key.alg,
verify_key.version,
str(e),
),
Codes.UNAUTHORIZED,
)

async def _inner_fetch_key_requests(
self, requests: List[_FetchKeyRequest]
) -> Dict[str, Dict[str, FetchKeyResult]]:
Expand Down
9 changes: 9 additions & 0 deletions tests/crypto/test_keyring.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,15 @@ def test_verify_json_for_server(self):
# self.assertFalse(d.called)
self.get_success(d)

def test_verify_for_server_locally(self):
clokep marked this conversation as resolved.
Show resolved Hide resolved
kr = keyring.Keyring(self.hs)
json1 = {}
signedjson.sign.sign_json(json1, self.hs.hostname, self.hs.signing_key)

# Test that verify_json_for_server succeeds on a object signed by ourselves
d = kr.verify_json_for_server(self.hs.hostname, json1, 0)
self.get_success(d)
clokep marked this conversation as resolved.
Show resolved Hide resolved

def test_verify_json_for_server_with_null_valid_until_ms(self):
"""Tests that we correctly handle key requests for keys we've stored
with a null `ts_valid_until_ms`
Expand Down