[svn] GnuPG - r5171 - trunk/g10
svn author wk
cvs at cvs.gnupg.org
Fri Oct 2 14:31:15 CEST 2009
Author: wk
Date: 2009-10-02 14:31:14 +0200 (Fri, 02 Oct 2009)
New Revision: 5171
Modified:
trunk/g10/ChangeLog
trunk/g10/encr-data.c
trunk/g10/parse-packet.c
Log:
Fixed EOF detection for encrypted packets.
The code won't get confused anymore by extra packages following the
encrypted one.
Modified: trunk/g10/ChangeLog
===================================================================
--- trunk/g10/ChangeLog 2009-10-02 09:15:10 UTC (rev 5170)
+++ trunk/g10/ChangeLog 2009-10-02 12:31:14 UTC (rev 5171)
@@ -1,6 +1,16 @@
+2009-10-02 Werner Koch <wk at g10code.com>
+
+ * encr-data.c (decode_filter_context_s): Add fields PARTIAL and
+ LENGTH.
+ (decrypt_data): Set them. Simplify premature EOF detection.
+ (mdc_decode_filter): Take fixed length packets in account.
+ (decode_filter): Ditto. Better EOF detection.
+ * parse-packet.c (parse_encrypted): Store ed->LEN without the MDC
+ version byte.
+
2009-09-30 Werner Koch <wk at g10code.com>
- * parse-packet.c (skip_packet, parse_gpg_control) <ist_mode>: Take
+ * parse-packet.c (skip_packet, parse_gpg_control) <list_mode>: Take
care of premature EOFs.
* gpg.c (main): Remove obsolete GCRYCTL_DISABLE_INTERNAL_LOCKING.
Modified: trunk/g10/encr-data.c
===================================================================
--- trunk/g10/encr-data.c 2009-10-02 09:15:10 UTC (rev 5170)
+++ trunk/g10/encr-data.c 2009-10-02 12:31:14 UTC (rev 5171)
@@ -45,6 +45,8 @@
int defer_filled;
int eof_seen;
int refcount;
+ int partial; /* Working on a partial length packet. */
+ size_t length; /* If !partial: Remaining bytes in the packet. */
} *decode_filter_ctx_t;
@@ -182,6 +184,8 @@
gcry_md_write (dfx->mdc_hash, temp, nprefix+2);
dfx->refcount++;
+ dfx->partial = ed->is_partial;
+ dfx->length = ed->len;
if ( ed->mdc_method )
iobuf_push_filter ( ed->buf, mdc_decode_filter, dfx );
else
@@ -189,7 +193,7 @@
proc_packets ( procctx, ed->buf );
ed->buf = NULL;
- if ( ed->mdc_method && dfx->eof_seen == 2 )
+ if (dfx->eof_seen > 1 )
rc = gpg_error (GPG_ERR_INV_PACKET);
else if ( ed->mdc_method )
{
@@ -235,7 +239,6 @@
-/* I think we should merge this with cipher_filter */
static int
mdc_decode_filter (void *opaque, int control, IOBUF a,
byte *buf, size_t *ret_len)
@@ -244,6 +247,14 @@
size_t n, size = *ret_len;
int rc = 0;
int c;
+
+ /* Note: We need to distinguish between a partial and a fixed length
+ packet. The first is the usual case as created by GPG. However
+ for short messages the format degrades to a fixed length packet
+ and other implementations might use fixed length as well. Only
+ looking for the EOF on fixed data works only if the encrypted
+ packet is not followed by other data. This used to be a long
+ standing bug which was fixed on 2009-10-02. */
if ( control == IOBUFCTRL_UNDERFLOW && dfx->eof_seen )
{
@@ -253,37 +264,71 @@
else if( control == IOBUFCTRL_UNDERFLOW )
{
assert (a);
- assert ( size > 44 );
+ assert (size > 44); /* Our code requires at least this size. */
- /* Get at least 22 bytes and put it somewhere ahead in the buffer. */
- for (n=22; n < 44 ; n++ )
+ /* Get at least 22 bytes and put it ahead in the buffer. */
+ if (dfx->partial)
{
- if( (c = iobuf_get(a)) == -1 )
- break;
- buf[n] = c;
- }
- if ( n == 44 )
+ for (n=22; n < 44; n++)
+ {
+ if ( (c = iobuf_get(a)) == -1 )
+ break;
+ buf[n] = c;
+ }
+ }
+ else
{
+ for (n=22; n < 44 && dfx->length; n++, dfx->length--)
+ {
+ c = iobuf_get (a);
+ if (c == -1)
+ break; /* Premature EOF. */
+ buf[n] = c;
+ }
+ }
+ if (n == 44)
+ {
/* We have enough stuff - flush the deferred stuff. */
- /* (we asserted that the buffer is large enough) */
if ( !dfx->defer_filled ) /* First time. */
{
- memcpy (buf, buf+22, 22 );
+ memcpy (buf, buf+22, 22);
n = 22;
}
else
{
- memcpy (buf, dfx->defer, 22 );
+ memcpy (buf, dfx->defer, 22);
}
- /* Now fill up. */
- for (; n < size; n++ )
+ /* Fill up the buffer. */
+ if (dfx->partial)
{
- if ( (c = iobuf_get(a)) == -1 )
- break;
- buf[n] = c;
- }
- /* Move the last 22 bytes back to the defer buffer. */
- /* (right, we are wasting 22 bytes of the supplied buffer.) */
+ for (; n < size; n++ )
+ {
+ if ( (c = iobuf_get(a)) == -1 )
+ {
+ dfx->eof_seen = 1; /* Normal EOF. */
+ break;
+ }
+ buf[n] = c;
+ }
+ }
+ else
+ {
+ for (; n < size && dfx->length; n++, dfx->length--)
+ {
+ c = iobuf_get(a);
+ if (c == -1)
+ {
+ dfx->eof_seen = 3; /* Premature EOF. */
+ break;
+ }
+ buf[n] = c;
+ }
+ if (!dfx->length)
+ dfx->eof_seen = 1; /* Normal EOF. */
+ }
+
+ /* Move the trailing 22 bytes back to the defer buffer. We
+ have at least 44 bytes thus a memmove is not needed. */
n -= 22;
memcpy (dfx->defer, buf+n, 22 );
dfx->defer_filled = 1;
@@ -313,7 +358,7 @@
else
{
assert ( dfx->eof_seen );
- rc = -1; /* eof */
+ rc = -1; /* Return EOF. */
}
*ret_len = n;
}
@@ -333,22 +378,59 @@
decode_filter( void *opaque, int control, IOBUF a, byte *buf, size_t *ret_len)
{
decode_filter_ctx_t fc = opaque;
- size_t n, size = *ret_len;
- int rc = 0;
-
- if ( control == IOBUFCTRL_UNDERFLOW )
+ size_t size = *ret_len;
+ size_t n;
+ int c, rc = 0;
+
+
+ if ( control == IOBUFCTRL_UNDERFLOW && fc->eof_seen )
{
+ *ret_len = 0;
+ rc = -1;
+ }
+ else if ( control == IOBUFCTRL_UNDERFLOW )
+ {
assert(a);
- n = iobuf_read ( a, buf, size );
- if ( n == -1 )
- n = 0;
- if ( n )
+
+ if (fc->partial)
{
+ for (n=0; n < size; n++ )
+ {
+ c = iobuf_get(a);
+ if (c == -1)
+ {
+ fc->eof_seen = 1; /* Normal EOF. */
+ break;
+ }
+ buf[n] = c;
+ }
+ }
+ else
+ {
+ for (n=0; n < size && fc->length; n++, fc->length--)
+ {
+ c = iobuf_get(a);
+ if (c == -1)
+ {
+ fc->eof_seen = 3; /* Premature EOF. */
+ break;
+ }
+ buf[n] = c;
+ }
+ if (!fc->length)
+ fc->eof_seen = 1; /* Normal EOF. */
+ }
+ if (n)
+ {
if (fc->cipher_hd)
gcry_cipher_decrypt (fc->cipher_hd, buf, n, NULL, 0);
}
else
- rc = -1; /* EOF */
+ {
+ if (!fc->eof_seen)
+ fc->eof_seen = 1;
+ rc = -1; /* Return EOF. */
+ }
*ret_len = n;
}
else if ( control == IOBUFCTRL_FREE )
Modified: trunk/g10/parse-packet.c
===================================================================
--- trunk/g10/parse-packet.c 2009-10-02 09:15:10 UTC (rev 5170)
+++ trunk/g10/parse-packet.c 2009-10-02 12:31:14 UTC (rev 5171)
@@ -2617,16 +2617,11 @@
unsigned long orig_pktlen = pktlen;
ed = pkt->pkt.encrypted = xmalloc (sizeof *pkt->pkt.encrypted);
- ed->len = pktlen;
- /* We don't know the extralen which is (cipher_blocksize+2) because
- the algorithm ist not specified in this packet. However, it is
- only important to know this for some sanity checks on the packet
- length - it doesn't matter that we can't do it. */
- ed->extralen = 0;
+ /* ed->len is set below. */
+ ed->extralen = 0; /* Unknown here; only used in build_packet. */
ed->buf = NULL;
ed->new_ctb = new_ctb;
ed->is_partial = partial;
- ed->mdc_method = 0;
if (pkttype == PKT_ENCRYPTED_MDC)
{
/* Fixme: add some pktlen sanity checks. */
@@ -2645,6 +2640,12 @@
}
ed->mdc_method = DIGEST_ALGO_SHA1;
}
+ else
+ ed->mdc_method = 0;
+
+ /* A basic sanity check. We need at least an 8 byte IV plus the 2
+ detection bytes. Note that we don't known the algorithm and thus
+ we may only check against the minimum blocksize. */
if (orig_pktlen && pktlen < 10)
{
/* Actually this is blocksize+2. */
@@ -2653,6 +2654,12 @@
iobuf_skip_rest (inp, pktlen, partial);
goto leave;
}
+
+ /* Store the remaining length of the encrypted data (i.e. without
+ the MDC version number but with the IV etc.). This value is
+ required during decryption. */
+ ed->len = pktlen;
+
if (list_mode)
{
if (orig_pktlen)
More information about the Gnupg-commits
mailing list