ecc: Montgomery curve always uses the prefix 0x40

[NIIBE Yutaka]

Hello,

I think that this is the final version.  This patch supports data
created older development version of libgcrypt.  It was possible that
the zero octets were removed when parsed as MPI.  This patch recovers
the value for such a data.


diff --git a/cipher/ecc-misc.c b/cipher/ecc-misc.c
index 2f2e593..67e3b3d 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 bd3e754..51621f8 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);
           }
--