Re: don't try this at home
On 30 May 2002, Jeff Trawick wrote: > > GET / HTTP/1.1 > > Accept: */* > > Host: test > > Content-Type: application/x-www-form-urlencoded > > Transfer-Encoding: chunked > > > > AAA > > Yesterday afternoon: > > on one build I was getting a 200 followed by a 500 (access_log said > 500 was for method A...) > > on another build I was getting a segfault Dear Jeff, I tried it on my server at my home webserver and the server (child) segfaults with an entry in error_log: [Fri May 31 19:52:12 2002] [error] [client 127.0.0.1] Directory index forbidden by rule: /usr/local/apache/www/ [Fri May 31 19:52:19 2002] [notice] child pid 17945 exit signal Segmentation fault (11) My configuration is listed below. Mail me privately for more information if you need it. Cheers, Allard Hoeve -- Self-compiled on Linux 2.4.19-pre6, Debian, gcc 2.95.4, ld version 2.12.90.0.1 Server: Apache/1.3.20 (Unix) mod_ssl/2.8.4 OpenSSL/0.9.6b mod_perl/1.26 PHP/4.1.2 Compiled-in modules: http_core.c mod_env.c mod_log_config.c mod_mime.c mod_negotiation.c mod_status.c mod_include.c mod_autoindex.c mod_dir.c mod_cgi.c mod_asis.c mod_imap.c mod_actions.c mod_userdir.c mod_alias.c mod_rewrite.c mod_access.c mod_auth.c mod_proxy.c mod_so.c mod_setenvif.c suexec: enabled; valid wrapper /usr/local/httpd/bin/suexec
[PATCH] Re: don't try this at home
Jeff Trawick <[EMAIL PROTECTED]> writes: > okay, do try it, but (unlike somebody last night) don't try it on daedalus > > GET / HTTP/1.1 > Accept: */* > Host: test > Content-Type: application/x-www-form-urlencoded > Transfer-Encoding: chunked > > AAA The situation where I (still) get 200 on the GET followed by 500 for method AAA... is when "GET /" is handled by mod_autoindex. Isn't mod_autoindex responsible for discarding the request body? With this change I'm getting 413 for the autoindex request: Index: modules/generators/mod_autoindex.c === RCS file: /home/cvs/httpd-2.0/modules/generators/mod_autoindex.c,v retrieving revision 1.108 diff -u -r1.108 mod_autoindex.c --- modules/generators/mod_autoindex.c 18 May 2002 04:13:12 - 1.108 +++ modules/generators/mod_autoindex.c 31 May 2002 13:30:52 - @@ -2133,6 +2133,12 @@ /* OK, nothing easy. Trot out the heavy artillery... */ if (allow_opts & OPT_INDEXES) { +int errstatus; + +if ((errstatus = ap_discard_request_body(r)) != OK) { +return errstatus; +} + /* KLUDGE --- make the sub_req lookups happen in the right directory. * Fixing this in the sub_req_lookup functions themselves is difficult, * and would probably break virtual includes... -- Jeff Trawick | [EMAIL PROTECTED] Born in Roswell... married an alien...
Re: don't try this at home
Justin Erenkrantz <[EMAIL PROTECTED]> writes: > My suggestion is as follows: if the ap_die() code is one that > forces us to drop the connection, we don't report the recursive > error, but instead just report this one. > > So the conditional on http_request.c:121 may work as: > > if (r->status != HTTP_OK && !ap_status_drops_connection(type)) { > > Can you try this? -- justin oops, just saw your note... but you fixed it already anyway :) Thanks! -- Jeff Trawick | [EMAIL PROTECTED] Born in Roswell... married an alien...
Re: don't try this at home
On Thu, May 30, 2002 at 11:02:09AM -0400, Jeff Trawick wrote: > Running HEAD, we get a segfault when the bogus request is for a > resource for which we aren't authorized. Here is the traceback: In particular, this problem is related to the fact that we have two errors on this request: 401 and 413. The problem is that since 401 was reported "first," the 413 is "lost" in ap_die() when we find the recursive error (http_request.c:122) and we reset r->status to 401. However, 401 isn't listed as a status code that drops the connection (line 146). Therefore, ap_die() calls ap_discard_request_body again - which causes HTTP_IN to get called again. But, since we are 413, we're not supposed to re-read the body. Oops. My suggestion is as follows: if the ap_die() code is one that forces us to drop the connection, we don't report the recursive error, but instead just report this one. So the conditional on http_request.c:121 may work as: if (r->status != HTTP_OK && !ap_status_drops_connection(type)) { Can you try this? -- justin
Re: don't try this at home
On Thu, May 30, 2002 at 01:33:45PM -0700, Justin Erenkrantz wrote: > On Thu, May 30, 2002 at 11:02:09AM -0400, Jeff Trawick wrote: > > Running HEAD, we get a segfault when the bogus request is for a > > resource for which we aren't authorized. Here is the traceback: > > Oh, that's not good. Something broke after I initially fixed it. > I'm trying to chase down what happened now. -- justin As an update: My problem was related to mod_bucketeer eating the error buckets. I fixed that and it works for me now. (Perhaps other output filters need to pay attention to the error bucket as well?) I'll try with a URL that generates a 401 now. -- justin
Re: don't try this at home
On Thu, May 30, 2002 at 11:02:09AM -0400, Jeff Trawick wrote: > Running HEAD, we get a segfault when the bogus request is for a > resource for which we aren't authorized. Here is the traceback: Oh, that's not good. Something broke after I initially fixed it. I'm trying to chase down what happened now. -- justin
Re: don't try this at home
Jeff Trawick <[EMAIL PROTECTED]> writes: > Jeff Trawick <[EMAIL PROTECTED]> writes: > > > okay, do try it, but (unlike somebody last night) don't try it on daedalus > > > > GET / HTTP/1.1 > > Accept: */* > > Host: test > > Content-Type: application/x-www-form-urlencoded > > Transfer-Encoding: chunked > > > > AAA > > Is this test (with cr-lf added where appropriate) working consistently > for everybody that tries? > > Yesterday afternoon: > > on one build I was getting a 200 followed by a 500 (access_log said > 500 was for method A...) > > on another build I was getting a segfault Running HEAD, we get a segfault when the bogus request is for a resource for which we aren't authorized. Here is the traceback: #0 0x4014bd41 in __kill () from /lib/libc.so.6 #1 0x4014b9b6 in raise (sig=6) at ../sysdeps/posix/raise.c:27 #2 0x4014d0d8 in abort () at ../sysdeps/generic/abort.c:88 #3 0x80be80c in ap_log_assert (szExp=0x80e945b "readbytes > 0", szFile=0x80e93e4 "http_protocol.c", nLine=957) at log.c:689 #4 0x807dc8c in ap_http_filter (f=0x81b46a8, b=0x81b3f48, mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=-1) at http_protocol.c:957 #5 0x80cae79 in ap_get_brigade (next=0x81b46a8, bb=0x81b3f48, mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192) at util_filter.c:507 #6 0x80d5125 in net_time_filter (f=0x81b3f68, b=0x81b3f48, mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192) at core.c:3324 #7 0x80cae79 in ap_get_brigade (next=0x81b3f68, bb=0x81b3f48, mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192) at util_filter.c:507 #8 0x807f46d in ap_get_client_block (r=0x81b3808, buffer=0xbfffb5c0, bufsiz=8192) at http_protocol.c:1769 #9 0x807f765 in ap_discard_request_body (r=0x81b3808) at http_protocol.c:1874 #10 0x8081df9 in ap_die (type=401, r=0x81b3808) at http_request.c:153 #11 0x807eaf8 in ap_http_header_filter (f=0x81b3fb0, b=0x81b4b48) at http_protocol.c:1448 #12 0x80caefd in ap_pass_brigade (next=0x81b3fb0, bb=0x81b4b48) at util_filter.c:534 #13 0x80ce3aa in ap_content_length_filter (f=0x81b3f98, b=0x81b4b48) at protocol.c:1292 #14 0x80caefd in ap_pass_brigade (next=0x81b3f98, bb=0x81b4b48) at util_filter.c:534 #15 0x8081223 in ap_byterange_filter (f=0x81b3f80, bb=0x81b4b48) at http_protocol.c:2745 #16 0x80caefd in ap_pass_brigade (next=0x81b3f80, bb=0x81b4b48) at util_filter.c:534 #17 0x807d94c in ap_http_filter (f=0x81b46a8, b=0x81b3f48, mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192) at http_protocol.c:888 #18 0x80cae79 in ap_get_brigade (next=0x81b46a8, bb=0x81b3f48, mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192) at util_filter.c:507 #19 0x80d5125 in net_time_filter (f=0x81b3f68, b=0x81b3f48, mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192) at core.c:3324 #20 0x80cae79 in ap_get_brigade (next=0x81b3f68, bb=0x81b3f48, mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192) at util_filter.c:507 #21 0x807f46d in ap_get_client_block (r=0x81b3808, buffer=0xbfffd878, bufsiz=8192) at http_protocol.c:1769 #22 0x807f765 in ap_discard_request_body (r=0x81b3808) at http_protocol.c:1874 #23 0x8081df9 in ap_die (type=401, r=0x81b3808) at http_request.c:153 #24 0x80820c2 in ap_process_request (r=0x81b3808) at http_request.c:277 #25 0x807bbe1 in ap_process_http_connection (c=0x81ad848) at http_core.c:291 #26 0x80c7ce0 in ap_run_process_connection (c=0x81ad848) at connection.c:85 #27 0x80c8145 in ap_process_connection (c=0x81ad848, csd=0x81ad788) at connection.c:207 #28 0x80b80a4 in child_main (child_num_arg=4) at prefork.c:671 #29 0x80b8259 in make_child (s=0x8158c50, slot=4) at prefork.c:765 #30 0x80b82f2 in startup_children (number_to_start=1) at prefork.c:783 #31 0x80b87f1 in ap_mpm_run (_pconf=0x810cfe8, plog=0x814d0e8, s=0x8158c50) at prefork.c:999 #32 0x80bfe7c in main (argc=3, argv=0xba84) at main.c:646 (gdb) cute recursion, huh? I'll try to see what is up with the weird 200-followed-by-500 problem. -- Jeff Trawick | [EMAIL PROTECTED] Born in Roswell... married an alien...
Re: don't try this at home
Jeff Trawick <[EMAIL PROTECTED]> writes: > okay, do try it, but (unlike somebody last night) don't try it on daedalus > > GET / HTTP/1.1 > Accept: */* > Host: test > Content-Type: application/x-www-form-urlencoded > Transfer-Encoding: chunked > > AAA Is this test (with cr-lf added where appropriate) working consistently for everybody that tries? Yesterday afternoon: on one build I was getting a 200 followed by a 500 (access_log said 500 was for method A...) on another build I was getting a segfault I'll try to understand this more later today. -- Jeff Trawick | [EMAIL PROTECTED] Born in Roswell... married an alien...
Re: don't try this at home
On Wed, May 29, 2002 at 02:47:35PM +0200, Martin Kraemer wrote: > But IMO we need to have a way to parse the hex string and detect an > integer overflow at the same time. If an overflow occurs, then > an 4XX message is appropriate (400 Bad Request rather than > 413 Request Entity Too Large) I mostly agree on the codes (not that it matters that much if it's 400 or 413, but I'm sure Roy has an opinion on this). I would think that 400 makes sense for overflow, but then again, if we can't handle the size it's not really a bad request... > Then, as a second step (if the number parsed all right, even if it > was incredibly long, as in this chunk of 33 bytes: > 00021 CRLF > ) we can try and verify whether we accept the size. For that, we > have an upper limit defined by "LimitRequestBody bytes". > Anything beyond that can impossibly be accepted. With this I completely agree with, but I think this is already happening. I'd need to review the code to be sure. Thanks for the leading-zeros hint, I'll fix that momentarily. -aaron
Re: don't try this at home
On Tue, May 28, 2002 at 08:00:16AM -0700, Justin Erenkrantz wrote: > On Tue, May 28, 2002 at 10:18:52AM -0400, Jeff Trawick wrote: > > okay, do try it, but (unlike somebody last night) don't try it on daedalus > > > > GET / HTTP/1.1 > > Accept: */* > > Host: test > > Content-Type: application/x-www-form-urlencoded > > Transfer-Encoding: chunked > > > > AAA > > Hmm. Isn't that legal? A is a hex digit. RFC2616: Chunked-Body = *chunk last-chunk trailer CRLF chunk = chunk-size [ chunk-extension ] CRLF chunk-data CRLF chunk-size = 1*HEX last-chunk = 1*("0") [ chunk-extension ] CRLF so, strictly spoken, it is "legal". The trailing chunk could have been 0 CRLF and still be legal. But IMO we need to have a way to parse the hex string and detect an integer overflow at the same time. If an overflow occurs, then an 4XX message is appropriate (400 Bad Request rather than 413 Request Entity Too Large) Then, as a second step (if the number parsed all right, even if it was incredibly long, as in this chunk of 33 bytes: 00021 CRLF ) we can try and verify whether we accept the size. For that, we have an upper limit defined by "LimitRequestBody bytes". Anything beyond that can impossibly be accepted. Martin -- <[EMAIL PROTECTED]> | Fujitsu Siemens Fon: +49-89-636-46021, FAX: +49-89-636-47655 | 81730 Munich, Germany
Re: don't try this at home
On Tue, May 28, 2002 at 10:37:51AM -0700, Justin Erenkrantz wrote: > I'll take a pass at figuring out how to return this. -- justin It's trivial to detect this case - ctx->remaining is < 0. But, it is actually way harder than it should be because we call ap_discard_body on the error handling path via ap_die(). That means we become re-entrant to HTTP_IN. Uh-oh. If we were to remove HTTP_IN, when ap_discard_body() is called, the core input filters will then block waiting for 8k of data to be read - which is bogus as well. So, what's the best way for an input filter to kill the request and return an error page? ISTR some discussion about the fact that there is a difference between filter return codes and HTTP error codes. Thoughts? Off to lunch while I ponder this some more. -- justin
Re: don't try this at home
On Tue, May 28, 2002 at 01:22:34PM -0400, Greg Ames wrote: > Jeff said something about the chunk size being negative. Clearly that's wrong, > but while we're at it, we should review the RFC and see what the rules are. Obviously, this is an overflow condition. We should be returning 413: 10.4.14 413 Request Entity Too Large The server is refusing to process a request because the request entity is larger than the server is willing or able to process. The server MAY close the connection to prevent the client from continuing the request. If the condition is temporary, the server SHOULD include a Retry- After header field to indicate that it is temporary and after what time the client MAY try again. I'll take a pass at figuring out how to return this. -- justin
Re: don't try this at home
Justin Erenkrantz wrote: > > On Tue, May 28, 2002 at 10:18:52AM -0400, Jeff Trawick wrote: > > okay, do try it, but (unlike somebody last night) don't try it on daedalus > > > > GET / HTTP/1.1 > > Accept: */* > > Host: test > > Content-Type: application/x-www-form-urlencoded > > Transfer-Encoding: chunked > > > > AAA > > Hmm. Isn't that legal? A is a hex digit. > > What should we be doing? -- justin well, not this for sure: (from /usr/local/apache2.0.37dev/corefiles/httpd.core.1) #0 0x2815e7b4 in kill () from /usr/lib/libc.so.4 #1 0x2819eb26 in abort () from /usr/lib/libc.so.4 #2 0x80699d2 in ap_log_assert (szExp=0x80854db "readbytes > 0", szFile=0x8085464 "http_protocol.c", nLine=911) at log.c:691 #3 0x805ffe8 in ap_http_filter (f=0x814bef8, b=0x814b798, mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=-1431655766) at http_protocol.c:911 #4 0x80717fc in ap_get_brigade (next=0x814bef8, bb=0x814b798, mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192) at util_filter.c:507 #5 0x8078f4d in net_time_filter (f=0x814b7b8, b=0x814b798, mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192) at core.c:3324 #6 0x80717fc in ap_get_brigade (next=0x814b7b8, bb=0x814b798, mode=AP_MODE_READBYTES, block=APR_BLOCK_READ, readbytes=8192) at util_filter.c:507 #7 0x8061225 in ap_get_client_block (r=0x814b048, buffer=0xbfbfd790 ".0-doc/config/ajp.html\n", bufsiz=8192) at http_protocol.c:1705 #8 0x8061407 in ap_discard_request_body (r=0x814b048) at http_protocol.c:1801 #9 0x8063153 in ap_die (type=302, r=0x814b048) at http_request.c:150 #10 0x8063376 in ap_process_request (r=0x814b048) at http_request.c:274 #11 0x805e8f3 in ap_process_http_connection (c=0x8134120) at http_core.c:291 Jeff said something about the chunk size being negative. Clearly that's wrong, but while we're at it, we should review the RFC and see what the rules are. Greg
Re: don't try this at home
Justin Erenkrantz <[EMAIL PROTECTED]> writes: > On Tue, May 28, 2002 at 10:18:52AM -0400, Jeff Trawick wrote: > > okay, do try it, but (unlike somebody last night) don't try it on daedalus > > > > GET / HTTP/1.1 > > Accept: */* > > Host: test > > Content-Type: application/x-www-form-urlencoded > > Transfer-Encoding: chunked > > > > AAA > > Hmm. Isn't that legal? A is a hex digit. > > What should we be doing? -- justin Here is what we shouldn't be doing: /usr/local/apache/corefiles/httpd.core.1 My guess is that we have an overflow or sign problem handling the chunk size. Another little detail is that we puked before reading any data following the chunk size. -- Jeff Trawick | [EMAIL PROTECTED] Born in Roswell... married an alien...
Re: don't try this at home
On Tue, May 28, 2002 at 10:18:52AM -0400, Jeff Trawick wrote: > okay, do try it, but (unlike somebody last night) don't try it on daedalus > > GET / HTTP/1.1 > Accept: */* > Host: test > Content-Type: application/x-www-form-urlencoded > Transfer-Encoding: chunked > > AAA Hmm. Isn't that legal? A is a hex digit. What should we be doing? -- justin
don't try this at home
okay, do try it, but (unlike somebody last night) don't try it on daedalus GET / HTTP/1.1 Accept: */* Host: test Content-Type: application/x-www-form-urlencoded Transfer-Encoding: chunked AAA -- Jeff Trawick | [EMAIL PROTECTED] Born in Roswell... married an alien...