Are we ready for 1.7 ?

NIIBE Yutaka gniibe at fsij.org
Fri Apr 8 10:31:20 CEST 2016


On 04/08/2016 03:53 PM, Werner Koch wrote:
> I don't fully understand.  gcry_mpi_ec_mul_point does not do any
> decoding at all.  This should be done by _gcry_ecc_mont_decodepoint for
> Curve25519.

Actually, I don't fully understand how to describe this correctly.

In the previous mail, I refered the function decodeScalar25519 in RFC
7748.  Note that this is for the scalar value.  In RFC 7748, the tweak
of bits is done when decoding octets to integer.

Our _gcry_ecc_mont_decodepoint does computation which is described as
decodeUCoordinate in RFC 7748.

> Our processing model is a but different so that a 1-1 relationship with
> other code is not necessary needed.  But I may have not understand the
> issue.  A patch may help me to understand it better.

Yes.  Our processing model is a bit different.  Currently, the caller
of gcry_mpi_ec_mul needs handling of tweak.

I encountered this issue when I wrote tests/t-cv25519.c.  If we keep
current API, the user would be confused.  That's my concern.

Here is a patch to show the possible change.  It's not mature yet.

diff --git a/cipher/ecc.c b/cipher/ecc.c
index 759ca42..7ab7244 100644
--- a/cipher/ecc.c
+++ b/cipher/ecc.c
@@ -356,7 +356,7 @@ test_ecdh_only_keys (ECC_secret_key *sk, unsigned int nbits, int flags)
       _gcry_mpi_randomize (test, nbits, GCRY_WEAK_RANDOM);
     }

-  ec = _gcry_mpi_ec_p_internal_new (pk.E.model, pk.E.dialect, 0,
+  ec = _gcry_mpi_ec_p_internal_new (pk.E.model, pk.E.dialect, flags,
                                     pk.E.p, pk.E.a, pk.E.b);
   x0 = mpi_new (0);
   x1 = mpi_new (0);
@@ -592,7 +592,7 @@ ecc_generate (const gcry_sexp_t genparms, gcry_sexp_t *r_skey)
       log_printpnt ("ecgen curve G", &E.G, NULL);
     }

-  ctx = _gcry_mpi_ec_p_internal_new (E.model, E.dialect, 0, E.p, E.a, E.b);
+  ctx = _gcry_mpi_ec_p_internal_new (E.model, E.dialect, flags, E.p, E.a, E.b);

   if (E.model == MPI_EC_MONTGOMERY)
     rc = nist_generate_key (&sk, &E, ctx, flags, nbits, &Qx, NULL);
@@ -830,7 +830,7 @@ ecc_check_secret_key (gcry_sexp_t keyparms)
       goto leave;
     }

-  ec = _gcry_mpi_ec_p_internal_new (sk.E.model, sk.E.dialect, 0,
+  ec = _gcry_mpi_ec_p_internal_new (sk.E.model, sk.E.dialect, flags,
                                     sk.E.p, sk.E.a, sk.E.b);

   if (mpi_q)
@@ -1349,7 +1349,7 @@ ecc_encrypt_raw (gcry_sexp_t *r_ciph, gcry_sexp_t s_data, gcry_sexp_t keyparms)
     }

   /* Compute the encrypted value.  */
-  ec = _gcry_mpi_ec_p_internal_new (pk.E.model, pk.E.dialect, 0,
+  ec = _gcry_mpi_ec_p_internal_new (pk.E.model, pk.E.dialect, flags,
                                     pk.E.p, pk.E.a, pk.E.b);

   /* Convert the public key.  */
@@ -1570,7 +1570,7 @@ ecc_decrypt_raw (gcry_sexp_t *r_plain, gcry_sexp_t s_data, gcry_sexp_t keyparms)
     }


-  ec = _gcry_mpi_ec_p_internal_new (sk.E.model, sk.E.dialect, 0,
+  ec = _gcry_mpi_ec_p_internal_new (sk.E.model, sk.E.dialect, flags,
                                     sk.E.p, sk.E.a, sk.E.b);

   /*
diff --git a/mpi/ec.c b/mpi/ec.c
index 26dd947..b253f72 100644
--- a/mpi/ec.c
+++ b/mpi/ec.c
@@ -29,6 +29,7 @@
 #include "context.h"
 #include "ec-context.h"
 #include "ec-internal.h"
+#include "cipher.h"


 #define point_init(a)  _gcry_mpi_point_init ((a))
@@ -1313,7 +1314,18 @@ _gcry_mpi_ec_mul_point (mpi_point_t result,
          Note that we don't use Y-coordinate in the points at all.
          RESULT->Y will be filled by zero.  */

-      nbits = mpi_get_nbits (scalar);
+      k  = mpi_copy (scalar);
+      if ((ctx->flags & PUBKEY_FLAG_DJB_TWEAK))
+	{
+	  nbits = mpi_get_nbits (ctx->p);
+	  mpi_set_bit (k, nbits - 1);
+	  mpi_clear_bit (k, 2);
+	  mpi_clear_bit (k, 1);
+	  mpi_clear_bit (k, 0);
+	}
+      else
+	nbits = mpi_get_nbits (scalar);
+
       point_init (&p1);
       point_init (&p2);
       point_init (&p1_);
@@ -1337,7 +1349,7 @@ _gcry_mpi_ec_mul_point (mpi_point_t result,
         {
           mpi_point_t t;

-          sw = mpi_test_bit (scalar, j);
+          sw = mpi_test_bit (k, j);
           point_swap_cond (q1, q2, sw, ctx);
           montgomery_ladder (prd, sum, q1, q2, point->x, ctx);
           point_swap_cond (prd, sum, sw, ctx);
@@ -1367,6 +1379,7 @@ _gcry_mpi_ec_mul_point (mpi_point_t result,
       point_free (&p2);
       point_free (&p1_);
       point_free (&p2_);
+      mpi_free (k);
       return;
     }

diff --git a/tests/t-cv25519.c b/tests/t-cv25519.c
index 69aa005..ee4b910 100644
--- a/tests/t-cv25519.c
+++ b/tests/t-cv25519.c
@@ -192,7 +192,6 @@ test_cv (int testno, const char *k_str, const char *u_str,
   gpg_error_t err;
   void *buffer = NULL;
   size_t buflen;
-  unsigned char *p;
   gcry_sexp_t s_sk = NULL;
   gcry_sexp_t s_data = NULL;
   gcry_sexp_t s_result = NULL;
@@ -210,13 +209,6 @@ test_cv (int testno, const char *k_str, const char *u_str,
       goto leave;
     }

-  /*
-   * Do what decodeScalar25519 does.
-   */
-  p = (unsigned char *)buffer;
-  p[0] &= 248;
-  p[31] &= 127;
-  p[31] |= 64;
   reverse_buffer (buffer, buflen);

   if ((err = gcry_sexp_build (&s_sk, NULL,
@@ -319,14 +311,21 @@ test_it (int testno, const char *k_str, int iter, const char *result_str)
   gcry_mpi_t mpi_k = NULL;
   gcry_mpi_t mpi_x = NULL;
   gcry_mpi_point_t P = NULL;
-  gcry_mpi_point_t Q;
+  gcry_mpi_point_t Q = NULL;
   int i;
-  gcry_mpi_t mpi_kk = NULL;
+  gcry_sexp_t s_flags = NULL;

   if (verbose > 1)
     show ("Running test %d: iteration=%d\n", testno, iter);

-  gcry_mpi_ec_new (&ctx, NULL, "Curve25519");
+  if ((err = gcry_sexp_build (&s_flags, NULL, "(flags djb-tweak)")))
+    {
+      fail ("error building s-exp for test %d, %s: %s",
+            testno, "flags", gpg_strerror (err));
+      goto leave;
+    }
+
+  gcry_mpi_ec_new (&ctx, s_flags, "Curve25519");
   Q = gcry_mpi_point_new (0);

   if (!(buffer = hex2buffer (k_str, &buflen)) || buflen != 32)
@@ -357,14 +356,7 @@ test_it (int testno, const char *k_str, int iter, const char *result_str)
       /*
        * Another variant of decodeScalar25519 thing.
        */
-      mpi_kk = gcry_mpi_set (mpi_kk, mpi_k);
-      gcry_mpi_set_bit (mpi_kk, 254);
-      gcry_mpi_clear_bit (mpi_kk, 255);
-      gcry_mpi_clear_bit (mpi_kk, 0);
-      gcry_mpi_clear_bit (mpi_kk, 1);
-      gcry_mpi_clear_bit (mpi_kk, 2);
-
-      gcry_mpi_ec_mul (Q, mpi_kk, P, ctx);
+      gcry_mpi_ec_mul (Q, mpi_k, P, ctx);

       P = gcry_mpi_point_set (P, mpi_k, NULL, GCRYMPI_CONST_ONE);
       gcry_mpi_ec_get_affine (mpi_k, NULL, Q, ctx);
@@ -401,7 +393,7 @@ test_it (int testno, const char *k_str, int iter, const char *result_str)
   }

  leave:
-  gcry_mpi_release (mpi_kk);
+  gcry_sexp_release (s_flags);
   gcry_mpi_release (mpi_k);
   gcry_mpi_point_release (P);
   gcry_mpi_release (mpi_x);
-- 



More information about the Gcrypt-devel mailing list