From bb8511580011be0f7fc56e579ad2a0ab6bb2b913 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Sat, 22 May 2021 12:57:07 +0300 Subject: [PATCH] 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 --- tests/test_api.py | 57 ++++++++++------------ tuf/api/metadata.py | 112 ++++++++++++++++++++++---------------------- 2 files changed, 82 insertions(+), 87 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index 183685c9f3..17c24d8570 100755 --- a/tests/test_api.py +++ b/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,6 +159,17 @@ 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) @@ -169,43 +177,28 @@ def test_sign_verify(self): # ... 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): diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 45a5c27635..73e1d37d04 100644 --- a/tuf/api/metadata.py +++ b/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: - id: An identifier string + id: An identifier string that must uniquely identify a key within + the metadata it is used in. This implementation does not verify + that the id 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.id, + "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.id) + except StopIteration: + raise exceptions.UnsignedMetadataError( + f"no signature for key {self.id} 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.id} signature for metadata", + metadata.signed, + ) + class Role: """A container class containing the set of keyids and threshold associated