gpg: Make sure to mark a duplicate registered keybox as primary.

* kbx/keybox-init.c (keybox_register_file): Change interface to return
the token even if the file has already been registered.
* g10/keydb.c (primary_keyring): Rename to primary_keydb.
(maybe_create_keyring_or_box): Change return type to gpg_error_t.
(keydb_add_resource): Ditto. s/rc/err/.
(keydb_add_resource): Mark an already registered as primary.
* sm/keydb.c (maybe_create_keybox): Change return type to gpg_error_t.
(keydb_add_resource): Ditto. s/rc/err/.
(keydb_add_resource): Adjust for changed keybox_register_file.
--

This change aligns the registering of keyboxes with those of
keyrings.  This fixes a potential bug:

  gpg --keyring foo.kbx --keyring bar.gpg --keyring foo.kbx

would have marked bar.gpg as primary resource and thus inserting new
keys there.  The correct and now fixed behavior is to insert to
foo.kbx.

Signed-off-by: Werner Koch <wk@gnupg.org>

g10/keydb.c

@@ -60,7 +60,10 @@ struct resource_item
 
 static struct resource_item all_resources[MAX_KEYDB_RESOURCES];
 static int used_resources;
-static void *primary_keyring=NULL;
+
+/* A pointer used to check for the primary key database by comparing
+   to the struct resource_item's TOKEN.  */
+static void *primary_keydb;
 
 
 /* This is a simple cache used to return the last result of a
@@ -261,7 +264,7 @@ keyblock_cache_clear (struct keydb_handle *hd)
    the keyring or keybox will be created.
 
    Return 0 if it is okay to access the specified file.  */
-static int
+static gpg_error_t
 maybe_create_keyring_or_box (char *filename, int is_box, int force_create)
 {
   dotlock_t lockhd = NULL;
@@ -592,7 +595,7 @@ keydb_add_resource (const char *url, unsigned int flags)
   int read_only = !!(flags&KEYDB_RESOURCE_FLAG_READONLY);
   int is_default = !!(flags&KEYDB_RESOURCE_FLAG_DEFAULT);
   int is_gpgvdef = !!(flags&KEYDB_RESOURCE_FLAG_GPGVDEF);
-  int rc = 0;
+  gpg_error_t err = 0;
   KeydbResourceType rt = KEYDB_RESOURCE_TYPE_NONE;
   void *token;
 
@@ -613,7 +616,7 @@ keydb_add_resource (const char *url, unsigned int flags)
   else if (strchr (resname, ':'))
     {
       log_error ("invalid key resource URL '%s'\n", url );
-      rc = gpg_error (GPG_ERR_GENERAL);
+      err = gpg_error (GPG_ERR_GENERAL);
       goto leave;
     }
 #endif /* !HAVE_DRIVE_LETTERS && !__riscos__ */
@@ -708,22 +711,22 @@ keydb_add_resource (const char *url, unsigned int flags)
     {
     case KEYDB_RESOURCE_TYPE_NONE:
       log_error ("unknown type of key resource '%s'\n", url );
-      rc = gpg_error (GPG_ERR_GENERAL);
+      err = gpg_error (GPG_ERR_GENERAL);
       goto leave;
 
     case KEYDB_RESOURCE_TYPE_KEYRING:
-      rc = maybe_create_keyring_or_box (filename, 0, create);
+      err = maybe_create_keyring_or_box (filename, 0, create);
-      if (rc)
+      if (err)
         goto leave;
 
       if (keyring_register_filename (filename, read_only, &token))
         {
           if (used_resources >= MAX_KEYDB_RESOURCES)
-            rc = gpg_error (GPG_ERR_RESOURCE_LIMIT);
+            err = gpg_error (GPG_ERR_RESOURCE_LIMIT);
           else
             {
               if ((flags & KEYDB_RESOURCE_FLAG_PRIMARY))
-                primary_keyring = token;
+                primary_keydb = token;
               all_resources[used_resources].type = rt;
               all_resources[used_resources].u.kr = NULL; /* Not used here */
               all_resources[used_resources].token = token;
@@ -736,26 +739,25 @@ keydb_add_resource (const char *url, unsigned int flags)
              However, we can still mark it as primary even if it was
              already registered.  */
           if ((flags & KEYDB_RESOURCE_FLAG_PRIMARY))
-            primary_keyring = token;
+            primary_keydb = token;
         }
       break;
 
     case KEYDB_RESOURCE_TYPE_KEYBOX:
       {
-        rc = maybe_create_keyring_or_box (filename, 1, create);
+        err = maybe_create_keyring_or_box (filename, 1, create);
-        if (rc)
+        if (err)
           goto leave;
 
-        /* FIXME: How do we register a read-only keybox?  */
+        err = keybox_register_file (filename, 0, &token);
-        token = keybox_register_file (filename, 0);
+        if (!err)
-        if (token)
           {
             if (used_resources >= MAX_KEYDB_RESOURCES)
-              rc = gpg_error (GPG_ERR_RESOURCE_LIMIT);
+              err = gpg_error (GPG_ERR_RESOURCE_LIMIT);
             else
               {
-                /* if ((flags & KEYDB_RESOURCE_FLAG_PRIMARY)) */
+                if ((flags & KEYDB_RESOURCE_FLAG_PRIMARY))
-                /*   primary_keyring = token; */
+                  primary_keydb = token;
                 all_resources[used_resources].type = rt;
                 all_resources[used_resources].u.kb = NULL; /* Not used here */
                 all_resources[used_resources].token = token;
@@ -766,32 +768,31 @@ keydb_add_resource (const char *url, unsigned int flags)
                 used_resources++;
               }
           }
-        else
+        else if (gpg_err_code (err) == GPG_ERR_EEXIST)
           {
             /* Already registered.  We will mark it as the primary key
                if requested.  */
-            /* FIXME: How to do that?  Change the keybox interface?  */
+            if ((flags & KEYDB_RESOURCE_FLAG_PRIMARY))
-            /* if ((flags & KEYDB_RESOURCE_FLAG_PRIMARY)) */
+              primary_keydb = token;
-            /*   primary_keyring = token; */
           }
       }
       break;
 
       default:
 	log_error ("resource type of '%s' not supported\n", url);
-	rc = gpg_error (GPG_ERR_GENERAL);
+	err = gpg_error (GPG_ERR_GENERAL);
 	goto leave;
     }
 
   /* fixme: check directory permissions and print a warning */
 
  leave:
-  if (rc)
+  if (err)
-    log_error (_("keyblock resource '%s': %s\n"), filename, gpg_strerror (rc));
+    log_error (_("keyblock resource '%s': %s\n"), filename, gpg_strerror (err));
   else
     any_registered = 1;
   xfree (filename);
-  return rc;
+  return err;
 }
 
 
@@ -1685,11 +1686,11 @@ keydb_locate_writable (KEYDB_HANDLE hd)
     return rc;
 
   /* If we have a primary set, try that one first */
-  if (primary_keyring)
+  if (primary_keydb)
     {
       for ( ; hd->current >= 0 && hd->current < hd->used; hd->current++)
 	{
-	  if(hd->active[hd->current].token==primary_keyring)
+	  if(hd->active[hd->current].token == primary_keydb)
 	    {
 	      if(keyring_is_writable (hd->active[hd->current].token))
 		return 0;

kbx/keybox-init.c

@@ -30,23 +30,30 @@
 static KB_NAME kb_names;
 
 
-/* Register a filename for plain keybox files.  Returns a pointer to
+/* Register a filename for plain keybox files.  Returns 0 on success,
-   be used to create a handles and so on.  Returns NULL to indicate
+ * GPG_ERR_EEXIST if it has already been registered, or another error
-   that FNAME has already been registered.  */
+ * code.  On success or with error code GPG_ERR_EEXIST a token usable
-void *
+ * to access the keybox handle is stored at R_TOKEN, NULL is stored
-keybox_register_file (const char *fname, int secret)
+ * for all other errors.  */
+gpg_error_t
+keybox_register_file (const char *fname, int secret, void **r_token)
 {
   KB_NAME kr;
 
+  *r_token = NULL;
+
   for (kr=kb_names; kr; kr = kr->next)
     {
       if (same_file_p (kr->fname, fname) )
-        return NULL; /* Already registered. */
+        {
+          *r_token = kr;
+          return gpg_error (GPG_ERR_EEXIST); /* Already registered. */
+        }
     }
 
   kr = xtrymalloc (sizeof *kr + strlen (fname));
   if (!kr)
-    return NULL;
+    return gpg_error_from_syserror ();
   strcpy (kr->fname, fname);
   kr->secret = !!secret;
 
@@ -64,7 +71,8 @@ keybox_register_file (const char *fname, int secret)
 /*      if (!kb_offtbl) */
 /*        kb_offtbl = new_offset_hash_table (); */
 
-  return kr;
+  *r_token = kr;
+  return 0;
 }
 
 int

kbx/keybox.h

@@ -64,7 +64,8 @@ typedef enum
 
 
 /*-- keybox-init.c --*/
-void *keybox_register_file (const char *fname, int secret);
+gpg_error_t keybox_register_file (const char *fname, int secret,
+                                  void **r_token);
 int keybox_is_writable (void *token);
 
 KEYBOX_HANDLE keybox_new_openpgp (void *token, int secret);

sm/keydb.c

@@ -107,7 +107,7 @@ try_make_homedir (const char *fname)
    locked.  This lock check does not work if the directory itself is
    not yet available.  If R_CREATED is not NULL it will be set to true
    if the function created a new keybox.  */
-static int
+static gpg_error_t
 maybe_create_keybox (char *filename, int force, int *r_created)
 {
   dotlock_t lockhd = NULL;
@@ -237,13 +237,13 @@ maybe_create_keybox (char *filename, int force, int *r_created)
  * does not exist.  If AUTO_CREATED is not NULL it will be set to true
  * if the function has created a new keybox.
  */
-int
+gpg_error_t
 keydb_add_resource (const char *url, int force, int secret, int *auto_created)
 {
   static int any_secret, any_public;
   const char *resname = url;
   char *filename = NULL;
-  int rc = 0;
+  gpg_error_t err = 0;
   KeydbResourceType rt = KEYDB_RESOURCE_TYPE_NONE;
 
   if (auto_created)
@@ -264,7 +264,7 @@ keydb_add_resource (const char *url, int force, int secret, int *auto_created)
       else if (strchr (resname, ':'))
         {
           log_error ("invalid key resource URL '%s'\n", url );
-          rc = gpg_error (GPG_ERR_GENERAL);
+          err = gpg_error (GPG_ERR_GENERAL);
           goto leave;
 	}
 #endif /* !HAVE_DRIVE_LETTERS && !__riscos__ */
@@ -312,20 +312,24 @@ keydb_add_resource (const char *url, int force, int secret, int *auto_created)
     {
     case KEYDB_RESOURCE_TYPE_NONE:
       log_error ("unknown type of key resource '%s'\n", url );
-      rc = gpg_error (GPG_ERR_GENERAL);
+      err = gpg_error (GPG_ERR_GENERAL);
       goto leave;
 
     case KEYDB_RESOURCE_TYPE_KEYBOX:
-      rc = maybe_create_keybox (filename, force, auto_created);
+      err = maybe_create_keybox (filename, force, auto_created);
-      if (rc)
+      if (err)
         goto leave;
       /* Now register the file */
       {
-        void *token = keybox_register_file (filename, secret);
+        void *token;
-        if (!token)
+
-          ; /* already registered - ignore it */
+        err = keybox_register_file (filename, secret, &token);
+        if (gpg_err_code (err) == GPG_ERR_EEXIST)
+          ; /* Already registered - ignore.  */
+        else if (err)
+          ; /* Other error.  */
         else if (used_resources >= MAX_KEYDB_RESOURCES)
-          rc = gpg_error (GPG_ERR_RESOURCE_LIMIT);
+          err = gpg_error (GPG_ERR_RESOURCE_LIMIT);
         else
           {
             all_resources[used_resources].type = rt;
@@ -358,21 +362,21 @@ keydb_add_resource (const char *url, int force, int secret, int *auto_created)
 
     default:
       log_error ("resource type of '%s' not supported\n", url);
-      rc = gpg_error (GPG_ERR_NOT_SUPPORTED);
+      err = gpg_error (GPG_ERR_NOT_SUPPORTED);
       goto leave;
     }
 
   /* fixme: check directory permissions and print a warning */
 
  leave:
-  if (rc)
+  if (err)
-    log_error ("keyblock resource '%s': %s\n", filename, gpg_strerror(rc));
+    log_error ("keyblock resource '%s': %s\n", filename, gpg_strerror (err));
   else if (secret)
     any_secret = 1;
   else
     any_public = 1;
   xfree (filename);
-  return rc;
+  return err;
 }
 
 

sm/keydb.h

@@ -31,8 +31,8 @@ typedef struct keydb_handle *KEYDB_HANDLE;
 
 
 /*-- keydb.c --*/
-int keydb_add_resource (const char *url, int force, int secret,
+gpg_error_t keydb_add_resource (const char *url, int force, int secret,
-                        int *auto_created);
+                                int *auto_created);
 KEYDB_HANDLE keydb_new (int secret);
 void keydb_release (KEYDB_HANDLE hd);
 int keydb_set_ephemeral (KEYDB_HANDLE hd, int yes);