-
Notifications
You must be signed in to change notification settings - Fork 20
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
Add TAP supporting content addressed targets #156
Conversation
39bbc23
to
fee9704
Compare
Very cool, I can see many ways in which this would be useful :) However, I fail to see what about this proposal is specific to Merkle DAGs. For instance, it seems like Docker images could also benefit (longer digression below). But it really just seems to be describing a pluggable way of adding targets that aren't simple files to TUF. I agree that Merkle DAGs are a good candidate for non-file types of things we want to hash, and that we need to be really careful about the characteristics of such hashing techniques when we add. But they seem to be inessential to the proposal. Docker's not the best example here because there is a single file that you can sha256 hash to get the digest---the "manifest", which is a JSON file. I suppose in some sense the manifest represents a Merkle DAG, because it contains hashes of the layers of the image. But checking that a full Docker image has the appropriate digest involves hashing the layers and comparing them to the manifest. So I think it fits. |
Cc @erickt |
Hi Zack, thanks for your comments! I want to note some thoughts:
There was interest in the last community meeting for something like ITE-4, i.e., more open to non-file entities than what is here, so you're not alone in that regard. |
fee9704
to
a1a3e77
Compare
a1a3e77
to
027da94
Compare
6d83f29
to
50b65db
Compare
This is interesting. Which of TUFs security properties do we actually care for here? Is TUFs integrity protection even relevant, when targets are content addressable? In other words, does the client need to verify the target hash at all, or is it enough that the target path is in targets metadata? |
That's a great point and I think emphasizing that in the text will greatly help. With content addressed systems, we care about all of TUF's properties minus artifact integrity. Let me take a pass on that. Also note that we've proposed a prototype of this TAP as a GSoC 2023 task. That should help us clarify some of these ideas and better evaluate how this TAP would work in practice. |
941d7c5
to
91d5caf
Compare
@lukpueh I reworked the TAP to focus on TUF properties that matter outside of artifact integrity validation. LMK what you think! |
111ba4b
to
98f8556
Compare
(Sorry, just wanted to say pls count me out of reviewing this TAP now as I will be on a few weeks of PTO. Thanks!) |
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.
This looks great and sounds really interesting! I dropped a bunch of noob questions in my review. One of the things I was quite sure where to put was around the common and implicit nature of many of these content addressable systems to be used in a distributed environment. This often leads to separate integrity checks at the "server" and the "client" of the application. I don't know if that needs to be more explicitly addressed or would just be simply covered in the "Security Assessment" of the ecosystem.
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.
Thanks for the updates, @adityasaky, this looks a lot better!
ebaace5
to
5112977
Compare
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.
I think that there are a couple of things that should be covered by the POUF according to the TAP that we haven't addressed in the TAF POUF, like backwards compatibility. Is that a blocker?
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.
I have a few minor concerns with this as stated in my comments. I am supportive in general, but would like to hear more from other community members (especially Lukas, Jussi, and Marina).
If we had a more definitive policy about having "core TUF" and "TUF extensions" this would be an easy, immediate approve from me as a TUF extension.
a97fb67
to
442951d
Compare
CC @sudo-bmitch who mentioned that OCI is a little weird—they wrap the content-addressed blobs in some metadata, then use that as the hash (I know OCI isn't a primary use case here, but it could be an interesting one.) |
The TL;DR on OCI is you have the following:
If a blob isn't referenced by a manifest, a registry will usually garbage collect it after some time. So to structure things in OCI you'd want to identify what the individual blobs need to be, manifests to reference those blobs, and what tags to use to locate the manifests. The sha256 hash of the blobs will be the same across different CAS implementations, but the hash of an OCI manifest will probably only exist in the OCI implementation (but it may be similar in concept to the Git directory listing and hash). |
442951d
to
c777393
Compare
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.
One nit, but this looks ready to merge as a draft
Signed-off-by: Aditya Sirish <[email protected]> Co-authored-by: Renata Vaderna <[email protected]> Co-authored-by: John Ericson <[email protected]>
c777393
to
a3eedca
Compare
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.
Given the recent changes, I've happy to approve merging this as a draft.
@sudo-bmitch thanks for the OCI specific information! This TAP should cleanly apply to OCI by using the digest of the "root" manifest. I'd prefer to add this use case in a separate PR though rather than in this one, so that we can explore this structure some more. |
This TAP proposes supporting Merkle DAG objects / nodes as targets in TUF metadata. This allows us to extend TUF's protections to various popular applications like Git and other content addressable systems like IPFS and OSTree.
The TAP goes into some detail about the properties the application of ecosystem under consideration must possess to ensure the integrity of the hash values. Feedback as a whole is welcome! There are some discussion points in line that I'm also going to copy here:
length
for Git objects? The commit object files pre-compression?