-
Notifications
You must be signed in to change notification settings - Fork 561
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
WIP: enable signing with cosign #999
base: master
Are you sure you want to change the base?
Conversation
Signed-off-by: Jason Hall <[email protected]>
@imjasonh I'm puzzled by a few perspectives of this PR and maybe you could help me understand:
Due to the first point (sign once for each digest), I actually do not feel the current PR will be extremely useful, because the same thing can be achieved with this line and it doesn't seem very complicated: cosign sign --recursive --yes "NAME@${{ steps.BUILD_STEP_NAME.outputs.digest }}" However, I'm certainly not against it if it will be actively maintained from now on. 🙂 Note: |
If this feature is to be added, as a user, I want the flag name to be |
Thanks for your feedback! This is exactly the kind of response I was hoping to get from this draft PR.
Excellent point! I'll definitely change this to sign the digest instead.
Another good call-out. I'll add this to the PR's TODOs in case there's interest in adding this at all.
Good point. We should definitely annotate the signature with both
That's my main interest in doing this, to have it an optional part of the "official" workflow for building and pushing, instead of making everyone do it themselves. Anybody who wants to can also |
I don't think it makes sense for different signing solutions to be plugged in at the GitHub Action level. I would have thought that in the first instance, we should see if we can make signing pluggable as part of build-kit, then configuring the desired signing solution in this Action would be straight-forward. |
That's a reasonable point. If that's how this ends up being possible I'd be more than happy to contribute a cosign plugin at the buildkit layer instead (or someone else from the @sigstore community can, I'm sure) It'd be great to hear more about the plan for general signing solutions, so we can decide whether it goes in this action or elsewhere. |
Opening this as draft to gather early feedback. If this is an acceptable change in general I'd love to discuss improvements we could make to it. Everything about this change is open to discussion, and I'm happy to make any change necessary.
This change adds a
sign: true
(name TBD) to the action, to let folks sign images that were just built-and-pushed withcosign
, a popular container image signing solution, part of Sigstore.When
sign: true
(andpush: true
, which requirestags:
to be specified), after building and pushing any images, thecosign
CLI will be invoked to sign the images. It's the callers responsibility to installcosign
prior to building, like it's their responsibility to setup buildx. Images are signed by digest, to avoid race conditions.If an ID token representing the GitHub Actions identity is not available, signing will fail. This can happen for a few reasons. Typically this means the workflow doesn't have the
id-token: write
permission.While building and testing this workflow I've built and signed a few images, which appear in Sigstore's Rekor transparency log: https://search.sigstore.dev/?logIndex=48727071
It's not my intention with this change to support signing with keys -- I'm only interested in supporting keyless signing using the GitHub Actions identity via OIDC.
Lots more info about GitHub Actions OIDC support here: https://docs.github.com/en/actions/deployment/security-hardening-your-deployments/about-security-hardening-with-openid-connect
TODOs:
toolkit
in docker/actions-toolkit (if you prefer)--recursive
cosign: true