You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by jo...@apache.org on 2009/05/08 16:13:15 UTC
svn commit: r772997 - in /httpd/httpd/trunk: include/http_core.h
modules/filters/mod_include.c server/config.c server/core.c
Author: jorton
Date: Fri May 8 14:13:15 2009
New Revision: 772997
URL: http://svn.apache.org/viewvc?rev=772997&view=rev
Log:
Security fix for CVE-2009-1195: fix Options handling such that
'AllowOverride Options=IncludesNoExec' does not permit Includes with
exec= enabled to be configured in an .htaccess file:
* include/http_core.h: Change semantics of Includes/IncludeNoExec
options bits to be additive; OPT_INCLUDES now means SSI is enabled
without exec=. OPT_INCLUDES|OPT_INC_WITH_EXEC means SSI is enabled
with exec=.
* server/core.c (create_core_dir_config): Remove defunct OPT_INCNOEXEC
from default override_opts; no functional change.
(merge_core_dir_configs): Update logic to ensure that exec= is
disabled in a context where IncludesNoexec is configured, even if
Includes-with-exec is permitted in the inherited options set.
(set_allow_opts, set_options): Update to reflect new semantics
of OPT_INCLUDES, OPT_INC_WITH_EXEC.
* server/config.c: Update to remove OPT_INCNOEXEC from default
override_opts; no functional change.
* modules/filters/mod_include.c (includes_filter): Update to reflect
new options semantics - disable exec= support if the
OPT_INC_WITH_EXEC bit is not set.
Submitted by: Jonathan Peatfield <j.s.peatfield damtp.cam.ac.uk>,
jorton
Thanks to: Vincent Danon <vdanon redhat.com>
Modified:
httpd/httpd/trunk/include/http_core.h
httpd/httpd/trunk/modules/filters/mod_include.c
httpd/httpd/trunk/server/config.c
httpd/httpd/trunk/server/core.c
Modified: httpd/httpd/trunk/include/http_core.h
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/include/http_core.h?rev=772997&r1=772996&r2=772997&view=diff
==============================================================================
--- httpd/httpd/trunk/include/http_core.h (original)
+++ httpd/httpd/trunk/include/http_core.h Fri May 8 14:13:15 2009
@@ -68,7 +68,7 @@
#define OPT_NONE 0
/** Indexes directive */
#define OPT_INDEXES 1
-/** Includes directive */
+/** SSI is enabled without exec= permission */
#define OPT_INCLUDES 2
/** FollowSymLinks directive */
#define OPT_SYM_LINKS 4
@@ -76,14 +76,14 @@
#define OPT_EXECCGI 8
/** directive unset */
#define OPT_UNSET 16
-/** IncludesNOEXEC directive */
-#define OPT_INCNOEXEC 32
+/** SSI exec= permission is permitted, iff OPT_INCLUDES is also set */
+#define OPT_INC_WITH_EXEC 32
/** SymLinksIfOwnerMatch directive */
#define OPT_SYM_OWNER 64
/** MultiViews directive */
#define OPT_MULTI 128
/** All directives */
-#define OPT_ALL (OPT_INDEXES|OPT_INCLUDES|OPT_SYM_LINKS|OPT_EXECCGI)
+#define OPT_ALL (OPT_INDEXES|OPT_INCLUDES|OPT_INC_WITH_EXEC|OPT_SYM_LINKS|OPT_EXECCGI)
/** @} */
/**
Modified: httpd/httpd/trunk/modules/filters/mod_include.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/modules/filters/mod_include.c?rev=772997&r1=772996&r2=772997&view=diff
==============================================================================
--- httpd/httpd/trunk/modules/filters/mod_include.c (original)
+++ httpd/httpd/trunk/modules/filters/mod_include.c Fri May 8 14:13:15 2009
@@ -2946,7 +2946,7 @@
intern->seen_eos = 0;
intern->state = PARSE_PRE_HEAD;
ctx->flags = (SSI_FLAG_PRINTING | SSI_FLAG_COND_TRUE);
- if (ap_allow_options(r) & OPT_INCNOEXEC) {
+ if ((ap_allow_options(r) & OPT_INC_WITH_EXEC) == 0) {
ctx->flags |= SSI_FLAG_NO_EXEC;
}
intern->access_func = conf->accessenable ? ssi_access : NULL;
Modified: httpd/httpd/trunk/server/config.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/config.c?rev=772997&r1=772996&r2=772997&view=diff
==============================================================================
--- httpd/httpd/trunk/server/config.c (original)
+++ httpd/httpd/trunk/server/config.c Fri May 8 14:13:15 2009
@@ -1524,7 +1524,7 @@
parms.temp_pool = ptemp;
parms.server = s;
parms.override = (RSRC_CONF | OR_ALL) & ~(OR_AUTHCFG | OR_LIMIT);
- parms.override_opts = OPT_ALL | OPT_INCNOEXEC | OPT_SYM_OWNER | OPT_MULTI;
+ parms.override_opts = OPT_ALL | OPT_SYM_OWNER | OPT_MULTI;
parms.config_file = ap_pcfg_open_custom(p, "-c/-C directives",
&arr_parms, NULL,
@@ -1631,7 +1631,7 @@
parms.temp_pool = ptemp;
parms.server = s;
parms.override = (RSRC_CONF | OR_ALL) & ~(OR_AUTHCFG | OR_LIMIT);
- parms.override_opts = OPT_ALL | OPT_INCNOEXEC | OPT_SYM_OWNER | OPT_MULTI;
+ parms.override_opts = OPT_ALL | OPT_SYM_OWNER | OPT_MULTI;
rv = ap_pcfg_openfile(&cfp, p, fname);
if (rv != APR_SUCCESS) {
@@ -1769,7 +1769,7 @@
parms.temp_pool = ptemp;
parms.server = s;
parms.override = (RSRC_CONF | OR_ALL) & ~(OR_AUTHCFG | OR_LIMIT);
- parms.override_opts = OPT_ALL | OPT_INCNOEXEC | OPT_SYM_OWNER | OPT_MULTI;
+ parms.override_opts = OPT_ALL | OPT_SYM_OWNER | OPT_MULTI;
parms.limited = -1;
errmsg = ap_walk_config(conftree, &parms, s->lookup_defaults);
Modified: httpd/httpd/trunk/server/core.c
URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/core.c?rev=772997&r1=772996&r2=772997&view=diff
==============================================================================
--- httpd/httpd/trunk/server/core.c (original)
+++ httpd/httpd/trunk/server/core.c Fri May 8 14:13:15 2009
@@ -108,8 +108,7 @@
conf->opts = dir ? OPT_UNSET : OPT_UNSET|OPT_ALL;
conf->opts_add = conf->opts_remove = OPT_NONE;
conf->override = dir ? OR_UNSET : OR_UNSET|OR_ALL;
- conf->override_opts = OPT_UNSET | OPT_ALL | OPT_INCNOEXEC | OPT_SYM_OWNER
- | OPT_MULTI;
+ conf->override_opts = OPT_UNSET | OPT_ALL | OPT_SYM_OWNER | OPT_MULTI;
conf->content_md5 = 2;
conf->accept_path_info = 3;
@@ -239,8 +238,13 @@
conf->opts_remove = (conf->opts_remove & ~new->opts_add)
| new->opts_remove;
conf->opts = (conf->opts & ~conf->opts_remove) | conf->opts_add;
- if ((base->opts & OPT_INCNOEXEC) && (new->opts & OPT_INCLUDES)) {
- conf->opts = (conf->opts & ~OPT_INCNOEXEC) | OPT_INCLUDES;
+
+ /* if Includes was enabled without exec in the new config, but
+ * was enabled with exec in the base, then disable exec in the
+ * resulting options. */
+ if ((base->opts & OPT_INC_WITH_EXEC)
+ && (new->opts & OPT_INC_WITH_EXEC) == 0) {
+ conf->opts &= ~OPT_INC_WITH_EXEC;
}
}
else {
@@ -1287,10 +1291,12 @@
opt = OPT_INDEXES;
}
else if (!strcasecmp(w, "Includes")) {
- opt = OPT_INCLUDES;
+ /* If Includes is permitted, both Includes and
+ * IncludesNOEXEC may be changed. */
+ opt = (OPT_INCLUDES | OPT_INC_WITH_EXEC);
}
else if (!strcasecmp(w, "IncludesNOEXEC")) {
- opt = (OPT_INCLUDES | OPT_INCNOEXEC);
+ opt = OPT_INCLUDES;
}
else if (!strcasecmp(w, "FollowSymLinks")) {
opt = OPT_SYM_LINKS;
@@ -1406,10 +1412,10 @@
opt = OPT_INDEXES;
}
else if (!strcasecmp(w, "Includes")) {
- opt = OPT_INCLUDES;
+ opt = (OPT_INCLUDES | OPT_INC_WITH_EXEC);
}
else if (!strcasecmp(w, "IncludesNOEXEC")) {
- opt = (OPT_INCLUDES | OPT_INCNOEXEC);
+ opt = OPT_INCLUDES;
}
else if (!strcasecmp(w, "FollowSymLinks")) {
opt = OPT_SYM_LINKS;
Re: svn commit: r772997 - in /httpd/httpd/trunk: include/http_core.h
modules/filters/mod_include.c server/config.c server/core.c
Posted by Eric Covener <co...@gmail.com>.
On Sat, May 9, 2009 at 5:24 PM, Eric Covener <co...@gmail.com> wrote:
> On Sat, May 9, 2009 at 2:16 PM, Eric Covener <co...@gmail.com> wrote:
>> On Fri, May 8, 2009 at 10:13 AM, <jo...@apache.org> wrote:
>>
>>> +
>>> + /* if Includes was enabled without exec in the new config, but
>>> + * was enabled with exec in the base, then disable exec in the
>>> + * resulting options. */
>>> + if ((base->opts & OPT_INC_WITH_EXEC)
>>> + && (new->opts & OPT_INC_WITH_EXEC) == 0) {
>>> + conf->opts &= ~OPT_INC_WITH_EXEC;
>>
>>
>> The above is wrapped in
>> if (new->opts & OPT_UNSET) {
>>
>> Which means checking new->opts against anything is probably not needed
>
> Not likely the case due to test failures removing this block.
Both failures are testing the tweak discussed in the other thread:
| 5) I'll post an updated patch soon which fixes the behaviour of "Options
| Includes"/"Options +IncludesNoExec" such that SSI is permitted without
| exec, as is the current 2.2.x behaviour, since that seems to be the
| rough consensus. Jon also spotted a minor logic flaw in the patch which
| I'll fix too.
### Test #82, context: Options Includes : AllowOverride
Options=IncludesNoExec : Options +IncludesNoExec
### Test #102, context: Options Includes: AllowOverride ALl; Options
+IncludesNoexec
Both failers [and rest of suite] , along with my non-htaccess issue,
seem to be corrected with:
- && (new->opts & OPT_INC_WITH_EXEC) == 0) {
+ && (new->opts_add & OPT_INC_WITH_EXEC) == 0) {
--
Eric Covener
covener@gmail.com
Re: svn commit: r772997 - in /httpd/httpd/trunk: include/http_core.h
modules/filters/mod_include.c server/config.c server/core.c
Posted by Eric Covener <co...@gmail.com>.
On Sat, May 9, 2009 at 2:16 PM, Eric Covener <co...@gmail.com> wrote:
> On Fri, May 8, 2009 at 10:13 AM, <jo...@apache.org> wrote:
>
>> +
>> + /* if Includes was enabled without exec in the new config, but
>> + * was enabled with exec in the base, then disable exec in the
>> + * resulting options. */
>> + if ((base->opts & OPT_INC_WITH_EXEC)
>> + && (new->opts & OPT_INC_WITH_EXEC) == 0) {
>> + conf->opts &= ~OPT_INC_WITH_EXEC;
>
>
> The above is wrapped in
> if (new->opts & OPT_UNSET) {
>
> Which means checking new->opts against anything is probably not needed
Not likely the case due to test failures removing this block.
--
Eric Covener
covener@gmail.com
Re: svn commit: r772997 - in /httpd/httpd/trunk:
include/http_core.h modules/filters/mod_include.c server/config.c
server/core.c
Posted by Joe Orton <jo...@redhat.com>.
On Sun, May 10, 2009 at 12:32:44PM +0200, Ruediger Pluem wrote:
> On 05/10/2009 12:26 AM, Eric Covener wrote:
> > On Sat, May 9, 2009 at 5:55 PM, Ruediger Pluem <rp...@apache.org> wrote:
> >> --- server/core.c (Revision 773105)
> >> +++ server/core.c (Arbeitskopie)
> >> @@ -242,8 +242,9 @@
> >> /* if Includes was enabled without exec in the new config, but
> >> * was enabled with exec in the base, then disable exec in the
> >> * resulting options. */
> >> - if ((base->opts & OPT_INC_WITH_EXEC)
> >> - && (new->opts & OPT_INC_WITH_EXEC) == 0) {
> >> + if ((base->opts & OPT_INC_WITH_EXEC)
> >> + && (new->opts & OPT_INC_WITH_EXEC) == 0
> >> + && (new->opts & OPT_INCLUDES)) {
> >> conf->opts &= ~OPT_INC_WITH_EXEC;
> >> }
> >> }
> >
> > Works for me. My response after the one you quoted was a SNAFU with
> > my test configuration and didn't really help my non-htaccess case.
>
> Commited as r773322.
Thanks a lot guys, and good catch Eric - just goes to show that no 120
cell test matrix is ever complete ;) I've tweaked that change slightly
in r773342.
Regards, Joe
Re: svn commit: r772997 - in /httpd/httpd/trunk: include/http_core.h
modules/filters/mod_include.c server/config.c server/core.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 05/10/2009 12:26 AM, Eric Covener wrote:
> On Sat, May 9, 2009 at 5:55 PM, Ruediger Pluem <rp...@apache.org> wrote:
>>
>> On 05/09/2009 08:16 PM, Eric Covener wrote:
>>
>>> This zaps OPT_INC_WITH_EXEC for a config w/o the htaccess issue (the
>>> real focus of the change):
>>>
>>> # only two containers in the config
>>>
>>> <Directory />
>>> Options Includes
>>> AllowOverride None
>>> </Directory>
>>>
>>> <Directory /home/covener>
>>> # with this container, mod_cgi/mod_cgid complains about exec being off
>>> # without it, exec cmd= works as expected
>>> SetEnv foo bar
>>> </Directory>
>> I guess this behaviour is not expected. Does the following patch fix this
>> (it still passes all test cases):
>>
>> Index: server/core.c
>> ===================================================================
>> --- server/core.c (Revision 773105)
>> +++ server/core.c (Arbeitskopie)
>> @@ -242,8 +242,9 @@
>> /* if Includes was enabled without exec in the new config, but
>> * was enabled with exec in the base, then disable exec in the
>> * resulting options. */
>> - if ((base->opts & OPT_INC_WITH_EXEC)
>> - && (new->opts & OPT_INC_WITH_EXEC) == 0) {
>> + if ((base->opts & OPT_INC_WITH_EXEC)
>> + && (new->opts & OPT_INC_WITH_EXEC) == 0
>> + && (new->opts & OPT_INCLUDES)) {
>> conf->opts &= ~OPT_INC_WITH_EXEC;
>> }
>> }
>
> Works for me. My response after the one you quoted was a SNAFU with
> my test configuration and didn't really help my non-htaccess case.
>
Commited as r773322.
Regards
Rüdiger
Re: svn commit: r772997 - in /httpd/httpd/trunk: include/http_core.h
modules/filters/mod_include.c server/config.c server/core.c
Posted by Eric Covener <co...@gmail.com>.
On Sat, May 9, 2009 at 5:55 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
>
> On 05/09/2009 08:16 PM, Eric Covener wrote:
>
>>
>> This zaps OPT_INC_WITH_EXEC for a config w/o the htaccess issue (the
>> real focus of the change):
>>
>> # only two containers in the config
>>
>> <Directory />
>> Options Includes
>> AllowOverride None
>> </Directory>
>>
>> <Directory /home/covener>
>> # with this container, mod_cgi/mod_cgid complains about exec being off
>> # without it, exec cmd= works as expected
>> SetEnv foo bar
>> </Directory>
>
> I guess this behaviour is not expected. Does the following patch fix this
> (it still passes all test cases):
>
> Index: server/core.c
> ===================================================================
> --- server/core.c (Revision 773105)
> +++ server/core.c (Arbeitskopie)
> @@ -242,8 +242,9 @@
> /* if Includes was enabled without exec in the new config, but
> * was enabled with exec in the base, then disable exec in the
> * resulting options. */
> - if ((base->opts & OPT_INC_WITH_EXEC)
> - && (new->opts & OPT_INC_WITH_EXEC) == 0) {
> + if ((base->opts & OPT_INC_WITH_EXEC)
> + && (new->opts & OPT_INC_WITH_EXEC) == 0
> + && (new->opts & OPT_INCLUDES)) {
> conf->opts &= ~OPT_INC_WITH_EXEC;
> }
> }
Works for me. My response after the one you quoted was a SNAFU with
my test configuration and didn't really help my non-htaccess case.
--
Eric Covener
covener@gmail.com
Re: svn commit: r772997 - in /httpd/httpd/trunk: include/http_core.h
modules/filters/mod_include.c server/config.c server/core.c
Posted by Ruediger Pluem <rp...@apache.org>.
On 05/09/2009 08:16 PM, Eric Covener wrote:
>
> This zaps OPT_INC_WITH_EXEC for a config w/o the htaccess issue (the
> real focus of the change):
>
> # only two containers in the config
>
> <Directory />
> Options Includes
> AllowOverride None
> </Directory>
>
> <Directory /home/covener>
> # with this container, mod_cgi/mod_cgid complains about exec being off
> # without it, exec cmd= works as expected
> SetEnv foo bar
> </Directory>
I guess this behaviour is not expected. Does the following patch fix this
(it still passes all test cases):
Index: server/core.c
===================================================================
--- server/core.c (Revision 773105)
+++ server/core.c (Arbeitskopie)
@@ -242,8 +242,9 @@
/* if Includes was enabled without exec in the new config, but
* was enabled with exec in the base, then disable exec in the
* resulting options. */
- if ((base->opts & OPT_INC_WITH_EXEC)
- && (new->opts & OPT_INC_WITH_EXEC) == 0) {
+ if ((base->opts & OPT_INC_WITH_EXEC)
+ && (new->opts & OPT_INC_WITH_EXEC) == 0
+ && (new->opts & OPT_INCLUDES)) {
conf->opts &= ~OPT_INC_WITH_EXEC;
}
}
Regards
Rüdiger
Re: svn commit: r772997 - in /httpd/httpd/trunk: include/http_core.h
modules/filters/mod_include.c server/config.c server/core.c
Posted by Eric Covener <co...@gmail.com>.
On Fri, May 8, 2009 at 10:13 AM, <jo...@apache.org> wrote:
> +
> + /* if Includes was enabled without exec in the new config, but
> + * was enabled with exec in the base, then disable exec in the
> + * resulting options. */
> + if ((base->opts & OPT_INC_WITH_EXEC)
> + && (new->opts & OPT_INC_WITH_EXEC) == 0) {
> + conf->opts &= ~OPT_INC_WITH_EXEC;
The above is wrapped in
if (new->opts & OPT_UNSET) {
Which means checking new->opts against anything is probably not needed
and harmful in the case illustrated below. new->opts can be
initialized with OPT_UNSET only when no htaccess is involved (if I
understand the 2nd parm to foo_create_config()):
create_core_dir_config:
conf->opts = dir ? OPT_UNSET : OPT_UNSET|OPT_ALL;
This zaps OPT_INC_WITH_EXEC for a config w/o the htaccess issue (the
real focus of the change):
# only two containers in the config
<Directory />
Options Includes
AllowOverride None
</Directory>
<Directory /home/covener>
# with this container, mod_cgi/mod_cgid complains about exec being off
# without it, exec cmd= works as expected
SetEnv foo bar
</Directory>
Is it safe to drop the entire quoted stanza since new->opts is UNSET anyway?
--
Eric Covener
covener@gmail.com
Re: svn commit: r772997 - in /httpd/httpd/trunk: include/http_core.h
modules/filters/mod_include.c server/config.c server/core.c
Posted by Eric Covener <co...@gmail.com>.
On Fri, May 8, 2009 at 10:13 AM, <jo...@apache.org> wrote:
> Author: jorton
> Date: Fri May 8 14:13:15 2009
> New Revision: 772997
> Modified:
> httpd/httpd/trunk/include/http_core.h
> httpd/httpd/trunk/modules/filters/mod_include.c
> httpd/httpd/trunk/server/config.c
> httpd/httpd/trunk/server/core.c
Lengthy commit msg (snipped) but no CHANGES.
--
Eric Covener
covener@gmail.com