From 8609693d79539628e556d1982e80f93ccd339814 Mon Sep 17 00:00:00 2001 From: David Shaw Date: Thu, 22 Aug 2002 17:47:42 +0000 Subject: [PATCH] * 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. --- g10/ChangeLog | 15 ++++ g10/g10.c | 1 + g10/getkey.c | 8 ++ g10/import.c | 191 ++++++++++++++++++++--------------------------- g10/keylist.c | 2 +- g10/options.skel | 6 +- 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 + + * 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 * 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.