ecc: CHANGE point representation of Curve25519.

* cipher/ecc-misc.c (_gcry_ecc_mont_decodepoint): Decode point with
the prefix 0x40, additional 0x00 by MPI handling, and shorter octets
by MPI normalization.
* cipher/ecc.c (ecc_generate, ecc_encrypt_raw, ecc_decrypt_raw):
Always add the prefix 0x40.

--

Curve25519 native little-endian point representation is not friendly
to existing practice of OpenPGP code, where MPI is assumed.  MPI
handling might insert 0x00 in the beginning to avoid sign confusion.
MPI handling also might remove 0x00s in the front.  So, it is safe
to put the prefix 0x40.

While we support old point representation of no prefix in
ecc_mont_decodepoint, new libgcrypt always put the prefix.

2 files changed, 52 insertions, 17 deletions

diff --git a/cipher/ecc-misc.c b/cipher/ecc-misc.c
index 2f2e5937..67e3b3d8 100644
--- a/cipher/ecc-misc.c
+++ b/cipher/ecc-misc.c
@@ -292,6 +292,7 @@ _gcry_ecc_compute_public (mpi_point_t Q, mpi_ec_t ec,
 gpg_err_code_t
 _gcry_ecc_mont_decodepoint (gcry_mpi_t pk, mpi_ec_t ctx, mpi_point_t result)
 {
+  unsigned char *a;
   unsigned char *rawmpi;
   unsigned int rawmpilen;
 
@@ -311,8 +312,8 @@ _gcry_ecc_mont_decodepoint (gcry_mpi_t pk, mpi_ec_t ctx, mpi_point_t result)
           buf++;
         }
 
-      rawmpi = xtrymalloc (rawmpilen? rawmpilen:1);
-      if (!rawmpi)
+      a = rawmpi = xtrymalloc (rawmpilen? rawmpilen:1);
+      if (!a)
         return gpg_err_code_from_syserror ();
 
       p = rawmpi + rawmpilen;
@@ -321,16 +322,47 @@ _gcry_ecc_mont_decodepoint (gcry_mpi_t pk, mpi_ec_t ctx, mpi_point_t result)
     }
   else
     {
-      /* Note: Without using an opaque MPI it is not reliable possible
-         to find out whether the public key has been given in
-         uncompressed format.  Thus we expect native EdDSA format.  */
-      rawmpi = _gcry_mpi_get_buffer (pk, ctx->nbits/8, &rawmpilen, NULL);
-      if (!rawmpi)
+      a = rawmpi = _gcry_mpi_get_buffer (pk, ctx->nbits/8, &rawmpilen, NULL);
+      if (!a)
         return gpg_err_code_from_syserror ();
+      /*
+       * It is not reliable to assume that 0x40 means the prefix.
+       *
+       * For newer implementation, it is reliable since we always put
+       * 0x40 for x-only coordinate.
+       *
+       * For data with older implementation (non-released development
+       * version), it is possibe to have the 0x40 as a part of data.
+       * Besides, when data was parsed as MPI, we might have 0x00
+       * prefix.
+       *
+       * So, we need to check if it's really the prefix or not.
+       * Only when it's the prefix, we remove it.
+       */
+      if (ctx->nbits/8 == rawmpilen - 1)
+        rawmpi++;
+      else if (rawmpilen < ctx->nbits/8)
+        {/*
+          * It is possible for data created by older implementation
+          * to have shorter length when it was parsed as MPI.
+          */
+          unsigned int new_rawmpilen = ctx->nbits/8;
+
+          rawmpi = xtrymalloc (new_rawmpilen);
+          if (!rawmpi)
+            {
+              gpg_err_code_t err = gpg_err_code_from_syserror ();
+              xfree (a);
+              return err;
+            }
+
+          memset (rawmpi, 0, new_rawmpilen - rawmpilen);
+          memcpy (rawmpi + new_rawmpilen - rawmpilen, a, rawmpilen);
+        }
     }
 
   _gcry_mpi_set_buffer (result->x, rawmpi, rawmpilen, 0);
-  xfree (rawmpi);
+  xfree (a);
   mpi_set_ui (result->z, 1);
 
   return 0;
diff --git a/cipher/ecc.c b/cipher/ecc.c
index bd3e7549..51621f8e 100644
--- a/cipher/ecc.c
+++ b/cipher/ecc.c
@@ -606,17 +606,14 @@ ecc_generate (const gcry_sexp_t genparms, gcry_sexp_t *r_skey)
                                           &encpk, &encpklen);
       else
         {
-          int off = !!(flags & PUBKEY_FLAG_COMP);
-
-          encpk = _gcry_mpi_get_buffer_extra (Qx, ctx->nbits/8, off?-1:0,
+          encpk = _gcry_mpi_get_buffer_extra (Qx, ctx->nbits/8, -1,
                                               &encpklen, NULL);
           if (encpk == NULL)
             rc = gpg_err_code_from_syserror ();
           else
             {
-              if (off)
-                encpk[0] = 0x40;
-              encpklen += off;
+              encpk[0] = 0x40;
+              encpklen++;
             }
         }
       if (rc)
@@ -1374,11 +1371,13 @@ ecc_encrypt_raw (gcry_sexp_t *r_ciph, gcry_sexp_t s_data, gcry_sexp_t keyparms)
       mpi_s = _gcry_ecc_ec2os (x, y, pk.E.p);
     else
       {
-        rawmpi = _gcry_mpi_get_buffer (x, ec->nbits/8, &rawmpilen, NULL);
+        rawmpi = _gcry_mpi_get_buffer_extra (x, ec->nbits/8, -1,
+                                             &rawmpilen, NULL);
         if (!rawmpi)
           rc = gpg_err_code_from_syserror ();
         else
           {
+            rawmpi[0] = 0x40;
             mpi_s = mpi_new (0);
             mpi_set_opaque (mpi_s, rawmpi, rawmpilen*8);
           }
@@ -1393,11 +1392,13 @@ ecc_encrypt_raw (gcry_sexp_t *r_ciph, gcry_sexp_t s_data, gcry_sexp_t keyparms)
       mpi_e = _gcry_ecc_ec2os (x, y, pk.E.p);
     else
       {
-        rawmpi = _gcry_mpi_get_buffer (x, ec->nbits/8, &rawmpilen, NULL);
+        rawmpi = _gcry_mpi_get_buffer_extra (x, ec->nbits/8, -1,
+                                             &rawmpilen, NULL);
         if (!rawmpi)
           rc = gpg_err_code_from_syserror ();
         else
           {
+            rawmpi[0] = 0x40;
             mpi_e = mpi_new (0);
             mpi_set_opaque (mpi_e, rawmpi, rawmpilen*8);
           }
@@ -1587,11 +1588,13 @@ ecc_decrypt_raw (gcry_sexp_t *r_plain, gcry_sexp_t s_data, gcry_sexp_t keyparms)
         unsigned char *rawmpi;
         unsigned int rawmpilen;
 
-        rawmpi = _gcry_mpi_get_buffer (x, ec->nbits/8, &rawmpilen, NULL);
+        rawmpi = _gcry_mpi_get_buffer_extra (x, ec->nbits/8, -1,
+                                             &rawmpilen, NULL);
         if (!rawmpi)
           rc = gpg_err_code_from_syserror ();
         else
           {
+            rawmpi[0] = 0x40;
             r = mpi_new (0);
             mpi_set_opaque (r, rawmpi, rawmpilen*8);
           }