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

Does webhook pull images for the sidecar and/ or multiple image repos #1653

Closed
adubey8 opened this issue Mar 24, 2022 · 28 comments
Closed

Does webhook pull images for the sidecar and/ or multiple image repos #1653

adubey8 opened this issue Mar 24, 2022 · 28 comments
Labels
question Further information is requested

Comments

@adubey8
Copy link

adubey8 commented Mar 24, 2022

I have images deployed from multiple source in one cluster+namespace.
For example I have three images:

  • ***.azurecr.io/image-repo/image-name/@sha256:
  • gcr.io/image-repo/authentication@sha256:
  • gcr.io/knative-releases/knative.dev/serving/cmd/queue@

As you may notice I have control over first two images so I can put them in one repo and deploy. But I have no control over the third image as it is knative image. Will webhook allow all three images to be deployed? If yes, how? What config changes need to be made?

@adubey8 adubey8 added the question Further information is requested label Mar 24, 2022
@vaikas
Copy link
Contributor

vaikas commented Mar 24, 2022

Great question! We're currently working on adding support to ClusterImagePolicy, which will allow multiple ways to validate the containers. So, to answer your question in couple of different ways (note the code is currently in flux, but hopefully these pointers give enough of an idea):

  • Webhook will inspect all the containers in a given podspec.
  • If the images are signed with different keys, then this will get a bit trickier, since today you can't specify directly that an image that matches a, say your gcr.io/image-repo/* uses different key.

So, here's the current design document on what we're working on implementing and are hoping to have this buttoned up in the next week or so:
https://docs.google.com/document/d/1gBLEOOHWOmvHVsoJbgGU74GdwA6CGxMRp3MAeEB50l4/edit#

Once that lands, you can create policies based on the image (glob/regex) and have finer level of control on how to validate images.

I thought there was one issue tracking what to do about debug containers, but I can't find it right now.

Hope this helps :)

@adubey8
Copy link
Author

adubey8 commented Mar 24, 2022

In my use case, some container are injected as sidecar by other webhook like Istio operator and we have no control where image comes from and we can't sign them either. So in your new policy can I specify some image can skip the validation. Or give warning instead of failure.

@hectorj2f
Copy link
Contributor

hectorj2f commented Mar 24, 2022

@adubey8 By default, the admission controller only validates the resources deployed on namespaces with the label cosigned.sigstore.dev/include, as you can see in the WebhookConfiguration https://github.com/sigstore/cosign/blob/main/config/500-webhook-configuration.yaml#L24

@adubey8
Copy link
Author

adubey8 commented Mar 31, 2022

As we mentioned this is not on the namespace level. We want to verify the image signature but unfortunately other application will inject sidecar pod. And we have no control on that part and injected sidecar pod signature. So we want to skip validation on the sidecar pod.
As @vaikas mentioned so there is some progress on the defined verification policy. Do you think that will resolve my problem. In the policy I can skip the validation of some image.

@vaikas
Copy link
Contributor

vaikas commented Apr 6, 2022

Sorry about the tardy reply, went to reply and totes spaced.
So, at the current moment this is not possible. You could create CIPs for the ones that are actually signed, but the problem currently is that iff there's no matching CIP (as would be the case for your image), it would take the 'default' or the old behaviour path and that would fail since there's no signature on it.

I think we're going to have to start implementing some sort of ability to specify policy at the ClusterImagePolicy level that would allow to say for example how many verified signatures an image needs (and in your case you would then be able to say 0). @hectorj2f @adubey8 Does that make sense?

I'd rather look at adding some policy behaviour rather than just say something like, do not expect any. Thoughts?

@hectorj2f
Copy link
Contributor

@vaikas Yes, it makes sense, we definitely need to be able to declare some policy behaviour. Another option could be to only validate the images for which a policy matches, and warn when there are not policies matching the image name defined in the CIP.

@vaikas
Copy link
Contributor

vaikas commented Apr 6, 2022

I think we should start the policy discussion, and see how it may look sooner rather than later :)

@hectorj2f
Copy link
Contributor

Yes, I 100% agree.

@hectorj2f
Copy link
Contributor

@adubey8 We've added support for policy behauvior definition in cosigned. However we require, at least, images within a namespace to match at least a signature of the defined. To ignore specific pods within a namespace, we might need to add a label or annotation to ignore those pods during the validation. It isn't an ideal solution, but I cannot think of better options without polluted the rest of the logic.

@vaikas
Copy link
Contributor

vaikas commented May 4, 2022

I think the label / annotation is not ideal, since a bad actor could then just slap that label on their pod and bypass the policy checks. Naively I think we might want to have to define an explicit CIP that would match that image (or a class of images) and in that policy you could specify somehow (flag, field, etc.) that if an image matches, it's not checked.

@DennyHoang
Copy link
Contributor

DennyHoang commented May 4, 2022

tl;dr: I believe you can sign public images yourself, store it in a remote signature repository, and then just have cosigned validate public images using your remote signature repository.

Hmm, maybe I misremembered how signatures are pushed using COSIGN_REPOSITORY, but a quick test showed that I can sign a public image using a remote signature location using:

COSIGN_REPOSITORY=dhoang679/signatures-private cosign sign --key tmp/cosign-repository.key gcr.io/knative-releases/knative.dev/serving/cmd/queue

It would only push a signature to a remote signature location and not to the source image location.

Therefore, should be possible for users to take a public sidecar image, such as gcr.io/knative-releases/knative.dev/serving/cmd/queue, sign it and have the signature stored in a repository that they do have access to.

I was able to verify my signing of a public image by storing it in a separate signature repository like so:

COSIGN_REPOSITORY=dhoang679/signatures-private cosign verify --key tmp/cosign-repository.pub gcr.io/knative-releases/knative.dev/serving/cmd/queue

Verification for gcr.io/knative-releases/knative.dev/serving/cmd/queue:latest --
The following checks were performed on each of these signatures:
  - The cosign claims were validated
  - The signatures were verified against the specified public key

[{"critical":{"identity":{"docker-reference":"gcr.io/knative-releases/knative.dev/serving/cmd/queue"},"image":{"docker-manifest-digest":"sha256:a40f6e84de1a0d145d27084a94cc7fa221159e75cafde7d332ac8f4f0aed58fb"},"type":"cosign container image signature"},"optional":null}]

Then the ClusterImagePolicy (CIP) authority can be configured to match that image, specify the remote source and then it would still validate.

  1. Sign public image but store signature in a separate signature repository
  2. Create cluster image policy that matches that public image
  3. Have an authority with that corresponding public key used to sign that image into the separate signature repository
  4. Have that authority's source be configured to the separate signature repository location by configuring authorities[].source.oci
  5. Add the authorities[].source.signaturePullSecrets credential required to access that separate signature repository (This isnt available in the latest cosign release v1.8.0, but it is present in the main branch and should be released soon. Until it becomes available, the remote signature location just has to have public read access to obtain the signatures)

I understand this is a different request compared to just ignoring the image/pod, but this is a potential workaround. There is more overhead on your end since you would have to always sign the public sidecar images whenever it changes. However, it is a more secure approach as you also then validate that the correct expected public sidecar image is used.

@vaikas
Copy link
Contributor

vaikas commented May 4, 2022

Yep, that's mosdef now doable because we now have all the pieces for it, woot woot!
This is definitely a workaround that should work!

@hectorj2f
Copy link
Contributor

Thanks @DennyHoang, it sounds like a workaround.

I think the label / annotation is not ideal, since a bad actor could then just slap that label on their pod and bypass the policy checks.

@vaikas I don't disagree in terms of security. But the same theory could be applied against bad actors removing the label from the namespace cosigned.sigstore.dev/include: "false", so pods are not verified for that namespace.

@vaikas
Copy link
Contributor

vaikas commented May 4, 2022

Yup, that's facts. Guess that part depends on how well the namespaces are locked down vs. deploying pods. So much to document lol :)

@coyote240
Copy link

I'm hesitant to consider this a workaround, but rather something to be formalized into a feature. @DennyHoang 's instructions above does require a number of steps (5, to be exact ;-) ). Would it be worth focusing here to tighten up the feature?

I'd also be interested in @adubey8 's thoughts.

I've never been way stoked on an "ignore" feature, and the compromise of sidecar pods like envoy could have devastating impact - they're mos def worth protecting.

@coyote240
Copy link

Another thought, that could avoid user PKI issues, should we add a feature or workflow to make it easier to sign public images against the transparent log?

@coyote240
Copy link

I think the label / annotation is not ideal, since a bad actor could then just slap that label on their pod and bypass the policy checks.

@vaikas I don't disagree in terms of security. But the same theory could be applied against bad actors removing the label from the namespace cosigned.sigstore.dev/include: "false", so pods are not verified for that namespace.

It's not unusual for certain Unix tools, like GPG or SSH to require specific permissions on their config files. I wonder if there is an analog here, where the admission controller could reflect on namespace RBAC and fail to admit with an Event if the namespace RBAC is too permissive?

@vaikas
Copy link
Contributor

vaikas commented May 11, 2022

Just thinking little more, the label issue is problematic since 2/3 containers in the pod do have signatures and it's only the last sidecar that does not have it, so either the label would have to say something like this particular container does not get blocked.
@coyote240 totes agree that ignore is not ideal. In the fulness of time, everything should be signed (and attested, etc.), but I think realistically there will be a period of time while folks are ramping up their systems and there will be some of these mixed cases. Reason why I personally lean towards the policy that says ignore, is that it's explicit and one has to very specifically (could include the entire hash to limit down very tightly what's allowed to go through).
And yes, it's a workaround, and yes agree that it's not ideal but I think that's why it's called a workaround :) You can work around (maybe a looong way around, but around regardless) :)

@adamd-vmw
Copy link

Some additional perspective from the customer and product side:

  • higher compliance customers have acknowledged that they need to import public images to a private registry and re-sign them internally, and are doing so
  • alternatively, users can pressure their vendors to sign their images
  • the remote signature repository is a potential, if cumbersome, workaround
  • we are starting to discuss an Authority that is essentially an ignore/auto trust but that's only one possible solution, will let @coyote240 work that through the community.

Given that there are a couple of solutions here, is it possible to say that we don't need to resolve this in order to bump up to v1beta1 ?

@vaikas
Copy link
Contributor

vaikas commented May 13, 2022

Using the authority that could be using the 'Warn' mode with the webhook is another possible way to handle this? So, instead of a block, it's a warn.

@hectorj2f
Copy link
Contributor

hectorj2f commented May 15, 2022

is it possible to say that we don't need to resolve this in order to bump up to v1beta1 ?

+1

@Zarmbinski
Copy link

Just a thought, but would it be possible to add something similar to the static validator feature that is offered by Connaisseur ?
(docs for reference: https://sse-secure-systems.github.io/connaisseur/v2.6.0/basics/#static-validators)

It would potentially look like this:

spec:
  images:
  - regex: istio/*
  authorities:
  - name: "Allow all Istio Service Mesh Images"
    static:
      approve: true

@MageshSrinivasulu
Copy link

MageshSrinivasulu commented Jun 21, 2022

@vaikas @hectorj2f I guess this document needs some update since the name had been changed from cosigned to policy. https://docs.google.com/document/d/1gBLEOOHWOmvHVsoJbgGU74GdwA6CGxMRp3MAeEB50l4/edit#

@MageshSrinivasulu
Copy link

@hectorj2f I read through the entire chain. It seems a very good feature to have. Looking forward to the release, since we need a solution to exclude or just warn for the images that are from public repo

@MageshSrinivasulu
Copy link

@hectorj2f Can you please let me know when this feature will be available?

@vaikas
Copy link
Contributor

vaikas commented Jul 5, 2022

Sorry for the tardy reply, was OOO. So, I like the approve: [true|false] format over the initial proposal for 'just pass' because then one could for example block a particular version, or set of registries, etc. One could also create a 'catch-all' CIP if one wanted to subvert the legacy behaviour by being able to create an allow list that would permit images that would otherwise be blocked, and then because CIPs are ANDed together create more restrictive policies for more specific images.
I do wonder if we might want to still consider being able to warn instead of block using this approach.
Instead of true/false, be basically able to say 'allow-but-warn'.

Wonder if that may also satisfy the requirements for:
sigstore/helm-charts#227

Depending on what is meant by logging.

@MageshSrinivasulu
Copy link

MageshSrinivasulu commented Aug 17, 2022

@vaikas @hectorj2f Let me explain a scenario why we need " warn " or " allow-but-warn "

Let's consider two images

  1. azurecr.io/image1 ==> This is signed
  2. azurecr.io/image2 ==> This is not signed

CASE 1: I cannot use ClusterImagePolicy for image2 if I have a wildcard like this azurecr.io/** . Because it will stop the image2 from getting deployed

apiVersion: policy.sigstore.dev/v1beta1
kind: ClusterImagePolicy
metadata:
  name: image-policy
spec:
  images:
  - glob: "azurecr.io/**"
  authorities: 
    - key:
        secretRef:
          name: "cosignpub"

CASE 2: Also, I cannot use the static check in this case since it will match the wildcard and pass all the images or fail all the images. My understanding is static overrides the key.

apiVersion: policy.sigstore.dev/v1beta1
kind: ClusterImagePolicy
metadata:
  name: image-policy
spec:
  images:
  - glob: "azurecr.io/**"
  authorities: 
    - static:
        action: pass

CASE 3: I want something that checks and allows the ones that are signed and warn if it's not signed but still allows it.

apiVersion: policy.sigstore.dev/v1beta1
kind: ClusterImagePolicy
metadata:
  name: image-policy
spec:
  images:
  - glob: "azurecr.io/**"
  authorities: 
    - key:
        secretRef:
          name: "cosignpub"
    - action: allow-but-warn  ======> 
         !!! Static will override the key in this scenario saying pass or fail as default. 
         !!! We must have the capability to allow-but-warn to be configured somewhere.

Why I am asking is enterprise organizations will have thousands of images. It will be super hard to co-ordinate and adapt to this image signing. We should have capabilities that allow if the image is signed and allow-but-warn if the image is not signed. So we can have a common rule that can handle the thing initially. Over the period of time app team will start seeing this warning and have their image signed

@vaikas
Copy link
Contributor

vaikas commented Aug 17, 2022

Yes for sure :) We agree lol
sigstore/policy-controller#98

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants