Re: Potential deadlock in psp.py
Nicolas Lehuen wrote: Hi Jim, Until now, we suspected that the way global locks are handled could be deadlock prone. You have just proved it. I know that global locks are expensive on some systems, especially if we want to use them in a multiprocess (forked) environment. That's why we are forced to have such a small pool of global locks. On the other hand, in a multithreaded environment, locks which are valid for the whole process are not so expensive ; indeed, we can create a whole bunch without bringing down the system (think about Java where all object potentially have a monitor which is equivalent to a lock). I think this is a strong incentive to abandon the forked model and go for the multi-threaded model (or the mono-threaded, asynchronous one). For concurrency control, the forked model does not scale, apparently. It would make life easier I suppose, but we are stuck with the fact that apache provides several different mpm models that need to be supported. Jim
Re: Potential deadlock in psp.py
Yeah, we've got to be inline with the HTTP Project - prefork is the default on unix systems, so we have to abide by it... So I guess the solution is that we need to reserve two locks instead of just one? Grisha On Thu, 23 Jun 2005, Jim Gallacher wrote: Nicolas Lehuen wrote: Hi Jim, Until now, we suspected that the way global locks are handled could be deadlock prone. You have just proved it. I know that global locks are expensive on some systems, especially if we want to use them in a multiprocess (forked) environment. That's why we are forced to have such a small pool of global locks. On the other hand, in a multithreaded environment, locks which are valid for the whole process are not so expensive ; indeed, we can create a whole bunch without bringing down the system (think about Java where all object potentially have a monitor which is equivalent to a lock). I think this is a strong incentive to abandon the forked model and go for the multi-threaded model (or the mono-threaded, asynchronous one). For concurrency control, the forked model does not scale, apparently. It would make life easier I suppose, but we are stuck with the fact that apache provides several different mpm models that need to be supported. Jim
Rev 1: [PATCH] 1.3 TraceEnable [on|off|extended]
The attached patch resolved the issue I noted below, 10.4.6 405 Method Not Allowed requires an Allow header (I would presume, even if empty), while 10.5.2 501 Not Implemented states This is the appropriate response when the server does not recognize the request method and is not capable of supporting it for any resource. If 'ProxyEnable off' is set for a given host, the setting is url-impotent, and does not vary. At 12:52 PM 6/22/2005, William A. Rowe, Jr. wrote: FYI there is one small issue still. The resulting Allow: null response to denied TRACE request. TRACE doesn't go through the normal processing, so methods aren't added. And since TRACE is denied, it's removed too. At 08:56 AM 6/22/2005, William A. Rowe, Jr. wrote: I've spent a large number of cycles investigating the Watchfire report (http://www.watchfire.com/resources/HTTP-Request-Smuggling.pdf) and come up with a genuine reason to adopt the attached patch. ... So the attached patch introduces the per-host directive TraceEnable on|off|extended where extended permits a message body, up to 64kb at the target server, and of an unlimited size through a proxy server. The default remains 'on', of course, denying a TRACE body request even via proxy. Following the semantics of TRACE, the request body is returned to the host verbatim as part of the response, following the headers, exactly as sent.
Rev 1: [PATCH] 1.3 TraceEnable [on|off|extended]
[Again, this time w/ the attachement]
The attached patch resolved the issue I noted below,
10.4.6 405 Method Not Allowed requires an Allow header
(I would presume, even if empty, based on #() grammar),
while 10.5.2 501 Not Implemented states;
This is the appropriate response when the server does not
recognize the request method and is not capable of supporting
it for any resource.
If 'ProxyEnable off' is set for a given host, the setting is
url-impotent, and does not vary.
Because the patch does append a new member to the core_server_config
structure, it seems a minor bump is in order.
At 12:52 PM 6/22/2005, William A. Rowe, Jr. wrote:
FYI there is one small issue still. The resulting Allow: null
response to denied TRACE request. TRACE doesn't go through the
normal processing, so methods aren't added. And since TRACE is
denied, it's removed too.
At 08:56 AM 6/22/2005, William A. Rowe, Jr. wrote:
I've spent a large number of cycles investigating the Watchfire report
(http://www.watchfire.com/resources/HTTP-Request-Smuggling.pdf) and
come up with a genuine reason to adopt the attached patch.
...
So the attached patch introduces the per-host directive
TraceEnable on|off|extended
where extended permits a message body, up to 64kb at the target server,
and of an unlimited size through a proxy server. The default remains
'on', of course, denying a TRACE body request even via proxy.
Following the semantics of TRACE, the request body is returned to the
host verbatim as part of the response, following the headers, exactly
as sent.
Index: src/modules/proxy/proxy_http.c
===
--- src/modules/proxy/proxy_http.c (revision 193075)
+++ src/modules/proxy/proxy_http.c (working copy)
@@ -15,6 +15,7 @@
/* HTTP routines for Apache proxy */
+#define CORE_PRIVATE /* To inspect core_server_conf-trace_enable */
#include mod_proxy.h
#include http_log.h
#include http_main.h
@@ -141,6 +142,30 @@
memset(server, '\0', sizeof(server));
server.sin_family = AF_INET;
+if (r-method_number == M_TRACE) {
+core_server_config *coreconf = (core_server_config *)
+ ap_get_module_config(r-server-module_config, core_module);
+
+/* 405 METHOD_NOT_ALLOWED may be more accurate; but we cannot
+ * provide the required Allow: field - this request is not
+ * fully processed. Once disabled for this host, the TRACE
+ * method truly does not exist, as it will not change based
+ * on context.
+ */
+if (coreconf-trace_enable == AP_TRACE_DISABLE)
+return ap_proxyerror(r, HTTP_NOT_IMPLEMENTED,
+ TRACE denied by server configuration);
+
+/* Can't test ap_should_client_block, we aren't ready to send
+ * the client a 100 Continue response till the connection has
+ * been established
+ */
+if (coreconf-trace_enable != AP_TRACE_EXTENDED
+ (r-read_length || (!r-read_chunked (r-remaining = 0
+return ap_proxyerror(r, HTTP_REQUEST_ENTITY_TOO_LARGE,
+ TRACE with request body is not allowed);
+}
+
/* We break the URL into host, port, path-search */
urlptr = strstr(url, ://);
Index: src/include/ap_mmn.h
===
--- src/include/ap_mmn.h(revision 193075)
+++ src/include/ap_mmn.h(working copy)
@@ -203,6 +203,7 @@
* 19990320.16 - ap_escape_errorlog_item()
* 19990320.17 - ap_auth_nonce() and ap_auth_nonce added
*in core_dir_config.
+ * 19990320.18 - trace_enable member added to core server_config
*/
#define MODULE_MAGIC_COOKIE 0x41503133UL /* AP13 */
@@ -210,7 +211,7 @@
#ifndef MODULE_MAGIC_NUMBER_MAJOR
#define MODULE_MAGIC_NUMBER_MAJOR 19990320
#endif
-#define MODULE_MAGIC_NUMBER_MINOR 17/* 0...n */
+#define MODULE_MAGIC_NUMBER_MINOR 18/* 0...n */
/* Useful for testing for features. */
#define AP_MODULE_MAGIC_AT_LEAST(major,minor) \
Index: src/include/http_core.h
===
--- src/include/http_core.h (revision 193075)
+++ src/include/http_core.h (working copy)
@@ -344,8 +344,18 @@
int recursion_limit_set; /* boolean */
int redirect_limit; /* maximum number of internal redirects */
int subreq_limit;/* maximum nesting level of subrequests */
+
+/* TRACE control */
+int trace_enable;/* see AP_TRACE_ below */
+
} core_server_config;
+/* trace_enable options */
+#define AP_TRACE_UNSET-1
+#define AP_TRACE_DISABLE 0
+#define AP_TRACE_ENABLE1
+#define AP_TRACE_EXTENDED 2
+
/* for http_config.c */
CORE_EXPORT(void) ap_core_reorder_directories(pool *, server_rec *);
Index: src/main/http_protocol.c
Re: 2.1.5 available for testing
William A. Rowe, Jr. wrote:
++1 To Joe's comments.
Jeff's fix is technically right, but scares the nibbles out
of me. If, for example, an exploit is able to inject the
T-E on top of the legit C-L, I really suspect we should not
trust the origin server at all.
For origin servers (as opposed to clients) make this choice
between ignore C-L, or fail, configurable?
My observation is that there are far more varied clients in
the world than servers, each with unique faults. But the
RFC 2616 is clear...
Messages MUST NOT include both a Content-Length header field and a
non-identity transfer-coding. If the message does include a non-
identity transfer-coding, the Content-Length MUST be ignored.
When a Content-Length is given in a message where a message-body is
allowed, its field value MUST exactly match the number of OCTETs in
the message-body. HTTP/1.1 user agents MUST notify the user when an
invalid length is received and detected.
...and server authors can be expected to be less buggy than clients.
Permissive in what we accept, strict in what we send implies some
strictness in what we trust to pass on to the client.
So +.5 to Jeff's patch, and let's discuss if the proxy response should
then be trusted at all with T-E and C-L, in httpd-2.x where we support
keepalives.
Once the patch applied we lose the information that the request was incorrect.
That means we won't be able to choose in proxy between sending C-L (and dechunk)
and T-E.
[FYI +1 in httpd-1.3, that proxy does not use keepalives.]
Bill
Bill
At 06:40 PM 6/22/2005, Jeff Trawick wrote:
On 6/22/05, Paul Querna [EMAIL PROTECTED] wrote:
Joe Orton wrote:
On Wed, Jun 22, 2005 at 03:02:50PM -0500, William Rowe wrote:
Prior to either patch we totally mishandled such requests. So the
only question which remains is; which behavior do we prefer?
As the RFC states this is not acceptable, my gut says reject ANY
request with both C-L and T-E of non-identity.
I don't see any reason to reject anything, 2616 dictates precisely how
to handle requests which are malformed in this way. I do think the
check against identity is actually redundant, though; not least
because the 2616 errata remove all references to the word.
I think correct behaviour is to just follow the letter of Section 4.4,
point 3, as below:
If a message is received with both a
Transfer-Encoding header field and a Content-Length header field,
the latter MUST be ignored.
(and it's also going to be better to check for T-E before C-L since
99.99% of requests received are not going to have a T-E header so it'll
fall through slightly quicker)
+1 to the posted patch.
+1 here as well
What about proxy reading from origin server?
Origin server sends this to Apache acting as proxy:
HTTP/1.1 200 OK\r\n
Content-Length: 99\r\n
Transfer-Encoding: chunked\r\n
\r\n
0A\r\n
aa\r\n
00\r\n
\r\n
Client receives this:
HTTP/1.1 200 OK
Date: Wed, 22 Jun 2005 23:12:31 GMT
Content-Length: 99
Content-Type: text/plain; charset=ISO-8859-1
aa(END)
Add this patch:
Index: modules/proxy/mod_proxy_http.c
===
--- modules/proxy/mod_proxy_http.c (revision 191655)
+++ modules/proxy/mod_proxy_http.c (working copy)
@@ -1128,7 +1128,17 @@
r-headers_out,
save_table);
}
-
+
+/* can't have both Content-Length and Transfer-Encoding */
+if (apr_table_get(r-headers_out, Transfer-Encoding)
+ apr_table_get(r-headers_out, Content-Length)) {
+/* 2616 section 4.4, point 3: if both Transfer-Encoding
+ * and Content-Length are received, the latter MUST be
+ * ignored; so unset it here to prevent any confusion
+ * later. */
+apr_table_unset(r-headers_out, Content-Length);
+}
+
/* strip connection listed hop-by-hop headers from response */
backend-close +=
ap_proxy_liststr(apr_table_get(r-headers_out,
Connection),
Client now gets this:
HTTP/1.1 200 OK
Date: Wed, 22 Jun 2005 23:22:19 GMT
Transfer-Encoding: chunked
Content-Type: text/plain; charset=ISO-8859-1
a
aa
0
Re: 2.1.5 available for testing
At 02:34 AM 6/23/2005, jean-frederic clere wrote: Once the patch applied we lose the information that the request was incorrect. That means we won't be able to choose in proxy between sending C-L (and dechunk) and T-E. s/request/response/ The point was, if one were to exploit the origin server to inject a fake T-E, and the C-L is legit, we can't catch it. Suggesting (to me) that it's better to insist on strictly conformant responses from origin servers, which are an entirely different beast than clients. But since there are (undoubtedly) bad origin servers out there, we would likely need to make this a configuration choice, not an absolute rule. After all, they are using Apache to front end their buggy/vulnerable backend servers out there :) Bill
My Input Filter doesn't get called
I add an input filter with an ap_add_input_filter from an ap_hook_header_parser hook. My filter doesn't get called, i guess probably because we are too far in the request processing cycle. Sadly this filter needs perdirectoryconfig information that is not available within previous hooks. So shall i write custom configuration code to solve this problem?? Is there a way to do it?? Thanks, Luca
httpd 1.3 mod_cgi argv[0]
[ Ok, trying this again as a subscriber... I guess the list mods missed it:-/ ] Hi, I've managed to tickle an obscure bug in Tcl's environment introspection by launching a 'starpack' (self contained Tcl executable+script) as a CGI. (*) here is the relevant bit of strace: 4763 execve(/usr/lib/cgi-bin/protect.cgi, [protect.cgi], [/* 25 vars */]) = 0 My question is this: why doesn't argv[0] get the full path of the file? It's not like the CGI can't figure it out from the cwd and the argv0 it does get. The Tcl folks are wondering why Apache does things this way, and I think it's a reasonable question. The problem for Tcl is that it wants to know the full path of the executable, which would, in this case, be /usr/lib/cgi-bin/protect.cgi, but it's not obvious how to work that out from just 'protect.cgi' in a generic way. It's easy enough to do if you know it's a CGI, but Tcl can't contain code just for CGI's without full-path argv0's. Thankyou, -- David N. Welton - http://www.dedasys.com/davidw/ Apache, Linux, Tcl Consulting - http://www.dedasys.com/ (*) http://sourceforge.net/tracker/index.php?func=detailaid=1224888group_id=10894atid=110894 -- David N. Welton - http://www.dedasys.com/davidw/ Apache, Linux, Tcl Consulting - http://www.dedasys.com/
Re: Accessing to per Directory configuration from an input filter: HOW?
Sorry if i disturb you but i still don't manage to solve my problems. Is it possible to alter cookie in the header_parser hook?? I don't mean to alter the value of the apr_table of the apache server that contains cookie values i really mean changing cookies in the request so that some underling (mostly java) web applications can se the right value for these special cookies. We are trying to reproduce the functioning of a security infracstructure developed originally for MS. It would really be a pity if we don't manage to do with apache what we alreay have in production with IIS. Thanks in advance for your attention. Luca On 6/22/05, Nick Kew [EMAIL PROTECTED] wrote: luca regini wrote: I need to write an input filter that is able to change the value of some cookies. However the name of the cookie to be changed is a per- directory value. So i have an input filter whose behavior depends programmatically from per-directory configuration. How should i gain information about per -dir configuration from within a filter?? Thanks in advance LucaSame as from anywhere else.But you don't want to do that in an input filter.Use the header_parserhook.Or, if it needs to happen before/after some other module whichviews cookies in a different hook, move as appropriate. --Nick Kew
Re: 2.1.5 available for testing
On 6/23/05, jean-frederic clere [EMAIL PROTECTED] wrote:
William A. Rowe, Jr. wrote:
++1 To Joe's comments.
Jeff's fix is technically right, but scares the nibbles out
of me. If, for example, an exploit is able to inject the
T-E on top of the legit C-L, I really suspect we should not
trust the origin server at all.
One important situation to fear is a buggy server or proxy+server that
we may not be able to talk to at all if we are extremely strict.
[As you implied w.r.t. Apache 1.3] The smuggling fear is really if we
allow keepalive on this connection to the origin server again. We
could be confused by making the wrong choice from {CL, TE} and
misunderstand the next response. We can set backend-close and
origin-keepalive to prevent that.
If we don't allow keepalive, then it is down to whether or not this
single request can be parsed correctly if our choice of {CL, TE} makes
sense.
For origin servers (as opposed to clients) make this choice
between ignore C-L, or fail, configurable?
try very hard to make a reasonable choice with no configuration :)
(speaking to the choir, of course)
My observation is that there are far more varied clients in
the world than servers, each with unique faults. But the
RFC 2616 is clear...
Messages MUST NOT include both a Content-Length header field and a
non-identity transfer-coding. If the message does include a non-
identity transfer-coding, the Content-Length MUST be ignored.
When a Content-Length is given in a message where a message-body is
allowed, its field value MUST exactly match the number of OCTETs in
the message-body. HTTP/1.1 user agents MUST notify the user when an
invalid length is received and detected.
...and server authors can be expected to be less buggy than clients.
Permissive in what we accept, strict in what we send implies some
strictness in what we trust to pass on to the client.
We're removing the protocol breakage in what we pass on to the client.
At this point, we either send a valid response or it is if the server
dropped the connection before sending the full response.
(I hear what you're screaming. I think the minimal-intervention path
should be preferred if we can justify it.)
So +.5 to Jeff's patch, and let's discuss if the proxy response should
then be trusted at all with T-E and C-L, in httpd-2.x where we support
keepalives.
Once the patch applied we lose the information that the request was
incorrect.
That means we won't be able to choose in proxy between sending C-L (and
dechunk)
and T-E.
I don't follow here. How does the backend choice of {TE, CL} affect
what we send the client?
Re: 2.1.5 available for testing
Jeff Trawick wrote:
On 6/23/05, jean-frederic clere [EMAIL PROTECTED] wrote:
William A. Rowe, Jr. wrote:
++1 To Joe's comments.
Jeff's fix is technically right, but scares the nibbles out
of me. If, for example, an exploit is able to inject the
T-E on top of the legit C-L, I really suspect we should not
trust the origin server at all.
One important situation to fear is a buggy server or proxy+server that
we may not be able to talk to at all if we are extremely strict.
[As you implied w.r.t. Apache 1.3] The smuggling fear is really if we
allow keepalive on this connection to the origin server again. We
could be confused by making the wrong choice from {CL, TE} and
misunderstand the next response. We can set backend-close and
origin-keepalive to prevent that.
If we don't allow keepalive, then it is down to whether or not this
single request can be parsed correctly if our choice of {CL, TE} makes
sense.
For origin servers (as opposed to clients) make this choice
between ignore C-L, or fail, configurable?
try very hard to make a reasonable choice with no configuration :)
(speaking to the choir, of course)
My observation is that there are far more varied clients in
the world than servers, each with unique faults. But the
RFC 2616 is clear...
Messages MUST NOT include both a Content-Length header field and a
non-identity transfer-coding. If the message does include a non-
identity transfer-coding, the Content-Length MUST be ignored.
When a Content-Length is given in a message where a message-body is
allowed, its field value MUST exactly match the number of OCTETs in
the message-body. HTTP/1.1 user agents MUST notify the user when an
invalid length is received and detected.
...and server authors can be expected to be less buggy than clients.
Permissive in what we accept, strict in what we send implies some
strictness in what we trust to pass on to the client.
We're removing the protocol breakage in what we pass on to the client.
At this point, we either send a valid response or it is if the server
dropped the connection before sending the full response.
(I hear what you're screaming. I think the minimal-intervention path
should be preferred if we can justify it.)
So +.5 to Jeff's patch, and let's discuss if the proxy response should
then be trusted at all with T-E and C-L, in httpd-2.x where we support
keepalives.
Once the patch applied we lose the information that the request was incorrect.
That means we won't be able to choose in proxy between sending C-L (and dechunk)
and T-E.
I don't follow here. How does the backend choice of {TE, CL} affect
what we send the client?
If we are acting as a proxy, what we send to the next proxy is changed by the
patch, isn't it?
Re: 2.1.5 available for testing
On 6/23/05, jean-frederic clere [EMAIL PROTECTED] wrote:
Jeff Trawick wrote:
On 6/23/05, jean-frederic clere [EMAIL PROTECTED] wrote:
William A. Rowe, Jr. wrote:
++1 To Joe's comments.
Jeff's fix is technically right, but scares the nibbles out
of me. If, for example, an exploit is able to inject the
T-E on top of the legit C-L, I really suspect we should not
trust the origin server at all.
One important situation to fear is a buggy server or proxy+server that
we may not be able to talk to at all if we are extremely strict.
[As you implied w.r.t. Apache 1.3] The smuggling fear is really if we
allow keepalive on this connection to the origin server again. We
could be confused by making the wrong choice from {CL, TE} and
misunderstand the next response. We can set backend-close and
origin-keepalive to prevent that.
If we don't allow keepalive, then it is down to whether or not this
single request can be parsed correctly if our choice of {CL, TE} makes
sense.
For origin servers (as opposed to clients) make this choice
between ignore C-L, or fail, configurable?
try very hard to make a reasonable choice with no configuration :)
(speaking to the choir, of course)
My observation is that there are far more varied clients in
the world than servers, each with unique faults. But the
RFC 2616 is clear...
Messages MUST NOT include both a Content-Length header field and a
non-identity transfer-coding. If the message does include a non-
identity transfer-coding, the Content-Length MUST be ignored.
When a Content-Length is given in a message where a message-body is
allowed, its field value MUST exactly match the number of OCTETs in
the message-body. HTTP/1.1 user agents MUST notify the user when an
invalid length is received and detected.
...and server authors can be expected to be less buggy than clients.
Permissive in what we accept, strict in what we send implies some
strictness in what we trust to pass on to the client.
We're removing the protocol breakage in what we pass on to the client.
At this point, we either send a valid response or it is if the server
dropped the connection before sending the full response.
(I hear what you're screaming. I think the minimal-intervention path
should be preferred if we can justify it.)
So +.5 to Jeff's patch, and let's discuss if the proxy response should
then be trusted at all with T-E and C-L, in httpd-2.x where we support
keepalives.
Once the patch applied we lose the information that the request was
incorrect.
That means we won't be able to choose in proxy between sending C-L (and
dechunk)
and T-E.
I don't follow here. How does the backend choice of {TE, CL} affect
what we send the client?
If we are acting as a proxy, what we send to the next proxy is changed by the
patch, isn't it?
This second patch is a bit confusing out of context because of the use
by that function of r-headers_OUT for information we have READ from
the origin server. It doesn't affect what we send to the next proxy
as far as I can tell.
The original patch could affect what we send to the next proxy.
Re: 2.1.5 available for testing
At 05:45 AM 6/23/2005, Jeff Trawick wrote:
On 6/23/05, jean-frederic clere [EMAIL PROTECTED] wrote:
William A. Rowe, Jr. wrote:
++1 To Joe's comments.
Jeff's fix is technically right, but scares the nibbles out
of me. If, for example, an exploit is able to inject the
T-E on top of the legit C-L, I really suspect we should not
trust the origin server at all.
If we don't allow keepalive, then it is down to whether or not this
single request can be parsed correctly if our choice of {CL, TE} makes
sense.
So close the proxy connection if C-L and T-E are returned from the
origin server? That would upgrade my +.5 to +1 - I totally agree.
For origin servers (as opposed to clients) make this choice
between ignore C-L, or fail, configurable?
try very hard to make a reasonable choice with no configuration :)
(speaking to the choir, of course)
Yup - that was my concern too.
Once the patch applied we lose the information that the request was
incorrect.
That means we won't be able to choose in proxy between sending C-L (and
dechunk)
and T-E.
I don't follow here. How does the backend choice of {TE, CL} affect
what we send the client?
I really didn't understand that either. The RFC really doesn't offer
any choice of how to interpret T-E and C-L (it states, ignore (drop)
the C-L header. And we will pass the response to the client however
we agreed.
Bill
Re: 2.1.5 available for testing
On 6/23/05, William A. Rowe, Jr. [EMAIL PROTECTED] wrote:
At 05:45 AM 6/23/2005, Jeff Trawick wrote:
On 6/23/05, jean-frederic clere [EMAIL PROTECTED] wrote:
William A. Rowe, Jr. wrote:
++1 To Joe's comments.
Jeff's fix is technically right, but scares the nibbles out
of me. If, for example, an exploit is able to inject the
T-E on top of the legit C-L, I really suspect we should not
trust the origin server at all.
If we don't allow keepalive, then it is down to whether or not this
single request can be parsed correctly if our choice of {CL, TE} makes
sense.
So close the proxy connection if C-L and T-E are returned from the
origin server? That would upgrade my +.5 to +1 - I totally agree.
Cool... I'm working on a code change and a test for this...
Re: apache developers documentation!!!
On Wednesday 22 June 2005 17:51, Joshua Slive wrote: Note that http://httpd.apache.org/docs-2.0/developer/ is supposed to be the canonical location for developer docs and links. Nick, could you add a link to your site? Done. Thanks for the suggestion. I guess I didn't originally, because I started the apachetutor site before getting commit, and didn't want to bug people and come across as a link to me wannabe:-) -- Nick Kew
REQUEST_CHUNKED_PASS Q for 2.x TraceEnable [on|off|extended]
Does anyone see a reason not to resurrect REQUEST_CHUNKED_PASS? The 'TraceEnable extended' semantics include the entire body handshaking for bodies 64kb (rejecting larger bodies as memory hogs). The body = REQUEST_CHUNKED_PASS; line ensures that the user sees the entire conversation. This appears gone from 2.x, but it remains useful if unfiltered. In fact, using some sort of metadata bucket with text, flagging the bucket as a chunk header, makes a certain sort of sense to me. I'll work up a patch this weekend if there is no objection. Bill
Rev 2: [PATCH] 1.3 TraceEnable [on|off|extended]
The patch, in final form, tested and works for T-E with C-L body, T-E with C-L body, C-L only, T-E only and no body. It correctly denies proxy TRACE with a body by default, and will deny all TRACE requests for 'TraceEnable off'. Votes please, before I invest in patching 2.x? A related message r.e. 2.x to follow. Bill At 01:30 AM 6/23/2005, William A. Rowe, Jr. wrote: The attached patch resolved the issue I noted below, 10.4.6 405 Method Not Allowed requires an Allow header (I would presume, even if empty, based on #() grammar), while 10.5.2 501 Not Implemented states; This is the appropriate response when the server does not recognize the request method and is not capable of supporting it for any resource. If 'ProxyEnable off' is set for a given host, the setting is url-impotent, and does not vary. Because the patch does append a new member to the core_server_config structure, it seems a minor bump is in order. At 12:52 PM 6/22/2005, William A. Rowe, Jr. wrote: FYI there is one small issue still. The resulting Allow: null response to denied TRACE request. TRACE doesn't go through the normal processing, so methods aren't added. And since TRACE is denied, it's removed too. At 08:56 AM 6/22/2005, William A. Rowe, Jr. wrote: I've spent a large number of cycles investigating the Watchfire report (http://www.watchfire.com/resources/HTTP-Request-Smuggling.pdf) and come up with a genuine reason to adopt the attached patch. ... So the attached patch introduces the per-host directive TraceEnable on|off|extended where extended permits a message body, up to 64kb at the target server, and of an unlimited size through a proxy server. The default remains 'on', of course, denying a TRACE body request even via proxy. Following the semantics of TRACE, the request body is returned to the host verbatim as part of the response, following the headers, exactly as sent. httpd-1.3-trace-rev2.patch Description: Binary data
Re: 2.1.5 available for testing
On 6/23/05, Jeff Trawick [EMAIL PROTECTED] wrote:
On 6/23/05, William A. Rowe, Jr. [EMAIL PROTECTED] wrote:
At 05:45 AM 6/23/2005, Jeff Trawick wrote:
On 6/23/05, jean-frederic clere [EMAIL PROTECTED] wrote:
William A. Rowe, Jr. wrote:
++1 To Joe's comments.
Jeff's fix is technically right, but scares the nibbles out
of me. If, for example, an exploit is able to inject the
T-E on top of the legit C-L, I really suspect we should not
trust the origin server at all.
If we don't allow keepalive, then it is down to whether or not this
single request can be parsed correctly if our choice of {CL, TE} makes
sense.
So close the proxy connection if C-L and T-E are returned from the
origin server? That would upgrade my +.5 to +1 - I totally agree.
Cool... I'm working on a code change and a test for this...
Even with plenty of caffeine I'm lost on how to get 2.1-dev proxy to
try to keep backend connection. Some sort of configuration is now
required? Backing down to 2.0.x for now.
[PATCH] strip C-L when origin server sends both T-E and C-L
I've attached patches for both trunk and 2.0.x. I was unable to
confirm that the trunk change drops the backend connection, probably
because of a basic user error when testing*. No issues verifying that
with 2.0 though.
*if the existing code earlier in the function to drop the backend
connection works, then this should too ;)
Index: modules/proxy/proxy_http.c
===
--- modules/proxy/proxy_http.c (revision 193172)
+++ modules/proxy/proxy_http.c (working copy)
@@ -1201,8 +1201,24 @@
return r-status;
} else {
+const char *buf;
+
+/* can't have both Content-Length and Transfer-Encoding */
+if (apr_table_get(r-headers_out, Transfer-Encoding)
+ apr_table_get(r-headers_out, Content-Length)) {
+/* 2616 section 4.4, point 3: if both Transfer-Encoding
+ * and Content-Length are received, the latter MUST be
+ * ignored; so unset it here to prevent any confusion
+ * later. */
+apr_table_unset(r-headers_out, Content-Length);
+ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
+ r-server,
+ proxy: server %s returned Transfer-Encoding
and Content-Length,
+ p_conn-name);
+p_conn-close += 1;
+}
+
/* strip connection listed hop-by-hop headers from response */
-const char *buf;
p_conn-close += ap_proxy_liststr(apr_table_get(r-headers_out,
Connection),
close);
Index: modules/proxy/mod_proxy_http.c
===
--- modules/proxy/mod_proxy_http.c (revision 193129)
+++ modules/proxy/mod_proxy_http.c (working copy)
@@ -1128,7 +1128,22 @@
r-headers_out,
save_table);
}
-
+
+/* can't have both Content-Length and Transfer-Encoding */
+if (apr_table_get(r-headers_out, Transfer-Encoding)
+ apr_table_get(r-headers_out, Content-Length)) {
+/* 2616 section 4.4, point 3: if both Transfer-Encoding
+ * and Content-Length are received, the latter MUST be
+ * ignored; so unset it here to prevent any confusion
+ * later. */
+apr_table_unset(r-headers_out, Content-Length);
+ap_log_error(APLOG_MARK, APLOG_DEBUG, 0,
+ r-server,
+ proxy: server %s returned Transfer-Encoding
and Content-Length,
+ backend-hostname);
+backend-close += 1;
+}
+
/* strip connection listed hop-by-hop headers from response */
backend-close +=
ap_proxy_liststr(apr_table_get(r-headers_out,
Connection),
Dynamic Configuration Module using Shared memory
Hi I am writing a module in which i am using my own configuration xml file. I provide the functionality of dynamic configuration. user can change the XML file and pass it as special request to my module to update the configuration. It works fine for non prefork module, but with the prefork MPM i use shared memory to store the DOM tree. The shared memory makes the traversing of the DOM tree difficult. Is their any other approach that i should work on ? Is it a good idea to store in shared memory ? One other issue is that, at the time of initialization of the module, i parse the xml file and create a shared memory of the size of the parsed object. Now, when it is updated, the updated xml configuration file size is larger than the shared memory allocated. In this case, i destroy the old shm and create a new shm with larger size and attach all the processes to it. Is it a good idea to implement it this way ? Please let me know, if their is a better way. Thanks in advance for your help. Regards Mihir Mohan -- There is nothing more difficult to take in hand, more perilous to conduct, or more uncertain in its success, than to take the lead in the introduction of a new order of things. Machiavelli in 'The Prince'
[PATCH] disable keepalive when both T-E and C-L received from client (or proxy)
Index: server/protocol.c
===
--- server/protocol.c (revision 194460)
+++ server/protocol.c (working copy)
@@ -906,6 +906,7 @@
* ignored; so unset it here to prevent any confusion
* later. */
apr_table_unset(r-headers_in, Content-Length);
+conn-keepalive = AP_CONN_CLOSE;
}
}
else {
Index: modules/http/http_core.c
===
--- modules/http/http_core.c(revision 194460)
+++ modules/http/http_core.c(working copy)
@@ -116,9 +116,9 @@
while (cs-state == CONN_STATE_READ_REQUEST_LINE) {
ap_update_child_status(c-sbh, SERVER_BUSY_READ, NULL);
+c-keepalive = AP_CONN_UNKNOWN;
if ((r = ap_read_request(c))) {
-c-keepalive = AP_CONN_UNKNOWN;
/* process the request if it was read without error */
ap_update_child_status(c-sbh, SERVER_BUSY_WRITE, r);
@@ -162,9 +162,9 @@
*/
ap_update_child_status(c-sbh, SERVER_BUSY_READ, NULL);
+c-keepalive = AP_CONN_UNKNOWN;
while ((r = ap_read_request(c)) != NULL) {
-c-keepalive = AP_CONN_UNKNOWN;
/* process the request if it was read without error */
ap_update_child_status(c-sbh, SERVER_BUSY_WRITE, r);
Potential deadlock in psp.py
I think I just spotted a potential deadlock in psp.py. def dbm_cache_store(srv, dbmfile, filename, mtime, val): dbm_type = dbm_cache_type(dbmfile) _apache._global_lock(srv, pspcache) try: dbm = dbm_type.open(dbmfile, 'c') dbm[filename] = %d %s % (mtime, code2str(val)) finally: try: dbm.close() except: pass _apache._global_unlock(srv, pspcache) pspcache will hash to one of 31 mutexes. Therefore there is a 1 in 31 chance for a hash collision if a session is used in the same request, which would result in a deadlock. I'm sure there are other possible deadlock scenarios where some combination of sessions and pspcache could end up trying to hold the same mutex. Most obvious solution is to use the global lock 0, which will serialize all accesses to either pspcache.dbm (and the dbmsession dbm in the case where DbmSession is being used). This will result in an obvious performance hit when psp are used in conjunction with DbmSession, but that's better than a deadlock. _apache._global_lock(srv, None, 0) The alternative would be to reserve a mutex for pspcache. If you agree that this is a possible deadlock let me know and I'll make the simple fix. We could also take another look at the bsddb transaction handling discussed last week in my Session benchmark post, and avoid using the mutex altogether. Regards, Jim
[PATCH] htdbm verify password field separator
One character fix to the verify password function to split the value (pw hash:group:comment) field in the DBM by the same field separator used when we stored it (Separator fixed from semicolon to colon in revision 101946) Index: support/htdbm.c === --- support/htdbm.c (revision 199711) +++ support/htdbm.c (working copy) @@ -226,7 +226,7 @@ if (apr_dbm_fetch(htdbm-dbm, key, val) != APR_SUCCESS) return APR_ENOENT; rec = apr_pstrndup(htdbm-pool, val.dptr, val.dsize); -cmnt = strchr(rec, ';'); +cmnt = strchr(rec, ':'); if (cmnt) strncpy(pwd, rec, cmnt - rec); else testcase: support/htdbm -ctb ~/dbm myuser mypass a comment support/htdbm -vb ~/dbm myuser mypass -- Eric Covener [EMAIL PROTECTED] htdbm_verify.diff Description: Binary data
Re: [PATCH] htdbm verify password field separator
On 6/23/05, Eric Covener [EMAIL PROTECTED] wrote: One character fix to the verify password function to split the value (pw hash:group:comment) field in the DBM by the same field separator used when we stored it committed, thanks!
[PATCH] htdbm group support
(An earlier patch I sent included changing htdbm to use apr_getopt -- this one retains the style of htdbm in the new parameter) This patch adds support for creating combined AuthDBMUserFile / AuthDMBGroupFile dbm files of the format: key=username, value=encyryptedPass:group1,group2:ignored comment This is the same format dbmmanage uses. There isn't a straightforward way in htdbm to create key=user,value=group AuthDBMGroupFiles previously or with this patch. In current htdbm, you can only get groups in a roundabout way by specifying the -t (comment) option and including an embedded colon in the comment field.This is unintended consequence is removed with patch. This patch adds a -g (groups) option and disallows colons in the string representing the groups (but allows them in comments). For example In past versions a comment of group1:my comment would be interpreted by mod_authz_dbm as being a member of group1 -- this patch makes sure the group field is empty if not specified (and allows a list of groups to be specified on the command line) -- Eric Covener [EMAIL PROTECTED] htdbmgroups-trunk-3.diff Description: Binary data
Re: Rev 2: [PATCH] 1.3 TraceEnable [on|off|extended]
On Jun 23, 2005, at 6:53 AM, William A. Rowe, Jr. wrote: The patch, in final form, tested and works for T-E with C-L body, T-E with C-L body, C-L only, T-E only and no body. It correctly denies proxy TRACE with a body by default, and will deny all TRACE requests for 'TraceEnable off'. Votes please, before I invest in patching 2.x? The correct response code to send is 403 Forbidden, not 405, since the method is being handled. That will simplify your patch considerably. I have submitted a change request for s/MUST/MAY/ in the text of 2616's definition of 405, since the Allow header field should not be required. Roy
httpd 1.3 mod_cgi argv[0]
[ Please CC replies to me - thankyou! ] Hi, I've managed to tickle an obscure bug in Tcl's environment introspection by launching a 'starpack' (self contained Tcl executable+script) as a CGI. (*) here is the relevant bit of strace: 4763 execve(/usr/lib/cgi-bin/protect.cgi, [protect.cgi], [/* 25 vars */]) = 0 My question is this: why doesn't argv[0] get the full path of the file? It's not like the CGI can't figure it out from the cwd and the argv0 it does get. The Tcl folks are wondering why Apache does things this way, and I think it's a reasonable question. Thankyou, -- David N. Welton - http://www.dedasys.com/davidw/ Apache, Linux, Tcl Consulting - http://www.dedasys.com/ (*) http://sourceforge.net/tracker/index.php?func=detailaid=1224888group_id=10894atid=110894
Re: [jira] Created: (MODPYTHON-60) PythonOption directive causes memory leak
Anybody had a chance to look at this yet?
I'm kind of suspicous of python_merge_config in mod_python.c. It gets
called once for every request, and calls apr_pcalloc which I assume
allocates some memory from the pool.
py_config *merged_conf =
(py_config *) apr_pcalloc(p, sizeof(py_config));
I don't see where or how this memory gets freed. I've taken a look at
the apache src code but get lost pretty quick.
Regards,
Jim
Jim Gallacher (JIRA) wrote:
PythonOption directive causes memory leak
-
Key: MODPYTHON-60
URL: http://issues.apache.org/jira/browse/MODPYTHON-60
Project: mod_python
Type: Bug
Components: core
Versions: 3.1.4, 3.1.3, 3.2.0
Environment: Linux
Reporter: Jim Gallacher
Priority: Critical
This was previously reported on the mod_python mailing list. See
http://www.modpython.org/pipermail/mod_python/2004-April/015395.html
A memory leak results when there is a PythonOption directive in the apache config file.
Leak occurs when PythonOption is in either VirtualHost or Directory
section.
For each request, approx 25 bytes of memory is leaked per PythonOption
directive.
Methodolgy (using top to gauge memory usage, 100,000 requests per test case):
def handler(req):
req.content_type = 'text/plain'
req.write('PythonOption test\n')
return apache.OK
1. No PythonOption directives:
1.4 % MEM
2. 50 PythonOption directives:
11.3% MEM
3. 100 PythonOption directives:
25.4 % MEM
I know 50 or 100 PythonOptions is not likely in a production system, but it
clearly demonstrate the leak.
