Re: svn commit: r546531 - in /tomcat/connectors/trunk/jk/native: apache-1.3/mod_jk.c apache-2.0/mod_jk.c common/jk_global.h common/jk_url.c common/jk_url.h common/list.mk.in
Sorry for being offline for 2 weeks :( Sorry for the long post coming :(( I just now read the URL rewriting discussion. Although the discussion seems to have come to an end, I find it worthwhile to actually add some structure to it: 1) How to forward the URL I already suggested a different forwarding default in my discussion 3 weeks ago. It was very close to what we ended up now and there are only minor points to decide upon. Since Tomcat does an additional decoding we have to prevent double decoding in order to ensure correct URL mapping (independent of security; an encoded percent is legitimate and needs to be handled correctly as a percent at the end). My original proposal included only reencoding percent signs (and maybe spaces), because all other URL characters will not get double decoded by Tomcat. If we don't trust this assumption, we could of course reencode a lot more characters, like mod_proxy does. There will be some performance penalty, because we could check pretty fast, if there are '%' or ' ' characters in the URL, and in case they are not, there's nothing to do in my proposal. mod_proxy has a far longer list of chars to reencode and so it doesn't do checking and instead runs the reencode loop directly. The mod_proxy people have a good reason for encoding more characters: the same code gets used for http, ftp and ajp forwarding. We could be able to know, how the AJP connector works (do we?), so we could choose to not reencode for instance characters above 127. But Remy made a good point: different components might have different decoding settings. I'm not sure, if this would apply to percent encoding, but it gives my thoughts a litle warning. At the moment there exist the following implementations: - Compat: forward the decoded URI, security issues - CompatUnparsed: forward the original request URI, breaks mod_rewrite - Proxy: Reencode using mod_proxy "full" reencoding (save for semicolon, i.e. URL session encoding), secure, adds some performance penalty - Escaped: Reencode fully, safe but breaks URL encoded sessions, - Minimal (the one I suggested): OK with mod_rewrite and sessions, a little faster than Proxy, but maybe open to not yet thought upon security threats, maybe also problematic if mod_jk and the backend use different character sets (I don't know if AJP URLs would be binary compatible for chars above 128 between mod_jk and the backend). All in all I also vote for "Proxy" as the new default, maybe with a little optimization by first checking the URL, if there are actual characters from the reencoding set in there (in most cases there won't, and we don't need to copy character by character). I strongly vote for leaving the old options in there for 1.2. Because for a long time default configurations had one of the old options as an explicit parameter in them (so they are not using the default), we should add a warning on startup, that the option is deprecated and removing it will most likely make the configuration safer and more interoperable with different environments. I would still allow using it. 2) Mladen's patch The core idea of Mladen's patch was to check the URL against a known classical way of breaking the combination mod_jk and Tomcat. Even after fixing this breakage by re-encoding the URL before forwarding, I find it worth of thinking about his patch. Apache httpd for instance has a parameter to *allow* URLs with encoded slashes which is by default *off*: AllowEncodedSlashes Off This disables some suspicious URLs stopps many of the standard break in attempts. So at least I see some reason for options to make the combination more sensible to evil URLs. Encoded slashes as well as encoded backslashes or encoded percent signs might be good candidates. Since those URL parts can have legitimate use, those can only be options, and one can discuss, if the default should be on or off. They will help to harden the security. Since mod_jk is for multiple web servers and maybe backends, there is some reason to actually implement those checks, even if some combinations might do it in front or behind mod_jk. I would propose a check for encoded slashes, backslashes and encoded percent signs, combined in one option "DenyUnsafeURL", which in 1.2 will be off by default. Room for discussion here! 3) IIS, Netscape We need to investigate, how we forward from IIS and Netscape. For IIS, Mladen will know. Netscape uses decoded URL like Apache, so we'll switch to the same handling as the new default in Apache. Regards, Rainer Remy Maucherat wrote: Mladen Turk wrote: That's why I suggested to stop for a while and see all the possibilities. We've talked about it for a while (see the length of this thread ...), and I consider it is time to think over code: apply the proposed patch which aims to harmonize with mod_proxy, and see what actual problems it causes, if any. This means I am vetoing r54413
Re: svn commit: r546531 - in /tomcat/connectors/trunk/jk/native: apache-1.3/mod_jk.c apache-2.0/mod_jk.c common/jk_global.h common/jk_url.c common/jk_url.h common/list.mk.in
Mladen Turk wrote: That's why I suggested to stop for a while and see all the possibilities. We've talked about it for a while (see the length of this thread ...), and I consider it is time to think over code: apply the proposed patch which aims to harmonize with mod_proxy, and see what actual problems it causes, if any. This means I am vetoing r544137 (the patch was an emergency fix with some side effects, and needs to be replaced with a proper implementation). Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r546531 - in /tomcat/connectors/trunk/jk/native: apache-1.3/mod_jk.c apache-2.0/mod_jk.c common/jk_global.h common/jk_url.c common/jk_url.h common/list.mk.in
Remy Maucherat wrote: Mladen Turk wrote: Why? Let's stop a bit and test things before. Jean-Frédéric proposes implementing the same behavior as mod_proxy, so I don't see how this can be a bad thing. First of all I didn't said it's a bad thing or anything like that. We need the same behavior on Apache and IIS (at least), so this patch needs to be tested on IIS and Netscape as well. Also I think we should consider and discuss do we really need extra directive or it can be done by using existing or default. That's why I suggested to stop for a while and see all the possibilities. Regards, Mladen. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r546531 - in /tomcat/connectors/trunk/jk/native: apache-1.3/mod_jk.c apache-2.0/mod_jk.c common/jk_global.h common/jk_url.c common/jk_url.h common/list.mk.in
Mladen Turk wrote: Why? Let's stop a bit and test things before. Jean-Frédéric has of course done extended testing before proposing this :) The original patch was meant to close the "security problem" as soon as possible, but in the end has a bad behavior and should be reverted. Jean-Frédéric proposes implementing the same behavior as mod_proxy, so I don't see how this can be a bad thing. Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r546531 - in /tomcat/connectors/trunk/jk/native: apache-1.3/mod_jk.c apache-2.0/mod_jk.c common/jk_global.h common/jk_url.c common/jk_url.h common/list.mk.in
On Tue, 2007-06-12 at 19:50 +0200, Mladen Turk wrote: > Jean-Frederic wrote: > >>> Add ForwardURIProxy to the URl handling option. > >>> common/jk_url.c is just a porting of the routines > >>> from proxy_util.c (Apache httpd). > >> After quite a few discussions, I think this should be the only mode > available for URI handling, as the two others are broken. > >> > >> Comments ? > > > > Additionaly I want to rollback r544137 too. > > > > Why? To reach the following: url file/dir TC Compat Proxy Proxy-r544137 %252007%2007 ok no okok %252E%252E %2E%2E ok no nook Of course using Compat-r544137 would reopen the vulnerability. Cheers Jean-Frederic > Let's stop a bit and test things before. > > Regards, > Mladen. > > > - > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r546531 - in /tomcat/connectors/trunk/jk/native: apache-1.3/mod_jk.c apache-2.0/mod_jk.c common/jk_global.h common/jk_url.c common/jk_url.h common/list.mk.in
Mladen Turk wrote: > Jean-Frederic wrote: Add ForwardURIProxy to the URl handling option. common/jk_url.c is just a porting of the routines from proxy_util.c (Apache httpd). >>> After quite a few discussions, I think this should be the only mode > available for URI handling, as the two others are broken. I was coming to the same conclusion but want to test some more first before +1'ing this idea > Let's stop a bit and test things before. +1. Now all I need is some time... ;) Mark - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r546531 - in /tomcat/connectors/trunk/jk/native: apache-1.3/mod_jk.c apache-2.0/mod_jk.c common/jk_global.h common/jk_url.c common/jk_url.h common/list.mk.in
Jean-Frederic wrote: >>> Add ForwardURIProxy to the URl handling option. >>> common/jk_url.c is just a porting of the routines >>> from proxy_util.c (Apache httpd). >> After quite a few discussions, I think this should be the only mode available for URI handling, as the two others are broken. >> >> Comments ? > > Additionaly I want to rollback r544137 too. > Why? Let's stop a bit and test things before. Regards, Mladen. - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r546531 - in /tomcat/connectors/trunk/jk/native: apache-1.3/mod_jk.c apache-2.0/mod_jk.c common/jk_global.h common/jk_url.c common/jk_url.h common/list.mk.in
On Tue, 2007-06-12 at 17:50 +0200, Remy Maucherat wrote: > [EMAIL PROTECTED] wrote: > > Author: jfclere > > Date: Tue Jun 12 08:32:10 2007 > > New Revision: 546531 > > > > URL: http://svn.apache.org/viewvc?view=rev&rev=546531 > > Log: > > Add ForwardURIProxy to the URl handling option. > > common/jk_url.c is just a porting of the routines > > from proxy_util.c (Apache httpd). > > After quite a few discussions, I think this should be the only mode > available for URI handling, as the two others are broken. > > Comments ? Additionaly I want to rollback r544137 too. Cheers Jean-Frederic > > Rémy > > - > To unsubscribe, e-mail: [EMAIL PROTECTED] > For additional commands, e-mail: [EMAIL PROTECTED] > - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
Re: svn commit: r546531 - in /tomcat/connectors/trunk/jk/native: apache-1.3/mod_jk.c apache-2.0/mod_jk.c common/jk_global.h common/jk_url.c common/jk_url.h common/list.mk.in
[EMAIL PROTECTED] wrote: Author: jfclere Date: Tue Jun 12 08:32:10 2007 New Revision: 546531 URL: http://svn.apache.org/viewvc?view=rev&rev=546531 Log: Add ForwardURIProxy to the URl handling option. common/jk_url.c is just a porting of the routines from proxy_util.c (Apache httpd). After quite a few discussions, I think this should be the only mode available for URI handling, as the two others are broken. Comments ? Rémy - To unsubscribe, e-mail: [EMAIL PROTECTED] For additional commands, e-mail: [EMAIL PROTECTED]
svn commit: r546531 - in /tomcat/connectors/trunk/jk/native: apache-1.3/mod_jk.c apache-2.0/mod_jk.c common/jk_global.h common/jk_url.c common/jk_url.h common/list.mk.in
Author: jfclere
Date: Tue Jun 12 08:32:10 2007
New Revision: 546531
URL: http://svn.apache.org/viewvc?view=rev&rev=546531
Log:
Add ForwardURIProxy to the URl handling option.
common/jk_url.c is just a porting of the routines
from proxy_util.c (Apache httpd).
Added:
tomcat/connectors/trunk/jk/native/common/jk_url.c
tomcat/connectors/trunk/jk/native/common/jk_url.h
Modified:
tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c
tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c
tomcat/connectors/trunk/jk/native/common/jk_global.h
tomcat/connectors/trunk/jk/native/common/list.mk.in
Modified: tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c
URL:
http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c?view=diff&rev=546531&r1=546530&r2=546531
==
--- tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c (original)
+++ tomcat/connectors/trunk/jk/native/apache-1.3/mod_jk.c Tue Jun 12 08:32:10
2007
@@ -60,6 +60,7 @@
#include "jk_uri_worker_map.h"
#include "jk_ajp13.h"
#include "jk_shm.h"
+#include "jk_url.h"
#define JK_LOG_DEF_FILE ("logs/mod_jk.log")
#define JK_SHM_DEF_FILE ("logs/jk-runtime-status")
@@ -519,6 +520,7 @@
request_rec *r = private_data->r;
char *ssl_temp = NULL;
s->route = NULL;/* Used for sticky session routing */
+int size;
/* Copy in function pointers (which are really methods) */
s->start_response = ws_start_response;
@@ -627,6 +629,13 @@
s->req_uri = r->uri;
break;
+case JK_OPT_FWDURIPROXY:
+size = strlen(r->uri);
+s->req_uri = ap_palloc(r->pool, size * 3 + 1);
+jk_canonenc(s->req_uri, r->uri, size, enc_path, 0,
+JK_PROXYREQ_REVERSE);
+break;
+
case JK_OPT_FWDURIESCAPED:
s->req_uri = ap_escape_uri(r->pool, r->uri);
break;
@@ -1725,6 +1734,10 @@
}
else if (!strcasecmp(w, "ForwardURIEscaped")) {
opt = JK_OPT_FWDURIESCAPED;
+mask = JK_OPT_FWDURIMASK;
+}
+else if (!strcasecmp(w, "ForwardURIProxy")) {
+opt = JK_OPT_FWDURIPROXY;
mask = JK_OPT_FWDURIMASK;
}
else if (!strcasecmp(w, "ForwardDirectories")) {
Modified: tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c
URL:
http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c?view=diff&rev=546531&r1=546530&r2=546531
==
--- tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c (original)
+++ tomcat/connectors/trunk/jk/native/apache-2.0/mod_jk.c Tue Jun 12 08:32:10
2007
@@ -102,6 +102,7 @@
#include "jk_util.h"
#include "jk_worker.h"
#include "jk_shm.h"
+#include "jk_url.h"
#define JK_LOG_DEF_FILE ("logs/mod_jk.log")
#define JK_SHM_DEF_FILE ("logs/jk-runtime-status")
@@ -539,6 +540,7 @@
request_rec *r = private_data->r;
char *ssl_temp = NULL;
+int size;
s->route = NULL;/* Used for sticky session routing */
/* Copy in function pointers (which are really methods) */
@@ -655,6 +657,13 @@
s->req_uri = r->uri;
break;
+case JK_OPT_FWDURIPROXY:
+size = strlen(r->uri);
+s->req_uri = apr_palloc(r->pool, size * 3 + 1);
+jk_canonenc(s->req_uri, r->uri, size, enc_path, 0,
+JK_PROXYREQ_REVERSE);
+break;
+
case JK_OPT_FWDURIESCAPED:
s->req_uri = ap_escape_uri(r->pool, r->uri);
break;
@@ -1758,6 +1767,10 @@
}
else if (!strcasecmp(w, "ForwardURIEscaped")) {
opt = JK_OPT_FWDURIESCAPED;
+mask = JK_OPT_FWDURIMASK;
+}
+else if (!strcasecmp(w, "ForwardURIProxy")) {
+opt = JK_OPT_FWDURIPROXY;
mask = JK_OPT_FWDURIMASK;
}
else if (!strcasecmp(w, "ForwardDirectories")) {
Modified: tomcat/connectors/trunk/jk/native/common/jk_global.h
URL:
http://svn.apache.org/viewvc/tomcat/connectors/trunk/jk/native/common/jk_global.h?view=diff&rev=546531&r1=546530&r2=546531
==
--- tomcat/connectors/trunk/jk/native/common/jk_global.h (original)
+++ tomcat/connectors/trunk/jk/native/common/jk_global.h Tue Jun 12 08:32:10
2007
@@ -228,15 +228,16 @@
* JK options
*/
-#define JK_OPT_FWDURIMASK 0x0003
+#define JK_OPT_FWDURIMASK 0x0007
#define JK_OPT_FWDURICOMPAT 0x0001
#define JK_OPT_FWDURICOMPATUNPARSED 0x0002
#define JK_OPT_FWDURIESCAPED0x0003
+#define JK_OPT_FWDURIPROXY 0x0004
-#define JK_OPT_FWDURIDEFAULTJK_OPT_FWDURICOMPAT
+#define JK_OPT_FWDURIDEFAULTJK_OPT_FWDURIPROXY
-#define JK_OPT_FWDKEYSIZE 0x0004
+#define JK_OPT_FWDKEYSIZE 0x0200
#define JK_OPT
