From f12b5dbdda557f11d631a49f30f8a6c913c5f0f3 Mon Sep 17 00:00:00 2001 From: Vladimir Diaz Date: Wed, 26 Oct 2016 17:04:06 -0400 Subject: [PATCH 01/11] Add code coverage for code recently added to repository_lib.py --- tests/test_repository_lib.py | 9 +++++++++ tuf/repository_lib.py | 5 ++++- 2 files changed, 13 insertions(+), 1 deletion(-) diff --git a/tests/test_repository_lib.py b/tests/test_repository_lib.py index ff30d725..36c2d4bb 100755 --- a/tests/test_repository_lib.py +++ b/tests/test_repository_lib.py @@ -793,6 +793,15 @@ def test_write_metadata_file(self): version_number, compression_algorithms, consistent_snapshot=True) + + # Try to create a link to root.json when root.json doesn't exist locally. + # repository_lib should log a message if this is the case. + tuf.conf.CONSISTENT_METHOD = 'hard_link' + os.remove(output_filename) + repo_lib.write_metadata_file(root_signable, output_filename, + version_number, + compression_algorithms, + consistent_snapshot=True) # Reset CONSISTENT_METHOD so that subsequent tests work as expected. tuf.conf.CONSISTENT_METHOD = 'copy' diff --git a/tuf/repository_lib.py b/tuf/repository_lib.py index e03da42a..d8973487 100755 --- a/tuf/repository_lib.py +++ b/tuf/repository_lib.py @@ -2045,9 +2045,12 @@ def write_metadata_file(metadata, filename, version_number, elif (tuf.conf.CONSISTENT_METHOD == 'hard_link'): logger.info('Hard linking ' + repr(written_consistent_filename)) - # 'written_filename' must not exist, otherwise os.link complains. + # 'written_filename' must not exist, otherwise os.link() complains. if os.path.exists(written_filename): os.remove(written_filename) + + else: + logger.debug(repr(written_filename) + ' does not exist.') os.link(written_consistent_filename, written_filename) From 58af0b54284d4703209e45a9efb4228ebdb550f5 Mon Sep 17 00:00:00 2001 From: Vladimir Diaz Date: Thu, 27 Oct 2016 15:04:16 -0400 Subject: [PATCH 02/11] Add unmark_dirty() to roledb.py --- tuf/roledb.py | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tuf/roledb.py b/tuf/roledb.py index 2fe36df0..744e9f14 100755 --- a/tuf/roledb.py +++ b/tuf/roledb.py @@ -471,6 +471,52 @@ def mark_dirty(roles, repository_name='default'): +def unmark_dirty(roles, repository_name='default'): + """ + + No longer mark the roles in 'roles' as dirty. + + + repository_name: + The name of the repository to get the dirty roles. If not supplied, the + 'default' repository is searched. + + roles: + A list of roles that should no longer be marked as dirty. + + + tuf.FormatError, if the arguments are improperly formatted. + + tuf.InvalidNameError, if 'repository_name' does not exist in the role + database. + + + None. + + + None. + """ + + # Are the arguments properly formatted? If not, raise tuf.FormatError. + tuf.formats.NAMES_SCHEMA.check_match(roles) + tuf.formats.NAME_SCHEMA.check_match(repository_name) + + global _roledb_dict + global _dirty_roles + + if repository_name not in _roledb_dict or repository_name not in _dirty_roles: + raise tuf.InvalidNameError('Repository name does not' ' exist: ' + + repository_name) + + for role in roles: + try: + _dirty_roles[repository_name].remove(role) + + except ValueError: + logger.debug(repr(role) + ' is not dirty.') + + + def role_exists(rolename, repository_name='default'): """ From 15b6b50038b79aa4fa1fa897222c07c0b486194c Mon Sep 17 00:00:00 2001 From: Vladimir Diaz Date: Thu, 27 Oct 2016 15:04:51 -0400 Subject: [PATCH 03/11] Add unmark_dirty() to repository_tool.py --- tuf/repository_tool.py | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tuf/repository_tool.py b/tuf/repository_tool.py index 5543199d..1ef141af 100755 --- a/tuf/repository_tool.py +++ b/tuf/repository_tool.py @@ -439,6 +439,30 @@ def mark_dirty(self, roles): """ tuf.roledb.mark_dirty(roles) + + + + def unmark_dirty(self, roles): + """ + + No longer mark the list of 'roles' as dirty. + + + roles: + A list of roles to mark as dirty. on the next write, these roles + will be written to disk. + + + None. + + + None. + + + None. + """ + + tuf.roledb.unmark_dirty(roles) From 3b71451287a26263e3b09cf16d1b242c557f2446 Mon Sep 17 00:00:00 2001 From: Vladimir Diaz Date: Thu, 27 Oct 2016 15:07:08 -0400 Subject: [PATCH 04/11] Convert some logger.info() statements to logger.debug() and reset roles dirtied by status() --- tuf/repository_lib.py | 26 +++++++++++++++++++++++--- 1 file changed, 23 insertions(+), 3 deletions(-) diff --git a/tuf/repository_lib.py b/tuf/repository_lib.py index d8973487..3a684e53 100755 --- a/tuf/repository_lib.py +++ b/tuf/repository_lib.py @@ -1917,7 +1917,7 @@ def sign_metadata(metadata_object, keyids, filename): logger.warning('Unable to create signature for keyid: ' + repr(keyid)) else: - logger.warning('Private key unset. Skipping: ' + repr(keyid)) + logger.debug('Private key unset. Skipping: ' + repr(keyid)) else: raise tuf.Error('The keydb contains a key with an invalid key type.') @@ -2039,7 +2039,7 @@ def write_metadata_file(metadata, filename, version_number, # to its expected filename (e.g., root.json). The option of either # creating a copy or link should be configurable in tuf.conf.py. if (tuf.conf.CONSISTENT_METHOD == 'copy'): - logger.info('Pointing ' + repr(filename) + ' to the consistent snapshot.') + logger.debug('Pointing ' + repr(filename) + ' to the consistent snapshot.') shutil.copyfile(written_consistent_filename, written_filename) elif (tuf.conf.CONSISTENT_METHOD == 'hard_link'): @@ -2153,7 +2153,7 @@ def _write_compressed_metadata(file_object, compressed_filename, # Move the 'tuf.util.TempFile' object to one of the filenames so that it is # saved and the temporary file closed. if not os.path.exists(consistent_filename): - logger.info('Saving ' + repr(consistent_filename)) + logger.debug('Saving ' + repr(consistent_filename)) file_object.move(consistent_filename) else: @@ -2204,6 +2204,7 @@ def _log_status_of_top_level_roles(targets_directory, metadata_directory): # Do the top-level roles contain a valid threshold of signatures? Top-level # metadata is verified in Root -> Targets -> Snapshot -> Timestamp order. # Verify the metadata of the Root role. + root_roleinfo = tuf.roledb.get_roleinfo('root') try: signable, root_filename = \ _generate_and_write_metadata('root', root_filename, @@ -2216,7 +2217,12 @@ def _log_status_of_top_level_roles(targets_directory, metadata_directory): _log_status('root', e.signable) return + finally: + tuf.roledb.unmark_dirty(['root']) + tuf.roledb.update_roleinfo('root', root_roleinfo) + # Verify the metadata of the Targets role. + targets_roleinfo = tuf.roledb.get_roleinfo('targets') try: signable, targets_filename = \ _generate_and_write_metadata('targets', targets_filename, @@ -2226,8 +2232,13 @@ def _log_status_of_top_level_roles(targets_directory, metadata_directory): except tuf.UnsignedMetadataError as e: _log_status('targets', e.signable) return + + finally: + tuf.roledb.unmark_dirty(['targets']) + tuf.roledb.update_roleinfo('targets', targets_roleinfo) # Verify the metadata of the snapshot role. + snapshot_roleinfo = tuf.roledb.get_roleinfo('snapshot') filenames = {'root': root_filename, 'targets': targets_filename} try: signable, snapshot_filename = \ @@ -2240,7 +2251,12 @@ def _log_status_of_top_level_roles(targets_directory, metadata_directory): _log_status('snapshot', e.signable) return + finally: + tuf.roledb.unmark_dirty(['snapshot']) + tuf.roledb.update_roleinfo('snapshot', snapshot_roleinfo) + # Verify the metadata of the Timestamp role. + timestamp_roleinfo = tuf.roledb.get_roleinfo('timestamp') filenames = {'snapshot': snapshot_filename} try: signable, timestamp_filename = \ @@ -2252,6 +2268,10 @@ def _log_status_of_top_level_roles(targets_directory, metadata_directory): except tuf.UnsignedMetadataError as e: _log_status('timestamp', e.signable) return + + finally: + tuf.roledb.unmark_dirty(['timestamp']) + tuf.roledb.update_roleinfo('timestamp', timestamp_roleinfo) From 986ffcb730d2b8faebfdb44520e0e31bfed2ea2d Mon Sep 17 00:00:00 2001 From: Vladimir Diaz Date: Thu, 27 Oct 2016 15:15:50 -0400 Subject: [PATCH 05/11] Add test case for unmark_dirty() in roledb --- tests/test_roledb.py | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/tests/test_roledb.py b/tests/test_roledb.py index 6149dd5d..4f90e39e 100755 --- a/tests/test_roledb.py +++ b/tests/test_roledb.py @@ -701,6 +701,32 @@ def test_mark_dirty(self): self.assertRaises(tuf.InvalidNameError, tuf.roledb.mark_dirty, ['dirty_role'], 'non-existent') + + + def test_unmark_dirty(self): + # Add a dirty role to roledb. + rolename = 'targets' + roleinfo1 = {'keyids': ['123'], 'threshold': 1} + tuf.roledb.add_role(rolename, roleinfo1) + rolename2 = 'dirty_role' + roleinfo2 = {'keyids': ['123'], 'threshold': 2} + tuf.roledb.add_role(rolename2, roleinfo2) + mark_role_as_dirty = True + tuf.roledb.update_roleinfo(rolename, roleinfo1, mark_role_as_dirty) + # Note: The 'default' repository is searched if the repository name is + # not given to get_dirty_roles(). + self.assertEqual([rolename], tuf.roledb.get_dirty_roles()) + tuf.roledb.update_roleinfo(rolename2, roleinfo2, mark_role_as_dirty) + + tuf.roledb.unmark_dirty(['dirty_role']) + self.assertEqual([rolename], sorted(tuf.roledb.get_dirty_roles())) + tuf.roledb.unmark_dirty(['targets']) + self.assertEqual([], sorted(tuf.roledb.get_dirty_roles())) + + # Verify that a role cannot be unmarked as dirty for a non-existent + # repository. + self.assertRaises(tuf.InvalidNameError, tuf.roledb.unmark_dirty, + ['dirty_role'], 'non-existent') def _test_rolename(self, test_function): From 6a10090cd128c882d58fb14e240cc34ff4425046 Mon Sep 17 00:00:00 2001 From: Vladimir Diaz Date: Thu, 27 Oct 2016 16:26:18 -0400 Subject: [PATCH 06/11] Clean up exception message --- tuf/roledb.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tuf/roledb.py b/tuf/roledb.py index 744e9f14..15742762 100755 --- a/tuf/roledb.py +++ b/tuf/roledb.py @@ -505,8 +505,7 @@ def unmark_dirty(roles, repository_name='default'): global _dirty_roles if repository_name not in _roledb_dict or repository_name not in _dirty_roles: - raise tuf.InvalidNameError('Repository name does not' ' exist: ' + - repository_name) + raise tuf.InvalidNameError('Repository name does not exist: ' + repository_name) for role in roles: try: From 4cec0284a7903111c5215d538be4c6df7ed67aed Mon Sep 17 00:00:00 2001 From: Vladimir Diaz Date: Thu, 27 Oct 2016 16:27:07 -0400 Subject: [PATCH 07/11] Ensure all roles are undirtied after a repository.writeall() --- tuf/repository_tool.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tuf/repository_tool.py b/tuf/repository_tool.py index 1ef141af..fc04088b 100755 --- a/tuf/repository_tool.py +++ b/tuf/repository_tool.py @@ -279,7 +279,9 @@ def writeall(self, consistent_snapshot=False, compression_algorithms=['gz']): self._targets_directory, self._metadata_directory, consistent_snapshot, filenames) - + + tuf.roledb.unmark_dirty(dirty_rolenames) + # Delete the metadata of roles no longer in 'tuf.roledb'. Obsolete roles # may have been revoked and should no longer have their metadata files # available on disk, otherwise loading a repository may unintentionally From 909bd78a5d0f8b2099d91d676369dbd9d6677963 Mon Sep 17 00:00:00 2001 From: Vladimir Diaz Date: Thu, 27 Oct 2016 16:29:28 -0400 Subject: [PATCH 08/11] Fix for status(), where it was causing roles to be dirty and not resetting changes --- tuf/repository_lib.py | 45 +++++++++++++++++++++++++++++++++++++------ 1 file changed, 39 insertions(+), 6 deletions(-) diff --git a/tuf/repository_lib.py b/tuf/repository_lib.py index 3a684e53..76914642 100755 --- a/tuf/repository_lib.py +++ b/tuf/repository_lib.py @@ -2204,7 +2204,16 @@ def _log_status_of_top_level_roles(targets_directory, metadata_directory): # Do the top-level roles contain a valid threshold of signatures? Top-level # metadata is verified in Root -> Targets -> Snapshot -> Timestamp order. # Verify the metadata of the Root role. + dirty_rolenames = tuf.roledb.get_dirty_roles() + root_roleinfo = tuf.roledb.get_roleinfo('root') + root_is_dirty = None + if 'root' in dirty_rolenames: + root_is_dirty = True + + else: + root_is_dirty = False + try: signable, root_filename = \ _generate_and_write_metadata('root', root_filename, @@ -2219,10 +2228,18 @@ def _log_status_of_top_level_roles(targets_directory, metadata_directory): finally: tuf.roledb.unmark_dirty(['root']) - tuf.roledb.update_roleinfo('root', root_roleinfo) + tuf.roledb.update_roleinfo('root', root_roleinfo, + mark_role_as_dirty=root_is_dirty) # Verify the metadata of the Targets role. targets_roleinfo = tuf.roledb.get_roleinfo('targets') + targets_is_dirty = None + if 'targets' in dirty_rolenames: + targets_is_dirty = True + + else: + targets_is_dirty = False + try: signable, targets_filename = \ _generate_and_write_metadata('targets', targets_filename, @@ -2235,10 +2252,18 @@ def _log_status_of_top_level_roles(targets_directory, metadata_directory): finally: tuf.roledb.unmark_dirty(['targets']) - tuf.roledb.update_roleinfo('targets', targets_roleinfo) + tuf.roledb.update_roleinfo('targets', targets_roleinfo, + mark_role_as_dirty=targets_is_dirty) # Verify the metadata of the snapshot role. snapshot_roleinfo = tuf.roledb.get_roleinfo('snapshot') + snapshot_is_dirty = None + if 'snapshot' in dirty_rolenames: + snapshot_is_dirty = True + + else: + snapshot_is_dirty = False + filenames = {'root': root_filename, 'targets': targets_filename} try: signable, snapshot_filename = \ @@ -2253,10 +2278,18 @@ def _log_status_of_top_level_roles(targets_directory, metadata_directory): finally: tuf.roledb.unmark_dirty(['snapshot']) - tuf.roledb.update_roleinfo('snapshot', snapshot_roleinfo) + tuf.roledb.update_roleinfo('snapshot', snapshot_roleinfo, + mark_role_as_dirty=snapshot_is_dirty) # Verify the metadata of the Timestamp role. timestamp_roleinfo = tuf.roledb.get_roleinfo('timestamp') + timestamp_is_dirty = None + if 'timestamp' in dirty_rolenames: + timestamp_is_dirty = True + + else: + timestamp_is_dirty = False + filenames = {'snapshot': snapshot_filename} try: signable, timestamp_filename = \ @@ -2271,9 +2304,9 @@ def _log_status_of_top_level_roles(targets_directory, metadata_directory): finally: tuf.roledb.unmark_dirty(['timestamp']) - tuf.roledb.update_roleinfo('timestamp', timestamp_roleinfo) - - + tuf.roledb.update_roleinfo('timestamp', timestamp_roleinfo, + mark_role_as_dirty=timestamp_is_dirty) + def _log_status(rolename, signable): From 0324263988173cbde7259d3404094a68c2bbaa0b Mon Sep 17 00:00:00 2001 From: Vladimir Diaz Date: Thu, 27 Oct 2016 16:30:30 -0400 Subject: [PATCH 09/11] Verify the changed behavior of status() --- tests/test_repository_tool.py | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/tests/test_repository_tool.py b/tests/test_repository_tool.py index 30938a6f..95dfd85e 100755 --- a/tests/test_repository_tool.py +++ b/tests/test_repository_tool.py @@ -347,6 +347,19 @@ def test_writeall(self): # successfully. repo_tool.load_repository(repository_directory) + # Verify the behavior of marking and unmarking roles as dirty. + # We begin by ensuring that writeall() cleared the list of dirty roles.. + self.assertEqual([], tuf.roledb.get_dirty_roles()) + + repository.mark_dirty(['root', 'timestamp']) + self.assertEqual(['root', 'timestamp'], sorted(tuf.roledb.get_dirty_roles())) + repository.unmark_dirty(['root']) + self.assertEqual(['timestamp'], tuf.roledb.get_dirty_roles()) + + # Ensure status() does not leave behind any dirty roles. + repository.status() + self.assertEqual(['timestamp'], tuf.roledb.get_dirty_roles()) + # Test improperly formatted arguments. self.assertRaises(tuf.FormatError, repository.writeall, 3, False) self.assertRaises(tuf.FormatError, repository.writeall, False, 3) From 0e6e3418d210697519e7bef8358fe467e452c547 Mon Sep 17 00:00:00 2001 From: Vladimir Diaz Date: Thu, 27 Oct 2016 16:38:07 -0400 Subject: [PATCH 10/11] Add code coverage for non-dirty roles given to role.db.unmark_dirty() --- tests/test_roledb.py | 4 ++++ tuf/roledb.py | 2 +- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/tests/test_roledb.py b/tests/test_roledb.py index 4f90e39e..13713979 100755 --- a/tests/test_roledb.py +++ b/tests/test_roledb.py @@ -723,6 +723,10 @@ def test_unmark_dirty(self): tuf.roledb.unmark_dirty(['targets']) self.assertEqual([], sorted(tuf.roledb.get_dirty_roles())) + # What happens for a role that isn't dirty? unmark_dirty() should just + # log a message. + tuf.roledb.unmark_dirty(['unknown_role']) + # Verify that a role cannot be unmarked as dirty for a non-existent # repository. self.assertRaises(tuf.InvalidNameError, tuf.roledb.unmark_dirty, diff --git a/tuf/roledb.py b/tuf/roledb.py index 15742762..a4338213 100755 --- a/tuf/roledb.py +++ b/tuf/roledb.py @@ -511,7 +511,7 @@ def unmark_dirty(roles, repository_name='default'): try: _dirty_roles[repository_name].remove(role) - except ValueError: + except (KeyError, ValueError): logger.debug(repr(role) + ' is not dirty.') From ab35f07583d9e6cbfb0b2b00e3adcd86d3043b2d Mon Sep 17 00:00:00 2001 From: Vladimir Diaz Date: Thu, 27 Oct 2016 16:55:25 -0400 Subject: [PATCH 11/11] Unmark dirty role after successful repository.write(rolename) --- tuf/repository_tool.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tuf/repository_tool.py b/tuf/repository_tool.py index fc04088b..cdf77b5e 100755 --- a/tuf/repository_tool.py +++ b/tuf/repository_tool.py @@ -341,6 +341,9 @@ def write(self, rolename, consistent_snapshot=False, increment_version_number=Tr filenames=filenames, allow_partially_signed=True, increment_version_number=increment_version_number) + + # Ensure 'rolename' is no longer marked as dirty after the successful write(). + tuf.roledb.unmark_dirty([rolename])