From ec332d58efc50f6508b87fc9f51db68c39cee044 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 9 Oct 2014 19:10:32 +0200 Subject: [PATCH] gpg: Take care to use pubring.kbx if it has ever been used. * 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 --- g10/keydb.c | 85 ++++++++++++++++++++++++++++++++++----------- kbx/keybox-blob.c | 10 ++++-- kbx/keybox-defs.h | 3 +- kbx/keybox-dump.c | 19 ++++++++++ kbx/keybox-file.c | 4 ++- kbx/keybox-init.c | 34 ++++++++++++++---- kbx/keybox-update.c | 38 +++++++++++++------- kbx/keybox.h | 5 +-- sm/keydb.c | 4 +-- 9 files changed, 153 insertions(+), 49 deletions(-) diff --git a/g10/keydb.c b/g10/keydb.c index 178456aaa..a38795120 100644 --- a/g10/keydb.c +++ b/g10/keydb.c @@ -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; + /* The file exists and we have the resource type in RT. - if (fread (&magic, 4, 1, fp) == 1 ) + 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); diff --git a/kbx/keybox-blob.c b/kbx/keybox-blob.c index f7abb6ccd..35ce3e360 100644 --- a/kbx/keybox-blob.c +++ b/kbx/keybox-blob.c @@ -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. */ } } diff --git a/kbx/keybox-defs.h b/kbx/keybox-defs.h index 7bbcf83cf..415a3efff 100644 --- a/kbx/keybox-defs.h +++ b/kbx/keybox-defs.h @@ -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, diff --git a/kbx/keybox-dump.c b/kbx/keybox-dump.c index af9052d69..bfe7b4899 100644 --- a/kbx/keybox-dump.c +++ b/kbx/keybox-dump.c @@ -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"); diff --git a/kbx/keybox-file.c b/kbx/keybox-file.c index f72099348..def896bca 100644 --- a/kbx/keybox-file.c +++ b/kbx/keybox-file.c @@ -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); diff --git a/kbx/keybox-init.c b/kbx/keybox-init.c index 8ae3ec315..0d4800e12 100644 --- a/kbx/keybox-init.c +++ b/kbx/keybox-init.c @@ -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) { diff --git a/kbx/keybox-update.c b/kbx/keybox-update.c index 6ade9e79c..693b7324a 100644 --- a/kbx/keybox-update.c +++ b/kbx/keybox-update.c @@ -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; diff --git a/kbx/keybox.h b/kbx/keybox.h index 96c6db549..9067fb8a4 100644 --- a/kbx/keybox.h +++ b/kbx/keybox.h @@ -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, diff --git a/sm/keydb.c b/sm/keydb.c index 5a250b020..fb0947a93 100644 --- a/sm/keydb.c +++ b/sm/keydb.c @@ -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);