From ed0ec03399af58923ff17cb491ef7dce807699b3 Mon Sep 17 00:00:00 2001
From: Jussi Kukkonen <jkukkonen@google.com>
Date: Thu, 1 Dec 2022 11:38:19 +0200
Subject: [PATCH] Metadata API: Fix verify_delegate for new Key API

verify_delegate() unfortunately needs an almost complete rewrite
as the Key.verify_signature() API change affects it quite a bit.

Refactoring the role and key lookup into a separate method makes the
code readable again.

Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
---
 tests/test_api.py   |  9 ++++++
 tuf/api/metadata.py | 78 ++++++++++++++++++++++++++++-----------------
 2 files changed, 58 insertions(+), 29 deletions(-)

diff --git a/tests/test_api.py b/tests/test_api.py
index bdbfbfc3..6e006ba0 100755
--- a/tests/test_api.py
+++ b/tests/test_api.py
@@ -353,6 +353,15 @@ def test_metadata_verify_delegate(self) -> None:
             root.verify_delegate(Snapshot.type, snapshot)
         snapshot.signed.expires = expires
 
+        # verify fails if sslib verify fails with VerificationError
+        # (in this case signature is malformed)
+        keyid = next(iter(root.signed.roles[Snapshot.type].keyids))
+        good_sig = snapshot.signatures[keyid].signature
+        snapshot.signatures[keyid].signature = "foo"
+        with self.assertRaises(exceptions.UnsignedMetadataError):
+            root.verify_delegate(Snapshot.type, snapshot)
+        snapshot.signatures[keyid].signature = good_sig
+
         # verify fails if roles keys do not sign the metadata
         with self.assertRaises(exceptions.UnsignedMetadataError):
             root.verify_delegate(Timestamp.type, snapshot)
diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py
index a52941fd..88d9e4d5 100644
--- a/tuf/api/metadata.py
+++ b/tuf/api/metadata.py
@@ -386,29 +386,11 @@ def sign(
 
         return signature
 
-    def verify_delegate(
-        self,
-        delegated_role: str,
-        delegated_metadata: "Metadata",
-        signed_serializer: Optional[SignedSerializer] = None,
-    ) -> None:
-        """Verify that ``delegated_metadata`` is signed with the required
-        threshold of keys for the delegated role ``delegated_role``.
+    def _get_role_and_keys(
+        self, delegated_role: str
+    ) -> Tuple["Role", Dict[str, Key]]:
+        """Return the keys and role for delegated_role"""
 
-        Args:
-            delegated_role: Name of the delegated role to verify
-            delegated_metadata: ``Metadata`` object for the delegated role
-            signed_serializer: Serializer used for delegate
-                serialization. Default is ``CanonicalJSONSerializer``.
-
-        Raises:
-            UnsignedMetadataError: ``delegated_role`` was not signed with
-                required threshold of keys for ``role_name``.
-            ValueError: no delegation was found for ``delegated_role``.
-            TypeError: called this function on non-delegating metadata class.
-        """
-
-        # Find the keys and role in delegator metadata
         role: Optional[Role] = None
         if isinstance(self.signed, Root):
             keys = self.signed.keys
@@ -431,17 +413,55 @@ def verify_delegate(
         if role is None:
             raise ValueError(f"No delegation found for {delegated_role}")
 
+        return (role, keys)
+
+    def verify_delegate(
+        self,
+        delegated_role: str,
+        delegated_metadata: "Metadata",
+        signed_serializer: Optional[SignedSerializer] = None,
+    ) -> None:
+        """Verify that ``delegated_metadata`` is signed with the required
+        threshold of keys for the delegated role ``delegated_role``.
+
+        Args:
+            delegated_role: Name of the delegated role to verify
+            delegated_metadata: ``Metadata`` object for the delegated role
+            signed_serializer: Serializer used for delegate
+                serialization. Default is ``CanonicalJSONSerializer``.
+
+        Raises:
+            UnsignedMetadataError: ``delegated_role`` was not signed with
+                required threshold of keys for ``role_name``.
+            ValueError: no delegation was found for ``delegated_role``.
+            TypeError: called this function on non-delegating metadata class.
+        """
+
+        if signed_serializer is None:
+            # pylint: disable=import-outside-toplevel
+            from tuf.api.serialization.json import CanonicalJSONSerializer
+
+            signed_serializer = CanonicalJSONSerializer()
+
+        data = signed_serializer.serialize(delegated_metadata.signed)
+        role, keys = self._get_role_and_keys(delegated_role)
+
         # verify that delegated_metadata is signed by threshold of unique keys
         signing_keys = set()
         for keyid in role.keyids:
-            key = keys[keyid]
+            if keyid not in keys:
+                logger.info("No key for keyid %s", keyid)
+                continue
+            if keyid not in delegated_metadata.signatures:
+                logger.info("No signature for keyid %s", keyid)
+                continue
+
+            sig = delegated_metadata.signatures[keyid]
             try:
-                key.verify_signature(delegated_metadata, signed_serializer)
-                signing_keys.add(key.keyid)
-            except UnsignedMetadataError:
-                logger.debug(
-                    "Key %s failed to verify %s", keyid, delegated_role
-                )
+                keys[keyid].verify_signature(sig, data)
+                signing_keys.add(keyid)
+            except sslib_exceptions.UnverifiedSignatureError:
+                logger.info("Key %s failed to verify %s", keyid, delegated_role)
 
         if len(signing_keys) < role.threshold:
             raise UnsignedMetadataError(