-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Added improved support for CA stored in PKCS#11 based token (RFC 7512 URIs). #433
base: master
Are you sure you want to change the base?
Conversation
Introducing PKCS#11 support, mainly restaging and reviewing work of 0xdecaf work from OpenVPN#332. *Successfully create a CA using a pkcs11 device * Added command line parameter for pkcs11 options * Only insert engine configuration when using pkcs11 * Ensure PKCS11 config is at the top of openssl configuration * Bringing PKCS#11 documentation up to date * Adding external pinpad readers support. Co-Authored-By: Tony <[email protected]>
…1_pin function; - Added support for spaces in PKCS11 label; - Added support for devices requiring SO-PIN login to generate keys (i.e. Yubikey) - Added guide for Yubikeys in PKCS11.md
Changes: - Moved the whole PKCS11 object's referencing to RFC 7512 PKCS11 URI (Better long-term support) - Updated vars.example accordingly - Fixed quoting of PKCS11 label & URI in `CA.key` and `pkcs11-tool` call - Added support for Nitrokey HSM - Ported support for Yubikey to RFC 7512 - Updated PKCS11.md and improved documentation - Updated Travis-CI tests for SoftHSM2 using RFC 7512
Fixed installation of gnutls-bin.
- Ported PKCS11 key generation from `pkcs11-tool` to GNU-tls `p11tool` (It enables listing of PKCS11 URIs) - Added a check to ensure GNUtls is installed - Added a control when no `PKCS11_URI` is selected, listing available tokens and asking the user to select one - Added a check for existing keys with specified ID (It asks the user to confirm usage of existing keys and warns in case of label mismatch) - Removed `PKCS11_EXTRA_OPTIONS` as it appear to be unnecessary - Changed `PKCS11_SLOT` to `PKCS11_KEY_ID` to avoid misunderstanding of terms - Changed `PKCS11_LABEL` to `PKCS11_KEY_LABEL` for better understanding - Removed trailing ";" from `PKCS11_TOKEN_URI`, so it does not deviate from the RFC 7512 standard. - Updated `PKCS11.md` usage guide - Minor style/typo fixes
Moved to legacy key generation commands for p11tool.
- Fixes missing quoting of `PKCS11_KEY_LABEL` which broke support of labels containing spaces - Informs the user when searching for existing keys (it might take a while for some tokens) - Minor fixes on `PKCS11.md` documentation.
Added support to use existing keys from a PKCS11 token, if a matching pair is found, before trying to generate them. In particular I would like to ask on a couple of points (though any other suggestion is of course welcome):
|
These fixes look pretty good. I will review them and merge this weekend. Poke me if I forget. :)
Eric F Crist
… On Mar 11, 2021, at 02:58, Rob ***@***.***> wrote:
Added support to use existing keys from a PKCS11 token, if a matching pair is found, before trying to generate them.
There is still room for improvement, but I would appreciate some feedback before moving further.
In particular I would like to ask on a couple of points (though any other suggestion is of course welcome):
Is it okay to use p11tool --generate-[rsa,ecc] commands (which are deprecated in recent versions of GnuTLS) instead of p11tool --generate-privkey=[rsa,ecc,..] (not deprecated but not yet present on Ubuntu Bionic 18.04 used for current testing)?
For sake of simplicity, I would suggest to drop SO/Admin login support for key generation. I implemented it as a first workaround solution for some token like Nitrokey Pro or Yubikey 5, but I would suggest using the "existing key" feature instead, after following the manufacturer suggested procedure to generate the keys.
—
You are receiving this because you are subscribed to this thread.
Reply to this email directly, view it on GitHub <#433 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AANXQPY5ENDMVEIC5AO4OBDTDCA4BANCNFSM4YCJIVYA>.
|
@ecrist, thanks for the feedback! |
@@ -7,7 +7,7 @@ | |||
# HOW TO USE THIS FILE | |||
# | |||
# vars.example contains built-in examples to Easy-RSA settings. You MUST name | |||
# this file 'vars' if you want it to be used as a configuration file. If you do | |||
# this file 'vars' if you want it to be usecat d as a configuration file. If you do |
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.
typo
Thank you for working on this.
|
I'm using an openssl wrapper like this in my setup.
My snippet is rather small, comparing to this PR. IMHO, leave the |
|
||
# Check to see that it exists | ||
if [ -f "$ca_key_file" ]; then | ||
grep PRIVATE "$ca_key_file" |
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.
stdout/stderr of grep should be redirected
Hi, some $EASYRSA_P11TOOL commands do not use the $PKCS11_MODULE_PATH provider, specifically those listing tokens urls / keys and labels. This prevents using Yubikey own pkcs11 module (libykcs11.so) which for Yubikey 5 NFC list all retired slots (82 through 95) by default thus allowing usage with easy-rsa / openssl. Patch here: PS: Sorry if this is not the right way to post a patch to an issue, first time doing this on github. |
Hi, I did some tests and I can confirm that this PR works perfectly with Gemalto MD840 smart card. I can create CA with private key generated and stored on the SC and I can sign other certs using this CA. It works even on the remote server using the "p11-kit server" and SSH tunneling (sources here and here). There are some small bugs like 'echo "\t…' is not printed as tab, just '\t' is printed. @robpower Thank you for your good job! I hope this PR will be merged soon. |
I'm not sure overloading the [private].key file is the correct approach: I'd propose using something like [private].pk11 or [private].hsm and rather than a fork to grep, test presence of the [private].pk11 (or chosen f.ext). reasoning: |
It seems some recent OpenSSL upgrade broke that functionality for me:
ca.key:
Invoked openssl command resulting in above error:
Might be related to: OpenSC/libp11#444 |
Hi. I'm curious. Are there plans to merge this PR? |
At this current time; No, there are no plans to merge this PR. This remains open, pending thorough review. |
Add support for PKCS11 tokens, based on initial proposal #332 in response to #268.
In comparison to previous implementation, this one makes use of RFC 7512 scheme to address tokens and keys, instead of legacy engine_PKCS11 ID (apparently going to be deprecated in openssl).
Tested on Smartcard-HSM, Nitrokey HSM, Nitrokey Pro, SoftHSM2 and Yubikey 5 NFC.
Introduces a new format for the
./pki/private/ca.key
which can specify environment variables to point to the PKCS#11 private key on a specific token.Modification of
op_test.orig
support initial testing on SoftHSM2.Please see
./PKCS11.md
for more details on the changes as well as tested configurations for Smartcard-HSM, Nitrokey HSM, Nitrokey Pro, SoftHSM2 and Yubikey 5 NFC.Please share any useful feedback needed to get this PR merged.