Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Provenance v1.0: initial draft #525
Provenance v1.0: initial draft #525
Changes from 26 commits
b20437c2a3eb61ad7e823cdecf522ef3659e7dd1e0c3089de25c78c31015166861d844f499fadcec378582a3c28df6bec9668f41a5a96d3ac397e76f1adaf3f5a4b06b661ee15aab59b78ae06f11459e65d1e79132f9d2fd044c3d3ae85a9e60f742479b7b39ca7346d63f6ba03a06600ca0d698b32358622c0b577d5814a18326b3b728b4a4494fb31094f7472ba94fdce758268a64daba878e1987abf40aeb774210074File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
Trying to map the discussion in #508 back to this PR and the updated requirements. We had recommended adding fields for evidence for the "scripted build" and "build as code" requirements. Even though the requirements have been updated, we still want to make sure we include a field in the provenance where the producer can attest to having used a specific build script and/or service addressing the
Producerrequirements, even if they do not want to reveal the build service or script in the provenance.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.
Based on the discussion, I think we do not want such a field. There is an out-of-band process for certifying that a given
builder.idimplies that various requirements have been met. It is not a property per build.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.
Ok, so the
builder.idwould establish the binding between the provenance and the infrastructure provider questionnaire? However, presumably, in the case of a third-party verifier service who certified the build level, does this mean that the provenance consumer needs to go separately query the verifier service for the certification?My main concern is that consumers of the provenance won't be able to (easily) link/connect the provenance to the evidence (i.e., questionnaire) for the claimed build level, so I'm still wondering why the provenance might not provide a hint for where to find the certification of the
builder.id, especially in the case when the consumer doesn't rely on the third-party verifier.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.
Put differently, does this current approach prevent attackers from producing "fake" provenance as
builder.idthat a consumer/verifier would deem valid, based on the questionnaire? Without a binding in the provenance to the authenticated questionnaire (or a verifier's certification), I don't see how this would be detected by a verifier.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.
IIUC, the concern is that a
builder.idinside the provenance can be forged by anyone, and a builder is defined not just by its ID, but by its keys. So you're asking how a verifier knows how to verify, i.e., not just the builder.id verification but also the signature verification.If there is an official tool like the slsa-verifier that supports the builder, users should know about it. If there is none, what do we do? Do we expect corporate builders to provide their own verifying tool...? Or shall we encourage folks to add support in the same tool (e.g., slsa-verifier) and work on a plug-in support slsa-framework/slsa-verifier#275 to scale that?
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 might be just me but I had always assumed that a lot of the verification assumed you in the least trusted the identity of the signature that it's reporting accurate information. I had also assumed the certification would be an external certification something like: Alice certified on this date that Bob's builder service is SLSA 3.
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.
Yes, as @laurentsimon said, the questionnaire is tied to signature verification plus an optional
builder.id.By "signature verification" this really means some sort of public key infrastructure that identifies the attestor, i.e. who signed the attestation. It could be a simple public key, or an X.509 cert chain like Sigstore, or something like SPIFFE.
The
builder.idexists to allow a single attestor identity to represent two different logical build systems, with two different SLSA properties. For example, suppose Awesome Builder meets SLSA Build L3 but allows users to disable isolation between builds, which allows tampering between builds but does not allow forging of the provenance. Awesome Builder could represent this in two ways:builder.idvalues, signed by the same attestor identity.I'm not 100% sure if (b) is needed. If not, we could drop
builder.idwhich would simplify things a lot, I think. But I hesitate to do that until we have confidence it's not needed. Maybe just a bigger warning around it for now?Sorry, I'm not sure who this comment was addressed to, or if you'd like to see anything in this PR about it.
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.
That second bullet point I think we just need to be clear about. I'm thinking about implementation issues where having the same builder use different identities in different contexts could complicate things. I think (b) probably makes the most sense though it could also lead to a compromised builder claiming a higher SLSA level.
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 clarifying. I think that the verification flow was not fully clear to me since we added in the infrastructure questionnaire step. There is a dependency between the artifact/provenance and the infrastructure on which they were produced, but what we're really saying is that the verification of this dependency can be handled OOB because the infrastructure for a given
builder.idisn't expected to change very often. And because thebuilder.idmaps to a specific signing key for the infrastructure, that's all a provenance consumer needs to verify the build infrastructure.As @mlieberman8 said, the documentation should be clear on all these assumptions and expectations.
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.
Is this thread OK to resolve, or is there a specific request here (hard to follow thread)?