Re: [gnutls-dev] GNUTLS_E_INTERNAL_ERROR in _gnutls_ciphertext2compressed

[Ludovic Courtès]

With the attached patch against 2.0.1 (your 2 fixes + additional
`gnutls assert's) and `NULL' encryption, I nailed it down to this part
of `gnutls_cipher.c':

  /* This one was introduced to avoid a timing attack against the TLS
   * 1.0 protocol.
   */
  if (pad_failed != 0)
    {
      gnutls_assert (); /* <-- This is where we fail */
      return pad_failed;
    }

That's the first `assert' I see, which seems to indicate that PAD_FAILED
was set here:

   /* Check the pading bytes (TLS 1.x)
    */
   if (ver >= GNUTLS_TLS1 && pad_failed == 0)
     for (i = 2; i < pad; i++)
       {
         if (ciphertext.data[ciphertext.size - i] !=
             ciphertext.data[ciphertext.size - 1])
           pad_failed = GNUTLS_E_DECRYPTION_FAILED;
       }

It's pretty hard for me to debug this on a Nokia so I hope you'll come
up with a bright idea.  :-)

Thanks,
Ludovic.

diff --git a/lib/gnutls_cipher.c b/lib/gnutls_cipher.c
index b2420f7..0c33632 100644
--- a/lib/gnutls_cipher.c
+++ b/lib/gnutls_cipher.c
@@ -1,5 +1,5 @@
 /*
- * Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005 Free Software Foundation
+ * Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2007 Free Software 
Foundation
  *
  * Author: Nikos Mavroyanopoulos
  *
@@ -91,7 +91,7 @@ _gnutls_encrypt (gnutls_session_t session, const opaque * 
headers,
       /* Here comp is allocated and must be 
        * freed.
        */
-      ret = _gnutls_m_plaintext2compressed (session, &comp, plain);
+      ret = _gnutls_m_plaintext2compressed (session, &comp, &plain);
       if (ret < 0)
        {
          gnutls_assert ();
@@ -143,6 +143,7 @@ _gnutls_decrypt (gnutls_session_t session, opaque * 
ciphertext,
                                   gcipher, type);
   if (ret < 0)
     {
+      gnutls_assert ();
       return ret;
     }
 
@@ -160,20 +161,21 @@ _gnutls_decrypt (gnutls_session_t session, opaque * 
ciphertext,
 
       gcomp.data = data;
       gcomp.size = ret;
-      ret = _gnutls_m_compressed2plaintext (session, &gtxt, gcomp);
+      ret = _gnutls_m_compressed2plaintext (session, &gtxt, &gcomp);
       if (ret < 0)
        {
+         gnutls_assert ();
          return ret;
        }
 
-      if (gtxt.size > max_data_size)
+      if (gtxt.size > MAX_RECORD_RECV_SIZE)
        {
          gnutls_assert ();
          _gnutls_free_datum (&gtxt);
          /* This shouldn't have happen and
           * is a TLS fatal error.
           */
-         return GNUTLS_E_INTERNAL_ERROR;
+         return GNUTLS_E_DECOMPRESSION_FAILED;
        }
 
       memcpy (data, gtxt.data, gtxt.size);
@@ -553,7 +555,10 @@ _gnutls_ciphertext2compressed (gnutls_session_t session,
    * 1.0 protocol.
    */
   if (pad_failed != 0)
-    return pad_failed;
+    {
+      gnutls_assert ();
+      return pad_failed;
+    }
 
   /* HMAC was not the same. 
    */
diff --git a/lib/gnutls_compress.c b/lib/gnutls_compress.c
index 9e42157..b2b8f87 100644
--- a/lib/gnutls_compress.c
+++ b/lib/gnutls_compress.c
@@ -36,15 +36,15 @@
 int
 _gnutls_m_plaintext2compressed (gnutls_session_t session,
                                gnutls_datum_t * compressed,
-                               gnutls_datum_t plaintext)
+                               const gnutls_datum_t* plaintext)
 {
   int size;
   opaque *data;
 
   size =
     _gnutls_compress (session->connection_state.write_compression_state,
-                     plaintext.data, plaintext.size, &data,
-                     MAX_RECORD_SEND_SIZE + 1024);
+                     plaintext->data, plaintext->size, &data,
+                     MAX_RECORD_SEND_SIZE + EXTRA_COMP_SIZE);
   if (size < 0)
     {
       gnutls_assert ();
@@ -59,15 +59,15 @@ _gnutls_m_plaintext2compressed (gnutls_session_t session,
 int
 _gnutls_m_compressed2plaintext (gnutls_session_t session,
                                gnutls_datum_t * plain,
-                               gnutls_datum_t compressed)
+                               const gnutls_datum_t* compressed)
 {
   int size;
   opaque *data;
 
   size =
     _gnutls_decompress (session->connection_state.
-                       read_compression_state, compressed.data,
-                       compressed.size, &data, MAX_RECORD_RECV_SIZE);
+                       read_compression_state, compressed->data,
+                       compressed->size, &data, MAX_RECORD_RECV_SIZE);
   if (size < 0)
     {
       gnutls_assert ();
diff --git a/lib/gnutls_compress.h b/lib/gnutls_compress.h
index 13e155e..fe42fea 100644
--- a/lib/gnutls_compress.h
+++ b/lib/gnutls_compress.h
@@ -24,7 +24,7 @@
 
 int _gnutls_m_plaintext2compressed (gnutls_session_t session,
                                    gnutls_datum_t * compressed,
-                                   gnutls_datum_t plaintext);
+                                   const gnutls_datum_t *plaintext);
 int _gnutls_m_compressed2plaintext (gnutls_session_t session,
                                    gnutls_datum_t * plain,
-                                   gnutls_datum_t compressed);
+                                   const gnutls_datum_t* compressed);
diff --git a/lib/gnutls_compress_int.c b/lib/gnutls_compress_int.c
index 6a25bf1..d08c8d0 100644
--- a/lib/gnutls_compress_int.c
+++ b/lib/gnutls_compress_int.c
@@ -308,7 +308,10 @@ _gnutls_decompress (comp_hd_t handle, opaque * compressed,
        lzo_uint new_size;
 
        if (_gnutls_lzo1x_decompress_safe == NULL)
-         return GNUTLS_E_DECOMPRESSION_FAILED;
+         {
+           gnutls_assert ();
+           return GNUTLS_E_DECOMPRESSION_FAILED;
+         }
 
        *plain = NULL;
        out_size = compressed_size + compressed_size;
diff --git a/lib/gnutls_record.c b/lib/gnutls_record.c
index 0db66a9..00a3944 100644
--- a/lib/gnutls_record.c
+++ b/lib/gnutls_record.c
@@ -777,13 +777,19 @@ record_check_type (gnutls_session_t session,
 inline static int
 get_temp_recv_buffer (gnutls_session_t session, gnutls_datum_t * tmp)
 {
+size_t max_record_size;
+
+  if (gnutls_compression_get(session) != GNUTLS_COMP_NULL)
+    max_record_size = MAX_RECORD_RECV_SIZE + EXTRA_COMP_SIZE;
+  else
+    max_record_size = MAX_RECORD_RECV_SIZE;
 
   /* We allocate MAX_RECORD_RECV_SIZE length
    * because we cannot predict the output data by the record
    * packet length (due to compression).
    */
 
-  if (MAX_RECORD_RECV_SIZE > session->internals.recv_buffer.size ||
+  if (max_record_size > session->internals.recv_buffer.size ||
       session->internals.recv_buffer.data == NULL)
     {
 
@@ -791,7 +797,7 @@ get_temp_recv_buffer (gnutls_session_t session, 
gnutls_datum_t * tmp)
        */
       session->internals.recv_buffer.data =
        gnutls_realloc (session->internals.recv_buffer.data,
-                       MAX_RECORD_RECV_SIZE);
+                       max_record_size);
 
       if (session->internals.recv_buffer.data == NULL)
        {
@@ -799,7 +805,7 @@ get_temp_recv_buffer (gnutls_session_t session, 
gnutls_datum_t * tmp)
          return GNUTLS_E_MEMORY_ERROR;
        }
 
-      session->internals.recv_buffer.size = MAX_RECORD_RECV_SIZE;
+      session->internals.recv_buffer.size = max_record_size;
     }
 
   tmp->data = session->internals.recv_buffer.data;