gpg: Take care to use pubring.kbx if it has ever been used.
authorWerner Koch <wk@gnupg.org>
Thu, 9 Oct 2014 17:10:32 +0000 (19:10 +0200)
committerWerner Koch <wk@gnupg.org>
Thu, 9 Oct 2014 17:10:32 +0000 (19:10 +0200)
* kbx/keybox-defs.h (struct keybox_handle): Add field for_openpgp.
* kbx/keybox-file.c (_keybox_write_header_blob): Set openpgp header
flag.
* kbx/keybox-blob.c (_keybox_update_header_blob): Add arg for_openpgp
and set header flag.
* kbx/keybox-init.c (keybox_new): Rename to do_keybox_new, make static
and add arg for_openpgp.
(keybox_new_openpgp, keybox_new_x509): New.  Use them instead of the
former keybox_new.
* kbx/keybox-update.c (blob_filecopy): Add arg for_openpgp and set the
openpgp header flags.

* g10/keydb.c (rt_from_file): New.  Factored out and extended from
keydb_add_resource.
(keydb_add_resource): Switch to the kbx file if it has the openpgp
flag set.

* kbx/keybox-dump.c (dump_header_blob): Print header flags.
--

The problem was reported by dkg on gnupg-devel (2014-10-07):

  I just discovered a new problem, though, which will affect people on
  systems that have gpg and gpg2 coinstalled:

   0) create a new keyring with gpg2, and use it exclusively with gpg2
  for a while.
   1) somehow (accidentally?) use gpg (1.4.x) again -- this creates
  ~/.gnupg/pubring.gpg
   2) future runs of gpg2 now only look at pubring.gpg and ignore
  pubring.kbx -- the keys you had accumulated in the keybox are no
  longer listed in the output of gpg2 --list-keys

Note that gpgsm has always used pubring.kbx and thus this file might
already be there but without gpg ever inserted a key.  The new flag in
the KBX header gives us an indication whether a KBX file has ever been
written by gpg >= 2.1.  If that is the case we will use it instead of
the default pubring.gpg.

Signed-off-by: Werner Koch <wk@gnupg.org>
g10/keydb.c
kbx/keybox-blob.c
kbx/keybox-defs.h
kbx/keybox-dump.c
kbx/keybox-file.c
kbx/keybox-init.c
kbx/keybox-update.c
kbx/keybox.h
sm/keydb.c

index 178456a..a387951 100644 (file)
@@ -242,7 +242,7 @@ maybe_create_keyring_or_box (char *filename, int is_box, int force_create)
         rc = gpg_error_from_syserror ();
       else
         {
-          rc = _keybox_write_header_blob (fp);
+          rc = _keybox_write_header_blob (fp, 1);
           fclose (fp);
         }
       if (rc)
@@ -277,6 +277,50 @@ maybe_create_keyring_or_box (char *filename, int is_box, int force_create)
 }
 
 
+/* Helper for keydb_add_resource.  Opens FILENAME to figures out the
+   resource type.  Returns the resource type and a flag at R_NOTFOUND
+   indicating whether FILENAME could be opened at all.  If the openpgp
+   flag is set in a keybox header, R_OPENPGP will be set to true.  */
+static KeydbResourceType
+rt_from_file (const char *filename, int *r_found, int *r_openpgp)
+{
+  u32 magic;
+  unsigned char verbuf[4];
+  FILE *fp;
+  KeydbResourceType rt = KEYDB_RESOURCE_TYPE_NONE;
+
+  *r_found = *r_openpgp = 0;
+  fp = fopen (filename, "rb");
+  if (fp)
+    {
+      *r_found = 1;
+
+      if (fread (&magic, 4, 1, fp) == 1 )
+        {
+          if (magic == 0x13579ace || magic == 0xce9a5713)
+            ; /* GDBM magic - not anymore supported. */
+          else if (fread (&verbuf, 4, 1, fp) == 1
+                   && verbuf[0] == 1
+                   && fread (&magic, 4, 1, fp) == 1
+                   && !memcmp (&magic, "KBXf", 4))
+            {
+              if ((verbuf[3] & 0x02))
+                *r_openpgp = 1;
+              rt = KEYDB_RESOURCE_TYPE_KEYBOX;
+            }
+          else
+            rt = KEYDB_RESOURCE_TYPE_KEYRING;
+        }
+      else /* Maybe empty: assume keyring. */
+        rt = KEYDB_RESOURCE_TYPE_KEYRING;
+
+      fclose (fp);
+    }
+
+  return rt;
+}
+
+
 /*
  * Register a resource (keyring or aeybox).  The first keyring or
  * keybox which is added by this function is created if it does not
@@ -337,33 +381,34 @@ keydb_add_resource (const char *url, unsigned int flags)
   /* See whether we can determine the filetype.  */
   if (rt == KEYDB_RESOURCE_TYPE_NONE)
     {
-      FILE *fp;
+      int found, openpgp_flag;
       int pass = 0;
       size_t filenamelen;
 
     check_again:
       filenamelen = strlen (filename);
-      fp = fopen (filename, "rb");
-      if (fp)
+      rt = rt_from_file (filename, &found, &openpgp_flag);
+      if (found)
         {
-          u32 magic;
-
-          if (fread (&magic, 4, 1, fp) == 1 )
+          /* The file exists and we have the resource type in RT.
+
+             Now let us check whether in addition to the "pubring.gpg"
+             a "pubring.kbx with openpgp keys exists.  This is so that
+             GPG 2.1 will use an existing "pubring.kbx" by default iff
+             that file has been created or used by 2.1.  This check is
+             needed because after creation or use of the kbx file with
+             2.1 an older version of gpg may have created a new
+             pubring.gpg for its own use.  */
+          if (!pass && is_default && rt == KEYDB_RESOURCE_TYPE_KEYRING
+              && filenamelen > 4 && !strcmp (filename+filenamelen-4, ".gpg"))
             {
-              if (magic == 0x13579ace || magic == 0xce9a5713)
-                ; /* GDBM magic - not anymore supported. */
-              else if (fread (&magic, 4, 1, fp) == 1
-                       && !memcmp (&magic, "\x01", 1)
-                       && fread (&magic, 4, 1, fp) == 1
-                       && !memcmp (&magic, "KBXf", 4))
+              strcpy (filename+filenamelen-4, ".kbx");
+              if ((rt_from_file (filename, &found, &openpgp_flag)
+                   == KEYDB_RESOURCE_TYPE_KEYBOX) && found && openpgp_flag)
                 rt = KEYDB_RESOURCE_TYPE_KEYBOX;
-              else
-                rt = KEYDB_RESOURCE_TYPE_KEYRING;
-           }
-          else /* Maybe empty: assume keyring. */
-            rt = KEYDB_RESOURCE_TYPE_KEYRING;
-
-          fclose (fp);
+              else /* Restore filename */
+                strcpy (filename+filenamelen-4, ".gpg");
+            }
        }
       else if (!pass
                && is_default && create
@@ -508,7 +553,7 @@ keydb_new (void)
         case KEYDB_RESOURCE_TYPE_KEYBOX:
           hd->active[j].type   = all_resources[i].type;
           hd->active[j].token  = all_resources[i].token;
-          hd->active[j].u.kb   = keybox_new (all_resources[i].token, 0);
+          hd->active[j].u.kb   = keybox_new_openpgp (all_resources[i].token, 0);
           if (!hd->active[j].u.kb)
             {
               xfree (hd);
index f7abb6c..35ce3e3 100644 (file)
@@ -42,8 +42,9 @@
    - u32  Length of this blob
    - byte Blob type (1)
    - byte Version number (1)
-   - byte RFU
-   - byte RFU
+   - u16  Header flags
+          bit 0 - RFU
+          bit 1 - Is being or has been used for OpenPGP blobs
    - b4   Magic 'KBXf'
    - u32  RFU
    - u32  file_created_at
@@ -1028,7 +1029,7 @@ _keybox_get_blob_fileoffset (KEYBOXBLOB blob)
 
 
 void
-_keybox_update_header_blob (KEYBOXBLOB blob)
+_keybox_update_header_blob (KEYBOXBLOB blob, int for_openpgp)
 {
   if (blob->bloblen >= 32 && blob->blob[4] == BLOBTYPE_HEADER)
     {
@@ -1039,5 +1040,8 @@ _keybox_update_header_blob (KEYBOXBLOB blob)
       blob->blob[20+1] = (val >> 16);
       blob->blob[20+2] = (val >>  8);
       blob->blob[20+3] = (val      );
+
+      if (for_openpgp)
+        blob->blob[7] |= 0x02;  /* OpenPGP data may be available.  */
     }
 }
index 7bbcf83..415a3ef 100644 (file)
@@ -101,6 +101,7 @@ struct keybox_handle {
   int eof;
   int error;
   int ephemeral;
+  int for_openpgp;        /* Used by gpg.  */
   struct keybox_found_s found;
   struct keybox_found_s saved_found;
   struct {
@@ -176,7 +177,7 @@ int  _keybox_new_blob (KEYBOXBLOB *r_blob,
 void _keybox_release_blob (KEYBOXBLOB blob);
 const unsigned char *_keybox_get_blob_image (KEYBOXBLOB blob, size_t *n);
 off_t _keybox_get_blob_fileoffset (KEYBOXBLOB blob);
-void _keybox_update_header_blob (KEYBOXBLOB blob);
+void _keybox_update_header_blob (KEYBOXBLOB blob, int for_openpgp);
 
 /*-- keybox-openpgp.c --*/
 gpg_error_t _keybox_parse_openpgp (const unsigned char *image, size_t imagelen,
index af9052d..bfe7b48 100644 (file)
@@ -141,6 +141,25 @@ dump_header_blob (const byte *buffer, size_t length, FILE *fp)
       return -1;
     }
   fprintf (fp, "Version: %d\n", buffer[5]);
+
+  n = get16 (buffer + 6);
+  fprintf( fp, "Flags:   %04lX", n);
+  if (n)
+    {
+      int any = 0;
+
+      fputs (" (", fp);
+      if ((n & 2))
+        {
+          if (any)
+            putc (',', fp);
+          fputs ("openpgp", fp);
+          any++;
+        }
+      putc (')', fp);
+    }
+  putc ('\n', fp);
+
   if ( memcmp (buffer+8, "KBXf", 4))
     fprintf (fp, "[Error: invalid magic number]\n");
 
index f720993..def896b 100644 (file)
@@ -132,7 +132,7 @@ _keybox_write_blob (KEYBOXBLOB blob, FILE *fp)
 
 /* Write a fresh header type blob. */
 int
-_keybox_write_header_blob (FILE *fp)
+_keybox_write_header_blob (FILE *fp, int for_openpgp)
 {
   unsigned char image[32];
   u32 val;
@@ -143,6 +143,8 @@ _keybox_write_header_blob (FILE *fp)
 
   image[4] = BLOBTYPE_HEADER;
   image[5] = 1; /* Version */
+  if (for_openpgp)
+    image[7] = 0x02; /* OpenPGP data may be available.  */
 
   memcpy (image+8, "KBXf", 4);
   val = time (NULL);
index 8ae3ec3..0d4800e 100644 (file)
@@ -77,15 +77,10 @@ keybox_is_writable (void *token)
 
 
 
-/* Create a new handle for the resource associated with TOKEN.  SECRET
-   is just a cross-check.
-
-   The returned handle must be released using keybox_release (). */
-KEYBOX_HANDLE
-keybox_new (void *token, int secret)
+static KEYBOX_HANDLE
+do_keybox_new (KB_NAME resource, int secret, int for_openpgp)
 {
   KEYBOX_HANDLE hd;
-  KB_NAME resource = token;
   int idx;
 
   assert (resource && !resource->secret == !secret);
@@ -94,6 +89,7 @@ keybox_new (void *token, int secret)
     {
       hd->kb = resource;
       hd->secret = !!secret;
+      hd->for_openpgp = for_openpgp;
       if (!resource->handle_table)
         {
           resource->handle_table_size = 3;
@@ -135,6 +131,30 @@ keybox_new (void *token, int secret)
   return hd;
 }
 
+
+/* Create a new handle for the resource associated with TOKEN.  SECRET
+   is just a cross-check.  This is the OpenPGP version.  The returned
+   handle must be released using keybox_release.  */
+KEYBOX_HANDLE
+keybox_new_openpgp (void *token, int secret)
+{
+  KB_NAME resource = token;
+
+  return do_keybox_new (resource, secret, 1);
+}
+
+/* Create a new handle for the resource associated with TOKEN.  SECRET
+   is just a cross-check.  This is the X.509 version.  The returned
+   handle must be released using keybox_release.  */
+KEYBOX_HANDLE
+keybox_new_x509 (void *token, int secret)
+{
+  KB_NAME resource = token;
+
+  return do_keybox_new (resource, secret, 0);
+}
+
+
 void
 keybox_release (KEYBOX_HANDLE hd)
 {
index 6ade9e7..693b732 100644 (file)
@@ -211,18 +211,18 @@ rename_tmp_file (const char *bakfname, const char *tmpfname,
 
 
 
-/* Perform insert/delete/update operation.
-   MODE is one of FILECOPY_INSERT, FILECOPY_DELETE, FILECOPY_UPDATE.
-*/
+/* Perform insert/delete/update operation.  MODE is one of
+   FILECOPY_INSERT, FILECOPY_DELETE, FILECOPY_UPDATE.  FOR_OPENPGP
+   indicates that this is called due to an OpenPGP keyblock change.  */
 static int
 blob_filecopy (int mode, const char *fname, KEYBOXBLOB blob,
-               int secret, off_t start_offset)
+               int secret, int for_openpgp, off_t start_offset)
 {
   FILE *fp, *newfp;
   int rc=0;
   char *bakfname = NULL;
   char *tmpfname = NULL;
-  char buffer[4096];
+  char buffer[4096];  /* (Must be at least 32 bytes) */
   int nread, nbytes;
 
   /* Open the source file. Because we do a rename, we have to check the
@@ -239,7 +239,7 @@ blob_filecopy (int mode, const char *fname, KEYBOXBLOB blob,
       if (!newfp )
         return gpg_error_from_syserror ();
 
-      rc = _keybox_write_header_blob (newfp);
+      rc = _keybox_write_header_blob (newfp, for_openpgp);
       if (rc)
         return rc;
 
@@ -275,9 +275,19 @@ blob_filecopy (int mode, const char *fname, KEYBOXBLOB blob,
   /* prepare for insert */
   if (mode == FILECOPY_INSERT)
     {
-      /* Copy everything to the new file. */
+      int first_record = 1;
+
+      /* Copy everything to the new file.  If this is for OpenPGP, we
+         make sure that the openpgp flag is set in the header.  (We
+         failsafe the blob type.) */
       while ( (nread = fread (buffer, 1, DIM(buffer), fp)) > 0 )
         {
+          if (first_record && for_openpgp && buffer[4] == BLOBTYPE_HEADER)
+            {
+              first_record = 0;
+              buffer[7] |= 0x02; /* OpenPGP data may be available.  */
+            }
+
           if (fwrite (buffer, nread, 1, newfp) != 1)
             {
               rc = gpg_error_from_syserror ();
@@ -409,7 +419,7 @@ keybox_insert_keyblock (KEYBOX_HANDLE hd, const void *image, size_t imagelen,
   _keybox_destroy_openpgp_info (&info);
   if (!err)
     {
-      err = blob_filecopy (FILECOPY_INSERT, fname, blob, hd->secret, 0);
+      err = blob_filecopy (FILECOPY_INSERT, fname, blob, hd->secret, 1, 0);
       _keybox_release_blob (blob);
       /*    if (!rc && !hd->secret && kb_offtbl) */
       /*      { */
@@ -462,7 +472,7 @@ keybox_update_keyblock (KEYBOX_HANDLE hd, const void *image, size_t imagelen)
   /* Update the keyblock.  */
   if (!err)
     {
-      err = blob_filecopy (FILECOPY_UPDATE, fname, blob, hd->secret, off);
+      err = blob_filecopy (FILECOPY_UPDATE, fname, blob, hd->secret, 1, off);
       _keybox_release_blob (blob);
     }
   return err;
@@ -495,7 +505,7 @@ keybox_insert_cert (KEYBOX_HANDLE hd, ksba_cert_t cert,
   rc = _keybox_create_x509_blob (&blob, cert, sha1_digest, hd->ephemeral);
   if (!rc)
     {
-      rc = blob_filecopy (FILECOPY_INSERT, fname, blob, hd->secret, 0);
+      rc = blob_filecopy (FILECOPY_INSERT, fname, blob, hd->secret, 0, 0);
       _keybox_release_blob (blob);
       /*    if (!rc && !hd->secret && kb_offtbl) */
       /*      { */
@@ -743,8 +753,10 @@ keybox_compress (KEYBOX_HANDLE hd)
           first_blob = 0;
           if (length > 4 && buffer[4] == BLOBTYPE_HEADER)
             {
-              /* Write out the blob with an updated maintenance time stamp. */
-              _keybox_update_header_blob (blob);
+              /* Write out the blob with an updated maintenance time
+                 stamp and if needed (ie. used by gpg) set the openpgp
+                 flag.  */
+              _keybox_update_header_blob (blob, hd->for_openpgp);
               rc = _keybox_write_blob (blob, newfp);
               if (rc)
                 break;
@@ -752,7 +764,7 @@ keybox_compress (KEYBOX_HANDLE hd)
             }
 
           /* The header blob is missing.  Insert it.  */
-          rc = _keybox_write_header_blob (newfp);
+          rc = _keybox_write_header_blob (newfp, hd->for_openpgp);
           if (rc)
             break;
           any_changes = 1;
index 96c6db5..9067fb8 100644 (file)
@@ -62,7 +62,8 @@ typedef enum
 void *keybox_register_file (const char *fname, int secret);
 int keybox_is_writable (void *token);
 
-KEYBOX_HANDLE keybox_new (void *token, int secret);
+KEYBOX_HANDLE keybox_new_openpgp (void *token, int secret);
+KEYBOX_HANDLE keybox_new_x509 (void *token, int secret);
 void keybox_release (KEYBOX_HANDLE hd);
 void keybox_push_found_state (KEYBOX_HANDLE hd);
 void keybox_pop_found_state (KEYBOX_HANDLE hd);
@@ -74,7 +75,7 @@ int keybox_lock (KEYBOX_HANDLE hd, int yes);
 /*-- keybox-file.c --*/
 /* Fixme: This function does not belong here: Provide a better
    interface to create a new keybox file.  */
-int _keybox_write_header_blob (FILE *fp);
+int _keybox_write_header_blob (FILE *fp, int openpgp_flag);
 
 /*-- keybox-search.c --*/
 gpg_error_t keybox_get_keyblock (KEYBOX_HANDLE hd, iobuf_t *r_iobuf,
index 5a250b0..fb0947a 100644 (file)
@@ -341,7 +341,7 @@ keydb_add_resource (const char *url, int force, int secret, int *auto_created)
             /* Do a compress run if needed and the file is not locked. */
             if (!dotlock_take (all_resources[used_resources].lockhandle, 0))
               {
-                KEYBOX_HANDLE kbxhd = keybox_new (token, secret);
+                KEYBOX_HANDLE kbxhd = keybox_new_x509 (token, secret);
 
                 if (kbxhd)
                   {
@@ -400,7 +400,7 @@ keydb_new (int secret)
           hd->active[j].token  = all_resources[i].token;
           hd->active[j].secret = all_resources[i].secret;
           hd->active[j].lockhandle = all_resources[i].lockhandle;
-          hd->active[j].u.kr = keybox_new (all_resources[i].token, secret);
+          hd->active[j].u.kr = keybox_new_x509 (all_resources[i].token, secret);
           if (!hd->active[j].u.kr)
             {
               xfree (hd);