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.

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

Add attestation store option #724

Closed
laurentsimon opened this issue Nov 22, 2023 · 7 comments
Closed

Add attestation store option #724

laurentsimon opened this issue Nov 22, 2023 · 7 comments

Comments

@laurentsimon
Copy link
Contributor

Following slsa-framework/slsa-github-generator#2962, we need to expose this functionality.
/cc @saisatishkarra

@saisatishkarra
Copy link
Contributor

@laurentsimon I am looking forward to a direction to add an explicit option to the verifier (name) and code details to get started. Can you also clarify if the verifier already works with the COSIGN_REPOSITORY env variable without the explicit CLI option?

@laurentsimon
Copy link
Contributor Author

I think the slsa-verifier should work with COSIGN_REPOSITORY, since we're just calling the cosign APIs. So setting the env variable in slsa-verifier should work.

Here are the places to add an --provenance-registry:

  1. https://github.com/slsa-framework/slsa-verifier/blob/main/cli/slsa-verifier/verify.go#L78 to add a CLI option
  2. Add to add a ProvenanceRegistry field
  3. Update this function https://github.com/slsa-framework/slsa-verifier/blob/main/verifiers/internal/gha/verifier.go#L245 and set the env variable with the provenance registry, if the field is set

If we can add a test, that'd be great... maybe in https://github.com/slsa-framework/slsa-verifier/blob/main/cli/slsa-verifier/main_regression_test.go. Not sure where the best place is for the test yet :)

Let me know if this makes sense or not. Thanks again for your help!

@laurentsimon
Copy link
Contributor Author

@saisatishkarra
Copy link
Contributor

saisatishkarra commented Jan 12, 2024

@laurentsimon can you elaborate on the following:

  • Can we use the option name as --provenance-repository instead of --provenance-registry since it is a full image repository and not to be confused with only the registry which can be ghcr.io / docker.io/xxx etc?

  • Should we completely deprecate using the COSIGN_REPOSITORY env and only allow --provenance-registry ?

  • Should we stick to below the order of precedence approach for evaluating explicit new input --provenance-registry and existing env variable COSIGN_REPOSITORY works? WDUT?

    1. Make input --provenance-registry as Optional field
    2. When BOTH input field --provenance-registry and environment value COSIGN_REPOSITORY are SET
      • Ignore env COSIGN_REPOSITORY
      • final value = user input set by provenance-registry and proceed
    3. When NO explicit input --provenance-registry is set (i.e Optional field)
      • final value = any output of os.env("COSIGN_REPOSITORY") and proceed

@laurentsimon
Copy link
Contributor Author

laurentsimon commented Jan 12, 2024

Hi

Can we use the option name as --provenance-repository instead of --provenance-registry since it is a full image repository and not to be confused with only the registry which can be ghcr.io / docker.io/xxx etc?

ah, you caught me unaware here. So you're saying that COSIGN_REPOSITORY is a full image name, I did not realize that. I thought the registry changed only. I think we'll need a --provenance-repository then, you're right. I was thinking that we could re-use provenance-path (like for blobs), but that seems confusing and feels like filesystem paths.

Should we completely deprecate using the COSIGN_REPOSITORY env and only allow --provenance-registry ?

yes.

Make input --provenance-registry as Optional field

yes

Does this answer all the questions?

Thanks again for all your contributions, really appreciated!

@saisatishkarra
Copy link
Contributor

@laurentsimon Here is the PR #736 for the added option.

I have NOT deprecated the COSIGN_REPOSITORY environment and instead took the overriding approach to prioritize user input --provenance-repository when both are simultaneously set. I felt this keeps slsa-verifier generically consistent with the default UX experience of cosign user as per their docs. LMK what you think and merge if everything looks good.

Looking forward for a release / tag with this functionality.

@laurentsimon
Copy link
Contributor Author

@laurentsimon Here is the PR #736 for the added option.

I have NOT deprecated the COSIGN_REPOSITORY environment and instead took the overriding approach to prioritize user input --provenance-repository when both are simultaneously set. I felt this keeps slsa-verifier generically consistent with the default UX experience of cosign user as per their docs. LMK what you think and merge if everything looks good.

The fact that we use cosign is an implementation detail that need / should not be exposed to client. I think we should remove this option. In fact, we're in the process of moving to sigstore-go instead of cosign

--provenance-repository

That made me realize we call this option --provenance-registry in the slsa-github-generator. I think we should update the option name too? Wdut?

ramonpetgrave64 pushed a commit to ramonpetgrave64/slsa-verifier that referenced this issue Apr 10, 2024
… while image verification (slsa-framework#736)

@laurentsimon Added a new image verification cmd input
`--provenance-repository`
This replicates the feature of the `COSIGN_REPOSITORY` environment
variable when provenance is stored in a different repository/registry

Order of precedence:
- If input `--provenance-repository` is set, leverages the non-empty
input value
- If the env variable `COSIGN_REPOSITORY` is set, it is NOT consumed

README edit :
https://github.com/slsa-framework/slsa-verifier/pull/736/files#diff-b335630551682c19a781afebcf4d07bf978fb1f8ac04c6bf87428ed5106870f5R280

---------

Signed-off-by: saisatishkarra <saisatish.karra@konghq.com>
Co-authored-by: laurentsimon <64505099+laurentsimon@users.noreply.github.com>
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
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants