Metadata API: Move signature verification to Key

This is likely not needed by users of the API (as they are interested
in the higher level functionality "verify delegate metadata with
threshold of signatures").

Moving verify to Key makes the API cleaner because including both
"verify myself" and "verify a delegate with threshold" can look awkward
in Metadata, and because the ugly Securesystemslib integration is now
Key class implementation detail (see Key.to_securesystemslib_key()).

Also raise on verify failure instead of returning false: this was found
to confuse API users (and was arguably not a pythonic way to handle it).

* Name the function verify_signature() to make it clear what is being
  verified.
* Assume only one signature per keyid exists: see #1422
* Raise only UnsignedMetadataError (when no signatures or verify failure),
  the remaining lower level errors will be handled in #1351
* Stop using a "keystore" in tests for the public keys: everything we
  need is in metadata already

This changes API, but also should not be something API users want to
call in the future when "verify a delegate with threshold" exists.

Signed-off-by: Jussi Kukkonen <[email protected]>

tests/test_api.py

@@ -81,13 +81,10 @@ def setUpClass(cls):
         # Load keys into memory
         cls.keystore = {}
         for role in ['delegation', 'snapshot', 'targets', 'timestamp']:
-            cls.keystore[role] = {
-                'private': import_ed25519_privatekey_from_file(
-                        os.path.join(cls.keystore_dir, role + '_key'),
-                        password="password"),
-                'public': import_ed25519_publickey_from_file(
-                        os.path.join(cls.keystore_dir, role + '_key.pub'))
-            }
+            cls.keystore[role] = import_ed25519_privatekey_from_file(
+                os.path.join(cls.keystore_dir, role + '_key'),
+                password="password"
+            )
 
 
     @classmethod
@@ -162,50 +159,46 @@ def test_read_write_read_compare(self):
 
 
     def test_sign_verify(self):
+        root_path = os.path.join(self.repo_dir, 'metadata', 'root.json')
+        root:Root = Metadata.from_file(root_path).signed
+
+        # Locate the public keys we need from root
+        targets_keyid = next(iter(root.roles["targets"].keyids))
+        targets_key = root.keys[targets_keyid]
+        snapshot_keyid = next(iter(root.roles["snapshot"].keyids))
+        snapshot_key = root.keys[snapshot_keyid]
+        timestamp_keyid = next(iter(root.roles["timestamp"].keyids))
+        timestamp_key = root.keys[timestamp_keyid]
+
         # Load sample metadata (targets) and assert ...
         path = os.path.join(self.repo_dir, 'metadata', 'targets.json')
         metadata_obj = Metadata.from_file(path)
 
         # ... it has a single existing signature,
         self.assertTrue(len(metadata_obj.signatures) == 1)
         # ... which is valid for the correct key.
-        self.assertTrue(metadata_obj.verify(
-                self.keystore['targets']['public']))
+        targets_key.verify_signature(metadata_obj)
+        with self.assertRaises(tuf.exceptions.UnsignedMetadataError):
+            snapshot_key.verify_signature(metadata_obj)
 
-        sslib_signer = SSlibSigner(self.keystore['snapshot']['private'])
+        sslib_signer = SSlibSigner(self.keystore['snapshot'])
         # Append a new signature with the unrelated key and assert that ...
         metadata_obj.sign(sslib_signer, append=True)
         # ... there are now two signatures, and
         self.assertTrue(len(metadata_obj.signatures) == 2)
         # ... both are valid for the corresponding keys.
-        self.assertTrue(metadata_obj.verify(
-                self.keystore['targets']['public']))
-        self.assertTrue(metadata_obj.verify(
-                self.keystore['snapshot']['public']))
+        targets_key.verify_signature(metadata_obj)
+        snapshot_key.verify_signature(metadata_obj)
 
-        sslib_signer.key_dict = self.keystore['timestamp']['private']
+        sslib_signer = SSlibSigner(self.keystore['timestamp'])
         # Create and assign (don't append) a new signature and assert that ...
         metadata_obj.sign(sslib_signer, append=False)
         # ... there now is only one signature,
         self.assertTrue(len(metadata_obj.signatures) == 1)
         # ... valid for that key.
-        self.assertTrue(metadata_obj.verify(
-                self.keystore['timestamp']['public']))
-
-        # Assert exception if there are more than one signatures for a key
-        metadata_obj.sign(sslib_signer, append=True)
-        with self.assertRaises(tuf.exceptions.Error) as ctx:
-            metadata_obj.verify(self.keystore['timestamp']['public'])
-        self.assertTrue(
-                '2 signatures for key' in str(ctx.exception),
-                str(ctx.exception))
-
-        # Assert exception if there is no signature for a key
-        with self.assertRaises(tuf.exceptions.Error) as ctx:
-            metadata_obj.verify(self.keystore['targets']['public'])
-        self.assertTrue(
-                'no signature for' in str(ctx.exception),
-                str(ctx.exception))
+        timestamp_key.verify_signature(metadata_obj)
+        with self.assertRaises(tuf.exceptions.UnsignedMetadataError):
+            targets_key.verify_signature(metadata_obj)
 
 
     def test_metadata_base(self):

tuf/api/metadata.py

@@ -19,7 +19,7 @@
 from datetime import datetime, timedelta
 from typing import Any, Dict, List, Mapping, Optional
 
-from securesystemslib.keys import verify_signature
+from securesystemslib import keys as sslib_keys
 from securesystemslib.signer import Signature, Signer
 from securesystemslib.storage import FilesystemBackend, StorageBackendInterface
 from securesystemslib.util import persist_temp_file
@@ -250,59 +250,6 @@ def sign(
 
         return signature
 
-    def verify(
-        self,
-        key: Mapping[str, Any],
-        signed_serializer: Optional[SignedSerializer] = None,
-    ) -> bool:
-        """Verifies 'signatures' over 'signed' that match the passed key by id.
-
-        Arguments:
-            key: A securesystemslib-style public key object.
-            signed_serializer: A SignedSerializer subclass instance that
-                implements the desired canonicalization format. Per default a
-                CanonicalJSONSerializer is used.
-
-        Raises:
-            # TODO: Revise exception taxonomy
-            tuf.exceptions.Error: None or multiple signatures found for key.
-            securesystemslib.exceptions.FormatError: Key argument is malformed.
-            tuf.api.serialization.SerializationError:
-                'signed' cannot be serialized.
-            securesystemslib.exceptions.CryptoError, \
-                    securesystemslib.exceptions.UnsupportedAlgorithmError:
-                Signing errors.
-
-        Returns:
-            A boolean indicating if the signature is valid for the passed key.
-
-        """
-        signatures_for_keyid = list(
-            filter(lambda sig: sig.keyid == key["keyid"], self.signatures)
-        )
-
-        if not signatures_for_keyid:
-            raise exceptions.Error(f"no signature for key {key['keyid']}.")
-
-        if len(signatures_for_keyid) > 1:
-            raise exceptions.Error(
-                f"{len(signatures_for_keyid)} signatures for key "
-                f"{key['keyid']}, not sure which one to verify."
-            )
-
-        if signed_serializer is None:
-            # Use local scope import to avoid circular import errors
-            # pylint: disable=import-outside-toplevel
-            from tuf.api.serialization.json import CanonicalJSONSerializer
-
-            signed_serializer = CanonicalJSONSerializer()
-
-        return verify_signature(
-            key,
-            signatures_for_keyid[0].to_dict(),
-            signed_serializer.serialize(self.signed),
-        )
-
 
 class Signed:
     """A base class for the signed part of TUF metadata.
@@ -417,7 +364,9 @@ class Key:
     """A container class representing the public portion of a Key.
 
     Attributes:
-        keyid: An identifier string
+        keyid: An identifier string that must uniquely identify a key within
+            the metadata it is used in. This implementation does not verify
+            that keyid is the hash of a specific representation of the key.
         keytype: A string denoting a public key signature system,
             such as "rsa", "ed25519", and "ecdsa-sha2-nistp256".
         scheme: A string denoting a corresponding signature scheme. For example:
@@ -461,6 +410,59 @@ def to_dict(self) -> Dict[str, Any]:
             **self.unrecognized_fields,
         }
 
+    def to_securesystemslib_key(self) -> Dict[str, Any]:
+        """Returns a Securesystemslib compatible representation of self."""
+        return {
+            "keyid": self.keyid,
+            "keytype": self.keytype,
+            "scheme": self.scheme,
+            "keyval": self.keyval,
+        }
+
+    def verify_signature(
+        self,
+        metadata: Metadata,
+        signed_serializer: Optional[SignedSerializer] = None,
+    ):
+        """Verifies that the 'metadata.signatures' contains a signature made
+        with this key, correctly signing 'metadata.signed'.
+
+        Arguments:
+            metadata: Metadata to verify
+            signed_serializer: Optional; SignedSerializer to serialize
+                'metadata.signed' with. Default is CanonicalJSONSerializer.
+
+        Raises:
+            UnsignedMetadataError: The signature could not be verified for a
+                variety of possible reasons: see error message.
+            TODO: Various other errors currently bleed through from lower
+                level components: Issue #1351
+        """
+        try:
+            sigs = metadata.signatures
+            signature = next(sig for sig in sigs if sig.keyid == self.keyid)
+        except StopIteration:
+            raise exceptions.UnsignedMetadataError(
+                f"no signature for key {self.keyid} found in metadata",
+                metadata.signed,
+            ) from None
+
+        if signed_serializer is None:
+            # pylint: disable=import-outside-toplevel
+            from tuf.api.serialization.json import CanonicalJSONSerializer
+
+            signed_serializer = CanonicalJSONSerializer()
+
+        if not sslib_keys.verify_signature(
+            self.to_securesystemslib_key(),
+            signature.to_dict(),
+            signed_serializer.serialize(metadata.signed),
+        ):
+            raise exceptions.UnsignedMetadataError(
+                f"Failed to verify {self.keyid} signature for metadata",
+                metadata.signed,
+            )
+
 
 class Role:
     """A container class containing the set of keyids and threshold associated