libp11 icon indicating copy to clipboard operation
libp11 copied to clipboard

Add PKCS11_generate_ec_key() for private EC key generation

Open vesajaaskelainen opened this issue 4 years ago • 15 comments

This PR adds support to allow client libraries to generate elliptic curve keys within the PKCS#11 token.

Existing PKCS11_generate_key() has a limitation that it can only do RSA keys, as there are existing users for the API this adds new API specific to EC key generation.

Observed that the result is more or less the same as:

pkcs11-tool --module <your pkcs11 module here> --token-label device --login --pin "<your pin here>" --keypairgen --key-type EC:prime256v1 --label testeckey

or with NSS's certutil (except that certutil doesn't set the label for some reason):

certutil -d sql:/etc/pki/nssdb -h device -f /run/pki/pin -z /run/pki/noise -R -k ec -q nistp256 -n ecdsa-client -a -o ecdsa-client.csr.pem -Z SHA256 -s "CN=example" --keyUsage digitalSignature,keyAgreement --extKeyUsage serverAuth,clientAuth

Example code snippet

    char *label = "mylabel";
    unsigned char id[20];
 
    //const char *curve = "P-256";
    const char *curve = "prime256v1";
...
    rc = PKCS11_open_session(slot, 1);
    error_queue("PKCS11_open_session");
    CHECK_ERR(rc < 0, "PKCS11_open_session failed", 4);
    if (slot->token->loginRequired && argc > 2) {
        /* perform pkcs #11 login */
        rc = PKCS11_login(slot, 0, argv[2]);
        error_queue("PKCS11_login");
        CHECK_ERR(rc < 0, "PKCS11_login failed", 6);
    }
...
    rc = RAND_bytes(id, sizeof(id));
    if (rc != 1) {
        error_queue("RAND_bytes");
        CHECK_ERR(rc < 0, "RAND_bytes failed", 4);
    }
 
    rc = PKCS11_generate_ec_key(slot->token, curve,
                                label, id, sizeof(id));
    if (rc) {
        error_queue("PKCS11_generate_ec_key");
        CHECK_ERR(rc < 0, "PKCS11_generate_ec_key failed", 4);
    }

vesajaaskelainen avatar Dec 11 '20 15:12 vesajaaskelainen

LGTM based on testing with SoftHSMv2.

hlounent avatar Dec 14 '20 10:12 hlounent

See the related https://github.com/OpenSC/libp11/pull/378#issuecomment-755602309 for a way to implement this functionality without changing the public API. We really should avoid adding new functions for each key type and use the EVP_PKEY interface instead. Could you please update your PR?

mtrojnar avatar Jan 06 '21 20:01 mtrojnar

See the related #378 (comment) for a way to implement this functionality without changing the public API. We really should avoid adding new functions for each key type and use the EVP_PKEY interface instead. Could you please update your PR?

How would you do the ID and label assignments with that API?

vesajaaskelainen avatar Jan 07 '21 07:01 vesajaaskelainen

See the related #378 (comment) for a way to implement this functionality without changing the public API. We really should avoid adding new functions for each key type and use the EVP_PKEY interface instead. Could you please update your PR?

How would you do the ID and label assignments with that API?

Could we (mis)use EVP_PKEY_CTX_set_app_data() for this purpose? https://www.openssl.org/docs/manmaster/man3/EVP_PKEY_keygen.html

mtrojnar avatar Apr 15 '21 17:04 mtrojnar

See the related #378 (comment) for a way to implement this functionality without changing the public API. We really should avoid adding new functions for each key type and use the EVP_PKEY interface instead. Could you please update your PR?

How would you do the ID and label assignments with that API?

Could we (mis)use EVP_PKEY_CTX_set_app_data() for this purpose? https://www.openssl.org/docs/manmaster/man3/EVP_PKEY_keygen.html

Hmm...

With: https://www.openssl.org/docs/man1.1.0/man1/genpkey.html

There is option -pkeyopt opt:value which allows one to set number of bits for RSA and also select curve for EC.

  • rsa_keygen_bits:numbits
  • ec_paramgen_curve:curve

If this could be expanded for something like:

  • pkcs11_cka_id:<string> or pkcs11_cka_id_hex:<hex string>
  • pkcs11_cka_label:<string>

I try to to experiment with the concept.

vesajaaskelainen avatar Apr 19 '21 07:04 vesajaaskelainen

Finally got more time to look at this.

In theory it is easy until you look under the hood :raised_eyebrow:.

These pages gives some examples of keygen usage: https://www.openssl.org/docs/manmaster/man7/EVP_KEYMGMT-RSA.html https://www.openssl.org/docs/manmaster/man7/EVP_KEYMGMT-EC.html

Basically the problem is that where to store temporary the options.

Planned to start with RSA keygen but even that has an issue on how to get numbits out. OpenSSL provides setter for the value (which internally does ctrl) but no getter.

I can easily hook into ctrl commands and capture those arguments. But there is nowhere to store it easily.

Our playground here for key generation is EVP_PKEY_CTX and what is being done in this openssl engine is that it uses both software engine (orig) and then pkcs11 engine together. If it does not fall in context of pkcs11 engine then operation is forwarded to orig engine.

Now back to RSA keygen issue.

EVP_PKEY_CTX does have data which is used by internal software engine (rsa_pmeth). Internal engine allocates private structure of data and stores options in there. This would be perfect way to do it also here BUT in here we are transparently calling back to pkcs11 engine and software engine which makes it a bit awkward to use data for pkcs11 engine's purpose.

Then one asks why current implementation then works in signing as it somehow must have reference to Cryptoki stuff. It uses ex data (RSA_set_ex_data and friends) mechanism of RSA_KEY or EC_KEY structures -- and we do not yet have *_KEY created.

Have been thinking different hackish methods to abuse the stuff a bit but there does not seem to be a good place without a danger that some other engine would do the same thing and then we would have random crashes which no one wants to have.

In theory one could become full blow engine and use Cryptoki to fuller extend but that kinda makes it harder to utilize transparency what the current method allows. Eg. use software engine for verify operation with public key and also encrypt with public key then there are bunch of other functions that would be better left as software solutions as those are often faster.

One could make shadowed EVP_PKEY_CTX that would be reserved for "inner" crypto. But that would require one to proxy all methods -- generates quite a bit boilerplate code and I believe openssl 3.0 brings even more methods to handle. Eg. needs to be openssl version specific.

Then alternative for that would be some kind of shared map with EVP_KEY_CTX being the key and value would be internal structure. This map would also require locking to keep its integrity.

If one would want to use openssl genpkey it seem to have mandatory -out argument which is a bit problematic. In theory one could extend openssl genpkey with -outform ENGINE and then have special handling in openssl code.

@mtrojnar do you have any recommendations on which way to go and try?

And if you think that map method would be way to go then if you have preferences which kind of map/which implementation to use.

vesajaaskelainen avatar Jul 04 '21 06:07 vesajaaskelainen

See the related #378 (comment) for a way to implement this functionality without changing the public API. We really should avoid adding new functions for each key type and use the EVP_PKEY interface instead. Could you please update your PR?

How would you do the ID and label assignments with that API?

Could we (mis)use EVP_PKEY_CTX_set_app_data() for this purpose? https://www.openssl.org/docs/manmaster/man3/EVP_PKEY_keygen.html

And about the app data -- I believe it should be left for user application as they may have C++ class or so where this would be needed. Especially if one does callbacks or so.

vesajaaskelainen avatar Jul 04 '21 10:07 vesajaaskelainen

Have been thinking different hackish methods to abuse the stuff a bit but there does not seem to be a good place without a danger that some other engine would do the same thing and then we would have random crashes which no one wants to have.

The ex_data is a stack so each application or lib can hang data off the same structure. See: https://www.openssl.org/docs/man1.1.1/man3/RSA_set_ex_data.html https://www.openssl.org/docs/man1.1.1/man3/CRYPTO_get_ex_new_index.html

Depending on your timing you may want to target OpenSSL-3.0.

OpenSSL-3.0 adds EVP_KEY to the list of structures that can have ex_data: https://www.openssl.org/docs/manmaster/man3/CRYPTO_get_ex_new_index.html

It also has "functions to create keys and key parameters from user data": https://www.openssl.org/docs/manmaster/man3/EVP_PKEY_fromdata.html

Here is an example from an OpenSC card driver for an EC public key: https://github.com/dengert/OpenSC/blob/PIV-4-extensions/src/libopensc/card-piv.c#L1922-L1935

dengert avatar Jul 04 '21 11:07 dengert

@dengert Which method do you recommend for supplying additional parameters?

mtrojnar avatar Mar 20 '22 21:03 mtrojnar

@mtrojnar I am not sure what you are asking: "Which method do you recommend for supplying additional parameters?

If you mean the difference of the OpenSC pkcs11-tool or NSS's certutil, I prefer the OpenSC. If you are asking about OpenSSL 3.0 or the ex_data? or OSSL_ stuff?

The reference for card-piv.c has changed to: https://github.com/dengert/OpenSC/blob/PIV-4-extensions/src/libopensc/card-piv.c#L1961-L1991 and was showing how to set ECC curve into pubkey to do a verify with 1.1.1 or 3.0 as examples. Thi the PIV case would use only P256 or P384 but the that could be expanded and maybe if the OID of the curve was provided, that might be usable instead. I dont think the the ex_data would be needed, but it is a way to hang application data off or OpenSSL structures.

dengert avatar Mar 20 '22 22:03 dengert

@dengert Good point. My question was too generic. Let's focus on the task at hand. Should we merge this PR as it is, or should we look for an alternative approach?

mtrojnar avatar Mar 22 '22 17:03 mtrojnar

This PR is over a years old and it is adding pkcs11_generate_ec_key Do you need to define a pkcs11_generate_rsa_key and also allow for other key types in the future? Or should a mechanism be a separate parameter to a new version of pkcs11_generate_key.

EC keys are created for use with ECDSA or ECDH, but not usually both. The templates are setting both. Do you want to to allow separate usage bits? (Although to generate a certificate request, the key needs to sign it) even if the key is only going to be used with ECDH. And certificate also contains keyUsage bits that also specify how the key will be used.

dengert avatar Mar 22 '22 19:03 dengert

@dengert I prefer adding something like PKCS11_generate_key_ex() with a flexible set of parameters over a separate API function for each key type. I also agree that we need a parameter for key usage (I hope I understood you correctly).

I understand this PR is quite old. I was busy with other projects. I'm going to have more time for libp11 now.

mtrojnar avatar Mar 22 '22 19:03 mtrojnar

Sounds good

On Tue, Mar 22, 2022, 2:38 PM Michał Trojnara @.***> wrote:

@dengert https://github.com/dengert I prefer adding something like PKCS11_generate_key_ex() with a flexible set of parameters over a separate API function for each key type. I also agree that we need a parameter for key usage (I hope I understood you correctly).

I understand this PR is quite old. I was busy with other projects. I'm going to have more time for libp11 now.

— Reply to this email directly, view it on GitHub https://github.com/OpenSC/libp11/pull/379#issuecomment-1075560237, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAGTIMIATYZ4OKFYW47XFU3VBIOTPANCNFSM4UWZ234A . You are receiving this because you were mentioned.Message ID: @.***>

dengert avatar Mar 22 '22 19:03 dengert

Hey @mtrojnar, are there any plans on implementing keygen functionality?

I went with your suggestion by (mis)using app_data from EVP_PKEY_CTX for passing key label, id and other things from OpenSSL to the engine since there seems to be no other option as EVP_PKEY_* does not have ex_data in OpenSSL 1.1. This can be done relatively cleanly but I struggle with finding a way to get the right PKCS11_CTX_private for the CRYPTOKI_call. I was thinking about finding the right slot by the token label (using PKCS11_enumerate_slots) and then fetching whatever I need from PKCS11_SLOT_private but I don't like that because I'm using public API for internal use. Do you have any suggestions?

istepic avatar Aug 31 '22 08:08 istepic