* import.c (clean_subkeys, import_one): Only allow at most 1 binding sig

and at most 1 revocation sig on a subkey, as per 2440:11.1.

* hkp.c (parse_hkp_index, hkp_search): Error if the keyserver returns an
unparseable HKP response.

3 files changed, 124 insertions, 6 deletions

diff --git a/g10/ChangeLog b/g10/ChangeLog
index 7e58c653a..c631f788d 100644
--- a/g10/ChangeLog
+++ b/g10/ChangeLog
@@ -1,12 +1,22 @@
+2002-06-17  David Shaw  <[email protected]>
+
+	* import.c (clean_subkeys, import_one): Only allow at most 1
+	binding sig and at most 1 revocation sig on a subkey, as per
+	2440:11.1.
+
+	* hkp.c (parse_hkp_index, hkp_search): Error if the keyserver
+	returns an unparseable HKP response.
+
 2002-06-15  David Shaw  <[email protected]>
 
 	* keyedit.c (show_key_with_all_names), keylist.c
 	(list_keyblock_print): Show "[expired]" before expired uids.
 
 	* keyedit.c (show_key_with_all_names_colon), mainproc.c
-	(list_node), keylist.c (list_keyblock_colon): Use "uat" for user
-	attribute packets instead of "uid".  Also use '<count> <length>'
-	rather than the fake user id string.
+	(list_node), keylist.c (list_keyblock_colon): Show flag 'e' for
+	expired user ids.  Use "uat" for user attribute packets instead of
+	"uid".  Also use '<count> <length>' rather than the fake user id
+	string on attributes.
 
 	* keygen.c (keygen_add_revkey): Remove unused code.
 
diff --git a/g10/hkp.c b/g10/hkp.c
index 4a2aa5e00..e2f5bed1a 100644
--- a/g10/hkp.c
+++ b/g10/hkp.c
@@ -269,7 +269,18 @@ parse_hkp_index(IOBUF buffer,char *line)
   static u32 bits,createtime;
   int ret=0;
 
-  /* printf("Open %d, LINE: %s\n",open,line); */
+  /* printf("Open %d, LINE: %s, uid: %s\n",open,line,uid); */
+
+  /* Try and catch some bastardization of HKP.  If we don't have
+     certain unchanging landmarks, we can't reliably parse the
+     response. */
+  if(open && ascii_memcasecmp(line,"</pre>",6)!=0 &&
+     ascii_memcasecmp(line,"pub  ",5)!=0 &&
+     ascii_memcasecmp(line,"     ",5)!=0)
+    {
+      log_error(_("this keyserver is not fully HKP compatible\n"));
+      return -1;
+    }
 
   /* For multiple UIDs */
   if(open && uid!=NULL)
@@ -534,6 +545,7 @@ int hkp_search(STRLIST tokens)
     {
       IOBUF buffer;
       int count=1;
+      int ret;
 
       buffer=iobuf_temp();
 
@@ -550,15 +562,20 @@ int hkp_search(STRLIST tokens)
 
 	  rc=iobuf_read_line(hd.fp_read,&line,&buflen,&maxlen);
 
+	  ret=parse_hkp_index(buffer,line);
+	  if(ret==-1)
+	    break;
+
 	  if(rc!=0)
-	    count+=parse_hkp_index(buffer,line);
+	    count+=ret;
 	}
 
       http_close(&hd);
 
       count--;
 
-      keyserver_search_prompt(buffer,count,searchstr);
+      if(ret>-1)
+	keyserver_search_prompt(buffer,count,searchstr);
 
       iobuf_close(buffer);
       m_free(line);
diff --git a/g10/import.c b/g10/import.c
index 5e5a28c02..ccc665145 100644
--- a/g10/import.c
+++ b/g10/import.c
@@ -395,6 +395,95 @@ remove_bad_stuff (KBNODE keyblock)
     }
 }
 
+/* Clean the subkeys on a pk so that they each have at most 1 binding
+   sig and at most 1 revocation sig.  This works based solely on the
+   timestamps like the rest of gpg.  If the standard does get
+   revocation targets, this may need to be revised. */
+
+static int
+clean_subkeys(KBNODE keyblock,u32 *keyid)
+{
+  int removed=0;
+  KBNODE node,sknode=keyblock;
+
+  while((sknode=find_kbnode(sknode,PKT_PUBLIC_SUBKEY)))
+    {
+      KBNODE bsnode=NULL,rsnode=NULL;
+      u32 bsdate=0,rsdate=0;
+
+      sknode=sknode->next;
+
+      for(node=sknode;node;node=node->next)
+	{
+	  if(node->pkt->pkttype==PKT_SIGNATURE)
+	    {
+	      PKT_signature *sig=node->pkt->pkt.signature;
+
+	      /* We're only interested in valid sigs */
+	      if(check_key_signature(keyblock,node,NULL))
+		continue;
+
+	      if(IS_SUBKEY_SIG(sig) && bsdate<=sig->timestamp)
+		{
+		  bsnode=node;
+		  bsdate=sig->timestamp;
+		}
+	      else if(IS_SUBKEY_REV(sig) && rsdate<=sig->timestamp)
+		{
+		  rsnode=node;
+		  rsdate=sig->timestamp;
+		}
+	      /* If it's not a subkey sig or rev, then it shouldn't be
+                 here so ignore it. */
+	    }
+	  else
+	    break;
+	}
+
+      /* We now know the most recent binding sig and revocation sig
+         (if any).  If the binding sig is more recent than the
+         revocation sig, strip everything but the binding sig.  If the
+         revocation sig is more recent than the binding sig, strip
+         everything but the binding sig and the revocation sig. */
+
+      if(bsdate>=rsdate)
+	{
+	  rsnode=NULL;
+	  rsdate=0;
+	}
+
+     for(node=sknode;node;node=node->next)
+	{
+	  if(node->pkt->pkttype==PKT_SIGNATURE)
+	    {
+	      PKT_signature *sig=node->pkt->pkt.signature;
+
+	      if(IS_SUBKEY_SIG(sig) && node!=bsnode)
+		{
+		  delete_kbnode(node);
+		  removed++;
+		}
+	      else if(IS_SUBKEY_REV(sig) && node!=rsnode)
+		{
+		  delete_kbnode(node);
+		  removed++;
+		}
+	    }
+	  else
+	    break;
+	}
+    }
+
+  if(removed)
+    {
+      log_info(_("key %08lX: removed multiple subkey binding\n"),
+	       (ulong)keyid[1]);
+      commit_kbnode(&keyblock);
+    }
+
+  return removed;
+}
+
 
 /****************
  * Try to import one keyblock.	Return an error only in serious cases, but
@@ -492,6 +581,7 @@ import_one( const char *fname, KBNODE keyblock, int fast,
 	}
 	if( opt.verbose > 1 )
 	    log_info (_("writing to `%s'\n"), keydb_get_resource_name (hd) );
+	clean_subkeys(keyblock,keyid);
 	rc = keydb_insert_keyblock (hd, keyblock );
         if (rc)
 	   log_error (_("error writing keyring `%s': %s\n"),
@@ -568,6 +658,7 @@ import_one( const char *fname, KBNODE keyblock, int fast,
 	if( n_uids || n_sigs || n_subk ) {
 	    mod_key = 1;
 	    /* keyblock_orig has been updated; write */
+	    n_sigs-=clean_subkeys(keyblock_orig,keyid);
 	    rc = keydb_update_keyblock (hd, keyblock_orig);
             if (rc)
 		log_error (_("error writing keyring `%s': %s\n"),