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

Add PKCS11_generate_ec_key() for private EC key generation #379

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

vesajaaskelainen
Copy link

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);
    }

@@ -336,6 +336,10 @@ extern int pkcs11_generate_key(PKCS11_TOKEN * token,
int algorithm, unsigned int bits,
char *label, unsigned char* id, size_t id_len);

/* Generate and store a private EC key on the token */
extern int pkcs11_generate_ec_key(PKCS11_TOKEN * token, const char *curve,
char *label, unsigned char* id, size_t id_len);
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A question. We had internal discussion about the constness of label and id. As the pkcs11_generate_key() did not have the const decided not to add in here. But logically it would make library users life a bit easier if the signature would be:

extern int pkcs11_generate_ec_key(PKCS11_TOKEN * token, const char *curve,
	const char *label, const unsigned char* id, size_t id_len);

I understand that in the PKCS#11 world there is no concept of 'const' so from that design point of view it should be as what is now in the PR.

@hlounent
Copy link

LGTM based on testing with SoftHSMv2.

Signed-off-by: Vesa Jääskeläinen <vesa.jaaskelainen@vaisala.com>
@mtrojnar
Copy link
Member

mtrojnar commented Jan 6, 2021

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?

@vesajaaskelainen
Copy link
Author

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?

@mtrojnar
Copy link
Member

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

@vesajaaskelainen
Copy link
Author

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
Copy link
Author

Finally got more time to look at this.

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

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
Copy link
Author

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.

@dengert
Copy link
Member

dengert commented Jul 4, 2021

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

@mtrojnar
Copy link
Member

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

@dengert
Copy link
Member

dengert commented Mar 20, 2022

@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.

@mtrojnar
Copy link
Member

mtrojnar commented Mar 22, 2022

@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?

@dengert
Copy link
Member

dengert commented Mar 22, 2022

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.

@mtrojnar
Copy link
Member

@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.

@dengert
Copy link
Member

dengert commented Mar 22, 2022 via email

@istepic
Copy link

istepic commented Aug 31, 2022

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 pushed a commit to istepic/libp11 that referenced this pull request Sep 12, 2022
As discussed in OpenSC#379 and
OpenSC#378 we need a generic interface
that supports multiple algorithms for key generation. Attempt was made
to create a new keygen method and register it in PKCS11_pkey_meths()in
p11_pkey.c but multiple design issues appeared. How and where do you
pass the key ID, token label and alike was the first question. As
suggested by the maintainer here:
OpenSC#379 (comment),
app_data was (mis)used and that worked well. The reason why this
approach was abandoned is because a good (or bad) way to get a handle
of the PKCS11_CTX_private, that is necessary for the Cryptoki call,
was not found.
The way other operations work is that they rely on the key being
loaded _first_ through ENGINE_load_public(private)_key because this
is when the PKCS11_CTX gets initialized and a handle to
PKCS11_OBJECT_private gets set to the ex_data of the underlying key.
Key generation obviously cannot rely on that mechanism since key
doesn't yet exist.

Instead, a generic PKCS11_generate_key interface was made that
takes a structure describing the key generation algorithm. For now
it only contains simple options like curve name for ECC or number
of bits for RSA key generation. This interface can then be used
as any other PKCS11 wrapper interface or using the ENGINE control
commands. Using it with ENGINE control commands is demonstrated in
the new tests/keygen.c file.

Code for ECC keygen was taken from:
OpenSC#379 and reworked to compile and
work with some new additions to libp11 i.e. templates.
istepic pushed a commit to istepic/libp11 that referenced this pull request Sep 12, 2022
As discussed in OpenSC#379 and
OpenSC#378 we need a generic interface
that supports multiple algorithms for key generation. Attempt was made
to create a new keygen method and register it in PKCS11_pkey_meths() in
p11_pkey.c (so that it's possible to generate keys using OpenSSL's
EVP_PKEY_* API) but multiple design issues appeared. How and where do you
pass the key ID, token label and alike was the first question. As
suggested by the maintainer here:
OpenSC#379 (comment),
app_data from EVP_PKEY_CTX was (mis)used and that worked well. The
reason why this approach was abandoned is because a good (or bad) way
to get a handle of the PKCS11_CTX_private, that is necessary for the
Cryptoki call, was not found.
The way other operations work is that they rely on the key being
loaded *_first_* through ENGINE_load_public(private)_key because this
is when the PKCS11_CTX gets initialized and a handle to
PKCS11_OBJECT_private gets set to the ex_data of the underlying key.
Key generation obviously cannot rely on that mechanism since key
doesn't yet exist.

Instead, a generic PKCS11_generate_key interface was made that
takes a structure describing the key generation algorithm. For now
it only contains simple options like curve name for ECC or number
of bits for RSA key generation. This interface can then be used
as any other PKCS11 wrapper interface or using the ENGINE control
commands. Using it with ENGINE control commands is demonstrated in
the new tests/keygen.c file.

Code for ECC keygen was taken from:
OpenSC#379 and reworked to compile and
work with some new additions to libp11 i.e. templates.
istepic pushed a commit to istepic/libp11 that referenced this pull request Nov 17, 2022
As discussed in OpenSC#379 and
OpenSC#378 we need a generic interface
that supports multiple algorithms for key generation. Attempt was made
to create a new keygen method and register it in PKCS11_pkey_meths() in
p11_pkey.c (so that it's possible to generate keys using OpenSSL's
EVP_PKEY_* API) but multiple design issues appeared. How and where do you
pass the key ID, token label and alike was the first question. As
suggested by the maintainer here:
OpenSC#379 (comment),
app_data from EVP_PKEY_CTX was (mis)used and that worked well. The
reason why this approach was abandoned is because a good (or bad) way
to get a handle of the PKCS11_CTX_private, that is necessary for the
Cryptoki call, was not found.
The way other operations work is that they rely on the key being
loaded *_first_* through ENGINE_load_public(private)_key because this
is when the PKCS11_CTX gets initialized and a handle to
PKCS11_OBJECT_private gets set to the ex_data of the underlying key.
Key generation obviously cannot rely on that mechanism since key
doesn't yet exist.

Instead, a generic PKCS11_generate_key interface was made that
takes a structure describing the key generation algorithm. For now
it only contains simple options like curve name for ECC or number
of bits for RSA key generation. This interface can then be used
as any other PKCS11 wrapper interface or using the ENGINE control
commands. Using it with ENGINE control commands is demonstrated in
the new tests/keygen.c file.

Code for ECC keygen was taken from:
OpenSC#379 and reworked to compile and
work with some new additions to libp11 i.e. templates.
istepic added a commit to istepic/libp11 that referenced this pull request Dec 5, 2022
As discussed in OpenSC#379 and
OpenSC#378 we need a generic interface
that supports multiple algorithms for key generation. Attempt was made
to create a new keygen method and register it in PKCS11_pkey_meths() in
p11_pkey.c (so that it's possible to generate keys using OpenSSL's
EVP_PKEY_* API) but multiple design issues appeared. How and where do you
pass the key ID, token label and alike was the first question. As
suggested by the maintainer here:
OpenSC#379 (comment),
app_data from EVP_PKEY_CTX was (mis)used and that worked well. The
reason why this approach was abandoned is because a good (or bad) way
to get a handle of the PKCS11_CTX_private, that is necessary for the
Cryptoki call, was not found.
The way other operations work is that they rely on the key being
loaded *_first_* through ENGINE_load_public(private)_key because this
is when the PKCS11_CTX gets initialized and a handle to
PKCS11_OBJECT_private gets set to the ex_data of the underlying key.
Key generation obviously cannot rely on that mechanism since key
doesn't yet exist.

Instead, a generic PKCS11_generate_key interface was made that
takes a structure describing the key generation algorithm. For now
it only contains simple options like curve name for ECC or number
of bits for RSA key generation. This interface can then be used
as any other PKCS11 wrapper interface or using the ENGINE control
commands. Using it with ENGINE control commands is demonstrated in
the new tests/keygen.c file.

Code for ECC keygen was taken from:
OpenSC#379 and reworked to compile and
work with some new additions to libp11 i.e. templates.
istepic added a commit to istepic/libp11 that referenced this pull request Dec 5, 2022
* Introduce generic keypair generation interface and engine ctrl command

As discussed in OpenSC#379 and
OpenSC#378 we need a generic interface
that supports multiple algorithms for key generation. Attempt was made
to create a new keygen method and register it in PKCS11_pkey_meths() in
p11_pkey.c (so that it's possible to generate keys using OpenSSL's
EVP_PKEY_* API) but multiple design issues appeared. How and where do you
pass the key ID, token label and alike was the first question. As
suggested by the maintainer here:
OpenSC#379 (comment),
app_data from EVP_PKEY_CTX was (mis)used and that worked well. The
reason why this approach was abandoned is because a good (or bad) way
to get a handle of the PKCS11_CTX_private, that is necessary for the
Cryptoki call, was not found.
The way other operations work is that they rely on the key being
loaded *_first_* through ENGINE_load_public(private)_key because this
is when the PKCS11_CTX gets initialized and a handle to
PKCS11_OBJECT_private gets set to the ex_data of the underlying key.
Key generation obviously cannot rely on that mechanism since key
doesn't yet exist.

Instead, a generic PKCS11_generate_key interface was made that
takes a structure describing the key generation algorithm. For now
it only contains simple options like curve name for ECC or number
of bits for RSA key generation. This interface can then be used
as any other PKCS11 wrapper interface or using the ENGINE control
commands. Using it with ENGINE control commands is demonstrated in
the new tests/keygen.c file.

Code for ECC keygen was taken from:
OpenSC#379 and reworked to compile and
work with some new additions to libp11 i.e. templates.

* Add a comment that explains "type" member. Since openssl uses #defines
for identifying operations and libp11 already uses these identifiers,
we keep using these for consistency.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants