httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Justin Erenkrantz <jus...@erenkrantz.com>
Subject Re: Possible mod_ssl bug (ssl_io_input_read) (fwd)
Date Sat, 07 Jun 2003 00:59:50 GMT
The suggested API change to char_buffer_read is incorrect.  The filter_ctx 
should not be passed to char_buffer_read.  The possibility I'd propose is just 
to set buffer->length to 0 when it is exhausted and keep buffer->value 
unchanged in this case (it's overwritten on char_buffer_write, so it will not 
append to the old buffer - its value is inconsequential once its length is 0). 
The AP_MODE_SPECULATIVE case in ssl_io_input_read could easily be modified to 
handle this by not adjusting buffer->value.  That seems like it should solve 
the problem and do it in a cleaner fashion (and save cycles!).

Yet, I wonder why AP_MODE_SPECULATIVE is being used.  Its purpose is very 
narrow - it should only be used to support HTTP pipelining and only asking for 
one byte.  Only connection-level filters will implement this mode - so any 
request-level filter transformations won't be applied (i.e. mod_deflate if the 
request body is inflated).  If you want to intercept the read data, then it 
needs to be an input filter not an AP_MODE_SPECULATIVE call.  -- justin

--On Friday, June 6, 2003 3:09 PM -0400 Cliff Woolley <jwoolley@virginia.edu> 
wrote:

>
>
> ---------- Forwarded message ----------
> Date: Fri, 06 Jun 2003 12:09:19 -0700
> From: Barry Brachman <brachman@dss.bc.ca>
> Reply-To: modssl-users@modssl.org
> To: modssl-users@modssl.org
> Subject: Possible mod_ssl bug (ssl_io_input_read)
>
>
> Hi --
>
> I am developing a new Apache 2.0 module and I have encountered what I think
> to be a bug in mod_ssl.  I have been unable to find any reports of a similar
> problem.  I think this is because I am using AP_MODE_SPECULATIVE, which is
> a bit unusual, so maybe no one else has run into this case yet.
>
>
> Brief description of the problem:
>   The problem is related to the use of both AP_MODE_SPECULATIVE and mod_ssl.
>   Under certain conditions, ssl_engine_io.c:ssl_io_input_read() and its
> helper   function char_buffer_read() might not handle "inctx->cbuf"
> properly.  In   particular, this can lead to the inctx->cbuf.value pointer
> being assigned an   incorrect value, after which Apache may segmentation
> fault.
>
>
> Software Environment:
>   I am running Apache 2.0.4[56] and using the mod_ssl that comes with it.
>   The compile and runtime environments that I have been working with are
>   Redhat 2.4.18-3 and FreeBSD 4.5.  I am using the standard system
> development   tools etc.
>
>   I am configuring Apache like so:
>   % ./configure --prefix=/usr/local/apache2-2.0.45
> --with-module=aaa:auth_dacs \
>     --enable-ssl
>   % ./httpd -l
>   Compiled in modules: core.c mod_access.c mod_auth.c mod_include.c \
>     mod_log_config.c mod_env.c mod_setenvif.c mod_ssl.c prefork.c \
>     http_core.c mod_mime.c mod_auth_dacs.c mod_status.c mod_autoindex.c \
>     mod_asis.c mod_cgi.c mod_negotiation.c mod_dir.c mod_imap.c
> mod_actions.c \     mod_userdir.c mod_alias.c mod_so.c
>
>   mod_auth_dacs.c is my new module.  The only thing unusual about it, I
> think,   is that it makes the following call:
>
>      rv = ap_get_brigade(r->input_filters, bb, AP_MODE_SPECULATIVE,
>                             APR_BLOCK_READ, need);
>
> Conditions:
>   The conditions under which this happens are browser dependent.  It doesn't
>   seem to ever happen with Mozilla 1.3 or Netscape 4.78 but it does happen
>   reproducibly with IE 6.0 and Netscape Navigator 3.01.  I think this is
>   related to the way they do SSL I/O because while debugging I see that the
>   SSL buffer processing in mod_ssl is slightly different with IE 6.0.
>
>   To trigger the bug, I invoke a POST method using https from IE 6.0, which
>   leads mod_auth_dacs to make the function call that appears above.
>
>   The bug does not seem to be present when SSL is not used.
>
>
> Detailed description of the problem:
>   The problem seems to occur if an AP_MODE_SPECULATIVE mode read operation
>   is done when ssl_io_input_read() already has input buffered in inctx->cbuf.
>
>   Here is ssl_engine_io.c:char_buffer_read():
>
>   static int char_buffer_read(char_buffer_t *buffer, char *in, int inl)
>   {
>     if (!buffer->length) {
>         return 0;
>     }
>     if (buffer->length > inl) {
>         /* we have have enough to fill the caller's buffer */
>         memcpy(in, buffer->value, inl);
>         buffer->value += inl;
>         buffer->length -= inl;
>     }
>     else {
>         /* swallow remainder of the buffer */
>         memcpy(in, buffer->value, buffer->length);
>         inl = buffer->length;
>         buffer->value = NULL;
>         buffer->length = 0;
>     }
>     return inl;
>   }
>
>  If there is not enough buffered input, char_buffer_read() sets
>  buffer->value to NULL.  Then, when it returns to ssl_io_input_read():
>      if ((bytes = char_buffer_read(&inctx->cbuf, buf, wanted))) {
>         *len = bytes;
>         if (inctx->mode == AP_MODE_SPECULATIVE) {
>             /* We want to rollback this read. */
>             inctx->cbuf.value -= bytes;
>             inctx->cbuf.length += bytes;
>             return APR_SUCCESS;
>         }
>
>   it will do "inctx->cbuf.value -= bytes" which sets cbuf.value to an invalid
>   pointer.  When the pointer is dereferenced later, a segmentation fault
>   occurs.  So at the very least, it would seem that the code should always
>   avoid doing this.
>
>   Also, I found that in char_buffer_read(), "in" and "buffer->value" may
>   overlap, so memmove() should probably be used instead of memcpy().
>
> Proposed fix:
>   I have not tested this heavily, but it does seem to solve the problem in
>   my test case (and my module then works with IE 6.0 and Navigator 3.01, as
>   well as Mozilla and Netscape 4.78).
>
>   I changed ssl_io_input_read() like so:
>
>     if ((bytes = char_buffer_read(inctx, buf, wanted))) {
>         *len = bytes;
>         if (inctx->mode == AP_MODE_SPECULATIVE) {
>             return APR_SUCCESS;
>         }
>
>   And I changed char_buffer_read() like so:
>
>   static int char_buffer_read(bio_filter_in_ctx_t *inctx, char *in, int inl)
>   {
>     char_buffer_t *buffer = &inctx->cbuf;
>
>     if (!buffer->length) {
>         return 0;
>     }
>
>     if (buffer->length > inl) {
>         /* we have have enough to fill the caller's buffer */
>         memmove(in, buffer->value, inl);
>         buffer->value += inl;
>         buffer->length -= inl;
>     }
>     else {
>         /* swallow remainder of the buffer */
>         memmove(in, buffer->value, buffer->length);
>         inl = buffer->length;
>         if (inctx->mode == AP_MODE_SPECULATIVE) {
>           buffer->value = in;
>         }
>         else {
>           buffer->value = NULL;
>           buffer->length = 0;
>         }
>     }
>
>     return inl;
>   }
>
> I would be happy to provide any other details.  I can also show what
> happens when I step through the code with gdb.
>
> Thank you for your attention.
>
> Barry
>
> ** Barry Brachman
> ** Distributed Systems Software, Inc.
> ** brachman@dss.bc.ca
>
>
>
> ______________________________________________________________________
> Apache Interface to OpenSSL (mod_ssl)                   www.modssl.org
> User Support Mailing List                      modssl-users@modssl.org
> Automated List Manager                            majordomo@modssl.org
>
>





Mime
View raw message