sigstore / cosign

Code signing and transparency for containers and binaries
Apache License 2.0
4.48k stars 547 forks source link

Improved CLI UX for verifying online/offline for {TSA,Rekor,Fulcio} #2648

Open znewman01 opened 1 year ago

znewman01 commented 1 year ago

I think these are all fine as incremental improvements. If I'm allowed to dream big:


My overall philosophy here is to have flags to specify really clearly what's going on, and then set sensible defaults so most people don't have to think about them. I think "boolean" is the wrong type for many of these flags, which can interact in complicated ways.

So here's a proposal:

I believe this accomplishes the following goals:


One point I'd like to respond to:

Check the certificate verification time against the tlogVerificationTime (if present) and the tsaVerificationTime. Ensure that at least one was checked to ensure signature validity.

What do we do if one fails? Print a warning? I think we should require that both succeed, and if there's pushback, add policy flags to dictate which you trust (also solvable via delegations in the future)

Agree, this checks both, and requires at least one to have been checked (just in case no TSA is provided, and no bundle was provided)

(Related also to "Remove the CheckOpt for TSAClient. It's not used for verification, and only used as a toggle for whether we should verify against the TSA.")

I'd like folks to be more explicit about what they're expecting here (with a reasonable default). This strikes me as a potential source of confusion.

Originally posted by @znewman01 in https://github.com/sigstore/cosign/issues/2466#issuecomment-1320213278

ivanayov commented 1 year ago

Can I work on this?

haydentherapper commented 1 year ago

I’d recommend doing this in a non-breaking way given we just rolled out 2.0 and shouldn’t introduce breaking behavior right away. This might introduce a lot of flag confusion though, so we need to be mindful of that.

haydentherapper commented 1 year ago

Some of the signing and verification refactors might also tie in with the Sigstore-go library work.

znewman01 commented 1 year ago

@ivanayov that'd be awesome.

+1 to being careful here about compatibility.

I think there's a nice story for rolling this out:

  1. make internal libraries work based on this behavior (enums instead of bools)
  2. the old boolean flags just set the appropriate enum values
  3. add new flags for the new behavior. using both types of flags together should be an error
  4. once we've tested the new flags a little, make them the default in docs/etc.
  5. deprecate the old flags

Especially if done over 6+ months, I think this is a reasonable transition for users

github-actions[bot] commented 1 year ago

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] commented 1 year ago

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] commented 1 year ago

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.

vishal-chdhry commented 1 year ago

Is this still open?

github-actions[bot] commented 12 months ago

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] commented 11 months ago

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