-
Notifications
You must be signed in to change notification settings - Fork 547
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 ignoring certificates with pkcs11 #3334
add support for ignoring certificates with pkcs11 #3334
Conversation
b78e1ae
to
f119b40
Compare
Codecov Report
@@ Coverage Diff @@
## main #3334 +/- ##
=======================================
Coverage 30.32% 30.32%
=======================================
Files 155 155
Lines 9921 9921
=======================================
Hits 3009 3009
Misses 6462 6462
Partials 450 450
|
I haven't tried to run these tests before. You'll need to use SoftHSMv2 as noted in the comment, though how exactly everything should be configured I'm not sure. If you do get tests running locally, please write up some docs, that would very helpful! |
For doc updates, you can add a section to https://github.com/sigstore/docs/blob/main/content/en/signing/pkcs11.md |
Are you able to compile this locally and confirm it's working as expected? |
Do the PKCS11 tests not run in CI at all? I'll see if I can get them to run locally tomorrow, along with those docs updates. I am able to compile Cosign with this patch and have confirmed that it works correctly with my specific HSM via PKCS11. |
I’ve got to double check, but it appears that these tests aren’t running on CI. |
Great! I am still working on the local tests. I opened a docs PR for afterwards here: sigstore/docs#266 |
This commit adds a new environment variable, COSIGN_PKCS11_IGNORE_CERTIFICATE, which will skip loading certificates into a PKCS11 key when set to "1". This is desirable when you want to sign with a private key that has a certificate associated with it, but do not want that certificate to be included with the signature for verification. Certificates are already optional for keys from non-PKCS11 sources via the --certificate command line flag. Signed-off-by: dylrich <[email protected]>
f119b40
to
ac202c9
Compare
@haydentherapper I was able to run the PKCS11 tests locally! #1256 introduced a compilation error into the tests, but I pushed an update to my commit that fixes it. The rest of the tests worked without issue. Here are my notes, tested in a Debian 11 container image:
|
@dylrich , I am also testing the same way as you did. The slots seems to be present using commands:
The issue I am facing is not getting
|
@viveksahu26 The tests already have a PKCS11 URI configured without using |
Yeah, "Definitely." Running the command Let's see below how we can get
NOTE:
|
Summary
This commit adds a new environment variable,
COSIGN_PKCS11_IGNORE_CERTIFICATE
, which will skip loading certificates into a PKCS11 key when set to "1". This is desirable when you want to sign with a private key that has a certificate associated with it, but do not want that certificate to be included with the signature for verification. Certificates are already optional for keys from non-PKCS11 sources via the --certificate command line flag.I also added a test, but I wasn't actually able to run the test. Do you have any advice for how to work with the HSM that Cosign's tests are supposed to use? I am happy to make any requested changes to the test or implementation! I'm also not sure how to indicate documentation should be changed, and would appreciate some guidance there.
Release Note
Closes #3333