Clarify how delegated roles are downloaded#72
Clarify how delegated roles are downloaded#72erickt wants to merge 1 commit intotheupdateframework:masterfrom
Conversation
Section 5.4.5 is a little vague how on delegated targets are fetched and validated. This updates that section to use the same logic and verification process as downloading the top-level targets role to be explicit. One thing to point out though is that the old section 4.5 suggests that we don't report verification errors to the user. I've preserved this in my explicit version. I imagine users would still be notified if their delegated roles may be undergoing an attack. Is this intentional, or should I switch to the "abort the update cycle, and report the potential rollback attack"-style phrasing used elsewhere in the spec?
lukpueh
left a comment
lukpueh
left a comment
There was a problem hiding this comment.
Thank you very much for the patch, @erickt! Your update should make things clearer.
Regarding your question about reporting failures to the user, I personally think that the specification does not have to go into details there. But since we do in other places we should do here too. And reporting the failure reason seems more helpful than just saying the target could not be found. Maybe other authors want to weigh in? cc @JustinCappos, @mnm678.
One other thing, since we now list all the checks, we should probably also mention that delegated targets metadata should be checked for expiration, just like the top-level targets metadata (see 4.4). What do you think?
| * **4.5.2.1**. Let DELEGATE denote the current target role TARGETS is | ||
| delegating to. | ||
|
|
||
| * **4.5.2.2**. **Download the DELEGATE tarets metadata file**, up to either |
There was a problem hiding this comment.
| * **4.5.2.2**. **Download the DELEGATE tarets metadata file**, up to either | |
| * **4.5.2.2**. **Download the DELEGATE targets metadata file**, up to either |
| * **4.5.2.5**. **Check for a rollback attack.** The version number of the | ||
| trusted DELEGATE metadata file, if any, MUST be less than or equal to the | ||
| version number of the new DELEGATE metadata file. If the new DELEGATE | ||
| `metadata file is older than the trusted DELEGATE metadata file, discard |
There was a problem hiding this comment.
| `metadata file is older than the trusted DELEGATE metadata file, discard | |
| metadata file is older than the trusted DELEGATE metadata file, discard |
| found, end the search and report the target cannot be found. If | ||
| consistent snapshots are not used (see Section 7), then the filename used | ||
| to download the targets metadata file is of the fixed form FILENAME.EXT | ||
| (e.g., delegated_rol.json). Otherwise, the filename is of the form |
There was a problem hiding this comment.
| (e.g., delegated_rol.json). Otherwise, the filename is of the form | |
| (e.g., delegated_role.json). Otherwise, the filename is of the form |
|
I agree that we should report this error like other places in the specification. |
|
@erickt, I took the liberty to take over this PR, addressing my own review comments (typos, missing freeze attack check) and your question regarding error reporting. I submitted a new PR under #86. It also adds work from me and @mnm678 to address the concerns you raised in #71. It would be great if you could take a look. Regardless, I'd like to thank you for your great contributions. |
3 participants
Section 5.4.5 is a little vague how on delegated targets are fetched and validated. This updates that section to use the same logic and verification process as downloading the top-level targets role to be explicit.
One thing to point out though is that the old section 4.5 suggests that we don't report verification errors to the user. I've preserved this in my explicit version. I imagine users would still be notified if their delegated roles may be undergoing an attack. Is this intentional, or should I switch to the "abort the update cycle, and report the potential rollback attack"-style phrasing used elsewhere in the spec?