From e3190fca9d6181edae5711226d0ffd1721177cda Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Fri, 28 May 2021 21:06:45 +0300 Subject: [PATCH 1/3] Metadata API: Store signatures as dict store signatures in a Dict of keyid to Signature. This ensures signature uniqueness. Raise in from_dict() if input contains multiple different signatures for a keyid. This changes Metadata object API, and makes it slightly different from the file format: this is justified by making the API safer to use and easier to validate. Signed-off-by: Jussi Kukkonen --- tests/test_api.py | 12 +++++++++--- tuf/api/metadata.py | 33 +++++++++++++++++---------------- 2 files changed, 26 insertions(+), 19 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index 7d00a85720..aaa09f5681 100755 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -175,7 +175,7 @@ def test_sign_verify(self): metadata_obj = Metadata.from_file(path) # ... it has a single existing signature, - self.assertTrue(len(metadata_obj.signatures) == 1) + self.assertEqual(len(metadata_obj.signatures), 1) # ... which is valid for the correct key. targets_key.verify_signature(metadata_obj) with self.assertRaises(tuf.exceptions.UnsignedMetadataError): @@ -185,7 +185,7 @@ def test_sign_verify(self): # 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) + self.assertEqual(len(metadata_obj.signatures), 2) # ... both are valid for the corresponding keys. targets_key.verify_signature(metadata_obj) snapshot_key.verify_signature(metadata_obj) @@ -194,7 +194,7 @@ def test_sign_verify(self): # 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) + self.assertEqual(len(metadata_obj.signatures), 1) # ... valid for that key. timestamp_key.verify_signature(metadata_obj) with self.assertRaises(tuf.exceptions.UnsignedMetadataError): @@ -236,6 +236,12 @@ def test_metadata_base(self): self.assertFalse(is_expired) md.signed.expires = expires + # Test deserializing metadata with non-unique signatures: + data = md.to_dict() + data["signatures"].append({"keyid": data["signatures"][0]["keyid"], "sig": "foo"}) + with self.assertRaises(ValueError): + Metadata.from_dict(data) + def test_metafile_class(self): # Test from_dict and to_dict with all attributes. diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 5f6a210e45..2a25d40272 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -48,12 +48,11 @@ class Metadata: signed: A subclass of Signed, which has the actual metadata payload, i.e. one of Targets, Snapshot, Timestamp or Root. - signatures: A list of Securesystemslib Signature objects, each signing - the canonical serialized representation of 'signed'. - + signatures: A dict of keyids to Securesystemslib Signature objects, + each signing the canonical serialized representation of 'signed'. """ - def __init__(self, signed: "Signed", signatures: List[Signature]) -> None: + def __init__(self, signed: "Signed", signatures: Dict[str, Signature]): self.signed = signed self.signatures = signatures @@ -89,10 +88,15 @@ def from_dict(cls, metadata: Dict[str, Any]) -> "Metadata": else: raise ValueError(f'unrecognized metadata type "{_type}"') - signatures = [] - for signature in metadata.pop("signatures"): - signature_obj = Signature.from_dict(signature) - signatures.append(signature_obj) + # Make sure signatures are unique + signatures: Dict[str, Signature] = {} + for sig_dict in metadata.pop("signatures"): + sig = Signature.from_dict(sig_dict) + if sig.keyid in signatures: + raise ValueError( + f"Multiple signatures found for keyid {sig.keyid}" + ) + signatures[sig.keyid] = sig return cls( signed=inner_cls.from_dict(metadata.pop("signed")), @@ -164,9 +168,7 @@ def from_bytes( def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self.""" - signatures = [] - for sig in self.signatures: - signatures.append(sig.to_dict()) + signatures = [sig.to_dict() for sig in self.signatures.values()] return {"signatures": signatures, "signed": self.signed.to_dict()} @@ -245,9 +247,9 @@ def sign( signature = signer.sign(signed_serializer.serialize(self.signed)) if append: - self.signatures.append(signature) + self.signatures[signature.keyid] = signature else: - self.signatures = [signature] + self.signatures = {signature.keyid: signature} return signature @@ -453,9 +455,8 @@ def verify_signature( level components: Issue #1351 """ try: - sigs = metadata.signatures - signature = next(sig for sig in sigs if sig.keyid == self.keyid) - except StopIteration: + signature = metadata.signatures[self.keyid] + except KeyError: raise exceptions.UnsignedMetadataError( f"no signature for key {self.keyid} found in metadata", metadata.signed, From 1ce36d0cd6ed1a25c5c2aa6f94f092013bbf390e Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Thu, 3 Jun 2021 10:09:43 +0300 Subject: [PATCH 2/3] Metadata API: Use OrderedDict for signatures Dict ordering is part of regular Dict from Python 3.7: Use OrderedDict for signatures to make sure signatures are serialized in a reproducible order even on 3.6. The added benefit is that reader will immediately understand that the order has some significance. The actual type annotations are a bit convoluted because: * typing does not include OrderedDict before 3.7 so can't use that * Annotating inner types does not work for collections.OrderedDict in older pythons (so have to use the "stringified annotations") Signed-off-by: Jussi Kukkonen --- tuf/api/metadata.py | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 2a25d40272..2fcb561a5d 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -17,6 +17,7 @@ """ import abc import tempfile +from collections import OrderedDict from datetime import datetime, timedelta from typing import Any, ClassVar, Dict, List, Mapping, Optional, Tuple, Type @@ -48,11 +49,13 @@ class Metadata: signed: A subclass of Signed, which has the actual metadata payload, i.e. one of Targets, Snapshot, Timestamp or Root. - signatures: A dict of keyids to Securesystemslib Signature objects, - each signing the canonical serialized representation of 'signed'. + signatures: An ordered dictionary of keyids to Signature objects, each + signing the canonical serialized representation of 'signed'. """ - def __init__(self, signed: "Signed", signatures: Dict[str, Signature]): + def __init__( + self, signed: "Signed", signatures: "OrderedDict[str, Signature]" + ): self.signed = signed self.signatures = signatures @@ -89,7 +92,7 @@ def from_dict(cls, metadata: Dict[str, Any]) -> "Metadata": raise ValueError(f'unrecognized metadata type "{_type}"') # Make sure signatures are unique - signatures: Dict[str, Signature] = {} + signatures: "OrderedDict[str, Signature]" = OrderedDict() for sig_dict in metadata.pop("signatures"): sig = Signature.from_dict(sig_dict) if sig.keyid in signatures: @@ -249,7 +252,7 @@ def sign( if append: self.signatures[signature.keyid] = signature else: - self.signatures = {signature.keyid: signature} + self.signatures = OrderedDict({signature.keyid: signature}) return signature From 146eb105c10b68a0c15ed3125eadb7fa7402b55c Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Fri, 11 Jun 2021 10:45:04 +0300 Subject: [PATCH 3/3] Metadata API: Be more explicit when appending sigs Clearing the OrderedDict makes it easier to see what happens and avoids having to call OrderedDict() again. Signed-off-by: Jussi Kukkonen --- tuf/api/metadata.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 2fcb561a5d..6c8b307161 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -249,10 +249,10 @@ def sign( signature = signer.sign(signed_serializer.serialize(self.signed)) - if append: - self.signatures[signature.keyid] = signature - else: - self.signatures = OrderedDict({signature.keyid: signature}) + if not append: + self.signatures.clear() + + self.signatures[signature.keyid] = signature return signature