po
Hi, I've been load-testing our module (mod_pagespeedhttp://code.google.com/speed/page-speed/docs/module.html) with httpd 2.2.16 built with these options: --enable-pool-debug --with-mpm=worker I've been getting periodic aborts from apr_table_addn that don't look like they are from my module. These do not occur when using 'prefork'. Here's a stack-trace recovered from a core file: Program terminated with signal 6, Aborted. #0 0x7fdd3bbd9a75 in raise (sig=value optimized out) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 64 ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory. in ../nptl/sysdeps/unix/sysv/linux/raise.c (gdb) where #0 0x7fdd3bbd9a75 in raise (sig=value optimized out) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #1 0x7fdd3bbdd5c0 in abort () at abort.c:92 #2 0x7fdd3c9a2e57 in apr_table_addn (t=0xe84980, key=0xd51d60 Accept-Encoding, val=0xd51d71 identity) at tables/apr_tables.c:813 #3 0x00433e36 in ap_get_mime_headers_core (r=0xf27de0, bb=0xdda3a0) at protocol.c:799 #4 0x0043456b in ap_read_request (conn=0xe51620) at protocol.c:918 #5 0x0047f772 in ap_process_http_connection (c=0xe51620) at http_core.c:183 #6 0x00446e28 in ap_run_process_connection (c=0xe51620) at connection.c:43 #7 0x004b3297 in process_socket (thd=value optimized out, dummy=value optimized out) at worker.c:544 #8 worker_thread (thd=value optimized out, dummy=value optimized out) at worker.c:894 #9 0x7fdd3c1339ca in start_thread (arg=value optimized out) at pthread_create.c:300 #10 0x7fdd3bc8c70d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 Questions 1. Is this a bug in httpd? 2. Or could I somehow have caused this with a programming error in my module? 3. Or is enable-pool-debug simply incompatible with the Worker MPM? Thanks, -Josh
Re: po
On Thu, Sep 1, 2011 at 13:52, Joshua Marantz jmara...@google.com wrote: Hi, I've been load-testing our module (mod_pagespeedhttp://code.google.com/speed/page-speed/docs/module.html) with httpd 2.2.16 built with these options: --enable-pool-debug --with-mpm=worker I've been getting periodic aborts from apr_table_addn that don't look like they are from my module. These do not occur when using 'prefork'. Here's a stack-trace recovered from a core file: Program terminated with signal 6, Aborted. #0 0x7fdd3bbd9a75 in raise (sig=value optimized out) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 64 ../nptl/sysdeps/unix/sysv/linux/raise.c: No such file or directory. in ../nptl/sysdeps/unix/sysv/linux/raise.c (gdb) where #0 0x7fdd3bbd9a75 in raise (sig=value optimized out) at ../nptl/sysdeps/unix/sysv/linux/raise.c:64 #1 0x7fdd3bbdd5c0 in abort () at abort.c:92 #2 0x7fdd3c9a2e57 in apr_table_addn (t=0xe84980, key=0xd51d60 Accept-Encoding, val=0xd51d71 identity) at tables/apr_tables.c:813 #3 0x00433e36 in ap_get_mime_headers_core (r=0xf27de0, bb=0xdda3a0) at protocol.c:799 #4 0x0043456b in ap_read_request (conn=0xe51620) at protocol.c:918 #5 0x0047f772 in ap_process_http_connection (c=0xe51620) at http_core.c:183 #6 0x00446e28 in ap_run_process_connection (c=0xe51620) at connection.c:43 #7 0x004b3297 in process_socket (thd=value optimized out, dummy=value optimized out) at worker.c:544 #8 worker_thread (thd=value optimized out, dummy=value optimized out) at worker.c:894 #9 0x7fdd3c1339ca in start_thread (arg=value optimized out) at pthread_create.c:300 #10 0x7fdd3bc8c70d in clone () at ../sysdeps/unix/sysv/linux/x86_64/clone.S:112 Questions 1. Is this a bug in httpd? Probably not. 2. Or could I somehow have caused this with a programming error in my module? That seems more likely. 3. Or is enable-pool-debug simply incompatible with the Worker MPM? Not that I know. That assertion is triggered when you add a string from pool A to a table in pool B where A is a child of B (adding headers from the request_rec to a conn_rec table, for example). It's a lifecycle issue.
Detecting which MPM a module is running in
Hello from mod_pagespeed again. We are adding support for running in the Worker MPM, having spent most of our time since we launched the product sheltered in the prefork MPM where our multi-threading challenges are all of our own making. Having tuned our threading model for prefork, where all request-parallelism will come at the process level, we now offer a functional but, almost certainly, suboptimal experience for Worker MPM users. In particular, for a given server_rec, in prefork we are only going to be processing one request at a time, hence we should employ only a small number of worker threads to help us. But in Worker MPM, from what I've seen, a single server_rec can service multiple concurrent requests. Thus I think we should be aware of this and bump up the number of worker threads we create to help us. Is it possible to detect the MPM under which mod_pagespeed is running, or, more generally, the number of concurrent requests that the current process is likely to need to handle? Thanks! -Josh
RE: non-splittable buckets (was: Regression with range fix)
-Original Message- From: Stefan Fritsch [mailto:s...@sfritsch.de] Sent: Mittwoch, 31. August 2011 23:09 To: dev@httpd.apache.org Subject: non-splittable buckets (was: Regression with range fix) On Wednesday 31 August 2011, Jim Jagielski wrote: Looking at the patch in 2.2.x; there is a lot of effort expended deadling with apr_bucket_split() returning ENOTIMPL - that looks unnecessary; the filter will only handle brigades containing buckets with known length, and all such buckets must be _split-able. So you think we can rip out the whole if (rv == APR_ENOTIMPL) blocks? Belt and suspenders? If we rip it out, we should replace it with ap_assert()s. And maybe only do it in 2.3? I guess asserts are fine. IMHO we should do it in 2.3 and 2.2 if we do it. Nothing is different between 2.2 and 2.3 in this respect and we keep the diff between 2.2 and 2.3 small. From what I can see Joe is correct with his assumption that this case cannot happen there. A different idea I had: If apr_bucket_split() returns ENOTIMPL, we could call apr_brigade_partition on the copied brigade. apr_brigade_partition() would then do all the complex handling of apr_bucket_read(), etc. It is only slightly less efficient because it has to traverse the brigade again. But the memory usage would still stay low because the original brigade would remain untouched. And we would get rid of much of the complexity in copy_brigade_range(). Would be an approach, but I would like to see Joe's feedback on this as I don't see value in reconstructing this part if ripping out is fine as well. Regards Rüdiger
RE: Appropriate patches for 2.2.19 and 2.0.64?
-Original Message- From: William A. Rowe Jr. [mailto:wr...@rowe-clan.net] Sent: Donnerstag, 1. September 2011 03:51 To: dev@httpd.apache.org Subject: Re: Appropriate patches for 2.2.19 and 2.0.64? On 8/31/2011 4:16 PM, William A. Rowe Jr. wrote: I've attempted to simply substitute the 2.2.19 filter code into the 2.0.64 http_protocol.c sources, and am unsure how far off these patches are from what they need to be; there's been a significant amount of drift and refactoring in the interim. Still looking for feedback, but the attached applies and corresponds to 2.2.20 with the exception of atoi rather than strtoi semantics, and without the no DefaultType exception.. Thanks for taking care. It applies cleanly, but most byterange tests from the framework fail: Failed Test Stat Wstat Total Fail Failed List of Failed --- t/apache/byterange.t 136 97 71.32% 2-9 11 16-17 19-20 25-26 28 33-35 37-39 42 44-47 49-61 63-68 70-73 77-79 83-88 90-96 98-99 101-103 105-113 115-123 126-127 130 132-136 t/apache/byterange5.t54 80.00% 1 3-5 t/apache/byterange7.t 135 38.46% 2-5 12 This might be because I use a 64 bit build of 2.0.x (no other test environment at hand at the moment) and 2.0.x is not really good at 64 bit (lots of compiler warnings). Regards Rüdiger
Re: svn commit: r1163833 - /httpd/httpd/trunk/modules/http/byterange_filter.c
On Wed, Aug 31, 2011 at 6:28 PM, Roy T. Fielding wrote: On Aug 31, 2011, at 6:10 PM, William A. Rowe Jr. wrote: The presumption here is that the client requests bytes=0- to begin the transmission, and provided it sees a 206, restarting somewhere in the stream results in aborting the connection and streaming bytes=n- from the restart point. Further testing should determine if this was the broken assumption. Do we send the Accept-Ranges header field? http://tools.ietf.org/html/rfc2616#page-105 Apache httpd 2.2.9 is sending this header in the Debian bug report at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=639825 Tim
Re: Appropriate patches for 2.2.19 and 2.0.64?
Is there anyone has tested the 2.2.19 with this patch? 2011/9/1 Plüm, Rüdiger, VF-Group ruediger.pl...@vodafone.com -Original Message- From: William A. Rowe Jr. [mailto:wr...@rowe-clan.net] Sent: Donnerstag, 1. September 2011 03:51 To: dev@httpd.apache.org Subject: Re: Appropriate patches for 2.2.19 and 2.0.64? On 8/31/2011 4:16 PM, William A. Rowe Jr. wrote: I've attempted to simply substitute the 2.2.19 filter code into the 2.0.64 http_protocol.c sources, and am unsure how far off these patches are from what they need to be; there's been a significant amount of drift and refactoring in the interim. Still looking for feedback, but the attached applies and corresponds to 2.2.20 with the exception of atoi rather than strtoi semantics, and without the no DefaultType exception.. Thanks for taking care. It applies cleanly, but most byterange tests from the framework fail: Failed Test Stat Wstat Total Fail Failed List of Failed --- t/apache/byterange.t 136 97 71.32% 2-9 11 16-17 19-20 25-26 28 33-35 37-39 42 44-47 49-61 63-68 70-73 77-79 83-88 90-96 98-99 101-103 105-113 115-123 126-127 130 132-136 t/apache/byterange5.t54 80.00% 1 3-5 t/apache/byterange7.t 135 38.46% 2-5 12 This might be because I use a 64 bit build of 2.0.x (no other test environment at hand at the moment) and 2.0.x is not really good at 64 bit (lots of compiler warnings). Regards Rüdiger
Another regression regarding byteranges
PR 51748 (https://issues.apache.org/bugzilla/show_bug.cgi?id=51748) is an IMHO valid regression in range behaviour (from the report): Request and response sample in each versions. = version 2.2.20 GET / HTTP/1.1 Host: localhost Range: bytes=-1 HTTP/1.1 206 Partial Content Server: Apache/2.2.20 (Unix) Accept-Ranges: bytes Content-Length: 2 Content-Range: bytes 0-1/10240 Content-Type: text/html = version 2.2.19 GET / HTTP/1.1 Host: localhost Range: bytes=-1 HTTP/1.1 206 Partial Content Server: Apache/2.2.19 (Unix) Accept-Ranges: bytes Content-Length: 1 Content-Range: bytes 10239-10239/10240 Content-Type: text/html = I already fixed that in trunk. I think this regression justifies another release for 2.2.x. But IMHO we should wait at least until mid next week to see if other regressions come thru and hit them all with a 2.2.21. Regards Rüdiger
Re: Next update
On Wed, Aug 31, 2011 at 9:03 PM, Dirk-WIllem van Gulik di...@webweaving.org wrote: Suggestion for http://people.apache.org/~dirkx/CVE-2011-3192.txt You probably mean deprecated not desecrated, amusing though that is.
Re: Next update
On 1 Sep 2011, at 12:06, Ben Laurie wrote: On Wed, Aug 31, 2011 at 9:03 PM, Dirk-WIllem van Gulik di...@webweaving.org wrote: Suggestion for http://people.apache.org/~dirkx/CVE-2011-3192.txt You probably mean deprecated not desecrated, amusing though that is. Darn Functional MRI - them spell checkers are getting scarily good at reading my mind ! Thanks Ben, Dw.
Re: Another regression regarding byteranges
On Sep 1, 2011, at 6:31 AM, Plüm, Rüdiger, VF-Group wrote: I already fixed that in trunk. I think this regression justifies another release for 2.2.x. But IMHO we should wait at least until mid next week to see if other regressions come thru and hit them all with a 2.2.21. +1
CVE-2003-1418 - still affects apache 2 current
Hi, CVE-2003-1418, a minor security issue, is still affecting the current codebase. someone opened a tracker bug a year ago without feedback: https://issues.apache.org/bugzilla/show_bug.cgi?id=49623 Do you have a statement? The Qualys security scanner detects and reports this issue and continues to confuse Apache users :/ The OpenBSD patch referenced adds some hashing and backing store for the etags. Ciao, Marcus
Re: non-splittable buckets (was: Regression with range fix)
On Wed, Aug 31, 2011 at 11:08:51PM +0200, Stefan Fritsch wrote: On Wednesday 31 August 2011, Jim Jagielski wrote: Looking at the patch in 2.2.x; there is a lot of effort expended deadling with apr_bucket_split() returning ENOTIMPL - that looks unnecessary; the filter will only handle brigades containing buckets with known length, and all such buckets must be _split-able. So you think we can rip out the whole if (rv == APR_ENOTIMPL) blocks? Yes. This code could only get exercised with a custom bucket type which has fixed-length buckets but no -split implementation. Belt and suspenders? If we rip it out, we should replace it with ap_assert()s. And maybe only do it in 2.3? It would seem odd to have ENOTIMPL as a fatal error but other real errors non-fatal. *No* error should occur here with unless somebody has cooked up a really whacky bucket type. Regards, Joe
RE: non-splittable buckets (was: Regression with range fix)
-Original Message- From: Joe Orton [mailto:jor...@redhat.com] Sent: Donnerstag, 1. September 2011 14:39 To: dev@httpd.apache.org Subject: Re: non-splittable buckets (was: Regression with range fix) On Wed, Aug 31, 2011 at 11:08:51PM +0200, Stefan Fritsch wrote: On Wednesday 31 August 2011, Jim Jagielski wrote: Looking at the patch in 2.2.x; there is a lot of effort expended deadling with apr_bucket_split() returning ENOTIMPL - that looks unnecessary; the filter will only handle brigades containing buckets with known length, and all such buckets must be _split-able. So you think we can rip out the whole if (rv == APR_ENOTIMPL) blocks? Yes. This code could only get exercised with a custom bucket type which has fixed-length buckets but no -split implementation. Belt and suspenders? If we rip it out, we should replace it with ap_assert()s. And maybe only do it in 2.3? It would seem odd to have ENOTIMPL as a fatal error but other real errors non-fatal. *No* error should occur here with unless somebody has cooked up a really whacky bucket type. So you would follow the rip-out-and-replace-with-asserts approach? Regards Rüdiger
Re: Another regression regarding byteranges
On 1 Sep 2011, at 13:33, Jim Jagielski wrote: On Sep 1, 2011, at 6:31 AM, Plüm, Rüdiger, VF-Group wrote: I already fixed that in trunk. I think this regression justifies another release for 2.2.x. But IMHO we should wait at least until mid next week to see if other regressions come thru and hit them all with a 2.2.21. +1 Ok - so this makes it sound we really should get the advisory out. Shall I update it with some caveats and stay tuned - but still make it FINAL ? Or should we make this an update - and not declare final victory ? Dw.
Re: non-splittable buckets (was: Regression with range fix)
On Thu, Sep 01, 2011 at 02:47:19PM +0200, Plüm, Rüdiger, VF-Group wrote:
If we rip it out, we should replace it with ap_assert()s. And maybe
only do it in 2.3?
It would seem odd to have ENOTIMPL as a fatal error but other
real errors non-fatal. *No* error should occur here with unless
somebody has cooked up a really whacky bucket type.
So you would follow the rip-out-and-replace-with-asserts approach?
No, just remove the special case for ENOTIMPL and don't change anything
else.
Index: modules/http/byterange_filter.c
===
--- modules/http/byterange_filter.c (revision 1163995)
+++ modules/http/byterange_filter.c (working copy)
@@ -94,8 +94,6 @@
apr_bucket *first = NULL, *last = NULL, *out_first = NULL, *e;
apr_uint64_t pos = 0, off_first = 0, off_last = 0;
apr_status_t rv;
-const char *s;
-apr_size_t len;
apr_uint64_t start64, end64;
apr_off_t pofft = 0;
@@ -147,44 +145,10 @@
if (e == first) {
if (off_first != start64) {
rv = apr_bucket_split(copy, (apr_size_t)(start64 - off_first));
-if (rv == APR_ENOTIMPL) {
-rv = apr_bucket_read(copy, s, len, APR_BLOCK_READ);
-if (rv != APR_SUCCESS) {
-apr_brigade_cleanup(bbout);
-return rv;
-}
-/*
- * The read above might have morphed copy in a bucket
- * of shorter length. So read and delete until we reached
- * the correct bucket for splitting.
- */
-while (start64 - off_first (apr_uint64_t)copy-length) {
-apr_bucket *tmp = APR_BUCKET_NEXT(copy);
-off_first += (apr_uint64_t)copy-length;
-APR_BUCKET_REMOVE(copy);
-apr_bucket_destroy(copy);
-copy = tmp;
-rv = apr_bucket_read(copy, s, len, APR_BLOCK_READ);
-if (rv != APR_SUCCESS) {
-apr_brigade_cleanup(bbout);
-return rv;
-}
-}
-if (start64 off_first) {
-rv = apr_bucket_split(copy, (apr_size_t)(start64 -
off_first));
-if (rv != APR_SUCCESS) {
-apr_brigade_cleanup(bbout);
-return rv;
-}
-}
-else {
-copy = APR_BUCKET_PREV(copy);
-}
+if (rv != APR_SUCCESS) {
+apr_brigade_cleanup(bbout);
+return rv;
}
-else if (rv != APR_SUCCESS) {
-apr_brigade_cleanup(bbout);
-return rv;
-}
out_first = APR_BUCKET_NEXT(copy);
APR_BUCKET_REMOVE(copy);
apr_bucket_destroy(copy);
@@ -200,38 +164,10 @@
}
if (end64 - off_last != (apr_uint64_t)e-length) {
rv = apr_bucket_split(copy, (apr_size_t)(end64 + 1 -
off_last));
-if (rv == APR_ENOTIMPL) {
-rv = apr_bucket_read(copy, s, len, APR_BLOCK_READ);
-if (rv != APR_SUCCESS) {
-apr_brigade_cleanup(bbout);
-return rv;
-}
-/*
- * The read above might have morphed copy in a bucket
- * of shorter length. So read until we reached
- * the correct bucket for splitting.
- */
-while (end64 + 1 - off_last (apr_uint64_t)copy-length) {
-off_last += (apr_uint64_t)copy-length;
-copy = APR_BUCKET_NEXT(copy);
-rv = apr_bucket_read(copy, s, len, APR_BLOCK_READ);
-if (rv != APR_SUCCESS) {
-apr_brigade_cleanup(bbout);
-return rv;
-}
-}
-if (end64 off_last + (apr_uint64_t)copy-length - 1) {
-rv = apr_bucket_split(copy, (apr_size_t)(end64 + 1 -
off_last));
-if (rv != APR_SUCCESS) {
-apr_brigade_cleanup(bbout);
-return rv;
-}
-}
+if (rv != APR_SUCCESS) {
+apr_brigade_cleanup(bbout);
+return rv;
}
-else if (rv != APR_SUCCESS) {
-apr_brigade_cleanup(bbout);
-
Re: po
Hi Ben, Hmmm...don't know what happened to that subject line po. Not what I meant to type, obviously! On Thu, Sep 1, 2011 at 8:14 AM, Ben Noordhuis i...@bnoordhuis.nl wrote: That assertion is triggered when you add a string from pool A to a table in pool B where A is a child of B (adding headers from the request_rec to a conn_rec table, for example). It's a lifecycle issue. OK given the stack-trace above it's hard for me to figure out a path back from my module. Of course a random memory stomp could make any weird thing happen, but only this one specific thing always happens. And we try to avoid memory-stomps via valgrind etc though it's impossible to prove that none exist under load. But I think I've found 2 potential problems in httpd: 1. protocol.c calls apr_table_addn using a char* value that is offset from a pool-allocated string. It's not obvious that it's doing this safely but it makes sense this code has to run crazy fast and has lots of mileage on it. And it's probably OK for the pool-find algorithm appears to do a pointer-range check in apr_pools.c:1908, function pool_find(). For the moment I'll assume this is not the actual problem. 2. there are a bunch of ifdefs in apr_pools.c for POOL_DEBUG and there's a some mutexing for APR_HAS_THREADS, e.g. in apr_allocator_max_free_set. But I suspect the debug structures for POOL_DEBUG are not adequately mutexed. Has anyone specifically used pool-debug and worker-mpm together under heavy load? Thanks! -Josh
Re: non-splittable buckets (was: Regression with range fix)
On Sep 1, 2011, at 8:59 AM, Joe Orton wrote: On Thu, Sep 01, 2011 at 02:47:19PM +0200, Plüm, Rüdiger, VF-Group wrote: If we rip it out, we should replace it with ap_assert()s. And maybe only do it in 2.3? It would seem odd to have ENOTIMPL as a fatal error but other real errors non-fatal. *No* error should occur here with unless somebody has cooked up a really whacky bucket type. So you would follow the rip-out-and-replace-with-asserts approach? No, just remove the special case for ENOTIMPL and don't change anything else. +1...
RequestHeader early with CVE-2011-3192
Hello In case I don't want to support Range and Request-Range headers at all, would it be safe to remove those headers in the early processing hook? Something like: RequestHeader unset Range early RequestHeader unset Range-Request early I'm asking because the documentation of mod_headers recommends not using the early mode in an operational server. Thanks Yehezkel Horowitz Check Point Software Technologies Ltd.
Re: Pool Debug Worker MPM compatibility
Oh also I should not that when I do my load-test with pool-debugging off, all is well. The error_log has zero signals/aborts. The main reason I was using pool-debug in the first place was to get better valgrind leak-checks. But if this is just not compatible with Worker MPM I can stay with pool debug off. -Josh On Thu, Sep 1, 2011 at 9:05 AM, Joshua Marantz jmara...@google.com wrote: Hi Ben, Hmmm...don't know what happened to that subject line po. Not what I meant to type, obviously! On Thu, Sep 1, 2011 at 8:14 AM, Ben Noordhuis i...@bnoordhuis.nl wrote: That assertion is triggered when you add a string from pool A to a table in pool B where A is a child of B (adding headers from the request_rec to a conn_rec table, for example). It's a lifecycle issue. OK given the stack-trace above it's hard for me to figure out a path back from my module. Of course a random memory stomp could make any weird thing happen, but only this one specific thing always happens. And we try to avoid memory-stomps via valgrind etc though it's impossible to prove that none exist under load. But I think I've found 2 potential problems in httpd: 1. protocol.c calls apr_table_addn using a char* value that is offset from a pool-allocated string. It's not obvious that it's doing this safely but it makes sense this code has to run crazy fast and has lots of mileage on it. And it's probably OK for the pool-find algorithm appears to do a pointer-range check in apr_pools.c:1908, function pool_find(). For the moment I'll assume this is not the actual problem. 2. there are a bunch of ifdefs in apr_pools.c for POOL_DEBUG and there's a some mutexing for APR_HAS_THREADS, e.g. in apr_allocator_max_free_set. But I suspect the debug structures for POOL_DEBUG are not adequately mutexed. Has anyone specifically used pool-debug and worker-mpm together under heavy load? Thanks! -Josh
Re: CVE-2003-1418 - still affects apache 2 current
On Thu, 1 Sep 2011 14:39:11 +0200
Marcus Meissner meiss...@suse.de wrote:
Hi,
CVE-2003-1418, a minor security issue, is still affecting the current
codebase.
someone opened a tracker bug a year ago without feedback:
https://issues.apache.org/bugzilla/show_bug.cgi?id=49623
I've just hacked up a simple candidate patch. Review?
(trunk patch - trivial offset when applied to 2.2.x)
--
Nick Kew
Index: modules/http/http_etag.c
===
--- modules/http/http_etag.c(revision 1164053)
+++ modules/http/http_etag.c(working copy)
@@ -26,6 +26,7 @@
#include http_core.h
#include http_protocol.h /* For index_of_response(). Grump. */
#include http_request.h
+#include util_md5.h
/* Generate the human-readable hex representation of an apr_uint64_t
* (basically a faster version of 'sprintf(%llx)')
@@ -50,6 +51,13 @@
*next++ = HEX_DIGITS[u (apr_uint64_t)0xf];
return next;
}
+static char *etag_uint64_to_md5(char *next, apr_uint64_t u, apr_pool_t *pool)
+{
+char *digest = ap_md5_binary(pool, (unsigned char*)u, sizeof(u));
+int len = strlen(digest);
+memcpy(next, digest, len);
+return next+len;
+}
#define ETAG_WEAK W/
#define CHARS_PER_UINT64 (sizeof(apr_uint64_t) * 2)
@@ -114,7 +122,7 @@
* FileETag keywords.
*/
etag = apr_palloc(r-pool, weak_len + sizeof(\--\) +
- 3 * CHARS_PER_UINT64 + 1);
+ 2 * CHARS_PER_UINT64 + 2 * APR_MD5_DIGESTSIZE + 1);
next = etag;
if (weak) {
while (*weak) {
@@ -124,7 +132,7 @@
*next++ = '';
bits_added = 0;
if (etag_bits ETAG_INODE) {
-next = etag_uint64_to_hex(next, r-finfo.inode);
+next = etag_uint64_to_md5(next, r-finfo.inode, r-pool);
bits_added |= ETAG_INODE;
}
if (etag_bits ETAG_SIZE) {
Re: RequestHeader early with CVE-2011-3192
On Thu, 1 Sep 2011 16:58:07 +0300 Yehezkel Horowitz horow...@checkpoint.com wrote: Hello In case I don't want to support Range and Request-Range headers at all, would it be safe to remove those headers in the early processing hook? Something like: RequestHeader unset Range early RequestHeader unset Range-Request early I'm asking because the documentation of mod_headers recommends not using the early mode in an operational server. This would be on-topic for the users list rather than here. The reason for that recommendation is that when used 'early' it will have side-effects, like ignoring the context it's supposed to be configured for. If you want the unset to apply server-wide, then early should be fine. -- Nick Kew
Re: CVE-2003-1418 - still affects apache 2 current
On Thu, Sep 01, 2011 at 03:30:57PM +0100, Nick Kew wrote:
On Thu, 1 Sep 2011 14:39:11 +0200
Marcus Meissner meiss...@suse.de wrote:
Hi,
CVE-2003-1418, a minor security issue, is still affecting the current
codebase.
someone opened a tracker bug a year ago without feedback:
https://issues.apache.org/bugzilla/show_bug.cgi?id=49623
I've just hacked up a simple candidate patch. Review?
(trunk patch - trivial offset when applied to 2.2.x)
This just md5s the inodenr, right?
If yes, this is just obfuscation if you do not add some kind of random salt.
(You can still just do
for (i=0;i...;i++) md5($i)
and compare, including use of Rainbow Tables.)
Ciao, Marcus
--
Nick Kew
Index: modules/http/http_etag.c
===
--- modules/http/http_etag.c (revision 1164053)
+++ modules/http/http_etag.c (working copy)
@@ -26,6 +26,7 @@
#include http_core.h
#include http_protocol.h /* For index_of_response(). Grump. */
#include http_request.h
+#include util_md5.h
/* Generate the human-readable hex representation of an apr_uint64_t
* (basically a faster version of 'sprintf(%llx)')
@@ -50,6 +51,13 @@
*next++ = HEX_DIGITS[u (apr_uint64_t)0xf];
return next;
}
+static char *etag_uint64_to_md5(char *next, apr_uint64_t u, apr_pool_t *pool)
+{
+char *digest = ap_md5_binary(pool, (unsigned char*)u, sizeof(u));
+int len = strlen(digest);
+memcpy(next, digest, len);
+return next+len;
+}
#define ETAG_WEAK W/
#define CHARS_PER_UINT64 (sizeof(apr_uint64_t) * 2)
@@ -114,7 +122,7 @@
* FileETag keywords.
*/
etag = apr_palloc(r-pool, weak_len + sizeof(\--\) +
- 3 * CHARS_PER_UINT64 + 1);
+ 2 * CHARS_PER_UINT64 + 2 * APR_MD5_DIGESTSIZE + 1);
next = etag;
if (weak) {
while (*weak) {
@@ -124,7 +132,7 @@
*next++ = '';
bits_added = 0;
if (etag_bits ETAG_INODE) {
-next = etag_uint64_to_hex(next, r-finfo.inode);
+next = etag_uint64_to_md5(next, r-finfo.inode, r-pool);
bits_added |= ETAG_INODE;
}
if (etag_bits ETAG_SIZE) {
--
Working, but not speaking, for the following german company:
SUSE LINUX Products GmbH, HRB 16746 (AG Nuernberg)
Geschaeftsfuehrer: Jeff Hawn, Jennifer Guild, Felix Imendoerffer
Re: CVE-2003-1418 - still affects apache 2 current
On Thu, Sep 01, 2011 at 02:39:11PM +0200, Marcus Meissner wrote: Hi, CVE-2003-1418, a minor security issue, is still affecting the current codebase. someone opened a tracker bug a year ago without feedback: https://issues.apache.org/bugzilla/show_bug.cgi?id=49623 Do you have a statement? Use FileETag -INode if you care about leaking inode numbers. I think there was consensus that the default should be changed to that, but I can't find the discussion. Regards, Joe
Re: CVE-2003-1418 - still affects apache 2 current
On Thu, 1 Sep 2011 16:36:24 +0200 Marcus Meissner meiss...@suse.de wrote: This just md5s the inodenr, right? If yes, this is just obfuscation if you do not add some kind of random salt. (You can still just do for (i=0;i...;i++) md5($i) and compare, including use of Rainbow Tables.) Erm, yeah. I guess brute force on 2^64 numbers is too easy, even if the information leaked is of low value. Would you consider it strong enough if we aggregate inode+size+mtime and make the etag an md5 hash of that? Brings the benefit of a slightly shorter string with a patch that's still simple. -- Nick Kew
Re: CVE-2003-1418 - still affects apache 2 current
On Thu, Sep 01, 2011 at 03:55:28PM +0100, Nick Kew wrote: On Thu, 1 Sep 2011 16:36:24 +0200 Marcus Meissner meiss...@suse.de wrote: This just md5s the inodenr, right? If yes, this is just obfuscation if you do not add some kind of random salt. (You can still just do for (i=0;i...;i++) md5($i) and compare, including use of Rainbow Tables.) Erm, yeah. I guess brute force on 2^64 numbers is too easy, even if the information leaked is of low value. Would you consider it strong enough if we aggregate inode+size+mtime and make the etag an md5 hash of that? Brings the benefit of a slightly shorter string with a patch that's still simple. Both size and mtime are easily retrievable from remote, you need to add some stuff the attacker cannot derive ;) Ciao, Marcus
Re: po
this code has to run crazy fast and has lots of mileage on it. ... OK given the stack-trace above it's hard for me to figure out a path back from my module. Why not run the test without your new module loaded? That sems like a far simpler and more reliable indication of whether the issue is in the code with mileage on it or in the new code. Hmmm...don't know what happened to that subject line po. Not what I meant to type, obviously! Another O would have been quite embarassing wouldn't it. -- Ray Morris supp...@bettercgi.com Strongbox - The next generation in site security: http://www.bettercgi.com/strongbox/ Throttlebox - Intelligent Bandwidth Control http://www.bettercgi.com/throttlebox/ Strongbox / Throttlebox affiliate program: http://www.bettercgi.com/affiliates/user/register.php
Re: CVE-2003-1418 - still affects apache 2 current
On Thu, Sep 01, 2011 at 05:17:16PM +0200, Reindl Harald wrote: .. mtime - well, is directly in the header - Last-Modified size - well, directly in the header - Content-Length inode - well, where is there any security implication? I could not directly think of one. The reason is just that there is a CVE entry that checkers check for and every user of those checkers asks back from their vendors. A statement from Apache project that its not seen as security issue is probably sufficient. Ciao, Marcus
Re: po
On Thu, Sep 1, 2011 at 11:16 AM, Ray Morris supp...@bettercgi.com wrote: this code has to run crazy fast and has lots of mileage on it. ... OK given the stack-trace above it's hard for me to figure out a path back from my module. Why not run the test without your new module loaded? This is such an excellent idea. The module is, unfortunately in this case, generating the load for the load-test, as well as handling it. I think I can have the module do *less* but if the module is not loaded at all, then there will be no load. That seems like a far simpler and more reliable indication of whether the issue is in the code with mileage on it or in the new code. The code with mileage in protocol.c is not really under suspicion at this point -- I probably shouldn't have mentioned it, except that it appears in the stack at the time of the abort(), and mod_pagespeed doesn't. The code that I suspect has a lot less mileage on it is the application of pool debugging to the Worker MPM. That's why I want to know if anyone else has put those two together under heavy load. -Josh
RE: CVE-2003-1418 - still affects apache 2 current
-Original Message- From: Joe Orton [mailto:jor...@redhat.com] Sent: Donnerstag, 1. September 2011 16:46 To: Marcus Meissner Cc: dev@httpd.apache.org Subject: Re: CVE-2003-1418 - still affects apache 2 current On Thu, Sep 01, 2011 at 02:39:11PM +0200, Marcus Meissner wrote: Hi, CVE-2003-1418, a minor security issue, is still affecting the current codebase. someone opened a tracker bug a year ago without feedback: https://issues.apache.org/bugzilla/show_bug.cgi?id=49623 Do you have a statement? Use FileETag -INode if you care about leaking inode numbers. I think there was consensus that the default should be changed to that, but I can't find the discussion. Can't find the discussion either, but I remember that it was not seen as a security issue. For those still concerned about this, the advice was as you said FileETag -INode. So IMHO no need for a patch here except for documentation and default config Regards Rüdiger
Re: svn commit: r1163918 - /httpd/httpd/trunk/modules/http/byterange_filter.c
On 9/1/2011 1:30 AM, rpl...@apache.org wrote:
Author: rpluem
Date: Thu Sep 1 06:30:02 2011
New Revision: 1163918
URL: http://svn.apache.org/viewvc?rev=1163918view=rev
Log:
* Fix error message
--- httpd/httpd/trunk/modules/http/byterange_filter.c (original)
+++ httpd/httpd/trunk/modules/http/byterange_filter.c Thu Sep 1 06:30:02 2011
@@ -640,7 +640,9 @@ static int ap_set_byterange(request_rec
}
else if (num_ranges == 0 unsatisfiable) {
/* If all ranges are unsatisfiable, we should return 416 */
- ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, 0U);
+ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ All ranges in Range header (%s) are unsatisfiable,
+ it);
I know this isn't your change, but just noticed it. APLOG_ERR?
At its highest shouldn't that be APLOG_INFO?
APLOG_ERR and APLOG_WARN should be reserved for server-side errors
that we have some control over, not bogus input :)
RE: svn commit: r1163918 - /httpd/httpd/trunk/modules/http/byterange_filter.c
-Original Message-
From: William A. Rowe Jr. [mailto:wr...@rowe-clan.net]
Sent: Donnerstag, 1. September 2011 18:38
To: dev@httpd.apache.org
Subject: Re: svn commit: r1163918 -
/httpd/httpd/trunk/modules/http/byterange_filter.c
On 9/1/2011 1:30 AM, rpl...@apache.org wrote:
Author: rpluem
Date: Thu Sep 1 06:30:02 2011
New Revision: 1163918
URL: http://svn.apache.org/viewvc?rev=1163918view=rev
Log:
* Fix error message
--- httpd/httpd/trunk/modules/http/byterange_filter.c (original)
+++ httpd/httpd/trunk/modules/http/byterange_filter.c Thu
Sep 1 06:30:02 2011
@@ -640,7 +640,9 @@ static int ap_set_byterange(request_rec
}
else if (num_ranges == 0 unsatisfiable) {
/* If all ranges are unsatisfiable, we should return 416 */
- ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r, 0U);
+ap_log_rerror(APLOG_MARK, APLOG_ERR, 0, r,
+ All ranges in Range header (%s) are
unsatisfiable,
+ it);
I know this isn't your change, but just noticed it. APLOG_ERR?
At its highest shouldn't that be APLOG_INFO?
APLOG_ERR and APLOG_WARN should be reserved for server-side errors
that we have some control over, not bogus input :)
See r1163920 (http://svn.apache.org/viewvc?rev=1163920view=rev) :-)
Regards
Rüdiger
Re: Next update
Hi Dirk, Am 31.08.2011 22:03, schrieb Dirk-WIllem van Gulik: Suggestion for http://people.apache.org/~dirkx/CVE-2011-3192.txt to be sent to announce and the usual security places. 4) Deploy a Range header count module as a temporary stopgap measure. An improved stop-gap module for the 2.x series was provided by Guenter Knauf and can be found at: http://people.apache.org/~dirkx/mod_rangecnt-improved/ I've just fixed the mod to also compile and work now with 1.3.x: http://people.apache.org/~fuankg/httpd/mod_rangecnt-improved/mod_rangecnt.tar.gz OS and Vendor specific information == NetWare:Pre compiled binaries available. I've now moved the module binaries into separate folders; original: http://people.apache.org/~fuankg/httpd/mod_rangecnt/ improved = configurable: http://people.apache.org/~fuankg/httpd/mod_rangecnt-improved/ Gregg has also compiled Win32 binaries; original: http://people.apache.org/~gsmith/httpd/binaries/modules/mod_rangecnt/ Gün.
Re: Appropriate patches for 2.2.19 and 2.0.64?
On 9/1/2011 2:41 AM, Plüm, Rüdiger, VF-Group wrote: -Original Message- From: William A. Rowe Jr. [mailto:wr...@rowe-clan.net] Sent: Donnerstag, 1. September 2011 03:51 To: dev@httpd.apache.org Subject: Re: Appropriate patches for 2.2.19 and 2.0.64? On 8/31/2011 4:16 PM, William A. Rowe Jr. wrote: I've attempted to simply substitute the 2.2.19 filter code into the 2.0.64 http_protocol.c sources, and am unsure how far off these patches are from what they need to be; there's been a significant amount of drift and refactoring in the interim. Still looking for feedback, but the attached applies and corresponds to 2.2.20 with the exception of atoi rather than strtoi semantics, and without the no DefaultType exception.. Thanks for taking care. It applies cleanly, but most byterange tests from the framework fail: Failed Test Stat Wstat Total Fail Failed List of Failed --- t/apache/byterange.t 136 97 71.32% 2-9 11 16-17 19-20 25-26 28 33-35 37-39 42 44-47 49-61 63-68 70-73 77-79 83-88 90-96 98-99 101-103 105-113 115-123 126-127 130 132-136 t/apache/byterange5.t54 80.00% 1 3-5 t/apache/byterange7.t 135 38.46% 2-5 12 This might be because I use a 64 bit build of 2.0.x (no other test environment at hand at the moment) and 2.0.x is not really good at 64 bit (lots of compiler warnings). Ideally can you provide me the -verbose output (offlist or to your people.a.o/ space if it's lengthy)?
Re: svn commit: r1163833 - /httpd/httpd/trunk/modules/http/byterange_filter.c
On Sep 1, 2011, at 1:11 AM, Tim Bannister wrote: On Wed, Aug 31, 2011 at 6:28 PM, Roy T. Fielding wrote: On Aug 31, 2011, at 6:10 PM, William A. Rowe Jr. wrote: The presumption here is that the client requests bytes=0- to begin the transmission, and provided it sees a 206, restarting somewhere in the stream results in aborting the connection and streaming bytes=n- from the restart point. Further testing should determine if this was the broken assumption. Do we send the Accept-Ranges header field? http://tools.ietf.org/html/rfc2616#page-105 Apache httpd 2.2.9 is sending this header in the Debian bug report at http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=639825 Then the client is broken and they also break intermediary caches (most of which don't cache 206 responses) by performing this stunt. We could add a version-specific browsermatch to do the 206, but I'd prefer to just tell the client developers to fix their code. Roy
Re: Appropriate patches for 2.2.19 and 2.0.64?
On 01.09.2011 19:18, William A. Rowe Jr. wrote: On 9/1/2011 2:41 AM, Plüm, Rüdiger, VF-Group wrote: Ideally can you provide me the -verbose output (offlist or to your people.a.o/ space if it's lengthy)? Sorry for kicking in late. I was on holidays until Sunday and was a bit overwhelmed by what had happened in between. I built 2.0 trunk with your patch on Solaris Sparc 32 Bits and a lot of the byterange tests fail for me also. I put the test output and the complete test directory for prefork and worker at: http://people.apache.org/~rjung/CVE-2011-3192/2.0.x/test/ The tarballs contain the error and access logs. Regards, Rainer
Re: CVE-2003-1418 - still affects apache 2 current
On 9/1/2011 10:23 AM, Marcus Meissner wrote: On Thu, Sep 01, 2011 at 05:17:16PM +0200, Reindl Harald wrote: .. mtime - well, is directly in the header - Last-Modified size - well, directly in the header - Content-Length inode - well, where is there any security implication? I could not directly think of one. The reason is just that there is a CVE entry that checkers check for and every user of those checkers asks back from their vendors. A statement from Apache project that its not seen as security issue is probably sufficient. Ciao, Marcus This is a sane response to the problem. I've been asking why this is a vulnerability for years and have yet to receive an answer... Maybe I haven't asked the right people. -- -- Daniel Ruggeri
Re: Another regression regarding byteranges
On 9/1/2011 7:51 AM, Dirk-Willem van Gulik wrote: On 1 Sep 2011, at 13:33, Jim Jagielski wrote: On Sep 1, 2011, at 6:31 AM, Plüm, Rüdiger, VF-Group wrote: I already fixed that in trunk. I think this regression justifies another release for 2.2.x. But IMHO we should wait at least until mid next week to see if other regressions come thru and hit them all with a 2.2.21. +1 Ok - so this makes it sound we really should get the advisory out. Shall I update it with some caveats and stay tuned - but still make it FINAL ? Or should we make this an update - and not declare final victory ? +1 An /update/ with patch pointers (the patches themselves being moving targets as we find these bugs) and to point out the feedback mechanism (request before and after patch request and response headers as examples and especially what patch level as documented in the patch) should do just fine as an update, and we should presume there are more bugs hiding in this code. Now that 2.2.20 was published, I see little reason to track this as a wiki page; bugzilla is probably sufficient now.
