Extend the new EC interface and fix two bugs.
authorWerner Koch <wk@gnupg.org>
Tue, 19 Mar 2013 14:12:07 +0000 (15:12 +0100)
committerWerner Koch <wk@gnupg.org>
Tue, 19 Mar 2013 14:12:07 +0000 (15:12 +0100)
* src/ec-context.h (mpi_ec_ctx_s): Add field NEED_SYNC.
* mpi/ec.c (ec_p_sync): New.
(ec_p_init): Only set NEED_SYNC.
(_gcry_mpi_ec_set_mpi): Set NEED_SYNC for 'p' and 'a'.
(_gcry_mpi_ec_dup_point, _gcry_mpi_ec_add_points)
(_gcry_mpi_ec_mul_point): Call ec_p_sync.
(_gcry_mpi_ec_get_point): Recompute 'q' is needed.
(_gcry_mpi_ec_get_mpi): Ditto.  Also allow for names 'q', 'q.x',
'q.y', and 'g'.
* cipher/ecc.c (_gcry_mpi_ec_ec2os): New.

* cipher/ecc.c (_gcry_mpi_ec_new): Fix init from parameters 'Q'->'q',
'G'->'q'.
--

Note that the parameter names are all lowercase.  This patch fixes an
inconsistency.

The other bug was that changing the parameters D or A may have
resulted in wrong computations because helper variables were not
updated.  Now we delay the computation of those helper variables until
we need them.

cipher/ecc.c
doc/gcrypt.texi
mpi/ec.c
src/ec-context.h
src/mpi.h
tests/t-mpi-point.c

index c95a57a..c23ba08 100644 (file)
@@ -930,6 +930,27 @@ ec2os (gcry_mpi_t x, gcry_mpi_t y, gcry_mpi_t p)
 }
 
 
+/* Convert POINT into affine coordinates using the context CTX and
+   return a newly allocated MPI.  If the conversion is not possible
+   NULL is returned.  This function won't print an error message.  */
+gcry_mpi_t
+_gcry_mpi_ec_ec2os (gcry_mpi_point_t point, mpi_ec_t ectx)
+{
+  gcry_mpi_t g_x, g_y, result;
+
+  g_x = mpi_new (0);
+  g_y = mpi_new (0);
+  if (_gcry_mpi_ec_get_affine (g_x, g_y, point, ectx))
+    result = NULL;
+  else
+    result = ec2os (g_x, g_y, ectx->p);
+  mpi_free (g_x);
+  mpi_free (g_y);
+
+  return result;
+}
+
+
 /* RESULT must have been initialized and is set on success to the
    point given by VALUE.  */
 static gcry_error_t
@@ -1838,13 +1859,13 @@ _gcry_mpi_ec_new (gcry_ctx_t *r_ctx,
       errc = mpi_from_keyparam (&b, keyparam, "b");
       if (errc)
         goto leave;
-      errc = point_from_keyparam (&G, keyparam, "G");
+      errc = point_from_keyparam (&G, keyparam, "g");
       if (errc)
         goto leave;
       errc = mpi_from_keyparam (&n, keyparam, "n");
       if (errc)
         goto leave;
-      errc = point_from_keyparam (&Q, keyparam, "Q");
+      errc = point_from_keyparam (&Q, keyparam, "q");
       if (errc)
         goto leave;
       errc = mpi_from_keyparam (&d, keyparam, "d");
index 4d48eb4..a6b585d 100644 (file)
@@ -3953,7 +3953,10 @@ modified, it is suggested to pass @code{1} to @var{copy}, so that the
 function guarantees that a modifiable copy of the MPI is returned.  If
 @code{0} is used for @var{copy}, this function may return a constant
 flagged MPI.  In any case @code{gcry_mpi_release} needs to be called
-to release the result.  For valid names @ref{ecc_keyparam}.
+to release the result.  For valid names @ref{ecc_keyparam}.  If a
+point parameter is requested it is returned as an uncompressed encoded
+point.  If the public key @code{q} is requested but only the private
+key @code{d} is available, @code{q} will be recomputed on the fly.
 @end deftypefun
 
 @deftypefun gcry_mpi_point_t gcry_mpi_ec_get_point ( @
@@ -3965,7 +3968,9 @@ modified, it is suggested to pass @code{1} to @var{copy}, so that the
 function guarantees that a modifiable copy of the MPI is returned.  If
 @code{0} is used for @var{copy}, this function may return a constant
 flagged point.  In any case @code{gcry_mpi_point_release} needs to be
-called to release the result.
+called to release the result.  If the public key @code{q} is requested
+but only the private key @code{d} is available, @code{q} will be
+recomputed on the fly.
 @end deftypefun
 
 @deftypefun gpg_error_t gcry_mpi_ec_set_mpi ( @
index 9a6868b..0a348d2 100644 (file)
--- a/mpi/ec.c
+++ b/mpi/ec.c
@@ -337,6 +337,25 @@ ec_invm (gcry_mpi_t x, gcry_mpi_t a, mpi_ec_t ctx)
 }
 
 
+/* Sync changed data in the context.  */
+static void
+ec_p_sync (mpi_ec_t ec)
+{
+  gcry_mpi_t tmp;
+
+  if (!ec->t.need_sync)
+    return;
+
+  tmp = mpi_alloc_like (ec->p);
+  mpi_sub_ui (tmp, ec->p, 3);
+  ec->t.a_is_pminus3 = !mpi_cmp (ec->a, tmp);
+  mpi_free (tmp);
+
+  ec_invm (ec->t.two_inv_p, mpi_const (MPI_C_TWO), ec);
+  ec->t.need_sync = 0;
+}
+
+
 
 /* This function initialized a context for elliptic curve based on the
    field GF(p).  P is the prime specifying this field, A is the first
@@ -345,20 +364,14 @@ static void
 ec_p_init (mpi_ec_t ctx, gcry_mpi_t p, gcry_mpi_t a)
 {
   int i;
-  gcry_mpi_t tmp;
 
   /* Fixme: Do we want to check some constraints? e.g.  a < p  */
 
   ctx->p = mpi_copy (p);
   ctx->a = mpi_copy (a);
 
-  tmp = mpi_alloc_like (ctx->p);
-  mpi_sub_ui (tmp, ctx->p, 3);
-  ctx->t.a_is_pminus3 = !mpi_cmp (ctx->a, tmp);
-  mpi_free (tmp);
-
+  ctx->t.need_sync = 1;
   ctx->t.two_inv_p = mpi_alloc (0);
-  ec_invm (ctx->t.two_inv_p, mpi_const (MPI_C_TWO), ctx);
 
   /* Allocate scratch variables.  */
   for (i=0; i< DIM(ctx->t.scratch); i++)
@@ -492,10 +505,29 @@ _gcry_mpi_ec_get_mpi (const char *name, gcry_ctx_t ctx, int copy)
     return mpi_is_const (ec->n) && !copy? ec->n : mpi_copy (ec->n);
   if (!strcmp (name, "d") && ec->d)
     return mpi_is_const (ec->d) && !copy? ec->d : mpi_copy (ec->d);
+
+  /* Return a requested point coordinate.  */
   if (!strcmp (name, "g.x") && ec->G && ec->G->x)
     return mpi_is_const (ec->G->x) && !copy? ec->G->x : mpi_copy (ec->G->x);
   if (!strcmp (name, "g.y") && ec->G && ec->G->y)
     return mpi_is_const (ec->G->y) && !copy? ec->G->y : mpi_copy (ec->G->y);
+  if (!strcmp (name, "q.x") && ec->Q && ec->Q->x)
+    return mpi_is_const (ec->Q->x) && !copy? ec->Q->x : mpi_copy (ec->Q->x);
+  if (!strcmp (name, "q.y") && ec->Q && ec->Q->y)
+    return mpi_is_const (ec->G->y) && !copy? ec->Q->y : mpi_copy (ec->Q->y);
+
+  /* If a point has been requested, return it in standard encoding.  */
+  if (!strcmp (name, "g") && ec->G)
+    return _gcry_mpi_ec_ec2os (ec->G, ec);
+  if (!strcmp (name, "q"))
+    {
+      /* If only the private key is given, compute the public key.  */
+      if (!ec->Q && ec->d && ec->G && ec->p && ec->a)
+        _gcry_mpi_ec_mul_point (ec->Q, ec->d, ec->G, ec);
+
+      if (ec->Q)
+        return _gcry_mpi_ec_ec2os (ec->Q, ec);
+    }
 
   return NULL;
 }
@@ -510,8 +542,15 @@ _gcry_mpi_ec_get_point (const char *name, gcry_ctx_t ctx, int copy)
 
   if (!strcmp (name, "g") && ec->G)
     return point_copy (ec->G);
-  if (!strcmp (name, "q") && ec->Q)
-    return point_copy (ec->Q);
+  if (!strcmp (name, "q"))
+    {
+      /* If only the private key is given, compute the public key.  */
+      if (!ec->Q && ec->d && ec->G && ec->p && ec->a)
+        _gcry_mpi_ec_mul_point (ec->Q, ec->d, ec->G, ec);
+
+      if (ec->Q)
+        return point_copy (ec->Q);
+    }
 
   return NULL;
 }
@@ -527,11 +566,13 @@ _gcry_mpi_ec_set_mpi (const char *name, gcry_mpi_t newvalue,
     {
       mpi_free (ec->p);
       ec->p = mpi_copy (newvalue);
+      ec->t.need_sync = 1;
     }
   else if (!strcmp (name, "a"))
     {
       mpi_free (ec->a);
       ec->a = mpi_copy (newvalue);
+      ec->t.need_sync = 1;
     }
   else if (!strcmp (name, "b"))
     {
@@ -628,6 +669,8 @@ _gcry_mpi_ec_dup_point (mpi_point_t result, mpi_point_t point, mpi_ec_t ctx)
 #define l2 (ctx->t.scratch[4])
 #define l3 (ctx->t.scratch[5])
 
+  ec_p_sync (ctx);
+
   if (!mpi_cmp_ui (point->y, 0) || !mpi_cmp_ui (point->z, 0))
     {
       /* P_y == 0 || P_z == 0 => [1:1:0] */
@@ -725,6 +768,8 @@ _gcry_mpi_ec_add_points (mpi_point_t result,
 #define t1 (ctx->t.scratch[9])
 #define t2 (ctx->t.scratch[10])
 
+  ec_p_sync (ctx);
+
   if ( (!mpi_cmp (x1, x2)) && (!mpi_cmp (y1, y2)) && (!mpi_cmp (z1, z2)) )
     {
       /* Same point; need to call the duplicate function.  */
@@ -854,6 +899,8 @@ _gcry_mpi_ec_mul_point (mpi_point_t result,
   unsigned int nbits;
   int i;
 
+  ec_p_sync (ctx);
+
   nbits = mpi_get_nbits (scalar);
   mpi_set_ui (result->x, 1);
   mpi_set_ui (result->y, 1);
@@ -871,6 +918,8 @@ _gcry_mpi_ec_mul_point (mpi_point_t result,
   unsigned int i, loops;
   mpi_point_struct p1, p2, p1inv;
 
+  ec_p_sync (ctx);
+
   x1 = mpi_alloc_like (ctx->p);
   y1 = mpi_alloc_like (ctx->p);
   h  = mpi_alloc_like (ctx->p);
index 88742bf..6827e18 100644 (file)
@@ -38,6 +38,8 @@ struct mpi_ec_ctx_s
 
   /* This structure is private to mpi/ec.c! */
   struct {
+    int need_sync;     /* Helper for ec_p_sync.  */
+
     int a_is_pminus3;  /* True if A = P - 3. */
 
     gcry_mpi_t two_inv_p;
index b727d5f..fd265bf 100644 (file)
--- a/src/mpi.h
+++ b/src/mpi.h
@@ -289,6 +289,8 @@ void _gcry_mpi_ec_mul_point (mpi_point_t result,
                              gcry_mpi_t scalar, mpi_point_t point,
                              mpi_ec_t ctx);
 
+gcry_mpi_t _gcry_mpi_ec_ec2os (gcry_mpi_point_t point, mpi_ec_t ectx);
+
 gpg_err_code_t _gcry_mpi_ec_p_new (gcry_ctx_t *r_ctx,
                                    gcry_mpi_t p, gcry_mpi_t a);
 gpg_err_code_t _gcry_mpi_ec_new (gcry_ctx_t *r_ctx,
index 31df12b..a3b6c56 100644 (file)
@@ -116,6 +116,15 @@ static struct
     { NULL, NULL, NULL, NULL, NULL }
   };
 
+/* A sample public key for NIST P-256.  */
+static const char sample_p256_q[] =
+  "04"
+  "42B927242237639A36CE9221B340DB1A9AB76DF2FE3E171277F6A4023DED146E"
+  "E86525E38CCECFF3FB8D152CC6334F70D23A525175C1BCBDDE6E023B2228770E";
+static const char sample_p256_q_x[] =
+  "42B927242237639A36CE9221B340DB1A9AB76DF2FE3E171277F6A4023DED146E";
+static const char sample_p256_q_y[] =
+  "00E86525E38CCECFF3FB8D152CC6334F70D23A525175C1BCBDDE6E023B2228770E";
 
 
 
@@ -164,7 +173,7 @@ die (const char *format, ...)
 
 
 static void
-print_mpi (const char *text, gcry_mpi_t a)
+print_mpi_2 (const char *text, const char *text2, gcry_mpi_t a)
 {
   gcry_error_t err;
   char *buf;
@@ -172,16 +181,41 @@ print_mpi (const char *text, gcry_mpi_t a)
 
   err = gcry_mpi_aprint (GCRYMPI_FMT_HEX, bufaddr, NULL, a);
   if (err)
-    fprintf (stderr, "%s: [error printing number: %s]\n",
-             text, gpg_strerror (err));
+    fprintf (stderr, "%s%s: [error printing number: %s]\n",
+             text, text2? text2:"", gpg_strerror (err));
   else
     {
-      fprintf (stderr, "%s: %s\n", text, buf);
+      fprintf (stderr, "%s%s: %s\n", text, text2? text2:"", buf);
       gcry_free (buf);
     }
 }
 
 
+static void
+print_mpi (const char *text, gcry_mpi_t a)
+{
+  print_mpi_2 (text, NULL, a);
+}
+
+
+static void
+print_point (const char *text, gcry_mpi_point_t a)
+{
+  gcry_mpi_t x, y, z;
+
+  x = gcry_mpi_new (0);
+  y = gcry_mpi_new (0);
+  z = gcry_mpi_new (0);
+  gcry_mpi_point_get (x, y, z, a);
+  print_mpi_2 (text, ".x", x);
+  print_mpi_2 (text, ".y", y);
+  print_mpi_2 (text, ".z", z);
+  gcry_mpi_release (x);
+  gcry_mpi_release (y);
+  gcry_mpi_release (z);
+}
+
+
 static gcry_mpi_t
 hex2mpi (const char *string)
 {
@@ -345,6 +379,8 @@ get_and_cmp_mpi (const char *name, const char *mpistring, const char *desc,
       fail ("error getting parameter '%s' of curve '%s'\n", name, desc);
       return 1;
     }
+  if (debug)
+    print_mpi (name, mpi);
   if (cmp_mpihex (mpi, mpistring))
     {
       fail ("parameter '%s' of curve '%s' does not match\n", name, desc);
@@ -371,6 +407,8 @@ get_and_cmp_point (const char *name,
       fail ("error getting point parameter '%s' of curve '%s'\n", name, desc);
       return 1;
     }
+  if (debug)
+    print_point (name, point);
 
   x = gcry_mpi_new (0);
   y = gcry_mpi_new (0);
@@ -404,12 +442,14 @@ context_param (void)
   gpg_error_t err;
   int idx;
   gcry_ctx_t ctx = NULL;
+  gcry_mpi_t q;
+  gcry_sexp_t keyparam;
 
   wherestr = "context_param";
 
+  show ("checking standard curves\n");
   for (idx=0; test_curve[idx].desc; idx++)
     {
-      show ("checking curve '%s'\n", test_curve[idx].desc);
       gcry_ctx_release (ctx);
       err = gcry_mpi_ec_new (&ctx, NULL, test_curve[idx].desc);
       if (err)
@@ -437,11 +477,35 @@ context_param (void)
     }
   gcry_ctx_release (ctx);
 
-  /* FIXME: Add tests for Q and d.  */
 
-  /* FIXME: Add test sor the set functions.  */
+  show ("checking sample public key\n");
+  q = hex2mpi (sample_p256_q);
+  err = gcry_sexp_build (&keyparam, NULL,
+                        "(public-key(ecdsa(curve %s)(q %m)))",
+                        "NIST P-256", q);
+  if (err)
+    die ("gcry_sexp_build failed: %s\n", gpg_strerror (err));
+  gcry_mpi_release (q);
+
+  /* We can't call gcry_pk_testkey because it is only implemented for
+     private keys.  */
+  /* err = gcry_pk_testkey (keyparam); */
+  /* if (err) */
+  /*   fail ("gcry_pk_testkey failed for sample public key: %s\n", */
+  /*         gpg_strerror (err)); */
 
+  err = gcry_mpi_ec_new (&ctx, keyparam, NULL);
+  if (err)
+    fail ("gcry_mpi_ec_new failed for sample public key: %s\n",
+          gpg_strerror (err));
+  else
+    {
+      get_and_cmp_mpi ("q", sample_p256_q, "NIST P-256", ctx);
+      get_and_cmp_point ("q", sample_p256_q_x, sample_p256_q_y, "NIST P-256",
+                         ctx);
+    }
 
+  gcry_sexp_release (keyparam);
 }