-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Support MSC3814: Dehydrated Devices Part 2 #16010
Changes from 7 commits
ac4f3fb
ceed144
8a7db88
217e2eb
61818c8
ec98b6a
ef2099d
8ca4c1f
c3cb646
f14c51d
9e3de14
4357f5b
cdc9859
c5f4357
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Update dehydrated devices implementation. | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,6 +26,8 @@ | |
Tuple, | ||
) | ||
|
||
from canonicaljson import encode_canonical_json | ||
|
||
from synapse.api import errors | ||
from synapse.api.constants import EduTypes, EventTypes | ||
from synapse.api.errors import ( | ||
|
@@ -385,6 +387,7 @@ def __init__(self, hs: "HomeServer"): | |
self.federation_sender = hs.get_federation_sender() | ||
self._account_data_handler = hs.get_account_data_handler() | ||
self._storage_controllers = hs.get_storage_controllers() | ||
self.db_pool = hs.get_datastores().main.db_pool | ||
|
||
self.device_list_updater = DeviceListUpdater(hs, self) | ||
|
||
|
@@ -656,15 +659,17 @@ async def store_dehydrated_device( | |
device_id: Optional[str], | ||
device_data: JsonDict, | ||
initial_device_display_name: Optional[str] = None, | ||
keys_for_device: Optional[JsonDict] = None, | ||
) -> str: | ||
"""Store a dehydrated device for a user. If the user had a previous | ||
dehydrated device, it is removed. | ||
"""Store a dehydrated device for a user, optionally storing the keys associated with | ||
it as well. If the user had a previous dehydrated device, it is removed. | ||
|
||
Args: | ||
user_id: the user that we are storing the device for | ||
device_id: device id supplied by client | ||
device_data: the dehydrated device information | ||
initial_device_display_name: The display name to use for the device | ||
keys_for_device: keys for the dehydrated device | ||
Returns: | ||
device id of the dehydrated device | ||
""" | ||
|
@@ -673,13 +678,135 @@ async def store_dehydrated_device( | |
device_id, | ||
initial_device_display_name, | ||
) | ||
old_device_id = await self.store.store_dehydrated_device( | ||
user_id, device_id, device_data | ||
) | ||
|
||
time_now = self.clock.time_msec() | ||
|
||
if keys_for_device: | ||
keys = await self._check_and_prepare_keys_for_dehydrated_device( | ||
user_id, device_id, keys_for_device | ||
) | ||
old_device_id = await self.store.store_dehydrated_device( | ||
user_id, device_id, device_data, time_now, keys | ||
) | ||
else: | ||
old_device_id = await self.store.store_dehydrated_device( | ||
user_id, device_id, device_data, time_now | ||
) | ||
H-Shay marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
if old_device_id is not None: | ||
await self.delete_devices(user_id, [old_device_id]) | ||
|
||
return device_id | ||
|
||
async def _check_and_prepare_keys_for_dehydrated_device( | ||
self, user_id: str, device_id: str, keys: JsonDict | ||
) -> dict: | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Aren't we creating the dehydrated device now? How can it already have keys? Aren't devices unique per user/device ID? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is creating/storing the dehydrated device - to your question of how it can already have keys I actually don't know - per the MSC the keys were to be uploaded over the #15929 changed this so that the key upload was integrated into the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Uploading the public keys at the time of the dehydrated device creation tries to address this race: matrix-org/matrix-spec-proposals#3814 (comment). As to how can it already have keys, well if we remember what a dehydrated device is then it becomes quite clear. A dehydrated device are the private identity and one-time keys of a device, of course they are encrypted so the server can't access them. It becomes quite natural, that we would like to upload the public parts of those same keys to the server at the same time, once we realize this. I think that doing it in a separate There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OK, but part of this function checks if we already have keys uploaded for this device (and bails if they're not exactly equal to the new keys). I still don't follow the order of events that would let you upload keys before the device is created. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This could be me being overzealous here - I basically made sure that all the checks that happen in the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Part of it would be straightforward (Move Although it seems this entire function is just a copy of There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I think if this is the case we can probably call the store methods that set the values directly and avoid the complicated logic of reusing keys. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
so the reason I didn't do this was that my understanding of what was being asked was that the storing of the keys and the storing of the device needs to all happen in the same transaction - ie in There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. They would need to be refactored to take a transaction as the first parameter and current callers would call that also. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right I think this is sorted now, sorry for the confusion. |
||
""" | ||
Check if any of the provided keys are duplicate and raise if they are, | ||
prepare keys for insertion in DB | ||
|
||
Args: | ||
user_id: user to store keys for | ||
device_id: the dehydrated device to store keys for | ||
keys: the keys - device_keys, onetime_keys, or fallback keys to store | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
Returns: | ||
keys that have been checked for duplicates and are ready to be inserted into | ||
DB | ||
""" | ||
keys_to_return: dict = {} | ||
device_keys = keys.get("device_keys", None) | ||
if device_keys: | ||
clokep marked this conversation as resolved.
Show resolved
Hide resolved
|
||
old_key_json = await self.db_pool.simple_select_one_onecol( | ||
table="e2e_device_keys_json", | ||
keyvalues={"user_id": user_id, "device_id": device_id}, | ||
retcol="key_json", | ||
allow_none=True, | ||
) | ||
|
||
# In py3 we need old_key_json to match new_key_json type. The DB | ||
# returns unicode while encode_canonical_json returns bytes. | ||
H-Shay marked this conversation as resolved.
Show resolved
Hide resolved
|
||
new_device_key_json = encode_canonical_json(device_keys).decode("utf-8") | ||
|
||
if old_key_json == new_device_key_json: | ||
raise SynapseError( | ||
400, | ||
f"Device key for user_id: {user_id}, device_id {device_id} already stored.", | ||
) | ||
|
||
keys_to_return["device_keys"] = new_device_key_json | ||
|
||
one_time_keys = keys.get("one_time_keys", None) | ||
if one_time_keys: | ||
# import this here to avoid a circular import | ||
from synapse.handlers.e2e_keys import _one_time_keys_match | ||
|
||
# make a list of (alg, id, key) tuples | ||
key_list = [] | ||
for key_id, key_obj in one_time_keys.items(): | ||
algorithm, key_id = key_id.split(":") | ||
key_list.append((algorithm, key_id, key_obj)) | ||
|
||
# First we check if we have already persisted any of the keys. | ||
existing_key_map = await self.store.get_e2e_one_time_keys( | ||
user_id, device_id, [k_id for _, k_id, _ in key_list] | ||
) | ||
|
||
new_one_time_keys = ( | ||
[] | ||
) # Keys that we need to insert. (alg, id, json) tuples. | ||
for algorithm, key_id, key in key_list: | ||
ex_json = existing_key_map.get((algorithm, key_id), None) | ||
if ex_json: | ||
if not _one_time_keys_match(ex_json, key): | ||
raise SynapseError( | ||
400, | ||
( | ||
"One time key %s:%s already exists. " | ||
"Old key: %s; new key: %r" | ||
) | ||
% (algorithm, key_id, ex_json, key), | ||
) | ||
else: | ||
new_one_time_keys.append( | ||
(algorithm, key_id, encode_canonical_json(key).decode("ascii")) | ||
) | ||
keys_to_return["one_time_keys"] = new_one_time_keys | ||
|
||
fallback_keys = keys.get("fallback_keys", None) | ||
if fallback_keys: | ||
new_fallback_keys = {} | ||
# there should actually only be one item in the dict but we iterate nevertheless - | ||
# see _set_e2e_fallback_keys_txn | ||
for key_id, fallback_key in fallback_keys.items(): | ||
algorithm, key_id = key_id.split(":", 1) | ||
old_key_json = await self.db_pool.simple_select_one_onecol( | ||
table="e2e_fallback_keys_json", | ||
keyvalues={ | ||
"user_id": user_id, | ||
"device_id": device_id, | ||
"algorithm": algorithm, | ||
}, | ||
retcol="key_json", | ||
allow_none=True, | ||
) | ||
|
||
new_fallback_key_json = encode_canonical_json(fallback_key).decode( | ||
"utf-8" | ||
) | ||
|
||
# If the uploaded key is the same as the current fallback key, | ||
# don't do anything. This prevents marking the key as unused if it | ||
# was already used. | ||
if old_key_json == new_fallback_key_json: | ||
raise SynapseError( | ||
400, f"Fallback key {old_key_json} already exists." | ||
) | ||
# TODO: should this be an update? it assumes that there will only be one fallback key | ||
new_fallback_keys[f"{algorithm}:{key_id}"] = fallback_key | ||
keys_to_return["fallback_keys"] = new_fallback_keys | ||
return keys_to_return | ||
|
||
async def rehydrate_device( | ||
self, user_id: str, access_token: str, device_id: str | ||
) -> dict: | ||
|
H-Shay marked this conversation as resolved.
Show resolved
Hide resolved
|
Uh oh!
There was an error while loading. Please reload this page.