Skip to content
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 support for CA hosted in PKCS#11 based token #332

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

0xdecaf
Copy link

@0xdecaf 0xdecaf commented Oct 8, 2019

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.

Leveraging op_test.orig with a modification to support tests initially. Once this PR is merged, the tests can be migrated to the new framework.

Please see ./PKCS11.md for more details on the changes and please share any useful feedback needed to get this PR merged.

* Successfully create a CA using a pkcs11 device
* Added command line parameter for pkcs11 options
* Added a debug function to aid in local debugging (commented out for public consumption)
* Only insert engine configuration when using pkcs11
* Ensure PKCS11 config is at the top of openssl configuration
* Bringing PKCS#11 documentation up to date
@robpower
Copy link

robpower commented Jan 5, 2021

Hi! Great work, thanks for sharing!
I would like to contribute to this feature implementation. I am working on some minor changes which would help supporting extra features, like for example "pinpad PIN entry" on supported reader.
I see this branch has unresolved conflicts with the main executable and is 312 commits behind master. Does still make sense to send PR to https://github.com/0xdecaf/easy-rsa/tree/feature/pkcs11-engine-support or would it be better to start a fresh PR to master re-adapting this code?

@0xdecaf
Copy link
Author

0xdecaf commented Jan 5, 2021

Hi! Great work, thanks for sharing!
I would like to contribute to this feature implementation. I am working on some minor changes which would help supporting extra features, like for example "pinpad PIN entry" on supported reader.
I see this branch has unresolved conflicts with the main executable and is 312 commits behind master. Does still make sense to send PR to https://github.com/0xdecaf/easy-rsa/tree/feature/pkcs11-engine-support or would it be better to start a fresh PR to master re-adapting this code?

I took a quick look at the merge conflicts and they seem trivial so maybe just start there? If it becomes problematic to merge develop/rebase into the branch then just use this as a guide for a new branch. A fresh branch might be easier to get merged as this has been lingering for a while now. Might be a question best for the maintainer(s) though.

@kwinz
Copy link

kwinz commented Feb 2, 2021

I have merged this with master (although not 100% sure I merged it correctly so not sharing). And I tested this with Yubikey 5 with the PIV applet, and I want to share some feedback.

First of all to use it with Yubikey you need both the PKCS11_SLOT and an id. The terminology is a bit confusing basically the PKCS11_SLOT is the reader id, and the id is the PIV slot. See https://developers.yubico.com/PIV/Introduction/Certificate_slots.html So PKCS11_SLOT would be be 0, except if you have multiple Yubikeys/readers/applets plugged in at the same time. And id would be 1 for 9a, 2 for 9c, 3 for 9d, 4 for 9e and so on. See https://developers.yubico.com/yubico-piv-tool/YKCS11/Supported_applications/pkcs11tool.html An example call to pkcs11-tool to sign FILE with the key in 9c slot would be to use: pkcs11-tool --module /usr/lib/libykcs11.so.2.20 --login -login-type so -sign -m RSA-SHA256 --id 2 -i FILE -o FILE.sig I only have one reader plugged in, so the slot 0 can be omitted.

Since I found no way to specify ID I tried to use 2 or "02" as PKCS11_LABEL because that seems to be the ID of the object that pkcs11-tool -O shows for the 9c PIV key slot too. But when I called .\easyrsa build-ca pkcs11 with those variables I only got error Key not found.\n PKCS11_get_private_key returned NULL.

I would therefore suggest to add a new environment variable PKCS11_ID in vars and pass that to pkcs11-tool and openssl.

Secondly I want to be able to generate the CA key on an offline machine and then import the CA cert and CA key into two identical Yubikeys with ykman piv import-key 9c ca.key and ykan piv import-certificate 9c ca.crt on each. So that I have a backup. I have successfully done this. But right now it does not seem possible with this branch to use an existing key slot without overwriting it with a new certificate during .\easyrsa build-ca pkcs11. I propose to add this possibility so that I can have a backup on a second Yubikey.

Hope this is useful. Thanks for your work!

@jans23
Copy link

jans23 commented Feb 2, 2021

Thank you for this long awaited feature.

If anybody is interested to test it with a free Nitrokey, send us an email and we ship it to you.

@ecrist
Copy link
Member

ecrist commented Feb 2, 2021 via email

@kwinz
Copy link

kwinz commented Feb 3, 2021

I have more feedback from the Yubikey tests:

I was able to get further without crashing by adding --login-type so to $EASYRSA_PKCS11TOOL in the easyrsa source code.
Yubikey needs this, as it can only store a PIV key or certificate with the mangement key, which itself can be stored on the device protected by PIN. Apparently the --login failes when it's not done with --login-type so
I propose we add a PKCS11_OPTIONS config variable or similar where you can add the options that your particular hardware token needs.

The next step in not crashing was to set PKCS11_LABEL "Certificate for Digital Signature" in vars. I want to add however that this slot object only got this label, after I had set a different key for this slot with the ykman piv tool before. If you just take a Yubikey fresh out of the box I think this would not work. Then finally openssl takes --id 2 as argument and after hardcoding that it almost works now with the Yubikey.

Since hardware token setup is so vendor specific: Yubikey vs Nitrokey vs SoftHSM2 ( https://github.com/0xdecaf/easy-rsa/blob/feature/pkcs11-engine-support/PKCS11.md#softhsm2 ), etc. I would suggest to not merge this in it's current form. Maybe go for the simpler taks first to enable to user to use the token at all. As have a way to setup the script to use the pkcs11 token as is and have me worry about how to generate the key in a vendor specific way. If I understand the PKCS11.md documentation I could just set the defines in ./pki/private/ca.key manually and not use easy-rsa build-ca pkcs11 if that doesn't work for my token and doesn't allow me to make a backup of the key?

On other thing to consider is: some PKCS#11 tokens store keypairs only (e.g. loads of tokens based on openpgp card), while some other tokens can store keys+certificate. Should the CA certificate come from a file or should it be read from the token if present?

@robpower
Copy link

robpower commented Feb 3, 2021

Thanks everybody for your efforts!
Good news: after solving a weird bug, I finalized porting it to v3. I have to clean up few things but I think I can push it before the end of the week. @ecrist I am going to open a new PR, as I have no rights on this branch, so I started freshly from master.

Final port works on my side, using smartcard-HSM with external pinpad reader. Pinpad code won't interfere, as the feature is optional and configurable via vars.

@kwinz : I own no Yubikey, yet I can look to include your suggestions. Which model are you using?

If I understand the PKCS11.md documentation I could just set the defines in ./pki/private/ca.key manually and not use easy-rsa build-ca pkcs11 if that doesn't work for my token and doesn't allow me to make a backup of the key?

I am quite sure you can, as a workaround. Yet, I think we can make it work adding a PKCS11_OPTIONS or similar.
As for backups, I am not sure about yubikeys. If it works like an HSM, you should not be able to directly extract unencrypted backups (intended behavior for security) and the key is generated inside the device. If you configured it properly before use, you can still extract an encrypted backup.

@jans23 : I own no Nitrokey either, yet the solution should be compatible. If you wish to send one, I would be happy to double check.

@TinCanTech
Copy link
Collaborator

TinCanTech commented Feb 3, 2021

If anybody is interested to test it with a free Nitrokey, send us an email and we ship it to you.

@jans23 Should we send email to [email protected] ?

@jans23
Copy link

jans23 commented Feb 3, 2021

Please do.

@kwinz
Copy link

kwinz commented Feb 3, 2021

@robpower That sounds great!

I have been testing with Yubikey5 Nanos so far, with the PIV applet. They also support openpgp card emulation, which I have not been testing yet. I also have a Yubikey 4 and various other hardware tokens and smartcards at home that I could test with (no Nitrokey yet ;-) ). But I think the Yubikeys are all pretty similar across generations.
I am not afiliated with Yubikey in any way. But I can test something for you or I can send/buy you one if that helps (email me your address and what model you want: krainz.io/wp-content/uploads/2016/12/mail.png or Twitter PM linked via profile page).

Thanks for considering the PKCS11_OPTIONS idea.

As for backups: the tokens don't allow you to extract the private key. But you can boot off a live CD on an offline computer, generate the keys there on tempfs, and then load the key into two identical tokens before wiping them from RAM. So I have a backup token but the keys will never leave the tokens again. For me that's the ideal solution to have a backup. I don't generate the keys on the token, I import them.
You won't get an attestation certificate that the CA keys were generated on the token, but that's more a feature that you would need for mass client key deployments, not for your one (sub-)CA key.

Let me know if I can help with anything.

robpower added a commit to robpower/easy-rsa that referenced this pull request Feb 5, 2021
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]>
robpower added a commit to robpower/easy-rsa that referenced this pull request Feb 5, 2021
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]>
robpower added a commit to robpower/easy-rsa that referenced this pull request Feb 5, 2021
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]>
robpower added a commit to robpower/easy-rsa that referenced this pull request Feb 5, 2021
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]>
@robpower
Copy link

robpower commented Feb 7, 2021

Hi again!

Sorry for the multiple commit references, last one is the good one. I had been implementing the port and it works fine on my side.

I still have some troubles with extra tests I added to travis for pkcs11 using softhsm. I tried to debug it on a fresh VM and it seems a permission problem over both /etc/softhsm/softhsm2.conf and var/lib/softhsm/tokens. I added some changes to solve permissions on a secondary branch, but I still cannot get travis to succeed.
Here is the error executing easyrsa build-ca pkcs11:

 DEBUG:  pkcs11-tool --module /usr/lib/softhsm/libsofthsm2.so --slot 1687927385 --login --pin 1234 --keypairgen --key-type rsa:2048 --label my-test-token --usage-sign --private 

pkcs11-tool: unrecognized option '--pin 1234'

I am sure I am missing something, so any hint is welcome.

@kwinz During the next days I am going to finish the PKCS11_OPTIONS solution to make it work with different implementations (i.e. Yubikey). I will share as soon as it is ready.

@robpower
Copy link

Made a new PR (#433), based on this one, using RFC 7512 PKCS#11 URIs to select tokens (and keys).

As OpenSSL warns about possible future deprecation of legacy engine_pkcs11 IDs, this solution should guarantee better long-term support, as well as it guarantees more versatility about different tokens.

It is currently tested on Nitrokey HSM/SmartcardHSM, Nitrokey Pro, SoftHSM2 and Yubikey 5 NFC.
It should work virtually with any other device, as long as they are minimally compatible with RFC7512

Any feedback is welcome as there still could be some edge to smooth and room for improvement.

@0xdecaf thanks for making it possible in the first place.
@jans23 @kwinz Thanks for making possible to test it on multiple devices and manufacturers.

@TinCanTech TinCanTech added conflicts Conflicts with current PKCS labels Mar 31, 2022
@TinCanTech
Copy link
Collaborator

Is this superseded by #433 or not ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants