From d41c8f6d4ddc647b91eb17ee7d410b039101e442 Mon Sep 17 00:00:00 2001 From: Richard van der Hoff Date: Mon, 20 Apr 2020 17:54:35 +0100 Subject: [PATCH] Revert "Query missing cross-signing keys on local sig upload" This was incorrectly merged to the release branch before it was ready. This reverts commit 72fe2affb6ac86d433b80b6452da57052365aa26. --- changelog.d/7289.bugfix | 1 - synapse/federation/transport/client.py | 14 +-- synapse/handlers/e2e_keys.py | 138 ++----------------------- 3 files changed, 12 insertions(+), 141 deletions(-) delete mode 100644 changelog.d/7289.bugfix diff --git a/changelog.d/7289.bugfix b/changelog.d/7289.bugfix deleted file mode 100644 index 5b4fbd77ac..0000000000 --- a/changelog.d/7289.bugfix +++ /dev/null @@ -1 +0,0 @@ -Fix an edge-case where it was not possible to cross-sign a user which did not share a room with any user on your homeserver. The bug only affected Synapse deployments in worker mode. diff --git a/synapse/federation/transport/client.py b/synapse/federation/transport/client.py index c35637a571..dc563538de 100644 --- a/synapse/federation/transport/client.py +++ b/synapse/federation/transport/client.py @@ -406,19 +406,13 @@ class TransportLayerClient(object): "device_keys": { "": { "": {...} - } } - "master_keys": { - "": {...} - } } - "self_signing_keys": { - "": {...} } } } Args: destination(str): The server to query. query_content(dict): The user ids to query. Returns: - A dict containing device and cross-signing keys. + A dict containg the device keys. """ path = _create_v1_path("/user/keys/query") @@ -435,16 +429,14 @@ class TransportLayerClient(object): Response: { "stream_id": "...", - "devices": [ { ... } ], - "master_key": { ... }, - "self_signing_key: { ... } + "devices": [ { ... } ] } Args: destination(str): The server to query. query_content(dict): The user ids to query. Returns: - A dict containing device and cross-signing keys. + A dict containg the device keys. """ path = _create_v1_path("/user/devices/%s", user_id) diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py index afc173ab2f..8d7075f2eb 100644 --- a/synapse/handlers/e2e_keys.py +++ b/synapse/handlers/e2e_keys.py @@ -16,7 +16,6 @@ # limitations under the License. import logging -from typing import Dict, Optional, Tuple from six import iteritems @@ -24,7 +23,6 @@ import attr from canonicaljson import encode_canonical_json, json from signedjson.key import decode_verify_key_bytes from signedjson.sign import SignatureVerifyException, verify_signed_json -from signedjson.types import VerifyKey from unpaddedbase64 import decode_base64 from twisted.internet import defer @@ -176,8 +174,8 @@ class E2eKeysHandler(object): """This is called when we are querying the device list of a user on a remote homeserver and their device list is not in the device list cache. If we share a room with this user and we're not querying for - specific user we will update the cache with their device list. - """ + specific user we will update the cache + with their device list.""" destination_query = remote_queries_not_in_cache[destination] @@ -963,19 +961,13 @@ class E2eKeysHandler(object): return signature_list, failures @defer.inlineCallbacks - def _get_e2e_cross_signing_verify_key( - self, user_id: str, key_type: str, from_user_id: str = None - ): - """Fetch locally or remotely query for a cross-signing public key. - - First, attempt to fetch the cross-signing public key from storage. - If that fails, query the keys from the homeserver they belong to - and update our local copy. + def _get_e2e_cross_signing_verify_key(self, user_id, key_type, from_user_id=None): + """Fetch the cross-signing public key from storage and interpret it. Args: - user_id: the user whose key should be fetched - key_type: the type of key to fetch - from_user_id: the user that we are fetching the keys for. + user_id (str): the user whose key should be fetched + key_type (str): the type of key to fetch + from_user_id (str): the user that we are fetching the keys for. This affects what signatures are fetched. Returns: @@ -984,128 +976,16 @@ class E2eKeysHandler(object): Raises: NotFoundError: if the key is not found - SynapseError: if `user_id` is invalid """ - user = UserID.from_string(user_id) - key_id = None - verify_key = None - key = yield self.store.get_e2e_cross_signing_key( user_id, key_type, from_user_id ) - - # If we couldn't find the key locally, and we're looking for keys of - # another user then attempt to fetch the missing key from the remote - # user's server. - # - # We may run into this in possible edge cases where a user tries to - # cross-sign a remote user, but does not share any rooms with them yet. - # Thus, we would not have their key list yet. We fetch the key here, - # store it and notify clients of new, associated device IDs. - if ( - key is None - and not self.is_mine(user) - # We only get "master" and "self_signing" keys from remote servers - and key_type in ["master", "self_signing"] - ): - key = yield self._retrieve_cross_signing_keys_for_remote_user( - user, key_type - ) - if key is None: - logger.debug("No %s key found for %s", key_type, user_id) + logger.debug("no %s key found for %s", key_type, user_id) raise NotFoundError("No %s key found for %s" % (key_type, user_id)) - - # If we retrieved the keys remotely, these values will already be set - if key_id is None or verify_key is None: - try: - key_id, verify_key = get_verify_key_from_cross_signing_key(key) - except ValueError as e: - logger.debug( - "Invalid %s key retrieved: %s - %s %s", key_type, key, type(e), e, - ) - raise SynapseError( - 502, "Invalid %s key retrieved from remote server", key_type - ) - + key_id, verify_key = get_verify_key_from_cross_signing_key(key) return key, key_id, verify_key - @defer.inlineCallbacks - def _retrieve_cross_signing_keys_for_remote_user( - self, user: UserID, desired_key_type: str, - ) -> Tuple[Optional[Dict], Optional[str], Optional[VerifyKey]]: - """Queries cross-signing keys for a remote user and saves them to the database - - Only the key specified by `key_type` will be returned, while all retrieved keys - will be saved regardless - - Args: - user: The user to query remote keys for - desired_key_type: The type of key to receive. One of "master", "self_signing" - - Returns: - A tuple of the retrieved key content, the key's ID and the matching VerifyKey. - If the key cannot be retrieved, all values in the tuple will instead be None. - """ - try: - remote_result = yield self.federation.query_user_devices( - user.domain, user.to_string() - ) - except Exception as e: - logger.warning( - "Unable to query %s for cross-signing keys of user %s: %s %s", - user.domain, - user.to_string(), - type(e), - e, - ) - return None - - # Process each of the retrieved cross-signing keys - final_key = None - final_key_id = None - final_verify_key = None - device_ids = [] - for key_type in ["master", "self_signing"]: - key_content = remote_result.get(key_type + "_key") - if not key_content: - continue - - # At the same time, store this key in the db for - # subsequent queries - yield self.store.set_e2e_cross_signing_key( - user.to_string(), key_type, key_content - ) - - # Note down the device ID attached to this key - try: - # verify_key is a VerifyKey from signedjson, which uses - # .version to denote the portion of the key ID after the - # algorithm and colon, which is the device ID - key_id, verify_key = get_verify_key_from_cross_signing_key(key_content) - except ValueError as e: - logger.debug( - "Invalid %s key retrieved: %s - %s %s", - key_type, - key_content, - type(e), - e, - ) - continue - device_ids.append(verify_key.version) - - # If this is the desired key type, save it and it's ID/VerifyKey - if key_type == desired_key_type: - final_key = key_content - final_verify_key = verify_key - final_key_id = key_id - - # Notify clients that new devices for this user have been discovered - if device_ids: - yield self.device_handler.notify_device_update(user.to_string(), device_ids) - - return final_key, final_key_id, final_verify_key - def _check_cross_signing_key(key, user_id, key_type, signing_key=None): """Check a cross-signing key uploaded by a user. Performs some basic sanity