Make the Q parameter optional for ECC signing.
authorWerner Koch <wk@gnupg.org>
Fri, 5 Apr 2013 16:08:36 +0000 (18:08 +0200)
committerWerner Koch <wk@gnupg.org>
Fri, 5 Apr 2013 16:08:51 +0000 (18:08 +0200)
* cipher/ecc.c (ecc_sign): Remove the need for Q.
* cipher/pubkey.c (sexp_elements_extract_ecc): Make Q optional for a
private key.
(sexp_to_key): Add optional arg R_IS_ECC.
(gcry_pk_sign): Do not call gcry_pk_get_nbits for ECC keys.
* tests/pubkey.c (die): Make sure to print a LF.
(check_ecc_sample_key): New.
(main): Call new test.
--

Q is the actual public key which is not used for signing.  Thus we
can make it optional and even speed up the signing by parsing less
stuff.

Note: There seems to be a memory leak somewhere.  Running tests/pubkey
with just the new test enabled shows it.

Signed-off-by: Werner Koch <wk@gnupg.org>
cipher/ecc.c
cipher/pubkey.c
doc/gcrypt.texi
tests/pubkey.c

index 8fcd57d..fbd8c6a 100644 (file)
@@ -1309,7 +1309,7 @@ ecc_sign (int algo, gcry_mpi_t *resarr, gcry_mpi_t data, gcry_mpi_t *skey)
   (void)algo;
 
   if (!data || !skey[0] || !skey[1] || !skey[2] || !skey[3] || !skey[4]
-      || !skey[5] || !skey[6] )
+      || !skey[6] )
     return GPG_ERR_BAD_MPI;
 
   sk.E.p = skey[0];
@@ -1323,14 +1323,7 @@ ecc_sign (int algo, gcry_mpi_t *resarr, gcry_mpi_t data, gcry_mpi_t *skey)
       return err;
     }
   sk.E.n = skey[4];
-  point_init (&sk.Q);
-  err = os2ec (&sk.Q, skey[5]);
-  if (err)
-    {
-      point_free (&sk.E.G);
-      point_free (&sk.Q);
-      return err;
-    }
+  /* Note: We don't have any need for Q here.  */
   sk.d = skey[6];
 
   resarr[0] = mpi_alloc (mpi_get_nlimbs (sk.E.p));
@@ -1343,7 +1336,6 @@ ecc_sign (int algo, gcry_mpi_t *resarr, gcry_mpi_t data, gcry_mpi_t *skey)
       resarr[0] = NULL; /* Mark array as released.  */
     }
   point_free (&sk.E.G);
-  point_free (&sk.Q);
   return err;
 }
 
index c4be81c..cd07d17 100644 (file)
@@ -1858,7 +1858,8 @@ sexp_elements_extract (gcry_sexp_t key_sexp, const char *element_names,
    of its intimate knowledge about the ECC parameters from ecc.c. */
 static gcry_err_code_t
 sexp_elements_extract_ecc (gcry_sexp_t key_sexp, const char *element_names,
-                           gcry_mpi_t *elements, pk_extra_spec_t *extraspec)
+                           gcry_mpi_t *elements, pk_extra_spec_t *extraspec,
+                           int want_private)
 
 {
   gcry_err_code_t err = 0;
@@ -1939,8 +1940,13 @@ sexp_elements_extract_ecc (gcry_sexp_t key_sexp, const char *element_names,
   for (name = element_names, idx = 0; *name; name++, idx++)
     if (!elements[idx])
       {
-        err = GPG_ERR_NO_OBJ;
-        goto leave;
+        if (want_private && *name == 'q')
+          ; /* Q is optional.  */
+        else
+          {
+            err = GPG_ERR_NO_OBJ;
+            goto leave;
+          }
       }
 
  leave:
@@ -1994,7 +2000,7 @@ sexp_elements_extract_ecc (gcry_sexp_t key_sexp, const char *element_names,
  */
 static gcry_err_code_t
 sexp_to_key (gcry_sexp_t sexp, int want_private, const char *override_elems,
-             gcry_mpi_t **retarray, gcry_module_t *retalgo)
+             gcry_mpi_t **retarray, gcry_module_t *retalgo, int *r_is_ecc)
 {
   gcry_err_code_t err = 0;
   gcry_sexp_t list, l2;
@@ -2060,7 +2066,8 @@ sexp_to_key (gcry_sexp_t sexp, int want_private, const char *override_elems,
   if (!err)
     {
       if (is_ecc)
-        err = sexp_elements_extract_ecc (list, elems, array, extraspec);
+        err = sexp_elements_extract_ecc (list, elems, array, extraspec,
+                                         want_private);
       else
         err = sexp_elements_extract (list, elems, array, pubkey->name);
     }
@@ -2079,6 +2086,8 @@ sexp_to_key (gcry_sexp_t sexp, int want_private, const char *override_elems,
     {
       *retarray = array;
       *retalgo = module;
+      if (r_is_ecc)
+        *r_is_ecc = is_ecc;
     }
 
   return err;
@@ -2854,7 +2863,7 @@ gcry_pk_encrypt (gcry_sexp_t *r_ciph, gcry_sexp_t s_data, gcry_sexp_t s_pkey)
   REGISTER_DEFAULT_PUBKEYS;
 
   /* Get the key. */
-  rc = sexp_to_key (s_pkey, 0, NULL, &pkey, &module);
+  rc = sexp_to_key (s_pkey, 0, NULL, &pkey, &module, NULL);
   if (rc)
     goto leave;
 
@@ -3027,7 +3036,7 @@ gcry_pk_decrypt (gcry_sexp_t *r_plain, gcry_sexp_t s_data, gcry_sexp_t s_skey)
 
   REGISTER_DEFAULT_PUBKEYS;
 
-  rc = sexp_to_key (s_skey, 1, NULL, &skey, &module_key);
+  rc = sexp_to_key (s_skey, 1, NULL, &skey, &module_key, NULL);
   if (rc)
     goto leave;
 
@@ -3149,13 +3158,14 @@ gcry_pk_sign (gcry_sexp_t *r_sig, gcry_sexp_t s_hash, gcry_sexp_t s_skey)
   const char *algo_name, *algo_elems;
   struct pk_encoding_ctx ctx;
   int i;
+  int is_ecc;
   gcry_err_code_t rc;
 
   *r_sig = NULL;
 
   REGISTER_DEFAULT_PUBKEYS;
 
-  rc = sexp_to_key (s_skey, 1, NULL, &skey, &module);
+  rc = sexp_to_key (s_skey, 1, NULL, &skey, &module, &is_ecc);
   if (rc)
     goto leave;
 
@@ -3168,8 +3178,10 @@ gcry_pk_sign (gcry_sexp_t *r_sig, gcry_sexp_t s_hash, gcry_sexp_t s_skey)
   algo_elems = pubkey->elements_sig;
 
   /* Get the stuff we want to sign.  Note that pk_get_nbits does also
-      work on a private key. */
-  init_encoding_ctx (&ctx, PUBKEY_OP_SIGN, gcry_pk_get_nbits (s_skey));
+     work on a private key.  We don't need the number of bits for ECC
+     here, thus set it to 0 so that we don't need to parse it.  */
+  init_encoding_ctx (&ctx, PUBKEY_OP_SIGN,
+                     is_ecc? 0 : gcry_pk_get_nbits (s_skey));
   rc = sexp_data_to_mpi (s_hash, &hash, &ctx);
   if (rc)
     goto leave;
@@ -3287,7 +3299,7 @@ gcry_pk_verify (gcry_sexp_t s_sig, gcry_sexp_t s_hash, gcry_sexp_t s_pkey)
 
   REGISTER_DEFAULT_PUBKEYS;
 
-  rc = sexp_to_key (s_pkey, 0, NULL, &pkey, &module_key);
+  rc = sexp_to_key (s_pkey, 0, NULL, &pkey, &module_key, NULL);
   if (rc)
     goto leave;
 
@@ -3360,7 +3372,7 @@ gcry_pk_testkey (gcry_sexp_t s_key)
   REGISTER_DEFAULT_PUBKEYS;
 
   /* Note we currently support only secret key checking. */
-  rc = sexp_to_key (s_key, 1, NULL, &key, &module);
+  rc = sexp_to_key (s_key, 1, NULL, &key, &module, NULL);
   if (! rc)
     {
       rc = pubkey_check_secret_key (module->mod_id, key);
@@ -3689,9 +3701,13 @@ gcry_pk_get_nbits (gcry_sexp_t key)
 
   REGISTER_DEFAULT_PUBKEYS;
 
-  rc = sexp_to_key (key, 0, NULL, &keyarr, &module);
+  /* FIXME: Parsing KEY is often too much overhead.  For example for
+     ECC we would only need to look at P and stop parsing right
+     away.  */
+
+  rc = sexp_to_key (key, 0, NULL, &keyarr, &module, NULL);
   if (rc == GPG_ERR_INV_OBJ)
-    rc = sexp_to_key (key, 1, NULL, &keyarr, &module);
+    rc = sexp_to_key (key, 1, NULL, &keyarr, &module, NULL);
   if (rc)
     return 0; /* Error - 0 is a suitable indication for that. */
 
@@ -3862,7 +3878,7 @@ gcry_pk_get_curve (gcry_sexp_t key, int iterator, unsigned int *r_nbits)
       /* Get the key.  We pass the names of the parameters for
          override_elems; this allows to call this function without the
          actual public key parameter.  */
-      if (sexp_to_key (key, want_private, "pabgn", &pkey, &module))
+      if (sexp_to_key (key, want_private, "pabgn", &pkey, &module, NULL))
         goto leave;
     }
   else
index 8f277c2..888a740 100644 (file)
@@ -2108,7 +2108,8 @@ used.  For example
     (d @var{d-mpi})))
 @end example
 
-The @code{curve} parameter may be given in any case and is used to replace
+Note that @var{q-point} is optional for a private key.  The
+@code{curve} parameter may be given in any case and is used to replace
 missing parameters.
 
 @noindent
index 92e5f5d..4534175 100644 (file)
@@ -110,6 +110,8 @@ die (const char *format, ...)
   va_start( arg_ptr, format ) ;
   vfprintf (stderr, format, arg_ptr );
   va_end(arg_ptr);
+  if (*format && format[strlen(format)-1] != '\n')
+    putc ('\n', stderr);
   exit (1);
 }
 
@@ -856,6 +858,79 @@ check_x931_derived_key (int what)
 
 
 
+static void
+check_ecc_sample_key (void)
+{
+  static const char ecc_private_key[] =
+    "(private-key\n"
+    " (ecdsa\n"
+    "  (curve \"NIST P-256\")\n"
+    "  (q #04D4F6A6738D9B8D3A7075C1E4EE95015FC0C9B7E4272D2BEB6644D3609FC781"
+    "B71F9A8072F58CB66AE2F89BB12451873ABF7D91F9E1FBF96BF2F70E73AAC9A283#)\n"
+    "  (d #5A1EF0035118F19F3110FB81813D3547BCE1E5BCE77D1F744715E1D5BBE70378#)"
+    "))";
+  static const char ecc_private_key_wo_q[] =
+    "(private-key\n"
+    " (ecdsa\n"
+    "  (curve \"NIST P-256\")\n"
+    "  (d #5A1EF0035118F19F3110FB81813D3547BCE1E5BCE77D1F744715E1D5BBE70378#)"
+    "))";
+  static const char ecc_public_key[] =
+    "(public-key\n"
+    " (ecdsa\n"
+    "  (curve \"NIST P-256\")\n"
+    "  (q #04D4F6A6738D9B8D3A7075C1E4EE95015FC0C9B7E4272D2BEB6644D3609FC781"
+    "B71F9A8072F58CB66AE2F89BB12451873ABF7D91F9E1FBF96BF2F70E73AAC9A283#)"
+    "))";
+  static const char hash_string[] =
+    "(data (flags raw)\n"
+    " (value #00112233445566778899AABBCCDDEEFF"
+    /* */    "000102030405060708090A0B0C0D0E0F#))";
+
+  gpg_error_t err;
+  gcry_sexp_t key, hash, sig;
+
+  if (verbose)
+    fprintf (stderr, "Checking sample ECC key.\n");
+
+  if ((err = gcry_sexp_new (&hash, hash_string, 0, 1)))
+    die ("line %d: %s", __LINE__, gpg_strerror (err));
+
+  if ((err = gcry_sexp_new (&key, ecc_private_key, 0, 1)))
+    die ("line %d: %s", __LINE__, gpg_strerror (err));
+
+  if ((err = gcry_pk_sign (&sig, hash, key)))
+    die ("gcry_pk_sign failed: %s", gpg_strerror (err));
+
+  gcry_sexp_release (key);
+  if ((err = gcry_sexp_new (&key, ecc_public_key, 0, 1)))
+    die ("line %d: %s", __LINE__, gpg_strerror (err));
+
+  if ((err = gcry_pk_verify (sig, hash, key)))
+    die ("gcry_pk_verify failed: %s", gpg_strerror (err));
+
+  /* Now try signing without the Q parameter.  */
+
+  gcry_sexp_release (key);
+  if ((err = gcry_sexp_new (&key, ecc_private_key_wo_q, 0, 1)))
+    die ("line %d: %s", __LINE__, gpg_strerror (err));
+
+  gcry_sexp_release (sig);
+  if ((err = gcry_pk_sign (&sig, hash, key)))
+    die ("gcry_pk_sign without Q failed: %s", gpg_strerror (err));
+
+  gcry_sexp_release (key);
+  if ((err = gcry_sexp_new (&key, ecc_public_key, 0, 1)))
+    die ("line %d: %s", __LINE__, gpg_strerror (err));
+
+  if ((err = gcry_pk_verify (sig, hash, key)))
+    die ("gcry_pk_verify signed without Q failed: %s", gpg_strerror (err));
+
+  gcry_sexp_release (sig);
+  gcry_sexp_release (key);
+  gcry_sexp_release (hash);
+}
+
 
 int
 main (int argc, char **argv)
@@ -886,5 +961,7 @@ main (int argc, char **argv)
   for (i=0; i < 4; i++)
     check_x931_derived_key (i);
 
+  check_ecc_sample_key ();
+
   return 0;
 }