[svn] GnuPG - r4036 - in trunk: . checks g10
svn author wk
cvs at cvs.gnupg.org
Mon Mar 6 22:28:31 CET 2006
Author: wk
Date: 2006-03-06 22:28:25 +0100 (Mon, 06 Mar 2006)
New Revision: 4036
Modified:
trunk/NEWS
trunk/checks/ChangeLog
trunk/checks/defs.inc
trunk/checks/multisig.test
trunk/checks/verify.test
trunk/g10/ChangeLog
trunk/g10/mainproc.c
Log:
Stricter test of allowed signature packet compositions.
There is still one problem to solve.
Modified: trunk/NEWS
===================================================================
--- trunk/NEWS 2006-03-06 12:28:46 UTC (rev 4035)
+++ trunk/NEWS 2006-03-06 21:28:25 UTC (rev 4036)
@@ -51,7 +51,11 @@
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)
------------------------------------------------
Modified: trunk/checks/ChangeLog
===================================================================
--- trunk/checks/ChangeLog 2006-03-06 12:28:46 UTC (rev 4035)
+++ trunk/checks/ChangeLog 2006-03-06 21:28:25 UTC (rev 4036)
@@ -1,3 +1,13 @@
+2006-03-06 Werner Koch <wk at g10code.com>
+
+ * 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 <wk at gnupg.org>
* verify.test: New.
Modified: trunk/checks/defs.inc
===================================================================
--- trunk/checks/defs.inc 2006-03-06 12:28:46 UTC (rev 4035)
+++ trunk/checks/defs.inc 2006-03-06 21:28:25 UTC (rev 4036)
@@ -31,25 +31,55 @@
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
+}
+
echo_n_init=no
echo_n () {
if test "$echo_n_init" = "no"; then
@@ -126,6 +156,8 @@
GPG="../g10/gpg --no-permission-warning --homedir . "
+[ -n "${BASH_VERSION+set}" ] && exec 5>/dev/stderr
+
exec 2> ${pgmname}.log
:
Modified: trunk/checks/multisig.test
===================================================================
--- trunk/checks/multisig.test 2006-03-06 12:28:46 UTC (rev 4035)
+++ trunk/checks/multisig.test 2006-03-06 21:28:25 UTC (rev 4036)
@@ -2,13 +2,15 @@
# 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 @@
-----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 @@
# $GPG --verify <x 2>/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 <x 2>/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 <x 2>/dev/null && error "invalid is valid ($i)"
+ linefeed
done
-IFS="${save_IFS}"
+
+
+resume_error
\ No newline at end of file
Modified: trunk/checks/verify.test
===================================================================
--- trunk/checks/verify.test 2006-03-06 12:28:46 UTC (rev 4035)
+++ trunk/checks/verify.test 2006-03-06 21:28:25 UTC (rev 4036)
@@ -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
Modified: trunk/g10/ChangeLog
===================================================================
--- trunk/g10/ChangeLog 2006-03-06 12:28:46 UTC (rev 4035)
+++ trunk/g10/ChangeLog 2006-03-06 21:28:25 UTC (rev 4036)
@@ -1,7 +1,8 @@
2006-03-06 Werner Koch <wk at g10code.com>
- * 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 <wk at g10code.com>
Modified: trunk/g10/mainproc.c
===================================================================
--- trunk/g10/mainproc.c 2006-03-06 12:28:46 UTC (rev 4035)
+++ trunk/g10/mainproc.c 2006-03-06 21:28:25 UTC (rev 4036)
@@ -114,27 +114,14 @@
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 @@
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;
+ 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.
- */
- {
- KBNODE n;
- int n_sig = 0;
- int n_plaintext = 0;
- int sig_seen, onepass_seen;
+ /* Check that the message composition is valid.
- for (n=c->list; n; n=n->next )
+ 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->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++;
+ if (n->next)
+ goto ambiguous; /* We only allow one P packet. */
}
-
- for (sig_seen=onepass_seen=0,n=c->list; n; n=n->next )
+ 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)
{
- 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 ("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 @@
/* 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);
}
More information about the Gnupg-commits
mailing list