security-and-privacy/gnupg
security-and-privacy
/
gnupg
mirror of git://git.gnupg.org/gnupg.git
1
0
Fork 0
Code Releases Activity

Fix for bug 1223

This commit is contained in:
Werner Koch 2010-05-07 13:13:56 +00:00
parent 51e2703abe
commit b9f2c0daaf
7 changed files with 298 additions and 161 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
g10/ChangeLog
View File

@ -1,3 +1,11 @@
2010-05-07 Werner Koch <wk@g10code.com>
* import.c (fix_bad_direct_key_sigs): New.
(import_one): Call it.
(chk_self_sigs): Re-indent, slighly re-arrange code, use test
macros for the sig class. Check direct key signatures. Fixes
bug#1223.
2010-04-27 Werner Koch <wk@g10code.com>
* passphrase.c (gpg_format_keydesc): New.

387
g10/import.c
View File

@ -521,6 +521,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_t keyblock, u32 *keyid)
{
gpg_error_t err;
kbnode_t node;
int count = 0;
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))
{
err = check_key_signature (keyblock, node, NULL);
if (err && gpg_err_code (err) != GPG_ERR_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)
{
@ -887,10 +927,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 )
@ -1354,8 +1399,8 @@ import_revoke_cert( const char *fname, KBNODE node, struct stats_s *stats )
}
/****************
* loop over the keyblock and check all self signatures.
/*
* 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. Invalid or
@ -1364,176 +1409,198 @@ import_revoke_cert( const char *fname, KBNODE node, struct stats_s *stats )
* in this keyblock.
*/
static int
chk_self_sigs( const char *fname, KBNODE keyblock,
chk_self_sigs (const char *fname, kbnode_t keyblock,
PKT_public_key *pk, u32 *keyid, int *non_self )
{
KBNODE n,knode=NULL;
PKT_signature *sig;
int rc;
u32 bsdate=0,rsdate=0;
KBNODE bsnode=NULL,rsnode=NULL;
kbnode_t n, knode = NULL;
PKT_signature *sig;
int rc;
u32 bsdate=0, rsdate=0;
kbnode_t bsnode = NULL, rsnode = NULL;
(void)fname;
(void)pk;
(void)fname;
(void)pk;
for( n=keyblock; (n = find_next_kbnode(n, 0)); ) {
if(n->pkt->pkttype==PKT_PUBLIC_SUBKEY)
for (n=keyblock; (n = find_next_kbnode (n, 0)); )
{
if (n->pkt->pkttype == PKT_PUBLIC_SUBKEY)
{
knode=n;
bsdate=0;
rsdate=0;
bsnode=NULL;
rsnode=NULL;
knode = n;
bsdate = 0;
rsdate = 0;
bsnode = NULL;
rsnode = NULL;
continue;
}
else if( n->pkt->pkttype != PKT_SIGNATURE )
continue;
sig = n->pkt->pkt.signature;
if( keyid[0] == sig->keyid[0] && keyid[1] == sig->keyid[1] ) {
/* This just caches the sigs for later use. That way we
import a fully-cached key which speeds things up. */
if(!opt.no_sig_cache)
check_key_signature(keyblock,n,NULL);
if ( n->pkt->pkttype != PKT_SIGNATURE )
continue;
sig = n->pkt->pkt.signature;
if ( keyid[0] != sig->keyid[0] || keyid[1] != sig->keyid[1] )
{
*non_self = 1;
continue;
}
if( IS_UID_SIG(sig) || IS_UID_REV(sig) )
{
KBNODE unode = find_prev_kbnode( keyblock, n, PKT_USER_ID );
if( !unode )
{
log_error( _("key %s: no user ID for signature\n"),
keystr(keyid));
return -1; /* the complete keyblock is invalid */
}
/* This just caches the sigs for later use. That way we
import a fully-cached key which speeds things up. */
if (!opt.no_sig_cache)
check_key_signature (keyblock, n, NULL);
if ( IS_UID_SIG(sig) || IS_UID_REV(sig) )
{
KBNODE unode = find_prev_kbnode( keyblock, n, PKT_USER_ID );
if ( !unode )
{
log_error( _("key %s: no user ID for signature\n"),
keystr(keyid));
return -1; /* The complete keyblock is invalid. */
}
/* If it hasn't been marked valid yet, keep trying. */
if (!(unode->flag&1))
{
rc = check_key_signature (keyblock, n, NULL);
if ( rc )
{
if ( opt.verbose )
{
char *p = utf8_to_native
(unode->pkt->pkt.user_id->name,
strlen (unode->pkt->pkt.user_id->name),0);
log_info (gpg_err_code(rc) == G10ERR_PUBKEY_ALGO ?
_("key %s: unsupported public key "
"algorithm on user ID \"%s\"\n"):
_("key %s: invalid self-signature "
"on user ID \"%s\"\n"),
keystr (keyid),p);
xfree (p);
}
}
else
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 (gpg_err_code (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 ( IS_SUBKEY_SIG (sig) )
{
/* 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 it hasn't been marked valid yet, keep trying */
if(!(unode->flag&1)) {
rc = check_key_signature( keyblock, n, NULL);
if( rc )
{
if( opt.verbose )
{
char *p=utf8_to_native(unode->pkt->pkt.user_id->name,
strlen(unode->pkt->pkt.user_id->name),0);
log_info( rc == G10ERR_PUBKEY_ALGO ?
_("key %s: unsupported public key "
"algorithm on user ID \"%s\"\n"):
_("key %s: invalid self-signature "
"on user ID \"%s\"\n"),
keystr(keyid),p);
xfree(p);
}
}
else
unode->flag |= 1; /* mark that signature checked */
}
}
else if( sig->sig_class == 0x18 ) {
/* 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 )
{
if(opt.verbose)
log_info( _("key %s: no subkey for key binding\n"),
keystr(keyid));
n->flag |= 4; /* delete this */
}
else
{
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 subkey binding\n"),
keystr(keyid));
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 */
if(opt.verbose)
log_info(_("key %s: removed multiple subkey"
" binding\n"),keystr(keyid));
}
bsnode=n;
bsdate=sig->timestamp;
}
else
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 )
{
if(opt.verbose)
log_info( _("key %s: no subkey for key revocation\n"),
keystr(keyid));
n->flag |= 4; /* delete this */
}
else
{
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 subkey revocation\n"),
keystr(keyid));
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 */
if(opt.verbose)
log_info(_("key %s: removed multiple subkey"
" revocation\n"),keystr(keyid));
}
rsnode=n;
rsdate=sig->timestamp;
}
else
n->flag|=4; /* older */
}
}
}
}
else
*non_self=1;
if ( !knode )
{
if (opt.verbose)
log_info (_("key %s: no subkey for key binding\n"),
keystr (keyid));
n->flag |= 4; /* delete this */
}
else
{
rc = check_key_signature (keyblock, n, NULL);
if ( rc )
{
if (opt.verbose)
log_info (gpg_err_code (rc) == G10ERR_PUBKEY_ALGO ?
_("key %s: unsupported public key"
" algorithm\n"):
_("key %s: invalid subkey binding\n"),
keystr (keyid));
n->flag |= 4;
}
else
{
/* It's valid, so is it newer? */
if (sig->timestamp >= bsdate)
{
knode->flag |= 1; /* The subkey is valid. */
if (bsnode)
{
/* Delete the last binding sig since this
one is newer */
bsnode->flag |= 4;
if (opt.verbose)
log_info (_("key %s: removed multiple subkey"
" binding\n"),keystr(keyid));
}
bsnode = n;
bsdate = sig->timestamp;
}
else
n->flag |= 4; /* older */
}
}
}
else if ( IS_SUBKEY_REV (sig) )
{
/* 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 )
{
if (opt.verbose)
log_info (_("key %s: no subkey for key revocation\n"),
keystr(keyid));
n->flag |= 4; /* delete this */
}
else
{
rc = check_key_signature (keyblock, n, NULL);
if ( rc )
{
if(opt.verbose)
log_info (gpg_err_code (rc) == G10ERR_PUBKEY_ALGO ?
_("key %s: unsupported public"
" key algorithm\n"):
_("key %s: invalid subkey revocation\n"),
keystr(keyid));
n->flag |= 4;
}
else
{
/* It's valid, so is it newer? */
if (sig->timestamp >= rsdate)
{
if (rsnode)
{
/* Delete the last revocation sig since
this one is newer. */
rsnode->flag |= 4;
if (opt.verbose)
log_info (_("key %s: removed multiple subkey"
" revocation\n"),keystr(keyid));
}
rsnode = n;
rsdate = sig->timestamp;
}
else
n->flag |= 4; /* older */
}
}
}
}
return 0;
return 0;
}
/****************
* delete all parts which are invalid and those signatures whose
* public key algorithm is not available in this implemenation;

5
tests/openpgp/ChangeLog
View File

@ -1,3 +1,8 @@
2010-05-07 Werner Koch <wk@g10code.com>
* import.test: Add test case for bug#1223.
* bug1223-good.asc, bug1223-bogus.asc: New.
2009-12-21 Werner Koch <wk@g10code.com>
* Makefile.am (required_pgms): New.

3
tests/openpgp/Makefile.am
View File

@ -40,7 +40,8 @@ TESTS = version.test mds.test \
TEST_FILES = pubring.asc secring.asc plain-1o.asc plain-2o.asc plain-3o.asc \
plain-1.asc plain-2.asc plain-3.asc plain-1-pgp.asc \
pubring.pkr.asc secring.skr.asc secdemo.asc pubdemo.asc \
gpg.conf.tmpl bug537-test.data.asc bug894-test.asc
gpg.conf.tmpl bug537-test.data.asc bug894-test.asc \
bug1223-good.asc bug1223-bogus.asc
DATA_FILES = data-500 data-9000 data-32000 data-80000 plain-large

21
tests/openpgp/bug1223-bogus.asc Normal file
View File

@ -0,0 +1,21 @@
Bogus test key for bug 1223 (Designated revoker sigs are not properly merged)
Thanks to Daniel Kahn Gillmor for providing the test keys.
-----BEGIN PGP PUBLIC KEY BLOCK-----
Version: GnuPG v1.4.10 (GNU/Linux)
mI0ES+OoSQEEAJUZ/+fC6DXN2X7Wxl4Huud/+i2qP1hcq+Qnbr7hVCKEnn0edYl+
6xfsKmAMBjl+qTZxPSDSx4r3ciMiIbnvXFtlBAQmji86kqoR6fm9s8BN7LTq7+2/
c2FHVF67D7zES7WgHc4i7CfiZnwXgkLvi5b1jBt+MTAOrFhdobxoy6/XABEBAAGI
twQfAQIAIQUCS+OsRRcMgAEAAAAAAAAAAAAAAAAAAAAAAAAAAQIHAAAKCRA0t9EL
wQjoOrRXBACBqhigTcj8pJY14AkjV+ZzUbm55kJRDPdU7NQ1PSvczm7HZaL3b8Lr
Psa5c5+caVLjsGWkQycQl7lUIGU84KoUfwACQKVVLkqJz8LkL54lLcwkG70+1NH5
xoSNcHHVbYtqDLNeCOq5jEIoXuz44wiWVEfF+/B115PvgwZ63pjH1rRGVGVzdCBL
ZXkgRGVtb25zdHJhdGluZyBSZXZva2VyIFRyb3VibGUgKERPIE5PVCBVU0UpIDx0
ZXN0QGV4YW1wbGUubmV0Poi+BBMBAgAoBQJL46hJAhsDBQkACTqABgsJCAcDAgYV
CAIJCgsEFgIDAQIeAQIXgAAKCRA0t9ELwQjoOgLpA/9/si2QYmietY9a6VlAmMri
mhZeqo6zyn8zrO9RGU7+8jmeb5nVnXw1YmZcw2fiJgI9+tTMkTfomyR6k0EDvcEu
2Mg3USkVnJfrrkPjSL9EajW6VpOUNxlox3ZT1oyEo3OOnVF1gC1reWYfy7Ns9zIB
1leLXbMr86zYdCoXp0Xu4g==
=YV5g
-----END PGP PUBLIC KEY BLOCK-----

20
tests/openpgp/bug1223-good.asc Normal file
View File

@ -0,0 +1,20 @@
Good test key for bug 1223 (Designated revoker sigs are not properly merged)
-----BEGIN PGP PUBLIC KEY BLOCK-----
Version: GnuPG v1.4.10 (GNU/Linux)
mI0ES+OoSQEEAJUZ/+fC6DXN2X7Wxl4Huud/+i2qP1hcq+Qnbr7hVCKEnn0edYl+
6xfsKmAMBjl+qTZxPSDSx4r3ciMiIbnvXFtlBAQmji86kqoR6fm9s8BN7LTq7+2/
c2FHVF67D7zES7WgHc4i7CfiZnwXgkLvi5b1jBt+MTAOrFhdobxoy6/XABEBAAGI
twQfAQIAIQUCS+OsRRcMgAEO5b6XkoLYC591QPHM0u2U0hc56QIHAAAKCRA0t9EL
wQjoOrRXBACBqhigTcj8pJY14AkjV+ZzUbm55kJRDPdU7NQ1PSvczm7HZaL3b8Lr
Psa5c5+caVLjsGWkQycQl7lUIGU84KoUfwACQKVVLkqJz8LkL54lLcwkG70+1NH5
xoSNcHHVbYtqDLNeCOq5jEIoXuz44wiWVEfF+/B115PvgwZ63pjH1rRGVGVzdCBL
ZXkgRGVtb25zdHJhdGluZyBSZXZva2VyIFRyb3VibGUgKERPIE5PVCBVU0UpIDx0
ZXN0QGV4YW1wbGUubmV0Poi+BBMBAgAoBQJL46hJAhsDBQkACTqABgsJCAcDAgYV
CAIJCgsEFgIDAQIeAQIXgAAKCRA0t9ELwQjoOgLpA/9/si2QYmietY9a6VlAmMri
mhZeqo6zyn8zrO9RGU7+8jmeb5nVnXw1YmZcw2fiJgI9+tTMkTfomyR6k0EDvcEu
2Mg3USkVnJfrrkPjSL9EajW6VpOUNxlox3ZT1oyEo3OOnVF1gC1reWYfy7Ns9zIB
1leLXbMr86zYdCoXp0Xu4g==
=xsEd
-----END PGP PUBLIC KEY BLOCK-----

15
tests/openpgp/import.test
View File

@ -18,6 +18,21 @@ else
fi
boguskey=$srcdir/bug1223-bogus.asc
goodkey=$srcdir/bug1223-good.asc
keyid=0xC108E83A
info "Checking bug 1223: designated revoker sigs are not properly merged."
$GPG --delete-key --batch --yes $keyid 2>/dev/null || true
$GPG --import $boguskey || true
$GPG --import $goodkey || true
if $GPG --list-keys --with-colons $keyid \
| grep '^rvk:.*:0EE5BE979282D80B9F7540F1CCD2ED94D21739E9:' >/dev/null; then
:
else
error "$goodkey: import failed (bug 1223)"
fi