You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe, Jr." <wr...@rowe-clan.net> on 2006/07/23 09:02:37 UTC

Re: svn commit: r424674 - /httpd/httpd/branches/2.0.x/STATUS

wrowe@apache.org wrote:
> Log:
> 
>   I really hated that bug's title :)
> 
> -    * mod_ssl: Solve SSLVerifyClient in <Location >
> -      (This was already committed to 2.2.x)
> +    * mod_ssl: Solve POST incompatible w/ renegotiate HTTPS connection
> +      (This was already committed to 2.2.x, and the reports persist.)
>          http://issues.apache.org/bugzilla/show_bug.cgi?id=12355
>        Patch; http://issues.apache.org/bugzilla/attachment.cgi?id=16495
>        +1: wrowe

Criminy!!!  If you are curious how many people really need this fix, figure
only half are ready to upgrade now to 2.2.x and scroll down through the list
of email notifications.  Never seen such a monstrous list in my life after
I clicked submit to change the bug title.

Re: svn commit: r424674 - /httpd/httpd/branches/2.0.x/STATUS

Posted by Ruediger Pluem <rp...@apache.org>.
On 25.07.2006 13:22, Joe Orton wrote:
> On Sun, Jul 23, 2006 at 10:58:39PM +0200, Ruediger Pluem wrote:

>>I don't know. Ask Joe :-)
> 
> 
> "A config directive is the last resort of the indecisive"... but I guess 
> it's going to have to happen in this case.  It would be similar in 
> spirit to LimitXMLRequestBody which is some kind of bound on in-RAM 
> buffering.
> 
> Buffering request bodies to disk is certainly *not* something which 
> should be generalised and encouraged IMO.  It's a DoS attack waiting to 
> happen in the proxy, and it would be a DoS attack waiting to happen in 
> mod_ssl.  Why should you assume that disk is an unlimited resource when 
> RAM isn't?  How do you know the chosen /tmp isn't RAM-backed tmpfs? etc

I don't assume that it is unlimited and of course there should be also an
upper limit for this disk file. As you may have seen I discussed with
Bill about a more generic aproach for setasides / ap_save_brigade.
I think it makes sense to have an upper limit for memory buffering and an
upper limit for disk buffering. Whether these settings can be different for
different areas like mod_ssl, mod_proxy, etc is another story.
I totally agree that having no limit imposes a high DoS risk and even if
we have a limit this is usually for a *single* request. So it might make
sense to have an upper limit for the whole server.
Just some thoughts.


Regards

Rüdiger


Re: svn commit: r424674 - /httpd/httpd/branches/2.0.x/STATUS

Posted by Joe Orton <jo...@redhat.com>.
On Sun, Jul 23, 2006 at 10:58:39PM +0200, Ruediger Pluem wrote:
> On 07/23/2006 09:59 PM, William A. Rowe, Jr. wrote:
> > Ruediger Pluem wrote:
> >> While we are at it. There is another related report (39243, 
> >> http://issues.apache.org/bugzilla/show_bug.cgi?id=39243). Some 
> >> people feel unhappy with the hardcoded 128k buffer. They would like 
> >> to see this size configurable at runtime. Joe does not like the 
> >> idea of another directive here. Another idea that came up to my 
> >> mind: A similar problem was solved in mod_proxy_http were we slurp 
> >> in the whole request body for C-L validation (even to a file if it 
> >> is too large for memory). Maybe this code could be generalized and 
> >> used by mod_ssl too.
> > 
> > +1, but... any reason to 1) not give the hard (or soft) buffer it's
> > own directive for the upper limit size?
> 
> I don't know. Ask Joe :-)

"A config directive is the last resort of the indecisive"... but I guess 
it's going to have to happen in this case.  It would be similar in 
spirit to LimitXMLRequestBody which is some kind of bound on in-RAM 
buffering.

Buffering request bodies to disk is certainly *not* something which 
should be generalised and encouraged IMO.  It's a DoS attack waiting to 
happen in the proxy, and it would be a DoS attack waiting to happen in 
mod_ssl.  Why should you assume that disk is an unlimited resource when 
RAM isn't?  How do you know the chosen /tmp isn't RAM-backed tmpfs? etc

joe

Re: svn commit: r424674 - /httpd/httpd/branches/2.0.x/STATUS

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Ruediger Pluem wrote:
> 
> Sounds like a good idea to me to work this either into the setaside code on apr-util side
> or into ap_save_brigade

++1 to both...

ap_save_brigade_ex ?

For setaside, of course, we could have a setaside_ex member, but it seems
to me we can have a common struct that persists as datum on the pool, itself,
consisting of setaside and setaside_limit members.  setaside would accumulate
the size of the setaside buckets, and setaside would fail if an allocation was
required and the setaside_limit would be exceeded.

Thoughts?

> But I guess for backwards compatibility we need to retain the old versions of setaside / ap_save_brigade
> that simply call the new ones with a limit of 0 (=unlimited).

Well, adding this as a datum to the pool (assuming unlimited if it's not found)
means that we could implement, in theory, in 1.2.x/0.9.x.  Adding any apr _ex
function would require 1.3.0, or 0.9.x which doesn't have the same versioning
rules.  For httpd, 1.2.3 can include this if ap_save_brigade_ex bumps the mmn.

ap_save_brigade_ex would know the rules for this operation and how to manipulate
the pool datum, or use the accessor calls against a pool to define setaside
limits.

Bill


Re: svn commit: r424674 - /httpd/httpd/branches/2.0.x/STATUS

Posted by Ruediger Pluem <rp...@apache.org>.

On 07/23/2006 09:59 PM, William A. Rowe, Jr. wrote:
> Ruediger Pluem wrote:
> 
>>
>> On 07/23/2006 09:02 AM, William A. Rowe, Jr. wrote:
>>
>>> Criminy!!!  If you are curious how many people really need this fix,
>>> figure
>>> only half are ready to upgrade now to 2.2.x and scroll down through the
>>> list
>>> of email notifications.  Never seen such a monstrous list in my life
>>> after
>>> I clicked submit to change the bug title.
>>
>>
>> While we are at it. There is another related report (39243,
>> http://issues.apache.org/bugzilla/show_bug.cgi?id=39243). Some people
>> feel unhappy
>> with the hardcoded 128k buffer. They would like to see this size
>> configurable at runtime.
>> Joe does not like the idea of another directive here. Another idea
>> that came up to my mind:
>> A similar problem was solved in mod_proxy_http were we slurp in the
>> whole request body
>> for C-L validation (even to a file if it is too large for memory).
>> Maybe this code could
>> be generalized and used by mod_ssl too.
> 
> 
> +1, but... any reason to 1) not give the hard (or soft) buffer it's
> own directive for the upper limit size?

I don't know. Ask Joe :-)

> 
> for the setaside code, we recently ran into this in John's new
> phpapachefilter
> code, and were thinking that if the setaside had an arguement for it's max
> setaside allocation, (per setaside, so phpfilter could have a
> PHPMaxFilterScript
> size directive, while ssl could have a SSLMaxRenegotiateBody size
> directive)
> then the user could avoid excessive setasides.
> 
> Since excessive setasides would -usually- be caused my misconfiguration
> (in the
> php filter case, trying to filter downloaded tar.gz files or something
> silly),
> capping setaside could be goodness.

Sounds like a good idea to me to work this either into the setaside code on apr-util side
or into ap_save_brigade, because excessive setasides can blow up the whole server. So
better a bunch of error messages and some wrong delivered content than a blown up server
where nobody knows why :-).
But I guess for backwards compatibility we need to retain the old versions of setaside / ap_save_brigade
that simply call the new ones with a limit of 0 (=unlimited).

Regards

Rüdiger

Re: svn commit: r424674 - /httpd/httpd/branches/2.0.x/STATUS

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Ruediger Pluem wrote:
> 
> On 07/23/2006 09:02 AM, William A. Rowe, Jr. wrote:
> 
>> Criminy!!!  If you are curious how many people really need this fix, figure
>> only half are ready to upgrade now to 2.2.x and scroll down through the
>> list
>> of email notifications.  Never seen such a monstrous list in my life after
>> I clicked submit to change the bug title.
> 
> While we are at it. There is another related report (39243,
> http://issues.apache.org/bugzilla/show_bug.cgi?id=39243). Some people feel unhappy
> with the hardcoded 128k buffer. They would like to see this size configurable at runtime.
> Joe does not like the idea of another directive here. Another idea that came up to my mind:
> A similar problem was solved in mod_proxy_http were we slurp in the whole request body
> for C-L validation (even to a file if it is too large for memory). Maybe this code could
> be generalized and used by mod_ssl too.

+1, but... any reason to 1) not give the hard (or soft) buffer it's
own directive for the upper limit size?

for the setaside code, we recently ran into this in John's new phpapachefilter
code, and were thinking that if the setaside had an arguement for it's max
setaside allocation, (per setaside, so phpfilter could have a PHPMaxFilterScript
size directive, while ssl could have a SSLMaxRenegotiateBody size directive)
then the user could avoid excessive setasides.

Since excessive setasides would -usually- be caused my misconfiguration (in the
php filter case, trying to filter downloaded tar.gz files or something silly),
capping setaside could be goodness.

Bill

Re: svn commit: r424674 - /httpd/httpd/branches/2.0.x/STATUS

Posted by Ruediger Pluem <rp...@apache.org>.

On 07/23/2006 09:02 AM, William A. Rowe, Jr. wrote:

> Criminy!!!  If you are curious how many people really need this fix, figure
> only half are ready to upgrade now to 2.2.x and scroll down through the
> list
> of email notifications.  Never seen such a monstrous list in my life after
> I clicked submit to change the bug title.

While we are at it. There is another related report (39243,
http://issues.apache.org/bugzilla/show_bug.cgi?id=39243). Some people feel unhappy
with the hardcoded 128k buffer. They would like to see this size configurable at runtime.
Joe does not like the idea of another directive here. Another idea that came up to my mind:
A similar problem was solved in mod_proxy_http were we slurp in the whole request body
for C-L validation (even to a file if it is too large for memory). Maybe this code could
be generalized and used by mod_ssl too.

Regards

Rüdiger