theupdateframework / specification

The Update Framework specification
https://theupdateframework.github.io/specification/
Other
365 stars 54 forks source link

How do TARGETS interact with delegations? #268

Open Eh2406 opened 1 year ago

Eh2406 commented 1 year ago

This issue is asking for the standard to be clearer about how delegations and explicitly listed targets interact. It is possible that with a full understanding of how TUF is intended to work, all of the details are in fact clearly specified in the specification. However as someone without the full picture I have gotten myself thoroughly confused. I will try and document how I have (mis)read things, so that small changes in wording can be merged to make it easier to understand.

As I am profoundly confused, I may be wrong about what I have misunderstood. When targets.json delegates to role A what is supposed to go in targets.json's "targets" and what is supposed to go in the A.json's "targets"? What happens if targets.json has a particular file in its "targets" and delegates for A to be responsible for that path, but A.json does not have the file listed?

https://theupdateframework.github.io/specification/latest/index.html#targets-obj-targets

Each key of the TARGETS object is a TARGETPATH. ... It is allowed to have a TARGETS object with no TARGETPATH elements. This can be used to indicate that no target files are available.

I originally misread the statement to suggest that the root targets.json must list all files available from the registry. Obviously this is not scalable. So I looked at PEP 458 which claims to have solved the scalability problem using Succinct hashed bin delegations (TAP 15). But when reading the specification path_hash_prefixes has to do with delegations not the targets list.

So, apparently, delegations have their own target like files. This is actually spelled out further down as:

The metadata files for delegated target roles has the same format as the top-level targets.json metadata file.

Once this was pointed out to me I reread specification from the top to try and figure out why I hadn't noticed it the first two times I read the specification. These files were in fact previously mentioned in "3.1.2.1. Metadata files for targets delegation". In previous readings, my brain had gotten focused on "role" in the name of the file and so had assumed these files described the keys and protocols for that role. I now see the error in this. The descriptions of what keys to trust for each role live exclusively in root.json.

Okay one more effort to figure this out on my own. Section 5 must have something about checking the "targets" list. ... no. The closest is the quote "Verify the desired target against its targets metadata." which leaves a lot of detail out.

(cc https://github.com/rust-lang/cargo/issues/10928#issuecomment-1383058704 , where I first attempted to describe my confusion.)

Proposed improvements

In "3.1.2.1. Metadata files for targets delegation", add the descriptive sentence from /targets.EXT. Ending up with a paragraph like:

When the targets role delegates trust to other roles, each delegated role provides one signed metadata file. This file lists hashes and sizes of target files, and specifies any sub-delegation information. As is the case with the directory structure of top-level metadata, the delegated files are relative to the base URL of metadata available from a given repository mirror.

In https://theupdateframework.github.io/specification/latest/index.html#targets-obj-targets and a sentence making it clear that these are files attested to by this role, but other files may be attested to by delegations. Ending up with a paragraph like:

Each key of the TARGETS object is a TARGETPATH. Each row entry represents a target file this role is attesting is part of the registry. There may be many other files that are part of the registry that are not in this list but only if they are attested to by a delegation role in the DELEGATIONS field.

Add precise steps to sections 5.6.7 and 5.7. For example in 5.6.7 in between 5.6.7.1 and 5.6.7.2 "if the file being searched for is in the "targets" then jump to step § 5.7 Fetch target.". And for 5.7.1 ... I don't know how to say this more clearly.

joshuagl commented 1 year ago

Thanks so much for filing this detailed issue! Apologies for the delayed reply.

I really appreciate your suggested improvements, too. I started with your suggestions and am trying to make the language fit the rest of the spec (though` ran into my old frustrations with object terminology, see #175).

I'll keep iterating on this in the coming days and open a PR when it's ready for feedback, you can see (but don't feel obliged to look at) my WIP changes in https://github.com/theupdateframework/specification/compare/master...joshuagl:specification:joshuagl/targets-delegations

JustinCappos commented 1 year ago

And I'd like to further commend you on the issue. Specification clarifications are definitely something we care about. We will be working to address your concerns.

trishankatdatadog commented 1 year ago

I have always wanted us to specify the delegations preorder DFS in pseudocode rather than English, which would go a long way to solving this problem

joshuagl commented 1 year ago

This is aligned with my goals too. While converting to bikeshed was an improvement, the spec is still unwieldy to maintain and difficult to understand.

x-ref #121 and #271.