feat: vsa support #777

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 <[email protected]>

Signed-off-by: Ramon Petgrave <[email protected]>

…ject digests Signed-off-by: Ramon Petgrave <[email protected]>

Signed-off-by: Ramon Petgrave <[email protected]>

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: ***@***.***>

haydentherapper

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 <[email protected]>

haydentherapper

🥳

Signed-off-by: Ramon Petgrave <[email protected]>

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 <[email protected]>

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 <[email protected]>

laurentsimon

Thanks!

haydentherapper

Great work!!

Signed-off-by: Ramon Petgrave <[email protected]>

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 <[email protected]>

skeletion verify-vsa command

b5eb147

Signed-off-by: Ramon Petgrave <[email protected]>

testdata, sample invocation in README.md

a25abe2

Signed-off-by: Ramon Petgrave <[email protected]>

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

b90ede0

Signed-off-by: Ramon Petgrave <[email protected]>

cleanup

a3a573a

Signed-off-by: Ramon Petgrave <[email protected]>

parse dsse envelope

9704c97

Signed-off-by: Ramon Petgrave <[email protected]>

different test example

2f76f12

Signed-off-by: Ramon Petgrave <[email protected]>

vsa parser

2dc64f7

Signed-off-by: Ramon Petgrave <[email protected]>

attempt to verify envelope

1f123f3

Signed-off-by: Ramon Petgrave <[email protected]>

cleanup, more skeleton

edde0a8

Signed-off-by: Ramon Petgrave <[email protected]>

use utility to parse envelope, docs, use keyID

ead4e9b

Signed-off-by: Ramon Petgrave <[email protected]>

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

13a74b5

…ject digests Signed-off-by: Ramon Petgrave <[email protected]>

verify reamining fields, print attestations

610ef6f

Signed-off-by: Ramon Petgrave <[email protected]>

singular print-attestation

944c9a6

Signed-off-by: Ramon Petgrave <[email protected]>

minify test data

2ef9a40

Signed-off-by: Ramon Petgrave <[email protected]>

verify vsa passed message

f0fedec

Signed-off-by: Ramon Petgrave <[email protected]>

update README

ad1b81d

Signed-off-by: Ramon Petgrave <[email protected]>

rename to PublicKeyHashAlgo

f5362e5

Signed-off-by: Ramon Petgrave <[email protected]>

rename to resource URI

5636d0a

Signed-off-by: Ramon Petgrave <[email protected]>

use pointers

fec61b1

Signed-off-by: Ramon Petgrave <[email protected]>

use plain bool

8befbc6

Signed-off-by: Ramon Petgrave <[email protected]>

switch wanted, got order

7fb5bf9

Signed-off-by: Ramon Petgrave <[email protected]>

change error type

fbe83fb

Signed-off-by: Ramon Petgrave <[email protected]>

typo

00fed87

Signed-off-by: Ramon Petgrave <[email protected]>

literl hash algo

e47312f

Signed-off-by: Ramon Petgrave <[email protected]>

specific errors and test cases

cba639f

Signed-off-by: Ramon Petgrave <[email protected]>

undo regression tag change

ff1cf43

Signed-off-by: Ramon Petgrave <[email protected]>

remove accidental binary

942d8bb

Signed-off-by: Ramon Petgrave <[email protected]>

lint: no pointer for crypto.publickkey

73c9884

Signed-off-by: Ramon Petgrave <[email protected]>

lint

0172a12

Signed-off-by: Ramon Petgrave <[email protected]>

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

haydentherapper 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 <[email protected]>

typo

15e9019

Signed-off-by: Ramon Petgrave <[email protected]>

haydentherapper 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 <[email protected]>

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 <[email protected]>

Merge branch 'main' into ramonpetgrave64-vsa

701d13a

fix readme

59deb51

Signed-off-by: Ramon Petgrave <[email protected]>

typo

4cf2972

Signed-off-by: Ramon Petgrave <[email protected]>

more keyid tests

475962b

Signed-off-by: Ramon Petgrave <[email protected]>

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 <[email protected]>

laurentsimon previously approved these changes Jul 9, 2024

View reviewed changes

haydentherapper approved these changes Jul 9, 2024

View reviewed changes

revise link to how-to-verify

9ad9097

Signed-off-by: Ramon Petgrave <[email protected]>

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
15 checks passed

ramonpetgrave64 mentioned this pull request Jul 23, 2024

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

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: vsa support #777

feat: vsa support #777

ramonpetgrave64 commented Jun 20, 2024 •

edited

Loading

kaixu-google commented Jun 26, 2024 via email

haydentherapper left a comment

haydentherapper left a comment

ramonpetgrave64 commented Jun 28, 2024

laurentsimon Jun 28, 2024

haydentherapper Jun 28, 2024

ramonpetgrave64 Jul 1, 2024

laurentsimon Jul 8, 2024 •

edited

Loading

laurentsimon Jul 8, 2024 •

edited

Loading

haydentherapper Jul 9, 2024

ramonpetgrave64 Jul 9, 2024

laurentsimon Jul 9, 2024

laurentsimon Jul 9, 2024

laurentsimon Jul 9, 2024

ramonpetgrave64 Jul 9, 2024

laurentsimon Jul 9, 2024

laurentsimon left a comment

haydentherapper left a comment

		accomodate subjects that are not simple-files.


		The verify-vsa command

feat: vsa support #777

feat: vsa support #777

Conversation

ramonpetgrave64 commented Jun 20, 2024 • edited Loading

Testing process

kaixu-google commented Jun 26, 2024 via email

haydentherapper left a comment

Choose a reason for hiding this comment

haydentherapper left a comment

Choose a reason for hiding this comment

ramonpetgrave64 commented Jun 28, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laurentsimon Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

laurentsimon Jul 8, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

laurentsimon left a comment

Choose a reason for hiding this comment

haydentherapper left a comment

Choose a reason for hiding this comment

ramonpetgrave64 commented Jun 20, 2024 •

edited

Loading

laurentsimon Jul 8, 2024 •

edited

Loading

laurentsimon Jul 8, 2024 •

edited

Loading