Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 298
Add hash and length verification to MetaFile and TargetFile#1437
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
+176
−16
Merged
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Jump to file
Failed to load files.
Loading
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -16,10 +16,23 @@ | ||
| """ | ||
| import abc | ||
| import io | ||
| import tempfile | ||
| from datetime import datetime, timedelta | ||
| from typing import Any, ClassVar, Dict, List, Mapping, Optional, Tuple, Type | ||
| from typing import ( | ||
| Any, | ||
| BinaryIO, | ||
| ClassVar, | ||
| Dict, | ||
| List, | ||
| Mapping, | ||
| Optional, | ||
| Tuple, | ||
| Type, | ||
| Union, | ||
| ) | ||
| from securesystemslib import hash as sslib_hash | ||
| from securesystemslib import keys as sslib_keys | ||
| from securesystemslib.signer import Signature, Signer | ||
| from securesystemslib.storage import FilesystemBackend, StorageBackendInterface | ||
| @@ -622,7 +635,53 @@ def remove_key(self, role: str, keyid: str) -> None: | ||
| del self.keys[keyid] | ||
| class MetaFile: | ||
| class BaseFile: | ||
| """A base class of MetaFile and TargetFile. | ||
| Encapsulates common static methods for length and hash verification. | ||
| """ | ||
| @staticmethod | ||
| def _verify_hashes( | ||
| data: Union[bytes, BinaryIO], expected_hashes: Dict[str, str] | ||
| ) -> None: | ||
| """Verifies that the hash of 'data' matches 'expected_hashes'""" | ||
| is_bytes = isinstance(data, bytes) | ||
| for algo, exp_hash in expected_hashes.items(): | ||
| if is_bytes: | ||
| digest_object = sslib_hash.digest(algo) | ||
| digest_object.update(data) | ||
| else: | ||
| # if data is not bytes, assume it is a file object | ||
| digest_object = sslib_hash.digest_fileobject(data, algo) | ||
| observed_hash = digest_object.hexdigest() | ||
| if observed_hash != exp_hash: | ||
| raise exceptions.LengthOrHashMismatchError( | ||
| f"Observed hash {observed_hash} does not match" | ||
| f"expected hash {exp_hash}" | ||
| ) | ||
| @staticmethod | ||
| def _verify_length( | ||
| data: Union[bytes, BinaryIO], expected_length: int | ||
| ) -> None: | ||
| """Verifies that the length of 'data' matches 'expected_length'""" | ||
| if isinstance(data, bytes): | ||
| observed_length = len(data) | ||
| else: | ||
| # if data is not bytes, assume it is a file object | ||
| data.seek(0, io.SEEK_END) | ||
| observed_length = data.tell() | ||
| if observed_length != expected_length: | ||
| raise exceptions.LengthOrHashMismatchError( | ||
| f"Observed length {observed_length} does not match" | ||
| f"expected length {expected_length}" | ||
| ) | ||
| class MetaFile(BaseFile): | ||
| """A container with information about a particular metadata file. | ||
| Attributes: | ||
| @@ -660,6 +719,13 @@ def from_dict(cls, meta_dict: Dict[str, Any]) -> "MetaFile": | ||
| version = meta_dict.pop("version") | ||
| 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) | ||
| @@ -678,6 +744,22 @@ def to_dict(self) -> Dict[str, Any]: | ||
| return res_dict | ||
| def verify_length_and_hashes(self, data: Union[bytes, BinaryIO]): | ||
| """Verifies that the length and hashes of "data" match expected | ||
| values. | ||
| Args: | ||
| data: File object or its content in bytes. | ||
| Raises: | ||
| LengthOrHashMismatchError: Calculated length or hashes do not | ||
| match expected values. | ||
| """ | ||
| 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: | ||
| self._verify_hashes(data, self.hashes) | ||
| class Timestamp(Signed): | ||
| """A container for the signed part of timestamp metadata. | ||
| @@ -905,7 +987,7 @@ def to_dict(self) -> Dict[str, Any]: | ||
| } | ||
| class TargetFile: | ||
| class TargetFile(BaseFile): | ||
| """A container with information about a particular target file. | ||
| Attributes: | ||
| @@ -923,12 +1005,6 @@ class TargetFile: | ||
| """ | ||
| @property | ||
| def custom(self): | ||
| if self.unrecognized_fields is None: | ||
| return None | ||
| return self.unrecognized_fields.get("custom", None) | ||
| def __init__( | ||
| self, | ||
| length: int, | ||
| @@ -939,11 +1015,24 @@ def __init__( | ||
| self.hashes = hashes | ||
| self.unrecognized_fields = unrecognized_fields or {} | ||
| @property | ||
| def custom(self): | ||
| if self.unrecognized_fields is None: | ||
| return None | ||
| return self.unrecognized_fields.get("custom", None) | ||
jku marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| @classmethod | ||
| def from_dict(cls, target_dict: Dict[str, Any]) -> "TargetFile": | ||
| """Creates TargetFile object from its dict representation.""" | ||
| 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") | ||
jku marked this conversation as resolved.
Uh oh!There was an error while loading. Please reload this page. | ||
| # All fields left in the target_dict are unrecognized. | ||
| return cls(length, hashes, target_dict) | ||
| @@ -955,6 +1044,18 @@ def to_dict(self) -> Dict[str, Any]: | ||
| **self.unrecognized_fields, | ||
| } | ||
| def verify_length_and_hashes(self, data: Union[bytes, BinaryIO]): | ||
| """Verifies that the length and hashes of "data" match expected | ||
| values. | ||
| Args: | ||
| data: File object or its content in bytes. | ||
| Raises: | ||
| LengthOrHashMismatchError: Calculated length or hashes do not | ||
| match expected values. | ||
| """ | ||
| self._verify_length(data, self.length) | ||
| self._verify_hashes(data, self.hashes) | ||
| class Targets(Signed): | ||
| """A container for the signed part of targets metadata. | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess this is where we get to decide what to do with input data that we don't have a "natural" check for: length and version get validated "enough" like this IMO but hashes could still be anything...
For the values we could run
But maybe we don't need to? Is it enough that the hash verification will fail when it's tried on completely bogus hash value?
For the dict keys: Can we make SSLib check that the algorithms are known to SSLib?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah ... I avoided intentionally the downward validation spiral here ...
Hash values: there is even a regex about correct hash value in formats.py now but I wasn't sure if it isn't an overkill.
Dict keys: Probably we can pre-define somewhere supported algorithms by sslib because now the check is done runtime during hash calculation.
I will file an issue about it, especially hash values.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here it is: #1441