diff --git a/changelog.d/19915.feature b/changelog.d/19915.feature new file mode 100644 index 00000000000..39446ae6a19 --- /dev/null +++ b/changelog.d/19915.feature @@ -0,0 +1 @@ +Don't validate signatures with unknown algorithms for master keys, and allow updates to signatures. \ No newline at end of file diff --git a/synapse/handlers/e2e_keys.py b/synapse/handlers/e2e_keys.py index 64f705a3dad..6284f75319c 100644 --- a/synapse/handlers/e2e_keys.py +++ b/synapse/handlers/e2e_keys.py @@ -1272,21 +1272,27 @@ def _check_master_key_signature( master_key_signature_list = [] sigs = signed_master_key["signatures"] for signing_key_id, signature in sigs[user_id].items(): - _, signing_device_id = signing_key_id.split(":", 1) - if ( - signing_device_id not in devices - or signing_key_id not in devices[signing_device_id]["keys"] - ): - # signed by an unknown device, or the - # device does not have the key - raise SynapseError(400, "Invalid signature", Codes.INVALID_SIGNATURE) - - # get the key and check the signature - pubkey = devices[signing_device_id]["keys"][signing_key_id] - verify_key = decode_verify_key_bytes(signing_key_id, decode_base64(pubkey)) - _check_device_signature( - user_id, verify_key, signed_master_key, stored_master_key - ) + algorithm, signing_device_id = signing_key_id.split(":", 1) + # we only check the signature for known algorithms + if algorithm == "ed25519": + if ( + signing_device_id not in devices + or signing_key_id not in devices[signing_device_id]["keys"] + ): + # signed by an unknown device, or the + # device does not have the key + raise SynapseError( + 400, "Invalid signature", Codes.INVALID_SIGNATURE + ) + + # get the key and check the signature + pubkey = devices[signing_device_id]["keys"][signing_key_id] + verify_key = decode_verify_key_bytes( + signing_key_id, decode_base64(pubkey) + ) + _check_device_signature( + user_id, verify_key, signed_master_key, stored_master_key + ) master_key_signature_list.append( SignatureListItem(signing_key_id, user_id, master_key_id, signature) diff --git a/synapse/storage/databases/main/end_to_end_keys.py b/synapse/storage/databases/main/end_to_end_keys.py index c93ebd3dda2..47ffcecc1a8 100644 --- a/synapse/storage/databases/main/end_to_end_keys.py +++ b/synapse/storage/databases/main/end_to_end_keys.py @@ -101,6 +101,15 @@ def __init__( columns=("user_id", "device_id", "algorithm", "ts_added_ms"), ) + self.db_pool.updates.register_background_index_update( + update_name="e2e_cross_signing_signatures_add_key_id_to_index", + index_name="e2e_cross_signing_signatures3_idx", + table="e2e_cross_signing_signatures", + columns=("user_id", "target_user_id", "target_device_id", "key_id"), + unique=True, + replaces_index="e2e_cross_signing_signatures2_idx", + ) + class EndToEndKeyWorkerStore(EndToEndKeyBackgroundStore, CacheInvalidationWorkerStore): def __init__( @@ -1821,26 +1830,21 @@ def _store_e2e_cross_signing_signatures( txn: LoggingTransaction, signatures: "Iterable[SignatureListItem]", ) -> None: - self.db_pool.simple_insert_many_txn( + self.db_pool.simple_upsert_many_txn( txn, "e2e_cross_signing_signatures", - keys=( - "user_id", - "key_id", - "target_user_id", - "target_device_id", - "signature", - ), - values=[ + key_names=("user_id", "key_id", "target_user_id", "target_device_id"), + key_values=[ ( user_id, item.signing_key_id, item.target_user_id, item.target_device_id, - item.signature, ) for item in signatures ], + value_names=("signature",), + value_values=[(item.signature,) for item in signatures], ) to_invalidate = [ diff --git a/synapse/storage/schema/main/delta/94/06_e2e_cross_signing_signatures_index.sql b/synapse/storage/schema/main/delta/94/06_e2e_cross_signing_signatures_index.sql new file mode 100644 index 00000000000..4a2eae812e5 --- /dev/null +++ b/synapse/storage/schema/main/delta/94/06_e2e_cross_signing_signatures_index.sql @@ -0,0 +1,18 @@ +-- +-- This file is licensed under the Affero General Public License (AGPL) version 3. +-- +-- Copyright (C) 2026 New Vector, Ltd +-- +-- This program is free software: you can redistribute it and/or modify +-- it under the terms of the GNU Affero General Public License as +-- published by the Free Software Foundation, either version 3 of the +-- License, or (at your option) any later version. +-- +-- See the GNU Affero General Public License for more details: +-- . + +-- Adds the `key_id` to the e2e_cross_signing_signatures index, since the +-- ("user_id", "key_id", "target_user_id", "target_device_id") should be +-- unique. +INSERT INTO background_updates (ordering, update_name, progress_json) VALUES + (9205, 'e2e_cross_signing_signatures_add_key_id_to_index', '{}'); diff --git a/tests/handlers/test_e2e_keys.py b/tests/handlers/test_e2e_keys.py index a4f9d55a133..8d3826f5d8d 100644 --- a/tests/handlers/test_e2e_keys.py +++ b/tests/handlers/test_e2e_keys.py @@ -20,7 +20,7 @@ # # import time -from typing import Iterable +from typing import Any, Iterable from unittest import mock from parameterized import parameterized @@ -817,6 +817,102 @@ def test_reupload_signatures(self) -> None: self.assertDictEqual(devices["device_keys"][local_user]["abc"], device_key_1) self.assertDictEqual(devices["device_keys"][local_user]["def"], device_key_2) + def test_update_signature_master_key(self) -> None: + """should be able to update a signature on the Master signing key with an unknown algorithm""" + local_user = "@boris:" + self.hs.hostname + master_key: dict[str, Any] = { + # private key: HvQBbU+hc2Zr+JP1sE0XwBe1pfZZEYtJNPJLZJtS+F8 + "user_id": local_user, + "usage": ["master"], + "keys": { + "ed25519:EmkqvokUn8p+vQAGZitOk4PWjp7Ukp3txV2TbMPEiBQ": "EmkqvokUn8p+vQAGZitOk4PWjp7Ukp3txV2TbMPEiBQ" + }, + "signatures": {local_user: {"unknown:abcdefg": "abcdefg"}}, + } + self_signing_key = { + # private key: 2lonYOM6xYKdEsO+6KrC766xBcHnYnim1x/4LFGF8B0 + "user_id": local_user, + "usage": ["self_signing"], + "keys": { + "ed25519:nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk": "nqOvzeuGWT/sRx3h7+MHoInYj3Uk2LD/unI9kDYcHwk" + }, + } + master_signing_key = key.decode_signing_key_base64( + "ed25519", + "EmkqvokUn8p+vQAGZitOk4PWjp7Ukp3txV2TbMPEiBQ", + "HvQBbU+hc2Zr+JP1sE0XwBe1pfZZEYtJNPJLZJtS+F8", + ) + sign.sign_json(self_signing_key, local_user, master_signing_key) + self.get_success( + self.handler.upload_signing_keys_for_user( + local_user, + {"master_key": master_key, "self_signing_key": self_signing_key}, + ) + ) + + device_key: JsonDict = { + "user_id": local_user, + "device_id": "abc", + "algorithms": [ + "m.olm.curve25519-aes-sha2", + RoomEncryptionAlgorithms.MEGOLM_V1_AES_SHA2, + ], + "keys": { + "ed25519:abc": "base64+ed25519+key", + "curve25519:abc": "base64+curve25519+key", + }, + "signatures": {local_user: {"ed25519:abc": "base64+signature"}}, + } + self.get_success( + self.handler.upload_keys_for_user( + local_user, "abc", {"device_keys": device_key} + ) + ) + + # update the signature and upload it + master_key["signatures"][local_user]["unknown:abcdefg"] = "ABCDEFG" + self.get_success( + self.handler.upload_signatures_for_device_keys( + local_user, + { + local_user: { + "EmkqvokUn8p+vQAGZitOk4PWjp7Ukp3txV2TbMPEiBQ": master_key + } + }, + ) + ) + + devices = self.get_success( + self.handler.query_devices( + {"device_keys": {local_user: []}}, 0, local_user, "device123" + ) + ) + if "unsigned" in devices["master_keys"][local_user]: + del devices["master_keys"][local_user]["unsigned"] + self.assertDictEqual(devices["master_keys"][local_user], master_key) + + # update the signature and upload it + master_key["signatures"][local_user]["unknown:abcdefg"] = "AbCdEfG" + self.get_success( + self.handler.upload_signatures_for_device_keys( + local_user, + { + local_user: { + "EmkqvokUn8p+vQAGZitOk4PWjp7Ukp3txV2TbMPEiBQ": master_key + } + }, + ) + ) + + devices = self.get_success( + self.handler.query_devices( + {"device_keys": {local_user: []}}, 0, local_user, "device123" + ) + ) + if "unsigned" in devices["master_keys"][local_user]: + del devices["master_keys"][local_user]["unsigned"] + self.assertDictEqual(devices["master_keys"][local_user], master_key) + def test_self_signing_key_doesnt_show_up_as_device(self) -> None: """signing keys should be hidden when fetching a user's devices""" local_user = "@boris:" + self.hs.hostname