-
Notifications
You must be signed in to change notification settings - Fork 41
Sign images built #116
Comments
Thanks for the request! Here's a proposed approach. ArchitectureThere are three potential places where we could implement image signing in There are multiple signing approaches in discussion across the cloud native ecosystem. While we are supportive of upstreaming a signing solution into BuildKit proper, focusing on this approach at first may not yield the quickest path to get a working solution in the hands of users. We should be able to define a UX that enables us to adapt to an upstream BuildKit native signing solution when/if it becomes available. Within this project, the fastest path to provide a viable signing solution to users is to implement in the CLI first. We can set up our UX to support builder-based signing in the future, be that via a wrapper on BuildKit or BuildKit native. Signing within the CLI also makes it more straightforward to support multi-arch image signing, as that is currently implemented purely CLI-side. The downside to a CLI only signing model is it requires the system where the CLI is running to be authenticated to the applicable cloud provider for cloud-based KMS to function, where the builder running within the cloud could have been able to rely on automatic node-based authentication. If this becomes a sticking point for users, we can add builder-based signing in a follow up enhancement. Supported Signing ModelsCosign supports multiple signing models. In general, these should be largely abstracted from the implementation by the Cosign code, however there are some nuances to consider.
UXWe should aim for a forward looking UX that could support different signing approaches (not exclusive to Cosign) as well as builder based signing models, and potentially local (non-push) signing in the future.
We will assume key generation/setup is handled outside of the scope of this tool, and will include documentation guiding users how to set this up. Future The initial implementation will only support signing when push is specified. Attempting to specify Any important or time consuming steps during the signing process should be logged through the progress reporting capability in the system so it is rendered in the build progress/status, along with timing information. Similar to https://github.com/vmware-tanzu/buildkit-cli-for-kubectl/blob/main/pkg/build/build.go#L655 The cosign key string will be parsed to determine if it is a URI. If not a URI, it will be assumed to be a local file path. Special casing based on the scheme in the URI will be implemented as needed. To reduce repetitive typing, the user may set COSIGN_KEY in their environment to record the key they want to use and only specify Keyless support will follow upstream Cosign’s experimental status and require COSIGN_EXPERIMENTAL=1 be set in the CLIs environment. With this env var set, only Given the experimental status of keyless and the added complexity of the user interaction, we might consider splitting this portion out into a follow-up PR so we can get basic signing support merged quicker. DesignAt this time, Cosign does not have a “clean” API, but rather, requires vendoring the CLI logic and calling into CLI code to accomplish signing tasks. We should consider developing a thin facade to keep the mainline code clean and once upstream work is completed to create a stable API for cosign, we can easily adapt. This abstraction will also help facilitate unit test coverage - https://github.com/sigstore/cosign/blob/main/cmd/cosign/cli/sign/sign.go Single Architecture ManifestsAt or around https://github.com/vmware-tanzu/buildkit-cli-for-kubectl/blob/278c70c3ecaf9e48226205ee1110830d441c8afc/pkg/build/build.go#L759 the Solve results come back as something like:
Of particular interest, the “containerimage.digest” and “image.name” represent the manifest and tag of the image that was built and optionally pushed. If pushed, and signing is enabled, this is roughly where the CLI would wire up calls to Cosign to pull the manifest in question, sign and push the signature back to the registry based on the image.name repo name. Multi-arch Manifest ListsManifest lists are currently constructed purely in the CLI (not the builder.) The final Combined Manifest List will be signed, around https://github.com/vmware-tanzu/buildkit-cli-for-kubectl/blob/278c70c3ecaf9e48226205ee1110830d441c8afc/pkg/build/build.go#L674 TestingIn addition to unit testing the new code, we should be able to develop an end-to-end test with our local registry suite https://github.com/vmware-tanzu/buildkit-cli-for-kubectl/blob/main/integration/suites/localregistry_test.go For example, a new test case patterned after TestBuildWithPush https://github.com/vmware-tanzu/buildkit-cli-for-kubectl/blob/main/integration/suites/localregistry_test.go#L279 could generate a random/throw-away local key, and sign with that. At present, we do not verify content after it is pushed into the registry. It may be worthwhile investing in some additional utility code in this local registry suite to be able to exec into a pod inside the cluster where we can poke at the registry API via curl or equivalent to perform deeper validation of the various scenarios that push content to the local registry. |
Describe the problem/challenge you have
Having the ability to authenticate the entity that created the container image is a good security practice to prevent malicious / unexpected images to be used in the cluster.
Folks today can use tools like cosign to perform image signing. However for in-cluster built images, that may have been loaded only into the container runtime, this becomes somewhat difficult to do.
Description of the solution you'd like
If both private keys are specified, two signatures would be attached to the built image. One signature being tied to the 'entity' that triggered the
kubectl build
, and the other signature tied to the in-cluster 'builder' of the image.cosign also supports keyless signing, which aims to make signing images as "easy as possible". It is still currently an experimental feature, but it would be great (assuming this is accepted) if the design changes made took that method into account.
Vote on this request
This is an invitation to the community to vote on issues. Use the "smiley face" up to the right of this comment to vote.
The text was updated successfully, but these errors were encountered: