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

PKCS11 HSM support after removal of dependent OpenSSL binding #4967

Closed
clover-bt opened this issue Aug 12, 2019 · 8 comments
Closed

PKCS11 HSM support after removal of dependent OpenSSL binding #4967

clover-bt opened this issue Aug 12, 2019 · 8 comments

Comments

@clover-bt
Copy link

https://github.com/space88man/cryptography_engine, prior to pyca/cryptography release 2.6, provided a relatively clean Python wrapper to access and utilize HSM objects through the PKCS11 interface.

https://github.com/pyca/cryptography/pull/4768/files removed the ENGINE_load_builtin_engines() binding, which appeared to be necessary for the wrapper's operation. OpenSSL still appears to have that function in its library. (Other required bindings may also have been removed; further research is needed.)

In #4446, it was suggested that having HSM support and upstreaming the wrapper code might be possible.

What might be the next steps to towards getting stable PKCS11-based HSM support in pyca/cryptography?

@reaperhulk
Copy link
Member

Not knowing/remembering our binding consumers strikes again.

I think we need to know the set of bindings required for this to work here. And we should also think about whether it makes sense to do the PKCS11 support through OpenSSL or if some other idea makes more sense. (e.g. should we should resurrect the backend concept in cryptography to allow a separate PKCS11 backend to be passed in?0

@clover-bt
Copy link
Author

Were used and now missing:

ENGINE_ctrl_cmd_string
ENGINE_load_builtin_engines
ENGINE_load_private_key
ENGINE_load_public_key

EVP_PKEY_CTX_set_rsa_mgf1_md
EVP_PKEY_CTX_set_rsa_oaep_md
EVP_PKEY_CTX_set_rsa_padding
EVP_PKEY_CTX_set_rsa_pss_saltlen

EVP_md5
EVP_sha1
EVP_ripemd160
EVP_sha224
EVP_sha256
EVP_sha384
EVP_sha512

Removal of EVP_hashfunction entries seems to have occurred via https://github.com/pyca/cryptography/pull/4775/files#diff-04d869a327dc59acfd9b090d3a166467L100, also February 2019.

Used and still present:

_lib.ENGINE_by_id
_lib.ENGINE_finish
_lib.ENGINE_free
_lib.ENGINE_init

_lib.EVP_PKEY_CTX_free
_lib.EVP_PKEY_CTX_new
_lib.EVP_PKEY_CTX_set_signature_md
_lib.EVP_PKEY_decrypt
_lib.EVP_PKEY_decrypt_init
_lib.EVP_PKEY_encrypt
_lib.EVP_PKEY_encrypt_init
_lib.EVP_PKEY_get1_EC_KEY
_lib.EVP_PKEY_get1_RSA
_lib.EVP_PKEY_id
_lib.EVP_PKEY_sign
_lib.EVP_PKEY_sign_init
_lib.EVP_PKEY_verify
_lib.EVP_PKEY_verify_init

@clover-bt
Copy link
Author

Would it be possible to follow up and see if there are any further thoughts on the preferred method of pkcs11 support?

@xzr
Copy link

xzr commented Sep 16, 2019

Just an IMO, not meant to press the issue left or right.

As cryptography does have a concept of a backend already (although not really utilized), it would make sense to implement a new backend that could hook directly to a pkcs11 library.

Having the openssl engine in the middle is an additional and a unnecessary dependency. The PKCS#11 API is very well defined and I've mostly run to very minor vendor-dependent implementation details over the course of working with it.

@aalba6675
Copy link

aalba6675 commented Sep 27, 2019

Another "Just an IMO, not meant to press the issue left or right."...

ENGINEs are used to hook into the OpenSSL TLS stack as OpenSSL does not interface directly with PKCS#11. Having ENGINE support in cryptography - maybe not so useful for pure crypto - is necessary for scripting scenarios to run TLS applications/tests that need to access keys via the ENGINE middleware (as OpenSSL would do), rather than directly using PKCS#11.

Example: a HSM like Gemalto SafeNET provides both the PKCS#11 library libCryptoki64_2.so and the ENGINE wrapper gemengine.so. The former can be used directly by a hypothetical cryptography PKCS#11 backend; however when running TLS scripts/tests we would still need to do it via ENGINE.

Indeed that is why the OpenSC folks provided the pkcs11.so ENGINE shim for "generic" PKCS#11 libraries.

@reaperhulk
Copy link
Member

Ugh, sorry for letting this slip so long...

The various forks of OpenSSL that we support/want to support make it desirable for us to keep our bindings as small as we can. However, I'd like to enable HSM consumers within reason. We've talked about exposing providers once OpenSSL 3.0 is released, but maybe we need to consider re-adding the ENGINE bindings and make them conditional for our experiments in supporting other OpenSSL forks (e.g. boringssl). @clover-bt is this still something you'd like to see or have you solved this in another way? (Our backend system does allow for implementing PKCS11 directly. An old experiment that direction: https://github.com/reaperhulk/cryptography-pkcs11)

@ronf
Copy link

ronf commented Aug 18, 2020

I recently decided to add PKCS11 support to AsyncSSH, and I ended up choosing the python-pkcs11 package for this purpose. It provides clean Pythonic access to the underlying PKCS#11 dynamic library, and seems quite complete. While it is not based on Cryptography right now, I had no problem making it interoperate with my code, which uses Cryptography almost exclusively for crypto operations. For instance, it was no problem to use python-pkcs11 to do signing and export the public keys from the security token into Cryptography for doing verification.

@reaperhulk
Copy link
Member

We've re-added ENGINE bindings for now. If there are more missing that are required please open a new issue and we can discuss. I'd like to do this more cleanly, but I don't want to block this use case so bindings it is...

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Dec 12, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

No branches or pull requests

5 participants