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

Introduce keypair generation as engine ctrl command #474

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

Conversation

istepic
Copy link

@istepic istepic commented Sep 8, 2022

As discussed in #379 and
#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:
#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:
#379 and reworked to compile and
work with some new additions to libp11 i.e. templates.

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

istepic commented Oct 5, 2022

Hello @mtrojnar, does this approach to key generation seem plausible at all to you?

@ldts
Copy link
Contributor

ldts commented Nov 2, 2022

what is the status of this PR? has it been abandoned as well? It seems there was a previous attempt at implementing EC keygen which didn't prosper either (#379).

is there are reason not to pursue either of them?

@dengert
Copy link
Member

dengert commented Nov 3, 2022

what is the status of this PR? has it been abandoned as well? It seems there was a previous attempt at implementing EC keygen which didn't prosper either (#379).

is there are reason not to pursue either of them?

IMHO biggest issue with both of these is when will OpenSSL drops engine support and what happens to libp11.

mwasilew added a commit to foundriesio/lmp-device-register that referenced this pull request Nov 8, 2022
This patch uses experimental change in libp11 to generate EC keypairs
instead of RSA:2048:
OpenSC/libp11#474

The patches from this PR need to be built with libp11 for
lmp-device-register to compile.

Signed-off-by: Milosz Wasilewski <milosz.wasilewski@foundries.io>
@mwasilew
Copy link

Is there an alternative way of generating EC key pairs on the token? It is currently possible to generate EC keys with pkcs11-tool, but not libp11 API.

@istepic
Copy link
Author

istepic commented Nov 14, 2022

Is there an alternative way of generating EC key pairs on the token? It is currently possible to generate EC keys with pkcs11-tool, but not libp11 API.

No alternative way for EC keys.

istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
Patch: OpenSC/libp11#474

Rework docker setup to allow for adding files to the image.
This is not possible when using " - < " and reading from STDIN
because there is no Docker Build Context there which is necessary
for ADD and COPY commands.
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
Patch: OpenSC/libp11#474

Rework docker setup to allow for adding files to the image.
This is not possible when using " - < " and reading from STDIN
because there is no Docker Build Context there which is necessary
for ADD and COPY commands.
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
Patch: OpenSC/libp11#474

Rework docker setup to allow for adding files to the image.
This is not possible when using " - < " and reading from STDIN
because there is no Docker Build Context there which is necessary
for ADD and COPY commands.
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
Patch: OpenSC/libp11#474

Rework docker setup to allow for adding files to the image.
This is not possible when using " - < " and reading from STDIN
because there is no Docker Build Context there which is necessary
for ADD and COPY commands.
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
Patch: OpenSC/libp11#474

Rework docker setup to allow for adding files to the image.
This is not possible when using " - < " and reading from STDIN
because there is no Docker Build Context there which is necessary
for ADD and COPY commands.
istepic pushed a commit to istepic/MoCOCrW that referenced this pull request Nov 18, 2022
- Patch: OpenSC/libp11#474

- Reworked docker setup to allow adding files to the image.
  This is not possible when using " - < " and reading from STDIN
  because there is no Docker Build Context there which is necessary
  for ADD and COPY commands.

- Built libp11 the same way upstream builds it in their CI. This
  requires libtool package.
istepic added a commit to istepic/MoCOCrW that referenced this pull request Dec 6, 2022
- Patch: OpenSC/libp11#474

- Reworked docker setup to allow adding files to the image.
  This is not possible when using " - < " and reading from STDIN
  because there is no Docker Build Context there which is necessary
  for ADD and COPY commands.

- Built libp11 the same way upstream builds it in their CI. This
  requires libtool package.
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

4 participants