You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Joe Orton <jo...@redhat.com> on 2009/05/25 09:45:47 UTC
Re: [concept PATCH] CVE-2009-1195 tweaks to provide binary
compatibility for stable branches
On Fri, May 22, 2009 at 05:12:31PM -0400, Jeff Trawick wrote:
> (untested)
>
> ap_allow_options() is how applications, including our mod_include, access
> the enabled options for a given request (other than evil apps which define
> CORE_PRIVATE and locate the core_dir_config). As this is a callable
> function, it can map internal, hidden bitmaps as appropriate before
> returning to the caller.
Interesting idea! I'm concerned this is going to be overly intrusive,
what with requiring the changes to all uses of OPT_ALL internally. Does
it really matter what value of OPT_ALL is exposed to modules?
(or specifically: does it break compatibility to change what value of
OPT_ALL is exposed to modules?)
Something simpler might be sufficient? Patch against 2.2.x still passes
the CVE-2009-1195 test: (proof of concept for a bank holiday morning ;)
Index: modules/filters/mod_include.c
===================================================================
--- modules/filters/mod_include.c (revision 777502)
+++ modules/filters/mod_include.c (working copy)
@@ -3565,7 +3565,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_INC_WITH_EXEC) == 0) {
+ if (ap_allow_options(r) & OPT_INCNOEXEC) {
ctx->flags |= SSI_FLAG_NO_EXEC;
}
intern->accessenable = conf->accessenable;
Index: include/http_core.h
===================================================================
--- include/http_core.h (revision 777502)
+++ include/http_core.h (working copy)
@@ -73,14 +73,18 @@
#define OPT_EXECCGI 8
/** directive unset */
#define OPT_UNSET 16
-/** SSI exec= permission is permitted, iff OPT_INCLUDES is also set */
-#define OPT_INC_WITH_EXEC 32
+/** IncludesNOEXEC directive */
+#define OPT_INCNOEXEC 32
+#ifdef CORE_PRIVATE
+/** internal-only -- do not use! */
+#define OPT_INC_WITH_EXEC OPT_INCNOEXEC
+#endif
/** SymLinksIfOwnerMatch directive */
#define OPT_SYM_OWNER 64
/** MultiViews directive */
#define OPT_MULTI 128
/** All directives */
-#define OPT_ALL (OPT_INDEXES|OPT_INCLUDES|OPT_INC_WITH_EXEC|OPT_SYM_LINKS|OPT_EXECCGI)
+#define OPT_ALL (OPT_INDEXES|OPT_INCLUDES|OPT_INCNOEXEC|OPT_SYM_LINKS|OPT_EXECCGI)
/** @} */
/**
Index: server/core.c
===================================================================
--- server/core.c (revision 777502)
+++ server/core.c (working copy)
@@ -661,7 +661,7 @@
core_dir_config *conf =
(core_dir_config *)ap_get_module_config(r->per_dir_config, &core_module);
- return conf->opts;
+ return conf->opts ^ OPT_INC_WITH_EXEC;
}
AP_DECLARE(int) ap_allow_overrides(request_rec *r)
Re: [concept PATCH] CVE-2009-1195 tweaks to provide binary
compatibility for stable branches
Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, May 27, 2009 at 2:41 PM, Ruediger Pluem <rp...@apache.org> wrote:
>
>
> On 05/27/2009 05:25 PM, Joe Orton wrote:
> > On Mon, May 25, 2009 at 12:03:23PM -0400, Jeff Trawick wrote:
> >> I'm fine with your patch plus a bit of commentary in ap_allow_options().
> >
> > Proposed patch as below:
>
> So we only want to do this for 2.2.x, correct?
y
>
> For 2.3.x and up the module authors need to fix their modules, correct?
y
(compilation will be forced by MMN, and affected modules will fail to
compile)
Re: [concept PATCH] CVE-2009-1195 tweaks to provide binary compatibility
for stable branches
Posted by Ruediger Pluem <rp...@apache.org>.
On 05/27/2009 05:25 PM, Joe Orton wrote:
> On Mon, May 25, 2009 at 12:03:23PM -0400, Jeff Trawick wrote:
>> I'm fine with your patch plus a bit of commentary in ap_allow_options().
>
> Proposed patch as below:
So we only want to do this for 2.2.x, correct?
For 2.3.x and up the module authors need to fix their modules, correct?
Regards
Rüdiger
Re: [concept PATCH] CVE-2009-1195 tweaks to provide binary
compatibility for stable branches
Posted by Joe Orton <jo...@redhat.com>.
On Mon, May 25, 2009 at 12:03:23PM -0400, Jeff Trawick wrote:
> I'm fine with your patch plus a bit of commentary in ap_allow_options().
Proposed patch as below:
Index: modules/filters/mod_include.c
===================================================================
--- modules/filters/mod_include.c (revision 777502)
+++ modules/filters/mod_include.c (working copy)
@@ -3565,7 +3565,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_INC_WITH_EXEC) == 0) {
+ if (ap_allow_options(r) & OPT_INCNOEXEC) {
ctx->flags |= SSI_FLAG_NO_EXEC;
}
intern->accessenable = conf->accessenable;
Index: include/http_core.h
===================================================================
--- include/http_core.h (revision 777502)
+++ include/http_core.h (working copy)
@@ -73,16 +73,29 @@
#define OPT_EXECCGI 8
/** directive unset */
#define OPT_UNSET 16
-/** SSI exec= permission is permitted, iff OPT_INCLUDES is also set */
-#define OPT_INC_WITH_EXEC 32
+/** IncludesNOEXEC directive */
+#define OPT_INCNOEXEC 32
/** SymLinksIfOwnerMatch directive */
#define OPT_SYM_OWNER 64
/** MultiViews directive */
#define OPT_MULTI 128
/** All directives */
-#define OPT_ALL (OPT_INDEXES|OPT_INCLUDES|OPT_INC_WITH_EXEC|OPT_SYM_LINKS|OPT_EXECCGI)
+#define OPT_ALL (OPT_INDEXES|OPT_INCLUDES|OPT_INCNOEXEC|OPT_SYM_LINKS|OPT_EXECCGI)
/** @} */
+#ifdef CORE_PRIVATE
+/* For internal use only - since 2.2.12, the OPT_INCNOEXEC bit is
+ * internally replaced by OPT_INC_WITH_EXEC. The internal semantics
+ * of the two SSI-related bits are hence:
+ *
+ * OPT_INCLUDES => "enable SSI, without exec= permission"
+ * OPT_INC_WITH_EXEC => "iff OPT_INCLUDES is set, also enable exec="
+ *
+ * The set of options exposed via ap_allow_options() retains the
+ * semantics of OPT_INCNOEXEC by flipping the bit. */
+#define OPT_INC_WITH_EXEC OPT_INCNOEXEC
+#endif
+
/**
* @defgroup get_remote_host Remote Host Resolution
* @ingroup APACHE_CORE_HTTPD
Index: server/core.c
===================================================================
--- server/core.c (revision 777502)
+++ server/core.c (working copy)
@@ -661,7 +661,11 @@
core_dir_config *conf =
(core_dir_config *)ap_get_module_config(r->per_dir_config, &core_module);
- return conf->opts;
+ /* Per comment in http_core.h - the OPT_INC_WITH_EXEC bit is
+ * inverted, such that the exposed semantics match that of
+ * OPT_INCNOEXEC; i.e., the bit is only enabled if exec= is *not*
+ * permitted. */
+ return conf->opts ^ OPT_INC_WITH_EXEC;
}
AP_DECLARE(int) ap_allow_overrides(request_rec *r)
Re: [concept PATCH] CVE-2009-1195 tweaks to provide binary
compatibility for stable branches
Posted by Jeff Trawick <tr...@gmail.com>.
On Mon, May 25, 2009 at 3:45 AM, Joe Orton <jo...@redhat.com> wrote:
> On Fri, May 22, 2009 at 05:12:31PM -0400, Jeff Trawick wrote:
> > (untested)
> >
> > ap_allow_options() is how applications, including our mod_include, access
> > the enabled options for a given request (other than evil apps which
> define
> > CORE_PRIVATE and locate the core_dir_config). As this is a callable
> > function, it can map internal, hidden bitmaps as appropriate before
> > returning to the caller.
>
> Interesting idea! I'm concerned this is going to be overly intrusive,
> what with requiring the changes to all uses of OPT_ALL internally. Does
> it really matter what value of OPT_ALL is exposed to modules?
>
> (or specifically: does it break compatibility to change what value of
> OPT_ALL is exposed to modules?)
It seems safe to include the additional bit in OPT_ALL.
>
> Something simpler might be sufficient?
Err, I think you're looking for an overlap in sensibilities ;) (We know
that answer is "yes" just as we also know that we can make it abundantly
clear (painful) throughout whether we are dealing with the internal or
external representation.)
I'm fine with your patch plus a bit of commentary in ap_allow_options().
Thanks a bunch!