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

Always communicate device OTK counts to clients #10485

Merged
merged 6 commits into from
Jul 27, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 1 addition & 0 deletions changelog.d/10485.bugfix
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Fix a long-standing bug where Synapse would not inform clients that a device had exhausted its one-time-key pool, potentially causing problems decrypting events.
8 changes: 8 additions & 0 deletions synapse/api/constants.py
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,14 @@ class ToDeviceEventTypes:
RoomKeyRequest = "m.room_key_request"


class DeviceKeyAlgorithms:
"""Spec'd algorithms for the generation of per-device keys"""

ED25519 = "ed25519"
CURVE25519 = "curve25519"
SIGNED_CURVE25519 = "signed_curve25519"


class EduTypes:
Presence = "m.presence"

Expand Down
4 changes: 4 additions & 0 deletions synapse/handlers/sync.py
Original file line number Diff line number Diff line change
Expand Up @@ -1093,6 +1093,10 @@ async def generate_sync_result(
one_time_key_counts: JsonDict = {}
unused_fallback_key_types: List[str] = []
if device_id:
# TODO: We should have a way to let clients differentiate between the states of:
# * no change in OTK count since the provided since token
# * the server has zero OTKs left for this device
# Spec issue: https://github.com/matrix-org/matrix-doc/issues/3298
one_time_key_counts = await self.store.count_e2e_one_time_keys(
user_id, device_id
)
Expand Down
9 changes: 8 additions & 1 deletion synapse/storage/databases/main/end_to_end_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@

from twisted.enterprise.adbapi import Connection

from synapse.api.constants import DeviceKeyAlgorithms
from synapse.logging.opentracing import log_kv, set_tag, trace
from synapse.storage._base import SQLBaseStore, db_to_json
from synapse.storage.database import DatabasePool, make_in_list_sql_clause
Expand Down Expand Up @@ -381,9 +382,15 @@ def _count_e2e_one_time_keys(txn):
" GROUP BY algorithm"
)
txn.execute(sql, (user_id, device_id))
result = {}

# Initially set the key count to 0. This ensures that the client will always
# receive *some count*, even if it's 0.
result = {DeviceKeyAlgorithms.SIGNED_CURVE25519: 0}

# Override entries with the count of any keys we pulled from the database
for algorithm, key_count in txn:
result[algorithm] = key_count

return result

return await self.db_pool.runInteraction(
Expand Down
20 changes: 15 additions & 5 deletions tests/handlers/test_e2e_keys.py
Original file line number Diff line number Diff line change
Expand Up @@ -47,12 +47,16 @@ def test_reupload_one_time_keys(self):
"alg2:k3": {"key": "key3"},
}

# Note that "signed_curve25519" is always returned in key count responses. This is necessary until
# https://github.com/matrix-org/matrix-doc/issues/3298 is fixed.
res = self.get_success(
self.handler.upload_keys_for_user(
local_user, device_id, {"one_time_keys": keys}
)
)
self.assertDictEqual(res, {"one_time_key_counts": {"alg1": 1, "alg2": 2}})
self.assertDictEqual(
res, {"one_time_key_counts": {"alg1": 1, "alg2": 2, "signed_curve25519": 0}}
)

# we should be able to change the signature without a problem
keys["alg2:k2"]["signatures"]["k1"] = "sig2"
Expand All @@ -61,7 +65,9 @@ def test_reupload_one_time_keys(self):
local_user, device_id, {"one_time_keys": keys}
)
)
self.assertDictEqual(res, {"one_time_key_counts": {"alg1": 1, "alg2": 2}})
self.assertDictEqual(
res, {"one_time_key_counts": {"alg1": 1, "alg2": 2, "signed_curve25519": 0}}
)

def test_change_one_time_keys(self):
"""attempts to change one-time-keys should be rejected"""
Expand All @@ -79,7 +85,9 @@ def test_change_one_time_keys(self):
local_user, device_id, {"one_time_keys": keys}
)
)
self.assertDictEqual(res, {"one_time_key_counts": {"alg1": 1, "alg2": 2}})
self.assertDictEqual(
res, {"one_time_key_counts": {"alg1": 1, "alg2": 2, "signed_curve25519": 0}}
)

# Error when changing string key
self.get_failure(
Expand All @@ -89,7 +97,7 @@ def test_change_one_time_keys(self):
SynapseError,
)

# Error when replacing dict key with strin
# Error when replacing dict key with string
self.get_failure(
self.handler.upload_keys_for_user(
local_user, device_id, {"one_time_keys": {"alg2:k3": "key2"}}
Expand Down Expand Up @@ -131,7 +139,9 @@ def test_claim_one_time_key(self):
local_user, device_id, {"one_time_keys": keys}
)
)
self.assertDictEqual(res, {"one_time_key_counts": {"alg1": 1}})
self.assertDictEqual(
res, {"one_time_key_counts": {"alg1": 1, "signed_curve25519": 0}}
)

res2 = self.get_success(
self.handler.claim_one_time_keys(
Expand Down