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

load_repository does not load target file info for delegated targets metadata #1046

Closed
Tracked by #10672
lukpueh opened this issue Jun 8, 2020 · 3 comments
Closed
Tracked by #10672

Comments

@lukpueh
Copy link
Member

lukpueh commented Jun 8, 2020

Related issues: #574, #1045

Description of issue or feature request:
repository_tool.load_repository loads JSON-formatted TUF metadata from disk into memory. When doing this it ignores target file information like hashes and length for delegated targets metadata (it does load it for top-level metadata).

Current behavior:
Using example TUF repo from TUF tutorial after writeall in "Delegations" section

# ... continuing from TUF tutorial (see link above)

import tuf.roledb
import pprint

# Clear and re-load roledb
tuf.roledb.clear_roledb()
repository = load_repository(repository._repository_directory)

# Full target file info is loaded for top-level targets metadata
>>> pprint.pprint(tuf.roledb._roledb_dict["default"]["targets"]["paths"])
{'file1.txt': {'hashes': {'sha256': 'c147efcfc2d7ea666a9e4f5187b115c90903f0fc896a56df9a6ef5d8f3fc9f31',
                          'sha512': '119c19f868a33109852c09d66f6a5c73a7cd52f38325020a461cd94a74edef88709fcbc547d96d0ad9da671260fc42322d177378bad7a285f5df03f8e28f8565'},
               'length': 5},
 'file2.txt': {'hashes': {'sha256': '3377870dfeaaa7adf79a374d2702a3fdb13e5e5ea0dd8aa95a802ad39044a92f',
                          'sha512': 'eb827f1c183373d14958e0253e58496455821fa747996f09d2670cb9f9ff17b5ef3346ffb9d122bf537fcc3bd6480fb916ed3e906763f3bc98b520626ef86329'},
               'length': 5},
 'file3.txt': {'hashes': {'sha256': '6f3fef6dc51c7996a74992b70d0c35f328ed909a5e07646cf0bab3383c95bb02',
                          'sha512': 'b10ff867df18165a0e100d99cd3d27f845f7ef9ad84eeb627a53aabaea04805940c3693154b8a32541a31887dda9fb1e667e93307473b1c581021714768bd032'},
               'length': 5}}

# Only "custom" target file info is loaded for delegated targets metadata
>>> pprint.pprint(tuf.roledb._roledb_dict["default"]["unclaimed"]["paths"])
{'myproject/file4.txt': {}}

Expected behavior:

Load full target file info from delegated targets metadata that was written to disk in writeall when calling load_repository. E.g.:

#  ... (see snippet above for setup)
>>> pprint.pprint(tuf.roledb._roledb_dict["default"]["unclaimed"]["paths"])
{'myproject/file4.txt': {'custom': {},
                         'hashes': {'sha256': '600456c60420b0c6ddfe3b8d50cb6e63af544fb26c5715ae58a601bcca9a055d',
                                    'sha512': '3eb3222d4b5629a27dede47a90c7665fcaff1305c2c6c645465edc8278768032ca223f9193f3e9d6bb7a8b950ed284030d256ce1ee33e1de7e9f3e2cb1a834a0'},
                         'length': 5}}
@lukpueh
Copy link
Member Author

lukpueh commented Jun 8, 2020

This needs double checking that it does not interfere with existing expectations for roledb but a fix might be as easy as:

diff --git a/tuf/repository_tool.py b/tuf/repository_tool.py
index 8d58d2d2..46100769 100755
--- a/tuf/repository_tool.py
+++ b/tuf/repository_tool.py
@@ -3170,7 +3170,7 @@ def load_repository(repository_directory, repository_name='default',
     roleinfo['expires'] = metadata_object['expires']
 
     for filepath, fileinfo in six.iteritems(metadata_object['targets']):
-      roleinfo['paths'].update({filepath: fileinfo.get('custom', {})})
+      roleinfo['paths'].update({filepath: fileinfo})
     roleinfo['delegations'] = metadata_object['delegations']
 
     tuf.roledb.add_role(metadata_name, roleinfo, repository_name)

@joshuagl
Copy link
Member

joshuagl commented Jun 8, 2020

The proposed 1-line change here matches what I came up with when digging into this issue today.

We should be sure to include a test to catch the issue in any PR including that fix.

@joshuagl
Copy link
Member

Thanks for the fix @sechkova !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants