make changes from PR review

synapse/handlers/e2e_keys.py

@@ -510,9 +510,18 @@ class E2eKeysHandler(object):
         if not master_key:
             raise SynapseError(400, "No master key available", Codes.MISSING_PARAM)
 
+        try:
             master_key_id, master_verify_key = get_verify_key_from_cross_signing_key(
                 master_key
             )
+        except ValueError:
+            if "master_key" in keys:
+                # the invalid key came from the request
+                raise SynapseError(400, "Invalid master key", Codes.INVALID_PARAM)
+            else:
+                # the invalid key came from the database
+                logger.error("Invalid master key found for user %s", user_id)
+                raise SynapseError(500, "Invalid master key")
 
         # for the other cross-signing keys, make sure that they have valid
         # signatures from the master key
@@ -539,9 +548,12 @@ class E2eKeysHandler(object):
             yield self.store.set_e2e_cross_signing_key(
                 user_id, "self_signing", self_signing_key
             )
+            try:
                 deviceids.append(
                     get_verify_key_from_cross_signing_key(self_signing_key)[1].version
                 )
+            except ValueError:
+                raise SynapseError(400, "Invalid self-signing key", Codes.INVALID_PARAM)
         if "user_signing_key" in keys:
             yield self.store.set_e2e_cross_signing_key(
                 user_id, "user_signing", user_signing_key

synapse/storage/schema/delta/56/hidden_devices.sql

@@ -13,47 +13,6 @@
  * limitations under the License.
  */
 
--- cross-signing keys
-CREATE TABLE IF NOT EXISTS e2e_cross_signing_keys (
-    user_id TEXT NOT NULL,
-    -- the type of cross-signing key (master, user_signing, or self_signing)
-    keytype TEXT NOT NULL,
-    -- the full key information, as a json-encoded dict
-    keydata TEXT NOT NULL,
-    -- time that the key was added
-    added_ts BIGINT NOT NULL
-);
-
-CREATE UNIQUE INDEX e2e_cross_signing_keys_idx ON e2e_cross_signing_keys(user_id, keytype, added_ts);
-
--- cross-signing signatures
-CREATE TABLE IF NOT EXISTS e2e_cross_signing_signatures (
-    -- user who did the signing
-    user_id TEXT NOT NULL,
-    -- key used to sign
-    key_id TEXT NOT NULL,
-    -- user who was signed
-    target_user_id TEXT NOT NULL,
-    -- device/key that was signed
-    target_device_id TEXT NOT NULL,
-    -- the actual signature
-    signature TEXT NOT NULL
-);
-
-CREATE UNIQUE INDEX e2e_cross_signing_signatures_idx ON e2e_cross_signing_signatures(user_id, target_user_id, target_device_id);
-
--- stream of user signature updates
-CREATE TABLE IF NOT EXISTS user_signature_stream (
-    -- uses the same stream ID as device list stream
-    stream_id BIGINT NOT NULL,
-    -- user who did the signing
-    from_user_id TEXT NOT NULL,
-    -- list of users who were signed, as a JSON array
-    user_ids TEXT NOT NULL
-);
-
-CREATE UNIQUE INDEX user_signature_stream_idx ON user_signature_stream(stream_id);
-
 -- device list needs to know which ones are "real" devices, and which ones are
 -- just used to avoid collisions
 ALTER TABLE devices ADD COLUMN hidden BOOLEAN DEFAULT FALSE;

synapse/storage/schema/delta/56/signing_keys.sql

@@ -0,0 +1,55 @@
+/* Copyright 2019 New Vector Ltd
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+-- cross-signing keys
+CREATE TABLE IF NOT EXISTS e2e_cross_signing_keys (
+    user_id TEXT NOT NULL,
+    -- the type of cross-signing key (master, user_signing, or self_signing)
+    keytype TEXT NOT NULL,
+    -- the full key information, as a json-encoded dict
+    keydata TEXT NOT NULL,
+    -- time that the key was added
+    added_ts BIGINT NOT NULL
+);
+
+CREATE UNIQUE INDEX e2e_cross_signing_keys_idx ON e2e_cross_signing_keys(user_id, keytype, added_ts);
+
+-- cross-signing signatures
+CREATE TABLE IF NOT EXISTS e2e_cross_signing_signatures (
+    -- user who did the signing
+    user_id TEXT NOT NULL,
+    -- key used to sign
+    key_id TEXT NOT NULL,
+    -- user who was signed
+    target_user_id TEXT NOT NULL,
+    -- device/key that was signed
+    target_device_id TEXT NOT NULL,
+    -- the actual signature
+    signature TEXT NOT NULL
+);
+
+CREATE UNIQUE INDEX e2e_cross_signing_signatures_idx ON e2e_cross_signing_signatures(user_id, target_user_id, target_device_id);
+
+-- stream of user signature updates
+CREATE TABLE IF NOT EXISTS user_signature_stream (
+    -- uses the same stream ID as device list stream
+    stream_id BIGINT NOT NULL,
+    -- user who did the signing
+    from_user_id TEXT NOT NULL,
+    -- list of users who were signed, as a JSON array
+    user_ids TEXT NOT NULL
+);
+
+CREATE UNIQUE INDEX user_signature_stream_idx ON user_signature_stream(stream_id);

synapse/types.py

@@ -492,10 +492,10 @@ def get_verify_key_from_cross_signing_key(key_info):
     """
     # make sure that exactly one key is provided
     if "keys" not in key_info:
-        raise SynapseError(400, "Invalid key")
+        raise ValueError("Invalid key")
     keys = key_info["keys"]
     if len(keys) != 1:
-        raise SynapseError(400, "Invalid key")
+        raise ValueError("Invalid key")
     # and return that one key
     for key_id, key_data in keys.items():
         return (key_id, decode_verify_key_bytes(key_id, decode_base64(key_data)))