From 92257a84c31acc1c87806bc5b051fe326781dc45 Mon Sep 17 00:00:00 2001 From: Teodora Sechkova Date: Wed, 21 Apr 2021 16:32:33 +0300 Subject: [PATCH 1/4] Fix metadata version check VERSION is an integer that is greater than 0. Signed-off-by: Teodora Sechkova --- tuf/api/metadata.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 86d1b7df9e..bf03845e5a 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -335,8 +335,8 @@ def __init__( self.expires = expires # TODO: Should we separate data validation from constructor? - if version < 0: - raise ValueError(f"version must be >= 0, got {version}") + if version <= 0: + raise ValueError(f"version must be > 0, got {version}") self.version = version @staticmethod From 79391f1d85f42fc513c9e197299786cccc8f13b1 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Fri, 9 Apr 2021 16:20:24 +0300 Subject: [PATCH 2/4] New API: accept metadata with unrecognized fields In order to support ADR 0008 we would want to accept unrecognized fields in all metadata classes. Input that contains unknown fields in the 'signed' dictionary should successfully deserialize into a Metadata object, and that object should successfully serialize with the unknown fields intact. Also, we should test that we support unrecognized fields when adding new classes or modifying existing ones to make sure we support ADR 0008. Signed-off-by: Martin Vrachev --- tests/test_api.py | 31 +++++++++++++++++++++++++++++++ tuf/api/metadata.py | 42 +++++++++++++++++++++++++++++++++--------- 2 files changed, 64 insertions(+), 9 deletions(-) diff --git a/tests/test_api.py b/tests/test_api.py index 24f6ace24a..820e8dc0fe 100755 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -330,6 +330,37 @@ def test_metadata_targets(self): # Verify that data is updated self.assertEqual(targets.signed.targets[filename], fileinfo) + def setup_dict_with_unrecognized_field(self, file_path, field, value): + json_dict = {} + with open(file_path) as f: + json_dict = json.loads(f.read()) + # We are changing the json dict without changing the signature. + # This could be a problem if we want to do verification on this dict. + json_dict["signed"][field] = value + return json_dict + + def test_support_for_unrecognized_fields(self): + for metadata in ["root", "timestamp", "snapshot", "targets"]: + path = os.path.join(self.repo_dir, "metadata", metadata + ".json") + dict1 = self.setup_dict_with_unrecognized_field(path, "f", "b") + # Test that the metadata classes store unrecognized fields when + # initializing and passes them when casting the instance to a dict. + + temp_copy = copy.deepcopy(dict1) + metadata_obj = Metadata.from_dict(temp_copy) + + self.assertEqual(dict1["signed"], metadata_obj.signed.to_dict()) + + # Test that two instances of the same class could have different + # unrecognized fields. + dict2 = self.setup_dict_with_unrecognized_field(path, "f2", "b2") + temp_copy2 = copy.deepcopy(dict2) + metadata_obj2 = Metadata.from_dict(temp_copy2) + self.assertNotEqual( + metadata_obj.signed.to_dict(), metadata_obj2.signed.to_dict() + ) + + # Run unit test. if __name__ == '__main__': utils.configure_test_logging(sys.argv) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 99efd9b510..bba2af1a96 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -300,6 +300,7 @@ class Signed: spec_version: The TUF specification version number (semver) the metadata format adheres to. expires: The metadata expiration datetime object. + unrecognized_fields: Dictionary of all unrecognized fields. """ @@ -307,7 +308,12 @@ class Signed: # we keep it to match spec terminology (I often refer to this as "payload", # or "inner metadata") def __init__( - self, _type: str, version: int, spec_version: str, expires: datetime + self, + _type: str, + version: int, + spec_version: str, + expires: datetime, + unrecognized_fields: Optional[Mapping[str, Any]] = None, ) -> None: self._type = _type @@ -318,6 +324,7 @@ def __init__( if version < 0: raise ValueError(f"version must be >= 0, got {version}") self.version = version + self.unrecognized_fields = unrecognized_fields or {} @staticmethod def _common_fields_from_dict(signed_dict: Mapping[str, Any]) -> list: @@ -349,6 +356,7 @@ def _common_fields_to_dict(self) -> Dict[str, Any]: "version": self.version, "spec_version": self.spec_version, "expires": self.expires.isoformat() + "Z", + **self.unrecognized_fields, } # Modification. @@ -409,8 +417,11 @@ def __init__( consistent_snapshot: bool, keys: Mapping[str, Any], roles: Mapping[str, Any], + unrecognized_fields: Optional[Mapping[str, Any]] = None, ) -> None: - super().__init__(_type, version, spec_version, expires) + super().__init__( + _type, version, spec_version, expires, unrecognized_fields + ) # TODO: Add classes for keys and roles self.consistent_snapshot = consistent_snapshot self.keys = keys @@ -423,7 +434,8 @@ def from_dict(cls, root_dict: Mapping[str, Any]) -> "Root": consistent_snapshot = root_dict.pop("consistent_snapshot") keys = root_dict.pop("keys") roles = root_dict.pop("roles") - return cls(*common_args, consistent_snapshot, keys, roles) + # All fields left in the root_dict are unrecognized. + return cls(*common_args, consistent_snapshot, keys, roles, root_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self. """ @@ -485,8 +497,11 @@ def __init__( spec_version: str, expires: datetime, meta: Mapping[str, Any], + unrecognized_fields: Optional[Mapping[str, Any]] = None, ) -> None: - super().__init__(_type, version, spec_version, expires) + super().__init__( + _type, version, spec_version, expires, unrecognized_fields + ) # TODO: Add class for meta self.meta = meta @@ -495,7 +510,8 @@ def from_dict(cls, timestamp_dict: Mapping[str, Any]) -> "Timestamp": """Creates Timestamp object from its dict representation. """ common_args = cls._common_fields_from_dict(timestamp_dict) meta = timestamp_dict.pop("meta") - return cls(*common_args, meta) + # All fields left in the timestamp_dict are unrecognized. + return cls(*common_args, meta, timestamp_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self. """ @@ -549,8 +565,11 @@ def __init__( spec_version: str, expires: datetime, meta: Mapping[str, Any], + unrecognized_fields: Optional[Mapping[str, Any]] = None, ) -> None: - super().__init__(_type, version, spec_version, expires) + super().__init__( + _type, version, spec_version, expires, unrecognized_fields + ) # TODO: Add class for meta self.meta = meta @@ -559,7 +578,8 @@ def from_dict(cls, snapshot_dict: Mapping[str, Any]) -> "Snapshot": """Creates Snapshot object from its dict representation. """ common_args = cls._common_fields_from_dict(snapshot_dict) meta = snapshot_dict.pop("meta") - return cls(*common_args, meta) + # All fields left in the snapshot_dict are unrecognized. + return cls(*common_args, meta, snapshot_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self. """ @@ -652,8 +672,11 @@ def __init__( expires: datetime, targets: Mapping[str, Any], delegations: Mapping[str, Any], + unrecognized_fields: Optional[Mapping[str, Any]] = None, ) -> None: - super().__init__(_type, version, spec_version, expires) + super().__init__( + _type, version, spec_version, expires, unrecognized_fields + ) # TODO: Add class for meta self.targets = targets self.delegations = delegations @@ -664,7 +687,8 @@ def from_dict(cls, targets_dict: Mapping[str, Any]) -> "Targets": common_args = cls._common_fields_from_dict(targets_dict) targets = targets_dict.pop("targets") delegations = targets_dict.pop("delegations") - return cls(*common_args, targets, delegations) + # All fields left in the targets_dict are unrecognized. + return cls(*common_args, targets, delegations, targets_dict) def to_dict(self) -> Dict[str, Any]: """Returns the dict representation of self. """ From 1712b71b5523f6f2989dcea0658dd307553e76d7 Mon Sep 17 00:00:00 2001 From: Martin Vrachev Date: Tue, 27 Apr 2021 14:08:24 +0300 Subject: [PATCH 3/4] Fix black docstring indentation errors Black was updated from 20.8b1 to 21.4b0 requiring that one-line docstring don't add additional space before the closing quotes. Signed-off-by: Martin Vrachev --- tuf/api/metadata.py | 30 +++++++++++++++--------------- tuf/api/serialization/__init__.py | 16 ++++++++-------- tuf/api/serialization/json.py | 8 ++++---- 3 files changed, 27 insertions(+), 27 deletions(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index fd98339817..e28310ae56 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -164,7 +164,7 @@ def from_bytes( return deserializer.deserialize(data) def to_dict(self) -> Dict[str, Any]: - """Returns the dict representation of self. """ + """Returns the dict representation of self.""" signatures = [] for sig in self.signatures: @@ -397,7 +397,7 @@ def is_expired(self, reference_time: datetime = None) -> bool: # Modification. def bump_expiration(self, delta: timedelta = timedelta(days=1)) -> None: - """Increments the expires attribute by the passed timedelta. """ + """Increments the expires attribute by the passed timedelta.""" self.expires += delta def bump_version(self) -> None: @@ -465,7 +465,7 @@ def __init__( @classmethod def from_dict(cls, root_dict: Mapping[str, Any]) -> "Root": - """Creates Root object from its dict representation. """ + """Creates Root object from its dict representation.""" common_args = cls._common_fields_from_dict(root_dict) consistent_snapshot = root_dict.pop("consistent_snapshot") keys = root_dict.pop("keys") @@ -474,7 +474,7 @@ def from_dict(cls, root_dict: Mapping[str, Any]) -> "Root": return cls(*common_args, consistent_snapshot, keys, roles, root_dict) def to_dict(self) -> Dict[str, Any]: - """Returns the dict representation of self. """ + """Returns the dict representation of self.""" root_dict = self._common_fields_to_dict() root_dict.update( { @@ -489,14 +489,14 @@ def to_dict(self) -> Dict[str, Any]: def add_key( self, role: str, keyid: str, key_metadata: Mapping[str, Any] ) -> None: - """Adds new key for 'role' and updates the key store. """ + """Adds new key for 'role' and updates the key store.""" if keyid not in self.roles[role]["keyids"]: self.roles[role]["keyids"].append(keyid) self.keys[keyid] = key_metadata # Remove key for a role. def remove_key(self, role: str, keyid: str) -> None: - """Removes key for 'role' and updates the key store. """ + """Removes key for 'role' and updates the key store.""" if keyid in self.roles[role]["keyids"]: self.roles[role]["keyids"].remove(keyid) for keyinfo in self.roles.values(): @@ -543,14 +543,14 @@ def __init__( @classmethod def from_dict(cls, timestamp_dict: Mapping[str, Any]) -> "Timestamp": - """Creates Timestamp object from its dict representation. """ + """Creates Timestamp object from its dict representation.""" common_args = cls._common_fields_from_dict(timestamp_dict) meta = timestamp_dict.pop("meta") # All fields left in the timestamp_dict are unrecognized. return cls(*common_args, meta, timestamp_dict) def to_dict(self) -> Dict[str, Any]: - """Returns the dict representation of self. """ + """Returns the dict representation of self.""" timestamp_dict = self._common_fields_to_dict() timestamp_dict.update({"meta": self.meta}) return timestamp_dict @@ -559,7 +559,7 @@ def to_dict(self) -> Dict[str, Any]: def update( self, version: int, length: int, hashes: Mapping[str, Any] ) -> None: - """Assigns passed info about snapshot metadata to meta dict. """ + """Assigns passed info about snapshot metadata to meta dict.""" self.meta["snapshot.json"] = { "version": version, "length": length, @@ -611,14 +611,14 @@ def __init__( @classmethod def from_dict(cls, snapshot_dict: Mapping[str, Any]) -> "Snapshot": - """Creates Snapshot object from its dict representation. """ + """Creates Snapshot object from its dict representation.""" common_args = cls._common_fields_from_dict(snapshot_dict) meta = snapshot_dict.pop("meta") # All fields left in the snapshot_dict are unrecognized. return cls(*common_args, meta, snapshot_dict) def to_dict(self) -> Dict[str, Any]: - """Returns the dict representation of self. """ + """Returns the dict representation of self.""" snapshot_dict = self._common_fields_to_dict() snapshot_dict.update({"meta": self.meta}) return snapshot_dict @@ -631,7 +631,7 @@ def update( length: Optional[int] = None, hashes: Optional[Mapping[str, Any]] = None, ) -> None: - """Assigns passed (delegated) targets role info to meta dict. """ + """Assigns passed (delegated) targets role info to meta dict.""" metadata_fn = f"{rolename}.json" self.meta[metadata_fn] = {"version": version} @@ -719,7 +719,7 @@ def __init__( @classmethod def from_dict(cls, targets_dict: Mapping[str, Any]) -> "Targets": - """Creates Targets object from its dict representation. """ + """Creates Targets object from its dict representation.""" common_args = cls._common_fields_from_dict(targets_dict) targets = targets_dict.pop("targets") delegations = targets_dict.pop("delegations") @@ -727,7 +727,7 @@ def from_dict(cls, targets_dict: Mapping[str, Any]) -> "Targets": return cls(*common_args, targets, delegations, targets_dict) def to_dict(self) -> Dict[str, Any]: - """Returns the dict representation of self. """ + """Returns the dict representation of self.""" targets_dict = self._common_fields_to_dict() targets_dict.update( { @@ -739,5 +739,5 @@ def to_dict(self) -> Dict[str, Any]: # Modification. def update(self, filename: str, fileinfo: Mapping[str, Any]) -> None: - """Assigns passed target file info to meta dict. """ + """Assigns passed target file info to meta dict.""" self.targets[filename] = fileinfo diff --git a/tuf/api/serialization/__init__.py b/tuf/api/serialization/__init__.py index ed3191a103..ed3a4843c0 100644 --- a/tuf/api/serialization/__init__.py +++ b/tuf/api/serialization/__init__.py @@ -19,35 +19,35 @@ # TODO: Should these be in tuf.exceptions or inherit from tuf.exceptions.Error? class SerializationError(Exception): - """Error during serialization. """ + """Error during serialization.""" class DeserializationError(Exception): - """Error during deserialization. """ + """Error during deserialization.""" class MetadataDeserializer(metaclass=abc.ABCMeta): - """Abstract base class for deserialization of Metadata objects. """ + """Abstract base class for deserialization of Metadata objects.""" @abc.abstractmethod def deserialize(self, raw_data: bytes) -> "Metadata": - """Deserialize passed bytes to Metadata object. """ + """Deserialize passed bytes to Metadata object.""" raise NotImplementedError class MetadataSerializer(metaclass=abc.ABCMeta): - """Abstract base class for serialization of Metadata objects. """ + """Abstract base class for serialization of Metadata objects.""" @abc.abstractmethod def serialize(self, metadata_obj: "Metadata") -> bytes: - """Serialize passed Metadata object to bytes. """ + """Serialize passed Metadata object to bytes.""" raise NotImplementedError class SignedSerializer(metaclass=abc.ABCMeta): - """Abstract base class for serialization of Signed objects. """ + """Abstract base class for serialization of Signed objects.""" @abc.abstractmethod def serialize(self, signed_obj: "Signed") -> bytes: - """Serialize passed Signed object to bytes. """ + """Serialize passed Signed object to bytes.""" raise NotImplementedError diff --git a/tuf/api/serialization/json.py b/tuf/api/serialization/json.py index 3c7828ae9f..3c223d0053 100644 --- a/tuf/api/serialization/json.py +++ b/tuf/api/serialization/json.py @@ -28,10 +28,10 @@ class JSONDeserializer(MetadataDeserializer): - """Provides JSON to Metadata deserialize method. """ + """Provides JSON to Metadata deserialize method.""" def deserialize(self, raw_data: bytes) -> Metadata: - """Deserialize utf-8 encoded JSON bytes into Metadata object. """ + """Deserialize utf-8 encoded JSON bytes into Metadata object.""" try: json_dict = json.loads(raw_data.decode("utf-8")) metadata_obj = Metadata.from_dict(json_dict) @@ -55,7 +55,7 @@ def __init__(self, compact: bool = False) -> None: self.compact = compact def serialize(self, metadata_obj: Metadata) -> bytes: - """Serialize Metadata object into utf-8 encoded JSON bytes. """ + """Serialize Metadata object into utf-8 encoded JSON bytes.""" try: indent = None if self.compact else 1 separators = (",", ":") if self.compact else (",", ": ") @@ -73,7 +73,7 @@ def serialize(self, metadata_obj: Metadata) -> bytes: class CanonicalJSONSerializer(SignedSerializer): - """Provides Signed to OLPC Canonical JSON serialize method. """ + """Provides Signed to OLPC Canonical JSON serialize method.""" def serialize(self, signed_obj: Signed) -> bytes: """Serialize Signed object into utf-8 encoded OLPC Canonical JSON From a890a78ac7f724c06f939d00070e227866da1e18 Mon Sep 17 00:00:00 2001 From: Jussi Kukkonen Date: Wed, 28 Apr 2021 10:08:52 +0300 Subject: [PATCH 4/4] Add pylint disable for file object handling We return the file object so cannot use a context manager to close it. Signed-off-by: Jussi Kukkonen --- tuf/client_rework/download.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tuf/client_rework/download.py b/tuf/client_rework/download.py index 858355523f..ec7b3e1ec0 100644 --- a/tuf/client_rework/download.py +++ b/tuf/client_rework/download.py @@ -90,7 +90,7 @@ def download_file(url, required_length, fetcher, strict_required_length=True): # This is the temporary file that we will return to contain the contents of # the downloaded file. - temp_file = tempfile.TemporaryFile() + temp_file = tempfile.TemporaryFile() # pylint: disable=consider-using-with average_download_speed = 0 number_of_bytes_received = 0