Incompatible Ed25519 secret key (no-encryption)
Open, NormalPublic

Description

In master, we use SOS encoding for ECC.

Since (traditional) non-SOS handling removes preceding zero-bytes, in the MPI representation, NBITS will be shorter than input, which causes checksum error.

This is an example key:

gniibe created this task.Wed, Nov 4, 3:20 AM

Note that there is no problem for encrypted key, because it is handled by opaque MPI.

gniibe added a comment.Wed, Nov 4, 3:57 AM

Applying following SOS-handling, the key can be handled.

diff --git a/g10/parse-packet.c b/g10/parse-packet.c
index 9cb254e24..be7fc6d67 100644
--- a/g10/parse-packet.c
+++ b/g10/parse-packet.c
@@ -188,6 +188,76 @@ mpi_read (iobuf_t inp, unsigned int *ret_nread, int secure)
 }
 
 
+/* Read an external representation of an SOS and return the opaque MPI
+   with GCRYMPI_FLAG_USER2.  The external format is a 16-bit unsigned
+   value stored in network byte order giving information for the
+   following octets.
+
+   The caller must set *RET_NREAD to the maximum number of bytes to
+   read from the pipeline INP.  This function sets *RET_NREAD to be
+   the number of bytes actually read from the pipeline.
+
+   If SECURE is true, the integer is stored in secure memory
+   (allocated using gcry_xmalloc_secure).  */
+static gcry_mpi_t
+sos_read (iobuf_t inp, unsigned int *ret_nread, int secure)
+{
+  int c, c1, c2, i;
+  unsigned int nmax = *ret_nread;
+  unsigned int nbits, nbytes;
+  size_t nread = 0;
+  gcry_mpi_t a = NULL;
+  byte *buf = NULL;
+  byte *p;
+
+  if (!nmax)
+    goto overflow;
+
+  if ((c = c1 = iobuf_get (inp)) == -1)
+    goto leave;
+  if (++nread == nmax)
+    goto overflow;
+  nbits = c << 8;
+  if ((c = c2 = iobuf_get (inp)) == -1)
+    goto leave;
+  ++nread;
+  nbits |= c;
+  if (nbits > MAX_EXTERN_MPI_BITS)
+    {
+      log_error ("mpi too large (%u bits)\n", nbits);
+      goto leave;
+    }
+
+  nbytes = (nbits + 7) / 8;
+  buf = secure ? gcry_xmalloc_secure (nbytes) : gcry_xmalloc (nbytes);
+  p = buf;
+  for (i = 0; i < nbytes; i++)
+    {
+      if (nread == nmax)
+        goto overflow;
+
+      c = iobuf_get (inp);
+      if (c == -1)
+        goto leave;
+
+      p[i] = c;
+      nread ++;
+    }
+
+  a = gcry_mpi_set_opaque (NULL, buf, nbits);
+  gcry_mpi_set_flag (a, GCRYMPI_FLAG_USER2);
+  *ret_nread = nread;
+  return a;
+
+ overflow:
+  log_error ("mpi larger than indicated length (%u bits)\n", 8*nmax);
+ leave:
+  *ret_nread = nread;
+  gcry_free(buf);
+  return a;
+}
+
+
 /* Register STRING as a known critical notation name.  */
 void
 register_known_notation (const char *string)
@@ -2728,7 +2798,12 @@ parse_key (IOBUF inp, int pkttype, unsigned long pktlen,
                   goto leave;
                 }
               n = pktlen;
-              pk->pkey[i] = mpi_read (inp, &n, 0);
+              if (algorithm == PUBKEY_ALGO_ECDSA
+                  || algorithm == PUBKEY_ALGO_EDDSA
+                  || algorithm == PUBKEY_ALGO_ECDH)
+                pk->pkey[i] = sos_read (inp, &n, 0);
+              else
+                pk->pkey[i] = mpi_read (inp, &n, 0);
               pktlen -= n;
               if (list_mode)
                 {

Need another patch to export it:

diff --git a/g10/export.c b/g10/export.c
index 8dd0b07d7..339424e19 100644
--- a/g10/export.c
+++ b/g10/export.c
@@ -627,6 +627,57 @@ canon_pk_algo (enum gcry_pk_algos algo)
 }
 
 
+/* Extract SOS representation from SEXP for PARAM, return the result
+   in R_SOS.  */
+static gpg_error_t
+sexp_extract_param_sos (gcry_sexp_t sexp, const char *param, gcry_mpi_t *r_sos)
+{
+  gpg_error_t err;
+  gcry_sexp_t l2 = gcry_sexp_find_token (sexp, param, 0);
+
+  *r_sos = NULL;
+  if (!l2)
+    err = gpg_error (GPG_ERR_NO_OBJ);
+  else
+    {
+      size_t buflen;
+      void *p0 = gcry_sexp_nth_buffer (l2, 1, &buflen);
+
+      if (!p0)
+        err = gpg_error_from_syserror ();
+      else
+        {
+          gcry_mpi_t sos;
+          unsigned int nbits = buflen*8;
+          unsigned char *p = p0;
+
+          if (*p && nbits >= 8 && !(*p & 0x80))
+            if (--nbits >= 7 && !(*p & 0x40))
+              if (--nbits >= 6 && !(*p & 0x20))
+                if (--nbits >= 5 && !(*p & 0x10))
+                  if (--nbits >= 4 && !(*p & 0x08))
+                    if (--nbits >= 3 && !(*p & 0x04))
+                      if (--nbits >= 2 && !(*p & 0x02))
+                        if (--nbits >= 1 && !(*p & 0x01))
+                          --nbits;
+
+          sos = gcry_mpi_set_opaque (NULL, p0, nbits);
+          if (sos)
+            {
+              gcry_mpi_set_flag (sos, GCRYMPI_FLAG_USER2);
+              *r_sos = sos;
+              err = 0;
+            }
+          else
+            err = gpg_error_from_syserror ();
+        }
+      gcry_sexp_release (l2);
+    }
+
+  return err;
+}
+
+
 /* Take a cleartext dump of a secret key in PK and change the
  * parameter array in PK to include the secret parameters.  */
 static gpg_error_t
@@ -764,9 +815,7 @@ cleartext_secret_key_to_openpgp (gcry_sexp_t s_key, PKT_public_key *pk)
         {
           gcry_mpi_release (pk->pkey[sec_start]);
           pk->pkey[sec_start] = NULL;
-          err = gcry_sexp_extract_param (key, NULL, "d",
-                                         &pk->pkey[sec_start],
-                                         NULL);
+          err = sexp_extract_param_sos (key, "d", &pk->pkey[sec_start]);
         }
 
       if (!err)
@@ -987,6 +1036,7 @@ transfer_format_to_openpgp (gcry_sexp_t s_pgp, PKT_public_key *pk)
           skey[skeyidx] = gcry_mpi_set_opaque (NULL, p, valuelen*8);
           if (!skey[skeyidx])
             goto outofmem;
+          gcry_mpi_set_flag (skey[skeyidx], GCRYMPI_FLAG_USER1);
         }
       else
         {
@@ -1144,7 +1194,7 @@ transfer_format_to_openpgp (gcry_sexp_t s_pgp, PKT_public_key *pk)
   /* Check that the first secret key parameter in SKEY is encrypted
      and that there are no more secret key parameters.  The latter is
      guaranteed by the v4 packet format.  */
-  if (!gcry_mpi_get_flag (skey[npkey], GCRYMPI_FLAG_OPAQUE))
+  if (!gcry_mpi_get_flag (skey[npkey], GCRYMPI_FLAG_USER1))
     goto bad_seckey;
   if (npkey+1 < DIM (skey) && skey[npkey+1])
     goto bad_seckey;
werner lowered the priority of this task from High to Normal.Mon, Nov 23, 1:54 PM