Skip to content
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

Closed
wants to merge 29 commits into from
Closed

Conversation

kpk47
Copy link
Contributor

@kpk47 kpk47 commented Jun 9, 2023

  1. Explain the VSA's core assertion.
  2. Update the introduction to be consistent with (1).
  3. Update language to differentiate "users" between "VSA producers" and "VSA consumers".

@netlify
Copy link

netlify bot commented Jun 9, 2023

Deploy Preview for slsa ready!

Name Link
🔨 Latest commit ead4e1c
🔍 Latest deploy log https://app.netlify.com/sites/slsa/deploys/64f8ff113067ce0008a34788
😎 Deploy Preview https://deploy-preview-882--slsa.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Member

@joshuagl joshuagl left a 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?

docs/verification_summary/v1.md Outdated Show resolved Hide resolved
docs/verification_summary/v1.md Outdated Show resolved Hide resolved
docs/verification_summary/v1.md Outdated Show resolved Hide resolved
@kpk47 kpk47 marked this pull request as ready for review June 23, 2023 15:24
@kpk47 kpk47 requested a review from MarkLodato as a code owner June 23, 2023 15:24
@kpk47 kpk47 requested a review from joshuagl June 26, 2023 19:11
Copy link
Contributor

@TomHennen TomHennen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

docs/verification_summary/v1.md Outdated Show resolved Hide resolved
docs/verification_summary/v1.md Outdated Show resolved Hide resolved
docs/verification_summary/v1.md Outdated Show resolved Hide resolved
docs/verification_summary/v1.md Outdated Show resolved Hide resolved
docs/verification_summary/v1.md Outdated Show resolved Hide resolved
@kpk47 kpk47 requested review from TomHennen, MarkLodato, AdamZWu, laurentsimon and olivekl and removed request for AdamZWu June 28, 2023 21:44
Copy link
Member

@MarkLodato MarkLodato left a 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.

docs/verification_summary/v1.md Outdated Show resolved Hide resolved
docs/verification_summary/v1.md Outdated Show resolved Hide resolved
docs/verification_summary/v1.md Outdated Show resolved Hide resolved
docs/verification_summary/v1.md Outdated Show resolved Hide resolved
Signed-off-by: kpk47 <[email protected]>
MarkLodato and others added 7 commits July 11, 2023 16:24
- 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]>
Update the list of meeting notes on slsa.dev/notes to no longer say
"bi-weekly".

Signed-off-by: Mark Lodato <[email protected]>
@kpk47 kpk47 changed the title content: Update VSA for SLSA v1.1 content: Clarify the VSA's purpose and how to verify VSAs. Jul 11, 2023
@kpk47 kpk47 requested a review from MarkLodato July 11, 2023 23:45
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
Copy link
Contributor

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?

Copy link
Member

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?

Copy link
Contributor

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


- `resourceUri` is R.

- `slsaResult` is `PASSED`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As above, verificationResult


- `resourceUri` is R.

- `slsaResult` is `PASSED`.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verificationResult

@@ -240,13 +252,111 @@ WARNING: This is just for demonstration purposes.

<div id="slsaresult">
Copy link
Contributor

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'?

Copy link
Member

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.

Copy link
Contributor

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. :)

>
> 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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

verifier -> consumer?

Copy link
Member

@MarkLodato MarkLodato left a 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.

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
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
artifact's verification, allowing consumers to delegate verification decisions
artifact's verification, allowing consumers to delegate verification decisions

Copy link
Member

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 and timeVerified 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...

@@ -240,13 +252,111 @@ WARNING: This is just for demonstration purposes.

<div id="slsaresult">
Copy link
Member

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.

Comment on lines 189 to 190
> This field MAY be absent if the verifier is not attesting to a specific SLSA
> level.
Copy link
Member

@MarkLodato MarkLodato Jul 13, 2023

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?

Comment on lines 43 to 53
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.
Copy link
Member

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

Copy link
Contributor

@AdamZWu AdamZWu Jul 13, 2023

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".

Comment on lines 281 to 283
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.
Copy link
Member

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.

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
Copy link
Member

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?


- `slsaResult` is `PASSED`.

- `verifiedLevels` contains `SLSA_BUILD_LEVEL_UNEVALUATED`.
Copy link
Member

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?

## _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
Copy link
Member

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.

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)_
Copy link
Member

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?

@MarkLodato
Copy link
Member

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?

@kpk47 kpk47 changed the title content: Clarify the VSA's purpose and how to verify VSAs. editorial: Clarify language in VSA spec Sep 6, 2023
@kpk47
Copy link
Contributor Author

kpk47 commented Sep 6, 2023

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.
Copy link
Contributor

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
Copy link
Contributor

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.

Copy link
Contributor

@AdamZWu AdamZWu Sep 7, 2023

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:

  1. 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;
  2. 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.
Copy link
Contributor

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
Copy link
Contributor

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
Copy link
Contributor

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...?

Copy link
Contributor

@AdamZWu AdamZWu Sep 7, 2023

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`.
Copy link
Contributor

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
Copy link
Contributor

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?

Copy link
Contributor

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]

@kpk47 kpk47 closed this Oct 10, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

10 participants