-
Notifications
You must be signed in to change notification settings - Fork 284
PoC(tap13): User Selection of the Top-Level Target Files Through Mapping Metadata #1103
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
Changes from all commits
1b49f08
b2fe6e3
b47e474
4ceda62
9b6f0bc
4238496
dce7e4a
8eb0c5d
a6d6cdc
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| { | ||
| "targets_rolename": "role1", | ||
| "keys":{ | ||
| "c8022fa1e9b9cb239a6b362bbdffa9649e61ad2cb699d2e4bc4fdf7930a0e64a": { | ||
| "keyid_hash_algorithms": [ | ||
| "sha256", | ||
| "sha512" | ||
| ], | ||
| "keytype": "ed25519", | ||
| "keyval": { | ||
| "public": "fcf224e55fa226056adf113ef1eb3d55e308b75b321c8c8316999d8c4fd9e0d9" | ||
| }, | ||
| "scheme": "ed25519" | ||
| } | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| { | ||
| "targets_rolename": "role1", | ||
| "keys":{ | ||
| "c8022fa1e9b9cb239a6b362bbdffa9649e61ad2cb699d2e4bc4fdf7930a0e64a": { | ||
| "keyid_hash_algorithms": [ | ||
| "sha256", | ||
| "sha512" | ||
| ], | ||
| "keytype": "ed25519", | ||
| "keyval": { | ||
| "public": "fcf224e55fa226056adf113ef1eb3d55e308b75b321c8c8316999d8c4fd9e0d9" | ||
| }, | ||
| "scheme": "ed25519" | ||
| } | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -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. | ||
|
|
@@ -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. | ||
|
|
@@ -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): | ||
|
|
@@ -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 | ||
|
|
@@ -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 | ||
|
|
@@ -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 | ||
|
|
@@ -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: | ||
|
|
@@ -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 | ||
|
|
@@ -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)) | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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).
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 = {} | ||
|
|
@@ -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_rolename'] + '.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. | ||
|
|
@@ -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. | ||
|
|
@@ -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) | ||
|
|
||
|
|
||
|
|
||
|
|
@@ -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_rolename'] + '.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, | ||
|
|
@@ -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_rolename'] + '.json' | ||
| else: | ||
| metadata_filename = metadata_role + '.json' | ||
| expected_versioninfo = None | ||
|
|
||
| # Ensure the referenced metadata has been loaded. The 'root' role may be | ||
|
|
||
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
cc @joshuagl, @trishankatdatadog, @JustinCappos
There was a problem hiding this comment.
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_filenameshould probably do that tooThere was a problem hiding this comment.
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.