gpg: Fix a memory leak in batch key generation

* g10/keygen.c (append_to_parameter): New.
(proc_parameter_file): Use new func to extend the parameter list.

* g10/passphrase.c (passphrase_to_dek_ext): Print a diagnostic of
gcry_kdf_derive failed.
* g10/keygen.c (proc_parameter_file): Print a diagnostic if
passphrase_to_dek failed.
--

Due to an improper way of using the linked list head, all memory for
items allocated in proc_parameter_file was never released.  If batched
key generation with a passphrase and more than ~200 keys was used this
exhausted the secure memory.

2 files changed, 40 insertions, 26 deletions

diff --git a/g10/keygen.c b/g10/keygen.c
index b5ccf0287..fc985eef3 100644
--- a/g10/keygen.c
+++ b/g10/keygen.c
@@ -2591,6 +2591,17 @@ generate_user_id (KBNODE keyblock)
 }
 
 
+/* Append R to the linked list PARA.  */
+static void
+append_to_parameter (struct para_data_s *para, struct para_data_s *r)
+{
+  assert (para);
+  while (para->next)
+    para = para->next;
+  para->next = r;
+}
+
+/* Release the parameter list R.  */
 static void
 release_parameter_list (struct para_data_s *r)
 {
@@ -2817,8 +2828,7 @@ proc_parameter_file( struct para_data_s *para, const char *fname,
       r->u.usage = (is_default
                     ? (PUBKEY_USAGE_CERT | PUBKEY_USAGE_SIG)
                     : openpgp_pk_algo_usage(algo));
-      r->next = para;
-      para = r;
+      append_to_parameter (para, r);
     }
   else if (err == -1)
     return -1;
@@ -2854,8 +2864,7 @@ proc_parameter_file( struct para_data_s *para, const char *fname,
 	  r->u.usage = (is_default
                         ? PUBKEY_USAGE_ENC
                         : openpgp_pk_algo_usage (algo));
-	  r->next = para;
-	  para = r;
+          append_to_parameter (para, r);
 	}
       else if (err == -1)
 	return -1;
@@ -2892,8 +2901,7 @@ proc_parameter_file( struct para_data_s *para, const char *fname,
 	    p = stpcpy(stpcpy(stpcpy(p," ("), s2 ),")");
 	  if( s3 )
 	    p = stpcpy(stpcpy(stpcpy(p," <"), s3 ),">");
-	  r->next = para;
-	  para = r;
+          append_to_parameter (para, r);
 	  have_user_id=1;
 	}
     }
@@ -2946,13 +2954,11 @@ proc_parameter_file( struct para_data_s *para, const char *fname,
           r = xmalloc_clear( sizeof *r );
           r->key = pPASSPHRASE_DEK;
           r->u.dek = dek;
-          r->next = para;
-          para = r;
+          append_to_parameter (para, r);
           r = xmalloc_clear( sizeof *r );
           r->key = pPASSPHRASE_S2K;
           r->u.s2k = s2k;
-          r->next = para;
-          para = r;
+          append_to_parameter (para, r);
         }
 
       if (canceled)
@@ -2971,27 +2977,32 @@ proc_parameter_file( struct para_data_s *para, const char *fname,
            * but because we do this always, why not here.  */
           STRING2KEY *s2k;
           DEK *dek;
+          static int count;
 
-          s2k = xmalloc_secure ( sizeof *s2k );
+          s2k = xmalloc ( sizeof *s2k );
           s2k->mode = opt.s2k_mode;
           s2k->hash_algo = S2K_DIGEST_ALGO;
           set_next_passphrase ( r->u.value );
           dek = passphrase_to_dek (NULL, 0, opt.s2k_cipher_algo, s2k, 2,
                                    NULL, NULL);
-          set_next_passphrase (NULL );
-          assert (dek);
+          if (!dek)
+            {
+              log_error ("%s:%d: error post processing the passphrase\n",
+                         fname, r->lnr );
+              xfree (s2k);
+              return -1;
+            }
+          set_next_passphrase (NULL);
           memset (r->u.value, 0, strlen(r->u.value));
 
           r = xmalloc_clear (sizeof *r);
           r->key = pPASSPHRASE_S2K;
           r->u.s2k = s2k;
-          r->next = para;
-          para = r;
+          append_to_parameter (para, r);
           r = xmalloc_clear (sizeof *r);
           r->key = pPASSPHRASE_DEK;
           r->u.dek = dek;
-          r->next = para;
-          para = r;
+          append_to_parameter (para, r);
         }
     }
 
@@ -3029,8 +3040,7 @@ proc_parameter_file( struct para_data_s *para, const char *fname,
       r = xmalloc_clear( sizeof *r + 20 );
       r->key = pSUBKEYEXPIRE;
       r->u.expire = seconds;
-      r->next = para;
-      para = r;
+      append_to_parameter (para, r);
     }
 
   do_generate_keypair( para, outctrl, card );
diff --git a/g10/passphrase.c b/g10/passphrase.c
index d872e36ae..f83e66825 100644
--- a/g10/passphrase.c
+++ b/g10/passphrase.c
@@ -569,17 +569,21 @@ passphrase_to_dek_ext (u32 *keyid, int pubkey_algo,
     dek->keylen = 0;
   else
     {
+      gpg_error_t err;
+
       dek->keylen = openpgp_cipher_get_algo_keylen (dek->algo);
       if (!(dek->keylen > 0 && dek->keylen <= DIM(dek->key)))
         BUG ();
-      if (gcry_kdf_derive (pw, strlen (pw),
-                           s2k->mode == 3? GCRY_KDF_ITERSALTED_S2K :
-                           s2k->mode == 1? GCRY_KDF_SALTED_S2K :
-                           /* */           GCRY_KDF_SIMPLE_S2K,
-                           s2k->hash_algo, s2k->salt, 8,
-                           S2K_DECODE_COUNT(s2k->count),
-                           dek->keylen, dek->key))
+      err = gcry_kdf_derive (pw, strlen (pw),
+                             s2k->mode == 3? GCRY_KDF_ITERSALTED_S2K :
+                             s2k->mode == 1? GCRY_KDF_SALTED_S2K :
+                             /* */           GCRY_KDF_SIMPLE_S2K,
+                             s2k->hash_algo, s2k->salt, 8,
+                             S2K_DECODE_COUNT(s2k->count),
+                             dek->keylen, dek->key);
+      if (err)
         {
+          log_error ("gcry_kdf_derive failed: %s", gpg_strerror (err));
           xfree (pw);
           xfree (dek);
 	  write_status( STATUS_MISSING_PASSPHRASE );