diff --git a/g10/ChangeLog b/g10/ChangeLog index 043da496e..5d9646067 100644 --- a/g10/ChangeLog +++ b/g10/ChangeLog @@ -1,6 +1,16 @@ +2009-10-02 Werner Koch + + * 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 - * parse-packet.c (skip_packet, parse_gpg_control) : Take + * parse-packet.c (skip_packet, parse_gpg_control) : Take care of premature EOFs. * gpg.c (main): Remove obsolete GCRYCTL_DISABLE_INTERNAL_LOCKING. diff --git a/g10/encr-data.c b/g10/encr-data.c index c559299ff..de26d0a4b 100644 --- a/g10/encr-data.c +++ b/g10/encr-data.c @@ -45,6 +45,8 @@ typedef struct decode_filter_context_s 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 @@ decrypt_data( void *procctx, PKT_encrypted *ed, DEK *dek ) 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 @@ decrypt_data( void *procctx, PKT_encrypted *ed, DEK *dek ) 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 @@ decrypt_data( void *procctx, PKT_encrypted *ed, DEK *dek ) -/* 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 @@ mdc_decode_filter (void *opaque, int control, IOBUF a, 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 @@ mdc_decode_filter (void *opaque, int control, IOBUF a, 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 ) - { - /* 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 ); - n = 22; - } - else - { - memcpy (buf, dfx->defer, 22 ); - } - /* Now fill up. */ - for (; n < size; n++ ) + 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. */ + if ( !dfx->defer_filled ) /* First time. */ + { + memcpy (buf, buf+22, 22); + n = 22; } - /* Move the last 22 bytes back to the defer buffer. */ - /* (right, we are wasting 22 bytes of the supplied buffer.) */ + else + { + memcpy (buf, dfx->defer, 22); + } + /* Fill up the buffer. */ + if (dfx->partial) + { + 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 @@ mdc_decode_filter (void *opaque, int control, IOBUF a, else { assert ( dfx->eof_seen ); - rc = -1; /* eof */ + rc = -1; /* Return EOF. */ } *ret_len = n; } @@ -333,22 +378,59 @@ static int 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 ) diff --git a/g10/parse-packet.c b/g10/parse-packet.c index 0b05cfb6e..e2a5ea39d 100644 --- a/g10/parse-packet.c +++ b/g10/parse-packet.c @@ -2617,16 +2617,11 @@ parse_encrypted (IOBUF inp, int pkttype, unsigned long pktlen, 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 @@ parse_encrypted (IOBUF inp, int pkttype, unsigned long pktlen, } 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 @@ parse_encrypted (IOBUF inp, int pkttype, unsigned long pktlen, 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)