From e64655c25de0b3797ace51239203792a15766949 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 23 Mar 2017 13:17:00 +0000 Subject: [PATCH 1/5] Don't user upsert to persist new one time keys Instead we no-op duplicate one time key uploads, an error if the key_id already exists but encodes a different key. --- synapse/storage/end_to_end_keys.py | 57 +++++++++++++++++++++++++----- 1 file changed, 48 insertions(+), 9 deletions(-) diff --git a/synapse/storage/end_to_end_keys.py b/synapse/storage/end_to_end_keys.py index b9f1365f9..77bb9b065 100644 --- a/synapse/storage/end_to_end_keys.py +++ b/synapse/storage/end_to_end_keys.py @@ -120,24 +120,63 @@ class EndToEndKeyStore(SQLBaseStore): return result + @defer.inlineCallbacks def add_e2e_one_time_keys(self, user_id, device_id, time_now, key_list): + """Insert some new one time keys for a device. + + Checks if any of the keys are already inserted, if they are then check + if they match. If they don't then we raise an error. + """ + + # First we check if we have already persisted any of the keys. + rows = yield self._simple_select_many_batch( + table="e2e_one_time_keys_json", + column="key_id", + iterable=[key_id for _, key_id, _ in key_list], + retcols=("algorithm", "key_id", "key_json",), + keyvalues={ + "user_id": user_id, + "device_id": device_id, + }, + desc="add_e2e_one_time_keys_check", + ) + + existing_key_map = { + row["key_id"]: (row["algorithm"], row["key_json"]) for row in rows + } + + new_keys = [] # Keys that we need to insert + for algorithm, key_id, json_bytes in key_list: + if key_id in existing_key_map: + ex_algo, ex_bytes = existing_key_map[key_id] + if algorithm != ex_algo or json_bytes != ex_bytes: + raise Exception( + "One time key with key_id %r already exists" % (key_id,) + ) + else: + new_keys.append((algorithm, key_id, json_bytes)) + def _add_e2e_one_time_keys(txn): - for (algorithm, key_id, json_bytes) in key_list: - self._simple_upsert_txn( - txn, table="e2e_one_time_keys_json", - keyvalues={ + # We are protected from race between lookup and insertion due to + # a unique constraint. If there is a race of two calls to + # `add_e2e_one_time_keys` then they'll conflict and we will only + # insert one set. + self._simple_insert_many_txn( + txn, table="e2e_one_time_keys_json", + values=[ + { "user_id": user_id, "device_id": device_id, "algorithm": algorithm, "key_id": key_id, - }, - values={ "ts_added_ms": time_now, "key_json": json_bytes, } - ) - return self.runInteraction( - "add_e2e_one_time_keys", _add_e2e_one_time_keys + for algorithm, key_id, json_bytes in new_keys + ], + ) + yield self.runInteraction( + "add_e2e_one_time_keys_insert", _add_e2e_one_time_keys ) def count_e2e_one_time_keys(self, user_id, device_id): From 6ebe2d23b1a366199e585dda671c9086c145e6b9 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Thu, 23 Mar 2017 13:48:30 +0000 Subject: [PATCH 2/5] Raise a more helpful exception --- synapse/storage/end_to_end_keys.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/synapse/storage/end_to_end_keys.py b/synapse/storage/end_to_end_keys.py index 77bb9b065..85340746c 100644 --- a/synapse/storage/end_to_end_keys.py +++ b/synapse/storage/end_to_end_keys.py @@ -14,6 +14,8 @@ # limitations under the License. from twisted.internet import defer +from synapse.api.errors import SynapseError + from canonicaljson import encode_canonical_json import ujson as json @@ -150,8 +152,8 @@ class EndToEndKeyStore(SQLBaseStore): if key_id in existing_key_map: ex_algo, ex_bytes = existing_key_map[key_id] if algorithm != ex_algo or json_bytes != ex_bytes: - raise Exception( - "One time key with key_id %r already exists" % (key_id,) + raise SynapseError( + 400, "One time key with key_id %r already exists" % (key_id,) ) else: new_keys.append((algorithm, key_id, json_bytes)) From 58a35366be4e9a56c6655972af5f26462a867f36 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Tue, 28 Mar 2017 11:34:37 +0100 Subject: [PATCH 3/5] The algorithm is part of the key id --- synapse/storage/end_to_end_keys.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/synapse/storage/end_to_end_keys.py b/synapse/storage/end_to_end_keys.py index 85340746c..b746f7426 100644 --- a/synapse/storage/end_to_end_keys.py +++ b/synapse/storage/end_to_end_keys.py @@ -144,14 +144,14 @@ class EndToEndKeyStore(SQLBaseStore): ) existing_key_map = { - row["key_id"]: (row["algorithm"], row["key_json"]) for row in rows + (row["algorithm"], row["key_id"]): row["key_json"] for row in rows } new_keys = [] # Keys that we need to insert for algorithm, key_id, json_bytes in key_list: - if key_id in existing_key_map: - ex_algo, ex_bytes = existing_key_map[key_id] - if algorithm != ex_algo or json_bytes != ex_bytes: + if (algorithm, key_id) in existing_key_map: + ex_bytes = existing_key_map[key_id] + if json_bytes != ex_bytes: raise SynapseError( 400, "One time key with key_id %r already exists" % (key_id,) ) From ac6bc5551204467e30d6544bd9ab520b71695687 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 29 Mar 2017 10:56:26 +0100 Subject: [PATCH 4/5] Correctly look up key --- synapse/storage/end_to_end_keys.py | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/synapse/storage/end_to_end_keys.py b/synapse/storage/end_to_end_keys.py index b746f7426..c4020869f 100644 --- a/synapse/storage/end_to_end_keys.py +++ b/synapse/storage/end_to_end_keys.py @@ -149,12 +149,11 @@ class EndToEndKeyStore(SQLBaseStore): new_keys = [] # Keys that we need to insert for algorithm, key_id, json_bytes in key_list: - if (algorithm, key_id) in existing_key_map: - ex_bytes = existing_key_map[key_id] - if json_bytes != ex_bytes: - raise SynapseError( - 400, "One time key with key_id %r already exists" % (key_id,) - ) + ex_bytes = existing_key_map.get((algorithm, key_id), None) + if ex_bytes and json_bytes != ex_bytes: + raise SynapseError( + 400, "One time key with key_id %r already exists" % (key_id,) + ) else: new_keys.append((algorithm, key_id, json_bytes)) From e4df0e189dea7f719c019c562ca811be90e2a5b1 Mon Sep 17 00:00:00 2001 From: Erik Johnston Date: Wed, 29 Mar 2017 11:02:35 +0100 Subject: [PATCH 5/5] Decrank last commit --- synapse/storage/end_to_end_keys.py | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/synapse/storage/end_to_end_keys.py b/synapse/storage/end_to_end_keys.py index 1fafeae3f..7cbc1470f 100644 --- a/synapse/storage/end_to_end_keys.py +++ b/synapse/storage/end_to_end_keys.py @@ -150,10 +150,11 @@ class EndToEndKeyStore(SQLBaseStore): new_keys = [] # Keys that we need to insert for algorithm, key_id, json_bytes in key_list: ex_bytes = existing_key_map.get((algorithm, key_id), None) - if ex_bytes and json_bytes != ex_bytes: - raise SynapseError( - 400, "One time key with key_id %r already exists" % (key_id,) - ) + if ex_bytes: + if json_bytes != ex_bytes: + raise SynapseError( + 400, "One time key with key_id %r already exists" % (key_id,) + ) else: new_keys.append((algorithm, key_id, json_bytes))