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

Don't user upsert to persist new one time keys #2053

Merged
merged 6 commits into from
Mar 29, 2017
Merged
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 50 additions & 9 deletions synapse/storage/end_to_end_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down Expand Up @@ -120,24 +122,63 @@ def _get_e2e_device_keys_txn(self, txn, query_list, include_all_devices):

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",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically the algorithm is part of the key - so it's valid to have an "ed25519:aaaaa" as well as a "rot13:aaaaa". I rather hope the index reflects that, but haven't checked...

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh bleurgh, thanks, have changed.

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 SynapseError(
400, "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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

... and throw a 500 or something, presumably? I suppose that's fine because the client should retry on the 500 and hit the api again, at which point the lookup should conclude that it's all fine.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup.

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):
Expand Down