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 1 commit
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
77 changes: 67 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 @@ -184,7 +184,11 @@ of the other top-level fields, such as `subject`, see [Statement]._
> 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].
> 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

Expand Down Expand Up @@ -253,22 +257,75 @@ WARNING: This is just for demonstration purposes.

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

Verification SHOULD include the following steps
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`.

1. Verify the signature on the VSA envelope using the preconfigured roots of 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.
4. Verify that the `verifier` matches the public key (or equivalent) used to
verify the signature in step 1.

Resulting threat mitigation: See
[Verifying artifacts](/spec/v1.0/verifying-artifacts) for details about which
threats are addressed by verifying each SLSA level.
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