Remove root, add delegation hashes to the snapshot metadata #40
Conversation
|
Good afternoon, has anyone had a chance to review this patch? Is this the right way to implement section 5.6 of the Mercury paper? |
|
Thanks for the review @trishankatdatadog! I incorporated your feedback. |
tuf-spec.md
Outdated
| numbers of all metadata on the repository, excluding timestamp.json and | ||
| mirrors.json. For the root role, the hash(es), size, and version number | ||
| are listed. | ||
| numbers of all metadata on the repository, excluding root.json, timestamp.json and |
There was a problem hiding this comment.
| numbers of all metadata on the repository, excluding root.json, timestamp.json and | |
| numbers of only the top-level and all delegated targets role metadata. |
There was a problem hiding this comment.
Can we make this even clearer? I read your change as "top-level roles and delegated targets roles", which would mean all roles, which is not what we want. What about...
| numbers of all metadata on the repository, excluding root.json, timestamp.json and | |
| numbers of only the top-level targets and all delegated targets role metadata. |
tuf-spec.md
Outdated
| mirrors.json. For the root role, the hash(es), size, and version number | ||
| are listed. | ||
| numbers of all metadata on the repository, excluding root.json, timestamp.json and | ||
| mirrors.json. The metadata length and hashes are OPTIONAL for the top-level and |
There was a problem hiding this comment.
| mirrors.json. The metadata length and hashes are OPTIONAL for the top-level and | |
| The metadata length and hashes are OPTIONAL for the top-level and |
There was a problem hiding this comment.
Same as above.
| mirrors.json. The metadata length and hashes are OPTIONAL for the top-level and | |
| The metadata length and hashes are OPTIONAL for the top-level targets and |
tuf-spec.md
Outdated
| , ... | ||
| } | ||
|
|
||
| METAPATH is the metadata file's path on the repository relative to the | ||
| metadata base URL. | ||
|
|
||
| VERSION is listed for the root file | ||
| and all other roles available on the repository. | ||
| VERSION is listed for all roles available on the repository. |
There was a problem hiding this comment.
| VERSION is listed for all roles available on the repository. | |
| VERSION is listed for the top-level and all delegated targets roles available on the repository. |
There was a problem hiding this comment.
Same as above.
| VERSION is listed for all roles available on the repository. | |
| VERSION is listed for the top-level targets and all delegated targets roles available on the repository. |
|
Can someone help to apply, commit, and merge the above 3 suggestions? Thanks! @lukpueh @JustinCappos |
|
@trishankatdatadog, I'm happy to apply suggestions and merge. Let me know if my updates make sense to you. |
|
@lukpueh Yes, please do apply, some GitHub bug prevents me from applying them, thanks |
|
@trishankatdatadog, I don't think it's a bug. We just don't have the permissions to commit to a branch on @erickt's fork. @erickt, would you mind applying the patches, so that we can merge? |
|
@lukpueh Isn't there an on-by-default option that lets upstream edit PR? |
|
@trishankatdatadog Interesting, didn't know that. And an API query to this PR page indeed returns |
|
Interesting (II): I cannot apply commits but I can force push to @erickt's branch, which is what I just did in order to fix a conflict with #43, re-applying a there made keyid update, i.e. ( I also added a commit to integrate @trishankatdatadog's and my suggestions (made via GitHub UI). @trishankatdatadog, would you mind taking another look? |
According to @JustinCappos in theupdateframework#31, the root.json is no longer needed in the snapshot metadata, because the workflow will already have updated the root metadata before the snapshot metadata is fetched. In addition, section 5.6 of the Mercury paper describes a need for delegations to contain hashes in order to protect against malicous mirrors that may substitute one version of the delegated metadata with another. This cannot be detected without hashing of these files. Closes theupdateframework#31
|
Thanks all! |
The workflow for downloading metadata for top-level roles has changed. Root is now updated and verified by stepping through a chain of trust based on the currently available root metadata. For that reason root.json is no longer needed in snapshot and has been dropped from there per theupdateframework/specification#40 Update docstrings and comments in the Updater object to reflect the correct flow of metadata updates: root (if necessary) -> timestamp -> snapshot -> targets Signed-off-by: Joshua Lock <jlock@vmware.com>
The workflow for downloading metadata for top-level roles has changed. Root is now updated and verified by stepping through a chain of trust based on the currently available root metadata. For that reason root.json is no longer needed in snapshot and has been dropped from there per theupdateframework/specification#40 Update docstrings and comments in the Updater object to reflect the correct flow of metadata updates: root (if necessary) -> timestamp -> snapshot -> targets Signed-off-by: Joshua Lock <jlock@vmware.com>
The workflow for downloading metadata for top-level roles has changed. Root is now updated and verified by stepping through a chain of trust based on the currently available root metadata. For that reason root.json is no longer needed in snapshot and has been dropped from there per theupdateframework/specification#40 Update docstrings and comments in the Updater object to reflect the correct flow of metadata updates: root (if necessary) -> timestamp -> snapshot -> targets Signed-off-by: Joshua Lock <jlock@vmware.com>
The workflow for downloading metadata for top-level roles has changed. Root is now updated and verified by stepping through a chain of trust based on the currently available root metadata. For that reason root.json is no longer needed in snapshot and has been dropped from there per theupdateframework/specification#40 Update docstrings and comments in the Updater object to reflect the correct flow of metadata updates: root (if necessary) -> timestamp -> snapshot -> targets Signed-off-by: Joshua Lock <jlock@vmware.com>
According to @JustinCappos in #31, the root.json is no longer needed in the snapshot metadata, because the workflow will already have updated the root metadata before the snapshot metadata is fetched.
In addition, section 5.6 of the Mercury paper describes a need for delegations to contain hashes in order to protect against malicous mirrors that may substitute one version of the delegated metadata with another. This cannot be detected without hashing of these files.
Closes #31