* import.c (clean_subkeys, chk_self_sigs): Merge clean_subkeys into

chk_self_sigs.  This improves efficiency as the same signatures are not
checked multiple times.  Clarify when a subkey is revoked (any revocation
signature, even if it is dated before the binding signature).

* getkey.c (merge_selfsigs_subkey): Subkey revocation comments.

* keylist.c (list_one): Stats are only for public key listings.

* g10.c (main), options.skel: Default should be include-revoked for
keyserver operations.

6 files changed, 110 insertions, 113 deletions

diff --git a/g10/ChangeLog b/g10/ChangeLog
index a3d8e087f..dde257b4d 100644
--- a/g10/ChangeLog
+++ b/g10/ChangeLog
@@ -1,3 +1,18 @@
+2002-08-22  David Shaw  <dshaw@jabberwocky.com>
+
+	* import.c (clean_subkeys, chk_self_sigs): Merge clean_subkeys
+	into chk_self_sigs.  This improves efficiency as the same
+	signatures are not checked multiple times.  Clarify when a subkey
+	is revoked (any revocation signature, even if it is dated before
+	the binding signature).
+
+	* getkey.c (merge_selfsigs_subkey): Subkey revocation comments.
+
+	* keylist.c (list_one): Stats are only for public key listings.
+
+	* g10.c (main), options.skel: Default should be include-revoked
+	for keyserver operations.
+
 2002-08-21  Werner Koch  <wk@gnupg.org>
 
 	* import.c (import_print_stats): Print new non_imported counter
diff --git a/g10/g10.c b/g10/g10.c
index ca8d8eff0..fbacee1fb 100644
--- a/g10/g10.c
+++ b/g10/g10.c
@@ -1092,6 +1092,7 @@ main( int argc, char **argv )
     opt.keyserver_options.export_options=
       EXPORT_INCLUDE_NON_RFC|EXPORT_INCLUDE_ATTRIBUTES;
     opt.keyserver_options.include_subkeys=1;
+    opt.keyserver_options.include_revoked=1;
 #if defined (__MINGW32__) || defined (__CYGWIN32__)
     opt.homedir = read_w32_registry_string( NULL, "Software\\GNU\\GnuPG", "HomeDir" );
 #else
diff --git a/g10/getkey.c b/g10/getkey.c
index 1f13dcacf..43db3abcb 100644
--- a/g10/getkey.c
+++ b/g10/getkey.c
@@ -1629,6 +1629,14 @@ merge_selfsigs_subkey( KBNODE keyblock, KBNODE subnode )
            	if ( check_key_signature( keyblock, k, NULL ) )
                     ; /* signature did not verify */
                 else if ( IS_SUBKEY_REV (sig) ) {
+		  /* Note that this means that the date on a
+                     revocation sig does not matter - even if the
+                     binding sig is dated after the revocation sig,
+                     the subkey is still marked as revoked.  This
+                     seems ok, as it is just as easy to make new
+                     subkeys rather than re-sign old ones as the
+                     problem is in the distribution.  Plus, PGP (7)
+                     does this the same way.  */
                     subpk->is_revoked = 1;
                     /* although we could stop now, we continue to 
                      * figure out other information like the old expiration
diff --git a/g10/import.c b/g10/import.c
index 76f2ddb44..03adafb2b 100644
--- a/g10/import.c
+++ b/g10/import.c
@@ -516,96 +516,6 @@ fix_hkp_corruption(KBNODE keyblock)
   return changed;
 }
 
-/* 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
  * never for an invalid keyblock.  It uses log_error to increase the
@@ -716,7 +626,6 @@ 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"),
@@ -793,7 +702,6 @@ 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"),
@@ -1042,18 +950,29 @@ import_revoke_cert( const char *fname, KBNODE node, struct stats_s *stats )
  * loop over the keyblock and check all self signatures.
  * Mark all user-ids with a self-signature by setting flag bit 0.
  * Mark all user-ids with an invalid self-signature by setting bit 1.
- * This works also for subkeys, here the subkey is marked.
+ * This works also for subkeys, here the subkey is marked.  Invalid or
+ * extra subkey sigs (binding or revocation) are marked for deletion.
  */
 static int
 chk_self_sigs( const char *fname, KBNODE keyblock,
 	       PKT_public_key *pk, u32 *keyid )
 {
-    KBNODE n;
+    KBNODE n,knode=NULL;
     PKT_signature *sig;
     int rc;
+    u32 bsdate=0,rsdate=0;
+    KBNODE bsnode=NULL,rsnode=NULL;
 
     for( n=keyblock; (n = find_next_kbnode(n, 0)); ) {
-	if( n->pkt->pkttype != PKT_SIGNATURE )
+      if(n->pkt->pkttype==PKT_PUBLIC_SUBKEY)
+	{
+	  knode=n;
+	  bsdate=0;
+	  rsdate=0;
+	  bsnode=NULL;
+	  rsnode=NULL;
+	}
+      else if( n->pkt->pkttype != PKT_SIGNATURE )
 	    continue;
 	sig = n->pkt->pkt.signature;
 	if( keyid[0] == sig->keyid[0] && keyid[1] == sig->keyid[1] ) {
@@ -1085,11 +1004,9 @@ chk_self_sigs( const char *fname, KBNODE keyblock,
 		}
 	    }
 	    else if( sig->sig_class == 0x18 ) {
-		KBNODE knode = find_prev_kbnode( keyblock,
-						 n, PKT_PUBLIC_SUBKEY );
-		if( !knode )
-		    knode = find_prev_kbnode( keyblock,
-						 n, PKT_SECRET_SUBKEY );
+	      /* Note that this works based solely on the timestamps
+		 like the rest of gpg.  If the standard gets
+		 revocation targets, this may need to be revised. */
 
 		if( !knode ) {
 		    log_info( _("key %08lX: no subkey for key binding\n"),
@@ -1097,21 +1014,77 @@ chk_self_sigs( const char *fname, KBNODE keyblock,
 		    n->flag |= 4; /* delete this */
 		}
 		else {
-		  /* If it hasn't been marked valid yet, keep trying */
-		  if(!(knode->flag&1)) {
-		    rc = check_key_signature( keyblock, n, NULL);
-		    if( rc )
-			log_info(  rc == G10ERR_PUBKEY_ALGO ?
-			   _("key %08lX: unsupported public key algorithm\n"):
-			   _("key %08lX: invalid subkey binding\n"),
-					 (ulong)keyid[1]);
+		  rc = check_key_signature( keyblock, n, NULL);
+		  if( rc ) {
+		    log_info(  rc == G10ERR_PUBKEY_ALGO ?
+			    _("key %08lX: unsupported public key algorithm\n"):
+		            _("key %08lX: invalid subkey binding\n"),
+		            (ulong)keyid[1]);
+		    n->flag|=4;
+		  }
+		  else {
+		    /* It's valid, so is it newer? */
+		    if(sig->timestamp>=bsdate) {
+		      knode->flag |= 1;  /* the subkey is valid */
+		      if(bsnode) {
+			bsnode->flag|=4; /* Delete the last binding
+					    sig since this one is
+					    newer */
+			log_info(_("key %08lX: removed multiple subkey "
+				   "binding\n"),(ulong)keyid[1]);
+		      }
+
+		      bsnode=n;
+		      bsdate=sig->timestamp;
+		    }
 		    else
-		      knode->flag |= 1; /* mark that signature checked */
+		      n->flag|=4; /* older */
+		  }
+		}
+	    }
+	    else if( sig->sig_class == 0x28 ) {
+	      /* We don't actually mark the subkey as revoked right
+                 now, so just check that the revocation sig is the
+                 most recent valid one.  Note that we don't care if
+                 the binding sig is newer than the revocation sig.
+                 See the comment in getkey.c:merge_selfsigs_subkey for
+                 more */
+		if( !knode ) {
+		    log_info( _("key %08lX: no subkey for key revocation\n"),
+					    (ulong)keyid[1]);
+		    n->flag |= 4; /* delete this */
+		}
+		else {
+		  rc = check_key_signature( keyblock, n, NULL);
+		  if( rc ) {
+		    log_info(  rc == G10ERR_PUBKEY_ALGO ?
+			    _("key %08lX: unsupported public key algorithm\n"):
+		            _("key %08lX: invalid subkey revocation\n"),
+			       (ulong)keyid[1]);
+		    n->flag|=4;
+		  }
+		  else {
+		    /* It's valid, so is it newer? */
+		    if(sig->timestamp>=rsdate) {
+		      if(rsnode) {
+			rsnode->flag|=4; /* Delete the last revocation
+					    sig since this one is
+					    newer */
+			log_info(_("key %08lX: removed multiple subkey "
+				   "revocation\n"),(ulong)keyid[1]);
+		      }
+
+		      rsnode=n;
+		      rsdate=sig->timestamp;
+		    }
+		    else
+		      n->flag|=4; /* older */
 		  }
 		}
 	    }
 	}
     }
+
     return 0;
 }
 
diff --git a/g10/keylist.c b/g10/keylist.c
index d588865db..307735184 100644
--- a/g10/keylist.c
+++ b/g10/keylist.c
@@ -252,7 +252,7 @@ list_one( STRLIST names, int secret )
 		    putchar('-');
 		putchar('\n');
 	    }
-	    list_keyblock( keyblock, 1, opt.fingerprint, &stats );
+	    list_keyblock( keyblock, 1, opt.fingerprint, NULL );
 	    release_kbnode( keyblock );
 	} while( !get_seckey_next( ctx, NULL, &keyblock ) );
 	get_seckey_end( ctx );
diff --git a/g10/options.skel b/g10/options.skel
index ed5a2a98a..924b0783e 100644
--- a/g10/options.skel
+++ b/g10/options.skel
@@ -115,8 +115,8 @@ lock-once
 # include-disabled = when searching, include keys marked as "disabled"
 #                    on the keyserver (not all keyservers support this).
 #
-# include-revoked = when searching, include keys marked as "revoked"
-#                   on the keyserver.
+# no-include-revoked = when searching, do not include keys marked as
+#                      "revoked" on the keyserver.
 #
 # verbose = show more information as the keys are fetched.
 #           Can be used more than once to increase the amount
@@ -142,7 +142,7 @@ lock-once
 # no-include-attributes = do not include attribute IDs (aka "photo IDs")
 #                         when sending keys to the keyserver.
 
-#keyserver-options auto-key-retrieve include-disabled include-revoked
+#keyserver-options auto-key-retrieve
 
 # Uncomment this line to display photo user IDs in key listings and
 # when a signature from a key with a photo is verified.