ecc: Montgomery curve always uses the prefix 0x40

NIIBE Yutaka gniibe at fsij.org
Mon Nov 16 04:20:56 CET 2015


Hello,

I'm checking GnuPG's Curve25519 implementation.

Currently, in the GnuPG implementation, ECDH shared point and
ephemeral public key (with x-coordinate only) is represented in native
little endian format with no prefix.

I think that it should be prefixed by 0x40, so that it can be also
accessed as a fixed size MPI.

Here is the change for libgcrypt, so that libgcrypt always uses the
prefix 0x40 for the representation of key for Montgomery curve
(regerdless of PUBKEY_FLAG_COMP flag).

(The change for GnuPG will be posted soon, after this patch will be
committed.)

OK to commit?

diff --git a/cipher/ecc-misc.c b/cipher/ecc-misc.c
index 2f2e593..79708f2 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,19 @@ _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.
+       * Now, we always put 0x40 for X-only coordinate, it is reliable.
+       */
+      if (a[0] == 0x40 && (rawmpilen%2))
+	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 4958fbb..6932489 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);
           }
--



More information about the Gcrypt-devel mailing list