Merge pull request #988 from joshuagl/joshuagl/issue-933

Remove root from snapshot

docs/METADATA.md

@@ -64,7 +64,7 @@ of delegated Targets metadata and [example](https://raw.githubusercontent.com/th
 
 Signed by: Snapshot role.
 
-The snapshot.json metadata file lists hashes and sizes of all metadata files other than timestamp.json. This file ensures that clients will see a consistent view of the files on the repository. That is, metadata files (and thus target file) that existed on the repository at different times cannot be combined and presented to clients by an attacker.
+The snapshot.json metadata file lists the version, and optionally the file hashes and sizes, of the top-level targets metadata and all delegated targets metadata. This file ensures that clients will see a consistent view of the files on the repository. That is, metadata files (and thus target file) that existed on the repository at different times cannot be combined and presented to clients by an attacker.
 
 ​See [example](https://raw.githubusercontent.com/theupdateframework/tuf/develop/tests/repository_data/repository/metadata/snapshot.json) of Snapshot metadata.
 

tests/repository_data/generate.py

@@ -20,7 +20,7 @@
   Provide a set of pre-generated key files and a basic repository that unit
   tests can use in their test cases.  The pre-generated files created by this
   script should be copied by the unit tests as needed.  The original versions
-  should be preserved.  'tuf/tests/unit/repository_files/' will store the files
+  should be preserved.  'tuf/tests/repository_data/' will store the files
   generated.  'generate.py' should not require re-execution if the
   pre-generated repository files have already been created, unless they need to
   change in some way.
@@ -107,6 +107,11 @@
 if not options.dry_run:
   with open(target1_filepath, 'wt') as file_object:
     file_object.write('This is an example target file.')
+  # As we will add this file's permissions to the custom_attribute in the
+  # target's metadata we need to ensure that the file has the same
+  # permissions when created by this script regardless of umask value on
+  # the host system generating the data
+  os.chmod(target1_filepath, 0o644)
 
   with open(target2_filepath, 'wt') as file_object:
     file_object.write('This is an another example target file.')
@@ -119,12 +124,12 @@
 # about the target (i.e., file permissions in octal format.)
 octal_file_permissions = oct(os.stat(target1_filepath).st_mode)[4:]
 file_permissions = {'file_permissions': octal_file_permissions}
-repository.targets.add_target(target1_filepath, file_permissions)
-repository.targets.add_target(target2_filepath)
+repository.targets.add_target(os.path.basename(target1_filepath), file_permissions)
+repository.targets.add_target(os.path.basename(target2_filepath))
 
 repository.targets.delegate('role1', [delegation_public],
     [os.path.basename(target3_filepath)])
-repository.targets('role1').add_target(target3_filepath)
+repository.targets('role1').add_target(os.path.basename(target3_filepath))
 repository.targets('role1').load_signing_key(delegation_private)
 
 repository.targets('role1').delegate('role2', [delegation_public], [])

tests/test_repository_lib.py

@@ -433,7 +433,6 @@ def test_generate_snapshot_metadata(self):
     metadata_directory = os.path.join(repository_directory,
                                       repo_lib.METADATA_STAGED_DIRECTORY_NAME)
     targets_directory = os.path.join(repository_directory, repo_lib.TARGETS_DIRECTORY_NAME)
-    root_filename = os.path.join(metadata_directory, repo_lib.ROOT_FILENAME)
     targets_filename = os.path.join(metadata_directory,
                                     repo_lib.TARGETS_FILENAME)
     version = 1
@@ -453,12 +452,11 @@ def test_generate_snapshot_metadata(self):
     with open(invalid_metadata_file, 'w') as file_object:
       file_object.write('bad extension on metadata file')
 
-    root_filename = 'root'
     targets_filename = 'targets'
 
     snapshot_metadata = \
       repo_lib.generate_snapshot_metadata(metadata_directory, version,
-                                          expiration_date, root_filename,
+                                          expiration_date,
                                           targets_filename,
                                           consistent_snapshot=False)
     self.assertTrue(tuf.formats.SNAPSHOT_SCHEMA.matches(snapshot_metadata))
@@ -467,22 +465,19 @@ def test_generate_snapshot_metadata(self):
     # Test improperly formatted arguments.
     self.assertRaises(securesystemslib.exceptions.FormatError, repo_lib.generate_snapshot_metadata,
                       3, version, expiration_date,
-                      root_filename, targets_filename, consistent_snapshot=False)
+                      targets_filename, consistent_snapshot=False)
     self.assertRaises(securesystemslib.exceptions.FormatError, repo_lib.generate_snapshot_metadata,
                       metadata_directory, '3', expiration_date,
-                      root_filename, targets_filename, consistent_snapshot=False)
+                      targets_filename, consistent_snapshot=False)
     self.assertRaises(securesystemslib.exceptions.FormatError, repo_lib.generate_snapshot_metadata,
                       metadata_directory, version, '3',
-                      root_filename, targets_filename, consistent_snapshot=False)
+                      targets_filename, consistent_snapshot=False)
     self.assertRaises(securesystemslib.exceptions.FormatError, repo_lib.generate_snapshot_metadata,
                       metadata_directory, version, expiration_date,
-                      3, targets_filename, consistent_snapshot=False)
+                      3, consistent_snapshot=False)
     self.assertRaises(securesystemslib.exceptions.FormatError, repo_lib.generate_snapshot_metadata,
                       metadata_directory, version, expiration_date,
-                      root_filename, 3, consistent_snapshot=False)
-    self.assertRaises(securesystemslib.exceptions.FormatError, repo_lib.generate_snapshot_metadata,
-                      metadata_directory, version, expiration_date,
-                      root_filename, targets_filename, 3)
+                      targets_filename, 3)
 
 
 

tests/test_updater.py

@@ -886,7 +886,6 @@ def test_3__update_metadata_if_changed(self):
     self.repository_updater._update_metadata('timestamp', DEFAULT_TIMESTAMP_FILELENGTH)
     self.repository_updater._update_metadata_if_changed('snapshot', 'timestamp')
     self.repository_updater._update_metadata_if_changed('targets')
-    self.repository_updater._update_metadata_if_changed('root')
     targets_path = os.path.join(self.client_metadata_current, 'targets.json')
     self.assertTrue(os.path.exists(targets_path))
     self.assertTrue(self.repository_updater.metadata['current']['targets'])

tuf/client/updater.py

@@ -589,7 +589,7 @@ class Updater(object):
   <Updater Methods>
     refresh():
       This method downloads, verifies, and loads metadata for the top-level
-      roles in a specific order (i.e., timestamp -> snapshot -> root -> targets)
+      roles in a specific order (i.e., root -> timestamp -> snapshot -> targets)
       The expiration time for downloaded metadata is also verified.
 
       The metadata for delegated roles are not refreshed by this method, but by
@@ -1002,7 +1002,7 @@ 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 (if necessary) -> targets.
+      root (if necessary) -> timestamp -> snapshot -> targets.
 
       Delegated metadata is not refreshed by this method. After this method is
       called, the use of get_one_valid_targetinfo() will update delegated
@@ -1072,10 +1072,8 @@ def refresh(self, unsafely_update_root_if_necessary=True):
         logger.info('An expired Root metadata was loaded and must be updated.')
         raise
 
-    # TODO: How should the latest root metadata be verified?  According to the
-    # currently trusted root keys?  What if all of the currently trusted
-    # root keys have since been revoked by the latest metadata?  Alternatively,
-    # do we blindly trust the downloaded root metadata here?
+    # Update the root metadata and verify it by building a chain of trusted root
+    # keys from the current trusted root metadata file
     self._update_root_metadata(root_metadata)
 
     # Ensure that the role and key information of the top-level roles is the
@@ -1093,9 +1091,6 @@ def refresh(self, unsafely_update_root_if_necessary=True):
     # require strict checks on its required length.
     self._update_metadata('timestamp', DEFAULT_TIMESTAMP_UPPERLENGTH)
 
-    # TODO: After fetching snapshot.json, we should either verify the root
-    # fileinfo referenced there matches what was fetched earlier in
-    # _update_root_metadata() or make another attempt to download root.json.
     self._update_metadata_if_changed('snapshot',
         referenced_metadata='timestamp')
     self._update_metadata_if_changed('targets')
@@ -1836,21 +1831,23 @@ def _update_metadata_if_changed(self, metadata_role,
     """
     <Purpose>
       Non-public method that updates the metadata for 'metadata_role' if it has
-      changed.  With the exception of the 'timestamp' role, all the top-level
+      changed.  All top-level roles other than the 'timestamp' and 'root'
       roles are updated by this method.  The 'timestamp' role is always
       downloaded from a mirror without first checking if it has been updated;
       it is updated in refresh() by calling _update_metadata('timestamp').
+      The 'root' role is always updated first and verified based on the trusted
+      root metadata file the client already has a copy of; it is updated in
+      refresh() by calling _update_root_metadata().
       This method is also called for delegated role metadata, which are
       referenced by 'snapshot'.
 
       If the metadata needs to be updated but an update cannot be obtained,
-      this method will delete the file (with the exception of the root
-      metadata, which never gets removed without a replacement).
+      this method will delete the file.
 
       Due to the way in which metadata files are updated, it is expected that
       'referenced_metadata' is not out of date and trusted.  The refresh()
-      method updates the top-level roles in 'timestamp -> snapshot ->
-      root -> targets' order.  For delegated metadata, the parent role is
+      method updates the top-level roles in 'root -> timestamp -> snapshot ->
+      targets' order.  For delegated metadata, the parent role is
       updated before the delegated role.  Taking into account that
       'referenced_metadata' is updated and verified before 'metadata_role',
       this method determines if 'metadata_role' has changed by checking
@@ -1925,9 +1922,9 @@ def _update_metadata_if_changed(self, metadata_role,
       self._ensure_not_expired(self.metadata['current'][metadata_role],
           metadata_role)
 
-      # TODO: If 'metadata_role' is root or snapshot, we should verify that
-      # root's hash matches what's in snapshot, and that snapshot hash matches
-      # what's listed in timestamp.json.
+      # TODO: If metadata role is snapshot, we should verify that snapshot's
+      # hash matches what's listed in timestamp.json per step 3.1 of the
+      # detailed workflows in the specification
 
       return
 
@@ -1940,9 +1937,6 @@ def _update_metadata_if_changed(self, metadata_role,
     if metadata_role == 'snapshot':
       upperbound_filelength = tuf.settings.DEFAULT_SNAPSHOT_REQUIRED_LENGTH
 
-    elif metadata_role == 'root':
-      upperbound_filelength = DEFAULT_ROOT_UPPERLENGTH
-
     # The metadata is considered Targets (or delegated Targets metadata).
     else:
       upperbound_filelength = tuf.settings.DEFAULT_TARGETS_REQUIRED_LENGTH

tuf/repository_lib.py

@@ -121,11 +121,10 @@ def _generate_and_write_metadata(rolename, metadata_filename,
 
 
   elif rolename == 'snapshot':
-    root_filename = ROOT_FILENAME[:-len(METADATA_EXTENSION)]
     targets_filename = TARGETS_FILENAME[:-len(METADATA_EXTENSION)]
     metadata = generate_snapshot_metadata(metadata_directory,
-        roleinfo['version'], roleinfo['expires'], root_filename,
-        targets_filename, consistent_snapshot, repository_name)
+        roleinfo['version'], roleinfo['expires'], targets_filename,
+        consistent_snapshot, repository_name)
 
 
     _log_warning_if_expires_soon(SNAPSHOT_FILENAME, roleinfo['expires'],
@@ -1356,8 +1355,7 @@ def generate_targets_metadata(targets_directory, target_files, version,
 
 
 def generate_snapshot_metadata(metadata_directory, version, expiration_date,
-    root_filename, targets_filename, consistent_snapshot=False,
-    repository_name='default'):
+    targets_filename, consistent_snapshot=False, repository_name='default'):
   """
   <Purpose>
     Create the snapshot metadata.  The minimum metadata must exist (i.e.,
@@ -1379,10 +1377,6 @@ def generate_snapshot_metadata(metadata_directory, version, expiration_date,
       The expiration date of the metadata file.
       Conformant to 'securesystemslib.formats.ISO8601_DATETIME_SCHEMA'.
 
-    root_filename:
-      The filename of the top-level root role.  The hash and file size of this
-      file is listed in the snapshot role.
-
     targets_filename:
       The filename of the top-level targets role.  The hash and file size of
       this file is listed in the snapshot role.
@@ -1417,7 +1411,6 @@ def generate_snapshot_metadata(metadata_directory, version, expiration_date,
   securesystemslib.formats.PATH_SCHEMA.check_match(metadata_directory)
   tuf.formats.METADATAVERSION_SCHEMA.check_match(version)
   securesystemslib.formats.ISO8601_DATETIME_SCHEMA.check_match(expiration_date)
-  securesystemslib.formats.PATH_SCHEMA.check_match(root_filename)
   securesystemslib.formats.PATH_SCHEMA.check_match(targets_filename)
   securesystemslib.formats.BOOLEAN_SCHEMA.check_match(consistent_snapshot)
   securesystemslib.formats.NAME_SCHEMA.check_match(repository_name)
@@ -1427,8 +1420,6 @@ def generate_snapshot_metadata(metadata_directory, version, expiration_date,
   # Snapshot's 'fileinfodict' shall contain the version number of Root,
   # Targets, and all delegated roles fo the repository.
   fileinfodict = {}
-  fileinfodict[ROOT_FILENAME] = get_metadata_versioninfo(root_filename,
-      repository_name)
   fileinfodict[TARGETS_FILENAME] = get_metadata_versioninfo(targets_filename,
       repository_name)