Relax validation of signature upload for master keys, and allow updates (#19915)

Co-authored-by: Andrew Morgan <andrew@amorgan.xyz>

changelog.d/19915.feature

@@ -0,0 +1 @@
+Don't validate signatures with unknown algorithms for master keys, and allow updates to signatures.

synapse/handlers/e2e_keys.py

@@ -1272,21 +1272,27 @@ class E2eKeysHandler:
         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)
+            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
-            )
+                # 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)

synapse/storage/database.py

@@ -120,6 +120,7 @@ UNIQUE_INDEX_BACKGROUND_UPDATES = {
     "event_push_summary": "event_push_summary_unique_index2",
     "receipts_linearized": "receipts_linearized_unique_index",
     "receipts_graph": "receipts_graph_unique_index",
+    "e2e_cross_signing_signatures": "e2e_cross_signing_signatures_add_key_id_to_index",
 }
 
 

synapse/storage/databases/main/end_to_end_keys.py

@@ -78,6 +78,10 @@ class DeviceKeyLookupResult:
 
 
 class EndToEndKeyBackgroundStore(SQLBaseStore):
+    CROSS_SIGNING_KEYS_REMOVE_DUPLICATES_NAME = (
+        "e2e_cross_signing_signatures_remove_duplicates"
+    )
+
     def __init__(
         self,
         database: DatabasePool,
@@ -101,6 +105,80 @@ class EndToEndKeyBackgroundStore(SQLBaseStore):
             columns=("user_id", "device_id", "algorithm", "ts_added_ms"),
         )
 
+        self.db_pool.updates.register_background_update_handler(
+            self.CROSS_SIGNING_KEYS_REMOVE_DUPLICATES_NAME,
+            self._background_cross_signing_signatures_remove_duplicates,
+        )
+
+        self.db_pool.updates.register_background_index_update(
+            update_name="e2e_cross_signing_signatures_add_key_id_to_index",
+            index_name="e2e_cross_signing_signatures_idx3",
+            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",
+        )
+
+    async def _background_cross_signing_signatures_remove_duplicates(
+        self, progress: dict, batch_size: int
+    ) -> int:
+        """Removes duplicate cross-signing signatures so that we can add a
+        unique index on `(user_id, target_user_id, target_device_id, key_id)` to
+        `e2e_cross_signing_signatures`.
+        """
+
+        def _remove_duplicate_signatures_txn(txn: LoggingTransaction) -> int:
+            sql = """
+                SELECT user_id, key_id, target_user_id, target_device_id, MAX(signature)
+                FROM e2e_cross_signing_signatures
+                GROUP BY user_id, key_id, target_user_id, target_device_id
+                HAVING COUNT(*) > 1
+                LIMIT ?
+            """
+            txn.execute(sql, (batch_size,))
+            duplicate_keys = cast(list[tuple[str, str, str, str, str]], list(txn))
+
+            for (
+                user_id,
+                key_id,
+                target_user_id,
+                target_device_id,
+                signature,
+            ) in duplicate_keys:
+                sql = """
+                    DELETE FROM e2e_cross_signing_signatures
+                    WHERE
+                        user_id = ? AND
+                        key_id = ? AND
+                        target_user_id = ? AND
+                        target_device_id = ?
+                """
+                txn.execute(sql, (user_id, key_id, target_user_id, target_device_id))
+
+                sql = """
+                    INSERT INTO e2e_cross_signing_signatures
+                        (user_id, key_id, target_user_id, target_device_id, signature)
+                    VALUES
+                        (?, ?, ?, ?, ?)
+                """
+                txn.execute(
+                    sql, (user_id, key_id, target_user_id, target_device_id, signature)
+                )
+
+            return len(duplicate_keys)
+
+        number_deleted = await self.db_pool.runInteraction(
+            self.CROSS_SIGNING_KEYS_REMOVE_DUPLICATES_NAME,
+            _remove_duplicate_signatures_txn,
+        )
+
+        if number_deleted < batch_size:
+            await self.db_pool.updates._end_background_update(
+                self.CROSS_SIGNING_KEYS_REMOVE_DUPLICATES_NAME
+            )
+
+        return number_deleted
+
 
 class EndToEndKeyWorkerStore(EndToEndKeyBackgroundStore, CacheInvalidationWorkerStore):
     def __init__(
@@ -1808,7 +1886,7 @@ class EndToEndKeyWorkerStore(EndToEndKeyBackgroundStore, CacheInvalidationWorker
             )
 
     async def store_e2e_cross_signing_signatures(
-        self, user_id: str, signatures: "Iterable[SignatureListItem]"
+        self, user_id: str, signatures: "list[SignatureListItem]"
     ) -> None:
         """Stores cross-signing signatures.
 
@@ -1819,28 +1897,23 @@ class EndToEndKeyWorkerStore(EndToEndKeyBackgroundStore, CacheInvalidationWorker
 
         def _store_e2e_cross_signing_signatures(
             txn: LoggingTransaction,
-            signatures: "Iterable[SignatureListItem]",
+            signatures: "list[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 = [

synapse/storage/schema/main/delta/94/09_e2e_cross_signing_signatures_index.sql

@@ -0,0 +1,21 @@
+--
+-- 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:
+-- <https://www.gnu.org/licenses/agpl-3.0.html>.
+
+-- Remove any rows `e2e_cross_signing_signatures` that have duplicate cross-signing signatures.
+-- Ensures that rows are unique across `(user_id, target_user_id, target_device_id, key_id)`, so that
+-- we can create an index on those columns. See
+-- `./10_e2e_cross_signing_signatures_add_key_id_to_index.sql`, which adds said
+-- index.
+        
+INSERT INTO background_updates (ordering, update_name, progress_json) VALUES
+  (9409, 'e2e_cross_signing_signatures_remove_duplicates', '{}');

synapse/storage/schema/main/delta/94/10_e2e_cross_signing_signatures_add_key_id_to_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:
+-- <https://www.gnu.org/licenses/agpl-3.0.html>.
+
+-- 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
+  (9410, 'e2e_cross_signing_signatures_add_key_id_to_index', '{}');

tests/handlers/test_e2e_keys.py

@@ -817,6 +817,103 @@ class E2eKeysHandlerTestCase(unittest.HomeserverTestCase):
         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: JsonDict = {
+            # 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 again 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
+                    }
+                },
+            )
+        )
+
+        # Assert that we were able to update the signature.
+        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

tests/storage/test_end_to_end_keys.py

@@ -22,6 +22,7 @@
 from twisted.internet.testing import MemoryReactor
 
 from synapse.server import HomeServer
+from synapse.storage.database import LoggingTransaction
 from synapse.util.clock import Clock
 
 from tests.unittest import HomeserverTestCase
@@ -118,3 +119,98 @@ class EndToEndKeyStoreTestCase(HomeserverTestCase):
         self.assertIn("user2", res)
         self.assertNotIn("device1", res["user2"])
         self.assertIn("device2", res["user2"])
+
+    def test_bg_signatures_migration(self) -> None:
+        updater = self.hs.get_datastores().main.db_pool.updates
+
+        # drop the constraint so we can insert duplicate signatures
+        def f(txn: LoggingTransaction) -> None:
+            txn.execute("DROP INDEX e2e_cross_signing_signatures_idx3")
+
+        self.get_success(self.store.db_pool.runInteraction("", f))
+
+        # save multiple copies of the same key in the database
+        for _i in range(2):
+            self.get_success(
+                self.store.db_pool.simple_insert(
+                    "e2e_cross_signing_signatures",
+                    {
+                        "user_id": "@alice:example.org",
+                        "key_id": "ed25519:abcdefg",
+                        "target_user_id": "@alice:example.org",
+                        "target_device_id": "hijklmnop",
+                        "signature": "some+signature",
+                    },
+                )
+            )
+
+        for _i in range(2):
+            self.get_success(
+                self.store.db_pool.simple_insert(
+                    "e2e_cross_signing_signatures",
+                    {
+                        "user_id": "@alice:example.org",
+                        "key_id": "ed25519:hijklmnop",
+                        "target_user_id": "@alice:example.org",
+                        "target_device_id": "abcdefg",
+                        "signature": "some+signature",
+                    },
+                )
+            )
+
+        # run the background task to remove duplicates
+        self.get_success(
+            self.store.db_pool.simple_insert(
+                "background_updates",
+                values={
+                    "update_name": "e2e_cross_signing_signatures_remove_duplicates",
+                    "progress_json": "{}",
+                },
+            )
+        )
+
+        self.get_success(
+            updater.run_background_updates(False),
+        )
+
+        # re-add the unique index
+        self.get_success(
+            self.store.db_pool.simple_insert(
+                "background_updates",
+                values={
+                    "update_name": "e2e_cross_signing_signatures_add_key_id_to_index",
+                    "progress_json": "{}",
+                },
+            )
+        )
+
+        self.get_success(
+            updater.run_background_updates(False),
+        )
+
+        # check that we only have one copy of each key
+        expected_values = [
+            (
+                "@alice:example.org",
+                "ed25519:abcdefg",
+                "@alice:example.org",
+                "hijklmnop",
+                "some+signature",
+            ),
+            (
+                "@alice:example.org",
+                "ed25519:hijklmnop",
+                "@alice:example.org",
+                "abcdefg",
+                "some+signature",
+            ),
+        ]
+
+        res = self.get_success(
+            self.store.db_pool.execute(
+                "",
+                "SELECT user_id, key_id, target_user_id, target_device_id, signature from e2e_cross_signing_signatures ORDER BY key_id",
+            )
+        )
+        self.assertEqual(len(res), len(expected_values))
+        self.assertEqual(res, expected_values)