cosign icon indicating copy to clipboard operation
cosign copied to clipboard

proposal: verify-manifest: Traversing Kubernetes Resources and Verify the Given Images

Open developer-guy opened this issue 3 years ago • 20 comments

Abstract

Slightly the same with verify-dockerfile, but for Kubernetes Resources, responsible for creating containers such as Deployments, ReplicaSets, DaemonSets. etc. The same logic we used to check ImageRefs in verify-dockerfile command can simply traverse the whole resource to check given images actually signed. There is an alternative way of doing the same with creating ValidatingAdmissionWebhook like a project cosigned. But with this command, we can verify the whole images as early as possible in the CI pipelines before applying them into the Kubernetes cluster.

Implementation

We may have to write a Resource decider to correctly decide where we should get the image ref from the manifest by looking at the kind of the Resource. For example like the following examples:

Kind: Deployment | |-> spec.template.spec.containers[*].image

Kind: Pod | |-> spec.containers[*].image

After founding the images, we should verify them one by one against the public key or keyless.

Usage

Maybe we can give this command the following name: verify-manifest.

$ cosign verify-manifest -key cosign.pub <path/to/manifest>

To propose as simple as possible, I don't want to add all the other options that we can suğpport, but we can support all the other commands that verify-dockerfile currently supports.

cc: @Dentrax

developer-guy avatar Jul 15 '21 16:07 developer-guy

KIND IMAGE PATH
Deployment spec.template.spec.containers[*].image
DaemonSet spec.template.spec.containers[*].image
ReplicaSet spec.template.spec.containers[*].image
StatefulSet spec.template.spec.containers[*].image
CronJob spec.spec.template.spec.containers[*].image

Dentrax avatar Jul 16 '21 07:07 Dentrax

This is awesome! I've also heard another use case that might be good to capture here: "inlining" the signatures for each image nto the k8s object itself as an annotation. That would make verification by a policy system easier.

dlorenc avatar Jul 16 '21 11:07 dlorenc

oh nice, yes we would definitely consider that once we start to implement this, what do you suggest we use as an annotation key, is there any standard key for it ?

developer-guy avatar Jul 16 '21 12:07 developer-guy

No, not that I know of yet!

dlorenc avatar Jul 16 '21 13:07 dlorenc

Does anyone want to take a stab at the "inlining" aspect?

dlorenc avatar Jul 22 '21 19:07 dlorenc

Here's a possible UX:

cosign verify-manifest (similar to blob and image syntax) cosign inline-attestation -f manifest.yaml -key key [-attestationType=something] [-annotation=something]

dlorenc avatar Jul 22 '21 21:07 dlorenc

I made a few comments over on #38 and was made aware of this thread. I think this is a good idea and could possibly dovetail nicely into that issue as a downstream side-effect, perhaps as a separate flag on the verify or a more explicit cli option outright.

Verify only:

cosign verify-manifest -key cosign.pub <path/to/manifest>

Verify and write meta (on verify):

cosign verify-manifest -write-meta -key cosign.pub <path/to/manifest>

Or explicitly:

cosign write-meta-manifest -key cosign.pub <path/to/manifest>

Once that exists, it doesn't feel like #38 would be required.

I can try and get something together this coming week if this sounds like an ok start. Probably two PR's would be incoming, one for the verify and another follow-up with the metadata write, they'd be closely linked as it could be done as part of the validation pass.

joshes avatar Jul 25 '21 18:07 joshes

Question around the manifest parser implementation.

Happy path parsing via regex solves for YAML resources and gets this out quickly which is a good thing. But, it is regex and wouldn't support JSON resources OOTB. We could feature-add JSON support by traversing the object looking for "image" keys to keep it simple - but wanted some feedback on this.

Should we enforce a .yaml extension for sane error for the time-being and also, how complex of an image "finder" should we consider?

Simple is better is my opinion.

[Edit] Also available for discussion: thinking if we add explicit support for manifests which I mentioned in #38 (e.g., push a signed manifest to a registry and perform validation on the resource itself) then we may want to rename this from verify-manifest to something more explicit like verify-manifest-images.

joshes avatar Jul 26 '21 23:07 joshes

#38

Le lun. 26 juill. 2021 7 h 03 p.m., Joshua Hansen @.***> a écrit :

Question around the manifest parser implementation.

Happy path parsing via regex solves for YAML resources and gets this out quickly which is a good thing. But, it is regex and wouldn't support JSON resources OOTB. We could feature-add JSON support by traversing the object looking for "image" keys to keep it simple - but wanted some feedback on this.

Should we enforce a .yaml extension for sane error for the time-being and also, how complex of an image "finder" should we consider?

Simple is better is my opinion.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sigstore/cosign/issues/437#issuecomment-887084109, or unsubscribe https://github.com/notifications/unsubscribe-auth/APCT2RYKOA5Y2EZUXG6BZY3TZXSVPANCNFSM5AN4YNQA .

pafco007 avatar Jul 27 '21 09:07 pafco007

Mon argent est ou

Le mar. 27 juill. 2021 4 h 59 a.m., Stéphane Dionne @.***> a écrit :

#38

Le lun. 26 juill. 2021 7 h 03 p.m., Joshua Hansen < @.***> a écrit :

Question around the manifest parser implementation.

Happy path parsing via regex solves for YAML resources and gets this out quickly which is a good thing. But, it is regex and wouldn't support JSON resources OOTB. We could feature-add JSON support by traversing the object looking for "image" keys to keep it simple - but wanted some feedback on this.

Should we enforce a .yaml extension for sane error for the time-being and also, how complex of an image "finder" should we consider?

Simple is better is my opinion.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/sigstore/cosign/issues/437#issuecomment-887084109, or unsubscribe https://github.com/notifications/unsubscribe-auth/APCT2RYKOA5Y2EZUXG6BZY3TZXSVPANCNFSM5AN4YNQA .

pafco007 avatar Jul 27 '21 09:07 pafco007

Actually, instead of "verify-manifest", what if we move this into the "verify" command itself? It could take an image URL or a path to a local manifest file.

Im worried now, because I'd also like to add "verify-attestations" for "manifests", so flattening "manifest" into the verify/verify-attestation commands might be nicer.

dlorenc avatar Jul 30 '21 15:07 dlorenc

I posted this on Slack yesterday and wondering if its applicable here or not (apologies for the cross-post)

... We are verifying Images, Dockerfiles & now Manifests. Do we see this continuing with specialized verifiers? If so I may propose a pluggable verifier system so you could add them generally and run them akin to a kubectl plugin (e.g., cosign verify -key ... ). OR…do we think the verifiers we have will be the generally supported ones?

I like the idea of flattening it to basically a URI or Manifest, so long as that covers the expected future wants of cosign and we are explicit about what a Manifest is and expectations. I could imagine arbitrary verifiers being wanted by future users though for unhandled use-cases.

joshes avatar Jul 30 '21 15:07 joshes

Ah, now I get it! Yeah, "verifiers" would be "the thing that lists the objects to verify"?

dlorenc avatar Jul 30 '21 15:07 dlorenc

Kind of. Here's an example session of what I think would be a good ux (IMO).

# List 
cosign list-verifiers
Available verifiers:
  image (default)
  dockerfile
  docker-compose
  manifest (json or yaml k8s resources)
  my-custom-verifier
 
# Still use image as default verifier - no change, no breaky!
cosign verify -key key.pub <IMAGE>

# Verify dockerfile
cosign verify -key key.pub --verifier dockerfile <URI or Path to Dockerfile>

# Verify manifest
cosign verify -key key.pub --verifier manifest <URI or Path to Manifest>

If the system was pluggable, new verifiers could be added without being so explicitly in the main cosign repository which may add to a more narrower surface area for cosign directly (e.g., "core" functionality vs extended functionality).

So requirements would be:

  • Basic plugin system (effectively a driver to add to list of verifiers, load URI or file path, execute verifier itself)
  • Verifier interface itself which would need to get/set cli opts and do the custom validation logic on the resource type
  • The verifier implementation(s)

Of course this is all overkill if cosign will ever only want to validate Dockerfiles and Manifests.

Food for thought.

joshes avatar Jul 30 '21 16:07 joshes

list-verifiers sounds cool idea! We may do not have to pass --verifier flag since if we check and recognize the input format. Same idea fits to list-signers command, IMO.

Dentrax avatar Jul 30 '21 16:07 Dentrax

  • Basic plugin system (effectively a driver to add to list of verifiers, load URI or file path, execute verifier itself)

I'd really like to avoid going with an extra install/plugin system. Keeping everything in-tree here would be fine with me!

Do you think we would actually need the --verifier flag in all cases, or could we sometimes detect the type of file automatically?

dlorenc avatar Jul 30 '21 18:07 dlorenc

Seems we wouldn't require the --verifier flag so no, which would be much simpler overall. After thinking on it more, what you propose makes sense, just keep the verifiers here (in-repo).

Sounds good!

joshes avatar Jul 30 '21 20:07 joshes

I only just saw the latter part of this discussion 😔

I have concerns about the usability and maintainability of overloading verify with different image discovery logic for each type of artifact we want to scan. With the intention of eventually adding additional artifact-specific commands, I went ahead and re-organized verify-dockerfile into dockerfile verify. The next step would be to add a command which resolves images to their digests, allowing users to avoid race conditions between verify and an image pull.

dekkagaijin avatar Sep 15 '21 18:09 dekkagaijin

/cc @n3wscott

dekkagaijin avatar Sep 15 '21 18:09 dekkagaijin

@dekkagaijin Do we still need this issue open ?

hectorj2f avatar Apr 11 '22 19:04 hectorj2f

This issue is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 5 days.

github-actions[bot] avatar Sep 18 '22 02:09 github-actions[bot]

This issue was closed because it has been stalled for 5 days with no activity.

github-actions[bot] avatar Sep 23 '22 02:09 github-actions[bot]