From 26551b95c9d00ab8218c1b928055a6e09130f476 Mon Sep 17 00:00:00 2001 From: dachshund Date: Wed, 11 Sep 2013 17:46:29 -0400 Subject: [PATCH 1/2] Fix #102. --- tuf/__init__.py | 9 ++- tuf/client/updater.py | 167 +++++++++++++++++++++++++----------------- tuf/util.py | 16 ++-- 3 files changed, 116 insertions(+), 76 deletions(-) diff --git a/tuf/__init__.py b/tuf/__init__.py index f44657e6..503e4fcf 100755 --- a/tuf/__init__.py +++ b/tuf/__init__.py @@ -176,7 +176,14 @@ class UnsupportedLibraryError(Error): class DecompressionError(Error): """Indicate that some error happened while decompressing a file.""" - pass + + def __init__(self, exception): + # Store the original exception. + self.exception = exception + + def __str__(self): + # Show the original exception. + return str(self.exception) diff --git a/tuf/client/updater.py b/tuf/client/updater.py index e2e90207..1d924f8c 100755 --- a/tuf/client/updater.py +++ b/tuf/client/updater.py @@ -636,20 +636,22 @@ def __check_hashes(self, file_object, trusted_hashes): - def __hard_check_length(self, file_object, trusted_length): + def __hard_check_compressed_file_length(self, file_object, + compressed_file_length): """ - A helper function that checks the expected length of a file-like object. - The length of the file must be strictly equal to the expected length. - This is a deliberately redundant implementation designed to complement - tuf.download._check_downloaded_length(). + A helper function that checks the expected compressed length of a + file-like object. The length of the file must be strictly equal to the + expected length. This is a deliberately redundant implementation designed + to complement tuf.download._check_downloaded_length(). file_object: A file-like object. - trusted_length: - A nonnegative integer that is the expected length of the file. + compressed_file_length: + A nonnegative integer that is the expected compressed length of the + file. tuf.DownloadLengthMismatchError, if the lengths don't match. @@ -662,28 +664,34 @@ def __hard_check_length(self, file_object, trusted_length): """ - observed_length = len(file_object) - if observed_length != trusted_length: - raise tuf.DownloadLengthMismatchError(trusted_length, observed_length) + observed_length = file_object.get_compressed_length() + if observed_length != compressed_file_length: + raise tuf.DownloadLengthMismatchError(compressed_file_length, + observed_length) + else: + logger.debug('file length ('+str(observed_length)+\ + ') == trusted length ('+str(compressed_file_length)+')') - def __soft_check_length(self, file_object, trusted_length): + def __soft_check_compressed_file_length(self, file_object, + compressed_file_length): """ - A helper function that checks the expected length of a file-like object. - The length of the file must be less than or equal to the expected length. - This is a deliberately redundant implementation designed to complement - tuf.download._check_downloaded_length(). + A helper function that checks the expected compressed length of a + file-like object. The length of the file must be less than or equal to + the expected length. This is a deliberately redundant implementation + designed to complement tuf.download._check_downloaded_length(). file_object: A file-like object. - trusted_length: - A nonnegative integer that is the expected length of the file. + compressed_file_length: + A nonnegative integer that is the expected compressed length of the + file. tuf.DownloadLengthMismatchError, if the lengths don't match. @@ -696,15 +704,20 @@ def __soft_check_length(self, file_object, trusted_length): """ - observed_length = len(file_object) - if observed_length > trusted_length: - raise tuf.DownloadLengthMismatchError(trusted_length, observed_length) + observed_length = file_object.get_compressed_length() + if observed_length > compressed_file_length: + raise tuf.DownloadLengthMismatchError(compressed_file_length, + observed_length) + else: + logger.debug('file length ('+str(observed_length)+\ + ') <= trusted length ('+str(compressed_file_length)+')') - def get_target_file(self, target_filepath, file_length, file_hashes): + def get_target_file(self, target_filepath, compressed_file_length, + uncompressed_file_hashes): """ Safely download a target file up to a certain length, and check its @@ -714,10 +727,11 @@ def get_target_file(self, target_filepath, file_length, file_hashes): target_filepath: The relative target filepath obtained from TUF targets metadata. - file_length: - The expected length of the target file. + compressed_file_length: + The expected compressed length of the target file. If the file is not + compressed, then it will simply be its uncompressed length. - file_hashes: + uncompressed_file_hashes: The expected hashes of the target file. @@ -735,24 +749,25 @@ def get_target_file(self, target_filepath, file_length, file_hashes): """ - def verify_decompressed_target_file(target_file_object): + def verify_uncompressed_target_file(target_file_object): # Every target file must have its length and hashes inspected. - self.__hard_check_length(target_file_object, file_length) - self.__check_hashes(target_file_object, file_hashes) + self.__hard_check_compressed_file_length(target_file_object, + compressed_file_length) + self.__check_hashes(target_file_object, uncompressed_file_hashes) - return self.__get_file(target_filepath, verify_decompressed_target_file, - 'target', file_length, download_safely=True, - compression=None) + return self.__get_file(target_filepath, verify_uncompressed_target_file, + 'target', compressed_file_length, + download_safely=True, compression=None) - def __verify_decompressed_metadata_file(self, metadata_file_object, + def __verify_uncompressed_metadata_file(self, metadata_file_object, metadata_role): """ - A private helper function to verify a decompressed downloaded metadata + A private helper function to verify an uncompressed downloaded metadata file. @@ -827,7 +842,7 @@ def __verify_decompressed_metadata_file(self, metadata_file_object, def unsafely_get_metadata_file(self, metadata_role, metadata_filepath, - file_length): + compressed_file_length): """ Unsafely download a metadata file up to a certain length. The actual file @@ -841,8 +856,9 @@ def unsafely_get_metadata_file(self, metadata_role, metadata_filepath, metadata_filepath: The relative metadata filepath. - file_length: - The expected length of the metadata file. + compressed_file_length: + The expected compressed length of the metadata file. If the file is not + compressed, then it will simply be its uncompressed length. tuf.NoWorkingMirrorError: @@ -859,14 +875,15 @@ def unsafely_get_metadata_file(self, metadata_role, metadata_filepath, """ - def unsafely_verify_decompressed_metadata_file(metadata_file_object): - self.__soft_check_length(metadata_file_object, file_length) - self.__verify_decompressed_metadata_file(metadata_file_object, + def unsafely_verify_uncompressed_metadata_file(metadata_file_object): + self.__soft_check_compressed_file_length(metadata_file_object, + compressed_file_length) + self.__verify_uncompressed_metadata_file(metadata_file_object, metadata_role) return self.__get_file(metadata_filepath, - unsafely_verify_decompressed_metadata_file, 'meta', - file_length, download_safely=False, + unsafely_verify_uncompressed_metadata_file, 'meta', + compressed_file_length, download_safely=False, compression=None) @@ -874,7 +891,8 @@ def unsafely_verify_decompressed_metadata_file(metadata_file_object): def safely_get_metadata_file(self, metadata_role, metadata_filepath, - file_length, file_hashes, compression): + compressed_file_length, + uncompressed_file_hashes, compression): """ Safely download a metadata file up to a certain length, and check its @@ -887,10 +905,11 @@ def safely_get_metadata_file(self, metadata_role, metadata_filepath, metadata_filepath: The relative metadata filepath. - file_length: - The expected length of the metadata file. + compressed_file_length: + The expected compressed length of the metadata file. If the file is not + compressed, then it will simply be its uncompressed length. - file_hashes: + uncompressed_file_hashes: The expected hashes of the metadata file. compression: @@ -911,15 +930,16 @@ def safely_get_metadata_file(self, metadata_role, metadata_filepath, """ - def safely_verify_decompressed_metadata_file(metadata_file_object): - self.__hard_check_length(metadata_file_object, file_length) - self.__check_hashes(metadata_file_object, file_hashes) - self.__verify_decompressed_metadata_file(metadata_file_object, + def safely_verify_uncompressed_metadata_file(metadata_file_object): + self.__hard_check_compressed_file_length(metadata_file_object, + compressed_file_length) + self.__check_hashes(metadata_file_object, uncompressed_file_hashes) + self.__verify_uncompressed_metadata_file(metadata_file_object, metadata_role) return self.__get_file(metadata_filepath, - safely_verify_decompressed_metadata_file, 'meta', - file_length, download_safely=True, + safely_verify_uncompressed_metadata_file, 'meta', + compressed_file_length, download_safely=True, compression=compression) @@ -929,8 +949,8 @@ def safely_verify_decompressed_metadata_file(metadata_file_object): # TODO: Instead of the more fragile 'download_safely' switch, unroll the # function into two separate ones: one for "safe" download, and the other one # for "unsafe" download? This should induce safer and more readable code. - def __get_file(self, filepath, verify_decompressed_file, file_type, - file_length, download_safely, compression): + def __get_file(self, filepath, verify_uncompressed_file, file_type, + compressed_file_length, download_safely, compression): """ Try downloading, up to a certain length, a metadata or target file from a @@ -941,9 +961,9 @@ def __get_file(self, filepath, verify_decompressed_file, file_type, filepath: The relative metadata or target filepath. - verify_decompressed_file: - A function which expects a decompressed file-like object and which will - raise an exception in case the file is not valid for any reason. + verify_uncompressed_file: + A function which expects an uncompressed file-like object and which + will raise an exception in case the file is not valid for any reason. file_type: Type of data needed for download, must correspond to one of the strings @@ -951,8 +971,9 @@ def __get_file(self, filepath, verify_decompressed_file, file_type, 'target' for target file type. It should correspond to NAME_SCHEMA format. - file_length: - The expected length of the metadata or target file. + compressed_file_length: + The expected compressed length of the target or metadata file. If the + file is not compressed, then it will simply be its uncompressed length. download_safely: A boolean switch to toggle safe or unsafe download of the file. @@ -984,9 +1005,11 @@ def __get_file(self, filepath, verify_decompressed_file, file_type, for file_mirror in file_mirrors: try: if download_safely: - file_object = tuf.download.safe_download(file_mirror, file_length) + file_object = tuf.download.safe_download(file_mirror, + compressed_file_length) else: - file_object = tuf.download.unsafe_download(file_mirror, file_length) + file_object = tuf.download.unsafe_download(file_mirror, + compressed_file_length) if compression: logger.debug('Decompressing '+str(file_mirror)) @@ -994,7 +1017,7 @@ def __get_file(self, filepath, verify_decompressed_file, file_type, else: logger.debug('Not decompressing '+str(file_mirror)) - verify_decompressed_file(file_object) + verify_uncompressed_file(file_object) except Exception, exception: # Remember the error from this mirror, and "reset" the target file. @@ -1033,6 +1056,9 @@ def _update_metadata(self, metadata_role, fileinfo, compression=None): A dictionary containing length and hashes of the metadata file. Ex: {"hashes": {"sha256": "3a5a6ec1f353...dedce36e0"}, "length": 1340} + The length must be that of the compressed metadata file if it is + compressed, or uncompressed metadata file if it is uncompressed. + The hashes must be that of the uncompressed metadata file. STRICT_REQUIRED_LENGTH: A Boolean indicator used to signal whether we should perform strict @@ -1074,8 +1100,8 @@ def _update_metadata(self, metadata_role, fileinfo, compression=None): # Extract file length and file hashes. They will be passed as arguments # to 'download_file' function. - file_length = fileinfo['length'] - file_hashes = fileinfo['hashes'] + compressed_file_length = fileinfo['length'] + uncompressed_file_hashes = fileinfo['hashes'] # Attempt a file download from each mirror until the file is downloaded and # verified. If the signature of the downloaded file is valid, proceed, @@ -1100,11 +1126,12 @@ def _update_metadata(self, metadata_role, fileinfo, compression=None): if metadata_role == 'timestamp': metadata_file_object = \ self.unsafely_get_metadata_file(metadata_role, metadata_filename, - file_length) + compressed_file_length) else: metadata_file_object = \ self.safely_get_metadata_file(metadata_role, metadata_filename, - file_length, file_hashes, + compressed_file_length, + uncompressed_file_hashes, compression=compression) # The metadata has been verified. Move the metadata file into place. @@ -1128,9 +1155,11 @@ def _update_metadata(self, metadata_role, fileinfo, compression=None): # 'metadata_file_object' is an instance of tuf.util.TempFile. metadata_signable = tuf.util.load_json_string(metadata_file_object.read()) if compression == 'gzip': - current_uncompressed_filepath = os.path.join(self.metadata_directory['current'], - uncompressed_metadata_filename) - current_uncompressed_filepath = os.path.abspath(current_uncompressed_filepath) + current_uncompressed_filepath = \ + os.path.join(self.metadata_directory['current'], + uncompressed_metadata_filename) + current_uncompressed_filepath = \ + os.path.abspath(current_uncompressed_filepath) metadata_file_object.move(current_uncompressed_filepath) else: metadata_file_object.move(current_filepath) @@ -1251,7 +1280,7 @@ def _update_metadata_if_changed(self, metadata_role, referenced_metadata='releas compressed_fileinfo = self.metadata['current'][referenced_metadata] \ ['meta'][gzip_metadata_filename] # NOTE: When we download the compressed file, we care about its - # compressed length. However, we check the hash of the decompressed + # compressed length. However, we check the hash of the uncompressed # file; therefore we use the hashes of the uncompressed file. fileinfo = {'length': compressed_fileinfo['length'], 'hashes': uncompressed_fileinfo['hashes']} @@ -2236,7 +2265,7 @@ def _visit_child_role(self, child_role, target_filepath): Ensure that we explore only delegated roles trusted with the target. We assume conservation of delegated paths in the complete tree of delegations. Note that the call to _ensure_all_targets_allowed in - __verify_decompressed_metadata_file should already ensure that all + __verify_uncompressed_metadata_file should already ensure that all targets metadata is valid; i.e. that the targets signed by a delegatee is a proper subset of the targets delegated to it by the delegator. Nevertheless, we check it again here for performance and safety reasons. diff --git a/tuf/util.py b/tuf/util.py index 2c2f01d4..1460e838 100755 --- a/tuf/util.py +++ b/tuf/util.py @@ -93,10 +93,11 @@ def __init__(self, prefix='tuf_temp_'): - def __len__(self): + def get_compressed_length(self): """ - Initializes TempFile. + Get the compressed length of the file. This will be correct information + even when the file is read as an uncompressed one. None. @@ -105,10 +106,12 @@ def __len__(self): OSError. - Nonnegative integer representing file size. + Nonnegative integer representing compressed file size. """ + # Even if we read a compressed file with the gzip standard library module, + # the original file will remain compressed. return os.stat(self.temporary_file.name).st_size @@ -295,9 +298,10 @@ def decompress_temp_file_object(self, compression): self._orig_file = self.temporary_file try: - self.temporary_file = gzip.GzipFile(fileobj=self.temporary_file, mode='rb') - except: - raise tuf.DecompressionError(self.temporary_file) + self.temporary_file = gzip.GzipFile(fileobj=self.temporary_file, + mode='rb') + except Exception, exception: + raise tuf.DecompressionError(exception) From dc146f97a141e80fe824c4b0b6edaa55aa1e8290 Mon Sep 17 00:00:00 2001 From: vladdd Date: Thu, 12 Sep 2013 08:14:18 -0400 Subject: [PATCH 2/2] Fix test_util_test_tools.py test case failure Clear the keystore before initializing the util_test_tools.py repository and again after every test case exits. --- tests/unit/test_util_test_tools.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/tests/unit/test_util_test_tools.py b/tests/unit/test_util_test_tools.py index 1ecfa219..825f4aab 100755 --- a/tests/unit/test_util_test_tools.py +++ b/tests/unit/test_util_test_tools.py @@ -22,13 +22,17 @@ import unittest import tuf.tests.util_test_tools as util_test_tools - +import tuf.repo.keystore class test_UtilTestTools(unittest.TestCase): def setUp(self): unittest.TestCase.setUp(self) + # Ensure the keystore is empty prior to initializing the repository + # generated by 'util_test_tools'. + tuf.repo.keystore.clear_keystore() + # Unpacking necessary parameters returned from init_repo() essential_params = util_test_tools.init_repo(tuf=True) self.root_repo = essential_params[0] @@ -42,11 +46,16 @@ def setUp(self): def tearDown(self): unittest.TestCase.tearDown(self) + + # 'util_test_tools.cleanup()' should clear the keystore... util_test_tools.cleanup(self.root_repo, self.server_proc) + # Clear the keystore here just in case. + tuf.repo.keystore.clear_keystore() + #================================================# -# Below are few quick tests to make sure that # +# Below are a few quick tests that make sure # # everything works smoothly in util_test_tools. # #================================================#