-
Notifications
You must be signed in to change notification settings - Fork 13
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 support for Sigstore signing and verification (v2) #46
base: main
Are you sure you want to change the base?
Conversation
Given the API change with SigningContext, it does not make sense anymore to create a wrapper class around AnsibleBaseSignatureSigner
d3c45ed
to
e906bdb
Compare
…ature verification
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.
I'm curious about the code organization and tests which didn't seem to work for me.
tests/test_cli.py
Outdated
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.
The tests under these fixtures seem to be missing a .sigstore file
[ERROR] Sigstore bundle file for signature verification not found: tests/fixtures/sigstore/signed-valid/.ansible-sign/sha256sum.txt.sigstore
For more information about the different command line arguments available, use ansible-sign sigstore-sign --help`. | ||
|
||
By default, ``ansible-sign`` will use the Sigstore public good instances of Fulcio, Rekor and of the OpenID Connect issuer. | ||
If you wish to connect to private instances of Sigstore, specify the corresponding URLs with the ``--rekor-url``, ``--fulcio-url`` and ``--oidc-issuer`` options. |
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.
Is this true? It looks like --oidc-issuer
has been renamed to --cert-oidc-issuer
but it still looks like it's marked required?
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.
--cert-oidc-issuer
is the option passed to verify signatures and --oidc-issuer
the one to specify a private OIDC issuer instance for signing
docs/rundown.rst
Outdated
``ansible-sign`` will assume that the project signing materials are always located under ``.ansible-sign/``; | ||
this is why the command should specify the path of the project root when verifying a signature. | ||
|
||
The Sigstore verify options are available under the ``ansible-sign sigstore-verify`` subcommand, either using ``ansible-sign sigstore-verify identity`` |
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.
These commands are now homed under project
so ansible-sign project sigstore-verify
for example.
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.
Addressed in f4206e7
@@ -357,6 +729,239 @@ def gpg_sign(self): | |||
self.logger.debug(f"GPG Details: {result.extra_information}") | |||
return retcode | |||
|
|||
def sigstore_sign(self): |
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.
These functions kinda make me wonder if we shouldn't move them out to sign/verify specific modules rather than have it colocated with the cli itself?
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.
That would make sense, but in this case should we also move the corresponding GPG commands to stay consistent?
Thanks a lot for your review @matburt ! I addressed your comments in the last 5 commits, please take a look. |
Quality Gate passedIssues Measures |
bump |
Replaces #33
This implementation uses
sigstore-python v2.0.0rc1
waiting for the first stable v2 to be released as the v2 will also be used in AWX for project signature verification.Some
sigstore-python
CLI options have been removed to simplify the user experience when it comes to signing projects, but could eventually be reintegrated later if necessary:.crt
) and signature (.sig
) output files. Only Sigstore bundles (.sigstore
files) are produced during the signing process and used as input for verification