ecc: Montgomery curve always uses the prefix 0x40

[NIIBE Yutaka]

On 11/16/2015 12:20 PM, NIIBE Yutaka wrote:
> I'm checking GnuPG's Curve25519 implementation.
[..]
> I think that it should be prefixed by 0x40, so that it can be also
> accessed as a fixed size MPI.

Here is update.  This can handle old data with no prefix, too.


diff --git a/cipher/ecc-misc.c b/cipher/ecc-misc.c
index 2f2e593..b9ad060 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,27 @@ _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 older implementation (non-released development version),
+       * it is possibe to have the 0x40 as a part of data.
+       *
+       * So, we need to check if it's really the prefix or not.
+       * Only when it's the prefix, we remove it.
+       */
+      if (a[0] == 0x40 && ctx->nbits/8 == rawmpilen - 1)
+        rawmpi++;
     }

   _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);
           }
--