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.

Sign up for GitHub

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

editorial: Clarify language in VSA spec #882

Changes from 5 commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
8b0b25e
Update VSA for SLSA v1.1
kpk47 Jun 9, 2023
1233829
Address review comments
kpk47 Jun 23, 2023
68adac3
clarify that you can set verifiedLevels without setting policy
kpk47 Jun 26, 2023
956dc5a
Clarify that verifiedLevels can contain custom values.
kpk47 Jun 28, 2023
b0a877b
reworked How to Verify section
kpk47 Jun 28, 2023
60396db
fix formatting
kpk47 Jun 29, 2023
a71652a
fix formatting
kpk47 Jun 29, 2023
0d386cf
review comments
kpk47 Jun 29, 2023
f841bcf
Apply suggestions from code review
kpk47 Jun 30, 2023
46dcb8f
resolve issues with verifiedLevels
kpk47 Jun 30, 2023
abe634c
Clarify wording around roles; update introduction
kpk47 Jul 5, 2023
b1ef965
Merge branch 'main' of https://github.com/kpk47/slsa into vsa
kpk47 Jul 5, 2023
7800f4b
Apply suggestions from code review
kpk47 Jul 11, 2023
ee07742
impl: update README to explain how to use Netlify (#898)
MarkLodato Jul 6, 2023
359c50a
impl: add missing links in relatedwork doc (#912)
joshuagl Jul 10, 2023
79fdfe1
impl: update renovate config for conventional commits (#911)
joshuagl Jul 10, 2023
2f22cf3
impl: Update github-actions (#909)
renovate-bot Jul 11, 2023
4cde255
impl: Remove dashes from types in PR name lint (#903)
arewm Jul 11, 2023
c0e01a1
nonspec: community meeting is no longer biweekly (#906)
MarkLodato Jul 11, 2023
d0ef5f4
grammar/clarity edits
kpk47 Jul 11, 2023
f2e1d11
Added changelog for this PR
kpk47 Jul 11, 2023
614a115
Merge branch 'main' of https://github.com/kpk47/slsa into vsa
kpk47 Jul 11, 2023
a219938
lint
kpk47 Jul 11, 2023
3c8ee37
Merge branch 'main' of https://github.com/slsa-framework/slsa into vsa
kpk47 Sep 5, 2023
1febf58
move changes to v1.1 directory
kpk47 Sep 5, 2023
1fa3fa9
backport editorial changes to v1.0
kpk47 Sep 6, 2023
e214c2a
move ediorial changes to v1.1 directory
kpk47 Sep 6, 2023
03fe050
fix link refs
kpk47 Sep 6, 2023
ead4e1c
fix link caps
kpk47 Sep 6, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
106 changes: 96 additions & 10 deletions docs/verification_summary/v1.md
View file
Open in desktop
kpk47 marked this conversation as resolved.
Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Could you update the PR title and description to be more, er, descriptive?

  • title: summarize the change rather than just "update for v1.1"
  • desc: Could you go into more detail? "Update the introduction" doesn't really explain how the intro was changed or why, and similarly for why the fields are now optional or why the section on verifying VSAs is warranted.

This is also a breaking change (change to required field) so we should use the BREAKING CHANGE: footer as per https://www.conventionalcommits.org/en/v1.0.0/.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

There's a lot of content in this PR. Do you think the summary does it justice?

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

+1 to splitting the PR. I'll keep "how to verify" in this one and move the rest to new ones.

Copy link
Member

Choose a reason for hiding this comment

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

I'm worried that this new version is in a weird limbo where it's half SLSA-specific and half generic. For example, it's not clear if you need to list a SLSA build level even if you're saying nothing about the build level.

Would it make sense to make it fully generic and perhaps move it to the in-toto repo? Moving it to in-toto would have the side benefit of avoiding a SLSA version number bump.

Note that provenance would still remain within the SLSA repo because it's inextricably tied to the SLSA build model and SLSA build levels. VSA, on the other hand, seems actually quite generic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've been wondering the same thing. I don't have an answer, but I've got a couple of questions/thoughts that may help shape the decision.

  • Do we expect VSAs that communicate strictly non-SLSA properties (e.g. an attestation for a property explicitly out of SLSA's scope)? If so, we should try to move the VSA spec out of slsa-framework.
  • Does SLSA expect to support any attestations that would be alternatives to VSA? If so, then we should update the spec with a more general description of a summary attestation and its minimum requirements to convey SLSA provenance faithfully.

If we do keep the VSA spec under slsa-framework, then the community needs to articulate a clearer vision of its role in the SLSA ecosystem. If nothing else, we should update the "How-to SLSA" pages with more VSA examples.

Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ Understanding of SLSA [Software Attestations](/attestation-model),

A Verification Summary Attestation (VSA) is an attestation that some entity
kpk47 marked this conversation as resolved.
Show resolved Hide resolved
(`verifier`) verified one or more software artifacts (the `subject` of an
in-toto attestation [Statement]) by evaluating the artifact and a `bundle`
of attestations against some `policy`. Users who trust the `verifier` may
assume that the artifacts 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.
in-toto attestation [Statement]) by evaluating the artifact and its associated
attestation(s) against some `policy`. Specifically, a VSA is a claim by
`verifier` that `subject` passed the policy for `resourceUri`. Users who trust
kpk47 marked this conversation as resolved.
Show resolved Hide resolved
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.
kpk47 marked this conversation as resolved.
Show resolved Hide resolved

The VSA also allows consumers to determine the verified levels of
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 this section is saying: if the transitive dependencies have VSAs themselves, then this VSA summarizes them also. And if there's provenance but no VSA for a dependency, this top-level VSA will summarize the provenance, essentially creating transitive VSAs within a top-level VSA. And if there's no provenance for dependencies, the VSA does nothing for them.

Is that correct?

Copy link
Contributor

@AdamZWu AdamZWu Jun 30, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The basic idea is that, the dependencyLevels in a VSA summarizes the policy verification results for all transitive dependencies.

The policy verification result of a dependency could either come from an "actual" verification, i.e. evaluation the provenance of the dependency against a policy for the dependency Uri; or from a VSA that summarizes a prior policy verification on this dependency.

Effectively, VSA functions as a verification "shortcut" -- if a matching and valid VSA is found, the verifier no longer needs to evaluate the artifact provenance and its entire dependency sub-tree.

If a dependency has no VSA nor provenance, what happens really depends on the policy for its Uri.

  • If the policy says "evaluate at L0" (effectively bypasses the provenance check), then the recursive verification will pass at L0;
  • Otherwise, since a provenance is required but missing, the recursive verification will fail.

The dependencyLevels in the dependent's VSA will record the recursive verification results accordingly.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, interesting, thanks! @kpk47 would you be open to including some of this language? This explanation makes sense to me, but I'm not sure I'd have gleaned that from the current paragraph.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I gave it a go, but went slightly higher level and removed the decision points. @olivekl @AdamZWu does my summary make sense to you and match your understanding of the chained verification process?

all of an artifact’s _transitive_ dependencies. The verifier does this by
Expand Down Expand Up @@ -122,7 +124,7 @@ of the other top-level fields, such as `subject`, see [Statement]._
> can sign provenance for the "Google Cloud Deploy" verifier, but "GitHub" cannot
> sign for the "Google Cloud Deploy" verifier.
>
> The field is required, even if it is implicit from the signer, to aid readability and
> This field is required, even if it is implicit from the signer, to aid readability and
> debugging. It is an object to allow additional fields in the future, in case one
> URI is not sufficient.

Expand All @@ -132,21 +134,25 @@ of the other top-level fields, such as `subject`, see [Statement]._
> URI indicating the verifier’s identity.

<a id="timeVerified"></a>
`timeVerified` _string ([Timestamp]), required_
`timeVerified` _string ([Timestamp]), optional_

> Timestamp indicating what time the verification occurred.

<a id="resourceUri"></a>
`resourceUri` _string ([ResourceURI]), required_

> URI that identifies the resource associated with the artifact being verified.
>
> This field is required to prevent confusion attacks. E.g., a VSA indicating
> that a package can be published as `foo` should not be useable to publish
kpk47 marked this conversation as resolved.
Show resolved Hide resolved
> the package as `bar`.
kpk47 marked this conversation as resolved.
Show resolved Hide resolved

<a id="policy"></a>
`policy` _object ([ResourceDescriptor]), required_
`policy` _object ([ResourceDescriptor]), optional_

> Describes the policy that the `subject` was verified against.
>
> The entry MUST contain a `uri`.
> This field is RECOMMENDED.
Copy link
Member

@MarkLodato MarkLodato Jul 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

nit: Why is it recommended? (or alternatively just remove the recommendation if we don't have a good reason)


<a id="inputAttestations"></a>
`inputAttestations` _array ([ResourceDescriptor]), optional_
Expand All @@ -167,7 +173,7 @@ of the other top-level fields, such as `subject`, see [Statement]._
> Either “PASSED” or “FAILED” to indicate if the artifact passed or failed the policy verification.

<a id="verifiedLevels"></a>
`verifiedLevels` _array ([SlsaResult]), required_
`verifiedLevels` _array ([SlsaResult]), optional_
laurentsimon marked this conversation as resolved.
Show resolved Hide resolved
kpk47 marked this conversation as resolved.
Show resolved Hide resolved

> Indicates the highest level of each track verified for the artifact (and not
> its dependencies), or "FAILED" if policy verification failed.
Expand All @@ -176,6 +182,15 @@ of the other top-level fields, such as `subject`, see [Statement]._
> level implies all levels below it (e.g. SLSA_BUILD_LEVEL_3 implies
> SLSA_BUILD_LEVEL_2 and SLSA_BUILD_LEVEL_1), so there is no need to
> include more than one level per track.
>
> Users MAY add additional, non-SLSA properties to this field provided the
kpk47 marked this conversation as resolved.
Show resolved Hide resolved
> values do not conflict with the definition of [SlsaResult]. Users SHOULD
kpk47 marked this conversation as resolved.
Show resolved Hide resolved
> negotiate the meaning of such properties with their intended verifier.
>
> This field MAY be absent if the verifier is not attesting to a specific SLSA
> level.
Copy link
Member

@MarkLodato MarkLodato Jul 13, 2023 •
edited
Loading

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?

>
> This field MAY be set even if the `policy` field is not set.
kpk47 marked this conversation as resolved.
Show resolved Hide resolved

<a id="dependencyLevels"></a>
`dependencyLevels` _object, optional_
Expand Down Expand Up @@ -240,6 +255,77 @@ 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. :)


## How to Verify
kpk47 marked this conversation as resolved.
Show resolved Hide resolved

Consumers use VSAs to accomplish goals based on delegated trust. We call the
process of establishing a VSA's authenticity and determining whether it meets
the consumer's goals 'verification'. Goals differ, as do levels of confidence
in VSA producers, so the verification procedure will changes to suit its
kpk47 marked this conversation as resolved.
Show resolved Hide resolved
context. However, there are certain steps that most verification procedures
have in common.

Verification SHOULD include the following steps:
kpk47 marked this conversation as resolved.
Show resolved Hide resolved

1. Verify the signature on the VSA envelope using the preconfigured roots of
kpk47 marked this conversation as resolved.
Show resolved Hide resolved
trust.

2. Verify the statement's `subject` matches the digest of the artifact in
question.

3. Verify that the `predicateType` is
`https://slsa.dev/verification_summary/v1`.

4. Verify that the `verifier` matches the public key (or equivalent) used to
verify the signature in step 1.

5. Verify that the value for `resourceUri` in the VSA matches the expected
value.

6. Verify that the value for `slsaResult` is `PASSED`.

kpk47 marked this conversation as resolved.
Show resolved Hide resolved
7. (Optional) Verify additional fields required to determine whether the VSA
meets your goal.

Verification mitigates different threats depending on the VSA's contents and the
verification procudure.

IMPORTANT: A VSA does not protect against compromise of the verifier, such as by
a malicious insider. Instead, VSA users SHOULD carefully consider which
verifiers they add to their roots of trust.

### Examples
kpk47 marked this conversation as resolved.
Show resolved Hide resolved

1. Suppose party X wants to delegate to party Y the decision for whether to
kpk47 marked this conversation as resolved.
Show resolved Hide resolved
publish artifact A to resource R. Party X verifies that:
kpk47 marked this conversation as resolved.
Show resolved Hide resolved
-The signature on the VSA envelope using Y's public signing key from their
preconfigured root of trust.
-`subject` is A.
-`predicateType` is `https://slsa.dev/verification_summary/v1`.
-`verifier.id` is Y.
-`resourceUri` is R.
-`slsaResult` is `PASSED`.

Note: This example is analogous to traditional code signing. The verifier
does not need to check additional fields, as X fully delegates the decision
to Y.
2. Suppose party X wants to enforce the rule "Artifact A at resource R must
Copy link
Contributor

Choose a reason for hiding this comment

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

How would you feel about a diagram here, or earlier in the page showing the process generally? Would it be overkill to show a high-level process diagram of who is creating what, who is verifying what, who is consuming that verification, etc?
Maybe not in scope of this PR, though!

Copy link
Contributor

Choose a reason for hiding this comment

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

+1 on having a diagram with names, and use the names consistently. I left a few comments where the use of users, party, verifiers (VSA consumer or VSA producer) is not always obvious to readers.

May even be simpler to call the "verifier" the "VSA producer" :)

iiuc, the examples in the text assume that the consumer is the same as the producer, and that's why the X and Y can negotiate. But that's not always the case: Example Chromium -> Google VSA -> consumers. Can we illustrate this example as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I like the idea of a diagram, but I'm having trouble imagining what it would look like. There's a lot of overlap in the roles

  • Artifact producer
  • Artifact verifier (also VSA producer)
  • Artifact consumer (may also be VSA verifier, VSA consumer)
  • VSA producer (also artifact verifier)
  • VSA verifier (may also be VSA consumer, artifact consumer)
  • VSA consumer (may also be VSA verifier, artifact consumer)

And the flow loops on itself because each VSA verifier can itself create VSAs indicating that some combination of VSAs passed its policy.

Any suggestions on how to represent that in a picture?

have a passing VSA from party Y showing it meets SLSA Build Level 2+ that
is at most Z old." Party X verifies that:
-The signature on the VSA envelope using Y's public signing key from their
preconfigured root of trust.
-`subject` is A.
-`predicateType` is `https://slsa.dev/verification_summary/v1`.
-`verifier.id` is Y.
-`resourceUri` is R.
-`slsaResult` is `PASSED`.
-'verifiedLevels` is `SLSA_BUILD_LEVEL_2` or `SLSA_BUILD_LEVEL_3`.
-`timeVerified` is no earlier than the current time - Z.
kpk47 marked this conversation as resolved.
Show resolved Hide resolved

Note: In this example, verifying the VSA mitigates the same threats as
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?


</div>
Expand Down