Re: Bug#638595: WWWOFFLE HTTPS now unusable

[Andrew M. Bishop]

address@hidden writes:

> reopen 638595
> thanks
>>>>>> "AM" == Andreas Metzler <address@hidden> writes:
> AM> it seems to fix the issue for me, the minimal testcase (lynx -dump
> AM> https://localhost:8443/) now works.
> Did you try the test cases I listed?
> No.

I don't know why the problem occurs now and not before but I don't
think that this part of it is a gnutls bug (although it may be a
change in gnutls behaviour that WWWOFFLE doesn't handle well).

The problem that you are having seems to be that one chunk of data
from gnutls_record_recv() uncompresses into much more data than the
output buffer can hold.  This means that there is still lots of
uncompressed raw data from the socket left over.  This patch for
WWWOFFLE tries to empty the existing socket buffer before requesting
more (otherwise you end up requesting data after the last packet and
gnutls gives an error).

Index: io.c
===================================================================
--- io.c        (revision 2162)
+++ io.c        (revision 2163)
@@ -477,45 +477,67 @@
    {
     if(!context->r_chunk_context)
       {
-       do
+       /* Pre-emptively try uncompressing the data that already exists */
+       if(context->r_file_data->length>0)
          {
+          
err=io_zlib_uncompress(context->r_file_data,context->r_zlib_context,&iobuffer);
+          nr=iobuffer.length;
+         }
+
+       /* Read more data and uncompress it only if there was no error and no 
data already */
+       if(err==0 && nr==0)
+         {
+          do
+            {
 #if USE_GNUTLS
-          if(context->gnutls_context)
-             
err=io_gnutls_read_with_timeout(context->gnutls_context,context->r_file_data,context->r_timeout);
-          else
+             if(context->gnutls_context)
+                
err=io_gnutls_read_with_timeout(context->gnutls_context,context->r_file_data,context->r_timeout);
+             else
 #endif
-             
err=io_read_with_timeout(fd,context->r_file_data,context->r_timeout);
-          if(err>0) context->r_raw_bytes+=err;
-          if(err<0) break;
-          
err=io_zlib_uncompress(context->r_file_data,context->r_zlib_context,&iobuffer);
-          if(err<0 || err==1) break;
+                
err=io_read_with_timeout(fd,context->r_file_data,context->r_timeout);
+             if(err>0) context->r_raw_bytes+=err;
+             if(err<0) break;
+             
err=io_zlib_uncompress(context->r_file_data,context->r_zlib_context,&iobuffer);
+             if(err<0 || err==1) break;
+            }
+          while(iobuffer.length==0);
+          nr=iobuffer.length;
          }
-       while(iobuffer.length==0);
-       nr=iobuffer.length;
       }
     else /* if(context->r_chunk_context) */
       {
-       do
+       /* Pre-emptively try uncompressing the data that already exists */
+       if(context->r_zlch_data->length>0)
          {
+          
err=io_zlib_uncompress(context->r_zlch_data,context->r_zlib_context,&iobuffer);
+          nr=iobuffer.length;
+         }
+
+       /* Read more data and uncompress it only if there was no error and no 
data already */
+       if(err==0 && nr==0)
+         {
+          do
+            {
 #if USE_GNUTLS
-          if(context->gnutls_context)
-             
err=io_gnutls_read_with_timeout(context->gnutls_context,context->r_file_data,context->r_timeout);
-          else
+             if(context->gnutls_context)
+                
err=io_gnutls_read_with_timeout(context->gnutls_context,context->r_file_data,context->r_timeout);
+             else
 #endif
-             
err=io_read_with_timeout(fd,context->r_file_data,context->r_timeout);
-          if(err>0) context->r_raw_bytes+=err;
-          if(err<0) break;
-          
err=io_chunk_decode(context->r_file_data,context->r_chunk_context,context->r_zlch_data);
-          if(err<0) break;
+                
err=io_read_with_timeout(fd,context->r_file_data,context->r_timeout);
+             if(err>0) context->r_raw_bytes+=err;
+             if(err<0) break;
+             
err=io_chunk_decode(context->r_file_data,context->r_chunk_context,context->r_zlch_data);
+             if(err<0) break;
 
-          /* Try uncompressing only if chunking is finished or there is data 
or zlib is initialised. */
-          if(err==1 || context->r_zlch_data->length>0 || 
context->r_zlib_context->stream.state)
-             
err=io_zlib_uncompress(context->r_zlch_data,context->r_zlib_context,&iobuffer);
+             /* Try uncompressing only if chunking is finished or there is 
data or zlib is initialised. */
+             if(err==1 || context->r_zlch_data->length>0 || 
context->r_zlib_context->stream.state)
+                
err=io_zlib_uncompress(context->r_zlch_data,context->r_zlib_context,&iobuffer);
 
-          if(err<0 || err==1) break;
+             if(err<0 || err==1) break;
+            }
+          while(iobuffer.length==0);
+          nr=iobuffer.length;
          }
-       while(iobuffer.length==0);
-       nr=iobuffer.length;
       }
    }
 #endif /* USE_ZLIB */


As a side-note to the gnutls thing I found that this change in
WWWOFFLE worked around the problem that we had instead of changing
libgnutls.  I can't be sure that it does the same thing though because
my test showed that calling gnutls_x509_crt_deinit() was safe so it
might be that we don't now free the crt when we finish.

-------------------- certificates.c.diff --------------------
--- certificates.c      (revision 2160)
+++ certificates.c      (revision 2161)
@@ -559,6 +559,8 @@
  if(!initialised)
     return;
 
+ gnutls_certificate_free_keys(cred);
+
  gnutls_certificate_free_credentials(cred);
 }
 
@@ -918,12 +920,6 @@
  free(keyfilename);
  free(crtfilename);
 
- if(crt)
-    gnutls_x509_crt_deinit(crt);
-
- if(privkey)
-    gnutls_x509_privkey_deinit(privkey);
-
  return(cred);
 }
 
-------------------- certificates.c.diff --------------------

-- 
Andrew.
----------------------------------------------------------------------
Andrew M. Bishop                             address@hidden
                                      http://www.gedanken.demon.co.uk/

WWWOFFLE users page:
        http://www.gedanken.demon.co.uk/wwwoffle/version-2.9/user.html