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

Assigned To

Authored By

	szszszsz-nitrokey
	Dec 1 2020, 2:14 PM

Description

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"

Environment

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

Connected

Logs

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

scdaemon.log660 KBDownload

scdaemon.log.filtered158 KBDownload

p521@test-sec-armor.asc1 KBDownload

p521@test-sec.asc1 KBDownload

run-full.log27 KBDownload

run-import-only.log11 KBDownload

Revisions and Commits

rG GnuPG
	rG5b1806454c03 scd: Fix zero-byte handling in ECC.
	rG5a03bf61304d scd:openpgp: Fix writing ECC key to card.
	rGa25c99b156ca scd:openpgp: Fix writing ECC key to card.

Related Objects

Mentioned In: T5405: Release GnuPG 2.3.2
T5153: Release GnuPG 2.2.26

Event Timeline

szszszsz-nitrokey created this task.Dec 1 2020, 2:14 PM

szszszsz-nitrokey created this object in space S1 Public.

szszszsz-nitrokey updated the task description. (Show Details)Dec 1 2020, 2:23 PM

• werner added projects: Bug Report, scd, gnupg (gpg22).Dec 1 2020, 8:49 PM

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

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 **),
   else
     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);
 
  leave:
+  xfree (ecc_d_padded);
   gcry_mpi_release (oid);
   return err;
 }

• gniibe triaged this task as High priority.Dec 2 2020, 4:07 AM

You better wipe ecc_d_padded or use xtrymalloc_secure.

In T5163#139750, @werner wrote:

You better wipe ecc_d_padded or use xtrymalloc_secure.

Ah, yes. Thanks for your good catch.

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 added a commit: rGa25c99b156ca: scd:openpgp: Fix writing ECC key to card..Dec 3 2020, 7:07 AM