Fixed an internal bug in rsa.c
authorWerner Koch <wk@gnupg.org>
Wed, 5 Dec 2007 09:50:50 +0000 (09:50 +0000)
committerWerner Koch <wk@gnupg.org>
Wed, 5 Dec 2007 09:50:50 +0000 (09:50 +0000)
Allow to decryption/signing using a minimal RSA key.

NEWS
cipher/ChangeLog
cipher/pubkey.c
cipher/rsa.c
configure.ac
tests/ChangeLog
tests/pubkey.c

diff --git a/NEWS b/NEWS
index c0a577f..27d2b76 100644 (file)
--- a/NEWS
+++ b/NEWS
@@ -1,3 +1,7 @@
+Noteworthy changes in version 1.4.0
+------------------------------------------------
+
+
 Noteworthy changes in version 1.3.2 (2007-12-03)
 ------------------------------------------------
 
index 90fc43a..f81c5da 100644 (file)
@@ -1,3 +1,14 @@
+2007-12-05  Werner Koch  <wk@g10code.com>
+
+       * rsa.c (secret): Fixed condition test for using CRT.  Reported by
+       Dean Scarff.  Fixes bug#864.
+       (_gcry_rsa_check_secret_key): Return an erro if the optional
+       parameters are missing.
+       * pubkey.c (sexp_elements_extract): Add arg ALGO_NAME. Changed all
+       callers to pass NULL. Add hack to allow for optional RSA
+       parameters.
+       (sexp_to_key): Pass algo name to sexp_elements_extract.
+
 2007-12-03  Werner Koch  <wk@g10code.com>
 
        * random.c (gcry_random_add_bytes): Implement it.
index 91fbfd4..fc57b40 100644 (file)
@@ -751,9 +751,9 @@ pubkey_verify (int algorithm, gcry_mpi_t hash, gcry_mpi_t *data,
 /* Internal function.   */
 static gcry_err_code_t
 sexp_elements_extract (gcry_sexp_t key_sexp, const char *element_names,
-                      gcry_mpi_t *elements)
+                      gcry_mpi_t *elements, const char *algo_name)
 {
-  gcry_err_code_t err = GPG_ERR_NO_ERROR;
+  gcry_err_code_t err = 0;
   int i, idx;
   const char *name;
   gcry_sexp_t list;
@@ -761,17 +761,41 @@ sexp_elements_extract (gcry_sexp_t key_sexp, const char *element_names,
   for (name = element_names, idx = 0; *name && !err; name++, idx++)
     {
       list = gcry_sexp_find_token (key_sexp, name, 1);
-      if (! list)
-       err = GPG_ERR_NO_OBJ;
+      if (!list)
+       elements[idx] = NULL;
       else
        {
          elements[idx] = gcry_sexp_nth_mpi (list, 1, GCRYMPI_FMT_USG);
          gcry_sexp_release (list);
-         if (! elements[idx])
+         if (!elements[idx])
            err = GPG_ERR_INV_OBJ;
        }
     }
 
+  if (!err)
+    {
+      /* Check that all elements are available.  */
+      for (name = element_names, idx = 0; *name; name++, idx++)
+        if (!elements[idx])
+          break;
+      if (*name)
+        {
+          err = GPG_ERR_NO_OBJ;
+          /* Some are missing.  Before bailing out we test for
+             optional parameters.  */
+          if (algo_name && !strcmp (algo_name, "RSA")
+              && !strcmp (element_names, "nedpqu") )
+            {
+              /* This is RSA.  Test whether we got N, E and D and that
+                 the optional P, Q and U are all missing.  */
+              if (elements[0] && elements[1] && elements[2]
+                  && !elements[3] && !elements[4] && !elements[5])
+                err = 0;
+            }
+        }
+    }
+
+
   if (err)
     {
       for (i = 0; i < idx; i++)
@@ -884,8 +908,6 @@ sexp_elements_extract_ecc (gcry_sexp_t key_sexp, const char *element_names,
  * NOTE: we look through the list to find a list beginning with
  * "private-key" or "public-key" - the first one found is used.
  *
- * FIXME: Allow for encrypted secret keys here.
- *
  * Returns: A pointer to an allocated array of MPIs if the return value is
  *         zero; the caller has to release this array.
  *
@@ -959,7 +981,7 @@ sexp_to_key (gcry_sexp_t sexp, int want_private, gcry_mpi_t **retarray,
       if (is_ecc)
         err = sexp_elements_extract_ecc (list, elems, array);
       else
-        err = sexp_elements_extract (list, elems, array);
+        err = sexp_elements_extract (list, elems, array, pubkey->name);
     }
   
   gcry_sexp_release (list);
@@ -1048,7 +1070,7 @@ sexp_to_sig (gcry_sexp_t sexp, gcry_mpi_t **retarray,
     err = gpg_err_code_from_errno (errno);
 
   if (!err)
-    err = sexp_elements_extract (list, elems, array);
+    err = sexp_elements_extract (list, elems, array, NULL);
 
   gcry_sexp_release (l2);
   gcry_sexp_release (list);
@@ -1190,7 +1212,7 @@ sexp_to_enc (gcry_sexp_t sexp, gcry_mpi_t **retarray, gcry_module_t *retalgo,
       goto leave;
     }
 
-  err = sexp_elements_extract (list, elems, array);
+  err = sexp_elements_extract (list, elems, array, NULL);
 
  leave:
   gcry_sexp_release (list);
@@ -1921,7 +1943,7 @@ gcry_pk_verify (gcry_sexp_t s_sig, gcry_sexp_t s_hash, gcry_sexp_t s_pkey)
    Test a key.
 
    This may be used either for a public or a secret key to see whether
-   internal structre is valid.
+   the internal structure is okay.
   
    Returns: 0 or an errorcode.
   
index 7d5fd4c..fe545c7 100644 (file)
@@ -1,6 +1,6 @@
 /* rsa.c  -  RSA function
- *     Copyright (C) 1997, 1998, 1999 by Werner Koch (dd9jn)
- *     Copyright (C) 2000, 2001, 2002, 2003 Free Software Foundation, Inc.
+ * Copyright (C) 1997, 1998, 1999 by Werner Koch (dd9jn)
+ * Copyright (C) 2000, 2001, 2002, 2003 Free Software Foundation, Inc.
  *
  * This file is part of Libgcrypt.
  *
@@ -15,8 +15,7 @@
  * GNU Lesser General Public License for more details.
  *
  * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA
+ * License along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
 /* This code uses an algorithm protected by U.S. Patent #4,405,829
@@ -357,7 +356,7 @@ stronger_key_check ( RSA_secret_key *skey )
 static void
 secret(gcry_mpi_t output, gcry_mpi_t input, RSA_secret_key *skey )
 {
-  if (!skey->p && !skey->q && !skey->u)
+  if (!skey->p || !skey->q || !skey->u)
     {
       mpi_powm (output, input, skey->d, skey->n);
     }
@@ -488,7 +487,10 @@ _gcry_rsa_check_secret_key( int algo, gcry_mpi_t *skey )
   sk.q = skey[4];
   sk.u = skey[5];
 
-  if (! check_secret_key (&sk))
+  if (!sk.p || !sk.q || !sk.u)
+    err = GPG_ERR_NO_OBJ;  /* To check the key we need the optional
+                              parameters. */
+  else if (!check_secret_key (&sk))
     err = GPG_ERR_PUBKEY_ALGO;
 
   return err;
@@ -529,9 +531,9 @@ _gcry_rsa_decrypt (int algo, gcry_mpi_t *result, gcry_mpi_t *data,
   sk.n = skey[0];
   sk.e = skey[1];
   sk.d = skey[2];
-  sk.p = skey[3];
-  sk.q = skey[4];
-  sk.u = skey[5];
+  sk.p = skey[3]; /* Optional. */
+  sk.q = skey[4]; /* Optional. */
+  sk.u = skey[5]; /* Optional. */
 
   y = gcry_mpi_snew (gcry_mpi_get_nbits (sk.n));
 
index 3ba7492..1f04e35 100644 (file)
@@ -27,8 +27,8 @@ min_automake_version="1.10"
 # Remember to change the version number immediately *after* a release.
 # Set my_issvn to "yes" for non-released code.  Remember to run an
 # "svn up" and "autogen.sh" right before creating a distribution.
-m4_define([my_version], [1.3.2])
-m4_define([my_issvn], [no])
+m4_define([my_version], [1.3.3])
+m4_define([my_issvn], [yes])
 
 m4_define([svn_revision], m4_esyscmd([echo -n $( (svn info 2>/dev/null \
             || echo 'Revision: 0')|sed -n '/^Revision:/ {s/[^0-9]//gp;q;}')]))
index 497fa9c..61dd775 100644 (file)
@@ -1,3 +1,11 @@
+2007-12-05  Werner Koch  <wk@g10code.com>
+
+       * pubkey.c (sample_private_key_1_1,sample_private_key_1_2): New.
+       (get_keys_sample): Add arg SECRET_VARIANT.
+       (check_run): Check all variants.  Also check gcry_pk_testkey.
+       (check_keys_crypt): Add DECRYPT_FAIL_CODE.
+       (check_keys): Ditto.
+
 2007-11-30  Werner Koch  <wk@g10code.com>
 
        * benchmark.c (main): Add optione --verbose and reworked the
index 2ee2e15..ac68a21 100644 (file)
@@ -14,8 +14,7 @@
  * GNU Lesser General Public License for more details.
  *
  * You should have received a copy of the GNU Lesser General Public
- * License along with this program; if not, write to the Free Software
- * Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA
+ * License along with this program; if not, see <http://www.gnu.org/licenses/>.
  */
 
 #ifdef HAVE_CONFIG_H
@@ -52,6 +51,43 @@ static const char sample_private_key_1[] =
 " )\n"
 ")\n";
 
+/* The same key as above but without p, q and u to test the non CRT case. */
+static const char sample_private_key_1_1[] =
+"(private-key\n"
+" (openpgp-rsa\n"
+"  (n #00e0ce96f90b6c9e02f3922beada93fe50a875eac6bcc18bb9a9cf2e84965caa"
+      "2d1ff95a7f542465c6c0c19d276e4526ce048868a7a914fd343cc3a87dd74291"
+      "ffc565506d5bbb25cbac6a0e2dd1f8bcaab0d4a29c2f37c950f363484bf269f7"
+      "891440464baf79827e03a36e70b814938eebdc63e964247be75dc58b014b7ea251#)\n"
+"  (e #010001#)\n"
+"  (d #046129F2489D71579BE0A75FE029BD6CDB574EBF57EA8A5B0FDA942CAB943B11"
+      "7D7BB95E5D28875E0F9FC5FCC06A72F6D502464DABDED78EF6B716177B83D5BD"
+      "C543DC5D3FED932E59F5897E92E6F58A0F33424106A3B6FA2CBF877510E4AC21"
+      "C3EE47851E97D12996222AC3566D4CCB0B83D164074ABF7DE655FC2446DA1781#)\n"
+" )\n"
+")\n";
+
+/* The same key as above but just without q to test the non CRT case.  This
+   should fail. */
+static const char sample_private_key_1_2[] =
+"(private-key\n"
+" (openpgp-rsa\n"
+"  (n #00e0ce96f90b6c9e02f3922beada93fe50a875eac6bcc18bb9a9cf2e84965caa"
+      "2d1ff95a7f542465c6c0c19d276e4526ce048868a7a914fd343cc3a87dd74291"
+      "ffc565506d5bbb25cbac6a0e2dd1f8bcaab0d4a29c2f37c950f363484bf269f7"
+      "891440464baf79827e03a36e70b814938eebdc63e964247be75dc58b014b7ea251#)\n"
+"  (e #010001#)\n"
+"  (d #046129F2489D71579BE0A75FE029BD6CDB574EBF57EA8A5B0FDA942CAB943B11"
+      "7D7BB95E5D28875E0F9FC5FCC06A72F6D502464DABDED78EF6B716177B83D5BD"
+      "C543DC5D3FED932E59F5897E92E6F58A0F33424106A3B6FA2CBF877510E4AC21"
+      "C3EE47851E97D12996222AC3566D4CCB0B83D164074ABF7DE655FC2446DA1781#)\n"
+"  (p #00e861b700e17e8afe6837e7512e35b6ca11d0ae47d8b85161c67baf64377213"
+      "fe52d772f2035b3ca830af41d8a4120e1c1c70d12cc22f00d28d31dd48a8d424f1#)\n"
+"  (u #304559a9ead56d2309d203811a641bb1a09626bc8eb36fffa23c968ec5bd891e"
+      "ebbafc73ae666e01ba7c8990bae06cc2bbe10b75e69fcacb353a6473079d8e9b#)\n"
+" )\n"
+")\n";
+
 static const char sample_public_key_1[] =
 "(public-key\n"
 " (rsa\n"
@@ -79,7 +115,7 @@ die (const char *format, ...)
 
 static void
 check_keys_crypt (gcry_sexp_t pkey, gcry_sexp_t skey,
-                 gcry_sexp_t plain0)
+                 gcry_sexp_t plain0, gpg_err_code_t decrypt_fail_code)
 {
   gcry_sexp_t plain1, cipher, l;
   gcry_mpi_t x0, x1;
@@ -103,7 +139,11 @@ check_keys_crypt (gcry_sexp_t pkey, gcry_sexp_t skey,
   rc = gcry_pk_decrypt (&plain1, cipher, skey);
   gcry_sexp_release (cipher);
   if (rc)
-    die ("decryption failed: %s\n", gcry_strerror (rc));
+    {
+      if (decrypt_fail_code && gpg_err_code (rc) == decrypt_fail_code)
+        return; /* This is the expected failure code.  */
+      die ("decryption failed: %s\n", gcry_strerror (rc));
+    }
 
   /* Extract decrypted data.  Note that for compatibility reasons, the
      output of gcry_pk_decrypt depends on whether a flags lists (even
@@ -133,7 +173,8 @@ check_keys_crypt (gcry_sexp_t pkey, gcry_sexp_t skey,
 }
 
 static void
-check_keys (gcry_sexp_t pkey, gcry_sexp_t skey, unsigned int nbits_data)
+check_keys (gcry_sexp_t pkey, gcry_sexp_t skey, unsigned int nbits_data,
+            gpg_err_code_t decrypt_fail_code)
 {
   gcry_sexp_t plain;
   gcry_mpi_t x;
@@ -148,7 +189,7 @@ check_keys (gcry_sexp_t pkey, gcry_sexp_t skey, unsigned int nbits_data)
     die ("converting data for encryption failed: %s\n",
         gcry_strerror (rc));
 
-  check_keys_crypt (pkey, skey, plain);
+  check_keys_crypt (pkey, skey, plain, decrypt_fail_code);
   gcry_sexp_release (plain);
   gcry_mpi_release (x);
 
@@ -162,21 +203,30 @@ check_keys (gcry_sexp_t pkey, gcry_sexp_t skey, unsigned int nbits_data)
     die ("converting data for encryption failed: %s\n",
         gcry_strerror (rc));
 
-  check_keys_crypt (pkey, skey, plain);
+  check_keys_crypt (pkey, skey, plain, decrypt_fail_code);
   gcry_sexp_release (plain);
 }
 
 static void
-get_keys_sample (gcry_sexp_t *pkey, gcry_sexp_t *skey)
+get_keys_sample (gcry_sexp_t *pkey, gcry_sexp_t *skey, int secret_variant)
 {
   gcry_sexp_t pub_key, sec_key;
   int rc;
+  static const char *secret;
+
+
+  switch (secret_variant)
+    {
+    case 0: secret = sample_private_key_1; break;
+    case 1: secret = sample_private_key_1_1; break;
+    case 2: secret = sample_private_key_1_2; break;
+    default: die ("BUG\n");
+    }
 
   rc = gcry_sexp_sscan (&pub_key, NULL, sample_public_key_1,
                        strlen (sample_public_key_1));
-  if (! rc)
-    rc = gcry_sexp_sscan (&sec_key, NULL, sample_private_key_1,
-                         strlen (sample_private_key_1));
+  if (!rc)
+    rc = gcry_sexp_sscan (&sec_key, NULL, secret, strlen (secret));
   if (rc)
     die ("converting sample keys failed: %s\n", gcry_strerror (rc));
 
@@ -249,33 +299,44 @@ get_elg_key_new (gcry_sexp_t *pkey, gcry_sexp_t *skey, int fixed_x)
 static void
 check_run (void)
 {
+  gpg_error_t err;
   gcry_sexp_t pkey, skey;
+  int variant;
+
+  for (variant=0; variant < 3; variant++)
+    {
+      if (verbose)
+        fprintf (stderr, "Checking sample key (%d).\n", variant);
+      get_keys_sample (&pkey, &skey, variant);
+      /* Check gcry_pk_testkey which requires all elements.  */
+      err = gcry_pk_testkey (skey);
+      if ((variant == 0 && err)
+          || (variant > 0 && gpg_err_code (err) != GPG_ERR_NO_OBJ))
+          die ("gcry_pk_testkey failed: %s\n", gpg_strerror (err));
+      /* Run the usual check but expect an error from variant 2.  */
+      check_keys (pkey, skey, 800, variant == 2? GPG_ERR_NO_OBJ : 0);
+      gcry_sexp_release (pkey);
+      gcry_sexp_release (skey);
+    }
 
-  if (verbose)
-    fprintf (stderr, "Checking sample key.\n");
-  get_keys_sample (&pkey, &skey);
-  check_keys (pkey, skey, 800);
-  gcry_sexp_release (pkey);
-  gcry_sexp_release (skey);
-  
   if (verbose)
     fprintf (stderr, "Checking generated RSA key.\n");
   get_keys_new (&pkey, &skey);
-  check_keys (pkey, skey, 800);
+  check_keys (pkey, skey, 800, 0);
   gcry_sexp_release (pkey);
   gcry_sexp_release (skey);
 
   if (verbose)
     fprintf (stderr, "Checking generated Elgamal key.\n");
   get_elg_key_new (&pkey, &skey, 0);
-  check_keys (pkey, skey, 400 );
+  check_keys (pkey, skey, 400, 0);
   gcry_sexp_release (pkey);
   gcry_sexp_release (skey);
 
   if (verbose)
     fprintf (stderr, "Checking passphrase generated Elgamal key.\n");
   get_elg_key_new (&pkey, &skey, 1);
-  check_keys (pkey, skey, 800);
+  check_keys (pkey, skey, 800, 0);
   gcry_sexp_release (pkey);
   gcry_sexp_release (skey);
 }
@@ -294,7 +355,7 @@ main (int argc, char **argv)
 
   gcry_control (GCRYCTL_DISABLE_SECMEM, 0);
   if (!gcry_check_version (GCRYPT_VERSION))
-    die ("version mismatch\n");
+    /*die ("version mismatch\n")*/;
   gcry_control (GCRYCTL_INITIALIZATION_FINISHED, 0);
   if (debug)
     gcry_control (GCRYCTL_SET_DEBUG_FLAGS, 1u , 0);