mirror of
https://github.com/theupdateframework/python-tuf
synced 2026-05-24 10:08:28 +00:00
Refactor _visit_child_role, remove obsolete test conditions, and improve coverage
This commit is contained in:
Vladimir Diaz
parent
63b4d73cbd
commit
bcabe0072b
2 changed files with 36 additions and 65 deletions
|
|
@ -820,6 +820,8 @@ def test_3__update_metadata_if_changed(self):
|
|||
target3 = os.path.join(self.repository_directory, 'targets', 'file3.txt')
|
||||
|
||||
repository.targets.add_target(target3)
|
||||
repository.root.version = repository.root.version + 1
|
||||
repository.root.load_signing_key(self.role_keys['root']['private'])
|
||||
repository.targets.load_signing_key(self.role_keys['targets']['private'])
|
||||
repository.snapshot.load_signing_key(self.role_keys['snapshot']['private'])
|
||||
repository.timestamp.load_signing_key(self.role_keys['timestamp']['private'])
|
||||
|
|
@ -838,6 +840,7 @@ 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'])
|
||||
|
|
@ -845,8 +848,7 @@ def test_3__update_metadata_if_changed(self):
|
|||
|
||||
# Test for an invalid 'referenced_metadata' argument.
|
||||
self.assertRaises(tuf.exceptions.RepositoryError,
|
||||
self.repository_updater._update_metadata_if_changed,
|
||||
'snapshot', 'bad_role')
|
||||
self.repository_updater._update_metadata_if_changed, 'snapshot', 'bad_role')
|
||||
|
||||
|
||||
|
||||
|
|
@ -951,17 +953,11 @@ def test_4__refresh_targets_metadata(self):
|
|||
# Verify that client's metadata files were refreshed successfully.
|
||||
self.assertEqual(len(self.repository_updater.metadata['current']), 6)
|
||||
|
||||
# Test for compressed metadata roles.
|
||||
self.repository_updater.metadata['current']['snapshot']['meta']['targets.json.gz'] = \
|
||||
self.repository_updater.metadata['current']['snapshot']['meta']['targets.json']
|
||||
self.repository_updater._refresh_targets_metadata(refresh_all_delegated_roles=True)
|
||||
|
||||
# Test for non-existing rolename.
|
||||
self.repository_updater._refresh_targets_metadata('bad_rolename',
|
||||
refresh_all_delegated_roles=False)
|
||||
|
||||
# Test that non-json metadata in Snapshot is ignored.
|
||||
print('attempting non-json file test')
|
||||
self.repository_updater.metadata['current']['snapshot']['meta']['bad_role.xml'] = {}
|
||||
self.repository_updater._refresh_targets_metadata(refresh_all_delegated_roles=True)
|
||||
|
||||
|
|
@ -1555,27 +1551,27 @@ def test_10__visit_child_role(self):
|
|||
file_object.write(repo_lib._get_written_metadata(role1_metadata))
|
||||
|
||||
self.assertEqual(self.repository_updater._visit_child_role(child_role,
|
||||
'/target.exe', targets_role['delegations']), child_role['name'])
|
||||
'/target.exe'), child_role['name'])
|
||||
|
||||
# Test path hash prefixes.
|
||||
child_role['path_hash_prefixes'] = ['8baf', '0000']
|
||||
child_role['path_hash_prefixes'] = ['8baf']
|
||||
self.assertEqual(self.repository_updater._visit_child_role(child_role,
|
||||
'/file3.txt', targets_role['delegations']), child_role['name'])
|
||||
'/file3.txt'), child_role['name'])
|
||||
|
||||
# Test for forbidden target.
|
||||
self.repository_updater._visit_child_role(child_role,
|
||||
'/target.exe', targets_role['delegations'])
|
||||
# Test for a forbidden target.
|
||||
del child_role['path_hash_prefixes']
|
||||
self.repository_updater._visit_child_role(child_role, '/forbidden.tgz')
|
||||
|
||||
# Verify that unequal path_hash_prefixes are skipped.
|
||||
child_role['path_hash_prefixes'] = ['bad', 'bad']
|
||||
self.assertEqual(None, self.repository_updater._visit_child_role(child_role,
|
||||
'/unknown.exe', targets_role['delegations']))
|
||||
'/unknown.exe'))
|
||||
|
||||
# Test if both 'path' and 'path_hash_prefixes' are missing.
|
||||
del child_role['paths']
|
||||
del child_role['path_hash_prefixes']
|
||||
self.assertRaises(securesystemslib.exceptions.FormatError, self.repository_updater._visit_child_role,
|
||||
child_role, targets_role['delegations'], child_role['name'])
|
||||
child_role, child_role['name'])
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
|
|
@ -1366,7 +1366,7 @@ def _update_metadata(self, metadata_role, upperbound_filelength, version=None):
|
|||
|
||||
|
||||
def _update_metadata_if_changed(self, metadata_role,
|
||||
referenced_metadata='snapshot'):
|
||||
referenced_metadata='snapshot'):
|
||||
"""
|
||||
<Purpose>
|
||||
Non-public method that updates the metadata for 'metadata_role' if it has
|
||||
|
|
@ -1971,7 +1971,7 @@ def all_targets(self):
|
|||
|
||||
|
||||
def _refresh_targets_metadata(self, rolename='targets',
|
||||
refresh_all_delegated_roles=False):
|
||||
refresh_all_delegated_roles=False):
|
||||
"""
|
||||
<Purpose>
|
||||
Non-public method that refreshes the targets metadata of 'rolename'. If
|
||||
|
|
@ -2301,8 +2301,7 @@ def _preorder_depth_first_walk(self, target_filepath):
|
|||
child_roles_to_visit = []
|
||||
# NOTE: This may be a slow operation if there are many delegated roles.
|
||||
for child_role in child_roles:
|
||||
child_role_name = self._visit_child_role(child_role, target_filepath,
|
||||
delegations)
|
||||
child_role_name = self._visit_child_role(child_role, target_filepath)
|
||||
if child_role['terminating'] and child_role_name is not None:
|
||||
logger.debug('Adding child role ' + repr(child_role_name))
|
||||
logger.debug('Not backtracking to other roles.')
|
||||
|
|
@ -2387,11 +2386,11 @@ def _get_target_from_targets_role(self, role_name, targets, target_filepath):
|
|||
|
||||
|
||||
|
||||
def _visit_child_role(self, child_role, target_filepath, parent_delegations):
|
||||
def _visit_child_role(self, child_role, target_filepath):
|
||||
"""
|
||||
<Purpose>
|
||||
Non-public method that determines whether the given 'child_role' has been
|
||||
delegated the target with the name 'target_filepath'.
|
||||
Non-public method that determines whether the given 'target_filepath'
|
||||
is an allowed path of 'child_role'.
|
||||
|
||||
Ensure that we explore only delegated roles trusted with the target. The
|
||||
metadata for 'child_role' should have been refreshed prior to this point,
|
||||
|
|
@ -2399,7 +2398,7 @@ def _visit_child_role(self, child_role, target_filepath, parent_delegations):
|
|||
verified (as intended). The paths/targets that 'child_role' is allowed
|
||||
to specify in its metadata depends on the delegating role, and thus is
|
||||
left to the caller to verify. We verify here that 'target_filepath'
|
||||
is an allowed path according to its parent role ('parent_delegations').
|
||||
is an allowed path according to the delegated 'child_role'.
|
||||
|
||||
TODO: Should the TUF spec restrict the repository to one particular
|
||||
algorithm? Should we allow the repository to specify in the role
|
||||
|
|
@ -2408,16 +2407,12 @@ def _visit_child_role(self, child_role, target_filepath, parent_delegations):
|
|||
<Arguments>
|
||||
child_role:
|
||||
The delegation targets role object of 'child_role', containing its
|
||||
paths, path_hash_prefixes, keys and so on.
|
||||
paths, path_hash_prefixes, keys, and so on.
|
||||
|
||||
target_filepath:
|
||||
The path to the target file on the repository. This will be relative to
|
||||
the 'targets' (or equivalent) directory on a given mirror.
|
||||
|
||||
parent_delegations:
|
||||
The 'delegations' entry of 'child_role's delegating role. A delegating
|
||||
role specifies the paths/targets that a child role is trusted to sign.
|
||||
|
||||
<Exceptions>
|
||||
None.
|
||||
|
||||
|
|
@ -2435,61 +2430,41 @@ def _visit_child_role(self, child_role, target_filepath, parent_delegations):
|
|||
child_role_paths = child_role.get('paths')
|
||||
child_role_path_hash_prefixes = child_role.get('path_hash_prefixes')
|
||||
|
||||
# A boolean indicator that tells us whether 'child_role' has been delegated
|
||||
# the target with the name 'target_filepath'.
|
||||
child_role_is_relevant = False
|
||||
|
||||
if child_role_path_hash_prefixes is not None:
|
||||
target_filepath_hash = self._get_target_hash(target_filepath)
|
||||
for child_role_path_hash_prefix in child_role_path_hash_prefixes:
|
||||
if target_filepath_hash.startswith(child_role_path_hash_prefix):
|
||||
child_role_is_relevant = True
|
||||
return child_role_name
|
||||
|
||||
else:
|
||||
continue
|
||||
|
||||
elif child_role_paths is not None:
|
||||
# Is 'child_role_name' allowed to sign for 'target_filepath'?
|
||||
for child_role_path in child_role_paths:
|
||||
# A child role path may be an explicit path or pattern (Unix
|
||||
# shell-style wildcards). The child role 'child_role_name' is added if
|
||||
# 'target_filepath' is equal or matches 'child_role_path'. Explicit
|
||||
# filepaths are also added.
|
||||
# shell-style wildcards). The child role 'child_role_name' is returned
|
||||
# if 'target_filepath' is equal to or matches 'child_role_path'.
|
||||
# Explicit filepaths are also considered matches.
|
||||
if fnmatch.fnmatch(target_filepath, child_role_path):
|
||||
child_role_is_relevant = True
|
||||
logger.debug('Child role ' + repr(child_role_name) + ' is allowed to'
|
||||
' sign for ' + repr(target_filepath))
|
||||
|
||||
return child_role_name
|
||||
|
||||
else:
|
||||
logger.debug('Target path' + repr(target_filepath) + ' does not'
|
||||
' match child role path ' + repr(child_role_path))
|
||||
logger.debug('The given target path' + repr(target_filepath) + ' is'
|
||||
' not an allowed trusted path of ' + repr(child_role_path))
|
||||
|
||||
continue
|
||||
|
||||
else:
|
||||
# 'role_name' should have been validated when it was downloaded.
|
||||
# The 'paths' or 'path_hash_prefixes' fields should not be missing,
|
||||
# so we raise a format error here in case they are both missing.
|
||||
raise securesystemslib.exceptions.FormatError(repr(child_role_name) + ' has neither '
|
||||
'"paths" nor "path_hash_prefixes".')
|
||||
|
||||
if child_role_is_relevant:
|
||||
# Is the child role allowed by its parent role to specify this path
|
||||
# in its metadata?
|
||||
try:
|
||||
securesystemslib.util.ensure_all_targets_allowed(child_role_name,
|
||||
[target_filepath], parent_delegations)
|
||||
|
||||
except tuf.exceptions.ForbiddenTargetError:
|
||||
logger.debug('Child role ' + repr(child_role_name) + ' has target ' + \
|
||||
repr(target_filepath) + ', but is not allowed to sign for'
|
||||
' it according to its delegating role.')
|
||||
return None
|
||||
|
||||
else:
|
||||
logger.debug('Child role ' + repr(child_role_name) + ' has target ' + \
|
||||
repr(target_filepath))
|
||||
return child_role_name
|
||||
|
||||
else:
|
||||
logger.debug('Child role ' + repr(child_role_name) + \
|
||||
' does not have target ' + repr(target_filepath))
|
||||
return None
|
||||
raise securesystemslib.exceptions.FormatError(repr(child_role_name) + ' '
|
||||
'has neither a "paths" nor "path_hash_prefixes". At least'
|
||||
' one of these attributes must be present.')
|
||||
|
||||
|
||||
|
||||
|
|
|
|||
Loading…
Reference in a new issue