sigstore / sigstore-python

A Sigstore client written in Python
https://pypi.org/p/sigstore
Other
219 stars 45 forks source link

CLI: Limit user confusion over Rekor/Fulcio instance and state URLs #324

Closed woodruffw closed 4 months ago

woodruffw commented 1 year ago

Follow-up from https://github.com/sigstore/sigstore-python/pull/323#discussion_r1034822945: the various CLI options that control the Fulcio and Rekor instance state aren't "all or nothing," meaning that a user might forget to pass one and end up with a confusing error somewhere deep in signing or verification (e.g., when they reach SCT verification).

This isn't dangerous, per se, but we can definitely improve the UX.

jku commented 1 year ago

351 is going to make this even more visible in at least a couple of places. It does things like:

        updater = TrustUpdater.production()
        if args.ctfe_pem is not None:
            ctfe_keys = [args.ctfe_pem.read()]
        else:
            ctfe_keys = updater.get_ctfe_keys()

This happens in a place where either rekor or fulcio URL is non-default... but we end up loading production ctfe keys if none are provided by user

woodruffw commented 9 months ago

Marking with 3.0, since we'll be able to remove a lot of these confusing state flags once we add --trusted-root.

dmitris commented 8 months ago

just a thought - wouldn't it be better to call the flag --trusted-roots (plural)? In case the user needs to provide multiple roots, it would be clear and intuitive from the flag name itself that it is a valid and supported use case. And it still should be clear to those who have just one trusted root, that it is a valid much for the {N>=1} cardinality implied by the name 😄 It would also cover better (IMO) the fact that with this flag, the user can provide CA roots for Fulcio, Rekor, BYO CA etc.

jku commented 8 months ago

In case the user needs to provide multiple roots, it would be clear and intuitive from the flag name itself that it is a valid and supported use case

I've seen this idea mentioned but I don't understand what exactly it would mean. I definitely don't think it's intuitively clear and would like to see the concept explained.

woodruffw commented 8 months ago

just a thought - wouldn't it be better to call the flag --trusted-roots (plural)? In case the user needs to provide multiple roots

At least in my English dialect, this can go either way: the "trusted root" is either the thing containing a whole bunch of trust material, or it's a single piece of trust material in a larger "trust bundle." The protobuf-specs use it in the former way (the TrustedRoot message), so IMO we should just be consistent with them here 🙂

dmitris commented 8 months ago

ok that's fine, thanks 👍

woodruffw commented 5 months ago

This has been unblocked by upstream protobuf-specs changes: we now have a ClientTrustConfig message that we can load as a JSON blob, allowing us to replace all of the individual flags with something like --trust-config config.json.