Fixed EOF detection for encrypted packets.

The code won't get confused anymore by extra packages following the
encrypted one.

3 files changed, 138 insertions, 39 deletions

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  <wk@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@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.
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 ) 
+          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 @@ 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)