Re: mod_dav inconsistent behaviour for GET requests
On Sunday 31 January 2010, Justin Erenkrantz wrote: On Sun, Jan 31, 2010 at 3:01 AM, Stefan Fritsch s...@sfritsch.de wrote: On Sat, 30 Jan 2010, Justin Erenkrantz wrote: I don't see how your patch would intentionally break - the failure mechanism is that the source scripts are served - not that a configuration error stops the server from running. -- justin Surely a fatal server error is not a necessary condition for something to be broken? When it has the capability of exposing source that would not otherwise be served, absolutely. The fundamental flaw with this patch is that dav_fixups runs after core_override_type - so the none handler simply won't get reassigned by the rest of the applicable configs - ie set to CGI or PHP or whatnot. So, it would simply fall through and go to the default handler. Ouch. -- justin That's exactly what the patch is supposed to do. Therefore I would not call it flawed. I think that the auth changes from 2.2 to trunk are so large that anyone upgrading will have to carefully review and test his configuration for security problems anyway. An additional change in the behaviour of mod_dav wouldn't create much of an additional problem (if it is documented correctly). But since quite a few people disagree with me here, an alternative could be an additional directive (or second argument to 'Dav') that allows to configure the behaviour. For example DavHandleMethods all DavHandleMethods exceptPOST DavHandleMethods exceptGET,POST For 2.4, one could then leave the default at exceptGET,POST / exceptPOST (depending on the dav provider), just like it is for 2.2.x. But if the user does not specify DavHandleMethods explicitly, httpd could log a notice saying: DavHandleMethods defaulting to 'exceptGET,POST'. The default will change to 'all' with the next major release of httpd. Please specify DavHandleMethods explicitly. Or one could even make the new directive mandatory, with httpd refusing to start without it. Would this address your concerns? Cheers, Stefan
Re: mod_dav inconsistent behaviour for GET requests
On Sat, 30 Jan 2010, Justin Erenkrantz wrote: I don't see how your patch would intentionally break - the failure mechanism is that the source scripts are served - not that a configuration error stops the server from running. -- justin Surely a fatal server error is not a necessary condition for something to be broken? But let's put it in a different way: It's a matter of precedence of different configuration directives, on the one hand AddHandler/AddType/SetHandler, on the other hand 'Dav'. The current behaviour is this: 'Dav' takes precedence over AddHandler/AddType/SetHandler for all methods except GET and POST. Except if the Dav provider handles GET by itself, then 'Dav' takes precedence over AddHandler/AddType/SetHandler for all methods except POST. I think this is illogical, difficult to understand, and makes debugging problems harder than necessary. IMHO, 'Dav' should take precedence for all methods (though my patch doesn't touch POST yet). Cheers, Stefan
Re: mod_dav inconsistent behaviour for GET requests
On Sun, Jan 31, 2010 at 3:01 AM, Stefan Fritsch s...@sfritsch.de wrote: On Sat, 30 Jan 2010, Justin Erenkrantz wrote: I don't see how your patch would intentionally break - the failure mechanism is that the source scripts are served - not that a configuration error stops the server from running. -- justin Surely a fatal server error is not a necessary condition for something to be broken? When it has the capability of exposing source that would not otherwise be served, absolutely. The fundamental flaw with this patch is that dav_fixups runs after core_override_type - so the none handler simply won't get reassigned by the rest of the applicable configs - ie set to CGI or PHP or whatnot. So, it would simply fall through and go to the default handler. Ouch. -- justin
Re: mod_dav inconsistent behaviour for GET requests
On Saturday 30 January 2010, Roy T. Fielding wrote:
*/
if (!conf-provider-repos-handle_get) {
+if (r-finfo.filetype != APR_DIR)
+r-handler = none;
return DECLINED;
}
}
It looks to me like that would introduce a security hole for
existing configs that expect a handler to run on GET (PHP/CGI
scripts that are authorable via DAV). -1 if so.
The recommended setup is to map separate URLs for DAV and script
execution to the content. It has been like this since at least 2.0.
The patch intentionally breaks existing configs that rely on the
ability to use the same URLs for DAV and script execution. Is this not
an acceptable change from 2.2 to 2.4 (if properly documented), as it
makes life a lot easier for people who use the recommended setup?
Re: mod_dav inconsistent behaviour for GET requests
On 30 Jan 2010, at 12:04 PM, Stefan Fritsch wrote: The recommended setup is to map separate URLs for DAV and script execution to the content. It has been like this since at least 2.0. The patch intentionally breaks existing configs that rely on the ability to use the same URLs for DAV and script execution. Is this not an acceptable change from 2.2 to 2.4 (if properly documented), as it makes life a lot easier for people who use the recommended setup? I don't follow how this makes it easier to use the recommended setup? In your example config, you defined /dav as being handled by mod_dav, and then you defined a FilesMatch (as I recall) that defined mod_php to be used by all URLs in the complete URL space that ended with .php. In so doing you're creating two configs that both overlap and contradict themselves, and this is specifically discouraged by the recommended setup. I also don't like the idea that mod_dav is treated differently to other handlers. If you want to really solve this problem, you need to do so generically. Regards, Graham --
Re: mod_dav inconsistent behaviour for GET requests
On Sat, 30 Jan 2010, Graham Leggett wrote: On 30 Jan 2010, at 12:04 PM, Stefan Fritsch wrote: I don't follow how this makes it easier to use the recommended setup? In your example config, you defined /dav as being handled by mod_dav, and then you defined a FilesMatch (as I recall) that defined mod_php to be used by all URLs in the complete URL space that ended with .php. In so doing you're creating two configs that both overlap and contradict themselves, and this is specifically discouraged by the recommended setup. It's not that easy to enable mod_php globally except for one subdir, at least for the casual admin. For the FilesMatch, one would need some advanced regexp foo. The same is true if you have AddType'd various script extensions. The example config at http://httpd.apache.org/docs/2.2/mod/mod_dav.html#complex recommends using 'ForceType text/plain' to override the 'AddType application/x-httpd-php .php' that most users have somewhere else in their config. This is a pretty bad hack, IMHO. Most files in the dav directory will be delivered with the wrong content type. I also don't like the idea that mod_dav is treated differently to other handlers. If you want to really solve this problem, you need to do so generically. mod_dav is not treated differently, is should just handle the request. The fact that it delegates this to the default handler for some providers should not concern the average user. Or how do you explain to a user that a config that works fine with 'Dav svn' does not work with 'Dav on'? (Or the other way round, depending on what you are trying to achieve.)
Re: mod_dav inconsistent behaviour for GET requests
Stefan Fritsch wrote: Hi, mod_dav doesn't handle GET requests in a consistent way: If a repos provider has handle_get == 1, mod_dav will handle GET requests by itself. This means no other handler will get a chance to interpret the file as script or SSI. On the other hand, if the repos provider has handle_get == 0, mod_dav will do nothing and another handler like mod_php or mod_include may handle scripts, causing the output of the script execution to be sent to the client instead of the script source. ... Lesson: don't serve the script's output and the script itself from the same URI. And if you really really have to, consider using the MS extension request header Translate. BR, Julian
RE: mod_dav inconsistent behaviour for GET requests
-Original Message- From: Stefan Fritsch Sent: Freitag, 29. Januar 2010 08:43 To: dev@httpd.apache.org Subject: mod_dav inconsistent behaviour for GET requests Hi, mod_dav doesn't handle GET requests in a consistent way: If a repos provider has handle_get == 1, mod_dav will handle GET requests by itself. This means no other handler will get a chance to interpret the file as script or SSI. On the other hand, if the repos provider has handle_get == 0, mod_dav will do nothing and another handler like mod_php or mod_include may handle scripts, causing the output of the script execution to be sent to the client instead of the script source. The documentation http://httpd.apache.org/docs/trunk/mod/mod_dav.html#complex suggests using ForceType 'text/plain' to circumvent this. Apart from the fact that this is not enough and one may need 'SetHandler none' as well, I think this is a kludge. mod_dav should make sure that the GET request is handled by the default handler by setting r-handler in its fixup hook. If nobody disagrees, I will change this in trunk. Please do not. Some mod_dav providers need to implement their own GET handler because the resource requested might not be a plain file (e.g. it is stored in the database). Apart from Julians comments, SSI is a filter and can be applied to these resources as well (no problem) and the same is true for PHP if mod_php is build as a filter. But usually: Use a different URL for WebDAV than for live access. Regards Rüdiger
Re: mod_dav inconsistent behaviour for GET requests
It seems my mail was not so clear. On Friday 29 January 2010, Plüm, Rüdiger, VF-Group wrote: The documentation http://httpd.apache.org/docs/trunk/mod/mod_dav.html#complex suggests using ForceType 'text/plain' to circumvent this. Apart from the fact that this is not enough and one may need 'SetHandler none' as well, I think this is a kludge. mod_dav should make sure that the GET request is handled by the default handler by setting r-handler in its fixup hook. If nobody disagrees, I will change this in trunk. Please do not. Some mod_dav providers need to implement their own GET handler because the resource requested might not be a plain file (e.g. it is stored in the database). If this was not clear, I only want to change the behaviour for providers that do not implement their own GET handler, i.e. handle_get == 0. Apart from Julians comments, SSI is a filter and can be applied to these resources as well (no problem) and the same is true for PHP if mod_php is build as a filter. Ok, filters are a different problem, then. But my argument still stands for mod_php as handler. But usually: Use a different URL for WebDAV than for live access. That's what I want. But mod_dav makes it more difficult than necessary. Consider a configuration with mod_php like this: FilesMatch \.php$ SetHandler application/x-httpd-php /Files Location /dav Dav on /Location Then I want mod_dav to serve the script source for /dav/test.php but instead I will get the script output from mod_php. To get the script source, I currently need: Location /dav Dav on SetHandler none /Location In contrast, if I use a dav provider that handles GETs (like mod_dav_svn), I don't need the 'SetHandler none'. This is inconsistent. IMNSHO, if I set 'Dav on' for some Location, mod_dav has to ensure that it works, even for providers that don't handle GETs. Cheers, Stefan
Re: mod_dav inconsistent behaviour for GET requests
On Friday 29 January 2010, Julian Reschke wrote: And if you really really have to, consider using the MS extension request header Translate. Not all DAV clients send this header. Therefore it is not an option.
RE: mod_dav inconsistent behaviour for GET requests
-Original Message- From: Stefan Fritsch Sent: Freitag, 29. Januar 2010 11:26 To: dev@httpd.apache.org Subject: Re: mod_dav inconsistent behaviour for GET requests It seems my mail was not so clear. On Friday 29 January 2010, Plüm, Rüdiger, VF-Group wrote: The documentation http://httpd.apache.org/docs/trunk/mod/mod_dav.html#complex suggests using ForceType 'text/plain' to circumvent this. Apart from the fact that this is not enough and one may need 'SetHandler none' as well, I think this is a kludge. mod_dav should make sure that the GET request is handled by the default handler by setting r-handler in its fixup hook. If nobody disagrees, I will change this in trunk. Please do not. Some mod_dav providers need to implement their own GET handler because the resource requested might not be a plain file (e.g. it is stored in the database). If this was not clear, I only want to change the behaviour for providers that do not implement their own GET handler, i.e. handle_get == 0. Apart from Julians comments, SSI is a filter and can be applied to these resources as well (no problem) and the same is true for PHP if mod_php is build as a filter. Ok, filters are a different problem, then. But my argument still stands for mod_php as handler. But usually: Use a different URL for WebDAV than for live access. That's what I want. But mod_dav makes it more difficult than necessary. Consider a configuration with mod_php like this: FilesMatch \.php$ SetHandler application/x-httpd-php /Files Location /dav Dav on /Location Then I want mod_dav to serve the script source for /dav/test.php but instead I will get the script output from mod_php. To get the script source, I currently need: Location /dav Dav on SetHandler none /Location In contrast, if I use a dav provider that handles GETs (like mod_dav_svn), I don't need the 'SetHandler none'. This is inconsistent. IMNSHO, if I set 'Dav on' for some Location, mod_dav has to ensure that it works, even for providers that don't handle GETs. Thanks for clarification. I guess I understand your intension better now. So basicly you want those providers that do not implement GET by themselves to enforce the usage of the default handler, correct? Mind to sent a patch to the list for better review? Regards Rüdiger
Re: mod_dav inconsistent behaviour for GET requests
On Friday 29 January 2010, Plüm, Rüdiger, VF-Group wrote:
Thanks for clarification. I guess I understand your intension
better now. So basicly you want those providers that do not
implement GET by themselves to enforce the usage of the default
handler, correct?
Mind to sent a patch to the list for better review?
Exactly. The patch below works with 2.2 (haven't tested with trunk
due to lack of mod_php).
BTW, I found PR 13025, which seems to suggest that being able to mix
script execution and DAV on the same URL is a feature. I am still for
removing this 'feature' in trunk, though. But I would be against a
backport to 2.2.x.
--- a/modules/dav/main/mod_dav.c
+++ b/modules/dav/main/mod_dav.c
@@ -4803,12 +4803,13 @@ static int dav_fixups(request_rec *r)
/*
* If the repository hasn't indicated that it will handle the
- * GET method, then just punt.
- *
- * ### this isn't quite right... taking over the response can break
- * ### things like mod_negotiation. need to look into this some more.
+ * GET method, then we let the default handler do it. Set the handler
+ * explicitly to ensure that no other handler takes the request.
+ * We don't care about directories, though.
*/
if (!conf-provider-repos-handle_get) {
+if (r-finfo.filetype != APR_DIR)
+r-handler = none;
return DECLINED;
}
}
Re: mod_dav inconsistent behaviour for GET requests
On Jan 29, 2010, at 10:46 AM, Stefan Fritsch wrote:
On Friday 29 January 2010, Plüm, Rüdiger, VF-Group wrote:
Thanks for clarification. I guess I understand your intension
better now. So basicly you want those providers that do not
implement GET by themselves to enforce the usage of the default
handler, correct?
Mind to sent a patch to the list for better review?
Exactly. The patch below works with 2.2 (haven't tested with trunk
due to lack of mod_php).
BTW, I found PR 13025, which seems to suggest that being able to mix
script execution and DAV on the same URL is a feature. I am still for
removing this 'feature' in trunk, though. But I would be against a
backport to 2.2.x.
--- a/modules/dav/main/mod_dav.c
+++ b/modules/dav/main/mod_dav.c
@@ -4803,12 +4803,13 @@ static int dav_fixups(request_rec *r)
/*
* If the repository hasn't indicated that it will handle the
- * GET method, then just punt.
- *
- * ### this isn't quite right... taking over the response can break
- * ### things like mod_negotiation. need to look into this some more.
+ * GET method, then we let the default handler do it. Set the handler
+ * explicitly to ensure that no other handler takes the request.
+ * We don't care about directories, though.
*/
if (!conf-provider-repos-handle_get) {
+if (r-finfo.filetype != APR_DIR)
+r-handler = none;
return DECLINED;
}
}
It looks to me like that would introduce a security hole for existing
configs that expect a handler to run on GET (PHP/CGI scripts that are
authorable via DAV). -1 if so.
Roy
