feat: vsa support by ramonpetgrave64 · Pull Request #777 · slsa-framework/slsa-verifier

ramonpetgrave64 · 2024-06-20T20:47:36Z

Fixes #542

Adds support for VSAs.

Testing process

added some unit an end-to-end tests

manually invoking

go run ./cli/slsa-verifier/ verify-vsa \
--subject-digest gce_image_id:8970095005306000053 \
--attestation-path ./cli/slsa-verifier/testdata/vsa/gce/v1/gke-gce-pre.bcid-vsa.jsonl \
--verifier-id https://bcid.corp.google.com/verifier/bcid_package_enforcer/v0.1 \
--resource-uri gce_image://gke-node-images:gke-12615-gke1418000-cos-101-17162-463-29-c-cgpv1-pre \
--verified-level BCID_L1 \
--verified-level SLSA_BUILD_LEVEL_2 \
--public-key-path ./cli/slsa-verifier/testdata/vsa/gce/v1/vsa_signing_public_key.pem \
--public-key-id keystore://76574:prod:vsa_signing_public_key \
--print-attestation


{"_type":"https://in-toto.io/Statement/v1","predicateType":"https://slsa.dev/verification_summary/v1","predicate":{"timeVerified":"2024-06-12T07:24:34.351608Z","verifier":{"id":"https://bcid.corp.google.com/verifier/bcid_package_enforcer/v0.1"},"verificationResult":"PASSED","verifiedLevels":["BCID_L1","SLSA_BUILD_LEVEL_2"],"resourceUri":"gce_image://gke-node-images:gke-12615-gke1418000-cos-101-17162-463-29-c-cgpv1-pre","policy":{"uri":"googlefile:/google_src/files/642513192/depot/google3/production/security/bcid/software/gce_image/gke/vm_images.sw_policy.textproto"}},"subject":[{"name":"_","digest":{"gce_image_id":"8970095005306000053"}}]}
Verifying VSA: PASSED

PASSED: SLSA verification passed

TODOS:

open issue on the in_toto attestations repo about the incorrect json fields for vsa 1.0
- VSA JSON struct tags in-toto/attestation#379

…passed" Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

…ject digests Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

kaixu-google · 2024-06-26T16:33:54Z

LGTM.

…

-- Best, Kai Xu

On Wed, Jun 26, 2024 at 10:56 AM Ramon Petgrave ***@***.***> wrote: @kaixu-google <https://github.com/kaixu-google> — Reply to this email directly, view it on GitHub <#777 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/BJNUY46DWIXVXROLBQUMNNDZJLJBDAVCNFSM6AAAAABJUVOGUOVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDCOJRHEZDCMRQGQ> . You are receiving this because you were mentioned.Message ID: ***@***.***>

Hayden-IO

Looks great, great work on this! Just a few tiny style changes

README.md

cli/slsa-verifier/main_regression_test.go

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

Hayden-IO

🥳

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

ramonpetgrave64 · 2024-06-28T16:33:34Z

@laurentsimon @ianlewis

laurentsimon · 2024-06-28T22:50:07Z

README.md

+accomodate subjects that are not simple-files.
+
+
+The verify-vsa command


Do we want to warn that this is a dangerous / advanced option that really nobody should be using. In the future we'll have verify-artifact --vsa-path that will verify the VSA for an artifact (using digest's hash). I don't know how to best discourage usage of this command. Maybe:

Put this command readme into a sub folder

Warn in the command hel this shodul not be used unless you know what you're doing

Why do you think this is dangerous? If you get to the point where you have a VSA you need to verify, I assume you should understand what the VSA represents. So maybe adding a link to https://slsa.dev/spec/v1.1/verification_summary#purpose to make sure users understand one?

@laurentsimon Instead of verify-artifact --vsa-path, I was thinking to continue with verify-vsa path1 path2 ..., or verify-vsa --artifact-path path1 --artifact-path path2 ...

i don't think re-using verify-vsa is consistent with the existing API. We currently use verify-artifact and verify-image, so I think it's the right way to build on existing APIs. You can verify an artifact using provenance, or a VSA, or something else. verify-vsa is a low-level API for the rare use case that the artifact is not available, and should not be used / advertised otherwise.

re: why it's dangerous API. TOCTOU is an example of a vuln https://github.com/slsa-framework/slsa-verifier?tab=readme-ov-file#containers-1. We've been very careful to design the CLI in a way that prevent users from shooting themselves in the foot (well, I suppose they still can :)). Leaving it up to users to calculate a sha is both poor UX (running the CLI now requires computing a hash) and prone to mistakes (not everyone can be an expert)

In general I think of verify-vsa and verify-provenance (which does not exist yet) as low-level APIs that verify signature and match content passed by the caller. The safer APIs users should use are the verify-artifact, verify-image. Wdut?

OK, I see what you're saying. This seems more like a not ideal UX rather than dangerous, since we are asking for a digest, and the digest should be an immutable reference - if it's not, then it's technically non-compliant with SLSA. If we were to add a comment, I might say that we'd like to refactor this at some point to move VSA verification under verify-artifact.

I think of the CLI options more like

verify-artifact actually means "verify-provenance-with-artifact", or "verify-artifact-provenance"

With this thinking, a VSA is not actually a provenance (build provenance). And I'm open to making a new command "verify-artifact-vsa". To avoid the danger of the user not realizing that they're not enjoying the benefits of an actual build provenance.

re: TOCU, you're referring to how verify-image relies on the user using external tools to calculate digests (docs incorrectly says "tarball")

https://github.com/slsa-framework/slsa-verifier?tab=readme-ov-file#the-verify-image-command

verify-artifact actually means "verify-provenance-with-artifact", or "verify-artifact-provenance"

That was not the original intention, and that's why the term 'provenance' is not in the command's name. It's intended to verify artifacts with whatever attestation. It so happens the only one currently supported is provenance, but it need not be. If you ever need to verify an artifact with several attestations, you'll be able to do verify-artifact --provenance-path --source-path etc... unless they are packed into a bundle and you'll likely use --attestations-path like in npm command.

OK, I see what you're saying. This seems more like a not ideal UX rather than dangerous, since we are asking for a digest, and the digest should be an immutable reference - if it's not, then it's technically non-compliant with SLSA. If we were to add a comment, I might say that we'd like to refactor this at some point to move VSA verification under verify-artifact.

just have to be careful that users don't use the non-immutable image for pulling

README.md

verifiers/utils/dsse.go

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

verifiers/internal/vsa/verifier.go

laurentsimon · 2024-07-09T15:36:27Z

verifiers/internal/vsa/verifier.go

+		return err
+	}
+	// 6. confirm the verificationResult is Passed
+	if err := confirmVerificationResult(vsa); err != nil {


nit: be consustent with function names. Other functions are called match*, this one is called confirm*

Only because, unlike the others, it doesn't compare the VSAs value with any user input.

gotcha. just a nit. up to you :)

verifiers/internal/vsa/verifier.go

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

laurentsimon

Thanks!

Hayden-IO

Great work!!

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

This reverts commit 9ad9097.

ramonpetgrave64 added 30 commits June 13, 2024 22:22

Changed success message to a more general "PASSED: SLSA verification …

7980fde

…passed" Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

skeletion verify-vsa command

b5eb147

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

testdata, sample invocation in README.md

a25abe2

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

rename to TrustedProducerID, allow muyltiple --subject-digest flags

b90ede0

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

cleanup

a3a573a

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

parse dsse envelope

9704c97

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

different test example

2f76f12

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

vsa parser

2dc64f7

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

attempt to verify envelope

1f123f3

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

cleanup, more skeleton

edde0a8

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

use utility to parse envelope, docs, use keyID

ead4e9b

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

embed the google vsa key, match against all signatures, match the sub…

13a74b5

…ject digests Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

verify reamining fields, print attestations

610ef6f

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

singular print-attestation

944c9a6

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

minify test data

2ef9a40

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

verify vsa passed message

f0fedec

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

update README

ad1b81d

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

rename to PublicKeyHashAlgo

f5362e5

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

rename to resource URI

5636d0a

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

use pointers

fec61b1

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

use plain bool

8befbc6

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

switch wanted, got order

7fb5bf9

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

change error type

fbe83fb

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

typo

00fed87

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

literl hash algo

e47312f

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

specific errors and test cases

cba639f

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

undo regression tag change

ff1cf43

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

remove accidental binary

942d8bb

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

lint: no pointer for crypto.publickkey

73c9884

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

lint

0172a12

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

ramonpetgrave64 modified the milestones: v1.0 support, SLSA VSA support Jun 26, 2024

Hayden-IO approved these changes Jun 28, 2024

View reviewed changes

README.md Outdated Show resolved Hide resolved

cli/slsa-verifier/main_regression_test.go Outdated Show resolved Hide resolved

cli/slsa-verifier/main_regression_test.go Show resolved Hide resolved

ramonpetgrave64 added 2 commits June 28, 2024 16:13

capitalization

8cf01ea

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

typo

15e9019

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

Hayden-IO approved these changes Jun 28, 2024

View reviewed changes

ramonpetgrave64 and others added 2 commits June 28, 2024 16:31

go.mod conflicts

9cd9553

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

Merge branch 'main' into ramonpetgrave64-vsa

891ffff

laurentsimon reviewed Jun 28, 2024

View reviewed changes

ramonpetgrave64 and others added 5 commits July 1, 2024 23:57

remove --public-key-hash-algo, make verified-levels an array

b145f36

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

Merge branch 'main' into ramonpetgrave64-vsa

701d13a

fix readme

59deb51

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

typo

4cf2972

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

more keyid tests

475962b

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

laurentsimon reviewed Jul 9, 2024

View reviewed changes

verifiers/internal/vsa/verifier.go Show resolved Hide resolved

laurentsimon reviewed Jul 9, 2024

View reviewed changes

verifiers/internal/vsa/verifier.go Show resolved Hide resolved

add slsa level inference

a1068c4

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

laurentsimon previously approved these changes Jul 9, 2024

View reviewed changes

Hayden-IO approved these changes Jul 9, 2024

View reviewed changes

revise link to how-to-verify

9ad9097

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>

ramonpetgrave64 dismissed laurentsimon’s stale review via 9ad9097 July 10, 2024 18:13

Revert "revise link to how-to-verify"

9f25bde

This reverts commit 9ad9097.

ramonpetgrave64 requested a review from laurentsimon July 10, 2024 21:03

laurentsimon approved these changes Jul 10, 2024

View reviewed changes

ramonpetgrave64 merged commit 208ac12 into slsa-framework:main Jul 11, 2024

ramonpetgrave64 mentioned this pull request Jul 23, 2024

feat: add vsa e2e slsa-framework/example-package#379

Merged

		accomodate subjects that are not simple-files.


		The verify-vsa command

Conversation

ramonpetgrave64 commented Jun 20, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Testing process

Uh oh!

kaixu-google commented Jun 26, 2024 via email

Uh oh!

Hayden-IO left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Hayden-IO left a comment

Choose a reason for hiding this comment

Uh oh!

ramonpetgrave64 commented Jun 28, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laurentsimon Jul 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

laurentsimon Jul 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

laurentsimon left a comment

Choose a reason for hiding this comment

Uh oh!

Hayden-IO left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ramonpetgrave64 commented Jun 20, 2024 •

edited

Loading

laurentsimon Jul 8, 2024 •

edited

Loading

laurentsimon Jul 8, 2024 •

edited

Loading