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

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

Open
znewman01 opened this issue Jan 23, 2023 · 10 comments
Open

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

znewman01 opened this issue Jan 23, 2023 · 10 comments
Assignees

Comments

@znewman01
Copy link
Contributor

znewman01 commented Jan 23, 2023

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:

  • Flags
    • What Sigstore infra to use?
      • Where do the keys come from?:
        • --tuf-root=URL (default: Sigstore public good)
        • --fulcio-root (replaces --certificate-chain; default: comes from TUF root)
        • --rekor-root (default: comes from TUF root)
        • --ct-log-root (default: comes from TUF root)
      • --rekor-url=URL, --fulcio-url=URL, --tsa-url=URL (defaults: URLs provided from the TUF root)
    • How should Cosign actually verify?
      • --rekor-verification={offline,fallback,online,insecure} (default fallback)
      • --sct-verification={offline,fallback,online,insecure} (default fallback; replaces --enforce-sct flag)
      • --tsa-verification={offline,fallback,online} (default fallback)
    • Verification material:
      • DATA (positional): file name for verify-blob, gcr.io/foo:latest for verify
      • --bundle (the new bundle format)
        • Can replace all of these:
          • --rekor-bundle=FILE
          • --certificate=FILE
          • --signature=FILE
        • For OCI only, default is fallback to fetching from OCI.
      • --offline flag doesn't affect fetching verification material from OCI registry!
    • Identity flags:
      • Require one of:
        • --certificate-issuer AND --certificate-identity (optionally: also --certificate-github-*)
        • --key
        • --sk (and optional --slot, which should be renamed to --sk-slot)
    • --offline: force all of the flags above to offline (except probably fetching the image from the OCI registry)
  • If anything is explicitly provided (e.g., via a flag) and doesn't validate, fail.
  • If anything is implicitly provided (e.g., via a "fallback", or via the bundle) and doesn't validate, warn ("found TSA in the Cosign annotation but it fails").

I believe this accomplishes the following goals:

  • Doesn't exhibit "adding more information makes verification fail" behavior for implicit
  • Only makes necessary network calls

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 #2466 (comment)

@ivanayov
Copy link
Contributor

ivanayov commented Mar 6, 2023

Can I work on this?

@haydentherapper
Copy link
Contributor

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

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

@znewman01
Copy link
Contributor Author

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

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

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

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

Is this still open?

Copy link

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.

Copy link

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

@github-actions github-actions bot closed this as not planned Won't fix, can't repro, duplicate, stale Nov 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants