From 03f39b01e722d2398d8b426a484ae00139f22e99 Mon Sep 17 00:00:00 2001 From: Teodora Sechkova Date: Wed, 16 Jun 2021 14:27:10 +0300 Subject: [PATCH 1/3] Add hash and length validation - valid length: greater than zero - valid hashes: a non-empty dictionary of type Dict[str, str] Checking the validity of hash algorithms is not part of the metadata input validation and is done by securesystemslib during hash verification. Signed-off-by: Teodora Sechkova --- tuf/api/metadata.py | 37 +++++++++++++++++++++++++------------ 1 file changed, 25 insertions(+), 12 deletions(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index 73f221a04e..cfd4514669 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -702,6 +702,19 @@ def _verify_length( f"expected length {expected_length}" ) + @staticmethod + def _validate_hashes(hashes: Dict[str, str]) -> None: + if not hashes: + raise ValueError("Hashes must be a non empty dictionary") + for key, value in hashes.items(): + if not (isinstance(key, str) and isinstance(value, str)): + raise TypeError("Hashes items must be strings") + + @staticmethod + def _validate_length(length: int) -> None: + if length <= 0: + raise ValueError(f"Length must be > 0, got {length}") + class MetaFile(BaseFile): """A container with information about a particular metadata file. @@ -730,6 +743,14 @@ def __init__( hashes: Optional[Dict[str, str]] = None, unrecognized_fields: Optional[Mapping[str, Any]] = None, ) -> None: + + if version <= 0: + raise ValueError(f"Metafile version must be > 0, got {version}") + if length is not None: + self._validate_length(length) + if hashes is not None: + self._validate_hashes(hashes) + self.version = version self.length = length self.hashes = hashes @@ -742,12 +763,6 @@ def from_dict(cls, meta_dict: Dict[str, Any]) -> "MetaFile": length = meta_dict.pop("length", None) hashes = meta_dict.pop("hashes", None) - # Do some basic input validation - if version <= 0: - raise ValueError(f"Metafile version must be > 0, got {version}") - if length is not None and length <= 0: - raise ValueError(f"Metafile length must be > 0, got {length}") - # All fields left in the meta_dict are unrecognized. return cls(version, length, hashes, meta_dict) @@ -1033,6 +1048,10 @@ def __init__( hashes: Dict[str, str], unrecognized_fields: Optional[Mapping[str, Any]] = None, ) -> None: + + self._validate_length(length) + self._validate_hashes(hashes) + self.length = length self.hashes = hashes self.unrecognized_fields = unrecognized_fields or {} @@ -1049,12 +1068,6 @@ def from_dict(cls, target_dict: Dict[str, Any]) -> "TargetFile": length = target_dict.pop("length") hashes = target_dict.pop("hashes") - # Do some basic validation checks - if length <= 0: - raise ValueError(f"Targetfile length must be > 0, got {length}") - if not hashes: - raise ValueError("Missing targetfile hashes") - # All fields left in the target_dict are unrecognized. return cls(length, hashes, target_dict) From e30faa89be45518a1517cc5a0ed2f95a812411c3 Mon Sep 17 00:00:00 2001 From: Teodora Sechkova Date: Wed, 16 Jun 2021 14:36:02 +0300 Subject: [PATCH 2/3] Remove empty hash dict check from MetaFile The check for an empty hash dictionary is now part of the hash validation function. Signed-off-by: Teodora Sechkova --- tuf/api/metadata.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tuf/api/metadata.py b/tuf/api/metadata.py index cfd4514669..ed8f1424d4 100644 --- a/tuf/api/metadata.py +++ b/tuf/api/metadata.py @@ -793,8 +793,7 @@ def verify_length_and_hashes(self, data: Union[bytes, BinaryIO]): if self.length is not None: self._verify_length(data, self.length) - # Skip the check in case of an empty dictionary too - if self.hashes: + if self.hashes is not None: self._verify_hashes(data, self.hashes) From 328f63726400b5aa1084dc62a7e3a6e46c40fc92 Mon Sep 17 00:00:00 2001 From: Teodora Sechkova Date: Wed, 16 Jun 2021 14:38:02 +0300 Subject: [PATCH 3/3] Remove trailing comma from test data A trailing comma makes any element a one-item tuple. Signed-off-by: Teodora Sechkova --- tests/test_api.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/test_api.py b/tests/test_api.py index e0b57114b2..b59b45a48b 100755 --- a/tests/test_api.py +++ b/tests/test_api.py @@ -569,7 +569,7 @@ def test_metadata_targets(self): hashes = { "sha256": "141f740f53781d1ca54b8a50af22cbf74e44c21a998fa2a8a05aaac2c002886b", "sha512": "ef5beafa16041bcdd2937140afebd485296cd54f7348ecd5a4d035c09759608de467a7ac0eb58753d0242df873c305e8bffad2454aa48f44480f15efae1cacd0" - }, + } fileinfo = TargetFile(length=28, hashes=hashes)