From 57528d38c02f8e3e28324c1bbc9049c960f9ced2 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Fri, 7 May 2010 12:32:06 +0000 Subject: [PATCH] Fix for bug 1223 --- g10/ChangeLog | 7 ++++++ g10/import.c | 60 ++++++++++++++++++++++++++++++++++++++++++++++++++- 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/g10/ChangeLog b/g10/ChangeLog index 00b2bed07..ccb6ae42a 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,3 +1,10 @@ +2010-05-07 Werner Koch + + * import.c (chk_self_sigs): Check direct key signatures. Fixes + bug#1223. + (fix_bad_direct_key_sigs): New. + (import_one): Call it. + 2010-03-26 David Shaw * plaintext.c (handle_plaintext): Make sure that the stdout flush diff --git a/g10/import.c b/g10/import.c index b179040f7..934101cff 100644 --- a/g10/import.c +++ b/g10/import.c @@ -516,6 +516,46 @@ fix_pks_corruption(KBNODE keyblock) } +/* Versions of GnuPG before 1.4.11 and 2.0.16 allowed to import bogus + direct key signatures. A side effect of this was that a later + import of the same good direct key signatures was not possible + because the cmp_signature check in merge_blocks considered them + equal. Although direct key signatures are now checked during + import, there might still be bogus signatures sitting in a keyring. + We need to detect and delete them before doing a merge. This + fucntion returns the number of removed sigs. */ +static int +fix_bad_direct_key_sigs (KBNODE keyblock, u32 *keyid) +{ + int rc; + int count = 0; + KBNODE node; + + for (node = keyblock->next; node; node=node->next) + { + if (node->pkt->pkttype == PKT_USER_ID) + break; + if (node->pkt->pkttype == PKT_SIGNATURE + && IS_KEY_SIG (node->pkt->pkt.signature)) + { + rc = check_key_signature (keyblock, node, NULL); + if (rc && rc != G10ERR_PUBKEY_ALGO ) + { + /* If we don't know the error, we can't decide; this is + not a problem because cmp_signature can't compare the + signature either. */ + log_info ("key %s: invalid direct key signature removed\n", + keystr (keyid)); + delete_kbnode (node); + count++; + } + } + } + + return count; +} + + static void print_import_ok (PKT_public_key *pk, PKT_secret_key *sk, unsigned int reason) { @@ -876,10 +916,15 @@ import_one( const char *fname, KBNODE keyblock, struct stats_s *stats, goto leave; } + /* Make sure the original direct key sigs are all sane. */ + n_sigs_cleaned = fix_bad_direct_key_sigs (keyblock_orig, keyid); + if (n_sigs_cleaned) + commit_kbnode (&keyblock_orig); + /* and try to merge the block */ clear_kbnode_flags( keyblock_orig ); clear_kbnode_flags( keyblock ); - n_uids = n_sigs = n_subk = n_sigs_cleaned = n_uids_cleaned = 0; + n_uids = n_sigs = n_subk = n_uids_cleaned = 0; rc = merge_blocks( fname, keyblock_orig, keyblock, keyid, &n_uids, &n_sigs, &n_subk ); if( rc ) @@ -1397,6 +1442,19 @@ chk_self_sigs( const char *fname, KBNODE keyblock, unode->flag |= 1; /* mark that signature checked */ } } + else if (IS_KEY_SIG (sig)) + { + rc = check_key_signature (keyblock, n, NULL); + if ( rc ) + { + if (opt.verbose) + log_info (rc == G10ERR_PUBKEY_ALGO ? + _("key %s: unsupported public key algorithm\n"): + _("key %s: invalid direct key signature\n"), + keystr (keyid)); + n->flag |= 4; + } + } else if( sig->sig_class == 0x18 ) { /* Note that this works based solely on the timestamps like the rest of gpg. If the standard gets