From dd8a7eb0dd155cdb672e27d1d86d4cc07064d77b Mon Sep 17 00:00:00 2001 From: Vladimir Diaz Date: Thu, 17 Mar 2016 11:36:11 -0400 Subject: [PATCH] Review test_indefinite_freeze_attack.py and updater.py. Minor edits made --- tests/test_indefinite_freeze_attack.py | 65 ++++++++++++++------------ tuf/client/updater.py | 46 +++++++++--------- 2 files changed, 59 insertions(+), 52 deletions(-) diff --git a/tests/test_indefinite_freeze_attack.py b/tests/test_indefinite_freeze_attack.py index ce70d2b3..8644df00 100755 --- a/tests/test_indefinite_freeze_attack.py +++ b/tests/test_indefinite_freeze_attack.py @@ -191,15 +191,17 @@ def test_without_tuf(self): # Test 2 Begin: # - # 'timestamp.json' specifies the latest version of the repository files. - # A client should only accept the same version of this file up to a certain - # point, or else it cannot detect that new files are available for download. - # Modify the repository's timestamp.json' so that it expires soon, copy it - # over the to client, and attempt to re-fetch the same expired version. + # 'timestamp.json' specifies the latest version of the repository files. A + # client should only accept the same version of this file up to a certain + # point, or else it cannot detect that new files are available for + # download. Modify the repository's timestamp.json' so that it expires + # soon, copy it over to the client, and attempt to re-fetch the same + # expired version. # # A non-TUF client (without a way to detect when metadata has expired) is # expected to download the same version, and thus the same outdated files. - # Verify that the same file size and hash of 'timestamp.json' is downloaded. + # Verify that the downloaded 'timestamp.json' contains the same file size + # and hash as the one available locally. timestamp_path = os.path.join(self.repository_directory, 'metadata', 'timestamp.json') @@ -285,8 +287,8 @@ def test_with_tuf(self): repository.snapshot.load_signing_key(snapshot_private) # Expire snapshot in 8s. This should be far enough into the future that we - # haven't reached it before the first refresh validates timestamp expiry. We - # want a successful refresh before expiry, then a second refresh after + # haven't reached it before the first refresh validates timestamp expiry. + # We want a successful refresh before expiry, then a second refresh after # expiry (which we then expect to raise an exception due to expired # metadata). expiry_time = time.time() + 8 @@ -294,8 +296,9 @@ def test_with_tuf(self): repository.snapshot.expiration = datetime_object - # Now write to the repository + # Now write to the repository. repository.write() + # And move the staged metadata to the "live" metadata. shutil.rmtree(os.path.join(self.repository_directory, 'metadata')) shutil.copytree(os.path.join(self.repository_directory, 'metadata.staged'), @@ -304,27 +307,28 @@ def test_with_tuf(self): # Refresh metadata on the client. For this refresh, all data is not expired. logger.info('Test: Refreshing #1 - Initial metadata refresh occurring.') self.repository_updater.refresh() - logger.info("Test: Refreshed #1 - Initial metadata refresh completed " - "successfully. Now sleeping until snapshot metadata expires.") + logger.info('Test: Refreshed #1 - Initial metadata refresh completed ' + 'successfully. Now sleeping until snapshot metadata expires.') # Sleep until expiry_time ('repository.snapshot.expiration') - time.sleep( max(0, expiry_time - time.time()) ) + time.sleep(max(0, expiry_time - time.time())) - logger.info("Test: Refreshing #2 - Now trying to refresh again after local " - "snapshot expiry.") + logger.info('Test: Refreshing #2 - Now trying to refresh again after local' + ' snapshot expiry.') try: self.repository_updater.refresh() # We expect this to fail! - except tuf.ExpiredMetadataError as e: - logger.info("Test: Refresh #2 - failed as expected. Expired local " - "snapshot case generated a tuf.ExpiredMetadataError exception" - " as expected. Test pass.") + except tuf.ExpiredMetadataError: + logger.info('Test: Refresh #2 - failed as expected. Expired local' + ' snapshot case generated a tuf.ExpiredMetadataError' + ' exception as expected. Test pass.') + # I think that I only expect tuf.ExpiredMetadata error here. A # NoWorkingMirrorError indicates something else in this case - unavailable # repo, for example. else: - self.fail("TUF failed to detect expired stale snapshot metadata. Freeze " - "attack successful.") + self.fail('TUF failed to detect expired stale snapshot metadata. Freeze' + ' attack successful.') @@ -334,13 +338,14 @@ def test_with_tuf(self): # 'timestamp.json' specifies the latest version of the repository files. # A client should only accept the same version of this file up to a certain # point, or else it cannot detect that new files are available for download. - # Modify the repository's timestamp.json' so that it is about to expire, + # Modify the repository's 'timestamp.json' so that it is about to expire, # copy it over the to client, wait a moment until it expires, and attempt to # re-fetch the same expired version. - # The same scenario as in test_without_tuf() is followed here, except with a - # TUF client. The TUF client performs a refresh of top-level metadata, which - # includes 'timestamp.json'. + # The same scenario as in test_without_tuf() is followed here, except with + # a TUF client. The TUF client performs a refresh of top-level metadata, + # which includes 'timestamp.json', and should detect a freeze attack if + # the repository serves an outdated 'timestamp.json'. # Modify the timestamp file on the remote repository. 'timestamp.json' # must be properly updated and signed with 'repository_tool.py', otherwise @@ -368,11 +373,11 @@ def test_with_tuf(self): shutil.copytree(os.path.join(self.repository_directory, 'metadata.staged'), os.path.join(self.repository_directory, 'metadata')) - # Wait just long enough for the timestamp metadata (which is now both on the - # repository and on the client) to expire. - time.sleep( max(0, expiry_time - time.time()) ) + # Wait just long enough for the timestamp metadata (which is now both on + # the repository and on the client) to expire. + time.sleep(max(0, expiry_time - time.time())) - # Try to refresh metadata on the client. Since we're already past + # Try to refresh top-level metadata on the client. Since we're already past # 'repository.timestamp.expiration', the TUF client is expected to detect # that timestamp metadata is outdated and refuse to continue the update # process. @@ -389,8 +394,8 @@ def test_with_tuf(self): self.assertTrue(isinstance(mirror_error, tuf.ExpiredMetadataError)) else: - self.fail("TUF failed to detect expired stale timestamp metadata. Freeze " - "attack successful.") + self.fail('TUF failed to detect expired, stale timestamp metadata.' + ' Freeze attack successful.') if __name__ == '__main__': diff --git a/tuf/client/updater.py b/tuf/client/updater.py index 652f661d..9eae01ec 100755 --- a/tuf/client/updater.py +++ b/tuf/client/updater.py @@ -565,20 +565,22 @@ def refresh(self, unsafely_update_root_if_necessary=True): Update the latest copies of the metadata for the top-level roles. The update request process follows a specific order to ensure the metadata - files are securely updated: timestamp -> snapshot -> root -> targets. + files are securely updated: + timestamp -> snapshot -> root (if necessary) -> targets. Delegated metadata is not refreshed by this method. After this method is - called, the use of target methods (e.g., all_targets(), targets_of_role(), - or target()) will update delegated metadata. - Calling refresh() ensures that top-level metadata is up-to-date, so that - the target methods can refer to the latest available content. Thus, - refresh() should always be called by the client before any requests of - target file information. + called, the use of target methods (e.g., all_targets(), + targets_of_role(), or target()) will update delegated metadata, when + required. Calling refresh() ensures that top-level metadata is + up-to-date, so that the target methods can refer to the latest available + content. Thus, refresh() should always be called by the client before any + requests of target file information. - The expiration time for downloaded metadata is also verified. + The expiration time for downloaded metadata is also verified, including + local metadata that the repository claims is up to date. If the refresh fails for any reason, then unless - unsafely_update_root_if_necessary is set, refresh will be retried once + 'unsafely_update_root_if_necessary' is set, refresh will be retried once after first attempting to update the root metadata file. Only after this check will the exceptions listed here potentially be raised. @@ -656,7 +658,7 @@ def refresh(self, unsafely_update_root_if_necessary=True): # If an exception is raised during the metadata update attempts, we will # attempt to update root metadata once by recursing with a special argument - # to avoid further recursion. + # (unsafely_update_root_if_necessary) to avoid further recursion. # Use default but sane information for timestamp metadata, and do not # require strict checks on its required length. @@ -675,35 +677,33 @@ def refresh(self, unsafely_update_root_if_necessary=True): # If a change to a metadata file IS detected in an # _update_metadata_if_changed call, but we are unable to download a # valid (not expired, properly signed, valid) version of that metadata - # file, and unexpired version of that metadata file, a - # tuf.NoWorkingMirrorError rises to this point. + # file, a tuf.NoWorkingMirrorError rises to this point. # # - tuf.ExpiredMetadataError: # - # If, on the other hand, a change to a metadata file IS NOT detected in - # a given _update_metadata_if_changed call, but we observe that the + # If, on the other hand, a change to a metadata file IS NOT detected + # in a given _update_metadata_if_changed call, but we observe that the # version of the metadata file we have on hand is now expired, a # tuf.ExpiredMetadataError exception rises to this point. # - except tuf.NoWorkingMirrorError as e: + except tuf.NoWorkingMirrorError: if unsafely_update_root_if_necessary: - logger.info('Valid top-level metadata cannot be downloaded. Unsafely ' - 'update the Root metadata.') + logger.info('Valid top-level metadata cannot be downloaded. Unsafely' + ' update the Root metadata.') self._update_metadata('root', DEFAULT_ROOT_UPPERLENGTH) self.refresh(unsafely_update_root_if_necessary=False) else: raise - except tuf.ExpiredMetadataError as e: + except tuf.ExpiredMetadataError: if unsafely_update_root_if_necessary: logger.info('No changes were detected from the mirrors for a given role' - ', and that metadata that is available on disk has been found to be ' - 'expired. Trying to update root in case of foul play.') + ', and that metadata that is available on disk has been found to be' + ' expired. Trying to update root in case of foul play.') self._update_metadata('root', DEFAULT_ROOT_UPPERLENGTH) self.refresh(unsafely_update_root_if_necessary=False) - # The caller explicitly requested not to unsafely fetch an expired Root. else: logger.info('No changes were detected from the mirrors for a given role' @@ -713,6 +713,8 @@ def refresh(self, unsafely_update_root_if_necessary=True): + + def _check_hashes(self, file_object, trusted_hashes): """ @@ -1624,7 +1626,7 @@ def _update_metadata_if_changed(self, metadata_role, logger.info(repr(uncompressed_metadata_filename) + ' up-to-date.') # Since we have not downloaded a new version of this metadata, we - # should check to see if our version is stale and notify the user + # should check to see if our local version is stale and notify the user # if so. This raises tuf.ExpiredMetadataError if the metadata we # have is expired. Resolves issue #322. self._ensure_not_expired(self.metadata['current'][metadata_role],