image-automation-controller icon indicating copy to clipboard operation
image-automation-controller copied to clipboard

[FEATURE]: Perform Image Verification Before Performing ImageAutomatedUpdates in Git

Open ChrisJBurns opened this issue 2 years ago • 7 comments

Problem

Currently, the way Flux works with regards to ImageAutomatedUpdates is that, when it detects a new image with a tag that matches the one set in the ImagePolicy, it will commit that new tag to Git, and then the other components of Flux will detect that a change has been made in Git that doesn't match the state of the cluster, and it reconciles it by deploying the new image. Whilst this follows the principles of GitOps technically, there are slight problems/inconveniences when bringing signed images into the equation. These problems include the fact that the way Flux works currently, it commits new image tags to Git before any image verification is performed.

Scenario

The following is my current situation, but I can imagine flavours of this will arrive at the same end.

Say you have a test-project that is a basic Java/Go service (it doesn't matter in this context). You have a Tekton pipeline building and testing the code, and then a Kaniko Tekton Task that is part of that pipeline that builds and pushes the project image to Harbor (or any other image registry). Once the task has completed, Tekton Chains kicks in and signs the image using Cosign (and does some attestation for good measure, but that is irrelevant for this scenario at the moment), it then pushes this image back into Harbor with the signature and you get a nice little Green tick in Harbor illustrating that the image has been signed.

image

Now, Flux at this point has scanned the repository and has detected that there is a new image and it makes the update to Git, and then Flux will reconcile this change and apply it to the cluster - and it attempts to deploy it.

Now here is where you may have an admission controller like Kyverno (in my case) that has policies that perform imageVerifications on the images. If in the case an image has been blocked due to the signature not being verified, it will fail to deploy. Now Kyverno in this case is doing it's job with regards to when a deployment is pending, it will perform the necessary checks and it will block them if it doesn't meet the verifications - no bug there. Flux is also doing it's job as it is trying to reconcile itself with what is in Git. However, technically in the cases where the image isn't signed, do we really want to allow that Git change to be made?

Myself and a few others have discussed this on the Flux Slack and have also concluded whilst nothing technically is broken and both tools (Flux and Kyverno - in my case) are doing what they are programmed to do, there is a slight inconvenience in this. To move more-so in-line with the way the current industry is going around secure supply chain, would it not make sense to possibly add functionality to Flux so that it either performs the image verifications before it commits to Git (Flux maybe able to do this imageVerification itself?, or it integrates with Kyverno in some way (or other admission controllers), to take advantage of it's imageVerification functionality). This avoids the commit to Git, only to be met with a blocking action on the admission controllers side, that doesn't allow the image to be deployed.

I'm hoping I've described the problem domain enough.

ChrisJBurns avatar Apr 21 '22 20:04 ChrisJBurns

@ChrisJBurns thank you very much for taking the time and thoroughly explaining the problem statement and feature suggestion.

I think we a few options here:

a) Implement Signature Verification in Image Automation Controller.

Pretty much what it says in the tin, and exactly as you explained it. This could be cumbersome in cases where users already have (and need) that verification/enforcement in other parts of their system (i.e. at Admission Validation point with Kyverno).

b) Implement a controller-level flag to observe signed tags only.

The controller could have a switch (e.g. --signed-tags-only) that would force it to only observe signed tags. This would be enabled at controller-level and would affect all ImageRepository objects. A potential challenge here would be for users that have a mix of repositories in which not all are currently signing images.

c) Change the ImageRepository specification to observe signed tags only at repository-level.

Effectively adding a new boolean field (e.g. spec.SignedTagsOnly) to the ImageRepository CRD, allows for users to enforce signed tags at a repository-level. This could be combined with Admission Validation Webhooks by a Platform Admin, to enforce the setting across all ImageRepository objects.

Given that the controller does not admit or use the images, I am inclined to think that the signature verification itself should live elsewhere - i.e. Kyverno/Container Registry. What are your thoughts on the above? Do you see options B/C working for your use case?

pjbgf avatar Apr 22 '22 07:04 pjbgf

I'm not sure of the value of verifying the images before a Git push. What if the image tag is Ok at push time, but later on, that tag is overwritten by a bad actor? I feel like this feature would do more harm than good, users will consider that their images are safe since Flux has verified them, but it's just an illusion as Flux does not scan images at runtime, kubelet could pull a totally different digest.

stefanprodan avatar Apr 25 '22 10:04 stefanprodan

@stefanprodan Those are valid concerns indeed. What if we don't verify them as much as Kyverno does for example with regard to checking it's cosign signature - but instead just check if the images are signed at all. It can be noted in the documentation somewhere that it doesn't verify that the signature is valid, but instead just verifies if there is a signature present. This will at least allow Flux to weed out unsigned images - which is still an improvement in security in some sense. Additionally, Kyverno or any police engine can then come along and actually do the image verification against the cosign keys - which they do anyway.

@pjbgf Welcome your comments also based on @stefanprodan's concerns above?

ChrisJBurns avatar Apr 25 '22 12:04 ChrisJBurns

@ChrisJBurns I believe consumers of a given artefact are responsible for ensuring their validness and to perform anti-tampering checks. I don't see why IAC would own signature verification, that sits better with an admission controller webhook instead, which aligns with what Stefan mentioned around TOCTOU issues.

However, IAC could have a feature/option in which it ignores unsigned tags (i.e. options B/C).

pjbgf avatar Apr 26 '22 12:04 pjbgf

Going going through some of the issues on my list, @pjbgf @stefanprodan following on from the above, is it something that we can do regarding adding the spec.SignedTagsOnly option to the IAC to only observe signed images? As the other two options don't seem as solid with regards to security.

ChrisJBurns avatar Nov 17 '22 22:11 ChrisJBurns

The verifying of signed artifacts before committing tags is an important and essential feature for me.

BobyMCbobs avatar Mar 22 '24 11:03 BobyMCbobs

No image signature is not the only case when an image may not be available for download. The image may not be available for download also due to the fact that vulnerabilities were found in it. If there is a 'Prevent vulnerable images from running', we can see the new tag without problems, but when we try to download the image, we will receive an error similar to this:

Error response from daemon: unknown: current image with 23 vulnerabilities cannot be pulled due to configured policy in 'Prevent images with vulnerability severity of "Low" or higher from running.' To continue with pull, please contact your project administrator to exempt matched vulnerabilities through configuring the CVE allowlist.

Therefore, if it is planned to check the availability of the image, then perhaps it would be correct to check all the reasons (it is possible to check simply by starting to download the image, perhaps there are other ways, I did not study it)

allexf avatar Apr 18 '24 16:04 allexf