diff --git a/NEWS b/NEWS index ec53b710c..853c6e3f8 100644 --- a/NEWS +++ b/NEWS @@ -51,6 +51,10 @@ Noteworthy changes in version 1.4.3 are HTTP and finger, plus anything that cURL supplies, if built with cURL support. + * Files containing several signed messages are not anymore allowed + because there is no clean way to report the status of such files + back to the caller. + Noteworthy changes in version 1.4.2 (2005-07-26) ------------------------------------------------ diff --git a/checks/ChangeLog b/checks/ChangeLog index df944a20c..854794814 100644 --- a/checks/ChangeLog +++ b/checks/ChangeLog @@ -1,3 +1,13 @@ +2006-03-06 Werner Koch + + * defs.inc: Print error messages also to stderr. Allow for + verbose environment variable. + (linefeed): New. + (suspend_error, resume_error): New. + * verify.test: More tests. + * multisig.test: Better error printing. + (sig_1ls1ls_valid, sig_ls_valid): Moved to the non-valid group. + 2006-02-14 Werner Koch * verify.test: New. diff --git a/checks/defs.inc b/checks/defs.inc index 3f87085cd..9917de913 100755 --- a/checks/defs.inc +++ b/checks/defs.inc @@ -31,22 +31,52 @@ LANGUAGE= LC_ALL= LC_MESSAGES= +# Internal use. +defs_stop_on_error=no +defs_error_seen=no + #-------------------------------- #------ utility functions ------- #-------------------------------- fatal () { echo "$pgmname: fatal:" $* >&2 + [ -n "${BASH_VERSION+set}" ] && echo "$pgmname: fatal:" $* >&5 exit 1; } error () { echo "$pgmname:" $* >&2 - exit 1 + defs_error_seen=yes + [ -n "${BASH_VERSION+set}" ] && echo "$pgmname:" $* >&5 + if [ x$defs_stop_on_error != xyes ]; then + exit 1 + fi +} + +# Call this at the start of a test and resume_error at the end to keep +# on running all subtests without immediately exiting on error. +suspend_error () { + defs_stop_on_error=yes +} + +resume_error () { + if [ x$defs_error_seen = xyes ]; then + exit 1 + fi + defs_stop_on_error=no + defs_error_seen=no } info () { echo "$pgmname:" $* >&2 + if [ -n "${verbose+set}" ]; then + [ -n "${BASH_VERSION+set}" ] && echo "$pgmname:" $* >&5 + fi +} + +linefeed () { + echo >&2 } @@ -126,6 +156,8 @@ fi GPG="../g10/gpg --no-permission-warning --homedir . " +[ -n "${BASH_VERSION+set}" ] && exec 5>/dev/stderr + exec 2> ${pgmname}.log : diff --git a/checks/multisig.test b/checks/multisig.test index c391f6fdc..ece6f22da 100755 --- a/checks/multisig.test +++ b/checks/multisig.test @@ -2,12 +2,14 @@ # Check that gpg verifies only signatures where there is no ambiguity # in the order of packets. Needs the Demo Keys Lima and Mike. +# Note: We do son't support multiple signaturess anymore thus thsi test is +# not really needed becuase verify could do the same. We keep it anyway. + . $srcdir/defs.inc || exit 3 -# (variable intialization was created using: -# for i in files; do echo "`echo $i | sed 's,[.-],_,g'`='"; \ -# gpg --no-version --enarmor <$i | grep -v ^Comment:; echo "'" ; done -# ) +suspend_error + + sig_1ls1ls_valid=' -----BEGIN PGP ARMORED FILE----- @@ -119,13 +121,11 @@ cnksIEkgY2FuJ3QgZG8gdGhhdAo= -----END PGP ARMORED FILE----- ' -save_IFS="${IFS}" -IFS="" -for i in "$sig_1ls1ls_valid" "$sig_ls_valid" "$sig_sl_valid"; do - echo "$i" | ./gpg_dearmor >x - IFS="${save_IFS}" - $GPG --verify x 2>/dev/null || error "valid is invalid" - IFS="" + +for i in sig_sl_valid ; do + eval "(IFS=; echo \"\$$i\")" | ./gpg_dearmor >x + $GPG --verify x 2>/dev/null || error "valid is invalid ($i)" + linefeed done #for i in "$sig_11lss_valid_but_is_not" "$sig_11lss11lss_valid_but_is_not" \ # "$sig_ssl_valid_but_is_not"; do @@ -133,13 +133,13 @@ done # $GPG --verify /dev/null || error "valid is invalid" #done -# without the +e ksh seems to terminate the for loop -set +e -for i in "$sig_1lsls_invalid" "$sig_lsls_invalid" \ - "$sig_lss_invalid" "$sig_slsl_invalid" ; do - echo "$i" | ./gpg_dearmor >x - IFS="${save_IFS}" - $GPG --verify /dev/null && error "invalid is valid" - IFS="" +for i in sig_1ls1ls_valid sig_ls_valid \ + sig_1lsls_invalid sig_lsls_invalid \ + sig_lss_invalid sig_slsl_invalid ; do + eval "(IFS=; echo \"\$$i\")" | ./gpg_dearmor >x + $GPG --verify /dev/null && error "invalid is valid ($i)" + linefeed done -IFS="${save_IFS}" + + +resume_error \ No newline at end of file diff --git a/checks/verify.test b/checks/verify.test index af93f3d79..29ef084e7 100755 --- a/checks/verify.test +++ b/checks/verify.test @@ -2,10 +2,126 @@ . $srcdir/defs.inc || exit 3 -#info check that verify fails for bad input data +suspend_error + +# +# Two simple tests to check that verify fails for bad input data +# +info "checking bogus signature 1" ../tools/mk-tdata --char 0x2d 64 >x $GPG --verify x data-500 && error "no error code from verify" +info "checking bogus signature 2" ../tools/mk-tdata --char 0xca 64 >x $GPG --verify x data-500 && error "no error code from verify" -exit 0 +linefeed + +# A variable to collect the test names +tests="" + +# A plain signed message created using +# echo abc | gpg --homedir . --passphrase-fd 0 -u Alpha -z0 -sa msg +tests="$tests msg_ols_asc" +msg_ols_asc='-----BEGIN PGP MESSAGE----- + +kA0DAAIRLXJ8x2hpdzQBrQEHYgNtc2dEDFJaSSB0aGluayB0aGF0IGFsbCByaWdo +dC10aGlua2luZyBwZW9wbGUgaW4gdGhpcyBjb3VudHJ5IGFyZSBzaWNrIGFuZAp0 +aXJlZCBvZiBiZWluZyB0b2xkIHRoYXQgb3JkaW5hcnkgZGVjZW50IHBlb3BsZSBh +cmUgZmVkIHVwIGluIHRoaXMKY291bnRyeSB3aXRoIGJlaW5nIHNpY2sgYW5kIHRp +cmVkLiAgSSdtIGNlcnRhaW5seSBub3QuICBCdXQgSSdtCnNpY2sgYW5kIHRpcmVk +IG9mIGJlaW5nIHRvbGQgdGhhdCBJIGFtLgotIE1vbnR5IFB5dGhvbgqIPwMFAEQM +UlotcnzHaGl3NBECR4IAoJlEGTY+bHjD2HYuCixLQCmk01pbAKCIjkzLOAmkZNm0 +D8luT78c/1x45Q== +=a29i +-----END PGP MESSAGE-----' + +# A plain signed message created using +# echo abc | gpg --homedir . --passphrase-fd 0 -u Alpha -sa msg +tests="$tests msg_cols_asc" +msg_cols_asc='-----BEGIN PGP MESSAGE----- + +owGbwMvMwCSoW1RzPCOz3IRxLSN7EnNucboLT6Cgp0JJRmZeNpBMLFFIzMlRKMpM +zyjRBQtm5qUrFKTmF+SkKmTmgdQVKyTnl+aVFFUqJBalKhRnJmcrJOalcJVkFqWm +KOSnKSSlgrSU5OekQMzLL0rJzEsEKk9JTU7NK4EZBtKcBtRRWgAzlwtmbnlmSQbU +GJjxCmDj9RQUPNVzFZJTi0oSM/NyKhXy8kuAYk6lJSBxLlTF2NziqZCYq8elq+Cb +n1dSqRBQWZKRn8fVYc/MygAKBljYCDIFiTDMT+9seu836Q+bevyHTJ0dzPNuvCjn +ZpgrwX38z58rJsfYDhwOSS4SkN/d6vUAAA== +=s6sY +-----END PGP MESSAGE-----' + +# A PGP 2 style message. +tests="$tests msg_sl_asc" +msg_sl_asc='-----BEGIN PGP MESSAGE----- + +iD8DBQBEDFJaLXJ8x2hpdzQRAkeCAKCZRBk2Pmx4w9h2LgosS0AppNNaWwCgiI5M +yzgJpGTZtA/Jbk+/HP9ceOWtAQdiA21zZ0QMUlpJIHRoaW5rIHRoYXQgYWxsIHJp +Z2h0LXRoaW5raW5nIHBlb3BsZSBpbiB0aGlzIGNvdW50cnkgYXJlIHNpY2sgYW5k +CnRpcmVkIG9mIGJlaW5nIHRvbGQgdGhhdCBvcmRpbmFyeSBkZWNlbnQgcGVvcGxl +IGFyZSBmZWQgdXAgaW4gdGhpcwpjb3VudHJ5IHdpdGggYmVpbmcgc2ljayBhbmQg +dGlyZWQuICBJJ20gY2VydGFpbmx5IG5vdC4gIEJ1dCBJJ20Kc2ljayBhbmQgdGly +ZWQgb2YgYmVpbmcgdG9sZCB0aGF0IEkgYW0uCi0gTW9udHkgUHl0aG9uCg== +=0ukK +-----END PGP MESSAGE-----' + +# An OpenPGP message lacking the onepass packet. We used to accept +# such messages but now consider them invalid. +tests="$tests bad_ls_asc" +bad_ls_asc='-----BEGIN PGP MESSAGE----- + +rQEHYgNtc2dEDFJaSSB0aGluayB0aGF0IGFsbCByaWdodC10aGlua2luZyBwZW9w +bGUgaW4gdGhpcyBjb3VudHJ5IGFyZSBzaWNrIGFuZAp0aXJlZCBvZiBiZWluZyB0 +b2xkIHRoYXQgb3JkaW5hcnkgZGVjZW50IHBlb3BsZSBhcmUgZmVkIHVwIGluIHRo +aXMKY291bnRyeSB3aXRoIGJlaW5nIHNpY2sgYW5kIHRpcmVkLiAgSSdtIGNlcnRh +aW5seSBub3QuICBCdXQgSSdtCnNpY2sgYW5kIHRpcmVkIG9mIGJlaW5nIHRvbGQg +dGhhdCBJIGFtLgotIE1vbnR5IFB5dGhvbgqIPwMFAEQMUlotcnzHaGl3NBECR4IA +oJlEGTY+bHjD2HYuCixLQCmk01pbAKCIjkzLOAmkZNm0D8luT78c/1x45Q== +=Mpiu +-----END PGP MESSAGE-----' + + +# A signed message prefixed with an unsigned literal packet. +# (fols = faked-literal-data, one-pass, literal-data, signature) +# This should throw an error because running gpg to extract the +# signed data will return both literal data packets +tests="$tests bad_fols_asc" +bad_fols_asc='-----BEGIN PGP MESSAGE----- + +rF1iDG1zZy51bnNpZ25lZEQMY0x0aW1lc2hhcmluZywgbjoKCUFuIGFjY2VzcyBt +ZXRob2Qgd2hlcmVieSBvbmUgY29tcHV0ZXIgYWJ1c2VzIG1hbnkgcGVvcGxlLgqQ +DQMAAhEtcnzHaGl3NAGtAQdiA21zZ0QMUlpJIHRoaW5rIHRoYXQgYWxsIHJpZ2h0 +LXRoaW5raW5nIHBlb3BsZSBpbiB0aGlzIGNvdW50cnkgYXJlIHNpY2sgYW5kCnRp +cmVkIG9mIGJlaW5nIHRvbGQgdGhhdCBvcmRpbmFyeSBkZWNlbnQgcGVvcGxlIGFy +ZSBmZWQgdXAgaW4gdGhpcwpjb3VudHJ5IHdpdGggYmVpbmcgc2ljayBhbmQgdGly +ZWQuICBJJ20gY2VydGFpbmx5IG5vdC4gIEJ1dCBJJ20Kc2ljayBhbmQgdGlyZWQg +b2YgYmVpbmcgdG9sZCB0aGF0IEkgYW0uCi0gTW9udHkgUHl0aG9uCog/AwUARAxS +Wi1yfMdoaXc0EQJHggCgmUQZNj5seMPYdi4KLEtAKaTTWlsAoIiOTMs4CaRk2bQP +yW5Pvxz/XHjl +=UNM4 +-----END PGP MESSAGE-----' + + + + +# +# Now run the tests. +# +for i in $tests ; do + info "checking: $i" + case "$i" in + msg_*_asc) + eval "(IFS=; echo \"\$$i\")" >x + $GPG --verify x || error "verify of $i failed" + ;; + bad_*_asc) + eval "(IFS=; echo \"\$$i\")" >x + $GPG --verify x && error "verify of $i succeeded but should not" + ;; + *) + error "No handler for test case $i" + ;; + esac + linefeed +done + + +resume_error diff --git a/g10/ChangeLog b/g10/ChangeLog index fb8337814..c43c1f9f0 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,7 +1,8 @@ 2006-03-06 Werner Koch - * mainproc.c (check_sig_and_print): Check for multiple plaintexts - before a signature. Reported by Tavis Ormandy. + * mainproc.c (check_sig_and_print): Made the composition test more + tight. This is due to another bug report by Tavis Ormandy. + (add_onepass_sig): Simplified. 2006-03-05 Werner Koch diff --git a/g10/mainproc.c b/g10/mainproc.c index a83fb9e45..8512a9336 100644 --- a/g10/mainproc.c +++ b/g10/mainproc.c @@ -114,27 +114,14 @@ release_list( CTX c ) static int add_onepass_sig( CTX c, PACKET *pkt ) { - KBNODE node; + KBNODE node; - if( c->list ) { /* add another packet */ - /* We can only append another onepass packet if the list - * does contain only onepass packets */ - for( node=c->list; node && node->pkt->pkttype == PKT_ONEPASS_SIG; - node = node->next ) - ; - if( node ) { - /* this is not the case, so we flush the current thing and - * allow this packet to start a new verification thing */ - release_list( c ); - c->list = new_kbnode( pkt ); - } - else - add_kbnode( c->list, new_kbnode( pkt )); - } - else /* insert the first one */ - c->list = node = new_kbnode( pkt ); + if ( c->list ) /* add another packet */ + add_kbnode( c->list, new_kbnode( pkt )); + else /* insert the first one */ + c->list = node = new_kbnode( pkt ); - return 1; + return 1; } @@ -1416,93 +1403,118 @@ pka_uri_from_sig (PKT_signature *sig) static int check_sig_and_print( CTX c, KBNODE node ) { - PKT_signature *sig = node->pkt->pkt.signature; - const char *astr; - int rc, is_expkey=0, is_revkey=0; + PKT_signature *sig = node->pkt->pkt.signature; + const char *astr; + int rc, is_expkey=0, is_revkey=0; - if( opt.skip_verify ) { - log_info(_("signature verification suppressed\n")); - return 0; - } - - /* It is not in all cases possible to check multiple signatures: - * PGP 2 (which is also allowed by OpenPGP), does use the packet - * sequence: sig+data, OpenPGP does use onepas+data=sig and GnuPG - * sometimes uses (because I did'nt read the specs right) data+sig. - * Because it is possible to create multiple signatures with - * different packet sequence (e.g. data+sig and sig+data) it might - * not be possible to get it right: let's say we have: - * data+sig, sig+data,sig+data and we have not yet encountered the last - * data, we could also see this a one data with 2 signatures and then - * data+sig. - * To protect against this we check that all signatures follow - * without any intermediate packets. Note, that we won't get this - * error when we use onepass packets or cleartext signatures because - * we reset the list every time - * - * FIXME: Now that we have these marker packets, we should create a - * real grammar and check against this. - */ + if (opt.skip_verify) { - KBNODE n; - int n_sig = 0; - int n_plaintext = 0; - int sig_seen, onepass_seen; - - for (n=c->list; n; n=n->next ) - { - if ( n->pkt->pkttype == PKT_SIGNATURE ) - n_sig++; - else if (n->pkt->pkttype == PKT_GPG_CONTROL - && (n->pkt->pkt.gpg_control->control - == CTRLPKT_PLAINTEXT_MARK) ) - n_plaintext++; - } - - for (sig_seen=onepass_seen=0,n=c->list; n; n=n->next ) - { - if (n->pkt->pkttype == PKT_ONEPASS_SIG) - { - onepass_seen++; - } - else if (n->pkt->pkttype == PKT_GPG_CONTROL - && (n->pkt->pkt.gpg_control->control - == CTRLPKT_CLEARSIGN_START) ) - { - onepass_seen++; /* Handle the same way as a onepass. */ - } - else if ( (sig_seen && n->pkt->pkttype != PKT_SIGNATURE) ) - { - log_error(_("can't handle these multiple signatures\n")); - return 0; - } - else if ( n->pkt->pkttype == PKT_SIGNATURE ) - { - sig_seen = 1; - } - else if (n_sig > 1 && !sig_seen && !onepass_seen - && n->pkt->pkttype == PKT_GPG_CONTROL - && (n->pkt->pkt.gpg_control->control - == CTRLPKT_PLAINTEXT_MARK) ) - { - /* Plaintext before signatures but no onepass - signature packets. */ - log_error(_("can't handle these multiple signatures\n")); - return 0; - } - else if (n_plaintext > 1 && !sig_seen && !onepass_seen - && n->pkt->pkttype == PKT_GPG_CONTROL - && (n->pkt->pkt.gpg_control->control - == CTRLPKT_PLAINTEXT_MARK) ) - { - /* More than one plaintext before a signature but no - onepass packets. */ - log_error(_("can't handle this ambiguous signed data\n")); - return 0; - } - } + log_info(_("signature verification suppressed\n")); + return 0; } + /* Check that the message composition is valid. + + Per RFC-2440bis (-15) allowed: + + S{1,n} -- detached signature. + S{1,n} P -- old style PGP2 signature + O{1,n} P S{1,n} -- standard OpenPGP signature. + C P S{1,n} -- cleartext signature. + + + O = One-Pass Signature packet. + S = Signature packet. + P = OpenPGP Message packet (Encrypted | Compressed | Literal) + (Note that the current rfc2440bis draft also allows + for a signed message but that does not work as it + introduces ambiguities.) + We keep track of these packages using the marker packet + CTRLPKT_PLAINTEXT_MARK. + C = Marker packet for cleartext signatures. + + We reject all other messages. + + Actually we are calling this too often, i.e. for verification of + each message but better have some duplicate work than to silently + introduce a bug here. + */ + { + KBNODE n; + int n_onepass, n_sig; + + log_debug ("checking signature packet composition\n"); + dump_kbnode (c->list); + + n = c->list; + assert (n); + if ( n->pkt->pkttype == PKT_SIGNATURE ) + { + /* This is either "S{1,n}" case (detached signature) or + "S{1,n} P" (old style PGP2 signature). */ + for (n = n->next; n; n = n->next) + if (n->pkt->pkttype != PKT_SIGNATURE) + break; + if (!n) + ; /* Okay, this is a detached signature. */ + else if (n->pkt->pkttype == PKT_GPG_CONTROL + && (n->pkt->pkt.gpg_control->control + == CTRLPKT_PLAINTEXT_MARK) ) + { + if (n->next) + goto ambiguous; /* We only allow one P packet. */ + } + else + goto ambiguous; + } + else if (n->pkt->pkttype == PKT_ONEPASS_SIG) + { + /* This is the "O{1,n} P S{1,n}" case (standard signature). */ + for (n_onepass=1, n = n->next; + n && n->pkt->pkttype == PKT_ONEPASS_SIG; n = n->next) + n_onepass++; + if (!n || !(n->pkt->pkttype == PKT_GPG_CONTROL + && (n->pkt->pkt.gpg_control->control + == CTRLPKT_PLAINTEXT_MARK))) + goto ambiguous; + for (n_sig=0, n = n->next; + n && n->pkt->pkttype == PKT_SIGNATURE; n = n->next) + n_sig++; + if (n || !n_sig) + goto ambiguous; + if (n_onepass != n_sig) + { + log_info ("number of one-pass packets does not match " + "number of signature packets\n"); + goto ambiguous; + } + } + else if (n->pkt->pkttype == PKT_GPG_CONTROL + && n->pkt->pkt.gpg_control->control == CTRLPKT_CLEARSIGN_START ) + { + /* This is the "C P S{1,n}" case (clear text signature). */ + n = n->next; + if (!n || !(n->pkt->pkttype == PKT_GPG_CONTROL + && (n->pkt->pkt.gpg_control->control + == CTRLPKT_PLAINTEXT_MARK))) + goto ambiguous; + for (n_sig=0, n = n->next; + n && n->pkt->pkttype == PKT_SIGNATURE; n = n->next) + n_sig++; + if (n || !n_sig) + goto ambiguous; + } + else + { + ambiguous: + log_error(_("can't handle this ambiguous signature data\n")); + return 0; + } + + } + + /* (Indendation below not yet changed to GNU style.) */ + astr = pubkey_algo_to_string( sig->pubkey_algo ); if(keystrlen()>8) { @@ -1926,7 +1938,8 @@ proc_tree( CTX c, KBNODE node ) /* prepare to create all requested message digests */ c->mfx.md = md_open(0, 0); - /* fixme: why looking for the signature packet and not 1passpacket*/ + /* fixme: why looking for the signature packet and not the + one-pass packet? */ for( n1 = node; (n1 = find_next_kbnode(n1, PKT_SIGNATURE )); ) { md_enable( c->mfx.md, n1->pkt->pkt.signature->digest_algo); }