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

Add cosign inspect command. #2210

Open
znewman01 opened this issue Aug 30, 2022 · 26 comments
Open

Add cosign inspect command. #2210

znewman01 opened this issue Aug 30, 2022 · 26 comments
Assignees
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@znewman01
Copy link
Contributor

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 znewman01 added the enhancement New feature or request label Aug 30, 2022
@znewman01
Copy link
Contributor Author

#1370 may be good inspiration for output

@haydentherapper
Copy link
Contributor

cc @kpk47

@Namanl2001
Copy link

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

@znewman01
Copy link
Contributor Author

znewman01 commented Oct 27, 2022 via email

@haydentherapper
Copy link
Contributor

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

@znewman01
Copy link
Contributor Author

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

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

@kpk47
Copy link
Contributor

kpk47 commented Nov 7, 2022

No, I haven't.

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

@znewman01
Copy link
Contributor Author

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
Copy link

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
Copy link
Contributor Author

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
Copy link
Contributor

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
Copy link

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
Copy link
Contributor

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.

  • bundles I believe is the Rekor Bundle, which is a struct that "holds metadata about recording a Signature's ephemeral key to a Rekor transparency log." - see here
  • payloads is the metadata inside the "signature" object that is signed. I believe that this in 99% of cases the contents looks something like:
[{"critical":{"identity":{"docker-reference":"foo.bar/foo/bar"},"image":{"docker-manifest-digest":"sha256:cc43a1e734b27b8f76992888ef40c1b6770d716af45083494683b0f45bd0c48d"},"type":"cosign container image signature"},"optional":null}]

Or at least something like that. In my mind, this is the kind of stuff that you want to get from a cosign inspect command. See here

  • signatures is the object that wraps around the payload with other fields. See in the same link for payload and it is the whole struct that make up a "signature".

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
Copy link
Contributor

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
Copy link
Contributor Author

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:

  • foo.txt: the data you're signing; this is a payload.
  • Public/private key pair: one-time use.
  • Certificate: this is a note from Fulcio that says "key K (the public key from above) belongs to user@example.com at time T." There may also be a "certificate chain" if there are extra intermediaries in between you and the Fulcio root key.
  • signature: the signature over the data, signed using the private key from above.
  • Rekor entry: basically, an envelope containing a hash of foo.txt, the signature, the certificate, and some optional metadata
  • Signed Entry Timestamp: a signature from Rekor over the Rekor entry, along with a timestamp (which proves that the signature was made before that time).

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:

  • The Rekor entry.
  • The certificate, and chain
  • The Signed Entry Timestamp.

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:

  • additional "annotations"
  • the certificate (and chain)
  • a "bundle", which confusingly refers to something different from the "bundle" above: a RekorBundle looks basically like the Rekor Entry plus a Signed Entry Timestamp.

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


Hopefully that was more helpful than confusing 😂

@znewman01
Copy link
Contributor Author

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

  • A subsection for each command: inspect, inspect-blob, inspect-attesation, inspect-blob-attestation (as @ChaosInTheCRD points out)
    • What flags do they have? (And note what's missing, as compared with their verify-* counterparts)
    • What will the output look like? Presumably the format is JSONL, but what's the structure that's being output?

@znewman01
Copy link
Contributor Author

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
Copy link
Contributor

ChaosInTheCRD commented Nov 18, 2022

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
Copy link

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
Copy link
Contributor Author

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 😄

@ChaosInTheCRD
Copy link
Contributor

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
Copy link
Contributor Author

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
Copy link
Contributor

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

@ChaosInTheCRD
Copy link
Contributor

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
Copy link
Contributor Author

  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
Copy link
Contributor

ChaosInTheCRD commented Feb 2, 2023

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants