Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PoC(tap13): User Selection of the Top-Level Target Files Through Mapping Metadata #1103

Closed
wants to merge 9 commits into from
16 changes: 16 additions & 0 deletions tests/repository_data/client/targets_map.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"targets_filename": "role1",
mnm678 marked this conversation as resolved.
Show resolved Hide resolved
"keys":{
"c8022fa1e9b9cb239a6b362bbdffa9649e61ad2cb699d2e4bc4fdf7930a0e64a": {
"keyid_hash_algorithms": [
"sha256",
"sha512"
],
"keytype": "ed25519",
"keyval": {
"public": "fcf224e55fa226056adf113ef1eb3d55e308b75b321c8c8316999d8c4fd9e0d9"
},
"scheme": "ed25519"
}
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there an agreement about the targets map metadata format? AFAIR TAP 12 still misses the definition.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There isn't. I based this format on a suggestion from the comments of the TAP 13 pr. It would probably be good to get some more consensus about this or a different format.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some of the other metadata files refer to other metadata files as well: they use the full name "targets.json" so targets_filename should probably do that too

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the intervening 1 year we've discovered multiple reasons not to use the full filename.

16 changes: 16 additions & 0 deletions tests/repository_data/targets_map.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
{
"targets_filename": "role1",
"keys":{
"c8022fa1e9b9cb239a6b362bbdffa9649e61ad2cb699d2e4bc4fdf7930a0e64a": {
"keyid_hash_algorithms": [
"sha256",
"sha512"
],
"keytype": "ed25519",
"keyval": {
"public": "fcf224e55fa226056adf113ef1eb3d55e308b75b321c8c8316999d8c4fd9e0d9"
},
"scheme": "ed25519"
}
}
}
46 changes: 46 additions & 0 deletions tests/test_updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -1801,6 +1801,36 @@ def test_13__targets_of_role(self):
targets=targets, skip_refresh=False)


def test_15__targets_map_file(self):
# test badly formed targets map file
self.assertRaises(tuf.exceptions.Error, updater.Updater, self.repository_name,
self.repository_mirrors, 'no_file')

targets_map_file = os.path.join(self.repository_directory, 'metadata', 'root.json')
self.assertRaises(securesystemslib.exceptions.FormatError, updater.Updater,
self.repository_name, self.repository_mirrors, targets_map_file)

# set the correct map file
targets_map_file = os.path.join(self.client_directory, 'targets_map.json')

# Creating a repository instance using the targets map file.
self.repository_updater = updater.Updater(self.repository_name,
self.repository_mirrors, targets_map_file)

#ensure the map file was set
self.assertEqual('role1', self.repository_updater.targets_map_file['targets_filename'])

# ensure that only targets in the targets map file are loaded
all_targets = self.repository_updater.all_targets()
self.assertEqual(len(all_targets), 1)

# ensure the targets map file replaces the targets role
self.repository_updater._load_metadata_from_file('current', 'targets')
self.assertRaises(tuf.exceptions.UnknownRoleError,
self.repository_updater._targets_of_role, 'role1')





class TestMultiRepoUpdater(unittest_toolbox.Modified_TestCase):
Expand Down Expand Up @@ -1830,6 +1860,7 @@ def setUp(self):
original_client = os.path.join(original_repository_files, 'client', 'test_repository1')
original_keystore = os.path.join(original_repository_files, 'keystore')
original_map_file = os.path.join(original_repository_files, 'map.json')
original_targets_map_file = os.path.join(original_repository_files, 'targets_map.json')

# Save references to the often-needed client repository directories.
# Test cases need these references to access metadata and target files.
Expand All @@ -1854,6 +1885,7 @@ def setUp(self):
'keystore')
self.map_file = os.path.join(self.client_directory, 'map.json')
self.map_file2 = os.path.join(self.client_directory2, 'map.json')
self.targets_map_file = os.path.join(self.temporary_repository_root, 'targets_map_file')

# Copy the original 'repository', 'client', and 'keystore' directories
# to the temporary repository the test cases can use.
Expand All @@ -1864,6 +1896,7 @@ def setUp(self):
shutil.copyfile(original_map_file, self.map_file)
shutil.copyfile(original_map_file, self.map_file2)
shutil.copytree(original_keystore, self.keystore_directory)
shutil.copyfile(original_targets_map_file, self.targets_map_file)

# Launch a SimpleHTTPServer (serves files in the current directory).
# Test cases will request metadata and target files that have been
Expand Down Expand Up @@ -2122,6 +2155,19 @@ def test_get_updater(self):





def test_targets_map_file(self):
map_file = os.path.join(self.client_directory, 'map.json')
multi_repo_updater = updater.MultiRepoUpdater(map_file, self.targets_map_file)

# Does the repository use the targets map file?
repository_updater = multi_repo_updater.get_updater('test_repository2')

self.assertEqual(repository_updater.targets_map_file['targets_filename'], 'role1')



def _load_role_keys(keystore_directory):

# Populating 'self.role_keys' by importing the required public and private
Expand Down
73 changes: 64 additions & 9 deletions tuf/client/updater.py
Original file line number Diff line number Diff line change
Expand Up @@ -186,6 +186,10 @@ class MultiRepoUpdater(object):
The path of the map file. The map file is needed to determine which
repositories to query given a target file.

targets_map_filename:
The path of the targets map file. This targets map file
will be used by all repositories.

<Exceptions>
securesystemslib.exceptions.FormatError, if the map file is improperly
formatted.
Expand All @@ -199,7 +203,7 @@ class MultiRepoUpdater(object):
None.
"""

def __init__(self, map_file):
def __init__(self, map_file, targets_map_filename=None):
# Is 'map_file' a path? If not, raise
# 'securesystemslib.exceptions.FormatError'. The actual content of the map
# file is validated later on in this method.
Expand Down Expand Up @@ -228,6 +232,13 @@ def __init__(self, map_file):
# }
self.repository_names_to_mirrors = self.map_file['repositories']

# If there is a targets_map_file, set self.targets_map_filename.
# This map file will be used with all repositories in place of the
# top level targets file.
if targets_map_filename is not None:
securesystemslib.formats.PATH_SCHEMA.check_match(targets_map_filename)

self.targets_map_filename = targets_map_filename


def get_valid_targetinfo(self, target_filename, match_custom_field=True):
Expand Down Expand Up @@ -517,7 +528,7 @@ def get_updater(self, repository_name):
# NOTE: State (e.g., keys) should NOT be shared across different
# updater instances.
logger.debug('Adding updater for ' + repr(repository_name))
updater = tuf.client.updater.Updater(repository_name, mirrors)
updater = tuf.client.updater.Updater(repository_name, mirrors, self.targets_map_filename)

except Exception:
return None
Expand Down Expand Up @@ -587,6 +598,9 @@ class Updater(object):
self.repository_name:
The name of the updater instance.

self.targets_map_file:
The contents of the targets map file.

<Updater Methods>
refresh():
This method downloads, verifies, and loads metadata for the top-level
Expand Down Expand Up @@ -626,7 +640,8 @@ class Updater(object):
http://www.python.org/dev/peps/pep-0008/#method-names-and-instance-variables
"""

def __init__(self, repository_name, repository_mirrors):
def __init__(self, repository_name, repository_mirrors,
targets_map_filename=None):
"""
<Purpose>
Constructor. Instantiating an updater object causes all the metadata
Expand Down Expand Up @@ -665,6 +680,10 @@ def __init__(self, repository_name, repository_mirrors):
'metadata_path': 'metadata',
'targets_path': 'targets',
'confined_target_dirs': ['']}}
targets_map_filename:
The name of the targets map file. If one is not provided,
self.targets_map_file will be set to None and the top-level
targets metadata from the repository will be used.
mnm678 marked this conversation as resolved.
Show resolved Hide resolved

<Exceptions>
securesystemslib.exceptions.FormatError:
Expand All @@ -674,6 +693,11 @@ def __init__(self, repository_name, repository_mirrors):
If there is an error with the updater's repository files, such
as a missing 'root.json' file.

tuf.exceptions.Error:
If the targets map file cannot be loaded. This may be due to a
securesystemslib.exceptions.Error if the targets map filename
cannot be parsed, or and IOError in the case of runtime exceptions.

<Side Effects>
Th metadata files (e.g., 'root.json', 'targets.json') for the top- level
roles are read from disk and stored in dictionaries. In addition, the
Expand All @@ -694,6 +718,22 @@ def __init__(self, repository_name, repository_mirrors):
# Save the validated arguments.
self.repository_name = repository_name
self.mirrors = repository_mirrors
self.targets_map_file = None

# If targets_map_filename is provided, load the targets_map_file.
if targets_map_filename is not None:
securesystemslib.formats.PATH_SCHEMA.check_match(targets_map_filename)

try:
self.targets_map_file = securesystemslib.util.load_json_file(
targets_map_filename)

except (securesystemslib.exceptions.Error) as e:
raise tuf.exceptions.Error('Cannot load the targets map file: ' + str(e))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please avoid generic exceptions in public API if possible.

Also document the exception (alternatively remove the 'Exceptions' section from the docs if it's known to be outdated).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think a FormatError would make more sense here (to indicate that the targets map file specified an invalid filename)? Or something else?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good question: securesystemslib seems to have a wide variety of errors here (some of which clearly aren't formaterrors) so maybe you can't avoid a generic one. In that case at least document the exceptions that may be raised.

By the way, the check_match() call just before this seems to not be needed as securesystemslib does that (unless the point was to trigger the FormatError specifically before the try-except)


# Raise securesystemslib.exceptions.FormatError if the targets map file is
# improperly formatted.
tuf.formats.TARGETS_MAPFILE_SCHEMA.check_match(self.targets_map_file)

# Store the trusted metadata read from disk.
self.metadata = {}
Expand Down Expand Up @@ -822,7 +862,14 @@ def _load_metadata_from_file(self, metadata_set, metadata_role):

# Save and construct the full metadata path.
metadata_directory = self.metadata_directory[metadata_set]
metadata_filename = metadata_role + '.json'
# For top-level targets, the targets map file may overwrite the
# targets metadata on the repository. If this is the case, ignore
# the 'targets.json' file on the repository and instead load
# the targets metadata indicated by the targets map file.
if metadata_role == 'targets' and self.targets_map_file is not None:
metadata_filename = self.targets_map_file['targets_filename'] + '.json'
else:
metadata_filename = metadata_role + '.json'
metadata_filepath = os.path.join(metadata_directory, metadata_filename)

# Ensure the metadata path is valid/exists, else ignore the call.
Expand Down Expand Up @@ -872,7 +919,9 @@ def _rebuild_key_and_role_db(self):
This private method is called when a new/updated 'root' metadata file is
loaded or when updater.refresh() is called. This method will only store
the role information of the top-level roles (i.e., 'root', 'targets',
'snapshot', 'timestamp').
'snapshot', 'timestamp'). If a targets map file is used, this will
additionally load the key and role information from the targets map
file.

<Arguments>
None.
Expand Down Expand Up @@ -900,10 +949,10 @@ def _rebuild_key_and_role_db(self):
# repository is first instantiated. Due to this setup, reloading delegated
# roles is not required here.
tuf.keydb.create_keydb_from_root_metadata(self.metadata['current']['root'],
self.repository_name)
self.repository_name, self.targets_map_file)

tuf.roledb.create_roledb_from_root_metadata(self.metadata['current']['root'],
self.repository_name)
self.repository_name, self.targets_map_file)



Expand Down Expand Up @@ -1785,7 +1834,10 @@ def _update_metadata(self, metadata_role, upperbound_filelength, version=None):

# Construct the metadata filename as expected by the download/mirror
# modules.
metadata_filename = metadata_role + '.json'
if self.targets_map_file is not None and metadata_role == 'targets':
metadata_filename = self.targets_map_file['targets_filename'] + '.json'
else:
metadata_filename = metadata_role + '.json'

# Attempt a file download from each mirror until the file is downloaded and
# verified. If the signature of the downloaded file is valid, proceed,
Expand Down Expand Up @@ -1920,7 +1972,10 @@ def _update_metadata_if_changed(self, metadata_role,
None.
"""

metadata_filename = metadata_role + '.json'
if self.targets_map_file is not None and metadata_role == 'targets':
metadata_filename = self.targets_map_file['targets_filename'] + '.json'
else:
metadata_filename = metadata_role + '.json'
expected_versioninfo = None

# Ensure the referenced metadata has been loaded. The 'root' role may be
Expand Down
6 changes: 6 additions & 0 deletions tuf/formats.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,12 @@
key_schema = NAME_SCHEMA,
value_schema = SCHEMA.ListOf(securesystemslib.formats.URL_SCHEMA))

# An object containing the targets map file. The format of the targets
# map file is covered in TAP 13
TARGETS_MAPFILE_SCHEMA = SCHEMA.Object(
targets_filename = NAME_SCHEMA,
keys = KEYDICT_SCHEMA)

# An object containing the map file's "mapping" attribute.
MAPPING_SCHEMA = SCHEMA.ListOf(SCHEMA.Object(
paths = RELPATHS_SCHEMA,
Expand Down
26 changes: 21 additions & 5 deletions tuf/keydb.py
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,8 @@
_keydb_dict['default'] = {}


def create_keydb_from_root_metadata(root_metadata, repository_name='default'):
def create_keydb_from_root_metadata(root_metadata, repository_name='default',
targets_map_file=None):
"""
<Purpose>
Populate the key database with the unique keys found in 'root_metadata'.
Expand All @@ -79,6 +80,11 @@ def create_keydb_from_root_metadata(root_metadata, repository_name='default'):
The name of the repository to store the key information. If not supplied,
the key database is populated for the 'default' repository.

targets_map_file:
The contents of the targets map file. The keys from the targets map file
will be added to the keydb. If not provided the keys from the default
targets files will be used.

<Exceptions>
securesystemslib.exceptions.FormatError, if 'root_metadata' does not have the correct format.

Expand Down Expand Up @@ -111,10 +117,20 @@ def create_keydb_from_root_metadata(root_metadata, repository_name='default'):
else:
create_keydb(repository_name)

# Iterate the keys found in 'root_metadata' by converting them to
# 'RSAKEY_SCHEMA' if their type is 'rsa', and then adding them to the
# key database using the provided keyid.
for keyid, key_metadata in six.iteritems(root_metadata['keys']):
keys_to_add = root_metadata['keys']

if targets_map_file is not None:
tuf.formats.TARGETS_MAPFILE_SCHEMA.check_match(targets_map_file)

# If any keyids from root metadata match those from targets_map_metadata,
# only the root metadata keys will be added. Add all keys to keys_to_add
keys_to_add = targets_map_file['keys'].copy()
keys_to_add.update(root_metadata['keys'])

# Iterate the keys found in 'root_metadata' and 'targets_map_file' by
# converting them to 'RSAKEY_SCHEMA' if their type is 'rsa', and then adding
# them to the key database using the provided keyid.
for keyid, key_metadata in six.iteritems(keys_to_add):
if key_metadata['keytype'] in _SUPPORTED_KEY_TYPES:
# 'key_metadata' is stored in 'KEY_SCHEMA' format. Call
# create_from_metadata_format() to get the key in 'RSAKEY_SCHEMA' format,
Expand Down
12 changes: 11 additions & 1 deletion tuf/roledb.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,8 @@
TOP_LEVEL_ROLES = ['root', 'targets', 'snapshot', 'timestamp']


def create_roledb_from_root_metadata(root_metadata, repository_name='default'):
def create_roledb_from_root_metadata(root_metadata, repository_name='default',
targets_map_file=None):
"""
<Purpose>
Create a role database containing all of the unique roles found in
Expand All @@ -92,6 +93,10 @@ def create_roledb_from_root_metadata(root_metadata, repository_name='default'):
The name of the repository to store 'root_metadata'. If not supplied,
'rolename' is added to the 'default' repository.

targets_map_file:
The contents of the targets map file. This will be used to add the keys
to the targets role. If not provided, the default targets keys will be used.

<Exceptions>
securesystemslib.exceptions.FormatError, if 'root_metadata' does not have
the correct object format.
Expand Down Expand Up @@ -140,6 +145,11 @@ def create_roledb_from_root_metadata(root_metadata, repository_name='default'):
roleinfo['previous_keyids'] = roleinfo['keyids']
roleinfo['previous_threshold'] = roleinfo['threshold']

elif rolename == 'targets' and targets_map_file is not None:
roleinfo['keyids'] = []
for keyid in targets_map_file['keys']:
roleinfo['keyids'].append(keyid)

roleinfo['signatures'] = []
roleinfo['signing_keyids'] = []
roleinfo['partial_loaded'] = False
Expand Down