Page MenuHome GnuPG
Create Task
Maniphest T3781

ECC encryption key on-card generation broken
Closed, ResolvedPublic
Actions

Description

Commits ea09b6cded9d31a8ebd91878553c3eaa2b76e817 and acb300543422c660c87ac2f0211a42f792a65cc4 break the possibility to generate ECC encryption key on-card when any curve other than 25519 is used.

The problem comes from the fact that PUBKEY_ALGO_ECDH is both used for 25519 and non-25519 curves.
The problem does not appear for signature and authentication keys as PUBKEY_ALGO_EDDSA is used for 25519 and PUBKEY_ALGO_ECDSA for non-25519 curves. The patch below fixes the problem, but will break the "magic trick" introduced for GnuK...

A possible solution is to use another constant such as PUBKEY_ALGO_EDDH (with a new value to identify this case such as 23 for instance...) for encryption with 25519, and leave PUBKEY_ALGO_ECDH for non-25519 curves, as previously.

Index: gnupg/g10/card-util.c
===================================================================
--- gnupg.orig/g10/card-util.c
+++ gnupg/g10/card-util.c
@@ -1486,7 +1486,7 @@ generate_card_keys (ctrl_t ctrl)
       for (keyno = 0; keyno < DIM (info.key_attr); keyno++)
         {
           if (info.key_attr[keyno].algo == PUBKEY_ALGO_RSA
-              || info.key_attr[keyno].algo == PUBKEY_ALGO_ECDH
+              //|| info.key_attr[keyno].algo == PUBKEY_ALGO_ECDH
               || info.key_attr[keyno].algo == PUBKEY_ALGO_EDDSA)
             {
               if (info.key_attr[keyno].algo == PUBKEY_ALGO_RSA)
@@ -1572,7 +1572,7 @@ card_generate_subkey (ctrl_t ctrl, kbnod
   if (info.is_v2 && info.extcap.aac)
     {
       if (info.key_attr[keyno-1].algo == PUBKEY_ALGO_RSA
-          || info.key_attr[keyno].algo == PUBKEY_ALGO_ECDH
+          //|| info.key_attr[keyno].algo == PUBKEY_ALGO_ECDH
           || info.key_attr[keyno].algo == PUBKEY_ALGO_EDDSA)
         {
           unsigned int nbits;

Revisions and Commits

rG GnuPG
rG820380335a20 g10: Add "key-attr" command for --card-edit.
rGa1515b3bbc10 g10: Support key attribute change at --card-edit/generate.

Related Objects

Event Timeline

Arnaud created this task.Feb 6 2018, 6:10 PM
Arnaud updated the task description. (Show Details)
Arnaud updated the task description. (Show Details)Feb 6 2018, 6:12 PM
• gniibe claimed this task.Feb 13 2018, 5:33 AM
• werner added a subscriber: • werner.Mar 5 2018, 9:39 AM

So should we revert this patch and replace it by an explicit command to switch the card to ECC?

• werner added a comment.Mar 5 2018, 9:40 AM

This has also the advantage that we could list the possible curves and let the user select them.

Arnaud added a comment.Mar 5 2018, 11:05 AM

This would be a good solution.

• gniibe added a commit: rGa1515b3bbc10: g10: Support key attribute change at --card-edit/generate..Mar 29 2018, 5:06 AM
• gniibe triaged this task as Normal priority.Mar 29 2018, 6:10 AM

I changed the interaction so that user can specify RSA or ECC, then when it's for ECC, specifying curve.

• gniibe added a commit: rG820380335a20: g10: Add "key-attr" command for --card-edit..Mar 30 2018, 4:04 AM
• gniibe added a comment.Mar 30 2018, 4:52 AM

Furthermore, I changed to have an explicit command: key-attr

• gniibe changed the task status from Open to Testing.Mar 30 2018, 4:52 AM
• gniibe mentioned this in T3299: scd: support key algorithm attribute change UI.Apr 9 2018, 7:27 AM
• gniibe closed this task as Resolved.Apr 11 2018, 1:58 AM

Fixed in 2.2.6.

nitroalex added a subscriber: nitroalex.Apr 12 2018, 3:30 PM

Hi,

works just fine, thx!

I wondered if we could enable brainpoolP384r1 or alike for non-expert mode as well, as this is one of the two possible curves for OpenPGP Card v3.

It could look like:

Please select which elliptic curve you want:

(1) Curve 25519
(4) NIST P-384
(7) Brainpool P-384

Kind regards
Alex

• werner added a comment.Apr 13 2018, 12:53 PM

Neither Brainpool nor NIST curves make any sense unless there is an organizational policy requirement. Thus the --expert requirement is the Right Thing (tm).

nitroalex added a comment.Apr 19 2018, 2:27 PM

Well, I surely would agree (and this is only a proposal anyway), but my point here is, that OpenPGP Card does not support Curve 25519, so that one *have to* choose between those other two. Considering me a tinfoil hat person, I would rather not choose NIST, as many others wouldn't too.

That is the reason I'd like to see Brainpool here as well, so that OpenPGP Card v3 users could choose between NIST and Brainpool, their only two options (at least for curves)...

• gniibe added a comment.Apr 20 2018, 10:10 AM

@nitroalex Perhaps, creating new ticker is better for this topic.
In the current OpenPGP card specification, there is no way for an application (except having a list of card implementation information) to know wich algo and which curve is supported or not.
So, what an application does is try and error.
I don't like this situation, but I don't know how we can modify the specification.