Re: help needed to complete regression fix for apache2 Bug#858373

[Brian Kroth]

Hi, sorry for the delay. Gmail filed this one into spam :-(

Unfortunately, I don't have access to that environment anymore to confirm.
I'll pass this on to the folks that do so hopefully they can.

My recollection from this issue was that I'd tested it against different
package versions and the 400 ErrorDocuments had worked beforehand (we used
them for ModSec types of things primarily, and I'm confident that mode was
working well before hand and after), though possibly not in that particular
protocol error context. I vaguely recall having issues reproducing a
working ErrorDocument with non-cgi methods in that protocol error mode
style test as well, but I don't recall if rhat was only in the newer
versions of the software that I had been testing with or true before that
update as well.

Anyways, thanks much for following up. Sorry I don't have more info to
offer at the moment.

Cheers,
Brian

On Fri, Jul 21, 2017, 08:44 Antoine Beaupré  wrote:

> TL;DR: New proposed package (deb7u11) doesn't ctually show a new
> regression, please test:
>
>
> https://people.debian.org/~anarcat/debian/wheezy-lts/apache2_2.2.22-13+deb7u11_amd64.changes
>
> In particular, Brian Kroth: are you *sure* you had that ErrorDocument
> 400 working in apache2_2.2.22-13+deb7u7 (ie. before the DLA-841-1
> upload)? In my tests, it didn't actually work at all. It wouldn't
> trigger a segfault, but the CGI script wouldn't get called either. In
> the above package, we don't segfault anymore, but we yield a 400 + 500
> error message (because the ErrorDocument fails). The solution, here, is
> obviously to update to a later Apache version (e.g. update to jessie,
> really) to get that functionality working, from my perspective.
>
> More technical details follow.
>
> On 2017-07-21 09:24:00, Stefan Fritsch wrote:
> > Hi Antoine,
> >
> > On Wednesday, 19 July 2017 15:45:20 CEST Antoine Beaupre wrote:
> >> As I mentioned in the #858373 bug report, I started looking at fixing
> >> the regression introduced by the 2.2.22-13+deb7u8 upload, part of
> >> DLA-841-1. The problem occurs when a CGI(d) ErrorDocument is configured
> >> to handle 400 error messages that can be triggered with a simple "GET /
> >> HTTP/1.0\n\n". Such a request segfaults Apache in Wheezy right now.
> >
> >> Unfortunately, re-introducing the protocol initialization code isn't
> >> sufficient: it does fix the segfaults, but the ErrorDocument handling is
> >> not quite working yet. Instead of seeing the output of the
> >> ErrorDocument, after 10 seconds, I get the raw 400 message, doubled with
> >> a 500 error document warning:
> >
> >> Note that I have also tried to see if sending "\r\n" instead of just
> >> "\n" in my "hello world" example would work around the issue: it
> >> doesn't, unfortunately.
> >>
> >> I am at a loss as where to go from here, to be honest. The patch
> >> (attached) at least fixes the segfault, which resolves the primary issue
> >> at hand here (DoS by crashing processes!) but it would be nice to
> >> actually fix the ErrorDocument as well..
> >
> > This sounds familiar. Maybe it's simply broken in 2.2.22. Can you
> compare with
> > 2.2.22-13+deb7u7 if that bug has been there already?
>
> Well, the problem is - how do I reproduce this? I can't generate the
> same 400 error message in deb7u7 (I tried!) with the previous techniques
> because the new request handling code isn't there. That is, the
> following query just works:
>
> # printf "GET / HTTP/1.0\n\n" | nc localhost 80 | head -1
> HTTP/1.1 200 OK
>
>
> Furthermore, generating a 400 error, when it works in deb7u7, doesn't
> trigger the ErrorDocument - not sure why:
>
> # printf "G ET / HTTP/1.0\r\n\r\n" | nc localhost 80
> HTTP/1.1 400 Bad Request
> Date: Fri, 21 Jul 2017 13:40:48 GMT
> Server: Apache/2.2.22 (Debian)
> Vary: Accept-Encoding
> Content-Length: 302
> Connection: close
> Content-Type: text/html; charset=iso-8859-1
>
> 
> 
> 400 Bad Request
> 
> Bad Request
> Your browser sent a request that this server could not understand.
> 
> 
> Apache/2.2.22 (Debian) Server at wheezy.raw Port 80
> 
>
> Logs show the following:
>
> [Fri Jul 21 13:40:48 2017] [error] [client 127.0.0.1] Invalid URI in
> request G ET / HTTP/1.0
>
> ... whether or not the 400 ErrorDocument directive is present. Notice
> how the ErrorDocument isn't triggered at all here.
>
> Of course, a 404 ErrorDocument still works correctly:
>
> # printf "GET /wtf HTTP/1.0\r\n\r\n" | nc localhost 80
> HTTP/1.1 404 Not Found
> Date: Fri, 21 Jul 2017 13:23:46 GMT
> Server: Apache/2.2.22 (Debian)
> Vary: Accept-Encoding
> Connection: close
> Content-Type: text/plain
>
> Hello, World.
>
> I get this behavior consistently with deb7u7 and the proposed deb7u11
> (which only adds a 500 error document to *certain* 400 errors,
> basically). I find that is an acceptable compromise to fix a segfault,
> and, from my perspective, doesn't introduce a regression.
>
> > In 2.2.30, there is this fix, which is obviously missing from 

Processed: Re: help needed to complete regression fix for apache2 Bug#858373

[Debian Bug Tracking System]

Processing control commands:

> fixed 858373 2.2.22-13+deb7u7
Bug #858373 [apache2.2-common] apache2: segfaults upon recieving bad request 
when using worker/event mpm and cgid errordoc
There is no source info for the package 'apache2.2-common' at version 
'2.2.22-13+deb7u7' with architecture ''
Unable to make a source version for version '2.2.22-13+deb7u7'
Marked as fixed in versions 2.2.22-13+deb7u7.
> tags 858373 +pending +patch
Bug #858373 [apache2.2-common] apache2: segfaults upon recieving bad request 
when using worker/event mpm and cgid errordoc
Added tag(s) pending.
Bug #858373 [apache2.2-common] apache2: segfaults upon recieving bad request 
when using worker/event mpm and cgid errordoc
Added tag(s) patch.

-- 
858373: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=858373
Debian Bug Tracking System
Contact ow...@bugs.debian.org with problems

Bug#858373: help needed to complete regression fix for apache2 Bug#858373

[Antoine Beaupré]

Control: fixed 858373 2.2.22-13+deb7u7
Control: tags 858373 +pending +patch

On 2017-07-21 09:44:38, Antoine Beaupré wrote:
> TL;DR: New proposed package (deb7u11) doesn't actually show a new
> regression, please test:
>
> https://people.debian.org/~anarcat/debian/wheezy-lts/apache2_2.2.22-13+deb7u11_amd64.changes
>
> In particular, Brian Kroth: are you *sure* you had that ErrorDocument
> 400 working in apache2_2.2.22-13+deb7u7 (ie. before the DLA-841-1
> upload)? In my tests, it didn't actually work at all. It wouldn't
> trigger a segfault, but the CGI script wouldn't get called either. In
> the above package, we don't segfault anymore, but we yield a 400 + 500
> error message (because the ErrorDocument fails). The solution, here, is
> obviously to update to a later Apache version (e.g. update to jessie,
> really) to get that functionality working, from my perspective.

Timing out on this one: I will assume that 2.2.22-13+deb7u7 didn't
segfault, but then didn't yield a proper ErrorDocument either (because I
cannot reproduce that behavior).

I have uploaded deb7u11 and will send the associated DLA-841-2
regression update when it hits the archives.

A.

-- 
Seul a un caractère scientifique ce qui peut être réfuté. Ce qui n'est
pas réfutable relève de la magie ou de la mystique.
- Karl Popper

Bug#858373: help needed to complete regression fix for apache2 Bug#858373

[Antoine Beaupré]

TL;DR: New proposed package (deb7u11) doesn't actually show a new
regression, please test:

https://people.debian.org/~anarcat/debian/wheezy-lts/apache2_2.2.22-13+deb7u11_amd64.changes

In particular, Brian Kroth: are you *sure* you had that ErrorDocument
400 working in apache2_2.2.22-13+deb7u7 (ie. before the DLA-841-1
upload)? In my tests, it didn't actually work at all. It wouldn't
trigger a segfault, but the CGI script wouldn't get called either. In
the above package, we don't segfault anymore, but we yield a 400 + 500
error message (because the ErrorDocument fails). The solution, here, is
obviously to update to a later Apache version (e.g. update to jessie,
really) to get that functionality working, from my perspective.

More technical details follow.

On 2017-07-21 09:24:00, Stefan Fritsch wrote:
> Hi Antoine,
>
> On Wednesday, 19 July 2017 15:45:20 CEST Antoine Beaupre wrote:
>> As I mentioned in the #858373 bug report, I started looking at fixing
>> the regression introduced by the 2.2.22-13+deb7u8 upload, part of
>> DLA-841-1. The problem occurs when a CGI(d) ErrorDocument is configured
>> to handle 400 error messages that can be triggered with a simple "GET /
>> HTTP/1.0\n\n". Such a request segfaults Apache in Wheezy right now.
>
>> Unfortunately, re-introducing the protocol initialization code isn't
>> sufficient: it does fix the segfaults, but the ErrorDocument handling is
>> not quite working yet. Instead of seeing the output of the
>> ErrorDocument, after 10 seconds, I get the raw 400 message, doubled with
>> a 500 error document warning:
>
>> Note that I have also tried to see if sending "\r\n" instead of just
>> "\n" in my "hello world" example would work around the issue: it
>> doesn't, unfortunately.
>> 
>> I am at a loss as where to go from here, to be honest. The patch
>> (attached) at least fixes the segfault, which resolves the primary issue
>> at hand here (DoS by crashing processes!) but it would be nice to
>> actually fix the ErrorDocument as well..
>
> This sounds familiar. Maybe it's simply broken in 2.2.22. Can you compare 
> with 
> 2.2.22-13+deb7u7 if that bug has been there already?

Well, the problem is - how do I reproduce this? I can't generate the
same 400 error message in deb7u7 (I tried!) with the previous techniques
because the new request handling code isn't there. That is, the
following query just works:

# printf "GET / HTTP/1.0\n\n" | nc localhost 80 | head -1
HTTP/1.1 200 OK


Furthermore, generating a 400 error, when it works in deb7u7, doesn't
trigger the ErrorDocument - not sure why:

# printf "G ET / HTTP/1.0\r\n\r\n" | nc localhost 80
HTTP/1.1 400 Bad Request
Date: Fri, 21 Jul 2017 13:40:48 GMT
Server: Apache/2.2.22 (Debian)
Vary: Accept-Encoding
Content-Length: 302
Connection: close
Content-Type: text/html; charset=iso-8859-1



400 Bad Request

Bad Request
Your browser sent a request that this server could not understand.


Apache/2.2.22 (Debian) Server at wheezy.raw Port 80


Logs show the following:

[Fri Jul 21 13:40:48 2017] [error] [client 127.0.0.1] Invalid URI in request G 
ET / HTTP/1.0

... whether or not the 400 ErrorDocument directive is present. Notice
how the ErrorDocument isn't triggered at all here.

Of course, a 404 ErrorDocument still works correctly:

# printf "GET /wtf HTTP/1.0\r\n\r\n" | nc localhost 80
HTTP/1.1 404 Not Found
Date: Fri, 21 Jul 2017 13:23:46 GMT
Server: Apache/2.2.22 (Debian)
Vary: Accept-Encoding
Connection: close
Content-Type: text/plain

Hello, World.

I get this behavior consistently with deb7u7 and the proposed deb7u11
(which only adds a 500 error document to *certain* 400 errors,
basically). I find that is an acceptable compromise to fix a segfault,
and, from my perspective, doesn't introduce a regression.

> In 2.2.30, there is this fix, which is obviously missing from 2.2.22:
>
>   *) core, modules: Avoid error response/document handling by the core if some
>  handler or input filter already did it while reading the request (causing
>  a double response body).  [Yann Ylavic]
>
> I could not find a changelog entry about the 10s delay, but it's possible 
> that 
> that has been fixed as well. If the issue is not a regression, you should 
> simply release the patch that you have. The fix for the error document seems 
> rather invasive:
>
> https://svn.apache.org/r1683808

But that's another big patch to backport:

 20 files changed, 196 insertions(+), 129 deletions(-)

Not sure we want to pile yet another backport on top of the pile we
already have. Now I really regret not updating to 2.2.34. :(

Since this issue doesn't seem to be a regression (the ErrorDocument
didn't seem to get called at all, previously), I think I'll just post a
test package with the regression fix and be done with it for now.

I'm more confident in the upload now, and hopefully it won't break too
many things now. At least we don't segfault. ;)

I'll be available to upload the test package tomorrow or by the end of
next week, if there 

Re: help needed to complete regression fix for apache2 Bug#858373

[Stefan Fritsch]

Hi Antoine,

On Wednesday, 19 July 2017 15:45:20 CEST Antoine Beaupre wrote:
> As I mentioned in the #858373 bug report, I started looking at fixing
> the regression introduced by the 2.2.22-13+deb7u8 upload, part of
> DLA-841-1. The problem occurs when a CGI(d) ErrorDocument is configured
> to handle 400 error messages that can be triggered with a simple "GET /
> HTTP/1.0\n\n". Such a request segfaults Apache in Wheezy right now.

> Unfortunately, re-introducing the protocol initialization code isn't
> sufficient: it does fix the segfaults, but the ErrorDocument handling is
> not quite working yet. Instead of seeing the output of the
> ErrorDocument, after 10 seconds, I get the raw 400 message, doubled with
> a 500 error document warning:

> Note that I have also tried to see if sending "\r\n" instead of just
> "\n" in my "hello world" example would work around the issue: it
> doesn't, unfortunately.
> 
> I am at a loss as where to go from here, to be honest. The patch
> (attached) at least fixes the segfault, which resolves the primary issue
> at hand here (DoS by crashing processes!) but it would be nice to
> actually fix the ErrorDocument as well..

This sounds familiar. Maybe it's simply broken in 2.2.22. Can you compare with 
2.2.22-13+deb7u7 if that bug has been there already?

In 2.2.30, there is this fix, which is obviously missing from 2.2.22:

  *) core, modules: Avoid error response/document handling by the core if some
 handler or input filter already did it while reading the request (causing
 a double response body).  [Yann Ylavic]

I could not find a changelog entry about the 10s delay, but it's possible that 
that has been fixed as well. If the issue is not a regression, you should 
simply release the patch that you have. The fix for the error document seems 
rather invasive:

https://svn.apache.org/r1683808

Cheers,
Stefan

Bug#858373: help needed to complete regression fix for apache2 Bug#858373

[Antoine Beaupré]

And then, obviously, I forget the patch.

Sorry for the noise.

-- 
The secret of life is to have no fear; it's the only way to function.
- Stokely Carmichael
diff -Nru apache2-2.2.22/debian/changelog apache2-2.2.22/debian/changelog
--- apache2-2.2.22/debian/changelog	2017-07-17 03:50:16.0 -0400
+++ apache2-2.2.22/debian/changelog	2017-07-19 14:12:44.0 -0400
@@ -1,3 +1,12 @@
+apache2 (2.2.22-13+deb7u11) UNRELEASED; urgency=high
+
+  * Non-maintainer upload by the LTS Security Team.
+  * fix regression introduced in 2.2.22-13+deb7u8 that re-introduced
+something like CVE-2015-0253 when fixing CVE-2016-8743 (Closes:
+#858373)
+
+ -- Antoine Beaupré   Wed, 19 Jul 2017 14:12:44 -0400
+
 apache2 (2.2.22-13+deb7u10) wheezy-security; urgency=high
 
   * CVE-2017-9788: The value placeholder in [Proxy-]Authorization headers of
diff -Nru apache2-2.2.22/debian/patches/CVE-2016-8743-regression.patch apache2-2.2.22/debian/patches/CVE-2016-8743-regression.patch
--- apache2-2.2.22/debian/patches/CVE-2016-8743-regression.patch	1969-12-31 19:00:00.0 -0500
+++ apache2-2.2.22/debian/patches/CVE-2016-8743-regression.patch	2017-07-19 14:12:44.0 -0400
@@ -0,0 +1,23 @@
+Description: fix regression introduced in CVE-2016-8743
+ The messy CVE-2016-8743 patchset introduced an error in protocol
+ initialization in some error cases. This makes sure that invalid
+ requests doesn't segfault apache.
+ .
+ This is similar, but not directly related to CVE-2015-0253.
+Origin: https://svn.apache.org/viewvc/httpd/httpd/branches/2.4.x/server/protocol.c?r1=1642403=1668879=1668879=patch
+Bug-Debian: 858373
+Forwarded: not-needed
+Author: Antoine Beaupré
+Last-update: 2017-07-19
+
+--- a/server/protocol.c
 b/server/protocol.c
+@@ -637,6 +637,8 @@ static int read_request_line(request_rec
+ else if (APR_STATUS_IS_EINVAL(rv)) {
+ r->status = HTTP_BAD_REQUEST;
+ }
++r->proto_num = HTTP_VERSION(1,0);
++r->protocol  = apr_pstrdup(r->pool, "HTTP/1.0");
+ return 0;
+ }
+ } while ((len <= 0) && (++num_blank_lines < max_blank_lines));
diff -Nru apache2-2.2.22/debian/patches/series apache2-2.2.22/debian/patches/series
--- apache2-2.2.22/debian/patches/series	2017-07-17 03:50:33.0 -0400
+++ apache2-2.2.22/debian/patches/series	2017-07-19 14:12:44.0 -0400
@@ -61,3 +61,4 @@
 CVE-2017-7668.patch
 CVE-2017-7669.patch
 CVE-2017-9788.patch
+CVE-2016-8743-regression.patch

help needed to complete regression fix for apache2 Bug#858373

[Antoine Beaupre]

Hi,

(Sorry for the large CC list, but I am hoping to get a broad approval of
the next changes for this in order to avoid previous mistakes. ;) In
particular, I'd be very grateful for some input by Stefan considering
his knowledge of the Apache codebase and how ... exotic this problems
is.)

As I mentioned in the #858373 bug report, I started looking at fixing
the regression introduced by the 2.2.22-13+deb7u8 upload, part of
DLA-841-1. The problem occurs when a CGI(d) ErrorDocument is configured
to handle 400 error messages that can be triggered with a simple "GET /
HTTP/1.0\n\n". Such a request segfaults Apache in Wheezy right now.

I have been able to confirm that there is an unitialized variable that
gets carried around. This issue was introduced as part of
CVE-2016-8743-aux.patch in the original upload, although I fail to
remember now why this hunk is there exactly. It seems to be related to a
patch I somewhat blindly and incorrectly merged (see
87r33tqvqs@curie.anarc.at for details).

Unfortunately, re-introducing the protocol initialization code isn't
sufficient: it does fix the segfaults, but the ErrorDocument handling is
not quite working yet. Instead of seeing the output of the
ErrorDocument, after 10 seconds, I get the raw 400 message, doubled with
a 500 error document warning:

$ echo -ne "GET /foo HTTP/1.0\n\n" | nc localhost 80
HTTP/1.1 400 Bad Request
Date: Wed, 19 Jul 2017 19:11:13 GMT
Server: Apache/2.2.22 (Debian)
Vary: Accept-Encoding
Content-Length: 433
Connection: close
Content-Type: text/html; charset=iso-8859-1



400 Bad Request

Bad Request
Your browser sent a request that this server could not understand.

Additionally, a 500 Internal Server Error
error was encountered while trying to use an ErrorDocument to handle the 
request.

Apache/2.2.22 (Debian) Server at wheezy.raw Port 80


In the error log, I see:

[Wed Jul 19 19:11:23 2017] [error] [client 127.0.0.1] (70007)The timeout
specified has expired: Error reading request entity data

The first part of the error is mod_reqtimeout kicking in as the request
parser stalls on the CGI script. The second part is mod_cgi(d) failing
to read the request from the CGI script, obviously.

My theory is that there is *still* something wrong with the request
parser, even after fixing the r->protocol initialization flaw. I base
this theory on the fact that a 404 ErrorDocument works without problem.

$ echo -ne "GET /foo HTTP/1.0\r\n\r\n" | nc localhost 80
HTTP/1.1 404 Not Found
Date: Wed, 19 Jul 2017 19:13:44 GMT
Server: Apache/2.2.22 (Debian)
Vary: Accept-Encoding
Connection: close
Content-Type: text/html

Hello, World.

Note that I have also tried to see if sending "\r\n" instead of just
"\n" in my "hello world" example would work around the issue: it
doesn't, unfortunately.

I am at a loss as where to go from here, to be honest. The patch
(attached) at least fixes the segfault, which resolves the primary issue
at hand here (DoS by crashing processes!) but it would be nice to
actually fix the ErrorDocument as well..

Any ideas?

Thanks in advance,

A.


signature.asc
Description: PGP signature