From 8edf2fc3f50c77ffb6e9d3ea0ba6cbcbbf79b952 Mon Sep 17 00:00:00 2001 From: dachshund Date: Tue, 6 Aug 2013 14:31:21 -0400 Subject: [PATCH] Removed an unsafe edge case, but updater unit tests need to be fixed. Specifically, we do not intentionally set any file metadata to be None and then download the file unsafely. Some of the tuf.client.updater unit tests fail because it was previously possible to unsafely download metadata for any role. We need to fix this. --- tuf/client/updater.py | 24 ++++++------------------ tuf/tests/test_updater.py | 12 +++++++++--- 2 files changed, 15 insertions(+), 21 deletions(-) diff --git a/tuf/client/updater.py b/tuf/client/updater.py index dac17a6c..427d271f 100755 --- a/tuf/client/updater.py +++ b/tuf/client/updater.py @@ -117,7 +117,7 @@ import tuf.sig import tuf.util -logger = logging.getLogger('tuf') +from tuf.log import logger class Updater(object): @@ -664,12 +664,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. - if fileinfo is not None: - file_length=fileinfo['length'] - file_hashes=fileinfo['hashes'] - else: - file_length=None - file_hashes=None + file_length=fileinfo['length'] + 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, @@ -836,12 +832,9 @@ def _update_metadata_if_changed(self, metadata_role, referenced_metadata='releas # The 'root' role may be updated without having 'release' # available. if referenced_metadata not in self.metadata['current']: - if metadata_role == 'root': - new_fileinfo = None - else: - message = 'Cannot update '+repr(metadata_role)+' because ' \ - +referenced_metadata+' is missing.' - raise tuf.RepositoryError(message) + message = 'Cannot update '+repr(metadata_role)+' because ' \ + +referenced_metadata+' is missing.' + raise tuf.RepositoryError(message) # The referenced metadata has been loaded. Extract the new # fileinfo for 'metadata_role' from it. else: @@ -1019,11 +1012,6 @@ def _fileinfo_has_changed(self, metadata_filename, new_fileinfo): if self.fileinfo.get(metadata_filename) is None: return True - # 'new_fileinfo' should only be 'None' if updating 'root.txt' - # without having 'release.txt'. - if new_fileinfo is None: - return True - current_fileinfo = self.fileinfo[metadata_filename] if current_fileinfo['length'] != new_fileinfo['length']: diff --git a/tuf/tests/test_updater.py b/tuf/tests/test_updater.py index ae4e7bbd..70376a02 100755 --- a/tuf/tests/test_updater.py +++ b/tuf/tests/test_updater.py @@ -61,11 +61,14 @@ class guarantees the order of unit tests. So that, 'test_something_A' roledb = tuf.roledb keydb = tuf.keydb +# This is the default metadata that we would create for the timestamp role, +# because it has no signed metadata for itself. DEFAULT_TIMESTAMP_FILEINFO = { - 'length': tuf.conf.DEFAULT_TIMESTAMP_REQUIRED_LENGTH, - 'hashes':None + 'hashes': None, + 'length': tuf.conf.DEFAULT_TIMESTAMP_REQUIRED_LENGTH } + class TestUpdater_init_(unittest_toolbox.Modified_TestCase): def test__init__exceptions(self): @@ -204,7 +207,7 @@ def _mock_download_url_to_tempfileobj(self, output): """ - def _mock_download(url, length, hashes=None, HARD_LIMIT_REQUIRED_LENGTH=True): + def _mock_download(url, length, hashes=None, STRICT_REQUIRED_LENGTH=True): if isinstance(output, (str, unicode)): file_path = output elif isinstance(output, list): @@ -505,12 +508,14 @@ def test_3__update_metadata(self): # Test: Invalid file downloaded. # Patch 'download.download_url_to_tempfileobj' function. self._mock_download_url_to_tempfileobj(self.release_filepath) + # TODO: Set fileinfo to a valid object. self.assertRaises(tuf.RepositoryError, _update_metadata, 'targets', None) # Test: normal case. # Patch 'download.download_url_to_tempfileobj' function. self._mock_download_url_to_tempfileobj(self.targets_filepath) + # TODO: Set fileinfo to a valid object. _update_metadata('targets', None) list_of_targets = self.Repository.metadata['current']['targets']['targets'] @@ -528,6 +533,7 @@ def test_3__update_metadata(self): # Re-patch 'download.download_url_to_tempfileobj' function. self._mock_download_url_to_tempfileobj(targets_filepath_compressed) + # TODO: Set fileinfo to a valid object. _update_metadata('targets', None, compression='gzip') list_of_targets = self.Repository.metadata['current']['targets']['targets']