From 3bb06531d38b85be295308e826a50a1a7ba935ec Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 18 Oct 2017 17:52:41 +0200 Subject: [PATCH] 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 --- g10/getkey.c | 76 +++++++++++++++++++++++++++++++++++++++++----------- g10/import.c | 64 +++++++++++++++---------------------------- g10/keydb.h | 13 +++++++-- 3 files changed, 94 insertions(+), 59 deletions(-) diff --git a/g10/getkey.c b/g10/getkey.c index 486330ad9..6d18d29a5 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 e7850023e..8dd6b501e 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 f503c9990..b173751ca 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. */