Re: [Patch] Be more selective on includes
* William A. Rowe, Jr. ([EMAIL PROTECTED]) wrote : At 07:58 PM 11/20/2002, Thom May wrote: This is in response to a debian bug request; basically it just tightens up the list of allowed characters, so we don't include .dotfiles and backups etc. Thoughts? Yea... I'd ask for exactly what I've been asking for over a year... ...does anyone have a decent reference to the exclusion applied for other packages that provide the config directory globbing that we added/emulated with this feature? Can we be consistent for the benefit of the administrator community here? Well, my suggested change is (on debian at least) providing exactly the same logic as run-parts(8), which is used to process /etc/cron.*/ and others. So it's consistent for our platform, at least. run-parts is present on Red Hat 8, also. Cheers, -Thom Bill
Re: [Patch] Be more selective on includes
At 07:58 PM 11/20/2002, Thom May wrote: This is in response to a debian bug request; basically it just tightens up the list of allowed characters, so we don't include .dotfiles and backups etc. Thoughts? Yea... I'd ask for exactly what I've been asking for over a year... ...does anyone have a decent reference to the exclusion applied for other packages that provide the config directory globbing that we added/emulated with this feature? Can we be consistent for the benefit of the administrator community here? Bill
Re: [Patch] Be more selective on includes
--On Wednesday, November 20, 2002 9:19 PM -0500 Joshua Slive [EMAIL PROTECTED] wrote: Right. People should be using Include conf/*.conf The docs now explicitly discourage the use of directory includes to avoid exactly this problem. ++1. -- justin
Re: [Patch] Be more selective on includes
* André Malo ([EMAIL PROTECTED]) wrote : * Thom May wrote: This is in response to a debian bug request; basically it just tightens up the list of allowed characters, so we don't include .dotfiles and backups etc. Thoughts? hmm. I don't like it. The most can easily be done with normal wildcard matching. If your patch is applied and I have filenames (already), that don't match the hardcoded (!) rules, I'm lost. OK, so Justin and I were just discussing this in the bar at apachecon, and I explained the use-case I was interested in, and why I wanted this patch: We have a directory tree containing vhosts: /etc/apache2/sites-enabled this contains file in this form: samizdat.positive-internet.com www.planetarytramp.net www.example.org.uk ... ie, the file name matches the ServerName entry. In this case, wildcard matching is either too generic (Include sites-enabled/*), or not workable at all(Include sites-enabled/*.com\nInclude sites-enabled/*.org etc etc). Now, say we edit a file in this directory, and while we are doing so, apache gets restarted. most (unix) editors will leave a .name-of-file.swp or name-of-file~ in the directory as a backup while you are editing the file, so at this point you know have two semi-identical lumps of config loaded, one of which may be broken, or missing config or whatever. This seems like an uber-bad thing. So, I guess the second proposal would be to keep the behaviour my patch proposes, but add a SafeInclude on/off directive (defaulting to Off) to define whether you want the behaviour or not. Cheers, -Thom
Re: [Patch] Be more selective on includes
* Thom May wrote: * André Malo ([EMAIL PROTECTED]) wrote : * Thom May wrote: This is in response to a debian bug request; basically it just tightens up the list of allowed characters, so we don't include .dotfiles and backups etc. Thoughts? hmm. I don't like it. The most can easily be done with normal wildcard matching. If your patch is applied and I have filenames (already), that don't match the hardcoded (!) rules, I'm lost. OK, so Justin and I were just discussing this in the bar at apachecon, and I explained the use-case I was interested in, and why I wanted this patch: We have a directory tree containing vhosts: /etc/apache2/sites-enabled this contains file in this form: samizdat.positive-internet.com www.planetarytramp.net www.example.org.uk ... ah ok, understand. A workaround would be: /etc/apache2/vhosts/ contains the config files and /etc/apache2/sites-enabled/ contains (sym-)links to the enabled ones. to be very safe, one can name the symlinks *.conf or similar and use wildcards to include them. So, I guess the second proposal would be to keep the behaviour my patch proposes, but add a SafeInclude on/off directive (defaulting to Off) to define whether you want the behaviour or not. A Feature that can't be disabled is a bug ;-) Seriously, I would like that - if configurable - but would then be +1 for defaulting to *On*, so the User can loosen the rules, if he's able to find the directive in the docs... Another alternative would be a directive similar to the mod_autoindex IndexIgnore (e.g. IncludeIgnore). I think, that would be the most flexible way to handle the problem. (Proposed priority handling: IncludeIgnore overrides Include wildcards and directory includes, but *not* wildcard-free filenames) nd -- Die Untergeschosse der Sempergalerie bleiben währenddessen aus statistischen Gründen geflutet. -- Spiegel Online
Re: [Patch] Be more selective on includes
On Thu, 21 Nov 2002, Thom May wrote: OK, so Justin and I were just discussing this in the bar at apachecon, and I explained the use-case I was interested in, and why I wanted this patch: We have a directory tree containing vhosts: /etc/apache2/sites-enabled this contains file in this form: samizdat.positive-internet.com www.planetarytramp.net www.example.org.uk ... ie, the file name matches the ServerName entry. In this case, wildcard matching is either too generic (Include sites-enabled/*), or not workable at all(Include sites-enabled/*.com\nInclude sites-enabled/*.org etc etc). Awww, come on... Just rename the damn things samizdat.postitive-internet.com.conf or something to that effect. I am opposed to any SafeInclude type directives because: 1. Almost all reasonable use cases can be solved with fnmatch wildcards. 2. As soon as we add those exceptions you want, 100 people will come crawling out of the woodwork asking for exceptions for their special files. 3. It is a bad idea to have file-types hardcoded in Apache. 4. It is unnecessary complexity, which makes things harder for everybody. Joshua.
[Patch] Be more selective on includes
This is in response to a debian bug request; basically it just tightens up
the list of allowed characters, so we don't include .dotfiles and backups
etc.
Thoughts?
-Thom
Index: server/config.c
===
RCS file: /home/cvspublic/httpd-2.0/server/config.c,v
retrieving revision 1.156
diff -u -r1.156 config.c
--- server/config.c 12 Sep 2002 20:04:07 - 1.156
+++ server/config.c 21 Nov 2002 01:58:06 -
@@ -76,6 +76,7 @@
#include apr_portable.h
#include apr_file_io.h
#include apr_fnmatch.h
+#include apr_lib.h
#define APR_WANT_STDIO
#define APR_WANT_STRFUNC
@@ -1434,6 +1435,20 @@
return strcmp(f1-fname,f2-fname);
}
+static int fname_valid(const char *fname)
+{
+const char *c = fname;
+if (!apr_isalnum(*c))
+ return 0;
+++c;
+while (*c) {
+ if(!apr_isalnum(*c) *c!='_' *c!='-' *c!='.')
+return 0;
+ ++c;
+}
+return 1;
+}
+
AP_DECLARE(void) ap_process_resource_config(server_rec *s, const char *fname,
ap_directive_t **conftree,
apr_pool_t *p,
@@ -1510,7 +1525,8 @@
strcmp(dirent.name, ..)
(!ispatt ||
apr_fnmatch(pattern, dirent.name,
-FNM_PERIOD) == APR_SUCCESS)) {
+FNM_PERIOD) == APR_SUCCESS)
+ fname_valid(dirent.name)) {
fnew = (fnames *) apr_array_push(candidates);
fnew-fname = ap_make_full_path(p, path, dirent.name);
}
Re: [Patch] Be more selective on includes
* Thom May wrote:
This is in response to a debian bug request; basically it just tightens up
the list of allowed characters, so we don't include .dotfiles and backups
etc.
Thoughts?
hmm. I don't like it. The most can easily be done with normal wildcard
matching. If your patch is applied and I have filenames (already), that
don't match the hardcoded (!) rules, I'm lost.
nd
--
sub the($){+shift} sub answer (){ord q
[* It is always 42! *] }
print the answer
# André Malo # http://www.perlig.de/ #
Re: [Patch] Be more selective on includes
On Thu, 21 Nov 2002, André Malo wrote: * Thom May wrote: This is in response to a debian bug request; basically it just tightens up the list of allowed characters, so we don't include .dotfiles and backups etc. Thoughts? hmm. I don't like it. The most can easily be done with normal wildcard matching. If your patch is applied and I have filenames (already), that don't match the hardcoded (!) rules, I'm lost. Right. People should be using Include conf/*.conf The docs now explicitly discourage the use of directory includes to avoid exactly this problem. Joshua.
