[git] GnuPG - branch, master, updated. gnupg-2.2.1-28-g645f30a

by Werner Koch cvs at cvs.gnupg.org
Wed Oct 18 18:45:10 CEST 2017


This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "The GNU Privacy Guard".

The branch, master has been updated
       via  645f30ad310a518a863eb7bd3e11251a7e7f2eca (commit)
       via  3bb06531d38b85be295308e826a50a1a7ba935ec (commit)
       via  d353287f721ffb56627d55bef04cc770ff0a8681 (commit)
      from  18e5946aef458cd95fdce4a04e144747b52b0472 (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 645f30ad310a518a863eb7bd3e11251a7e7f2eca
Author: Werner Koch <wk at gnupg.org>
Date:   Wed Oct 18 18:28:52 2017 +0200

    gpg: Keep a lock during the read-update/insert cycle in import.
    
    * g10/keydb.c (keydb_handle): New field 'keep_lock'.
    (keydb_release): Clear that flag.
    (keydb_lock): New function.
    (unlock_all): Skip if KEEP_LOCK is set.
    * g10/getkey.c (get_keyblock_byfprint_fast): Call keep_lock if
    requested.
    --
    
    That change is straightforward.  It helps to avoid the race condition
    that another gpg process inserts a key while the first process is
    between the search and the insert.
    
    A similar change is due for gpgsm.
    
    Note that the key edit operations may still suffer from a race.
    
    GnuPG-bug-id: 3446

diff --git a/g10/getkey.c b/g10/getkey.c
index 6d18d29..7061f51 100644
--- a/g10/getkey.c
+++ b/g10/getkey.c
@@ -1877,14 +1877,25 @@ get_keyblock_byfprint_fast (kbnode_t *r_keyblock, KEYDB_HANDLE *r_hd,
   hd = keydb_new ();
   if (!hd)
     return gpg_error_from_syserror ();
-  if (r_hd)
-    *r_hd = hd;
 
   if (lock)
     {
+      err = keydb_lock (hd);
+      if (err)
+        {
+          /* If locking did not work, we better don't return a handle
+           * at all - there was a reason that locking has been
+           * requested.  */
+          keydb_release (hd);
+          return err;
+        }
       keydb_disable_caching (hd);
     }
 
+  /* Fo all other errors we return the handle.  */
+  if (r_hd)
+    *r_hd = hd;
+
   err = keydb_search_fpr (hd, fprbuf);
   if (gpg_err_code (err) == GPG_ERR_NOT_FOUND)
     {
diff --git a/g10/keydb.c b/g10/keydb.c
index 0f28bc3..58a14a8 100644
--- a/g10/keydb.c
+++ b/g10/keydb.c
@@ -96,6 +96,10 @@ struct keydb_handle
      / keybox_lock, as appropriate).  */
   int locked;
 
+  /* If this flag is set a lock will only be released by
+   * keydb_release.  */
+  int keep_lock;
+
   /* The index into ACTIVE of the resources in which the last search
      result was found.  Initially -1.  */
   int found;
@@ -964,6 +968,7 @@ keydb_release (KEYDB_HANDLE hd)
   log_assert (active_handles > 0);
   active_handles--;
 
+  hd->keep_lock = 0;
   unlock_all (hd);
   for (i=0; i < hd->used; i++)
     {
@@ -985,6 +990,24 @@ keydb_release (KEYDB_HANDLE hd)
 }
 
 
+/* Take a lock on the files immediately and not only during insert or
+ * update.  This lock is released with keydb_release.  */
+gpg_error_t
+keydb_lock (KEYDB_HANDLE hd)
+{
+  gpg_error_t err;
+
+  if (!hd)
+    return gpg_error (GPG_ERR_INV_ARG);
+
+  err = lock_all (hd);
+  if (!err)
+    hd->keep_lock = 1;
+
+  return err;
+}
+
+
 /* Set a flag on the handle to suppress use of cached results.  This
  * is required for updating a keyring and for key listings.  Fixme:
  * Using a new parameter for keydb_new might be a better solution.  */
@@ -1098,7 +1121,7 @@ unlock_all (KEYDB_HANDLE hd)
 {
   int i;
 
-  if (!hd->locked)
+  if (!hd->locked || hd->keep_lock)
     return;
 
   for (i=hd->used-1; i >= 0; i--)
diff --git a/g10/keydb.h b/g10/keydb.h
index b173751..7393768 100644
--- a/g10/keydb.h
+++ b/g10/keydb.h
@@ -154,6 +154,10 @@ KEYDB_HANDLE keydb_new (void);
 /* Free all resources owned by the database handle.  */
 void keydb_release (KEYDB_HANDLE hd);
 
+/* Take a lock on the files immediately and not only during insert or
+ * update.  This lock is released with keydb_release.  */
+gpg_error_t keydb_lock (KEYDB_HANDLE hd);
+
 /* Set a flag on the handle to suppress use of cached results.  This
    is required for updating a keyring and for key listings.  Fixme:
    Using a new parameter for keydb_new might be a better solution.  */

commit 3bb06531d38b85be295308e826a50a1a7ba935ec
Author: Werner Koch <wk at gnupg.org>
Date:   Wed Oct 18 17:52:41 2017 +0200

    gpg: Improve keydb handling in the main import function.
    
    * g10/getkey.c (get_pubkey_byfprint_fast): Factor most code out to ...
    (get_keyblock_byfprint_fast): .. new function.
    * g10/import.c (revocation_present): s/int rc/gpg_error_t err/.
    (import_one): Use get_keyblock_byfprint_fast to get the keyblock and a
    handle.  Remove the now surplus keyblock fetch in the merge branch.
    
    Signed-off-by: Werner Koch <wk at gnupg.org>

diff --git a/g10/getkey.c b/g10/getkey.c
index 486330a..6d18d29 100644
--- a/g10/getkey.c
+++ b/g10/getkey.c
@@ -1828,16 +1828,47 @@ get_pubkey_byfprint (ctrl_t ctrl, PKT_public_key *pk, kbnode_t *r_keyblock,
  *
  * Like get_pubkey_byfprint, PK may be NULL.  In that case, this
  * function effectively just checks for the existence of the key.  */
-int
+gpg_error_t
 get_pubkey_byfprint_fast (PKT_public_key * pk,
 			  const byte * fprint, size_t fprint_len)
 {
-  int rc = 0;
-  KEYDB_HANDLE hd;
+  gpg_error_t err;
   KBNODE keyblock;
+
+  err = get_keyblock_byfprint_fast (&keyblock, NULL, fprint, fprint_len, 0);
+  if (!err)
+    {
+      if (pk)
+        copy_public_key (pk, keyblock->pkt->pkt.public_key);
+      release_kbnode (keyblock);
+    }
+
+  return err;
+}
+
+
+/* This function is similar to get_pubkey_byfprint_fast but returns a
+ * keydb handle at R_HD and the keyblock at R_KEYBLOCK.  R_KEYBLOCK or
+ * R_HD may be NULL.  If LOCK is set the handle has been opend in
+ * locked mode and keydb_disable_caching () has been called.  On error
+ * R_KEYBLOCK is set to NULL but R_HD must be released by the caller;
+ * it may have a value of NULL, though.  This allows to do an insert
+ * operation on a locked keydb handle.  */
+gpg_error_t
+get_keyblock_byfprint_fast (kbnode_t *r_keyblock, KEYDB_HANDLE *r_hd,
+                            const byte *fprint, size_t fprint_len, int lock)
+{
+  gpg_error_t err;
+  KEYDB_HANDLE hd;
+  kbnode_t keyblock;
   byte fprbuf[MAX_FINGERPRINT_LEN];
   int i;
 
+  if (r_keyblock)
+    *r_keyblock = NULL;
+  if (r_hd)
+    *r_hd = NULL;
+
   for (i = 0; i < MAX_FINGERPRINT_LEN && i < fprint_len; i++)
     fprbuf[i] = fprint[i];
   while (i < MAX_FINGERPRINT_LEN)
@@ -1846,33 +1877,48 @@ get_pubkey_byfprint_fast (PKT_public_key * pk,
   hd = keydb_new ();
   if (!hd)
     return gpg_error_from_syserror ();
+  if (r_hd)
+    *r_hd = hd;
 
-  rc = keydb_search_fpr (hd, fprbuf);
-  if (gpg_err_code (rc) == GPG_ERR_NOT_FOUND)
+  if (lock)
     {
-      keydb_release (hd);
-      return GPG_ERR_NO_PUBKEY;
+      keydb_disable_caching (hd);
     }
-  rc = keydb_get_keyblock (hd, &keyblock);
-  keydb_release (hd);
-  if (rc)
+
+  err = keydb_search_fpr (hd, fprbuf);
+  if (gpg_err_code (err) == GPG_ERR_NOT_FOUND)
     {
-      log_error ("keydb_get_keyblock failed: %s\n", gpg_strerror (rc));
-      return GPG_ERR_NO_PUBKEY;
+      if (!r_hd)
+        keydb_release (hd);
+      return gpg_error (GPG_ERR_NO_PUBKEY);
+    }
+  err = keydb_get_keyblock (hd, &keyblock);
+  if (err)
+    {
+      log_error ("keydb_get_keyblock failed: %s\n", gpg_strerror (err));
+      if (!r_hd)
+        keydb_release (hd);
+      return gpg_error (GPG_ERR_NO_PUBKEY);
     }
 
   log_assert (keyblock->pkt->pkttype == PKT_PUBLIC_KEY
               || keyblock->pkt->pkttype == PKT_PUBLIC_SUBKEY);
-  if (pk)
-    copy_public_key (pk, keyblock->pkt->pkt.public_key);
-  release_kbnode (keyblock);
 
   /* Not caching key here since it won't have all of the fields
      properly set. */
 
+  if (r_keyblock)
+    *r_keyblock = keyblock;
+  else
+    release_kbnode (keyblock);
+
+  if (!r_hd)
+    keydb_release (hd);
+
   return 0;
 }
 
+
 const char *
 parse_def_secret_key (ctrl_t ctrl)
 {
diff --git a/g10/import.c b/g10/import.c
index e785002..8dd6b50 100644
--- a/g10/import.c
+++ b/g10/import.c
@@ -1618,7 +1618,6 @@ import_one (ctrl_t ctrl,
 {
   gpg_error_t err = 0;
   PKT_public_key *pk;
-  PKT_public_key *pk_orig = NULL;
   kbnode_t node, uidnode;
   kbnode_t keyblock_orig = NULL;
   byte fpr2[MAX_FINGERPRINT_LEN];
@@ -1800,16 +1799,15 @@ import_one (ctrl_t ctrl,
     goto leave;
 
   /* Do we have this key already in one of our pubrings ? */
-  hd = keydb_new ();
-  if (!hd)
-    return gpg_error_from_syserror ();
-  keydb_disable_caching (hd);
-  pk_orig = xmalloc_clear( sizeof *pk_orig );
-  err = get_pubkey_byfprint_fast (pk_orig, fpr2, fpr2len);
-  if (err
-      && gpg_err_code (err) != GPG_ERR_NO_PUBKEY
-      && gpg_err_code (err) != GPG_ERR_UNUSABLE_PUBKEY )
-    {
+  err = get_keyblock_byfprint_fast (&keyblock_orig, &hd,
+                                    fpr2, fpr2len, 1/*locked*/);
+  if ((err
+       && gpg_err_code (err) != GPG_ERR_NO_PUBKEY
+       && gpg_err_code (err) != GPG_ERR_UNUSABLE_PUBKEY)
+      || !hd)
+    {
+      /* The !hd above is to catch a misbehaving function which
+       * returns NO_PUBKEY for failing to allocate a handle.  */
       if (!silent)
         log_error (_("key %s: public key not found: %s\n"),
                    keystr(keyid), gpg_strerror (err));
@@ -1823,6 +1821,7 @@ import_one (ctrl_t ctrl,
     }
   else if (err)  /* Insert this key. */
     {
+      /* Note: ERR can only be NO_PUBKEY or UNUSABLE_PUBKEY.  */
       int n_sigs_cleaned, n_uids_cleaned;
 
       err = keydb_locate_writable (hd);
@@ -1892,45 +1891,26 @@ import_one (ctrl_t ctrl,
       stats->imported++;
       new_key = 1;
     }
-  else /* Merge the key.  */
+  else /* Key already exists - merge.  */
     {
       int n_uids, n_sigs, n_subk, n_sigs_cleaned, n_uids_cleaned;
       u32 curtime = make_timestamp ();
 
       /* Compare the original against the new key; just to be sure nothing
        * weird is going on */
-      if (cmp_public_keys( pk_orig, pk ) )
+      if (cmp_public_keys (keyblock_orig->pkt->pkt.public_key, pk))
         {
           if (!silent)
             log_error( _("key %s: doesn't match our copy\n"),keystr(keyid));
           goto leave;
         }
 
-      /* Now read the original keyblock again so that we can use the
-       * handle for updating the keyblock.  FIXME: Why not let
-       * get_pubkey_byfprint_fast do that - it fetches the keyblock
-       * anyway. */
-      err = keydb_search_fpr (hd, fpr2);
-      if (err)
-        {
-          log_error (_("key %s: can't locate original keyblock: %s\n"),
-                     keystr(keyid), gpg_strerror (err));
-          goto leave;
-        }
-      err = keydb_get_keyblock (hd, &keyblock_orig);
-      if (err)
-        {
-          log_error (_("key %s: can't read original keyblock: %s\n"),
-                     keystr(keyid), gpg_strerror (err));
-          goto leave;
-        }
-
       /* Make sure the original direct key sigs are all sane.  */
       n_sigs_cleaned = fix_bad_direct_key_sigs (ctrl, keyblock_orig, keyid);
       if (n_sigs_cleaned)
         commit_kbnode (&keyblock_orig);
 
-      /* and try to merge the block */
+      /* Try to merge KEYBLOCK into KEYBLOCK_ORIG.  */
       clear_kbnode_flags( keyblock_orig );
       clear_kbnode_flags( keyblock );
       n_uids = n_sigs = n_subk = n_uids_cleaned = 0;
@@ -2091,7 +2071,6 @@ import_one (ctrl_t ctrl,
     }
 
   release_kbnode( keyblock_orig );
-  free_public_key( pk_orig );
 
   return err;
 }
@@ -3266,14 +3245,15 @@ revocation_present (ctrl_t ctrl, kbnode_t keyblock)
 		      /* Okay, we have a revocation key, and a
                        * revocation issued by it.  Do we have the key
                        * itself?  */
-		      int rc;
+                      gpg_error_t err;
 
-		      rc=get_pubkey_byfprint_fast (NULL,sig->revkey[idx].fpr,
-                                                   MAX_FINGERPRINT_LEN);
-		      if (gpg_err_code (rc) == GPG_ERR_NO_PUBKEY
-                          || gpg_err_code (rc) == GPG_ERR_UNUSABLE_PUBKEY)
+		      err = get_pubkey_byfprint_fast (NULL,
+                                                      sig->revkey[idx].fpr,
+                                                      MAX_FINGERPRINT_LEN);
+		      if (gpg_err_code (err) == GPG_ERR_NO_PUBKEY
+                          || gpg_err_code (err) == GPG_ERR_UNUSABLE_PUBKEY)
 			{
-			  char *tempkeystr=xstrdup(keystr_from_pk(pk));
+			  char *tempkeystr = xstrdup (keystr_from_pk (pk));
 
 			  /* No, so try and get it */
 			  if ((opt.keyserver_options.options
@@ -3289,13 +3269,13 @@ revocation_present (ctrl_t ctrl, kbnode_t keyblock)
                                                        opt.keyserver, 0);
 
 			      /* Do we have it now? */
-			      rc=get_pubkey_byfprint_fast (NULL,
+			      err = get_pubkey_byfprint_fast (NULL,
 						     sig->revkey[idx].fpr,
 						     MAX_FINGERPRINT_LEN);
 			    }
 
-			  if (gpg_err_code (rc) == GPG_ERR_NO_PUBKEY
-                              || gpg_err_code (rc) == GPG_ERR_UNUSABLE_PUBKEY)
+			  if (gpg_err_code (err) == GPG_ERR_NO_PUBKEY
+                              || gpg_err_code (err) == GPG_ERR_UNUSABLE_PUBKEY)
 			    log_info(_("WARNING: key %s may be revoked:"
 				       " revocation key %s not present.\n"),
 				     tempkeystr,keystr(keyid));
diff --git a/g10/keydb.h b/g10/keydb.h
index f503c99..b173751 100644
--- a/g10/keydb.h
+++ b/g10/keydb.h
@@ -339,8 +339,17 @@ int get_pubkey_byfprint (ctrl_t ctrl, PKT_public_key *pk, kbnode_t *r_keyblock,
 /* This function is similar to get_pubkey_byfprint, but it doesn't
    merge the self-signed data into the public key and subkeys or into
    the user ids.  */
-int get_pubkey_byfprint_fast (PKT_public_key *pk,
-                              const byte *fprint, size_t fprint_len);
+gpg_error_t get_pubkey_byfprint_fast (PKT_public_key *pk,
+                                      const byte *fprint, size_t fprint_len);
+
+/* This function is similar to get_pubkey_byfprint, but it doesn't
+   merge the self-signed data into the public key and subkeys or into
+   the user ids.  */
+gpg_error_t get_keyblock_byfprint_fast (kbnode_t *r_keyblock,
+                                        KEYDB_HANDLE *r_hd,
+                                        const byte *fprint, size_t fprint_len,
+                                        int lock);
+
 
 /* Returns true if a secret key is available for the public key with
    key id KEYID.  */

commit d353287f721ffb56627d55bef04cc770ff0a8681
Author: Werner Koch <wk at gnupg.org>
Date:   Wed Oct 18 13:09:47 2017 +0200

    gpg: Simplify keydb handling of the main import function.
    
    * g10/import.c (import_keys_internal): Return gpg_error_t instead of
    int.  Change var names.
    (import_keys_es_stream): Ditto.
    (import_one): Ditto.  Use a single keydb_new and simplify the use of
    of keydb_release.
    --
    
    Note that this opens a keydb handle before we call
    get_pubkey_byfprint_fast which internally uses another key db handle.
    A further patch will cleanup this double use.  Note that we also
    disable the keydb caching for the insert case.
    
    The s/int/gpg_error_t/ has been done while checking the call chains of
    the import functions and making sure that gpg_err_code is always used.
    
    Signed-off-by: Werner Koch <wk at gnupg.org>

diff --git a/g10/import.c b/g10/import.c
index 5b55f8f..e785002 100644
--- a/g10/import.c
+++ b/g10/import.c
@@ -102,7 +102,7 @@ static int import (ctrl_t ctrl,
 static int read_block (IOBUF a, int with_meta,
                        PACKET **pending_pkt, kbnode_t *ret_root, int *r_v3keys);
 static void revocation_present (ctrl_t ctrl, kbnode_t keyblock);
-static int import_one (ctrl_t ctrl,
+static gpg_error_t import_one (ctrl_t ctrl,
                        kbnode_t keyblock,
                        struct import_stats_s *stats,
                        unsigned char **fpr, size_t *fpr_len,
@@ -436,7 +436,7 @@ read_key_from_file (ctrl_t ctrl, const char *fname, kbnode_t *r_keyblock)
  *
  *  Key revocation certificates have special handling.
  */
-static int
+static gpg_error_t
 import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames,
 		      import_stats_t stats_handle,
                       unsigned char **fpr, size_t *fpr_len,
@@ -445,7 +445,7 @@ import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames,
                       int origin, const char *url)
 {
   int i;
-  int rc = 0;
+  gpg_error_t err = 0;
   struct import_stats_s *stats = stats_handle;
 
   if (!stats)
@@ -453,8 +453,8 @@ import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames,
 
   if (inp)
     {
-      rc = import (ctrl, inp, "[stream]", stats, fpr, fpr_len, options,
-                   screener, screener_arg, origin, url);
+      err = import (ctrl, inp, "[stream]", stats, fpr, fpr_len, options,
+                    screener, screener_arg, origin, url);
     }
   else
     {
@@ -478,14 +478,14 @@ import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames,
             log_error (_("can't open '%s': %s\n"), fname, strerror (errno));
           else
             {
-              rc = import (ctrl, inp2, fname, stats, fpr, fpr_len, options,
+              err = import (ctrl, inp2, fname, stats, fpr, fpr_len, options,
                            screener, screener_arg, origin, url);
               iobuf_close (inp2);
               /* Must invalidate that ugly cache to actually close it. */
               iobuf_ioctl (NULL, IOBUF_IOCTL_INVALIDATE_CACHE, 0, (char*)fname);
-              if (rc)
+              if (err)
                 log_error ("import from '%s' failed: %s\n",
-                           fname, gpg_strerror (rc) );
+                           fname, gpg_strerror (err) );
             }
           if (!fname)
             break;
@@ -507,7 +507,7 @@ import_keys_internal (ctrl_t ctrl, iobuf_t inp, char **fnames, int nnames,
   if (!(options & IMPORT_FAST))
     check_or_update_trustdb (ctrl);
 
-  return rc;
+  return err;
 }
 
 
@@ -521,7 +521,7 @@ import_keys (ctrl_t ctrl, char **fnames, int nnames,
 }
 
 
-int
+gpg_error_t
 import_keys_es_stream (ctrl_t ctrl, estream_t fp,
                        import_stats_t stats_handle,
                        unsigned char **fpr, size_t *fpr_len,
@@ -529,23 +529,23 @@ import_keys_es_stream (ctrl_t ctrl, estream_t fp,
                        import_screener_t screener, void *screener_arg,
                        int origin, const char *url)
 {
-  int rc;
+  gpg_error_t err;
   iobuf_t inp;
 
   inp = iobuf_esopen (fp, "rb", 1);
   if (!inp)
     {
-      rc = gpg_error_from_syserror ();
-      log_error ("iobuf_esopen failed: %s\n", gpg_strerror (rc));
-      return rc;
+      err = gpg_error_from_syserror ();
+      log_error ("iobuf_esopen failed: %s\n", gpg_strerror (err));
+      return err;
     }
 
-  rc = import_keys_internal (ctrl, inp, NULL, 0, stats_handle,
+  err = import_keys_internal (ctrl, inp, NULL, 0, stats_handle,
                              fpr, fpr_len, options,
                              screener, screener_arg, origin, url);
 
   iobuf_close (inp);
-  return rc;
+  return err;
 }
 
 
@@ -1608,7 +1608,7 @@ update_key_origin (kbnode_t keyblock, u32 curtime, int origin, const char *url)
  * even most error messages are suppressed.  ORIGIN is the origin of
  * the key (0 for unknown) and URL the corresponding URL.
  */
-static int
+static gpg_error_t
 import_one (ctrl_t ctrl,
             kbnode_t keyblock, struct import_stats_s *stats,
 	    unsigned char **fpr, size_t *fpr_len, unsigned int options,
@@ -1616,6 +1616,7 @@ import_one (ctrl_t ctrl,
             import_screener_t screener, void *screener_arg,
             int origin, const char *url)
 {
+  gpg_error_t err = 0;
   PKT_public_key *pk;
   PKT_public_key *pk_orig = NULL;
   kbnode_t node, uidnode;
@@ -1623,7 +1624,6 @@ import_one (ctrl_t ctrl,
   byte fpr2[MAX_FINGERPRINT_LEN];
   size_t fpr2len;
   u32 keyid[2];
-  int rc = 0;
   int new_key = 0;
   int mod_key = 0;
   int same_key = 0;
@@ -1632,6 +1632,7 @@ import_one (ctrl_t ctrl,
   char pkstrbuf[PUBKEY_STRING_SIZE];
   int merge_keys_done = 0;
   int any_filter = 0;
+  KEYDB_HANDLE hd = NULL;
 
   /* Get the key and print some info about it. */
   node = find_kbnode( keyblock, PKT_PUBLIC_KEY );
@@ -1791,7 +1792,7 @@ import_one (ctrl_t ctrl,
           merge_keys_and_selfsig (ctrl, keyblock);
           merge_keys_done = 1;
         }
-      rc = write_keyblock_to_output (keyblock, opt.armor, opt.export_options);
+      err = write_keyblock_to_output (keyblock, opt.armor, opt.export_options);
       goto leave;
     }
 
@@ -1799,37 +1800,37 @@ import_one (ctrl_t ctrl,
     goto leave;
 
   /* Do we have this key already in one of our pubrings ? */
+  hd = keydb_new ();
+  if (!hd)
+    return gpg_error_from_syserror ();
+  keydb_disable_caching (hd);
   pk_orig = xmalloc_clear( sizeof *pk_orig );
-  rc = get_pubkey_byfprint_fast (pk_orig, fpr2, fpr2len);
-  if (rc && gpg_err_code (rc) != GPG_ERR_NO_PUBKEY
-      && gpg_err_code (rc) != GPG_ERR_UNUSABLE_PUBKEY )
+  err = get_pubkey_byfprint_fast (pk_orig, fpr2, fpr2len);
+  if (err
+      && gpg_err_code (err) != GPG_ERR_NO_PUBKEY
+      && gpg_err_code (err) != GPG_ERR_UNUSABLE_PUBKEY )
     {
       if (!silent)
         log_error (_("key %s: public key not found: %s\n"),
-                   keystr(keyid), gpg_strerror (rc));
+                   keystr(keyid), gpg_strerror (err));
     }
-  else if ( rc && (opt.import_options&IMPORT_MERGE_ONLY) )
+  else if (err && (opt.import_options&IMPORT_MERGE_ONLY) )
     {
       if (opt.verbose && !silent )
         log_info( _("key %s: new key - skipped\n"), keystr(keyid));
-      rc = 0;
+      err = 0;
       stats->skipped_new_keys++;
     }
-  else if (rc )  /* Insert this key. */
+  else if (err)  /* Insert this key. */
     {
-      KEYDB_HANDLE hd;
       int n_sigs_cleaned, n_uids_cleaned;
 
-      hd = keydb_new ();
-      if (!hd)
-        return gpg_error_from_syserror ();
-
-      rc = keydb_locate_writable (hd);
-      if (rc)
+      err = keydb_locate_writable (hd);
+      if (err)
         {
-          log_error (_("no writable keyring found: %s\n"), gpg_strerror (rc));
-          keydb_release (hd);
-          return GPG_ERR_GENERAL;
+          log_error (_("no writable keyring found: %s\n"), gpg_strerror (err));
+          err = gpg_error (GPG_ERR_GENERAL);
+          goto leave;
 	}
       if (opt.verbose > 1 )
         log_info (_("writing to '%s'\n"), keydb_get_resource_name (hd) );
@@ -1843,19 +1844,19 @@ import_one (ctrl_t ctrl,
        * and thus the address of KEYBLOCK won't change.  */
       if ( !(options & IMPORT_RESTORE) )
         {
-          rc = insert_key_origin (keyblock, origin, url);
-          if (rc)
+          err = insert_key_origin (keyblock, origin, url);
+          if (err)
             {
-              log_error ("insert_key_origin failed: %s\n", gpg_strerror (rc));
-              keydb_release (hd);
-              return GPG_ERR_GENERAL;
+              log_error ("insert_key_origin failed: %s\n", gpg_strerror (err));
+              err = gpg_error (GPG_ERR_GENERAL);
+              goto leave;
             }
         }
 
-      rc = keydb_insert_keyblock (hd, keyblock );
-      if (rc)
+      err = keydb_insert_keyblock (hd, keyblock );
+      if (err)
         log_error (_("error writing keyring '%s': %s\n"),
-                   keydb_get_resource_name (hd), gpg_strerror (rc));
+                   keydb_get_resource_name (hd), gpg_strerror (err));
       else if (!(opt.import_options & IMPORT_KEEP_OWNERTTRUST))
         {
           /* This should not be possible since we delete the
@@ -1868,7 +1869,10 @@ import_one (ctrl_t ctrl,
           if (non_self)
             revalidation_mark (ctrl);
         }
+
+      /* Release the handle and thus unlock the keyring asap.  */
       keydb_release (hd);
+      hd = NULL;
 
       /* We are ready.  */
       if (!opt.quiet && !silent)
@@ -1890,7 +1894,6 @@ import_one (ctrl_t ctrl,
     }
   else /* Merge the key.  */
     {
-      KEYDB_HANDLE hd;
       int n_uids, n_sigs, n_subk, n_sigs_cleaned, n_uids_cleaned;
       u32 curtime = make_timestamp ();
 
@@ -1903,29 +1906,22 @@ import_one (ctrl_t ctrl,
           goto leave;
         }
 
-      /* Now read the original keyblock again so that we can use
-         that handle for updating the keyblock.  */
-      hd = keydb_new ();
-      if (!hd)
-        {
-          rc = gpg_error_from_syserror ();
-          goto leave;
-        }
-      keydb_disable_caching (hd);
-      rc = keydb_search_fpr (hd, fpr2);
-      if (rc )
+      /* Now read the original keyblock again so that we can use the
+       * handle for updating the keyblock.  FIXME: Why not let
+       * get_pubkey_byfprint_fast do that - it fetches the keyblock
+       * anyway. */
+      err = keydb_search_fpr (hd, fpr2);
+      if (err)
         {
           log_error (_("key %s: can't locate original keyblock: %s\n"),
-                     keystr(keyid), gpg_strerror (rc));
-          keydb_release (hd);
+                     keystr(keyid), gpg_strerror (err));
           goto leave;
         }
-      rc = keydb_get_keyblock (hd, &keyblock_orig);
-      if (rc)
+      err = keydb_get_keyblock (hd, &keyblock_orig);
+      if (err)
         {
           log_error (_("key %s: can't read original keyblock: %s\n"),
-                     keystr(keyid), gpg_strerror (rc));
-          keydb_release (hd);
+                     keystr(keyid), gpg_strerror (err));
           goto leave;
         }
 
@@ -1938,14 +1934,11 @@ import_one (ctrl_t ctrl,
       clear_kbnode_flags( keyblock_orig );
       clear_kbnode_flags( keyblock );
       n_uids = n_sigs = n_subk = n_uids_cleaned = 0;
-      rc = merge_blocks (ctrl, options, keyblock_orig, keyblock, keyid,
-                         curtime, origin, url,
-                         &n_uids, &n_sigs, &n_subk );
-      if (rc )
-        {
-          keydb_release (hd);
-          goto leave;
-        }
+      err = merge_blocks (ctrl, options, keyblock_orig, keyblock, keyid,
+                          curtime, origin, url,
+                          &n_uids, &n_sigs, &n_subk );
+      if (err)
+        goto leave;
 
       if ((options & IMPORT_CLEAN))
         clean_key (ctrl, keyblock_orig, opt.verbose, (options&IMPORT_MINIMAL),
@@ -1958,25 +1951,28 @@ import_one (ctrl_t ctrl,
            * and thus the address of KEYBLOCK won't change.  */
           if ( !(options & IMPORT_RESTORE) )
             {
-              rc = update_key_origin (keyblock_orig, curtime, origin, url);
-              if (rc)
+              err = update_key_origin (keyblock_orig, curtime, origin, url);
+              if (err)
                 {
                   log_error ("update_key_origin failed: %s\n",
-                             gpg_strerror (rc));
-                  keydb_release (hd);
+                             gpg_strerror (err));
                   goto leave;
                 }
             }
 
           mod_key = 1;
           /* KEYBLOCK_ORIG has been updated; write */
-          rc = keydb_update_keyblock (ctrl, hd, keyblock_orig);
-          if (rc)
+          err = keydb_update_keyblock (ctrl, hd, keyblock_orig);
+          if (err)
             log_error (_("error writing keyring '%s': %s\n"),
-                       keydb_get_resource_name (hd), gpg_strerror (rc) );
+                       keydb_get_resource_name (hd), gpg_strerror (err));
           else if (non_self)
             revalidation_mark (ctrl);
 
+          /* Release the handle and thus unlock the keyring asap.  */
+          keydb_release (hd);
+          hd = NULL;
+
           /* We are ready.  */
           if (!opt.quiet && !silent)
             {
@@ -2025,6 +2021,10 @@ import_one (ctrl_t ctrl,
 	}
       else
         {
+          /* Release the handle and thus unlock the keyring asap.  */
+          keydb_release (hd);
+          hd = NULL;
+
           /* Fixme: we do not track the time we last checked a key for
            * updates.  To do this we would need to rewrite even the
            * keys which have no changes.  */
@@ -2041,11 +2041,10 @@ import_one (ctrl_t ctrl,
 
           stats->unchanged++;
         }
-
-      keydb_release (hd); hd = NULL;
     }
 
  leave:
+  keydb_release (hd);
   if (mod_key || new_key || same_key)
     {
       /* A little explanation for this: we fill in the fingerprint
@@ -2094,7 +2093,7 @@ import_one (ctrl_t ctrl,
   release_kbnode( keyblock_orig );
   free_public_key( pk_orig );
 
-  return rc;
+  return err;
 }
 
 
diff --git a/g10/main.h b/g10/main.h
index 4850360..4a8f8c3 100644
--- a/g10/main.h
+++ b/g10/main.h
@@ -356,7 +356,7 @@ gpg_error_t read_key_from_file (ctrl_t ctrl, const char *fname,
 void import_keys (ctrl_t ctrl, char **fnames, int nnames,
 		  import_stats_t stats_hd, unsigned int options,
                   int origin, const char *url);
-int import_keys_es_stream (ctrl_t ctrl, estream_t fp,
+gpg_error_t import_keys_es_stream (ctrl_t ctrl, estream_t fp,
                            import_stats_t stats_handle,
                            unsigned char **fpr, size_t *fpr_len,
                            unsigned int options,

-----------------------------------------------------------------------

Summary of changes:
 g10/getkey.c |  87 ++++++++++++++++++++++-----
 g10/import.c | 189 ++++++++++++++++++++++++++---------------------------------
 g10/keydb.c  |  25 +++++++-
 g10/keydb.h  |  17 +++++-
 g10/main.h   |   2 +-
 5 files changed, 196 insertions(+), 124 deletions(-)


hooks/post-receive
-- 
The GNU Privacy Guard
http://git.gnupg.org




More information about the Gnupg-commits mailing list