Merge pull request #1451 from sechkova/hash-validation

api/metadata input validation: length and hashes

Author: Jussi Kukkonen

tests/test_api.py

@@ -569,7 +569,7 @@ def test_metadata_targets(self):
         hashes = {
             "sha256": "141f740f53781d1ca54b8a50af22cbf74e44c21a998fa2a8a05aaac2c002886b",
             "sha512": "ef5beafa16041bcdd2937140afebd485296cd54f7348ecd5a4d035c09759608de467a7ac0eb58753d0242df873c305e8bffad2454aa48f44480f15efae1cacd0"
-        },
+        }
 
         fileinfo = TargetFile(length=28, hashes=hashes)
 

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)
 
@@ -778,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)
 
 
@@ -1033,6 +1047,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 +1067,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)