You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Doug MacEachern <do...@covalent.net> on 2001/10/02 22:24:25 UTC
ssl is broken
with current cvs all httpd-test ssl tests hang, stacktrace is same for
all...
(gdb) where
#0 0x401c91de in __select () from /lib/libc.so.6
#1 0x40047ab0 in __DTOR_END__ () from /home/dougm/ap/prefork/lib/libapr.so.0
#2 0x40039fa4 in apr_recv (sock=0x827902c, buf=0x8284638 "", len=0xbfffd1f4)
at sendrecv.c:142
#3 0x4001ba94 in socket_read (a=0x8284618, str=0xbfffd1f8, len=0xbfffd1f4,
block=APR_BLOCK_READ) at apr_buckets_socket.c:75
#4 0x80c984a in core_input_filter (f=0x827b484, b=0x827b42c,
mode=AP_MODE_BLOCKING, readbytes=0xbfffd6d8) at core.c:2863
#5 0x80c1806 in ap_get_brigade (next=0x827b484, bb=0x827b42c,
mode=AP_MODE_BLOCKING, readbytes=0xbfffd6d8) at util_filter.c:250
#6 0x8090231 in churn (pRec=0x827b3bc, eMode=AP_MODE_BLOCKING,
readbytes=0xbfffd6d8) at ssl_engine_io.c:201
#7 0x8090749 in ssl_io_filter_Input (f=0x827b3ec, pbbOut=0x822d164,
eMode=AP_MODE_BLOCKING, readbytes=0xbfffd6d8) at ssl_engine_io.c:391
#8 0x80c1806 in ap_get_brigade (next=0x827b3ec, bb=0x822d164,
mode=AP_MODE_BLOCKING, readbytes=0xbfffd6d8) at util_filter.c:250
#9 0x80c26c2 in ap_getline (s=0xbfffd740 "@È\034\b", n=8192, r=0x822c98c,
fold=0) at protocol.c:221
#10 0x80c2b2e in read_request_line (r=0x822c98c) at protocol.c:397
#11 0x80c32d9 in ap_read_request (conn=0x82790fc) at protocol.c:584
#12 0x809c4f3 in ap_process_http_connection (c=0x82790fc) at http_core.c:283
#13 0x80bfbd7 in ap_run_process_connection (c=0x82790fc) at connection.c:82
#14 0x80bfdf5 in ap_process_connection (c=0x82790fc) at connection.c:219
#15 0x80b3638 in child_main (child_num_arg=0) at prefork.c:830
#16 0x80b36e8 in make_child (s=0x81a4b1c, slot=0) at prefork.c:866
#17 0x80b3829 in startup_children (number_to_start=1) at prefork.c:940
#18 0x80b3c8c in ap_mpm_run (_pconf=0x81a31cc, plog=0x81dd39c, s=0x81a4b1c)
at prefork.c:1156
#19 0x80b989e in main (argc=8, argv=0xbffff984) at main.c:432
#20 0x40112b5c in __libc_start_main (main=0x80b9380 <main>, argc=8,
ubp_av=0xbffff984, init=0x8071fc8 <_init>, fini=0x815d74c <_fini>,
rtld_fini=0x4000d634 <_dl_fini>, stack_end=0xbffff97c)
at ../sysdeps/generic/libc-start.c:129
Re: ssl is broken
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Oct 03, 2001 at 11:35:45AM -0500, William A. Rowe, Jr. wrote:
> From: "Justin Erenkrantz" <je...@ebuilt.com>
> Sent: Wednesday, October 03, 2001 11:15 AM
>
> > On Wed, Oct 03, 2001 at 08:10:11AM -0500, William A. Rowe, Jr. wrote:
> > > Now ... the core input filter can't decide where to break input, it has to allow
> > > connection filters to insert themselves and decode whatever is read.
> > >
> > > That means that the core filter needs to be split, and another line input
> > > dequeue filter needs to be inserted as the last 'connection' filter. That
> > > leaves us ready for the request filter to read 'lines' or 'bytes' and make
> > > decisions based on the lines and bytes read, and fall back on the line input
> > > dequeue to keep the 'next' request's input set-aside for the next request.
No... The input filters have a mode which says "give me a line of input."
*Every* filter must be able to obey that mode. Reading lines can happen at
any point in the input filter stack -- it is *not* just something for the
top level app (httpd) to perform. Consider:
*) httpd needs to read a line for the request ("GET /foo HTTP/1.0")
*) httpd reads lines for the MIME headers
*) HTTP_IN reads lines that specify a chunk size
*) HTTP_IN reads lines for trailer "headers"
In each case, the next filter is supposed to return a single line.
This is exactly why I proposed adding the brigade functions to read lines.
We don't want to build line-reading code into each and every filter.
> > Yes and no. You are kind of right here - I see why you might want to
> > do that, but that was the previous architecture with HTTP_IN and
> > CORE_IN. (HTTP_IN was an admittedly bad name for it, but that is
> > what it tried to do - do readlines.)
Nah. HTTP_IN was simply the only guy who happened to respond properly to the
"*readbytes == 0" mode.
Of course, using count==0 as a mode specifier is whacky. We already pass a
mode parameter, so it should be used. There are two sets of flags that go in
that param: blocking, and read block/line. The "peek" mode and -1 are other
bastard styles; one is "delete any blank lines you see" and the other is
"read everything from the input".
> Then that was the change, to eliminate the HTTP_IN filter?
No.
The idea was to get CORE_IN to handle the various "get_brigade" modes.
Before, it was doing something totally "wrong" -- it returned a brigade that
(effectively) had "everything" in it. That prevented higher level filters
from reading only a specific amount of data from the socket (e.g. whatever
the Content-Length header said). In turn, that meant our input reading and
parsing was busted. (well, it worked, but the coupling created a completely
fragile system)
The second item that was changed was to combine HTTP_IN and DECHUNK. The
former dealt with Content-Length type requests and the latter with chunked
requests. The former could read lines, which happened to support what
DECHUNK was doing.
Of course, when DECHUNK read a line, it used the broken ap_getline()
function. That thing would read from the *top* of the filter stack. That
would actually mean that DECHUNK was re-entered, it would read a line again,
etc etc. I cannot imagine how the original code would have *ever* worked
with chunking on the request.
> Removing HTTP_IN
> and trying to have a CORE_IN of the everything and the bathwater is a broken
> model.
Yes, it would be. Thankfully, we weren't shooting for that :-)
> You've succeeded in reverting Apache to an http-only server, if that was your
> intention. As this eliminates our flexibile interoperability for other
> protocols, I need to veto this patch.
Slow down, Tex :-)
I would suggest learning what happened, and where things are going, before
pulling out that veto card.
> > A lot of the complexity was removed by assuming that only one filter
> > has the -1 brigade. And, Greg and Ryan have commented that they
> > would rather see CORE_IN not deal with socket buckets at all but
> > read directly from the socket.
Yah. The socket bucket and the brigade it was put in is a lot of needless
structure. Consider what is going on there: you're using the brigade to read
from the socket. A bit of buffering is going on to hold unused data that was
read.
But note: you're *copying* data in there.
* apr_bucket_read() to generate a HEAP bucket from the SOCKET bucket
* ->split() to split that bucket into (requested) / (unused)
- the split mallocs and copies a new bucket.
* move the post-split bucket from ctx->b to the passed-brigade
That is a lot of work, and a needless copy. Instead, the CORE_IN can alloc a
buffer of the requested size and do a apr_socket_read() right into that
buffer. Wrap a bucket around it and place it into the passed-brigade.
Of course, you might not read everything requested, and you might also want
to limit the allocation to some maximum number of bytes. (note that the
current code uses apr_brigade_partition() which blocks until the requested
number of bytes is read)
> The CORE_IN filter reads directly from the socket. If you want to provide a
> readline and readbytes here, both blocking and nonblocking, that's fine. It
> doesn't mitigate the fact that the ssl filter will transform any readline into
> read bytes, and that an HTTP filter must handle readline after recieving some
> readbytes, and handle ALL the chunking behavior.
No problem.
> > Ryan and Greg, how do you guys see this working? I yield to
> > your wisdom here... If the CORE_IN read from the socket
> > directly as you both have suggested, how would (or should)
> > mod_ssl interact?
Why should mod_ssl care *how* data is read? Why should it care whether the
data comes from a socket? Feh.
mod_ssl says "give me N bytes" and the *next* filter returns *up to* that
number of bytes. That next filter *might* be CORE_IN.
CORE_IN reads from the socket, placing buckets into the passed-brigade (per
the comment above).
> > I see it being a much simpler task to write a module that
> > replaces CORE_IN than trying to work around it. It's not
No. As Ryan said, "bogus". mod_ssl reads from the "next" filter. It can do
that quite fine. Why should it care about sockets? Why should we remove
CORE_IN when the SSL filter happens to be installed?
> > that much code - and I think mod_ssl could even ditch
> > some of the lesser-supported modes (readbytes==-1 and PEEK
> > which it already doesn't support). We'll probably end up
> > removing them in core at some point. -- justin
Those modes should be fixed independent of mod_ssl. The "peek" thing really
isn't, and I don't even know whether and how Netscape's "padding" interacts
with SSL on the wire (do they still pop out of the encrypted pipe? maybe it
knows better than to send them?)
It Netscape still puts that \r\n down the encrypted pipe, then mod_ssl would
need to deal with seeing that stuff and tossing it.
> If you have to replace CORE_IN, then your implementation of CORE_IN is
> broken. For that too I would veto the patch you've committed, if this
> can't be fixed to allow the old functionality.
It doesn't have to be replaced. *PLEASE* ... Ease up on your veto threats.
Take a breath and stop to learn what is happening.
> Maybe filtering needs some pushback so each higher level filter can avoid
> hanging on to bytes it doesn't want or need, and push them back at a lower
> level filter that can keep them in their queue for the next transaction
> within the request or the next request within the connection.
At this point, we don't need any concept of pushback. A filter should return
up to what was asked and no more.
But I seriously doubt that is the problem here. I'm guessing that mod_ssl
was coded "knowing" that the brigade it got back from f->next was a socket
bucket. Or that the brigade had certain properties when reading from it.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: ssl is broken
Posted by Greg Stein <gs...@lyra.org>.
On Wed, Oct 03, 2001 at 02:34:57PM -0700, Roy T. Fielding wrote:
>...
> I think we have to get off the notion that recent modifications have
> broken mod_ssl
Hell yah. I'm going to follow up to this thread in a sec, but just wanted to
chime in with a high-level comment here. Justin's recent changes have
*improved* the input filter stack. The problems that we're seeing at higher
levels are because those higher levels made assumptions. At the moment, the
various filters are all coupled together, when they should be completely
independent.
> -- it has yet to work correctly with filters. Any module
> that makes implementation assumptions about the implementation of the
> upstream filter, or the nature of the data being passed through the filter,
> is broken, and I personally think that includes every implementation of
> filters in httpd. That is why we have a showstopper item about rethinking
> the input filters.
The output filters seem to be quite fine. The input filters have problems,
but Justin just made a big step in fixing them. There are a couple more
steps to do.
The problem is that people are screaming about the first step and saying
"back up" rather than seeing the target and saying "take the next two steps"
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: ssl is broken
Posted by "Roy T. Fielding" <fi...@ebuilt.com>.
> Maybe filtering needs some pushback so each higher level filter can avoid
> hanging on to bytes it doesn't want or need, and push them back at a lower
> level filter that can keep them in their queue for the next transaction
> within the request or the next request within the connection.
Yes, as we discussed several times before, the only thing that the filter
closest to the socket (CORE_IN or whatever else you might call it) can
do is read large blocks of data from the network and place them in a
buffer with a connection lifetime. In other words, what BUFF did.
The only way filters can improve on that is by passing brigades and
allowing the downstream filter to "unread" (pass back to the CORE_IN)
any data that is beyond its application frame (message boundary).
OTOH, the notion of replacing CORE_IN based on the notion of the connection
is really no different than the IOL code that Dean introduced. There are
reasons why we don't want to do that, but nobody bothered to document them.
I think we have to get off the notion that recent modifications have
broken mod_ssl -- it has yet to work correctly with filters. Any module
that makes implementation assumptions about the implementation of the
upstream filter, or the nature of the data being passed through the filter,
is broken, and I personally think that includes every implementation of
filters in httpd. That is why we have a showstopper item about rethinking
the input filters.
In any case, this part of httpd-2.0 is not ready for prime time. It
desperately needs our unfettered attention, which means not approaching
every change as if it were a life-or-death situation. Just fix it.
....Roy
Re: ssl is broken
Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
From: "Justin Erenkrantz" <je...@ebuilt.com>
Sent: Wednesday, October 03, 2001 11:15 AM
> On Wed, Oct 03, 2001 at 08:10:11AM -0500, William A. Rowe, Jr. wrote:
> > Now ... the core input filter can't decide where to break input, it has to allow
> > connection filters to insert themselves and decode whatever is read.
> >
> > That means that the core filter needs to be split, and another line input
> > dequeue filter needs to be inserted as the last 'connection' filter. That
> > leaves us ready for the request filter to read 'lines' or 'bytes' and make
> > decisions based on the lines and bytes read, and fall back on the line input
> > dequeue to keep the 'next' request's input set-aside for the next request.
>
> Yes and no. You are kind of right here - I see why you might want to
> do that, but that was the previous architecture with HTTP_IN and
> CORE_IN. (HTTP_IN was an admittedly bad name for it, but that is
> what it tried to do - do readlines.)
Then that was the change, to eliminate the HTTP_IN filter? Removing HTTP_IN
and trying to have a CORE_IN of the everything and the bathwater is a broken
model.
You've succeeded in reverting Apache to an http-only server, if that was your
intention. As this eliminates our flexibile interoperability for other
protocols, I need to veto this patch.
> A lot of the complexity was removed by assuming that only one filter
> has the -1 brigade. And, Greg and Ryan have commented that they
> would rather see CORE_IN not deal with socket buckets at all but
> read directly from the socket.
The CORE_IN filter reads directly from the socket. If you want to provide a
readline and readbytes here, both blocking and nonblocking, that's fine. It
doesn't mitigate the fact that the ssl filter will transform any readline into
read bytes, and that an HTTP filter must handle readline after recieving some
readbytes, and handle ALL the chunking behavior.
> Ryan and Greg, how do you guys see this working? I yield to
> your wisdom here... If the CORE_IN read from the socket
> directly as you both have suggested, how would (or should)
> mod_ssl interact?
>
> I see it being a much simpler task to write a module that
> replaces CORE_IN than trying to work around it. It's not
> that much code - and I think mod_ssl could even ditch
> some of the lesser-supported modes (readbytes==-1 and PEEK
> which it already doesn't support). We'll probably end up
> removing them in core at some point. -- justin
If you have to replace CORE_IN, then your implementation of CORE_IN is
broken. For that too I would veto the patch you've committed, if this
can't be fixed to allow the old functionality.
Maybe filtering needs some pushback so each higher level filter can avoid
hanging on to bytes it doesn't want or need, and push them back at a lower
level filter that can keep them in their queue for the next transaction
within the request or the next request within the connection.
Bill
Re: ssl is broken
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Wed, Oct 03, 2001 at 08:10:11AM -0500, William A. Rowe, Jr. wrote:
> Now ... the core input filter can't decide where to break input, it has to allow
> connection filters to insert themselves and decode whatever is read.
>
> That means that the core filter needs to be split, and another line input
> dequeue filter needs to be inserted as the last 'connection' filter. That
> leaves us ready for the request filter to read 'lines' or 'bytes' and make
> decisions based on the lines and bytes read, and fall back on the line input
> dequeue to keep the 'next' request's input set-aside for the next request.
Yes and no. You are kind of right here - I see why you might want to
do that, but that was the previous architecture with HTTP_IN and
CORE_IN. (HTTP_IN was an admittedly bad name for it, but that is
what it tried to do - do readlines.)
A lot of the complexity was removed by assuming that only one filter
has the -1 brigade. And, Greg and Ryan have commented that they
would rather see CORE_IN not deal with socket buckets at all but
read directly from the socket.
Ryan and Greg, how do you guys see this working? I yield to
your wisdom here... If the CORE_IN read from the socket
directly as you both have suggested, how would (or should)
mod_ssl interact?
I see it being a much simpler task to write a module that
replaces CORE_IN than trying to work around it. It's not
that much code - and I think mod_ssl could even ditch
some of the lesser-supported modes (readbytes==-1 and PEEK
which it already doesn't support). We'll probably end up
removing them in core at some point. -- justin
Re: ssl is broken
Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
From: "Greg Stein" <gs...@lyra.org>
Sent: Wednesday, October 03, 2001 12:33 AM
> On Tue, Oct 02, 2001 at 08:34:30PM -0500, William A. Rowe, Jr. wrote:
> > From: "Justin Erenkrantz" <je...@ebuilt.com>
> > Sent: Tuesday, October 02, 2001 8:29 PM
> >
> > > On Tue, Oct 02, 2001 at 01:24:25PM -0700, Doug MacEachern wrote:
> > > > with current cvs all httpd-test ssl tests hang, stacktrace is same for
> > > > all...
> > >
> > > I bet it's related to the input filtering changes.
> > >
> > > I have no clue how the whole mod_ssl input filtering works, but I'll
> > > try to take a look. My guess is that mod_ssl is just a big quagmire.
> >
> > That, or your input filtering changes were the big quagmire. Either way,
> > that sure sounds like the interaction that broke ssl.
>
> I'll lay good odds that it is in mod_ssl rather than the input filtering
> changes. Even *if* it is the latter, at least we have a codebase that is
> much more maintainable now.
If that statement was harsh, I'm sorry, and yes - legibility and maintainability
is important. That said...
We have only one extensive use of Input Filters in the httpd tree at this time,
and that's mod_ssl. That this wasn't even tested sort of surprizes me.
Now ... the core input filter can't decide where to break input, it has to allow
connection filters to insert themselves and decode whatever is read.
That means that the core filter needs to be split, and another line input
dequeue filter needs to be inserted as the last 'connection' filter. That
leaves us ready for the request filter to read 'lines' or 'bytes' and make
decisions based on the lines and bytes read, and fall back on the line input
dequeue to keep the 'next' request's input set-aside for the next request.
Bill
Re: ssl is broken
Posted by Greg Stein <gs...@lyra.org>.
On Tue, Oct 02, 2001 at 08:34:30PM -0500, William A. Rowe, Jr. wrote:
> From: "Justin Erenkrantz" <je...@ebuilt.com>
> Sent: Tuesday, October 02, 2001 8:29 PM
>
> > On Tue, Oct 02, 2001 at 01:24:25PM -0700, Doug MacEachern wrote:
> > > with current cvs all httpd-test ssl tests hang, stacktrace is same for
> > > all...
> >
> > I bet it's related to the input filtering changes.
> >
> > I have no clue how the whole mod_ssl input filtering works, but I'll
> > try to take a look. My guess is that mod_ssl is just a big quagmire.
>
> That, or your input filtering changes were the big quagmire. Either way,
> that sure sounds like the interaction that broke ssl.
I'll lay good odds that it is in mod_ssl rather than the input filtering
changes. Even *if* it is the latter, at least we have a codebase that is
much more maintainable now.
Cheers,
-g
--
Greg Stein, http://www.lyra.org/
Re: ssl is broken
Posted by "William A. Rowe, Jr." <wr...@covalent.net>.
From: "Justin Erenkrantz" <je...@ebuilt.com>
Sent: Tuesday, October 02, 2001 8:29 PM
> On Tue, Oct 02, 2001 at 01:24:25PM -0700, Doug MacEachern wrote:
> > with current cvs all httpd-test ssl tests hang, stacktrace is same for
> > all...
>
> I bet it's related to the input filtering changes.
>
> I have no clue how the whole mod_ssl input filtering works, but I'll
> try to take a look. My guess is that mod_ssl is just a big quagmire.
That, or your input filtering changes were the big quagmire. Either way,
that sure sounds like the interaction that broke ssl.
Re: ssl is broken
Posted by Justin Erenkrantz <je...@ebuilt.com>.
On Tue, Oct 02, 2001 at 01:24:25PM -0700, Doug MacEachern wrote:
> with current cvs all httpd-test ssl tests hang, stacktrace is same for
> all...
I bet it's related to the input filtering changes.
I have no clue how the whole mod_ssl input filtering works, but I'll
try to take a look. My guess is that mod_ssl is just a big quagmire.
-- justin
OpenSSL < 0.9.6 (Was: RE: ssl is broken)
Posted by Jeroen Massar <je...@unfix.org>.
Boo,
First I'll twack myself for the fact that I didn't look at the openssl
version any sooner...
This is prolly at least one nice for the archives for other donkeys
hitting their heads against this brick...
I wanted a newer version than the 2.0.16 beta's to experiment some more
with Apache 2.0..
so I pulled down http://dev.apache.org/dist/httpd-2_0_25-alpha.tar.gz
and tried to compile it with
--enable-ssl --with-openssl=/usr/include/openssl/ and such on a almost
standard FreeBSD 4.2 box which
comes with the "old" OpenSSL 0.9.5a, ./configure etc go just fine untill
the make pops up the following error:
8<-----------
modules/ssl/.libs/mod_ssl.al(ssl_engine_kernel.lo): In function
`bio_hook_set':
/usr/home/www/compile/httpd-2_0_25/modules/ssl/ssl_engine_kernel.c:264:
undefined reference to `BIO_next'
----------->8
And after some more tries and peeking at google I finally noticed that
my "old" OpenSSL version (twack me :)
Also http://www.openssl.org/docs/crypto/BIO_find_type.html nicely
states:
8<--------------------------
NOTES
BIO_next() was added to OpenSSL 0.9.6 to provide a 'clean' way to
traverse a BIO chain or find multiple matches using BIO_find_type().
Previous versions had to use: next = bio->next_bio;
-------------------------->8
Then added the following after the "#include <openssl/ssl.h>" in
modules/ssl/ssl_engine_kernel.c:
8<------------
#ifndef BIO_next
#define BIO_next(b) b->next_bio;
#endif
-------------->8
Ran another make..... et tada it compiled :)
I'd better upgrade my OpenSSL sooner or later though :)
[03/Oct/2001 01:37:59 91292] [info] Connection: Client IP:
3ffe:8114:2000:240:2d0:b7ff:fe8f:5d42, Protocol: SSLv3, Cipher: RC4-MD5
(128/128 bits)
But it works ;)
Next-stop... stability tests & mod_proxy <grin>
Greets,
Jeroen