From 8567ff6b4938a477c5b82c5c235124b1ca2e5fbe Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 22 May 2019 16:34:28 -0400 Subject: [PATCH 01/10] implement uploading/downloading of cross-signing keys --- changelog.d/4970.feature | 1 + synapse/api/errors.py | 1 + synapse/handlers/device.py | 19 ++ synapse/handlers/e2e_keys.py | 201 +++++++++++++++++- synapse/handlers/sync.py | 7 +- synapse/rest/client/v2_alpha/keys.py | 47 +++- synapse/storage/__init__.py | 5 +- synapse/storage/devices.py | 79 ++++++- synapse/storage/end_to_end_keys.py | 150 ++++++++++++- synapse/storage/prepare_database.py | 2 +- .../storage/schema/delta/55/signing_keys.sql | 48 +++++ synapse/types.py | 30 +++ 12 files changed, 570 insertions(+), 20 deletions(-) create mode 100644 changelog.d/4970.feature create mode 100644 synapse/storage/schema/delta/55/signing_keys.sql diff --git a/changelog.d/4970.feature b/changelog.d/4970.feature new file mode 100644 index 000000000000..3374ae51fe7f --- /dev/null +++ b/changelog.d/4970.feature @@ -0,0 +1 @@ +Add support for cross-signing. diff --git a/synapse/api/errors.py b/synapse/api/errors.py index e91697049ce2..a230cde93812 100644 --- a/synapse/api/errors.py +++ b/synapse/api/errors.py @@ -61,6 +61,7 @@ class Codes(object): INCOMPATIBLE_ROOM_VERSION = "M_INCOMPATIBLE_ROOM_VERSION" WRONG_ROOM_KEYS_VERSION = "M_WRONG_ROOM_KEYS_VERSION" EXPIRED_ACCOUNT = "ORG_MATRIX_EXPIRED_ACCOUNT" + INVALID_SIGNATURE = "M_INVALID_SIGNATURE" class CodeMessageException(RuntimeError): diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index b398848079ef..0f9e36dcd0c7 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -1,5 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2016 OpenMarket Ltd +# Copyright 2019 New Vector Ltd +# Copyright 2019 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -419,6 +421,23 @@ def notify_device_update(self, user_id, device_ids): for host in hosts: self.federation_sender.send_device_messages(host) + @defer.inlineCallbacks + def notify_user_signature_update(self, from_user_id, user_ids): + """Notify a user that they have made new signatures of other users. + + Args: + from_user_id (str): the user who made the signature + user_ids (list[str]): the users IDs that have new signatures + """ + + position = yield self.store.add_user_signature_change_to_streams( + from_user_id, user_ids + ) + + self.notifier.on_new_event( + "device_list_key", position, users=[from_user_id], + ) + @defer.inlineCallbacks def on_federation_query_user_devices(self, user_id): stream_id, devices = yield self.store.get_devices_with_keys_by_user(user_id) diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py index 9dc46aa15f75..98671336cebe 100644 --- a/synapse/handlers/e2e_keys.py +++ b/synapse/handlers/e2e_keys.py @@ -1,6 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2016 OpenMarket Ltd -# Copyright 2018 New Vector Ltd +# Copyright 2018-2019 New Vector Ltd +# Copyright 2019 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -19,11 +20,23 @@ from six import iteritems from canonicaljson import encode_canonical_json, json +from signedjson.sign import SignatureVerifyException, verify_signed_json from twisted.internet import defer -from synapse.api.errors import CodeMessageException, FederationDeniedError, SynapseError -from synapse.types import UserID, get_domain_from_id +from synapse.api.errors import ( + CodeMessageException, + Codes, + FederationDeniedError, + SynapseError, +) +from synapse.types import ( + UserID, + get_domain_from_id, + get_verify_key_from_cross_signing_key, +) +from synapse.util.async_helpers import Linearizer +from synapse.util.caches.expiringcache import ExpiringCache from synapse.util.logcontext import make_deferred_yieldable, run_in_background from synapse.util.retryutils import NotRetryingDestination @@ -46,7 +59,7 @@ def __init__(self, hs): ) @defer.inlineCallbacks - def query_devices(self, query_body, timeout): + def query_devices(self, query_body, timeout, from_user_id=None): """ Handle a device key query from a client { @@ -64,6 +77,11 @@ def query_devices(self, query_body, timeout): } } } + + Args: + from_user_id (str): the user making the query. This is used when + adding cross-signing signatures to limit what signatures users + can see. """ device_keys_query = query_body.get("device_keys", {}) @@ -120,6 +138,11 @@ def query_devices(self, query_body, timeout): r = remote_queries_not_in_cache.setdefault(domain, {}) r[user_id] = remote_queries[user_id] + # Get cached cross-signing keys + cross_signing_keys = yield self.query_cross_signing_keys( + device_keys_query, from_user_id + ) + # Now fetch any devices that we don't have in our cache @defer.inlineCallbacks def do_remote_query(destination): @@ -135,6 +158,14 @@ def do_remote_query(destination): if user_id in destination_query: results[user_id] = keys + for user_id, key in remote_result["master_keys"].items(): + if user_id in destination_query: + cross_signing_keys["master"][user_id] = key + + for user_id, key in remote_result["self_signing_keys"].items(): + if user_id in destination_query: + cross_signing_keys["self_signing"][user_id] = key + except Exception as e: failures[destination] = _exception_to_failure(e) @@ -143,9 +174,60 @@ def do_remote_query(destination): for destination in remote_queries_not_in_cache ], consumeErrors=True)) - defer.returnValue({ + ret = { "device_keys": results, "failures": failures, - }) + } + + for key, value in iteritems(cross_signing_keys): + ret[key + "_keys"] = value + + defer.returnValue(ret) + + @defer.inlineCallbacks + def query_cross_signing_keys(self, query, from_user_id=None): + """Get cross-signing keys for users + + Args: + query (dict[string, *]): map from user_id. This function only looks + at the dict's keys, and the values are ignored, so the query + format used for query_devices can be used. + from_user_id (str): the user making the query. This is used when + adding cross-signing signatures to limit what signatures users + can see. + + Returns: + defer.Deferred: (resolves to dict[string, dict[string, dict]]): map from + (master|self_signing) -> map from user_id -> master key + """ + master_keys = {} + self_signing_keys = {} + + @defer.inlineCallbacks + def get_cross_signing_key(user_id): + try: + key = yield self.store.get_e2e_cross_signing_key( + user_id, "master", from_user_id + ) + if key: + master_keys[user_id] = key + except Exception: + pass + + try: + key = yield self.store.get_e2e_cross_signing_key( + user_id, "self_signing", from_user_id + ) + if key: + self_signing_keys[user_id] = key + except Exception: + pass + + yield make_deferred_yieldable(defer.gatherResults([ + run_in_background(get_cross_signing_key, user_id) + for user_id in query.keys() + ])) + + defer.returnValue({"master": master_keys, "self_signing": self_signing_keys}) @defer.inlineCallbacks def query_local_devices(self, query): @@ -337,6 +419,113 @@ def _upload_one_time_keys_for_user(self, user_id, device_id, time_now, user_id, device_id, time_now, new_keys ) + @defer.inlineCallbacks + def upload_signing_keys_for_user(self, user_id, keys): + """Upload signing keys for cross-signing + + Args: + user_id (string): the user uploading the keys + keys (dict[string, dict]): the signing keys + """ + + # if a master key is uploaded, then check it. Otherwise, load the + # stored master key, to check signatures on other keys + if "master_key" in keys: + master_key = keys["master_key"] + + _check_cross_signing_key(master_key, user_id, "master") + else: + master_key = yield self.store.get_e2e_cross_signing_key(user_id, "master") + + # if there is no master key, then we can't do anything, because all the + # other cross-signing keys need to be signed by the master key + if not master_key: + raise SynapseError( + 400, + "No master key available", + Codes.MISSING_PARAM + ) + + master_key_id, master_verify_key = get_verify_key_from_cross_signing_key( + master_key + ) + + # for the other cross-signing keys, make sure that they have valid + # signatures from the master key + if "self_signing_key" in keys: + self_signing_key = keys["self_signing_key"] + + _check_cross_signing_key( + self_signing_key, user_id, "self_signing", master_verify_key + ) + + if "user_signing_key" in keys: + user_signing_key = keys["user_signing_key"] + + _check_cross_signing_key( + user_signing_key, user_id, "user_signing", master_verify_key + ) + + # if everything checks out, then store the keys and send notifications + deviceids = [] + if "master_key" in keys: + yield self.store.set_e2e_cross_signing_key( + user_id, "master", master_key + ) + deviceids.append(master_verify_key.version) + if "self_signing_key" in keys: + yield self.store.set_e2e_cross_signing_key( + user_id, "self_signing", self_signing_key + ) + deviceids.append( + get_verify_key_from_cross_signing_key(self_signing_key)[1].version + ) + if "user_signing_key" in keys: + yield self.store.set_e2e_cross_signing_key( + user_id, "user_signing", user_signing_key + ) + # the signature stream matches the semantics that we want for + # user-signing key updates: only the user themselves is notified of + # their own user-signing key updates + yield self.defice_handler.notify_user_signature_update(user_id, [user_id]) + + # master key and self-signing key updates match the semantics of device + # list updates: all users who share an encrypted room are notified + if len(deviceids): + yield self.device_handler.notify_device_update(user_id, deviceids) + + defer.returnValue({}) + + + +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 + checking, and ensures that it is signed, if a signature is required. + + Args: + key (dict): the key data to verify + user_id (str): the user whose key is being checked + key_type (str): the type of key that the key should be + signing_key (VerifyKey): (optional) the signing key that the key should + be signed with. If omitted, signatures will not be checked. + """ + if "user_id" not in key or key["user_id"] != user_id \ + or "usage" not in key or key_type not in key["usage"]: + raise SynapseError( + 400, + ("Invalid %s key" % key_type), + Codes.INVALID_PARAM + ) + + if signing_key: + try: + verify_signed_json(key, user_id, signing_key) + except SignatureVerifyException: + raise SynapseError( + 400, + ("Invalid signature or %s key" % key_type), + Codes.INVALID_SIGNATURE + ) def _exception_to_failure(e): if isinstance(e, CodeMessageException): diff --git a/synapse/handlers/sync.py b/synapse/handlers/sync.py index 1ee9a6e313f0..cc19132db9a8 100644 --- a/synapse/handlers/sync.py +++ b/synapse/handlers/sync.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2015, 2016 OpenMarket Ltd -# Copyright 2018 New Vector Ltd +# Copyright 2018, 2019 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -1061,6 +1061,11 @@ def _generate_sync_entry_for_device_list(self, sync_result_builder, # weren't in the previous sync *or* they left and rejoined. changed.update(newly_joined_or_invited_users) + user_signatures_changed = yield self.store.get_users_whose_signatures_changed( + user_id, since_token.device_list_key + ) + changed.update(user_signatures_changed) + if not changed and not newly_left_users: defer.returnValue(DeviceLists( changed=[], diff --git a/synapse/rest/client/v2_alpha/keys.py b/synapse/rest/client/v2_alpha/keys.py index 8486086b510a..5da8c3322731 100644 --- a/synapse/rest/client/v2_alpha/keys.py +++ b/synapse/rest/client/v2_alpha/keys.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2015, 2016 OpenMarket Ltd +# Copyright 2019 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -26,7 +27,7 @@ ) from synapse.types import StreamToken -from ._base import client_v2_patterns +from ._base import client_v2_patterns, interactive_auth_handler logger = logging.getLogger(__name__) @@ -143,10 +144,11 @@ def __init__(self, hs): @defer.inlineCallbacks def on_POST(self, request): - yield self.auth.get_user_by_req(request, allow_guest=True) + requester = yield self.auth.get_user_by_req(request, allow_guest=True) + user_id = requester.user.to_string() timeout = parse_integer(request, "timeout", 10 * 1000) body = parse_json_object_from_request(request) - result = yield self.e2e_keys_handler.query_devices(body, timeout) + result = yield self.e2e_keys_handler.query_devices(body, timeout, user_id) defer.returnValue((200, result)) @@ -228,8 +230,47 @@ def on_POST(self, request): defer.returnValue((200, result)) +class SigningKeyUploadServlet(RestServlet): + """ + POST /keys/device_signing/upload HTTP/1.1 + Content-Type: application/json + + { + } + """ + PATTERNS = client_v2_patterns("/keys/device_signing/upload$") + + def __init__(self, hs): + """ + Args: + hs (synapse.server.HomeServer): server + """ + super(SigningKeyUploadServlet, self).__init__() + self.hs = hs + self.auth = hs.get_auth() + self.e2e_keys_handler = hs.get_e2e_keys_handler() + self.auth_handler = hs.get_auth_handler() + + @interactive_auth_handler + @defer.inlineCallbacks + def on_POST(self, request): + requester = yield self.auth.get_user_by_req(request, allow_guest=True) + user_id = requester.user.to_string() + body = parse_json_object_from_request(request) + + yield self.auth_handler.validate_user_via_ui_auth( + requester, body, self.hs.get_ip_from_request(request), + ) + + result = yield self.e2e_keys_handler.upload_signing_keys_for_user( + user_id, body + ) + defer.returnValue((200, result)) + + def register_servlets(hs, http_server): KeyUploadServlet(hs).register(http_server) KeyQueryServlet(hs).register(http_server) KeyChangesServlet(hs).register(http_server) OneTimeKeyServlet(hs).register(http_server) + SigningKeyUploadServlet(hs).register(http_server) diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py index 66675d08aee5..36e41b99d5a7 100644 --- a/synapse/storage/__init__.py +++ b/synapse/storage/__init__.py @@ -1,6 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2014-2016 OpenMarket Ltd -# Copyright 2018 New Vector Ltd +# Copyright 2018,2019 New Vector Ltd # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -205,6 +205,9 @@ def __init__(self, db_conn, hs): self._device_list_stream_cache = StreamChangeCache( "DeviceListStreamChangeCache", device_list_max ) + self._user_signature_stream_cache = StreamChangeCache( + "UserSignatureStreamChangeCache", device_list_max, + ) self._device_list_federation_stream_cache = StreamChangeCache( "DeviceListFederationStreamChangeCache", device_list_max ) diff --git a/synapse/storage/devices.py b/synapse/storage/devices.py index fd869b934c7b..b61b0297efec 100644 --- a/synapse/storage/devices.py +++ b/synapse/storage/devices.py @@ -1,5 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2016 OpenMarket Ltd +# Copyright 2019 New Vector Ltd +# Copyright 2019 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -20,10 +22,11 @@ from twisted.internet import defer -from synapse.api.errors import StoreError +from synapse.api.errors import Codes, StoreError from synapse.metrics.background_process_metrics import run_as_background_process from synapse.storage._base import Cache, SQLBaseStore, db_to_json from synapse.storage.background_updates import BackgroundUpdateStore +from synapse.types import get_verify_key_from_cross_signing_key from synapse.util.caches.descriptors import cached, cachedInlineCallbacks, cachedList logger = logging.getLogger(__name__) @@ -47,7 +50,7 @@ def get_device(self, user_id, device_id): """ return self._simple_select_one( table="devices", - keyvalues={"user_id": user_id, "device_id": device_id}, + keyvalues={"user_id": user_id, "device_id": device_id, "hidden": False}, retcols=("user_id", "device_id", "display_name"), desc="get_device", ) @@ -65,7 +68,7 @@ def get_devices_by_user(self, user_id): """ devices = yield self._simple_select_list( table="devices", - keyvalues={"user_id": user_id}, + keyvalues={"user_id": user_id, "hidden": False}, retcols=("user_id", "device_id", "display_name"), desc="get_devices_by_user", ) @@ -205,6 +208,33 @@ def _mark_as_sent_devices_by_remote_txn(self, txn, destination, stream_id): """ txn.execute(sql, (destination, stream_id)) + @defer.inlineCallbacks + def add_user_signature_change_to_streams(self, from_user_id, user_ids): + """Persist that a user has made new signatures + """ + + with self._device_list_id_gen.get_next() as stream_id: + yield self.runInteraction( + "add_user_sig_change_to_streams", self._add_user_signature_change_txn, + from_user_id, user_ids, stream_id, + ) + defer.returnValue(stream_id) + + def _add_user_signature_change_txn(self, txn, from_user_id, user_ids, stream_id): + txn.call_after( + self._user_signature_stream_cache.entity_has_changed, + from_user_id, stream_id, + ) + self._simple_insert_txn( + txn, + "user_signature_stream", + values={ + "stream_id": stream_id, + "from_user_id": from_user_id, + "user_ids": json.dumps(user_ids), + }, + ) + def get_device_stream_token(self): return self._device_list_id_gen.get_current_token() @@ -318,6 +348,25 @@ def get_user_whose_devices_changed(self, from_key): ) defer.returnValue(set(row[0] for row in rows)) + @defer.inlineCallbacks + def get_users_whose_signatures_changed(self, user_id, from_key): + """Get set of users who have new cross-signing signatures have changed since + `from_key`. + + """ + from_key = int(from_key) + if self._user_signature_stream_cache.has_entity_changed(user_id, from_key): + sql = """ + SELECT DISTINCT user_ids FROM user_signature_stream + WHERE from_user_id = ? AND stream_id > ? + """ + rows = yield self._execute( + "get_users_whose_signatures_changed", None, sql, user_id, from_key + ) + defer.returnValue(set(user for row in rows for user in json.loads(row[0]))) + else: + defer.returnValue(set()) + def get_all_device_list_changes_for_remotes(self, from_key, to_key): """Return a list of `(stream_id, user_id, destination)` which is the combined list of changes to devices, and which destinations need to be @@ -436,12 +485,30 @@ def store_device(self, user_id, device_id, initial_device_display_name): "user_id": user_id, "device_id": device_id, "display_name": initial_device_display_name, + "hidden": False }, desc="store_device", or_ignore=True, ) + if not inserted: + # if the device already exists, check if it's a real device, or + # if the device ID is reserved by something else + hidden = yield self._simple_select_one_onecol( + "devices", + keyvalues={ + "user_id": user_id, + "device_id": device_id + }, + retcol="hidden" + ) + if hidden: + raise StoreError( + 400, "The device ID is reserved", Codes.FORBIDDEN + ) self.device_id_exists_cache.prefill(key, True) defer.returnValue(inserted) + except StoreError as e: + raise e except Exception as e: logger.error( "store_device with device_id=%s(%r) user_id=%s(%r)" @@ -468,7 +535,7 @@ def delete_device(self, user_id, device_id): """ yield self._simple_delete_one( table="devices", - keyvalues={"user_id": user_id, "device_id": device_id}, + keyvalues={"user_id": user_id, "device_id": device_id, "hidden": False}, desc="delete_device", ) @@ -488,7 +555,7 @@ def delete_devices(self, user_id, device_ids): table="devices", column="device_id", iterable=device_ids, - keyvalues={"user_id": user_id}, + keyvalues={"user_id": user_id, "hidden": False}, desc="delete_devices", ) for device_id in device_ids: @@ -514,7 +581,7 @@ def update_device(self, user_id, device_id, new_display_name=None): return defer.succeed(None) return self._simple_update_one( table="devices", - keyvalues={"user_id": user_id, "device_id": device_id}, + keyvalues={"user_id": user_id, "device_id": device_id, "hidden": False}, updatevalues=updates, desc="update_device", ) diff --git a/synapse/storage/end_to_end_keys.py b/synapse/storage/end_to_end_keys.py index 2fabb9e2cbd2..d99f434cfa64 100644 --- a/synapse/storage/end_to_end_keys.py +++ b/synapse/storage/end_to_end_keys.py @@ -1,5 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2015, 2016 OpenMarket Ltd +# Copyright 2019 New Vector Ltd +# Copyright 2019 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -12,9 +14,11 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. +import time + from six import iteritems -from canonicaljson import encode_canonical_json +from canonicaljson import encode_canonical_json, json from twisted.internet import defer @@ -85,7 +89,7 @@ def _get_e2e_device_keys_txn( " k.key_json" " FROM devices d" " %s JOIN e2e_device_keys_json k USING (user_id, device_id)" - " WHERE %s" + " WHERE (%s) AND d.hidden = 0" ) % ( "LEFT" if include_all_devices else "INNER", " OR ".join("(" + q + ")" for q in query_clauses), @@ -281,3 +285,145 @@ def delete_e2e_keys_by_device_txn(txn): return self.runInteraction( "delete_e2e_keys_by_device", delete_e2e_keys_by_device_txn ) + + def _set_e2e_device_signing_key_txn(self, txn, user_id, key_type, key): + """Set a user's cross-signing key. + + Args: + txn (twisted.enterprise.adbapi.Connection): db connection + user_id (str): the user to set the signing key for + key_type (str): the type of key that is being set: either 'master' + for a master key, 'self_signing' for a self-signing key, or + 'user_signing' for a user-signing key + key (dict): the key data + """ + # the cross-signing keys need to occupy the same namespace as devices, + # since signatures are identified by device ID. So add an entry to the + # device table to make sure that we don't have a collision with device + # IDs + for v in key["keys"].values(): + pubkey = v + break + self._simple_insert( + "devices", + values={ + "user_id": user_id, + "device_id": pubkey, + "display_name": key_type + " signing key", + "hidden": True + }, + desc="store_master_key_device" + ) + + # and finally, store the key itself + self._simple_insert( + "e2e_device_signing_keys", + values={ + "user_id": user_id, + "keytype": key_type, + "keydata": json.dumps(key), + "ts": time.time() * 1000 + }, + desc="store_master_key" + ) + + def set_e2e_cross_signing_key(self, user_id, key_type, key): + """Set a user's cross-signing key. + + Args: + user_id (str): the user to set the user-signing key for + key_type (str): the type of cross-signing key to set + key (dict): the key data + """ + return self.runInteraction( + "add_e2e_device_signing_key", + self._set_e2e_device_signing_key_txn, + user_id, key_type, key + ) + + def _get_e2e_device_signing_key_txn(self, txn, user_id, key_type, from_user_id=None): + """Returns a user's cross-signing key. + + Args: + txn (twisted.enterprise.adbapi.Connection): db connection + user_id (str): the user whose key is being requested + key_type (str): the type of key that is being set: either 'master' + for a master key, 'self_signing' for a self-signing key, or + 'user_signing' for a user-signing key + from_user_id (str): if specified, signatures made by this user on + the key will be included in the result + + Returns: + dict of the key data + """ + sql = ( + "SELECT keydata " + " FROM e2e_device_signing_keys " + " WHERE user_id = ? AND keytype = ? ORDER BY ts DESC LIMIT 1" + ) + txn.execute(sql, (user_id, key_type)) + row = txn.fetchone() + if not row: + return None + key = json.loads(row[0]) + + device_id = None + for k in key["keys"].values(): + device_id = k + + if from_user_id is not None: + sql = ( + "SELECT key_id, signature " + " FROM e2e_device_signatures " + " WHERE user_id = ? " + " AND target_user_id = ? " + " AND target_device_id = ? " + ) + txn.execute(sql, (from_user_id, user_id, device_id)) + row = txn.fetchone() + if row: + key.setdefault("signatures", {}) \ + .setdefault(from_user_id, {})[row[0]] = row[1] + + return key + + def get_e2e_cross_signing_key(self, user_id, key_type, from_user_id=None): + """Returns a user's cross-signing key. + + Args: + user_id (str): the user whose self-signing key is being requested + key_type (str): the type of cross-signing key to get + from_user_id (str): if specified, signatures made by this user on + the self-signing key will be included in the result + + Returns: + dict of the key data + """ + return self.runInteraction( + "get_e2e_device_signing_key", + self._get_e2e_device_signing_key_txn, + user_id, key_type, from_user_id + ) + + def store_e2e_device_signatures(self, user_id, signatures): + """Stores cross-signing signatures. + + Args: + user_id (str): the user who made the signatures + signatures (iterable[(str, str, str, str)]): signatures to add - each + a tuple of (key_id, target_user_id, target_device_id, signature), + where key_id is the ID of the key (including the signature + algorithm) that made the signature, target_user_id and + target_device_id indicate the device being signed, and signature + is the signature of the device + """ + return self._simple_insert_many( + "e2e_device_signatures", + [{"user_id": user_id, + "key_id": key_id, + "target_user_id": target_user_id, + "target_device_id": target_device_id, + "signature": signature} + for (key_id, target_user_id, target_device_id, signature) in signatures], + "add_e2e_signing_key" + ) diff --git a/synapse/storage/prepare_database.py b/synapse/storage/prepare_database.py index c1711bc8bd5f..23a4baa4841d 100644 --- a/synapse/storage/prepare_database.py +++ b/synapse/storage/prepare_database.py @@ -25,7 +25,7 @@ # Remember to update this number every time a change is made to database # schema files, so the users will be informed on server restarts. -SCHEMA_VERSION = 54 +SCHEMA_VERSION = 55 dir_path = os.path.abspath(os.path.dirname(__file__)) diff --git a/synapse/storage/schema/delta/55/signing_keys.sql b/synapse/storage/schema/delta/55/signing_keys.sql new file mode 100644 index 000000000000..70330ac49bd2 --- /dev/null +++ b/synapse/storage/schema/delta/55/signing_keys.sql @@ -0,0 +1,48 @@ +/* Copyright 2019 New Vector Ltd + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +-- device signing keys for cross-signing +CREATE TABLE e2e_device_signing_keys ( + user_id TEXT NOT NULL, + keytype TEXT NOT NULL, + keydata TEXT NOT NULL, + ts BIGINT NOT NULL +); + +CREATE UNIQUE INDEX e2e_device_signing_keys_idx ON e2e_device_signing_keys(user_id, keytype, ts); + +-- devices signatures for cross-signing +CREATE TABLE e2e_device_signatures ( + user_id TEXT NOT NULL, + key_id TEXT NOT NULL, + target_user_id TEXT NOT NULL, + target_device_id TEXT NOT NULL, + signature TEXT NOT NULL +); + +CREATE UNIQUE INDEX e2e_device_signatures_idx ON e2e_device_signatures(user_id, target_user_id, target_device_id); + +-- stream of user signature updates +CREATE TABLE user_signature_stream ( + stream_id BIGINT NOT NULL, + from_user_id TEXT NOT NULL, + user_ids TEXT NOT NULL +); + +CREATE INDEX user_signature_stream_idx ON user_signature_stream(stream_id, from_user_id); + +-- device list needs to know which ones are "real" devices, and which ones are +-- just used to avoid collisions +ALTER TABLE devices ADD COLUMN hidden BOOLEAN DEFAULT FALSE; diff --git a/synapse/types.py b/synapse/types.py index 3de94b633524..08eb65049756 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -1,5 +1,6 @@ # -*- coding: utf-8 -*- # Copyright 2014-2016 OpenMarket Ltd +# Copyright 2019 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -17,6 +18,8 @@ from collections import namedtuple import attr +from signedjson.key import decode_verify_key_bytes +from unpaddedbase64 import decode_base64 from synapse.api.errors import SynapseError @@ -467,3 +470,30 @@ class ReadReceipt(object): user_id = attr.ib() event_ids = attr.ib() data = attr.ib() + + +def get_verify_key_from_cross_signing_key(key_info): + """Get the key ID and signedjson verify key from a cross-signing key dict + + Args: + key_info (dict): a cross-signing key dict, which must have a "keys" + property that has exactly one item in it + + Returns: + (str, VerifyKey): the key ID and verify key for the cross-signing key + """ + # make sure that exactly one key is provided + if "keys" not in key_info: + raise SynapseError( + 400, + "Invalid key" + ) + keys = key_info["keys"] + if len(keys) != 1: + raise SynapseError( + 400, + "Invalid key" + ) + # and return that one key + for key_id, key_data in keys.items(): + return (key_id, decode_verify_key_bytes(key_id, decode_base64(key_data))) From b2330615ec24ee9fa1d32821119b0ce05ae03682 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 12 Jun 2019 10:37:44 -0400 Subject: [PATCH 02/10] return the user's own user-signing key --- synapse/handlers/e2e_keys.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py index 98671336cebe..8ec64ddcd3ae 100644 --- a/synapse/handlers/e2e_keys.py +++ b/synapse/handlers/e2e_keys.py @@ -201,6 +201,7 @@ def query_cross_signing_keys(self, query, from_user_id=None): """ master_keys = {} self_signing_keys = {} + user_signing_keys = {} @defer.inlineCallbacks def get_cross_signing_key(user_id): @@ -222,12 +223,28 @@ def get_cross_signing_key(user_id): except Exception: pass + # users can see other users' master and self-signing keys, but can + # only see their own user-signing keys + if from_user_id == user_id: + try: + key = yield self.store.get_e2e_cross_signing_key( + user_id, "user_signing", from_user_id + ) + if key: + user_signing_keys[user_id] = key + except Exception: + pass + yield make_deferred_yieldable(defer.gatherResults([ run_in_background(get_cross_signing_key, user_id) for user_id in query.keys() ])) - defer.returnValue({"master": master_keys, "self_signing": self_signing_keys}) + defer.returnValue({ + "master": master_keys, + "self_signing": self_signing_keys, + "user_signing": user_signing_keys, + }) @defer.inlineCallbacks def query_local_devices(self, query): From 5b632c8acc90678d4173e5706d6cb1c5b4c8f847 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 8 Jul 2019 17:41:58 -0400 Subject: [PATCH 03/10] fix typo --- synapse/handlers/e2e_keys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py index 8ec64ddcd3ae..23ee90770323 100644 --- a/synapse/handlers/e2e_keys.py +++ b/synapse/handlers/e2e_keys.py @@ -504,7 +504,7 @@ def upload_signing_keys_for_user(self, user_id, keys): # the signature stream matches the semantics that we want for # user-signing key updates: only the user themselves is notified of # their own user-signing key updates - yield self.defice_handler.notify_user_signature_update(user_id, [user_id]) + yield self.device_handler.notify_user_signature_update(user_id, [user_id]) # master key and self-signing key updates match the semantics of device # list updates: all users who share an encrypted room are notified From 158104c41fc3fba317b6ef5d12dac29bde42ad57 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Wed, 10 Jul 2019 14:18:41 -0400 Subject: [PATCH 04/10] add/fix some doc strings --- synapse/storage/devices.py | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/synapse/storage/devices.py b/synapse/storage/devices.py index 6fb5c614e695..bc76a5961aaa 100644 --- a/synapse/storage/devices.py +++ b/synapse/storage/devices.py @@ -302,6 +302,10 @@ def _mark_as_sent_devices_by_remote_txn(self, txn, destination, stream_id): @defer.inlineCallbacks def add_user_signature_change_to_streams(self, from_user_id, user_ids): """Persist that a user has made new signatures + + Args: + from_user_id (str): the user who made the signatures + user_ids (list[str]): the users who were signed """ with self._device_list_id_gen.get_next() as stream_id: @@ -466,9 +470,12 @@ def _get_users_whose_devices_changed_txn(txn): @defer.inlineCallbacks def get_users_whose_signatures_changed(self, user_id, from_key): - """Get set of users who have new cross-signing signatures have changed since + """Get the users who have new cross-signing signatures made by `user_id` since `from_key`. + Args: + user_id (str): the user who made the signatures + from_key (str): The device lists stream token """ from_key = int(from_key) if self._user_signature_stream_cache.has_entity_changed(user_id, from_key): @@ -589,6 +596,8 @@ def store_device(self, user_id, device_id, initial_device_display_name): Returns: defer.Deferred: boolean whether the device was inserted or an existing device existed with that ID. + Raises: + StoreError: if the device is already in use """ key = (user_id, device_id) if self.device_id_exists_cache.get(key, None): From 1b94f0079ec1c0c89883f966f2501dbabe38590a Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Fri, 12 Jul 2019 17:20:47 -0400 Subject: [PATCH 05/10] use updated function name for patterns --- synapse/rest/client/v2_alpha/keys.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/synapse/rest/client/v2_alpha/keys.py b/synapse/rest/client/v2_alpha/keys.py index deee772f00c4..1ec482391006 100644 --- a/synapse/rest/client/v2_alpha/keys.py +++ b/synapse/rest/client/v2_alpha/keys.py @@ -237,7 +237,7 @@ class SigningKeyUploadServlet(RestServlet): { } """ - PATTERNS = client_v2_patterns("/keys/device_signing/upload$") + PATTERNS = client_patterns("/keys/device_signing/upload$") def __init__(self, hs): """ From 723619f813789d791c9743c37228d246f9b497f0 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Fri, 12 Jul 2019 18:02:01 -0400 Subject: [PATCH 06/10] add unit tests for storing/retrieving cross-signing keys --- tests/handlers/test_e2e_keys.py | 73 +++++++++++++++++++++++++++++++++ 1 file changed, 73 insertions(+) diff --git a/tests/handlers/test_e2e_keys.py b/tests/handlers/test_e2e_keys.py index 8dccc6826eb2..28cac58a0545 100644 --- a/tests/handlers/test_e2e_keys.py +++ b/tests/handlers/test_e2e_keys.py @@ -1,5 +1,7 @@ # -*- coding: utf-8 -*- # Copyright 2016 OpenMarket Ltd +# Copyright 2019 New Vector Ltd +# Copyright 2019 The Matrix.org Foundation C.I.C. # # Licensed under the Apache License, Version 2.0 (the "License"); # you may not use this file except in compliance with the License. @@ -145,3 +147,74 @@ def test_claim_one_time_key(self): "one_time_keys": {local_user: {device_id: {"alg1:k1": "key1"}}}, }, ) + + @defer.inlineCallbacks + def test_replace_master_key(self): + """uploading a new signing key should make the old signing key unavailable""" + local_user = "@boris:" + self.hs.hostname + keys1 = { + "master_key": { + # private key: 2lonYOM6xYKdEsO+6KrC766xBcHnYnim1x/4LFGF8B0 + "user_id": local_user, + "usage": ["master"], + "keys": { + "ed25519:nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk": + "nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk" + } + } + } + yield self.handler.upload_signing_keys_for_user(local_user, keys1) + + keys2 = { + "master_key": { + # private key: 4TL4AjRYwDVwD3pqQzcor+ez/euOB1/q78aTJ+czDNs + "user_id": local_user, + "usage": ["master"], + "keys": { + "ed25519:Hq6gL+utB4ET+UvD5ci0kgAwsX6qP/zvf8v6OInU5iw": + "Hq6gL+utB4ET+UvD5ci0kgAwsX6qP/zvf8v6OInU5iw" + } + } + } + yield self.handler.upload_signing_keys_for_user(local_user, keys2) + + devices = yield self.handler.query_devices( + {"device_keys": {local_user: []}}, 0 + ) + self.assertDictEqual( + devices["master_keys"], + { + local_user: keys2["master_key"] + }, + ) + + @defer.inlineCallbacks + def test_self_signing_key_doesnt_show_up_as_device(self): + """signing keys should be hidden when fetching a user's devices""" + local_user = "@boris:" + self.hs.hostname + keys1 = { + "master_key": { + # private key: 2lonYOM6xYKdEsO+6KrC766xBcHnYnim1x/4LFGF8B0 + "user_id": local_user, + "usage": ["master"], + "keys": { + "ed25519:nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk": + "nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk" + } + } + } + yield self.handler.upload_signing_keys_for_user(local_user, keys1) + + res = None + try: + yield self.hs.get_device_handler().check_device_registered( + user_id=local_user, + device_id="nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk", + initial_device_display_name="new display name", + ) + except errors.SynapseError as e: + res = e.code + self.assertEqual(res, 400) + + res = yield self.handler.query_local_devices({local_user: None}) + self.assertDictEqual(res, {local_user: {}}) From fb888426edc66aacb15d634879070c1b63d46e50 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Fri, 12 Jul 2019 18:04:33 -0400 Subject: [PATCH 07/10] implement some review feedback --- synapse/storage/devices.py | 2 +- synapse/storage/end_to_end_keys.py | 42 ++++++++++++------- .../storage/schema/delta/55/signing_keys.sql | 27 ++++++++---- 3 files changed, 48 insertions(+), 23 deletions(-) diff --git a/synapse/storage/devices.py b/synapse/storage/devices.py index bc76a5961aaa..28f8dbaaa300 100644 --- a/synapse/storage/devices.py +++ b/synapse/storage/devices.py @@ -628,7 +628,7 @@ def store_device(self, user_id, device_id, initial_device_display_name): ) if hidden: raise StoreError( - 400, "The device ID is reserved", Codes.FORBIDDEN + 400, "The device ID is in use", Codes.FORBIDDEN ) self.device_id_exists_cache.prefill(key, True) defer.returnValue(inserted) diff --git a/synapse/storage/end_to_end_keys.py b/synapse/storage/end_to_end_keys.py index d99f434cfa64..61cee32eba68 100644 --- a/synapse/storage/end_to_end_keys.py +++ b/synapse/storage/end_to_end_keys.py @@ -286,7 +286,7 @@ def delete_e2e_keys_by_device_txn(txn): "delete_e2e_keys_by_device", delete_e2e_keys_by_device_txn ) - def _set_e2e_device_signing_key_txn(self, txn, user_id, key_type, key): + def _set_e2e_cross_signing_key_txn(self, txn, user_id, key_type, key): """Set a user's cross-signing key. Args: @@ -301,9 +301,23 @@ def _set_e2e_device_signing_key_txn(self, txn, user_id, key_type, key): # since signatures are identified by device ID. So add an entry to the # device table to make sure that we don't have a collision with device # IDs - for v in key["keys"].values(): - pubkey = v - break + + # the 'key' dict will look something like: + # { + # "user_id": "@alice:example.com", + # "usage": ["self_signing"], + # "keys": { + # "ed25519:base64+self+signing+public+key": "base64+self+signing+public+key", + # }, + # "signatures": { + # "@alice:example.com": { + # "ed25519:base64+master+public+key": "base64+signature" + # } + # } + # } + # The "keys" property must only have one entry, which will be the public + # key, so we just grab the first value in there + pubkey = next(iter(key["keys"].values())) self._simple_insert( "devices", values={ @@ -317,7 +331,7 @@ def _set_e2e_device_signing_key_txn(self, txn, user_id, key_type, key): # and finally, store the key itself self._simple_insert( - "e2e_device_signing_keys", + "e2e_cross_signing_keys", values={ "user_id": user_id, "keytype": key_type, @@ -336,12 +350,12 @@ def set_e2e_cross_signing_key(self, user_id, key_type, key): key (dict): the key data """ return self.runInteraction( - "add_e2e_device_signing_key", - self._set_e2e_device_signing_key_txn, + "add_e2e_cross_signing_key", + self._set_e2e_cross_signing_key_txn, user_id, key_type, key ) - def _get_e2e_device_signing_key_txn(self, txn, user_id, key_type, from_user_id=None): + def _get_e2e_cross_signing_key_txn(self, txn, user_id, key_type, from_user_id=None): """Returns a user's cross-signing key. Args: @@ -358,7 +372,7 @@ def _get_e2e_device_signing_key_txn(self, txn, user_id, key_type, from_user_id=N """ sql = ( "SELECT keydata " - " FROM e2e_device_signing_keys " + " FROM e2e_cross_signing_keys " " WHERE user_id = ? AND keytype = ? ORDER BY ts DESC LIMIT 1" ) txn.execute(sql, (user_id, key_type)) @@ -374,7 +388,7 @@ def _get_e2e_device_signing_key_txn(self, txn, user_id, key_type, from_user_id=N if from_user_id is not None: sql = ( "SELECT key_id, signature " - " FROM e2e_device_signatures " + " FROM e2e_cross_signing_signatures " " WHERE user_id = ? " " AND target_user_id = ? " " AND target_device_id = ? " @@ -400,12 +414,12 @@ def get_e2e_cross_signing_key(self, user_id, key_type, from_user_id=None): dict of the key data """ return self.runInteraction( - "get_e2e_device_signing_key", - self._get_e2e_device_signing_key_txn, + "get_e2e_cross_signing_key", + self._get_e2e_cross_signing_key_txn, user_id, key_type, from_user_id ) - def store_e2e_device_signatures(self, user_id, signatures): + def store_e2e_cross_signing_signatures(self, user_id, signatures): """Stores cross-signing signatures. Args: @@ -418,7 +432,7 @@ def store_e2e_device_signatures(self, user_id, signatures): is the signature of the device """ return self._simple_insert_many( - "e2e_device_signatures", + "e2e_cross_signing_signatures", [{"user_id": user_id, "key_id": key_id, "target_user_id": target_user_id, diff --git a/synapse/storage/schema/delta/55/signing_keys.sql b/synapse/storage/schema/delta/55/signing_keys.sql index 70330ac49bd2..da7ea63e27d5 100644 --- a/synapse/storage/schema/delta/55/signing_keys.sql +++ b/synapse/storage/schema/delta/55/signing_keys.sql @@ -13,35 +13,46 @@ * limitations under the License. */ --- device signing keys for cross-signing -CREATE TABLE e2e_device_signing_keys ( +-- cross-signing keys +CREATE TABLE IF NOT EXISTS e2e_cross_signing_keys ( user_id TEXT NOT NULL, + -- the type of cross-signing key (master, user_signing, or self_signing) keytype TEXT NOT NULL, + -- the full key information keydata TEXT NOT NULL, + -- time that the key was added ts BIGINT NOT NULL ); -CREATE UNIQUE INDEX e2e_device_signing_keys_idx ON e2e_device_signing_keys(user_id, keytype, ts); +CREATE UNIQUE INDEX e2e_cross_signing_keys_idx ON e2e_cross_signing_keys(user_id, keytype, ts); --- devices signatures for cross-signing -CREATE TABLE e2e_device_signatures ( +-- cross-signing signatures +CREATE TABLE IF NOT EXISTS e2e_cross_signing_signatures ( + -- user who did the signing user_id TEXT NOT NULL, + -- key used to sign key_id TEXT NOT NULL, + -- user who was signed target_user_id TEXT NOT NULL, + -- device/key that was signed target_device_id TEXT NOT NULL, + -- the actual signature signature TEXT NOT NULL ); -CREATE UNIQUE INDEX e2e_device_signatures_idx ON e2e_device_signatures(user_id, target_user_id, target_device_id); +CREATE UNIQUE INDEX e2e_cross_signing_signatures_idx ON e2e_cross_signing_signatures(user_id, target_user_id, target_device_id); -- stream of user signature updates -CREATE TABLE user_signature_stream ( +CREATE TABLE IF NOT EXISTS user_signature_stream ( + -- uses the same stream ID as device list stream stream_id BIGINT NOT NULL, + -- user who did the signing from_user_id TEXT NOT NULL, + -- list of users who were signed, as a JSON array user_ids TEXT NOT NULL ); -CREATE INDEX user_signature_stream_idx ON user_signature_stream(stream_id, from_user_id); +CREATE UNIQUE INDEX user_signature_stream_idx ON user_signature_stream(stream_id); -- device list needs to know which ones are "real" devices, and which ones are -- just used to avoid collisions From f5bad36c79bec729066fbe7a3606d2a5fab05291 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 15 Jul 2019 17:35:04 -0400 Subject: [PATCH 08/10] make hidden field nullable --- synapse/storage/devices.py | 50 ++++++++++++------- synapse/storage/end_to_end_keys.py | 3 +- .../storage/schema/delta/55/signing_keys.sql | 2 +- 3 files changed, 34 insertions(+), 21 deletions(-) diff --git a/synapse/storage/devices.py b/synapse/storage/devices.py index 28f8dbaaa300..aae765754eea 100644 --- a/synapse/storage/devices.py +++ b/synapse/storage/devices.py @@ -38,6 +38,7 @@ class DeviceWorkerStore(SQLBaseStore): + @defer.inlineCallbacks def get_device(self, user_id, device_id): """Retrieve a device. @@ -49,12 +50,15 @@ def get_device(self, user_id, device_id): Raises: StoreError: if the device is not found """ - return self._simple_select_one( + ret = yield self._simple_select_one( table="devices", - keyvalues={"user_id": user_id, "device_id": device_id, "hidden": False}, - retcols=("user_id", "device_id", "display_name"), + keyvalues={"user_id": user_id, "device_id": device_id}, + retcols=("user_id", "device_id", "display_name", "hidden"), desc="get_device", ) + if ret["hidden"]: + raise StoreError(404, "No row found (devices)") + return ret @defer.inlineCallbacks def get_devices_by_user(self, user_id): @@ -69,12 +73,12 @@ def get_devices_by_user(self, user_id): """ devices = yield self._simple_select_list( table="devices", - keyvalues={"user_id": user_id, "hidden": False}, - retcols=("user_id", "device_id", "display_name"), + keyvalues={"user_id": user_id}, + retcols=("user_id", "device_id", "display_name", "hidden"), desc="get_devices_by_user", ) - defer.returnValue({d["device_id"]: d for d in devices}) + defer.returnValue({d["device_id"]: d for d in devices if not d["hidden"]}) @defer.inlineCallbacks def get_devices_by_remote(self, destination, from_stream_id, limit): @@ -658,11 +662,11 @@ def delete_device(self, user_id, device_id): Returns: defer.Deferred """ - yield self._simple_delete_one( - table="devices", - keyvalues={"user_id": user_id, "device_id": device_id, "hidden": False}, - desc="delete_device", - ) + sql = """ + DELETE FROM devices + WHERE user_id = ? AND device_id = ? AND NOT COALESCE(hidden, ?) + """ + yield self._execute("delete_device", None, sql, user_id, device_id, False) self.device_id_exists_cache.invalidate((user_id, device_id)) @@ -676,13 +680,19 @@ def delete_devices(self, user_id, device_ids): Returns: defer.Deferred """ - yield self._simple_delete_many( - table="devices", - column="device_id", - iterable=device_ids, - keyvalues={"user_id": user_id, "hidden": False}, - desc="delete_devices", - ) + + if not device_ids or len(device_ids) == 0: + return + sql = """ + DELETE FROM devices + WHERE user_id = ? AND device_id IN (%s) AND NOT COALESCE(hidden, ?) + """ % (",".join("?" for _ in device_ids)) + values = [user_id] + values.extend(device_ids) + values.append(False) + + yield self._execute("delete_devices", None, sql, *values) + for device_id in device_ids: self.device_id_exists_cache.invalidate((user_id, device_id)) @@ -704,9 +714,11 @@ def update_device(self, user_id, device_id, new_display_name=None): updates["display_name"] = new_display_name if not updates: return defer.succeed(None) + # FIXME: should only update if hidden is not True. But updating the + # display name of a hidden device should be harmless return self._simple_update_one( table="devices", - keyvalues={"user_id": user_id, "device_id": device_id, "hidden": False}, + keyvalues={"user_id": user_id, "device_id": device_id}, updatevalues=updates, desc="update_device", ) diff --git a/synapse/storage/end_to_end_keys.py b/synapse/storage/end_to_end_keys.py index 61cee32eba68..b53b096def91 100644 --- a/synapse/storage/end_to_end_keys.py +++ b/synapse/storage/end_to_end_keys.py @@ -89,11 +89,12 @@ def _get_e2e_device_keys_txn( " k.key_json" " FROM devices d" " %s JOIN e2e_device_keys_json k USING (user_id, device_id)" - " WHERE (%s) AND d.hidden = 0" + " WHERE (%s) AND NOT COALESCE(d.hidden, ?)" ) % ( "LEFT" if include_all_devices else "INNER", " OR ".join("(" + q + ")" for q in query_clauses), ) + query_params.append(False) txn.execute(sql, query_params) rows = self.cursor_to_dict(txn) diff --git a/synapse/storage/schema/delta/55/signing_keys.sql b/synapse/storage/schema/delta/55/signing_keys.sql index da7ea63e27d5..01136ad2f9d3 100644 --- a/synapse/storage/schema/delta/55/signing_keys.sql +++ b/synapse/storage/schema/delta/55/signing_keys.sql @@ -56,4 +56,4 @@ CREATE UNIQUE INDEX user_signature_stream_idx ON user_signature_stream(stream_id -- device list needs to know which ones are "real" devices, and which ones are -- just used to avoid collisions -ALTER TABLE devices ADD COLUMN hidden BOOLEAN DEFAULT FALSE; +ALTER TABLE devices ADD COLUMN hidden BOOLEAN NULLABLE; From 8f63558f6e3558879cbd7bfcee912a4d92067d3e Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 22 Jul 2019 12:40:24 -0400 Subject: [PATCH 09/10] make black happy --- synapse/handlers/device.py | 4 +-- synapse/handlers/e2e_keys.py | 52 +++++++++++++--------------- synapse/rest/client/v2_alpha/keys.py | 7 ++-- synapse/storage/__init__.py | 2 +- synapse/storage/devices.py | 27 ++++++++------- synapse/storage/end_to_end_keys.py | 39 +++++++++++++-------- synapse/types.py | 10 ++---- tests/handlers/test_e2e_keys.py | 26 +++++--------- 8 files changed, 78 insertions(+), 89 deletions(-) diff --git a/synapse/handlers/device.py b/synapse/handlers/device.py index 4e65825f0fbf..2a8fa9c8181f 100644 --- a/synapse/handlers/device.py +++ b/synapse/handlers/device.py @@ -423,9 +423,7 @@ def notify_user_signature_update(self, from_user_id, user_ids): from_user_id, user_ids ) - self.notifier.on_new_event( - "device_list_key", position, users=[from_user_id], - ) + self.notifier.on_new_event("device_list_key", position, users=[from_user_id]) @defer.inlineCallbacks def on_federation_query_user_devices(self, user_id): diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py index 454f99524b46..1501fde9902d 100644 --- a/synapse/handlers/e2e_keys.py +++ b/synapse/handlers/e2e_keys.py @@ -227,16 +227,22 @@ def get_cross_signing_key(user_id): except Exception: pass - yield make_deferred_yieldable(defer.gatherResults([ - run_in_background(get_cross_signing_key, user_id) - for user_id in query.keys() - ])) + yield make_deferred_yieldable( + defer.gatherResults( + [ + run_in_background(get_cross_signing_key, user_id) + for user_id in query.keys() + ] + ) + ) - defer.returnValue({ - "master": master_keys, - "self_signing": self_signing_keys, - "user_signing": user_signing_keys, - }) + defer.returnValue( + { + "master": master_keys, + "self_signing": self_signing_keys, + "user_signing": user_signing_keys, + } + ) @defer.inlineCallbacks def query_local_devices(self, query): @@ -455,11 +461,7 @@ def upload_signing_keys_for_user(self, user_id, keys): # if there is no master key, then we can't do anything, because all the # other cross-signing keys need to be signed by the master key if not master_key: - raise SynapseError( - 400, - "No master key available", - Codes.MISSING_PARAM - ) + raise SynapseError(400, "No master key available", Codes.MISSING_PARAM) master_key_id, master_verify_key = get_verify_key_from_cross_signing_key( master_key @@ -484,9 +486,7 @@ def upload_signing_keys_for_user(self, user_id, keys): # if everything checks out, then store the keys and send notifications deviceids = [] if "master_key" in keys: - yield self.store.set_e2e_cross_signing_key( - user_id, "master", master_key - ) + yield self.store.set_e2e_cross_signing_key(user_id, "master", master_key) deviceids.append(master_verify_key.version) if "self_signing_key" in keys: yield self.store.set_e2e_cross_signing_key( @@ -523,22 +523,20 @@ def _check_cross_signing_key(key, user_id, key_type, signing_key=None): signing_key (VerifyKey): (optional) the signing key that the key should be signed with. If omitted, signatures will not be checked. """ - if "user_id" not in key or key["user_id"] != user_id \ - or "usage" not in key or key_type not in key["usage"]: - raise SynapseError( - 400, - ("Invalid %s key" % key_type), - Codes.INVALID_PARAM - ) + if ( + "user_id" not in key + or key["user_id"] != user_id + or "usage" not in key + or key_type not in key["usage"] + ): + raise SynapseError(400, ("Invalid %s key" % key_type), Codes.INVALID_PARAM) if signing_key: try: verify_signed_json(key, user_id, signing_key) except SignatureVerifyException: raise SynapseError( - 400, - ("Invalid signature or %s key" % key_type), - Codes.INVALID_SIGNATURE + 400, ("Invalid signature or %s key" % key_type), Codes.INVALID_SIGNATURE ) diff --git a/synapse/rest/client/v2_alpha/keys.py b/synapse/rest/client/v2_alpha/keys.py index 1ec482391006..29825a20812c 100644 --- a/synapse/rest/client/v2_alpha/keys.py +++ b/synapse/rest/client/v2_alpha/keys.py @@ -237,6 +237,7 @@ class SigningKeyUploadServlet(RestServlet): { } """ + PATTERNS = client_patterns("/keys/device_signing/upload$") def __init__(self, hs): @@ -258,12 +259,10 @@ def on_POST(self, request): body = parse_json_object_from_request(request) yield self.auth_handler.validate_user_via_ui_auth( - requester, body, self.hs.get_ip_from_request(request), + requester, body, self.hs.get_ip_from_request(request) ) - result = yield self.e2e_keys_handler.upload_signing_keys_for_user( - user_id, body - ) + result = yield self.e2e_keys_handler.upload_signing_keys_for_user(user_id, body) defer.returnValue((200, result)) diff --git a/synapse/storage/__init__.py b/synapse/storage/__init__.py index bad709171bd6..c20ba1001c73 100644 --- a/synapse/storage/__init__.py +++ b/synapse/storage/__init__.py @@ -208,7 +208,7 @@ def __init__(self, db_conn, hs): "DeviceListStreamChangeCache", device_list_max ) self._user_signature_stream_cache = StreamChangeCache( - "UserSignatureStreamChangeCache", device_list_max, + "UserSignatureStreamChangeCache", device_list_max ) self._device_list_federation_stream_cache = StreamChangeCache( "DeviceListFederationStreamChangeCache", device_list_max diff --git a/synapse/storage/devices.py b/synapse/storage/devices.py index aae765754eea..138ced7d7b4a 100644 --- a/synapse/storage/devices.py +++ b/synapse/storage/devices.py @@ -314,15 +314,19 @@ def add_user_signature_change_to_streams(self, from_user_id, user_ids): with self._device_list_id_gen.get_next() as stream_id: yield self.runInteraction( - "add_user_sig_change_to_streams", self._add_user_signature_change_txn, - from_user_id, user_ids, stream_id, + "add_user_sig_change_to_streams", + self._add_user_signature_change_txn, + from_user_id, + user_ids, + stream_id, ) defer.returnValue(stream_id) def _add_user_signature_change_txn(self, txn, from_user_id, user_ids, stream_id): txn.call_after( self._user_signature_stream_cache.entity_has_changed, - from_user_id, stream_id, + from_user_id, + stream_id, ) self._simple_insert_txn( txn, @@ -614,7 +618,7 @@ def store_device(self, user_id, device_id, initial_device_display_name): "user_id": user_id, "device_id": device_id, "display_name": initial_device_display_name, - "hidden": False + "hidden": False, }, desc="store_device", or_ignore=True, @@ -624,16 +628,11 @@ def store_device(self, user_id, device_id, initial_device_display_name): # if the device ID is reserved by something else hidden = yield self._simple_select_one_onecol( "devices", - keyvalues={ - "user_id": user_id, - "device_id": device_id - }, - retcol="hidden" + keyvalues={"user_id": user_id, "device_id": device_id}, + retcol="hidden", ) if hidden: - raise StoreError( - 400, "The device ID is in use", Codes.FORBIDDEN - ) + raise StoreError(400, "The device ID is in use", Codes.FORBIDDEN) self.device_id_exists_cache.prefill(key, True) defer.returnValue(inserted) except StoreError as e: @@ -686,7 +685,9 @@ def delete_devices(self, user_id, device_ids): sql = """ DELETE FROM devices WHERE user_id = ? AND device_id IN (%s) AND NOT COALESCE(hidden, ?) - """ % (",".join("?" for _ in device_ids)) + """ % ( + ",".join("?" for _ in device_ids) + ) values = [user_id] values.extend(device_ids) values.append(False) diff --git a/synapse/storage/end_to_end_keys.py b/synapse/storage/end_to_end_keys.py index b53b096def91..f0bf7ab65410 100644 --- a/synapse/storage/end_to_end_keys.py +++ b/synapse/storage/end_to_end_keys.py @@ -325,9 +325,9 @@ def _set_e2e_cross_signing_key_txn(self, txn, user_id, key_type, key): "user_id": user_id, "device_id": pubkey, "display_name": key_type + " signing key", - "hidden": True + "hidden": True, }, - desc="store_master_key_device" + desc="store_master_key_device", ) # and finally, store the key itself @@ -337,9 +337,9 @@ def _set_e2e_cross_signing_key_txn(self, txn, user_id, key_type, key): "user_id": user_id, "keytype": key_type, "keydata": json.dumps(key), - "ts": time.time() * 1000 + "ts": time.time() * 1000, }, - desc="store_master_key" + desc="store_master_key", ) def set_e2e_cross_signing_key(self, user_id, key_type, key): @@ -353,7 +353,9 @@ def set_e2e_cross_signing_key(self, user_id, key_type, key): return self.runInteraction( "add_e2e_cross_signing_key", self._set_e2e_cross_signing_key_txn, - user_id, key_type, key + user_id, + key_type, + key, ) def _get_e2e_cross_signing_key_txn(self, txn, user_id, key_type, from_user_id=None): @@ -397,8 +399,9 @@ def _get_e2e_cross_signing_key_txn(self, txn, user_id, key_type, from_user_id=No txn.execute(sql, (from_user_id, user_id, device_id)) row = txn.fetchone() if row: - key.setdefault("signatures", {}) \ - .setdefault(from_user_id, {})[row[0]] = row[1] + key.setdefault("signatures", {}).setdefault(from_user_id, {})[ + row[0] + ] = row[1] return key @@ -417,7 +420,9 @@ def get_e2e_cross_signing_key(self, user_id, key_type, from_user_id=None): return self.runInteraction( "get_e2e_cross_signing_key", self._get_e2e_cross_signing_key_txn, - user_id, key_type, from_user_id + user_id, + key_type, + from_user_id, ) def store_e2e_cross_signing_signatures(self, user_id, signatures): @@ -434,11 +439,15 @@ def store_e2e_cross_signing_signatures(self, user_id, signatures): """ return self._simple_insert_many( "e2e_cross_signing_signatures", - [{"user_id": user_id, - "key_id": key_id, - "target_user_id": target_user_id, - "target_device_id": target_device_id, - "signature": signature} - for (key_id, target_user_id, target_device_id, signature) in signatures], - "add_e2e_signing_key" + [ + { + "user_id": user_id, + "key_id": key_id, + "target_user_id": target_user_id, + "target_device_id": target_device_id, + "signature": signature, + } + for (key_id, target_user_id, target_device_id, signature) in signatures + ], + "add_e2e_signing_key", ) diff --git a/synapse/types.py b/synapse/types.py index ce886d8ac4fc..7a80471a0c72 100644 --- a/synapse/types.py +++ b/synapse/types.py @@ -492,16 +492,10 @@ def get_verify_key_from_cross_signing_key(key_info): """ # make sure that exactly one key is provided if "keys" not in key_info: - raise SynapseError( - 400, - "Invalid key" - ) + raise SynapseError(400, "Invalid key") keys = key_info["keys"] if len(keys) != 1: - raise SynapseError( - 400, - "Invalid key" - ) + raise SynapseError(400, "Invalid key") # and return that one key for key_id, key_data in keys.items(): return (key_id, decode_verify_key_bytes(key_id, decode_base64(key_data))) diff --git a/tests/handlers/test_e2e_keys.py b/tests/handlers/test_e2e_keys.py index 28cac58a0545..211e7357d6fa 100644 --- a/tests/handlers/test_e2e_keys.py +++ b/tests/handlers/test_e2e_keys.py @@ -158,9 +158,8 @@ def test_replace_master_key(self): "user_id": local_user, "usage": ["master"], "keys": { - "ed25519:nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk": - "nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk" - } + "ed25519:nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk": "nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk" + }, } } yield self.handler.upload_signing_keys_for_user(local_user, keys1) @@ -171,22 +170,14 @@ def test_replace_master_key(self): "user_id": local_user, "usage": ["master"], "keys": { - "ed25519:Hq6gL+utB4ET+UvD5ci0kgAwsX6qP/zvf8v6OInU5iw": - "Hq6gL+utB4ET+UvD5ci0kgAwsX6qP/zvf8v6OInU5iw" - } + "ed25519:Hq6gL+utB4ET+UvD5ci0kgAwsX6qP/zvf8v6OInU5iw": "Hq6gL+utB4ET+UvD5ci0kgAwsX6qP/zvf8v6OInU5iw" + }, } } yield self.handler.upload_signing_keys_for_user(local_user, keys2) - devices = yield self.handler.query_devices( - {"device_keys": {local_user: []}}, 0 - ) - self.assertDictEqual( - devices["master_keys"], - { - local_user: keys2["master_key"] - }, - ) + devices = yield self.handler.query_devices({"device_keys": {local_user: []}}, 0) + self.assertDictEqual(devices["master_keys"], {local_user: keys2["master_key"]}) @defer.inlineCallbacks def test_self_signing_key_doesnt_show_up_as_device(self): @@ -198,9 +189,8 @@ def test_self_signing_key_doesnt_show_up_as_device(self): "user_id": local_user, "usage": ["master"], "keys": { - "ed25519:nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk": - "nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk" - } + "ed25519:nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk": "nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk" + }, } } yield self.handler.upload_signing_keys_for_user(local_user, keys1) From c44c684e317a1981d00a9311311c951c0821d451 Mon Sep 17 00:00:00 2001 From: Hubert Chathi Date: Mon, 22 Jul 2019 12:46:21 -0400 Subject: [PATCH 10/10] make flake8 happy --- synapse/storage/devices.py | 1 - 1 file changed, 1 deletion(-) diff --git a/synapse/storage/devices.py b/synapse/storage/devices.py index 138ced7d7b4a..09d578940255 100644 --- a/synapse/storage/devices.py +++ b/synapse/storage/devices.py @@ -26,7 +26,6 @@ from synapse.metrics.background_process_metrics import run_as_background_process from synapse.storage._base import Cache, SQLBaseStore, db_to_json from synapse.storage.background_updates import BackgroundUpdateStore -from synapse.types import get_verify_key_from_cross_signing_key from synapse.util import batch_iter from synapse.util.caches.descriptors import cached, cachedInlineCallbacks, cachedList