Update error messages, comments, and tests for snapshot merkle tree functionality.

Signed-off-by: marinamoore <[email protected]>

Author: mnm678

tests/test_repository_lib.py

@@ -439,8 +439,7 @@ def test_build_merkle_tree(self):
     temporary_directory = tempfile.mkdtemp(dir=self.temporary_directory)
     storage_backend = securesystemslib.storage.FilesystemBackend()
 
-    # Test building the tree one node at a time with identical nodes
-    # to verify the hashes
+    # Test building the tree one node at a time to verify the hashes
 
     test_nodes = {}
     test_nodes['file1'] = tuf.formats.make_metadata_fileinfo(5, None, None)

tests/test_repository_tool.py

@@ -264,11 +264,17 @@ def test_writeall(self):
     repository.mark_dirty(['role1', 'targets', 'root', 'snapshot', 'timestamp'])
     repository.writeall(snapshot_merkle=True)
 
+    # Were the merkle snapshots written?
     targets_snapshot_filepath = os.path.join(metadata_directory,
         'targets-snapshot.json')
     targets_snapshot = securesystemslib.util.load_json_file(targets_snapshot_filepath)
     tuf.formats.SNAPSHOT_MERKLE_SCHEMA.check_match(targets_snapshot)
 
+    # Does timestamp have the root hash?
+    timestamp_filepath = os.path.join(metadata_directory, 'timestamp.json')
+    timestamp = securesystemslib.util.load_json_file(timestamp_filepath)
+    timestamp['signed']['merkle_root']
+
     # Verify that status() does not raise
     # 'tuf.exceptions.InsufficientKeysError' if a top-level role
     # does not contain a threshold of keys.

tests/test_updater.py

@@ -699,7 +699,6 @@ def test_2__ensure_not_expired(self):
 
 
 
-
   def test_3__update_metadata(self):
     # Setup
     # _update_metadata() downloads, verifies, and installs the specified
@@ -843,6 +842,7 @@ def test_3__get_metadata_file(self):
 
 
 
+
   def test_3__update_metadata_if_changed(self):
     # Setup.
     # The client repository is initially loaded with only four top-level roles.
@@ -1786,6 +1786,13 @@ def test_snapshot_merkle(self):
 
     self.assertEqual(snapshot_info['version'], 1)
 
+    # verify merkle path with invalid role
+    self.assertRaises(tuf.exceptions.NoWorkingMirrorError,
+        repository_updater._verify_merkle_path, 'foo')
+
+    # Test get_one_valid_targetinfo with snapshot merkle
+    repository_updater.get_one_valid_targetinfo('file1.txt')
+
 
 
 

tuf/client/updater.py

@@ -1083,11 +1083,9 @@ def refresh(self, unsafely_update_root_if_necessary=True):
     # require strict checks on its required length.
     self._update_metadata('timestamp', DEFAULT_TIMESTAMP_UPPERLENGTH)
 
-    try:
+    if 'merkle_root' not in self.metadata['current']['timestamp']:
       # If merkle root is set, do not update snapshot metadata. Instead,
       # download the relevant merkle path when downloading a target.
-      self.metadata['current']['timestamp']['merkle_root']
-    except KeyError:
       self._update_metadata_if_changed('snapshot',
           referenced_metadata='timestamp')
     self._update_metadata_if_changed('targets')
@@ -1846,6 +1844,7 @@ def _update_metadata(self, metadata_role, upperbound_filelength, version=None,
     # 'current_metadata_object' set to 'None' if there is not an object
     # stored for 'metadata_role'.
     if snapshot_merkle:
+      # Snaphot merkle files are not signed
       updated_metadata_object=metadata_signable
     else:
       updated_metadata_object = metadata_signable['signed']
@@ -1865,9 +1864,16 @@ def _update_metadata(self, metadata_role, upperbound_filelength, version=None,
 
 
 
-  def _verify_merkle_path(self, metadata_role, referenced_metadata='snapshot'):
+  def _verify_merkle_path(self, metadata_role):
     """
-    Download the merkle path associated with metadata_role and verify the hashes.
+    <Purpose>
+      Download the merkle path associated with metadata_role and verify the hashes.
+    <Arguments>
+      metadata_role:
+        The name of the metadata role. This should not include a file extension.
+    <Exceptions>
+      tuf.exceptions.RepositoryError:
+        If the snapshot merkle file is invalid or the verification fails
     Returns the snapshot information about metadata role.
     """
     merkle_root = self.metadata['current']['timestamp']['merkle_root']
@@ -1910,30 +1916,33 @@ def _verify_merkle_path(self, metadata_role, referenced_metadata='snapshot'):
       # If merkle_path and path_directions have different lengths,
       # the verification will not be possible
       if len(merkle_path) != len(path_directions):
-        # error
-        return
+        raise tuf.exceptions.RepositoryError('Invalid merkle path for ' +
+            metadata_role)
 
       for index in range(len(merkle_path)):
         i = str(index)
         if path_directions[i] < 0:
+          # The current node is a left node
           digest_object = securesystemslib.hash.digest()
           digest_object.update((node_hash + merkle_path[i]).encode('utf-8'))
         else:
+          # The current node is a right node
           digest_object = securesystemslib.hash.digest()
           digest_object.update((merkle_path[i] + node_hash).encode('utf-8'))
         node_hash = digest_object.hexdigest()
 
       # Does the result match the merkle root?
       if node_hash != merkle_root:
-        # error
-        return 1
+        raise tuf.exceptions.RepositoryError('The merkle root does not match ' +
+            'the hash for ' + metadata_role)
 
       # return the verified snapshot contents
       return contents
 
     else:
-      # No merkle path found, error?
-      return 2
+      # No merkle path found
+      raise tuf.exceptions.RepositoryError('No snapshot merkle file for ' +
+          metadata_role)
 
 
 
@@ -2017,10 +2026,9 @@ def _update_metadata_if_changed(self, metadata_role,
         repr(referenced_metadata)+ '.  ' + repr(metadata_role) +
         ' may be updated.')
 
-    if 'merkle_root' in self.metadata['current'][referenced_metadata]:
+    if 'merkle_root' in self.metadata['current']['timestamp']:
       # Download version information from merkle tree
-      contents = self._verify_merkle_path(metadata_filename,
-          referenced_metadata=referenced_metadata)
+      contents = self._verify_merkle_path(metadata_role)
       expected_versioninfo = contents
 
     else:

tuf/repository_lib.py

@@ -138,8 +138,6 @@ def _generate_and_write_metadata(rolename, metadata_filename,
           use_length=use_snapshot_length, use_hashes=use_snapshot_hashes,
           snapshot_merkle=True)
 
-      print_merkle_tree(root)
-
       # Add the merkle tree root hash to the timestamp roleinfo
       timestamp_roleinfo = tuf.roledb.get_roleinfo('timestamp', repository_name)
       timestamp_roleinfo['merkle_root'] = root.hash()
@@ -2013,17 +2011,14 @@ def generate_timestamp_metadata(snapshot_file_path, version, expiration_date,
       tuf.formats.make_metadata_fileinfo(snapshot_version['version'],
           length, hashes)
 
-  if(roleinfo):
-    try:
-      merkle_root = roleinfo['merkle_root']
-      return tuf.formats.build_dict_conforming_to_schema(
-          tuf.formats.TIMESTAMP_SCHEMA,
-          version=version,
-          expires=expiration_date,
-          meta=snapshot_fileinfo,
-          merkle_root=merkle_root)
-    except KeyError:
-      pass
+  if roleinfo and 'merkle_root' in roleinfo:
+    merkle_root = roleinfo['merkle_root']
+    return tuf.formats.build_dict_conforming_to_schema(
+        tuf.formats.TIMESTAMP_SCHEMA,
+        version=version,
+        expires=expiration_date,
+        meta=snapshot_fileinfo,
+        merkle_root=merkle_root)
 
   # Generate the timestamp metadata object.
   # Use generalized build_dict_conforming_to_schema func to produce a dict that