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

2017-07-29 Thread 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

2017-07-21 Thread 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 

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

2017-07-19 Thread 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é <anar...@debian.org>  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


Re: Wheezy update of apache2?

2017-07-19 Thread Antoine Beaupré
On 2017-07-18 20:53:35, Stefan Fritsch wrote:
> On Monday, 17 July 2017 16:57:00 CEST Roberto C. Sánchez wrote:
>> I did the deb7u9 update of apache2 and I was not aware of the regression
>> either.  I wonder if it makes sense for bugs above a certain severity
>> affecting versions of a package which are security uploads to show up in
>> the security tracker.  Or would there be some other sensible way, aside
>> from having to go to the BTS directly?
>
> Sorry that I haven't forwarded that to you in a timely manner. I think I have 
> mentioned it before the previous upload, but it may have gotten lost 
> somewhere.
>
> I don't know how a reasonable automatic notification could look like. 
> Probably 
> it has to be up to the maintainer to forward such bug reports.

I would agree as well - we can't possibly watch all of the BTS for such
reports. :)

Honestly, I was surprised there wasn't more pushback on DLA-841-1: it
was a major change with significant impact. The patch was a mess to
backport, and basically rewrote the request parser in Apache (!). It was
bound to introduce more issues.

I'll try to tackle this one, naturally, since I'm the one who issued the
DLA in the end!

sorry about the trouble.

a.

-- 
A genius is someone who discovers that the stone that falls and the
moon that doesn't fall represent one and the same phenomenon.
 - Ernesto Sabato



Re: testing and review requested for Wheezy update of apache2

2017-02-28 Thread Antoine Beaupré
On 2017-02-23 19:14:59, Jonas Meurer wrote:
> Am 23.02.2017 um 11:59 schrieb Guido Günther:
>> On Wed, Feb 22, 2017 at 06:54:46PM +0100, Jonas Meurer wrote:
>>> Am 22.02.2017 um 18:46 schrieb Guido Günther:
 Hi Jonas,
 On Wed, Feb 22, 2017 at 05:28:46PM +0100, Jonas Meurer wrote:
> This time with the debdiff between Antoine's version and mine.
 Are there packages available for testing? I could give it another whirl.
>>>
>>> Sorry, yes you can find them at
>>> https://people.debian.org/~mejo/wheezy-lts/
>> 
>> Thanks I tested these with several combinations of HttpProtocolOptions
>> on a apache serving pages directly and in reverse proxy mode with no
>> noticed ill effects.
>
> All right, then we should go for the update. Antoine, do you take care
> of it?

Yes, I'll review and complete the upload today.

Thanks for the reviews and testing everyone...

A.
-- 
If builders built houses the way programmers built programs,
The first woodpecker to come along would destroy civilization.
- Gerald Weinberg



Re: testing and review requested for Wheezy update of apache2

2017-02-20 Thread Antoine Beaupré
On 2017-02-13 21:48:45, Stefan Fritsch wrote:
> Hi Antoine,

Hi!

With a fresh mind (and 30 days delay!) I am looking at this again...

>> here are those tests:
>> 
>> 2: [ "GET / HTTP/1.0\n\n"  => 400],
>> 8: [ "GET / HTTP/1.0\0\r\n\r\n"=> 400],
>> 26: [ "GET / HTTP/1.0\r\nFoo: b\0ar\r\n\r\n"=> 400],
>> 
>> #2 is weird - it just returns nothing now:
>> 
>> $ printf "GET / HTTP/1.0\n\n" | nc localhost 80
>> $
>
> This works better in 2.4.10 with the backport and in 2.4.25: There,
>
> printf "GET / HTTP/1.0\n\n" | nc localhost 80
>
> gives a 400 Bad request, as expected.

Hm... I tried the packages I uploaded last month to people.d.o, and they
do not fail those tests anymore. At least,

# printf "GET / HTTP/1.0\n\n" | nc localhost 80
HTTP/1.1 400 Bad Request
# printf "GET / HTTP/1.0\r\nFoo: b\0ar\r\n\r\n" | nc localhost 80 
HTTP/1.1 400 Bad Request
# printf "GET / HTTP/1.0\0\r\n\r\n" | nc localhost 80
HTTP/1.1 400 Bad Request

This all looks good...

> On Monday, 23 January 2017 17:03:55 CET Antoine Beaupré wrote:
>> On 2017-01-23 15:14:30, Antoine Beaupré wrote:
>> > On 2017-01-22 11:25:08, Stefan Fritsch wrote:
>> >>  Test Summary Report
>> >>  ---
>> >>  t/apache/chunkinput.t (Wstat: 0 Tests: 37 Failed: 1)
>> >>  
>> >>Failed test:  3
>> >>  
>> >>  t/apache/contentlength.t  (Wstat: 0 Tests: 24 Failed: 8)
>> >>  
>> >>Failed tests:  2, 4, 14, 16, 18, 20, 22, 24
>> >> 
>> >> +t/apache/http_strict.t(Wstat: 0 Tests: 85 Failed: 3)
>> >> +  Failed tests:  2, 8, 26
>> > 
>> > here are those tests:
>> > 
>> > 2: [ "GET / HTTP/1.0\n\n"  => 400],
>> > 8: [ "GET / HTTP/1.0\0\r\n\r\n"=> 400],
>> > 26: [ "GET / HTTP/1.0\r\nFoo: b\0ar\r\n\r\n"=> 400],
>> 
>> turns out the latter two here are unrelated issues. 2.2.32 includes this
>> code:
>> 
>> /* PR#43039: We shouldn't accept NULL bytes within the line */
>> if (strlen(*s) < bytes_handled) {
>> return APR_EINVAL;
>> }
>> 
>> which is fair, but not directly part of this rewrite, as far as I know
>> - this seems more related to this patch:
>> 
>> http://svn.apache.org/viewvc?view=revision=1758671
>> http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/protocol.c?r1
>> =1757394=1758671=1758671_format=h
>> 
>> I am not sure we should factor this into the package, but without it,
>> test case #2 is so broken that I am worried it introduces other
>> regressions, so I bundled it in.
>
> Have you tried if that diff helps with
>
>   printf "GET / HTTP/1.0\n\n" | nc localhost 80
>
> returning nothing? I don't see how the change would help, but I haven't tried 
> it or looked at the code in total.

Well, it's rather strange but things all seem to work here.. .The above
now returns the 400 error as expected.

>> It does mean that "echo GET / | nc localhost 80" now fails, but that
>> seems to be the design of the Apache team, unfortunately. :( No more
>> "telnet into port 80" it seems?
>
> Note that telnet does LF -> CRLF conversion, so telnet still works for 
> debugging. But nc does not.

That should be acceptable. I am not sure it does it correctly, however:

# printf "GET / HTTP/1.0\r\n\r\n" | nc localhost 80
HTTP/1.1 400 Bad Request

That doesn't look right, actually - shouldn't this return a 200?

# printf "GET /\r\n\r\n" | nc localhost 80

returns a correct 200, however.

>> I am really wondering why we shouldn't just package 2.2.32 after
>> all. the change is kind of massive, but it would make me feel much
>> better than the current patch set:
>> 
>>  136 files changed, 1738 insertions(+), 4409 deletions(-)
>
> I am not sure either. For 2.4.10/jessie  the changes are a bit less intrusive 
> and I intend to use the backported version:
>
>  9 files changed, 1064 insertions(+), 303 deletions(-)
>
> But for 2.2/wheezy it's a difficult decision.

The problem with wheezy is that it would require reviewing years of
patches to see which one have been factored upstream:

apache2-2.2.22$ diffstat debian/patches/* | tail -1
 92 files changed, 3687 insertions(+), 1533 deletions(-)

It's interesting that this is similar to the diff between the two
versions. ;) In other words, the diff betwee

Re: testing and review requested for Wheezy update of apache2

2017-01-23 Thread Antoine Beaupré
On 2017-01-23 15:14:30, Antoine Beaupré wrote:
> On 2017-01-22 11:25:08, Stefan Fritsch wrote:
>>  Test Summary Report
>>  ---
>>  t/apache/chunkinput.t (Wstat: 0 Tests: 37 Failed: 1)
>>Failed test:  3
>>  t/apache/contentlength.t  (Wstat: 0 Tests: 24 Failed: 8)
>>Failed tests:  2, 4, 14, 16, 18, 20, 22, 24
>> +t/apache/http_strict.t(Wstat: 0 Tests: 85 Failed: 3)
>> +  Failed tests:  2, 8, 26
>
> here are those tests:
>
> 2: [ "GET / HTTP/1.0\n\n"  => 400],
> 8: [ "GET / HTTP/1.0\0\r\n\r\n"=> 400],
> 26: [ "GET / HTTP/1.0\r\nFoo: b\0ar\r\n\r\n"=> 400],

turns out the latter two here are unrelated issues. 2.2.32 includes this
code:

/* PR#43039: We shouldn't accept NULL bytes within the line */
if (strlen(*s) < bytes_handled) {
return APR_EINVAL;
}

which is fair, but not directly part of this rewrite, as far as I know
- this seems more related to this patch:

http://svn.apache.org/viewvc?view=revision=1758671
http://svn.apache.org/viewvc/httpd/httpd/branches/2.2.x/server/protocol.c?r1=1757394=1758671=1758671_format=h

I am not sure we should factor this into the package, but without it,
test case #2 is so broken that I am worried it introduces other
regressions, so I bundled it in.

It does mean that "echo GET / | nc localhost 80" now fails, but that
seems to be the design of the Apache team, unfortunately. :( No more
"telnet into port 80" it seems?

I am really wondering why we shouldn't just package 2.2.32 after
all. the change is kind of massive, but it would make me feel much
better than the current patch set:

 136 files changed, 1738 insertions(+), 4409 deletions(-)

I'm running out of hours for this month, unfortunately. I will be able
to continue the work in february, but it would probably be better for
others to pick that up before that.

I have reuploaded a new version of the package with the extra above
patch, and I believe it passes the test suite correctly now.

I am not confident enough to upload the result as is, so I would like
another LTS worker to look into this before a final upload.

Thanks so much for the review and feedback, Stefan!

A.

-- 
How inappropriate to call this planet 'Earth' when it is quite clearly
'Ocean'.
- Arthur C. Clarke



Re: testing and review requested for Wheezy update of apache2

2017-01-23 Thread Antoine Beaupré
On 2017-01-22 11:25:08, Stefan Fritsch wrote:
>  Test Summary Report
>  ---
>  t/apache/chunkinput.t (Wstat: 0 Tests: 37 Failed: 1)
>Failed test:  3
>  t/apache/contentlength.t  (Wstat: 0 Tests: 24 Failed: 8)
>Failed tests:  2, 4, 14, 16, 18, 20, 22, 24
> +t/apache/http_strict.t(Wstat: 0 Tests: 85 Failed: 3)
> +  Failed tests:  2, 8, 26

here are those tests:

2: [ "GET / HTTP/1.0\n\n"  => 400],
8: [ "GET / HTTP/1.0\0\r\n\r\n"=> 400],
26: [ "GET / HTTP/1.0\r\nFoo: b\0ar\r\n\r\n"=> 400],

#2 is weird - it just returns nothing now:

$ printf "GET / HTTP/1.0\n\n" | nc localhost 80
$

which is a major WTF for someone used to "telnet into port 80", as UNIX
line endings now fail - which is unexpected of course, but actually
respecting RFC, iirc.

nothing in the logs, and nothing i could find in GDB. the apache2
process just seems to close the socket and leave it at that.

#8 is a plain failure - it actually works and returns 200 instead of
400. same with #26.

so definitely more work to be done there... sigh. i'll review the final
diff to figure out what's going on here.

a.

-- 
During times of universal deceit, telling the truth becomes a
revolutionary act.- Georges Orwell



Re: testing and review requested for Wheezy update of apache2

2017-01-23 Thread Antoine Beaupré
On 2017-01-22 11:25:08, Stefan Fritsch wrote:
> On Thursday, 19 January 2017 20:47:15 CET Stefan Fritsch wrote:
>> On Tuesday, 17 January 2017 11:59:17 CET Antoine Beaupré wrote:
>> > I would need people to start testing the package at this point, not
>> > necessarily in production considering how big the change is, but your
>> > comfort level will vary with the severity and complexity of services. :)
>> 
>> There is a separate test suite available, though it needs some tweaks to
>> make it run with the Debian config layout. I will try to find some time
>> coming week- end to run it against the wheezy package with and without your
>> changes.
>
> This doesn't look too bad, but not perfect. The diff of the results from 
> 2.2.22-13+deb7u7 to your packages, with tweaks to make the test suite run the 
> tests that are new for 2.2.32 is this:
>
>  Test Summary Report
>  ---
>  t/apache/chunkinput.t (Wstat: 0 Tests: 37 Failed: 1)
>Failed test:  3
>  t/apache/contentlength.t  (Wstat: 0 Tests: 24 Failed: 8)
>Failed tests:  2, 4, 14, 16, 18, 20, 22, 24
> +t/apache/http_strict.t(Wstat: 0 Tests: 85 Failed: 3)
> +  Failed tests:  2, 8, 26
> +t/apache/mmn.t(Wstat: 0 Tests: 2 Failed: 1)
> +  Failed test:  2
> +t/apache/server_name_port.t   (Wstat: 0 Tests: 84 Failed: 0)
> +  TODO passed:   57, 60, 81, 84
>  t/security/CVE-2005-3352.t(Wstat: 0 Tests: 2 Failed: 1)
>Failed test:  2
> -Files=116, Tests=3479, 233 wallclock secs ( 1.31 usr  0.12 sys + 51.14 cusr  
> 8.12 csys = 60.69 CPU)
> +Files=116, Tests=3567, 238 wallclock secs ( 1.36 usr  0.07 sys + 52.43 cusr  
> 8.02 csys = 61.88 CPU)
>
> The mmn.t fail is expected and ok. You haven't backported all new features 
> from 2.2.32, so you should not bump the magic number to the number from 
> 2.2.32.

Understood. I was wondering if I should change it somehow... Note that I
do not use the 2.2.32 version, but some inferior one, which is probably
also wrong... I have just removed this chunk from the patchset.

> The messages from t/apache/server_name_port.t are just unexpected PASSes, 
> probably the test suite lacks proper TODO specification for 2.2.32 here.

Ack.

> Apart from that, your package does not cause any regressions. However, of the 
> new tests for the HTTPProtocolOptions feature, three tests fail. Two of these 
> have to do with NUL-Bytes in the request. 

Right, good catch, I'll take a look at those now.

> I have put the full logs and some scripts and diffs to run the test suite at 
> [1]. 

Thanks!!

> For jessie, I am not that far, yet. So I don't have any hints about the 
> http_strict.t test fails.

Understood, I'll report back what I found.

> In 2.4.25, the changes have been reported to break underscores in hostnames 
> [2]. While these are not RFC-conforming, this is probably not something one 
> should break in a security update. Therefore I would relax the hostname 
> checking to also accept underscores. I think the relevant code is in vhost.c 
> in fix_hostname_non_v6() .

I agree. Lots of domain names break RFC, I was surprised to find in the
past... I'll take a look.

By the way, would it be possible to enable the test suite in the package
build, since we have the code ready to go there anyways? Or in
autopkgtest?

I was wondering how to run the apache2 test suite, which I figured would
necessarily exist, but didn't find it in the package so I gave up...

a.

-- 
We must learn to live together as brothers or perish together as fools.
- Martin Luther King, Jr.



testing and review requested for Wheezy update of apache2

2017-01-17 Thread Antoine Beaupré
On 2016-12-28 15:44:25, Stefan Fritsch wrote:
> Hi Ola,
>
> On Friday, 23 December 2016 23:56:45 CET Ola Lundqvist wrote:
>> the Debian LTS team would like to fix the security issues which are
>> currently open in the Wheezy version of apache2:
>> https://security-tracker.debian.org/tracker/CVE-2016-8743
>> 
>> Would you like to take care of this yourself?
>
> The fix for that is very invasive and may well break some things. I would 
> wait 
> with a backport until the fix has seen more exposure, both upstream and in 
> stretch (the fix will migrate from sid in a few days). 
>
> Also, there is some work upstream to get the changes backported to 2.2 in a 
> separate 2.2.x-merge-http-strict branch [1]. But it has not landed in the 
> 2.2.x branch, yet.
>
> I will share with you any insights I get from backporting the changes to 
> jessie. But it is somewhat unlikely that I will have time to do the backport 
> to wheezy myself.

Hi!

Thanks for the response. We'll be happy to take on the update for
wheezy, and hopefully our work will be useful for Jessie as well...

I started looking at the 2.2.x backport, which has now landed in the
[2.2.32 release][1]. I unfortunately don't think we can use the release
directly, because it is 10 minor releases away from the version
currently in Wheezy (2.2.22), but I'd be glad to be proven wrong... It
may be better to just follow upstream here as well, since their 2.2
branch is designed for LTS support. But they do fix way more stuff than
just security issues in there...

[1]: http://www.apache.org/dist/httpd/Announcement2.2.html

The patch is far from trivial and it's a bit all over the place. I've
gathered it into a single patch from two upstream commits ([r1777405][]
and [r1777999][]), taken from the [2.2.32 tag][].

[r1777405]: http://svn.apache.org/viewvc?view=revision=1777405
[r1777999]: http://svn.apache.org/viewvc?view=revision=1777999
[2.2.32 tag]: http://svn.apache.org/viewvc/httpd/httpd/tags/2.2.32/?view=log

I hammered at the code until the patch actually builds and I tested it
summarily in a wheezy VM, but i'm not sure where or what to test in
there... There's a webserver and "it works!" but that's about it for
now. I still need to review the new patch and compare it with upstream
to see if I really followed the spirit of the patch - so far I just made
sure the chunks apply cleanly...

Attached is a debdiff of the resulting package, which has been uploaded
to the [usual location][].

[usual location]: https://people.debian.org/~anarcat/debian/wheezy-lts/

I would need people to start testing the package at this point, not
necessarily in production considering how big the change is, but your
comfort level will vary with the severity and complexity of services. :)

Pointers on how to reproduce actual security issues would be welcome,
along with code reviews.

I am especially concerned by the proxy changes in the 2.2.x branch we
don't have in our code base: since the vulnerability implies mostly
proxy chains, this could mean we can't fix the issue properly without a
full backport of all those changes..

In fact, I wouldn't feel completely confident in this upload unless we
ship 2.2.32. Apache's codebase really reflects its name ("a patchy
server") so it's quite hard to figure out what exactly is going on in
there. They are also quite liberal in the changes they do - the patch
basically rewrites request handling completely!

Thanks for any feedback,

A.

-- 
Il est sage de nous réconcilier avec notre adolescence ; haїr, mépriser,
nier ou simplement oublier l’adolescent que nous fûmes est en soi une
attitude adolescente.
- Daniel Pennac, Comme un roman
diff -Nru apache2-2.2.22/debian/changelog apache2-2.2.22/debian/changelog
--- apache2-2.2.22/debian/changelog	2016-07-20 01:04:30.0 -0400
+++ apache2-2.2.22/debian/changelog	2017-01-17 10:53:56.0 -0500
@@ -1,3 +1,16 @@
+apache2 (2.2.22-13+deb7u8) UNRELEASED; urgency=high
+
+  * Non-maintainer upload by the LTS Security Team.
+  * Security: CVE-2016-8743:
+Enforce HTTP request grammar corresponding to RFC7230 for request lines
+and request headers, to prevent response splitting and cache pollution by
+malicious clients or downstream proxies.
+  * The stricter HTTP enforcement may cause compatibility problems with
+non-conforming clients. Fine-tuning is possible with the new
+HttpProtocolOptions directive.
+
+ -- Antoine Beaupré <anar...@debian.org>  Tue, 17 Jan 2017 10:53:56 -0500
+
 apache2 (2.2.22-13+deb7u7) wheezy-security; urgency=high
 
   * Non-maintainer upload.
diff -Nru apache2-2.2.22/debian/patches/CVE-2016-8743.patch apache2-2.2.22/debian/patches/CVE-2016-8743.patch
--- apache2-2.2.22/debian/patches/CVE-2016-8743.patch	1969-12-31 19:00:00.0 -0500
+++ apache2-2.2.22/debian/patches/CVE-2016-8743.patch	2017-01-17 10:53:56.0 -0500
@@

Bug#821229: AddCharset config should state more clearly which documentation to read

2016-04-16 Thread Antoine Beaupré
Package: apache2
Version: 2.4.10-10+deb8u4
Severity: wishlist

/etc/apache2/conf-available/charset.conf currently says:

# Read the documentation before enabling AddDefaultCharset.
# In general, it is only a good idea if you know that all your files
# have this encoding. It will override any encoding given in the files
# in meta http-equiv or xml encoding tags.

#AddDefaultCharset UTF-8

# vim: syntax=apache ts=4 sw=4 sts=4 sr noet

It's been a while since I looked at that setting. From memory, I
*think* there's a security issue with enabling this setting, but the
comment does not make that clear at all. Furthermore, it doesn't say
*why* I should read the documentation, or worse, which. I don't have a
README.Debian installed here.

The Apache documentation upstream is pretty large. I could find this:

https://httpd.apache.org/docs/current/mod/core.html#adddefaultcharset

Which links to:

https://httpd.apache.org/docs/current/mod/mod_mime.html#addcharset
http://www.iana.org/assignments/character-sets/character-sets.xhtml

Then that links to:

https://httpd.apache.org/docs/current/mod/mod_mime.html#multipleext
https://httpd.apache.org/docs/current/content-negotiation.html

... when do I stop reading? :) What exactly is the point of the notice?

I would suggest adding a recommendation in the text explicitly stating
that the user should read the issues documented in the
AddDefaultCharset documentation with the URL, that way it's clear that
the user does not need to get familiar with all the details of content
negotiation and IANA numbering. :p

It's also unclear to me why there's a config in conf-enabled that does
nothing by default. It would seem to me more rational to have the
config disabled by default, but then have AddDefaultCharset
actually activated in there...

Thanks!

-- System Information:
Debian Release: 8.4
  APT prefers stable-updates
  APT policy: (500, 'stable-updates'), (500, 'proposed-updates'), (500, 
'stable'), (1, 'testing')
Architecture: amd64 (x86_64)

Kernel: Linux 4.2.0-0.bpo.1-amd64 (SMP w/2 CPU cores)
Locale: LANG=fr_CA.UTF-8, LC_CTYPE=fr_CA.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash
Init: systemd (via /run/systemd/system)

Versions of packages apache2 depends on:
pn  apache2-mpm-worker | apache2-mpm-prefork | apache2-mpm-event | apac  
pn  apache2.2-common 

apache2 recommends no packages.

apache2 suggests no packages.



Bug#771679: apache2-mpm-itk: fails to restart properly on some occasions

2014-12-01 Thread Antoine Beaupré
Package: apache2-mpm-itk
Version: 2.2.22-13+deb7u3
Severity: important

Since we have switched our Apache2 + mod_php web cluster to mod-itk,
we have had intermittent problems starting the webservers.

We have an automated system (AlternC.org) that frequently reloads the
apache webservers on different web nodes and sometimes it just fails.

It restarts the servers with the following command:

ssh alternc@$(basename $0) sudo invoke-rc.d apache2 reload

Sometimes the webserver is left in an inconsistent state. We've had
situations where two trees of processes would be present, or where the
parent process would be gone. We have yet to investigate more fully
what exactly is happening, but the webserver basically becomes
unresponsive and gets kicked out of the load balancer.

The workaround we have found so far is to stop apache, kill all apache
processes then restart it:

service apache2 stop
killall -9 apache2
service apache2 start

It seems to me there should be provisions in the init.d for that kind
of situations and a reload should never leave apache in such a weird
state.

We'll try to provide more details later, but so far we associate this
with the mod_itk switch, because we didn't have those problems before.

A.

-- Package-specific info:

List of /etc/apache2/mods-enabled/*.load:
  alias auth_basic authn_file authnz_ldap authz_default
  authz_groupfile authz_host authz_user autoindex cgi deflate dir env
  expires headers include info ldap mime negotiation php5 proxy
  proxy_http reqtimeout rewrite setenvif ssl status

List of enabled php5 extensions:

 apc curl gd imap intl ldap mcrypt mysql mysqli pdo pdo_mysql
 pdo_pgsql pdo_sqlite pgsql pspell sqlite3 tidy xsl yaz

-- System Information:
Debian Release: 7.6
  APT prefers stable
  APT policy: (500, 'stable'), (500, 'oldstable')
  Architecture: i386 (x86_64)

Kernel: Linux 3.2.0-4-amd64 (SMP w/8 CPU cores)
Locale: LANG=fr_CA.UTF-8, LC_CTYPE=fr_CA.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/bash

Versions of packages apache2-mpm-itk depends on:
ii  apache2.2-bin 2.2.22-13+deb7u3
ii  apache2.2-common  2.2.22-13+deb7u3

apache2-mpm-itk recommends no packages.

apache2-mpm-itk suggests no packages.

-- no debconf information


-- 
To UNSUBSCRIBE, email to debian-apache-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: 
https://lists.debian.org/20141201161243.29528.25102.report...@marcos.anarc.at



Bug#759382: do not keep so much logs

2014-09-22 Thread Antoine Beaupré
On 2014-09-22 05:29:10, Vincent Lefevre wrote:
 On 2014-08-26 14:28:48 -0700, Antoine Beaupré wrote:
 Apache, at least in Wheezy, seems to be configured by default to keep 52
 log files, rotated on a weekly basis, meaning that logs are kept for a
 year.
 
 This is a long time to keep longs. It exposes our users unduly to
 surveillance and privacy breaches.

 Not your users, but people who connect to the web server. But the
 French law requires (required?) / advises to keep the logs for one
 year. There's a discussion in French here:

   http://forum.ovh.com/archive/index.php/t-47594.html

 Basically this is needed when:
   * Users can create contents.
   * In case of security breach, when someone can do bad things
 via Apache only.

Ouzbekistan law may also require providers to send their logs directly
to the state and install backdoors into their servers, are we going to
do that for all of Debian by default?

It is the provider's responsability to comply with the local laws, not
Debian's, as it is impossible to make a configuration that will work
with every local law.

 It also means a lot of data to keep on disk for busy webservers. For any
 moderately to high traffic webserver, this can actually fill up /var
 pretty fast. For example, a server with an average of 12 hits per
 second:
 
 http://stats.koumbit.net/koumbit.net/ceres.koumbit.net/apache_accesses.html
 
 ... accumulates around 30MB *per day*. That means 11GB per year.

 Everyone says that disk space is cheap.

I don't. Do you?

 So, this is a very poor argument. Moreover old logs are compressed, so
 that it isn't 11GB per year, but much smaller. With gzip compression
 (which is not very good), I get more than a 10x compression. So, in
 practice, 30 MB per day would mean around 1 GB of disk space on the
 previous default of one year, possibly less.

Those logs were compressed. I know about gzip compression, thank you,
and it is actually pretty decent for log files.

 I suspect the default partitionning would not allocate enough space
 for /var at all on most systems to cover for that.

 By default, the Debian installer creates a single partition (unless
 this has changed recently).

Ah. I am surprised by that, but I would assume that people would create
a separate /var on server installs. In our experience, we've had
webservers run out of space on /var a few times.

 I would suggest following the policies set for /var/log/syslog, which
 are rotate daily and keey 7 days.

 Not everyone has such a busy webserver.

Not everyone lives in a country that forces their providers to spy on
their users. Yet anyone can be a victim of massive visits on their
website (aka slashdotting) which will basically fill up the drives,
regardless of the country they live in.

(Arguably, slashdottings themselves are a difficult problem to deal
with and may occur only within a day so log rotation will not help much,
but daily log rotation will certainly be better than weekly to deal with
this.)

We don't want to implement policies that make it difficult to run a
popular webserver - it's the whole point of those things anyways.

 IMHO, the default log rotation should be changed back to 1 year,
 at least to protect users in case of legal matters. Alternatively,
 size-based log rotation could be used, e.g. with:

 rotate 15
 size 100M

I think keeping logs does not protect users, it actually exposes them to
undue surveillance. When speaking of users here, I refer also to the
visitors of the website, which never agreed to install debian, choose
how much logs are kept and so on. We have a responsability towards those
as well. I prefer to refer to operators or admins for people that
have the power to do the above configuration.

Operators that want to comply with legal legislation should get a lawyer
and review their storage and compliance policy accordingly. Just keeping
52 weeks of logs is hardly legal compliance, even in france you need
to do more than this to comply with the authorities (for example there
are specific delays by which you need to answer).

Telling our operators that they can just install debian by default and
assume legal compliance worldwide is actually hurting them and exposing
them to undue legal complications.

Also, the above configuration, on small sites, could even mean keeping
logs even longer than the original configuration. On big sites, it will
not respect the legal requirements.

I would have proposed to keep no logs at all if I didn't know this would
have created a huge backlash. Furthermore, I think it makes sense to
keep a minimal amount of logs to help people understand what's happening
on a daily basis, so for me 14 days is a compromise. We'll still deploy
a 5 day retention policy at Koumbit, and I know of a lot of groups that
simply censor IPs in the logs altogether, with no ill legal effect or
managmeent problems.

Please keep the change in. It is a good compromise.

A.

-- 
A man is none the less a slave because he

Bug#759382: do not keep so much logs

2014-09-22 Thread Antoine Beaupré
On 2014-09-22 10:14:34, Vincent Lefevre wrote:
 On 2014-09-22 09:23:11 -0400, Antoine Beaupré wrote:
 On 2014-09-22 05:29:10, Vincent Lefevre wrote:
  Not your users, but people who connect to the web server. But the
  French law requires (required?) / advises to keep the logs for one
  year. There's a discussion in French here:
 
http://forum.ovh.com/archive/index.php/t-47594.html
 
  Basically this is needed when:
* Users can create contents.
* In case of security breach, when someone can do bad things
  via Apache only.
 
 Ouzbekistan law may also require providers to send their logs directly
 to the state and install backdoors into their servers, are we going to
 do that for all of Debian by default?

 I don't care about Ouzbekistan. In most countries, users are
 responsible for what their servers do, and keeping logs is a
 way to protect them.

I care about Ouzbekistan the same way I care about France.

 Note also that Debian cares about local laws. Otherwise there
 would be no problems with patented algorithms.

Because the servers hosting the source code and binaries, not because of
installed systems so much.

  Everyone says that disk space is cheap.
 
 I don't. Do you?

 Debian devs do.

I'm a debian dev.

 Not everyone lives in a country that forces their providers to spy on
 their users.

 Please could you avoid saying stupid things?

No, as they are not stupid. I would prefer it if you would refrain from
qualifying what I consider to be reasonable statements as stupid. That
you disagree doesn't make them stupid.

I do believe that the european logging directives, for example, are a
way to force providers to spy on their users on the behalf of the
state. Other countries do not have such requirements and still have
other legal means of getting to the data they need for criminal
prosecution. Forcing providers to keep logs is a way to force
deanonymisation of our users on the network, and is a fundamental issue
with freedom of speech and association.

 Yet anyone can be a victim of massive visits on their website (aka
 slashdotting) which will basically fill up the drives, regardless
 of the country they live in.

 In such a case, size based rules would be better than date based ones.

It's not a one-sided issues, there are multiple arguments for reducing
the logs we keep, one of them is surveillance, another is disk usage.

  IMHO, the default log rotation should be changed back to 1 year,
  at least to protect users in case of legal matters. Alternatively,
  size-based log rotation could be used, e.g. with:
 
  rotate 15
  size 100M
 
 I think keeping logs does not protect users,

 By users, I meant here the responsible of web servers.

 it actually exposes them to undue surveillance. When speaking of
 users here, I refer also to the visitors of the website, which
 never agreed to install debian, choose how much logs are kept and so
 on. We have a responsability towards those as well.

 Wow! Most web servers keep logs for a long time by choice. Visitors
 who do not agree with that should not use the web.

Webservers that want to choose to keep logs for a long time can do
so. Admins that do not agree can change the policies, visitors cannot.

 Also, the above configuration, on small sites, could even mean keeping
 logs even longer than the original configuration.

 Not a real problem.

It's actually exactly the problem that was raised in this bug report in
the first place, and it's a real problem.

 On big sites, it will not respect the legal requirements.

 Admins of big sites will probably have a closer look at the config
 anyway.

And we do. But by bringing our (legal) experience to a larger audience,
we hope to share our expertise and hard-learned lessons with everyone.

Besides, the change was done in the package, and unless you are one of
the maintainers, I am not the one you need to argue with, and I do not
need to convince you this is the right way.

A.
-- 
Lorsque l'on range des objets dans des tiroirs, et que l'on a plus
d'objets que de tiroirs, alors un tiroir au moins contient deux
objets.
- Lejeune-Dirichlet, Peter Gustav


pgp6KKdFYF9ZR.pgp
Description: PGP signature


Bug#759382: do not keep so much logs

2014-09-22 Thread Antoine Beaupré
On 2014-09-22 10:52:48, Vincent Lefevre wrote:
 I don't know where you live, but this is the same in most countries,
 except that the period varies.

Where I live is irrelevant. It is not the same in all countries: some
have more or less strict restrictions, some don't have any at all.

The United States of America, for example, do not enforce logging.

   Everyone says that disk space is cheap.
  
  I don't. Do you?
 
  Debian devs do.
 
 I'm a debian dev.

 You may be in the minority.

I may be.

It could also be because I am involved in buying the hardware at work.

  Not everyone lives in a country that forces their providers to spy on
  their users.
 
  Please could you avoid saying stupid things?
 
 No, as they are not stupid. I would prefer it if you would refrain from
 qualifying what I consider to be reasonable statements as stupid. That
 you disagree doesn't make them stupid.

 What you say is a lie. France does not force users to spy on other
 users.

I disagree. I think that forcing logging is forcing webserver operators
to surveil their users, in the ultimate goal of revealing their
activities to the authorities, and therefore spying.

Saying that this is a lie implies that I am deliberately construing
another reality and masking information I would know. That is incorrect:
we are in a disagreement about the purpose of those policies. You think
it's good, I think it's bad. It doesn't make me stupid or a liar.

 I do believe that the european logging directives, for example, are a
 way to force providers to spy on their users on the behalf of the
 state. Other countries do not have such requirements and still have
 other legal means of getting to the data they need for criminal
 prosecution. Forcing providers to keep logs is a way to force
 deanonymisation of our users on the network, and is a fundamental issue
 with freedom of speech and association.

 When someone connects to my web server, this is not my user.
 This is someone (human or not) I don't know.

I disagree. I think the client of a webserver is a user of a webserver.

Aren't people visiting (say) github.com users of github? Isn't there a
terms of service policy at the bottom of every freaking website these
days that they assume you have read and that implies you are a user of
their services?

  Wow! Most web servers keep logs for a long time by choice. Visitors
  who do not agree with that should not use the web.
 
 Webservers that want to choose to keep logs for a long time can do
 so.

 And webservers that want to choose to keep logs for a short time
 can do so. So, there was no reason to change the default period.

I guess there was a compelling reason enough so that the default was
changed. I have given numerous reasons why globally, the default logging
should be reduced (resource usage, privacy, etc). You have given a
single reason why, locally (namely in France), the default should be 52
weeks, and haven't adressed the question as to what to do with
variations in those policies outside of France.

I do not see why we should keep 52 weeks of logs to satisfy the legal
requirements of just one country in the world, especially since that
change isn't sufficient to ensure compliance, and may break compliance
in other countries.

Anyways, I do not need to be CC'd further in your communications here, I
am not the maintainer of this package, I just reported this bug and I'm
not the one you need to convince.

A.
-- 
We have no friends but the mountains.
- Kurdish saying


pgpG1tE8DxyOz.pgp
Description: PGP signature


Bug#759382: do not keep so much logs

2014-09-22 Thread Antoine Beaupré
On 2014-09-22 10:52:48, Vincent Lefevre wrote:
 On 2014-09-22 10:23:05 -0400, Antoine Beaupré wrote:
 On 2014-09-22 10:14:34, Vincent Lefevre wrote:
  On 2014-09-22 09:23:11 -0400, Antoine Beaupré wrote:
  On 2014-09-22 05:29:10, Vincent Lefevre wrote:
   Not your users, but people who connect to the web server. But the
   French law requires (required?) / advises to keep the logs for one
   year. There's a discussion in French here:
  
 http://forum.ovh.com/archive/index.php/t-47594.html
  
   Basically this is needed when:
 * Users can create contents.
 * In case of security breach, when someone can do bad things
   via Apache only.
  
  Ouzbekistan law may also require providers to send their logs directly
  to the state and install backdoors into their servers, are we going to
  do that for all of Debian by default?
 
  I don't care about Ouzbekistan. In most countries, users are
  responsible for what their servers do, and keeping logs is a
  way to protect them.
 
 I care about Ouzbekistan the same way I care about France.

 I don't know where you live, but this is the same in most countries,
 except that the period varies.

After a little more research, here's an overview of the national data
retention policies in Europe:

http://wiki.vorratsdatenspeicherung.de/Overview_of_national_data_retention_policies

Data retention seems to have been ruled out as inconstitutional in
Germany, to show an example of how important it could be to keep minimal
logs.

According to people well versed in those legal intricaties, there are
currently no restrictions on web server logging. Most of the data
retention directives apply to ISPs (Internet Service Providers) and
require that organisations providing connectivity to the internet need
to keep track of which IP belongs to which customer. There are also
restrictions for phone service providers.

As far as I know, there are no legal requirements for web hosting
providers to keep logs in the european directive. I would be curious to
hear on which basis you claim that french law requires hosting providers
to keep logs at all. The forum post you refer to is vague at best, and
doesn't even seem to be an official position of OVH. It also mentions
that keeping *more* logs is illegal and that most accounts hosted on OVH
probably don't respect the law.

So please provide more references to back up your laim that most
countries need data retention if you want to make a proper point here.

Thanks,

A.

-- 
Man really attains the state of complete humanity when he produces,
without being forced by physical need to sell himself as a commodity.
- Ernesto Che Guevara


pgpggKuLV6wYm.pgp
Description: PGP signature


Bug#759382: do not keep so much logs

2014-08-26 Thread Antoine Beaupré
Package: apache2
Severity: wishlist

Apache, at least in Wheezy, seems to be configured by default to keep 52
log files, rotated on a weekly basis, meaning that logs are kept for a
year.

This is a long time to keep longs. It exposes our users unduly to
surveillance and privacy breaches.

It also means a lot of data to keep on disk for busy webservers. For any
moderately to high traffic webserver, this can actually fill up /var
pretty fast. For example, a server with an average of 12 hits per
second:

http://stats.koumbit.net/koumbit.net/ceres.koumbit.net/apache_accesses.html

... accumulates around 30MB *per day*. That means 11GB per year. I
suspect the default partitionning would not allocate enough space for
/var at all on most systems to cover for that.

I would suggest following the policies set for /var/log/syslog, which
are rotate daily and keey 7 days.

-- System Information:
Debian Release: 7.6
  APT prefers stable
  APT policy: (500, 'stable')
Architecture: amd64 (x86_64)

Kernel: Linux 3.2.0-4-amd64 (SMP w/2 CPU cores)
Locale: LANG=fr_CA.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8)
Shell: /bin/sh linked to /bin/dash

Versions of packages apache2 depends on:
pn  apache2-mpm-worker | apache2-mpm-prefork | apache2-mpm-event | apac  none
pn  apache2.2-common none

apache2 recommends no packages.

apache2 suggests no packages.


-- 
To UNSUBSCRIBE, email to debian-apache-requ...@lists.debian.org
with a subject of unsubscribe. Trouble? Contact listmas...@lists.debian.org
Archive: 
https://lists.debian.org/20140826212848.9716.81869.report...@angela.anarc.at