Cannot import NIST-P521 key to OpenPGP v3.3 smart card
Closed, ResolvedPublic


It seems to not always be possible to import the ECC NIST-P521 keys to all slots of the OpenPGP smart card.
I managed to import Authentication subkey to slot 3, but failed with other two.
I have searched for similar tickets, but not found any.

Before the import the slots attributes were set with scdaemon directly (as below), did not help either.

gpg-connect-agent "SCD SETATTR KEY-ATTR --force 1 19 nistp521"
gpg-connect-agent "SCD SETATTR KEY-ATTR --force 2 18 nistp521"
gpg-connect-agent "SCD SETATTR KEY-ATTR --force 3 19 nistp521"
  • GnuPG 2.2.25, built from sources.
  • Nitrokey Pro2 with OpenPGP 3.3.
root@stumpy:/app# gpg-connect-agent "GETINFO version"
D 2.2.25                                               
root@stumpy:/app# gpg --version
gpg (GnuPG) 2.2.25             
libgcrypt 1.8.7

Logs from scdaemon ("filtered" is with status updates removed) and run logs (short and full) attached.

szszszsz-nitrokey created this object in space S1 Public.
gniibe added a subscriber: gniibe.EditedDec 2 2020, 3:31 AM

In future, please try to minimize your log. Your log actually includes information of the session of keytocard before setting key attributes correctly.

IIUC, the success mode is that the length of private key data is 66-byte (which matches 521-bit); you can see in the log:

send apdu: c=00 i=DB p1=3F p2=FF lc=78 le=-1 em=0

In the failure mode, the length is shorter, as in:

send apdu: c=00 i=DB p1=3F p2=FF lc=77 le=-1 em=0

In GnuPG, for ECC of classic curves (NIST, Brainpool and seckp256), the private key format on disk uses standard MPI (as same as RSA).
When "keytocard", this MPI is sent to the card as is;
So, when the integer value represented is smaller, the representation is shorter.

It seems that for your implementation, padding would be needed. But I'm afraid of other implementations of OpenPGPcard.
I checked mine (Gnuk), and it requires padding when it is shorter. Oh, well.

gniibe claimed this task.Dec 2 2020, 3:32 AM
gniibe added a comment.Dec 2 2020, 4:06 AM

Here is a patch:

diff --git a/scd/app-openpgp.c b/scd/app-openpgp.c
index 7480f5041..04cd29fd9 100644
--- a/scd/app-openpgp.c
+++ b/scd/app-openpgp.c
@@ -3843,6 +3843,8 @@ ecc_writekey (app_t app, gpg_error_t (*pincb)(void*, const char *, char **),
   unsigned int n;
   size_t oid_len;
   unsigned char fprbuf[20];
+  size_t ecc_d_fixed_len;
+  unsigned char *ecc_d_padded = NULL;
   /* (private-key(ecc(curve%s)(q%m)(d%m))(created-at%d)):
      curve = "NIST P-256" */
@@ -3985,7 +3987,9 @@ ecc_writekey (app_t app, gpg_error_t (*pincb)(void*, const char *, char **),
     algo = PUBKEY_ALGO_ECDSA;
-  oidstr = openpgp_curve_to_oid (curve, NULL);
+  oidstr = openpgp_curve_to_oid (curve, &n);
+  ecc_d_fixed_len = (n+7)/8;
   err = openpgp_oid_from_str (oidstr, &oid);
   if (err)
     goto leave;
@@ -4032,6 +4036,21 @@ ecc_writekey (app_t app, gpg_error_t (*pincb)(void*, const char *, char **),
+  /* For private key material (except EdDSA), the MPI should be padded.  */
+  if ((keyno == 1 || !flag_djb_tweak)
+      && (ecc_d_len < ecc_d_fixed_len))
+    {
+      ecc_d_padded = xtrymalloc (ecc_d_fixed_len);
+      if (!ecc_d_padded)
+        {
+          err = gpg_error_from_syserror ();
+          goto leave;
+        }
+      memset (ecc_d_padded, 0, ecc_d_fixed_len - ecc_d_len);
+      memcpy (ecc_d_padded + ecc_d_fixed_len - ecc_d_len, ecc_d, ecc_d_len);
+      ecc_d_len = ecc_d_fixed_len;
+    }
   if (opt.verbose)
     log_info ("ECC private key size is %u bytes\n", (unsigned int)ecc_d_len);
@@ -4050,7 +4069,8 @@ ecc_writekey (app_t app, gpg_error_t (*pincb)(void*, const char *, char **),
       int exmode;
       err = build_ecc_privkey_template (app, keyno,
-                                        ecc_d, ecc_d_len,
+                                        ecc_d_padded? ecc_d_padded: ecc_d,
+                                        ecc_d_len,
                                         ecc_q, ecc_q_len,
                                         &template, &template_len);
       if (err)
@@ -4088,6 +4108,7 @@ ecc_writekey (app_t app, gpg_error_t (*pincb)(void*, const char *, char **),
                    ecc_q, ecc_q_len, ecdh_params (curve), (size_t)4);
+  xfree (ecc_d_padded);
   gcry_mpi_release (oid);
   return err;
gniibe triaged this task as High priority.Dec 2 2020, 4:07 AM
werner added a subscriber: werner.Dec 2 2020, 8:45 AM

You better wipe ecc_d_padded or use xtrymalloc_secure.

gniibe added a comment.Dec 2 2020, 9:07 AM

You better wipe ecc_d_padded or use xtrymalloc_secure.

Ah, yes. Thanks for your good catch.

gniibe added a comment.EditedDec 3 2020, 6:33 AM

I was wrong. Patch is being updated...

Problem here is somehow complicated. Not only recovering zeros, there is also another case we need to remove zero.

Except gnupg master with libgcrypt in master (of ECC SOS), gnupg may have classic ECC private key with malformed MPI (under OpenPGP term) having additional preceding zero by '%m' of gcry_sexp_build when it is imported from OpenPGP (see agent/cvt-openpgp.c of GnuPG 2.2). For this situation, we also need to remove an additional zero. (This problem is handled in gnupg master by agent/sexp-secret.c when it is read from file. For GnuPG 2.2, some work is needed.)

gniibe edited projects, added gnupg, backport; removed gnupg (gpg22).Dec 3 2020, 7:08 AM

Fixed in master. I will backport to 2.2.

gniibe edited projects, added Testing; removed backport.Dec 7 2020, 2:16 AM
gniibe changed the task status from Open to Testing.


gniibe closed this task as Resolved.Dec 25 2020, 8:24 AM