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

OpenPGP Card 3.3: pkcs11-tool does not work with ECC keys #1351

Closed
alex-nitrokey opened this issue May 7, 2018 · 8 comments · Fixed by #1506
Closed

OpenPGP Card 3.3: pkcs11-tool does not work with ECC keys #1351

alex-nitrokey opened this issue May 7, 2018 · 8 comments · Fixed by #1506

Comments

@alex-nitrokey
Copy link
Contributor

alex-nitrokey commented May 7, 2018

Problem Description

When using ECC key the pkcs11-tool -L command fails to detect Card.

Steps to reproduce

Set key usage to brainpoolP256r1 with gpg-connect-agent like this:

gpg-connect-agent "SCD SETATTR KEY-ATTR --force 1 19 brainpoolP256r1" /bye
gpg-connect-agent "SCD SETATTR KEY-ATTR --force 2 18 brainpoolP256r1" /bye

Restart pcscd, unplug and insert smartcard (in this case: Nitrokey Storage 2 with OpenPGP Card v3.3) and type

pkcs-tool -L

This results reproducible in:

$ pkcs11-tool -L -vvv
Available slots:
Slot 0 (0x0): Nitrokey Nitrokey Storage (0000000000000) 00 00
  manufacturer:  Nitrokey
  hardware ver:  0.0
  firmware ver:  0.0
  flags:         token present, removable device, hardware slot
C_GetTokenInfo() failed: rv = CKR_TOKEN_NOT_PRESENT

Any ideas what's happening here and how to fix?

PS: This does not happen if rsa-2048 or alike is used.

@frankmorgner
Copy link
Member

We don't currently have ECC support for any OpenPGP card, so the fix would be to implement it 😉

@alex-nitrokey
Copy link
Contributor Author

Well, that explains a lot 😄 The older Cards do not support ECC anyway.
Does OpenSC support curves for other cards yet? Or is ECC or totally new thing to implement?

The following OpenPGP based Cards support ECC:

Gnuk from firmware 1.2 on
OpenPGP Card devices from version 3 on

@frankmorgner
Copy link
Member

ECC is supported for some cards

@dengert
Copy link
Member

dengert commented May 24, 2018

Grep for brainpool. Looks like there is some support.

PKCS#11 v2.40 2.3.3 ECDSA public key objects and 2.3.4 Elliptic curve private key objects says: "The use of a namedCurve is recommended over the choice ecParameters. The choice implicitlyCA must not be used in Cryptoki" OpenSC only supports namedCurves. So your driver will have to map the OID to something internal.

OpenSC supports ECDSA, and ECDH. Grep for SC_ALGORITHM_ECDH_CDH_RAW
Grep for SC_ALGORITHM_ECDH_CDH_RAW and CKM_ECDH1_COFACTOR_DERIVE.
It only supports CKD_NULL.

@marschap
Copy link
Contributor

marschap commented Oct 7, 2018

@alex-nitrokey @frankmorgner @dengert @hongquan

I have started with a series of patches resulting in a first PR: #1498
This should be rather uncontroversial as it only "mechanically" changes code, but keeps the logic.

In addition, I have more patches on top of this PR in the branch OpenPGP-towards-EC of my forked repo.
Please feel free to comment there.
As all this is work in progress, I will re-create/rebase this branch as I see necessary.

@alex-nitrokey
Copy link
Contributor Author

🤔 Maybe I should have said, that I am currently working on it? :-/

Anyway, I will have a look at your PR and may add comments on how I did stuff.

@alex-nitrokey
Copy link
Contributor Author

alex-nitrokey commented Oct 9, 2018

Okay, I see. I did some changes similiar to yours to prepare the code, but also actually re-wrote a lot of the functions in card-openpgp.c for ECC (e.g. generation is already working).

I am not sure yet, how to proceed now with the two approaches and I probably should have done a preparation PR at first, like you did.

Please have a look at my branch. Mind that the code is not completely ready yet.

@frankmorgner
Copy link
Member

The PR is still open for review...

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

Successfully merging a pull request may close this issue.

4 participants