sigstore / cosign

Code signing and transparency for containers and binaries
Apache License 2.0
4.38k stars 537 forks source link

Add `cosign inspect` command. #2210

Open znewman01 opened 2 years ago

znewman01 commented 2 years ago

Related to Sigstore clients should require a provided identity.

Right now, if you want to poke around at a signature/cert, the easiest way to do that is to run cosign verify. This is a little dangerous: commands that make it easy to verify artifacts just so you can pretty-print them often lead to patterns like "verify that this artifact was signed, but not who signed it."

The above doc proposes dropping support in cosign verify for this purpose. We want to replace it with a cosign inspect command. This will probably look similar to cosign verify or cosign verify-blob but with all of the "predicates" removed (e.g. --certificate-email). It should print human-readable output about the certificate/etc. that were passed.

znewman01 commented 2 years ago

https://github.com/sigstore/cosign/issues/1370 may be good inspiration for output

haydentherapper commented 2 years ago

cc @kpk47

Namanl2001 commented 1 year ago

@znewman01 - I would like to work on this issue. However, I'm not able to access the doc linked in the issue description.

znewman01 commented 1 year ago

Most sigstore docs can be accessed if you join @.***

https://groups.google.com/g/sigstore-dev

Let me know if that doesn’t work for you and I can add you directly

On Oct 27, 2022, at 2:26 PM, Naman Lakhwani @.***> wrote:

ο»Ώ @znewman01 - I would like to work on this issue. However, I'm not able to access the doc linked in the issue description.

β€” Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

haydentherapper commented 1 year ago

@kpk47 You haven't begun work on this yet, correct?

znewman01 commented 1 year ago

@kpk47 You haven't begun work on this yet, correct?

Ping, would like to make sure we don't cross paths here.

kpk47 commented 1 year ago

No, I haven't.

Sorry for the delayed response. I missed the email notification.

znewman01 commented 1 year ago

No worries! @Namanl2001 has expressed interest so I'd like to offer them the opportunity to work on it.

@kpk47 : If it becomes urgent or rises to the top of your TODO list, comment here and we can coordinate.

@Namanl2001 : Are you still able to do this? If so, do you need any help from us?

Namanl2001 commented 1 year ago

My understanding of this issue: currently for keyless signing, cosign verify does the insecure verification and adding --certificate-oidc-issuer --certificate-email makes it secure but we also want to have the current functionality of the cosign verify ie: verify that the artifact was signed, but not who signed it. And for this purpose we want to introduce a new command cosign inspect (please correct me if I'm wrong anywhere!)

(from the issue desc.)

with all of the "predicates" removed (e.g. --certificate-email).

what's the purpose of removing all the predicates? to support NO verification at all in inspect?

znewman01 commented 1 year ago

My understanding of this issue: currently for keyless signing, cosign verify does the insecure verification and adding --certificate-oidc-issuer --certificate-email makes it secure but we also want to have the current functionality of the cosign verify ie: verify that the artifact was signed, but not who signed it. And for this purpose we want to introduce a new command cosign inspect (please correct me if I'm wrong anywhere!)

Yup, that's right :)

with all of the "predicates" removed (e.g. --certificate-email).

what's the purpose of removing all the predicates? to support NO verification at all in inspect?

Precisely. Inspect isn't going to check anything, it's just going to report what's there.

haydentherapper commented 1 year ago

I think it might be useful to have a design doc before coding for this feature, so we can discuss formatting and what is to be reported by inspect

Namanl2001 commented 1 year ago

I think it might be useful to have a design doc before coding for this feature, so we can discuss formatting and what is to be reported by inspect

Drafted: https://docs.google.com/document/d/1V2Sag9aK4Vua8c_toTsJi9S-goIpv43EnicOuBZESBE/edit?usp=sharing

PHAL and share your thoughts.

Noob question πŸ˜… : I'm a bit unclear with the terms bundles, payloads & signatures, and need an explanation. You can also redirect me to some doc where common terminology is already mentioned. Thanks :)

ChaosInTheCRD commented 1 year ago

Hey @Namanl2001! sorry, it is always tough to find the time to work on open-source but really pressing to do more.

I was particularly interested in this feature and actually had a desire to write it and raise the PR. I also feel pretty comfortable with the terms you listed above, would you be interested in working together on this somewhat? Can't make a lot of promises on how much time I can dedicate though.

Also, no n00b questions. The way I sort of see it, we will be n00bs for the rest of our lives because we are all constantly learning πŸ˜„

@znewman01 please correct me if I have misconstrued any of the above

ChaosInTheCRD commented 1 year ago

oh yeah also - I guess the same functionality is going to be added for inspect-attestation and inspect-blob? the attestation is a big one to me as it always contains stuff that one would want to strip out and 'inspect' more closely πŸ˜„

znewman01 commented 1 year ago

Terminology is a little messy here, and it's all under-documented sadly because things keep changing (very normal in open source!).

@ChaosInTheCRD: yup, that's all right but there's some nuance: we need to distinguish between various "signature"/"bundle" concepts.

Let's ignore OCI images for a second and consider Cosign in the context of keylessly signing files (cosign sign-blob foo.txt). You wind up with a bunch of stuff:

To verify, a client needs to:

  1. Check that the certificate is signed by Fulcio.
  2. Check that the subject of the certificate matches the expected signer (such as zjn@chainguard.dev).
  3. Check that the Rekor entry is signed by Rekor.
  4. Check that the timestamp on the Rekor entry is during the validity period of the certificate.
  5. Check that the Rekor entry refers to the same data that you’re trying to verify.
  6. Check that the signature verifies against the public key from the certificate.

In order to do those things, they need:

That's a lot to keep track of, so we've decided to "bundle" most of it up into a common bundle format. Clients also need information about Rekor/Fulcio (namely, their public keys and the data itself, but those can be distributed separately.


Okay, now imagine you're signing a container image (cosign sign gcr.io/foo/bar@sha256:abcdef). Everything is the same, but now we need to store the bundle information alongside the container image in the OCI registry.

To do that, we use some hacks which aren't important, but importantly they involve coercing all of that data into a format that OCI supports. That's an "OCI Signature", which refers to:

We expect all of this to get replaced with the main Sigstore bundle format soon.


Hopefully that was more helpful than confusing :joy:

znewman01 commented 1 year ago

Doc is off to a good start! I think I'd like to see a little more detail on the following:

znewman01 commented 1 year ago

Oh, I want to echo @ChaosInTheCRD here too:

Also, no n00b questions. The way I sort of see it, we will be n00bs for the rest of our lives because we are all constantly learning πŸ˜„

ChaosInTheCRD commented 1 year ago

We expect all of this to get replaced with the main Sigstore bundle format soon.

Is there somewhere you could point me to so I can see this format? @znewman01

Namanl2001 commented 1 year ago

Thanks, @ChaosInTheCRD & @znewman01, for the detailed explanation; although I'm not able to digest everything, I believe I'll learn along the way πŸ’ͺ

would you be interested in working together on this somewhat?

Yes, @ChaosInTheCRD, that would be awesome.

znewman01 commented 1 year ago

We expect all of this to get replaced with the main Sigstore bundle format soon.

Is there somewhere you could point me to so I can see this format? @znewman01

https://github.com/sigstore/protobuf-specs apologies for confusion :smile:

ChaosInTheCRD commented 1 year ago

Hi all - Happy New Year! πŸ₯³

I was knocked out by a back problem through the majority of December, so I didn't get any time between finishing client engagements and Christmas to make progress on this work.

I thought I would make a small start today by sifting through all the flags available in each of the verify commands and adding to the doc here.

I have separated those that are generic to all the verify/verify- commands at the top, and those that are not generic feature under each commands subsection.

From here, I suppose it would be worthwhile to determine: a) do these flags all look present and correct? I suppose missing flags is a possibility? Just a quick look. b) are any of these not relevant for inspect? c) are there any obvious flags that may need to be present? This might become more clear while writing the features, so I suppose shouldn't be overboiled.

znewman01 commented 1 year ago

I was knocked out by a back problem through the majority of December, so I didn't get any time between finishing client engagements and Christmas to make progress on this work.

No worries! Please prioritize your health πŸ™‚ Hope your holidays were good otherwise.

a) do these flags all look present and correct? I suppose missing flags is a possibility? Just a quick look.

I double-checked your work: https://gist.github.com/znewman01/1cdea8669a69f05c7292ab6c8ac6a283

(CC @haydentherapper: you may find this interesting.)

b) are any of these not relevant for inspect?

So, for inspect I think we do want the flags that are related to fetching input materials (certificate, signature, artifact, bundle) and we don't want the flags that are related to verification policy (--sk, --check-claims, --certificate-github-workflow-ref).

Maybe the right format here is a spreadsheet? I'm finding the "list all flags for every command" format a little hard to navigate.

Once we have that, I'm happy to do a quick pass to check your gut instincts.

c) are there any obvious flags that may need to be present? This might become more clear while writing the features, so I suppose shouldn't be overboiled.

+1

ChaosInTheCRD commented 1 year ago

Sorry @znewman01 - should have said earlier. I am actively working on this - could I be assigned to the issue?

ChaosInTheCRD commented 1 year ago

just a couple of questions that I have found myself pondering this morning while working on it:

Do we see this as a command purely used for manually inspecting / debugging signatures/attestations etc.? Or do we imagine this being something that people want to use for policy evaluation (e.g., cosign inspect | opa eval...)? If so, we need to add appropriate warnings to ensure users understand that no signature evaluation is performed within the command Do we plan on removing the json currently being outputted with cosign verify, and this to be the alternative? Just making sure I fully get where we want to go with this.

znewman01 commented 1 year ago
  1. Do we see this as a command purely used for manually inspecting / debugging signatures/attestations etc.? Or do we imagine this being something that people want to use for policy evaluation (e.g., cosign inspect | opa eval...)? If so, we need to add appropriate warnings to ensure users understand that no signature evaluation is performed within the command

I think the name (cosign inspect) indicates pretty clearly that nothing is checked, especially if we mention that in docs as well.

In the long run, we probably want to do enforcement for complex policies inside of verify by letting folks pass .rego policies or similar. So that's not a goal here.

  1. Do we plan on removing the json currently being outputted with cosign verify, and this to be the alternative? Just making sure I fully get where we want to go with this.

I think the cosign verify output perhaps should change but we can deal with that a bit later.

ChaosInTheCRD commented 1 year ago

I have a draft PR here. Really scrappy, and haven't started on all the commands, but felt it would make sense to put this on here so people can start looking at this sooner rather than later. I can imagine that I could have missed / removed things that are important and done things people may disagree with 😊 @znewman01