-
Notifications
You must be signed in to change notification settings - Fork 63
cli: Support using other Sigstore instances #1548
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
Conversation
8dcbda4
to
6be6ef7
Compare
example: $ sigstore --instance https://sigstore.github.io/root-signing \ init-instance ~/src/root-signing/metadata/root.json $ sigstore --instance https://sigstore.github.io/root-signing \ sign README.md Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
6be6ef7
to
f853732
Compare
The intent seems clearer with new name. Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
7f66ddf
to
2473f4e
Compare
This way issues with initial root will also lead to TUFError Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
2473f4e
to
90e14c5
Compare
This case now leads to TUFError that is already handled upstream. The error is now a little less specific but still good. Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
A note on the security aspect here: the instances with embedded trust roots (currently sigstore.dev, sigstage.dev) are more secure than instances trusted with
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM with/without my suggestions.
48c9cd3
to
c658b73
Compare
Signed-off-by: Jussi Kukkonen <jkukkonen@google.com>
c658b73
to
8efbe66
Compare
Thanks for comments: I updated the help texts with "Mutually exclusive with other instance configuration arguments", and maybe finally got check-README to approve... |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks @jku!
I started prototyping #1546 and I think this turned out quite nice (since all of the internals already support this).
Summary
We already support
--staging
to change the used sigstore instance (and--trust-config
to completely manually define the instance) but supporting any sigstore instance would be nice (it would also be directly useful in the root-signing testing as that essentially uses other sigstore instances like https://sigstore.github.io/root-signing/).The complication is that there are two separate things that need to be supported:
This PR solves this by adding a top level subcommand
trust-instance
for 1 and a flag--instance
for 2. The former takes a TUF root metadata file and latter takes the URL as argment. I think this is the best compromise as--staging
is now basically a shortcut for--instance https://tuf-repo-cdn.sigstage.dev
and likewise you could say that the default is--instance https://tuf-repo-cdn.sigstore.dev
Similar implementations
Other clients have similar functionality: cosign via
cosign initialize
and sigstore-js withsigstore initialize
but these have the unpleasant side effect of modifying the default that the client uses from that point on: I think this is a bad idea and requiring a--instance ...
flag on every command is better. Users who want to always use a private instance should write a wrapper over sigstore-python to do that.example
Review items
trust.sigstore.dev
as the tuf url to make it clearer but 🤷.trust-instance
:--verbose
will show a stack trace that looks useful. I think that's reasonable.sigstore plumbing trust-instance
but that does not feel quite rightsigstore trust-instance --list
)