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

Jump to bottom

feat: Verification of VSA #542

Closed
laurentsimon opened this issue Mar 23, 2023 · 16 comments · Fixed by #777
Closed

feat: Verification of VSA #542

laurentsimon opened this issue Mar 23, 2023 · 16 comments · Fixed by #777
Assignees
@laurentsimon
Labels
type:feature New feature request
Milestone
SLSA VSA support

Comments

@laurentsimon
Copy link
Contributor

laurentsimon commented Mar 23, 2023 •
edited
Loading

See VSA https://slsa.dev/verification_summary/v0.2

High-level verification in CLI:

$ slsa-verifier verify-vsa <artifact> --vsa-path <vsa-file> verifier-id google.com [--resource-uri <purl>] --policy-level X
@laurentsimon laurentsimon added this to the VSA milestone Mar 23, 2023
@laurentsimon
Copy link
Contributor Author

laurentsimon commented Mar 23, 2023 •
edited
Loading

Note: we may need to differentiate between containers and non-containers, maybe via --artifact-type image... or artifact-storage [file:registry] ... or verify-vsa [file | registry]:path?

@asraa
Copy link
Contributor

asraa commented Mar 24, 2023

What if it were verify-artifact --vsa?

@laurentsimon
Copy link
Contributor Author

laurentsimon commented Mar 24, 2023 •
edited
Loading

May work. You're right the my proposal deviates from the current convention that our commands follow verify-<artifact-type>, not verify-<attestation-type>.

One limitation is that it mangles verify different verification routines into one command line option, so we'd lose namespaces? Eg: builder-id becomes verifier-id, etc... but maybe that's still better?:

... verify-artifact --vsa-path ...

Maybe verify-artifact-vsa? Does not seem ideal either.

/cc @ianlewis @mihaimaruseac

@asraa
Copy link
Contributor

asraa commented Mar 24, 2023

Ah true - yes, I only say this because I worry that for each command we'll have a (artifact/image) pathway like cosign.

Hmmm

Yeah, VSAs definitely seem like a different pathway altogether.

I've usually liked detecting the resource type depending on file://blah or gcr.io/blah URI, but I'm not sure it's good to be implicit about types..

@laurentsimon
Copy link
Contributor Author

I'm also worried about implicit detection.

@laurentsimon
Copy link
Contributor Author

Another option could be to create another sub-namespace:

$ slsa-verifier verify-artifact --with-provenance \ # that's a new namespace
                                             --provenance-path  <provenance-specific-args>
$ slsa-verifier verify-artifact --with-vsa \ # that's a new namespace
                                            --vsa-path  <vsa-specific-args>

This would allow us to keep the existing namespaces.

@ianlewis
Copy link
Member

I think it's ok to have flags that are mutually exclusive. Though, maybe --with-vsa and --with-provenance are redundant. We know they want one or the other based on whether --vsa-path or --provenance-path is given.

@laurentsimon
Copy link
Contributor Author

laurentsimon commented Mar 30, 2023 •
edited
Loading

there are also corner cases, e.g. for containers where provenance and vsa may be stored on the registry. For Sigstore, users need not provide it to us, we infer this based on builderID.

@ianlewis
Copy link
Member

Right, those options can vary by the type of artifact (e.g. the command, verify-artifact vs. verify-image) though right?

@laurentsimon
Copy link
Contributor Author

I think so.

@ianlewis ianlewis added the type:feature New feature request label Apr 20, 2023
@laurentsimon
Copy link
Contributor Author

laurentsimon commented May 19, 2023 •
edited
Loading

I may have found a better way: spf13/cobra#1327
This lets you group flags. Essentially, it keeps the commands as-is, but we can group commands in the --help command so that its clear to users that vsa commands != provenance commands, etc:

Provenance flags:
  -provenance-path
  -source-uri ...

VSA flags:
  -vsa-path ...
  -resource-uri ...

The link above has a bunch of code example to implement it.

@mihaimaruseac
Copy link
Contributor

Grouping is nice, but is only for documentation. From my experiments, if you want to enforce only flags from a single group, you need to write code to validate that.

@laurentsimon
Copy link
Contributor Author

+1, I'm planning to have a manual validation to ensure no flags overlap with different groups. Besides this downside, that seems like a good direction?

@mihaimaruseac
Copy link
Contributor

Yes, I like it!

Was looking over that during the last end of year break when updating the verifier flags but didn't get a chance to do something. I think doing it as part of the VSA verification work is great

@laurentsimon laurentsimon self-assigned this May 23, 2023
@laurentsimon
Copy link
Contributor Author

So the above was for verification of artifacts with a VSA.

To support custom digests in-toto/attestation#338, I think we need a dedicated API verify-vsa which will only verify the VSA claims from the attestation (but does nothing to verify the image itself). If we were to re-use the verify-artifact -vsa for custom digests, this would expose an unsafe / misleading API because verification does not do anything about the artifact / container, ie we're just matching fields in the VSA against user-provided values. Keeping the artifact argument would make users believe we're doing some verification with it, yet we are not.

Eventually, I think all this can be backed by a set of APIs #756:

  • verify-artifact computes expected claims (digest, etc) and calls
  • verify-vsa (which matches attestation claims against caller-provided claims) which calls
  • inspect-vsa (verifies signature and outputs verifiied metadata like subject, verifier id, levels, etc)

For the CLI, we don't need this level of re-factoring, zo.

@haydentherapper

@laurentsimon
Copy link
Contributor Author

laurentsimon commented May 2, 2024 •
edited
Loading

First step towards supporting VSA verification is to add e2e tests in https://github.com/laurentsimon/slsa-verifier/tree/feat/vsa and merge.
Second step will be to support to add the verify-vsa command.

@ramonpetgrave64 ramonpetgrave64 mentioned this issue Jun 20, 2024
Merged
ramonpetgrave64 added a commit that referenced this issue Jul 11, 2024
@ramonpetgrave64
feat: vsa support (#777)
208ac12 
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](https://github.com/in-toto/attestation/blob/36c11295429a997d5bb520b4e80a1d0c16845f9c/go/predicates/vsa/v1/vsa.pb.go#L26-L40)
for vsa 1.0

---------

Signed-off-by: Ramon Petgrave <ramon.petgrave64@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@laurentsimon laurentsimon

Labels
type:feature New feature request
Projects
None yet
Milestone
SLSA VSA support
Development

Successfully merging a pull request may close this issue.

feat: vsa support ramonpetgrave64/slsa-verifier
4 participants
@ianlewis @mihaimaruseac @asraa @laurentsimon