gpg: Allow importing keys with duplicated long key ids.

* g10/keydb.c (keydb_handle): Add field no_caching.
(keyblock_cache): Repalce field kid by fpr.
(keydb_disable_caching): New.
(keydb_search): Use the fingerprint as cache index.

* g10/import.c (import_one): Use the fingerprint and not the kid to
lookup the key.  Call keydb_disable_caching beofre re-searching for
update.

* tests/openpgp/import.test: Add a test case.

Signed-off-by: Werner Koch <[email protected]>

4 files changed, 51 insertions, 20 deletions

diff --git a/g10/import.c b/g10/import.c
index ca35ce1b9..be2fd6302 100644
--- a/g10/import.c
+++ b/g10/import.c
@@ -855,12 +855,15 @@ import_one (ctrl_t ctrl,
     PKT_public_key *pk_orig;
     KBNODE node, uidnode;
     KBNODE keyblock_orig = NULL;
+    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;
     int non_self = 0;
+    size_t an;
     char pkstrbuf[PUBKEY_STRING_SIZE];
 
     /* get the key and print some info about it */
@@ -870,6 +873,9 @@ import_one (ctrl_t ctrl,
 
     pk = node->pkt->pkt.public_key;
 
+    fingerprint_from_pk (pk, fpr2, &fpr2len);
+    for (an = fpr2len; an < MAX_FINGERPRINT_LEN; an++)
+      fpr2[an] = 0;
     keyid_from_pk( pk, keyid );
     uidnode = find_next_kbnode( keyblock, PKT_USER_ID );
 
@@ -957,7 +963,7 @@ import_one (ctrl_t ctrl,
 
     /* do we have this key already in one of our pubrings ? */
     pk_orig = xmalloc_clear( sizeof *pk_orig );
-    rc = get_pubkey_fast ( pk_orig, keyid );
+    rc = get_pubkey_byfprint_fast (pk_orig, fpr2, fpr2len);
     if( rc && rc != G10ERR_NO_PUBKEY && rc != G10ERR_UNU_PUBKEY )
       {
         if (!silent)
@@ -1033,17 +1039,11 @@ import_one (ctrl_t ctrl,
 	    goto leave;
 	  }
 
-	/* now read the original keyblock */
+	/* Now read the original keyblock again so that we can use
+           that handle for updating the keyblock.  */
         hd = keydb_new ();
-        {
-            byte afp[MAX_FINGERPRINT_LEN];
-            size_t an;
-
-            fingerprint_from_pk (pk_orig, afp, &an);
-            while (an < MAX_FINGERPRINT_LEN)
-                afp[an++] = 0;
-            rc = keydb_search_fpr (hd, afp);
-        }
+        keydb_disable_caching (hd);
+        rc = keydb_search_fpr (hd, fpr2);
 	if( rc )
 	  {
 	    log_error (_("key %s: can't locate original keyblock: %s\n"),
@@ -1051,7 +1051,7 @@ import_one (ctrl_t ctrl,
             keydb_release (hd);
 	    goto leave;
 	  }
-	rc = keydb_get_keyblock (hd, &keyblock_orig );
+	rc = keydb_get_keyblock (hd, &keyblock_orig);
 	if (rc)
 	  {
 	    log_error (_("key %s: can't read original keyblock: %s\n"),
diff --git a/g10/keydb.c b/g10/keydb.c
index a9a975378..c192e06b4 100644
--- a/g10/keydb.c
+++ b/g10/keydb.c
@@ -68,6 +68,7 @@ struct keydb_handle
   int locked;
   int found;
   unsigned long skipped_long_blobs;
+  int no_caching;
   int current;
   int used;   /* Number of items in ACTIVE. */
   struct resource_item active[MAX_KEYDB_RESOURCES];
@@ -75,7 +76,7 @@ struct keydb_handle
 
 
 /* This is a simple cache used to return the last result of a
-   successful long kid search.  This works only for keybox resources
+   successful fingerprint search.  This works only for keybox resources
    because (due to lack of a copy_keyblock function) we need to store
    an image of the keyblock which is fortunately instantly available
    for keyboxes.  */
@@ -87,7 +88,7 @@ enum keyblock_cache_states {
 
 struct {
   enum keyblock_cache_states state;
-  u32 kid[2];
+  byte fpr[MAX_FINGERPRINT_LEN];
   iobuf_t iobuf; /* Image of the keyblock.  */
   u32 *sigstatus;
   int pk_no;
@@ -570,6 +571,7 @@ keydb_new (void)
   return hd;
 }
 
+
 void
 keydb_release (KEYDB_HANDLE hd)
 {
@@ -600,6 +602,17 @@ keydb_release (KEYDB_HANDLE hd)
 }
 
 
+/* Set a flag on handle to not use cached results.  This is required
+   for updating a keyring.  Fixme: Using a new parameter for keydb_new
+   might be a better solution.  */
+void
+keydb_disable_caching (KEYDB_HANDLE hd)
+{
+  if (hd)
+    hd->no_caching = 1;
+}
+
+
 /*
  * Return the name of the current resource.  This is function first
  * looks for the last found found, then for the current search
@@ -1407,10 +1420,12 @@ keydb_search (KEYDB_HANDLE hd, KEYDB_SEARCH_DESC *desc,
   if (DBG_CACHE)
     dump_search_desc ("keydb_search", desc, ndesc);
 
-  if (ndesc == 1 && desc[0].mode == KEYDB_SEARCH_MODE_LONG_KID
+  if (!hd->no_caching
+      && ndesc == 1
+      && (desc[0].mode == KEYDB_SEARCH_MODE_FPR20
+          || desc[0].mode == KEYDB_SEARCH_MODE_FPR)
       && keyblock_cache.state  == KEYBLOCK_CACHE_FILLED
-      && keyblock_cache.kid[0] == desc[0].u.kid[0]
-      && keyblock_cache.kid[1] == desc[0].u.kid[1])
+      && !memcmp (keyblock_cache.fpr, desc[0].u.fpr, 20))
     {
       /* (DESCINDEX is already set).  */
       if (DBG_CLOCK)
@@ -1450,11 +1465,13 @@ keydb_search (KEYDB_HANDLE hd, KEYDB_SEARCH_DESC *desc,
         : rc);
 
   keyblock_cache_clear ();
-  if (!rc && ndesc == 1 && desc[0].mode == KEYDB_SEARCH_MODE_LONG_KID)
+  if (!hd->no_caching
+      && !rc
+      && ndesc == 1 && (desc[0].mode == KEYDB_SEARCH_MODE_FPR20
+                        || desc[0].mode == KEYDB_SEARCH_MODE_FPR))
     {
       keyblock_cache.state = KEYBLOCK_CACHE_PREPARED;
-      keyblock_cache.kid[0] = desc[0].u.kid[0];
-      keyblock_cache.kid[1] = desc[0].u.kid[1];
+      memcpy (keyblock_cache.fpr, desc[0].u.fpr, 20);
     }
 
   if (DBG_CLOCK)
diff --git a/g10/keydb.h b/g10/keydb.h
index 78d151a51..55f8fc22b 100644
--- a/g10/keydb.h
+++ b/g10/keydb.h
@@ -135,6 +135,7 @@ gpg_error_t keydb_add_resource (const char *url, unsigned int flags);
 
 KEYDB_HANDLE keydb_new (void);
 void keydb_release (KEYDB_HANDLE hd);
+void keydb_disable_caching (KEYDB_HANDLE hd);
 const char *keydb_get_resource_name (KEYDB_HANDLE hd);
 gpg_error_t keydb_get_keyblock (KEYDB_HANDLE hd, KBNODE *ret_kb);
 gpg_error_t keydb_update_keyblock (KEYDB_HANDLE hd, kbnode_t kb);
diff --git a/tests/openpgp/import.test b/tests/openpgp/import.test
index eb6860e88..a58db40fe 100755
--- a/tests/openpgp/import.test
+++ b/tests/openpgp/import.test
@@ -31,3 +31,16 @@ if $GPG --list-keys --with-colons $keyid \
 else
   error "$goodkey: import failed (bug 1223)"
 fi
+
+
+key1=$srcdir/samplekeys/dda252ebb8ebe1af-1.asc
+key2=$srcdir/samplekeys/dda252ebb8ebe1af-2.asc
+fpr1=9E669861368BCA0BE42DAF7DDDA252EBB8EBE1AF
+fpr2=A55120427374F3F7AA5F1166DDA252EBB8EBE1AF
+info "Checking import of two keys with colliding long key ids."
+$GPG --delete-key --batch --yes $fpr1 $fpr2 2>/dev/null || true
+$GPG --import $key1 || true
+$GPG --import $key2 || true
+n=$($GPG --list-keys --with-colons $fpr1 $fpr2 2>/dev/null \
+    | grep '^pub:.:4096:1:DDA252EBB8EBE1AF:' | wc -l)
+[ $n -ne 2 ] && error "Importing keys with long id collision failed"