-
Notifications
You must be signed in to change notification settings - Fork 225
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
editorial: Clarify language in VSA spec #882
Conversation
kpk47
commented
Jun 9, 2023
•
edited
Loading
edited
- Explain the VSA's core assertion.
- Update the introduction to be consistent with (1).
- Update language to differentiate "users" between "VSA producers" and "VSA consumers".
✅ Deploy Preview for slsa ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
Signed-off-by: kpk47 <[email protected]>
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 is a great start, thanks!
I compared the PR here to the suggested actions in #878 and noticed that the intro still includes bundle
. I'm not entirely certain we need to remove bundle
, perhaps we could spend a few more words describing that a VSA may be attesting to a single attestation OR a bundle of attestations?
Signed-off-by: kpk47 <[email protected]>
Signed-off-by: kpk47 <[email protected]>
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!
Signed-off-by: kpk47 <[email protected]>
Signed-off-by: kpk47 <[email protected]>
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.
Looks good to me. The examples really help! A few more suggestions.
Signed-off-by: kpk47 <[email protected]>
- Document that we use Netlify instead of GitHub Pages now. - Update the "developing and testing locally" instructions to use `netlify dev` instead of running Jekyll directly so that redirects are handled properly. - Document how deploy previews work. - Document our production Netlify and DNS configuration. - Start a short playbook for how to debug. It likely could be expanded, but this is a good first start. Issue: slsa-framework#266 Signed-off-by: Mark Lodato <[email protected]> Co-authored-by: Joshua Lock <[email protected]>
The reference links for "Threats, Risks, and Mitigations in the Open Source Ecosystem" and "Binary Authorization for Borg" were missing. Signed-off-by: Joshua Lock <[email protected]>
…#911) Update the renovate configuration so that patches and PRs from renovate follow the project's conventional commit guidelines. Fixes: slsa-framework#910 Signed-off-by: Joshua Lock <[email protected]>
This PR contains the following updates: actions/deploy-pages: `v2.0.2` -> `v2.0.3` actions/setup-node: `v3.6.0` -> `v3.7.0` actions/upload-pages-artifact: `v1.0.9` -> `v1.0.10 Signed-off-by: Mend Renovate <[email protected]>
Dashes are not supported by the GitHub Action PR linter we use: https://github.com/amannn/action-semantic-pull-request resolves: slsa-framework#902 Signed-off-by: arewm <[email protected]>
Update the list of meeting notes on slsa.dev/notes to no longer say "bi-weekly". Signed-off-by: Mark Lodato <[email protected]>
Signed-off-by: kpk47 <[email protected]>
Signed-off-by: kpk47 <[email protected]>
Signed-off-by: kpk47 <[email protected]>
docs/verification_summary/v1.md
Outdated
value. This step ensures that the consumer is using the VSA for the | ||
producer's intended purpose. | ||
|
||
6. Verify that the value for `slsaResult` is `PASSED`. This step ensures the |
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.
Instead of slsaResult
this should be verificationResult
, right?
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 the confusion results from overlap in verificationResult
and verifiedLevels
. The latter also has a FAILED
entry, and the name implies that it's only for PASSED. Should we deprecate verificationResult
and just say verifiedLevels
are only for things that passed?
I'm assuming the original intention was to support saying "I checked at L2 and it failed". If so, is that a foot gun?
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.
So, if verificationResult != 'PASSED'
then verifiedLevels
must be empty? If so, SGTM
docs/verification_summary/v1.md
Outdated
|
||
- `resourceUri` is R. | ||
|
||
- `slsaResult` is `PASSED`. |
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.
As above, verificationResult
docs/verification_summary/v1.md
Outdated
|
||
- `resourceUri` is R. | ||
|
||
- `slsaResult` is `PASSED`. |
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.
verificationResult
docs/verification_summary/v1.md
Outdated
@@ -240,13 +252,111 @@ WARNING: This is just for demonstration purposes. | |||
|
|||
<div id="slsaresult"> |
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.
Should this div go down near line 352? Should there be a different div id for 'how to verify'?
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 actually think SlsaResult should move up here.
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's fine too, as long as they go together. :)
docs/verification_summary/v1.md
Outdated
> | ||
> VSA producers MAY add additional, non-SLSA properties to this field provided | ||
> the values do not conflict with the definition of [SlsaResult]. VSA Producers | ||
> SHOULD negotiate the meaning of such properties with their intended 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.
verifier -> consumer?
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.
Sorry for so many comments.
docs/verification_summary/v1.md
Outdated
Verification summary attestations communicate that an artifact has been verified | ||
at a specific SLSA level and details about that verification. | ||
Verification summary attestations convey high-level information about an | ||
artifact's verification, allowing consumers to delegate verification decisions |
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.
artifact's verification, allowing consumers to delegate verification decisions | |
artifact's verification, allowing consumers to delegate verification decisions |
docs/verification_summary/v1.md
Outdated
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.
Personally I would go into more detail, particularly to explain the reason for the change. Something like:
- Clarify that VSA can be used to express verification of things other than the SLSA Level. Previously this was technically possible, but it was only mentioned in one sentence (under SlsaResult). Now the purpose, model, and field documentation make this explicit.
- Make fields
policy
andtimeVerified
optional because [...] - Add new "How to verify" section, including examples. Previously it was unclear to many readers exactly how VSAs were intended to be used, leading to possible mistakes. Now we make this explicit. This also brings the VSA spec up to parity with Provenance, which already has such a section.
- Add a
SLSA_BUILD_LEVEL_UNEVALUATED
entry because [...] - Explain why
resourceUri
is required. - Other edits for clarity.
Hmm, this PR is getting a bit big. Might make sense to split...
docs/verification_summary/v1.md
Outdated
@@ -240,13 +252,111 @@ WARNING: This is just for demonstration purposes. | |||
|
|||
<div id="slsaresult"> |
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 actually think SlsaResult should move up here.
docs/verification_summary/v1.md
Outdated
> This field MAY be absent if the verifier is not attesting to a specific 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.
Line 175 says required but here it says it's optional. Which is it?
If required, what does that mean? Is one of the SLSA_
values required? When we have more than one track, is it required to have exactly one entry for each track?
docs/verification_summary/v1.md
Outdated
The VSA also allows consumers to determine the verified levels of | ||
all of an artifact’s _transitive_ dependencies. The verifier does this by | ||
either a) verifying the provenance of each non-source dependency listed in | ||
the [resolvedDependencies](/provenance/v1#resolvedDependencies) of the artifact | ||
being verified (recursively) or b) matching the non-source dependency | ||
listed in `resolvedDependencies` (`subject.digest` == | ||
`resolvedDependencies.digest` and, ideally, `vsa.resourceUri` == | ||
`resolvedDependencies.uri`) to a VSA _for that dependency_ and using | ||
`vsa.verifiedLevels` and `vsa.dependencyLevels`. Policy verifiers wishing | ||
to establish minimum requirements on dependencies SLSA levels may use | ||
`vsa.dependencyLevels` to do so. |
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.
We seem to have lost this level of detail, e.g. checking vsa.resourceUri
against provenance.resolvedDependencies.uri
. Should this go somewhere as well? Perhaps a new "How to produce" section?
Alternatively should this detail go in the SLSA Provenance "How to verify" section, since it's more about provenance?
/cc @TomHennen
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 vote not to mention those here.
The check of vsa.resourceUri
and provenance.resolvedDependencies.uri
really belongs to the topic of policy and verification, and the URI matching is an opinionated way to check whether the short-circuit is permitted.
In order for that to work, there are some implicit assumptions, namely 1) the policy being evaluated is completely determined by the URIs; 2) there is a 1:1 mapping between the URIs and policies; 3) both vsa.resourceUri
and provenance.resolvedDependencies.uri
follow the same specs.
We could either spend a lot of energy fully flesh out all these, and in the end we just described one way it can work; Meanwhile, there are other ways short-circuiting could work, for example the policy.uri
and policy.digest
also contain information that may be used for making decision.
Alternatively, I think it is better just leave a high-level description here that "the verifier can use VSA to short-circuit policy verification when appropriate" and leave the details in SLSA Provenance "How to verify".
docs/verification_summary/v1.md
Outdated
5. Verify that the value for `resourceUri` in the VSA matches the expected | ||
value. This step ensures that the consumer is using the VSA for the | ||
producer's intended purpose. |
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 piece probably needs more fleshing out, perhaps in a separate PR. There have been many questions about what the resource URI is, e.g. #891.
docs/verification_summary/v1.md
Outdated
value. This step ensures that the consumer is using the VSA for the | ||
producer's intended purpose. | ||
|
||
6. Verify that the value for `slsaResult` is `PASSED`. This step ensures the |
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 the confusion results from overlap in verificationResult
and verifiedLevels
. The latter also has a FAILED
entry, and the name implies that it's only for PASSED. Should we deprecate verificationResult
and just say verifiedLevels
are only for things that passed?
I'm assuming the original intention was to support saying "I checked at L2 and it failed". If so, is that a foot gun?
docs/verification_summary/v1.md
Outdated
|
||
- `slsaResult` is `PASSED`. | ||
|
||
- `verifiedLevels` contains `SLSA_BUILD_LEVEL_UNEVALUATED`. |
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 don't understand this piece. Why does someone check that the build level was unevaluated? If they don't care, shouldn't they just ignore it?
docs/verification_summary/v1.md
Outdated
## _SlsaResult (String)_ | ||
|
||
</div> | ||
|
||
The result of evaluating an artifact (or set of artifacts) against SLSA. | ||
SHOULD be one of these values: | ||
|
||
- SLSA_BUILD_LEVEL_UNEVALUATED |
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.
What is the purpose of this new field? That's not clear to me.
docs/verification_summary/v1.md
Outdated
verifying the artifact's SLSA provenance. See | ||
[Verifying artifacts](/spec/v1.0/verifying-artifacts) for details about which | ||
threats are addressed by verifying each SLSA level. | ||
|
||
## _SlsaResult (String)_ |
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.
Q: Should this enum name be changed to something not slsa specific?
The v1.1 directory now exists as of #942. Should this change move there? or alternatively split out the editorial from content changes, and apply the former to v1.0 and the latter to v1.1? |
Signed-off-by: kpk47 <[email protected]>
Signed-off-by: kpk47 <[email protected]>
Signed-off-by: kpk47 <[email protected]>
Signed-off-by: kpk47 <[email protected]>
Signed-off-by: kpk47 <[email protected]>
I've split this PR into two changes. This one contains the original's editorial changes, moved to the v1.1 directory. I moved the content changes to #964. |
the dependencies were verified at. | ||
Assert that the VSA producer has verified an artifact or set of artifacts. | ||
Optionally include details about the verification process, such as the verified | ||
SLSA level(s) and the verifier's 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.
verifier's expectations
== policy
? In the rest of the text we say policy
. Shall we use the same term throughout the doc? It's a bit confusing otherwise, at least for me.
in-toto attestation [Statement]) by evaluating the artifact and its associated | ||
attestation(s) against the `policy` for `resourceUri`. Consumers who trust | ||
the `verifier` may assume that the artifacts identified by the | ||
`(subject, resourceUri)` pair met the indicated SLSA level without |
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.
to be more general and follow earlier wording, we could say pair met the details about the verification process
.
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 including both may be more helpful, i.e. "... met the indicated SLSA level and other criteria in the policy".
We are implementing a producer/consumer of VSA v1.0 and realized that there are two different scenarios:
- We consume VSA produced by ourselves: in this case we could leverage the extension attributes to fully convey all the information that we are interested in, such as additional verification beyond what is defined in the SLSA BUILD levels;
- We consume VSA produced by others (and similarly, others consume VSA produced by us): in this case, since the consumers and producers may not have mutual definitions on extended checks, to make the VSA useful, we must all resort to the standard language, i.e. SLSA defined levels.
So, if we are producing a VSA without foreknowledge of who will consume it, we should put in both SLSA levels and the extension attributes in the VSA.
the `verifier` may assume that the artifacts identified by the | ||
`(subject, resourceUri)` pair met the indicated SLSA level without | ||
themselves needing to evaluate the artifact or to have access to the | ||
attestations the `verifier` used to make its determination. |
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.
may add another example that the VSA works in cases where the repository is not public. That's why we can't verify SLSA provenance directly for closed-source software.
attestations the `verifier` used to make its determination. | ||
|
||
VSAs can also be chained together to meet higher level goals, such as tracking | ||
the verified SLSA level(s) for the `subject`'s transitive dependencies. Rather |
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.
same comment as before: if we're saying that levels and other verified metadata is optional, we could be more generic tracking details about the verification process for the subject's transitive dependencies
.
VSAs can also be chained together to meet higher level goals, such as tracking | ||
the verified SLSA level(s) for the `subject`'s transitive dependencies. Rather | ||
than verifying provenance for the artifact and each of its transitive | ||
dependencies all at once, the verifier can verify each dependency independently |
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.
If a verifier can verify provenance and generate VSA, why does it need to verify them independently? Is this for security or is it a caching (optimization) mechanism? Is there an assumption in the sentence that maybe there are multiple verifiers
, one that verified dependencies (say, RedHat) while another consumers it (say, Google) to validate a Google binary? Maybe the sentence can be expanded to motivate both cases...?
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 this part refers to the "divide-and-conquer" strategy the VSA enables.
For a naive example, if a system image has 20 software packages, and each package has 50 dependency libraries. A complete transitive dependency verification would involve checking 1+20+20*50 = 1021 artifacts during a single verification, which may consume too much time and violate the verifier's latency SLO.
However, with VSA, what can happen is that, when the 20 software packages get staged in an artifact repo, the repo invokes transitive verification for each, so each verification only checks 1+50 artifacts; and the verification produces VSAs, which propagate to the system image's attestation bundle. Then, when we verify the system image, the verifier will only need to perform at 1+20 checks, because the presence of VSAs of the packages eliminates the need to check the dependency libraries (since VSA is a proof that they are already checked).
Effectively, VSA functions as a "distributed dynamic programming table" and facilitates efficient transitive dependency verification.
dependencies all at once, the verifier can verify each dependency independently | ||
and produce VSAs. Finally, the verifier combines those VSAs; the artifact | ||
is the final VSA's `subject` and each transitive dependency is an | ||
entry in `dependencyLevels`. |
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.
same comment: do we want to be generic about "verified metadata" or mention verified levels? If the former, we could have a dedicated section with examples for different types of metadata, starting with a verified level. Just an idea...
themselves needing to evaluate the artifact or to have access to the | ||
attestations the `verifier` used to make its determination. | ||
|
||
VSAs can also be chained together to meet higher level goals, such as tracking |
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.
what does higher-level goal mean?
Not clear what "chained" means. Do VSAs reference each other like in a block chain, ie do they link to a prior VSA in their output format?
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 "chained" refers to the following:
- VSA summarizes verification results; and
- VSA can short-circuit verification.
These two properties feed each other, so that a VSA is able to summarize the verification results on all transitive dependencies. A VSA represents cumulative verification along the dependency chains of an artifact.
Maybe it is more accurate to describe VSAs subsumes from one another.
For a native example, a system image with 20 packages, each with 50 dependency libraries.
Without VSA, the attestation bundle of the system image will look like:
[system image build provenance]
[package 1 build provenance]
...
[package 20 build provenance]
[package 1 library 1 build provenance]
...
[package 1 library 50 build provenance]
[package 2 library 1 build provenance]
...
...
[package 20 library 50 build provenance]
With VSA and staging repo verification, the attestation bundle of the system image will look like:
[system image build provenance]
[package 1 VSA]
...
[package 20 VSA]
After the system image get verified, the attestation bundle of the system image can be reduced to just:
[system image VSA]