Individual workers will fetch remote server keys without sharing a cache · Issue #12767 · matrix-org/synapse

We don't seem to try fetching server keys from the database when validating federation requests on a worker. This can lead to multiple workers each making a server key request to the same remote homeserver.

synapse/synapse/crypto/keyring.py

Lines 260 to 298 in e0bb268

    
               async def process_request(self, verify_request: VerifyJsonRequest) -> None: 
        
                   """Processes the `VerifyJsonRequest`. Raises if the object is not signed 
        
                   by the server, the signatures don't match or we failed to fetch the 
        
                   necessary keys. 
        
                   """ 
        
                   if not verify_request.key_ids: 
        
                       raise SynapseError( 
        
                           400, 
        
                           f"Not signed by {verify_request.server_name}", 
        
                           Codes.UNAUTHORIZED, 
        
                       ) 
        
                   found_keys: Dict[str, FetchKeyResult] = {} 
        
                   # If we are the originating server, short-circuit the key-fetch for any keys 
        
                   # we already have 
        
                   if verify_request.server_name == self._hostname: 
        
                       for key_id in verify_request.key_ids: 
        
                           if key_id in self._local_verify_keys: 
        
                               found_keys[key_id] = self._local_verify_keys[key_id] 
        
                   key_ids_to_find = set(verify_request.key_ids) - found_keys.keys() 
        
                   if key_ids_to_find: 
        
                       # 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. 
        
                       key_request = _FetchKeyRequest( 
        
                           server_name=verify_request.server_name, 
        
                           minimum_valid_until_ts=verify_request.minimum_valid_until_ts, 
        
                           key_ids=list(key_ids_to_find), 
        
                       ) 
        
                       found_keys_by_server = await self._server_queue.add_to_queue( 
        
                           key_request, key=verify_request.server_name 
        
                       ) 
        
                       # Since we batch up requests the returned set of keys may contain keys 
        
                       # from other servers, so we pull out only the ones we care about. 
        
                       found_keys.update(found_keys_by_server.get(verify_request.server_name, {}))

We have a database table where we cache server key responses (server_keys_json). I can't think of any reason why we wouldn't use it here?

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Individual workers will fetch remote server keys without sharing a cache #12767

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

	async def process_request(self, verify_request: VerifyJsonRequest) -> None:
	"""Processes the `VerifyJsonRequest`. Raises if the object is not signed
	by the server, the signatures don't match or we failed to fetch the
	necessary keys.
	"""

	if not verify_request.key_ids:
	raise SynapseError(
	400,
	f"Not signed by {verify_request.server_name}",
	Codes.UNAUTHORIZED,
	)

	found_keys: Dict[str, FetchKeyResult] = {}

	# If we are the originating server, short-circuit the key-fetch for any keys
	# we already have
	if verify_request.server_name == self._hostname:
	for key_id in verify_request.key_ids:
	if key_id in self._local_verify_keys:
	found_keys[key_id] = self._local_verify_keys[key_id]

	key_ids_to_find = set(verify_request.key_ids) - found_keys.keys()
	if key_ids_to_find:
	# 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.
	key_request = _FetchKeyRequest(
	server_name=verify_request.server_name,
	minimum_valid_until_ts=verify_request.minimum_valid_until_ts,
	key_ids=list(key_ids_to_find),
	)
	found_keys_by_server = await self._server_queue.add_to_queue(
	key_request, key=verify_request.server_name
	)

	# Since we batch up requests the returned set of keys may contain keys
	# from other servers, so we pull out only the ones we care about.
	found_keys.update(found_keys_by_server.get(verify_request.server_name, {}))

Uh oh!

Individual workers will fetch remote server keys without sharing a cache #12767

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions