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/20 14:53:19 UTC

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

On Sun, May 17, 2009 at 11:15:00AM -0400, Jeff Trawick wrote:
> On Tue, May 12, 2009 at 9:17 AM, <co...@apache.org> wrote:
> 
> > Author: covener
> > Date: Tue May 12 13:17:29 2009
> > New Revision: 773881
> >
> > URL: http://svn.apache.org/viewvc?rev=773881&view=rev
> > Log:
> > backport 772997, 773322, 773342 from trunk.
> > Reviewed By: jorton, rpluem, covener
> >
> > 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=.
> 
> 
> Current mod_perl tarballs reference OPT_INC_WITH_EXEC as part of mapping the
> httpd API into perl, and the mod_perl build fails because of this.
>
> ("modperl_config.c", line 525: undefined symbol: OPT_INCNOEXEC)

Ick :( For some reason I thought this was hidden by CORE_PRIVATE, for 
what little that's worth.
 
> While I don't understand why the mod_perl mappings are created at release
> time against who knows what httpd, it brings up an interesting httpd issue
> anyway.
> 
> If some module does have OPT_INCNOEXEC baked in (32), it matches what
> 2.2.12+ thinks is OPT_INC_WITH_EXEC.  Similarly, the old OPT_INC_WITH_EXEC
> (previously called OPT_INCLUDES), maps what 2.2.12+ thinks is
> OPT_INCLUDES-without-exec.
> 
> We could swap the values of OPT_INCLUDES and OPT_INC_WITH_EXEC to lessen the
> chance of some theoretical module making the wrong decision.
> 
> We can also define OPT_INCNOEXEC to something (either the new OPT_INCLUDES
> or "Get your mod_perl patch at XXX").

Given that the semantics of the options has changed, I don't think it's 
worth changing httpd to maintain any pretence of compile-time or 
run-time compatibility here.  Any code using the OPT_* constants as 
exposed by mod_perl cannot work as expected any more.

Regards, Joe

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, May 22, 2009 at 2:46 PM, Torsten Foertsch
<to...@gmx.net>wrote:

> On Fri 22 May 2009, Jeff Trawick wrote:
> > Hmmm, after trying to use what seems like a cool feature, I find that
> > mod_perl was never taught to use the Apache 2's mod_include plug-in
> > interface.
>
> AFAIK, that is provided by Geoff's CPAN module Apache::IncludeHook or
> so.


Neat, and the use of [SSI_]FLAG_NO_EXEC in the filter context isn't affected
by this change.

FWIW, it looks like it won't work with httpd 2.2 (IncludeHook.xs uses
FLAG_NO_EXEC instead of SSI_FLAG_NO_EXEC) unless there's some compile-time
mapping going on somewhere that I don't see.

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, May 22, 2009 at 2:46 PM, Torsten Foertsch
<to...@gmx.net>wrote:

> On Fri 22 May 2009, Jeff Trawick wrote:
> > Hmmm, after trying to use what seems like a cool feature, I find that
> > mod_perl was never taught to use the Apache 2's mod_include plug-in
> > interface.
>
> AFAIK, that is provided by Geoff's CPAN module Apache::IncludeHook or
> so.


Neat, and the use of [SSI_]FLAG_NO_EXEC in the filter context isn't affected
by this change.

FWIW, it looks like it won't work with httpd 2.2 (IncludeHook.xs uses
FLAG_NO_EXEC instead of SSI_FLAG_NO_EXEC) unless there's some compile-time
mapping going on somewhere that I don't see.

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Torsten Foertsch <to...@gmx.net>.
On Fri 22 May 2009, Jeff Trawick wrote:
> Hmmm, after trying to use what seems like a cool feature, I find that
> mod_perl was never taught to use the Apache 2's mod_include plug-in
> interface.

AFAIK, that is provided by Geoff's CPAN module Apache::IncludeHook or 
so.

Torsten

-- 
Need professional mod_perl support?
Just hire me: torsten.foertsch@gmx.net

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Torsten Foertsch <to...@gmx.net>.
On Fri 22 May 2009, Jeff Trawick wrote:
> Hmmm, after trying to use what seems like a cool feature, I find that
> mod_perl was never taught to use the Apache 2's mod_include plug-in
> interface.

AFAIK, that is provided by Geoff's CPAN module Apache::IncludeHook or 
so.

Torsten

-- 
Need professional mod_perl support?
Just hire me: torsten.foertsch@gmx.net

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Fred Moyer <fr...@redhotpenguin.com>.
On Fri, May 22, 2009 at 10:17 AM, Jeff Trawick <tr...@gmail.com> wrote:
>
> On Thu, May 21, 2009 at 3:25 PM, Jeff Trawick <tr...@gmail.com> wrote:
>>
>> On Thu, May 21, 2009 at 3:08 PM, William A. Rowe, Jr.
>> <wr...@rowe-clan.net> wrote:
>>>
>>> Jeff Trawick wrote:
>>> > Does somebody else care to share their opinion on this?  Which of these
>>> > are okay?
>>> >
>>> > - existing mod_perl releases (and potentially other third-party
>>> > modules)
>>> > won't compile with 2.2.12
>>>
>>> CORE_PRIVATE may be broken from release to release, it's a necessary
>>> concession to prevent utter stagnation :(
>>
>> The bits are not CORE_PRIVATE.
>>
>> You can find sample Perl code on the web that even tests these bits,
>> though it isn't clear to me if that is a normal practice when using the
>> Perl/mod_include interface.
>
> Hmmm, after trying to use what seems like a cool feature, I find that
> mod_perl was never taught to use the Apache 2's mod_include plug-in
> interface.

Jeff can you post the sample code and some details to dev@perl.apache.org?

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Thu, May 21, 2009 at 3:25 PM, Jeff Trawick <tr...@gmail.com> wrote:

>
>
> On Thu, May 21, 2009 at 3:08 PM, William A. Rowe, Jr. <wrowe@rowe-clan.net
> > wrote:
>
>> Jeff Trawick wrote:
>> > Does somebody else care to share their opinion on this?  Which of these
>> > are okay?
>> >
>> > - existing mod_perl releases (and potentially other third-party modules)
>> > won't compile with 2.2.12
>>
>> CORE_PRIVATE may be broken from release to release, it's a necessary
>> concession to prevent utter stagnation :(
>
>
> The bits are not CORE_PRIVATE.
>
> You can find sample Perl code on the web that even tests these bits, though
> it isn't clear to me if that is a normal practice when using the
> Perl/mod_include interface.
>

Hmmm, after trying to use what seems like a cool feature, I find that
mod_perl was never taught to use the Apache 2's mod_include plug-in
interface.

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Thu, May 21, 2009 at 3:25 PM, Jeff Trawick <tr...@gmail.com> wrote:

>
>
> On Thu, May 21, 2009 at 3:08 PM, William A. Rowe, Jr. <wrowe@rowe-clan.net
> > wrote:
>
>> Jeff Trawick wrote:
>> > Does somebody else care to share their opinion on this?  Which of these
>> > are okay?
>> >
>> > - existing mod_perl releases (and potentially other third-party modules)
>> > won't compile with 2.2.12
>>
>> CORE_PRIVATE may be broken from release to release, it's a necessary
>> concession to prevent utter stagnation :(
>
>
> The bits are not CORE_PRIVATE.
>
> You can find sample Perl code on the web that even tests these bits, though
> it isn't clear to me if that is a normal practice when using the
> Perl/mod_include interface.
>

Hmmm, after trying to use what seems like a cool feature, I find that
mod_perl was never taught to use the Apache 2's mod_include plug-in
interface.

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, May 22, 2009 at 12:33 PM, Fred Moyer <fr...@redhotpenguin.com> wrote:

> On Thu, May 21, 2009 at 12:25 PM, Jeff Trawick <tr...@gmail.com> wrote:
> > On Thu, May 21, 2009 at 3:08 PM, William A. Rowe, Jr. <
> wrowe@rowe-clan.net>
> > wrote:
> >> Jeff Trawick wrote:
> >> > Does somebody else care to share their opinion on this?  Which of
> these
> >> > are okay?
> >> > - existing mod_perl releases (and potentially other third-party
> modules)
> >> > won't compile with 2.2.12
> >> CORE_PRIVATE may be broken from release to release, it's a necessary
> >> concession to prevent utter stagnation :(
> >
> > The bits are not CORE_PRIVATE.
> >
> > You can find sample Perl code on the web that even tests these bits,
> though
> > it isn't clear to me if that is a normal practice when using the
> > Perl/mod_include interface.
>
> Jeff would you be so kind as to post a link to this sample code so
> that those of us here without a clue (myself) can understand the issue
> in depth?


search for ->mod_perl allow_options opt_incnoexec<- or similar and weed out
the uninteresting search hits

these seems to be an example of real code

http://cpansearch.perl.org/src/KWILLIAMS/Apache-SSI-2.19/lib/Apache/SSI.pm
http://cpansearch.perl.org/src/KWILLIAMS/Apache-SSI-2.19/lib/Apache/FakeSSI.pm

(I thought I found an pure coding example in an illegally posted copy of an
O'Reilly book, but I can't find ATM ;) )

Fwd: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Fred Moyer <fr...@redhotpenguin.com>.
Forwarding sent mail - thought I was replying to dev@perl but it was dev@httpd


---------- Forwarded message ----------
From: Fred Moyer <fr...@redhotpenguin.com>
Date: Fri, May 22, 2009 at 9:33 AM
Subject: Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x:
CHANGES STATUS include/http_core.h modules/filters/mod_include.c
server/config.c server/core.c
To: Jeff Trawick <tr...@gmail.com>
Cc: dev@httpd.apache.org


On Thu, May 21, 2009 at 12:25 PM, Jeff Trawick <tr...@gmail.com> wrote:
> On Thu, May 21, 2009 at 3:08 PM, William A. Rowe, Jr. <wr...@rowe-clan.net>
> wrote:
>> Jeff Trawick wrote:
>> > Does somebody else care to share their opinion on this?  Which of these
>> > are okay?
>> > - existing mod_perl releases (and potentially other third-party modules)
>> > won't compile with 2.2.12
>> CORE_PRIVATE may be broken from release to release, it's a necessary
>> concession to prevent utter stagnation :(
>
> The bits are not CORE_PRIVATE.
>
> You can find sample Perl code on the web that even tests these bits, though
> it isn't clear to me if that is a normal practice when using the
> Perl/mod_include interface.

Jeff would you be so kind as to post a link to this sample code so
that those of us here without a clue (myself) can understand the issue
in depth?




>>
>>
>> I believe it was a mistake that this particular symbol/this particular
>> directive is not a part of the mod_includes internals :(
>
> Perhaps, though mod_include does have a plug-in interface and we have this
> non-internal-detail-sounding function called ap_allow_options().  The
> include option variants could be interesting to such a plug-in.
>
>
>>
>>
>> So given we have a .23 mmn bump, perhaps document this in that section.
>> But the actual behavior of this flag changes significantly and I can't
>> see how to properly maintain mod_perl, deep internal compatibility.
>>
>
> The requirement is to fix combinations of option specifications in the main
> conf file and .htaccess.  There's nothing incompatible with mod_perl there.
> We just can't change the meaning of existing bits.
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@perl.apache.org
For additional commands, e-mail: dev-help@perl.apache.org


Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Thu, May 21, 2009 at 3:08 PM, William A. Rowe, Jr.
<wr...@rowe-clan.net>wrote:

> Jeff Trawick wrote:
> > Does somebody else care to share their opinion on this?  Which of these
> > are okay?
> >
> > - existing mod_perl releases (and potentially other third-party modules)
> > won't compile with 2.2.12
>
> CORE_PRIVATE may be broken from release to release, it's a necessary
> concession to prevent utter stagnation :(


The bits are not CORE_PRIVATE.

You can find sample Perl code on the web that even tests these bits, though
it isn't clear to me if that is a normal practice when using the
Perl/mod_include interface.

>
>
> I believe it was a mistake that this particular symbol/this particular
> directive is not a part of the mod_includes internals :(


Perhaps, though mod_include does have a plug-in interface and we have this
non-internal-detail-sounding function called ap_allow_options().  The
include option variants could be interesting to such a plug-in.



>
>
> So given we have a .23 mmn bump, perhaps document this in that section.
> But the actual behavior of this flag changes significantly and I can't
> see how to properly maintain mod_perl, deep internal compatibility.
>
>
The requirement is to fix combinations of option specifications in the main
conf file and .htaccess.  There's nothing incompatible with mod_perl there.
We just can't change the meaning of existing bits.

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Thu, May 21, 2009 at 3:08 PM, William A. Rowe, Jr.
<wr...@rowe-clan.net>wrote:

> Jeff Trawick wrote:
> > Does somebody else care to share their opinion on this?  Which of these
> > are okay?
> >
> > - existing mod_perl releases (and potentially other third-party modules)
> > won't compile with 2.2.12
>
> CORE_PRIVATE may be broken from release to release, it's a necessary
> concession to prevent utter stagnation :(


The bits are not CORE_PRIVATE.

You can find sample Perl code on the web that even tests these bits, though
it isn't clear to me if that is a normal practice when using the
Perl/mod_include interface.

>
>
> I believe it was a mistake that this particular symbol/this particular
> directive is not a part of the mod_includes internals :(


Perhaps, though mod_include does have a plug-in interface and we have this
non-internal-detail-sounding function called ap_allow_options().  The
include option variants could be interesting to such a plug-in.



>
>
> So given we have a .23 mmn bump, perhaps document this in that section.
> But the actual behavior of this flag changes significantly and I can't
> see how to properly maintain mod_perl, deep internal compatibility.
>
>
The requirement is to fix combinations of option specifications in the main
conf file and .htaccess.  There's nothing incompatible with mod_perl there.
We just can't change the meaning of existing bits.

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Jeff Trawick wrote:
> Does somebody else care to share their opinion on this?  Which of these
> are okay?
> 
> - existing mod_perl releases (and potentially other third-party modules)
> won't compile with 2.2.12

CORE_PRIVATE may be broken from release to release, it's a necessary
concession to prevent utter stagnation :(

I believe it was a mistake that this particular symbol/this particular
directive is not a part of the mod_includes internals :(

So given we have a .23 mmn bump, perhaps document this in that section.
But the actual behavior of this flag changes significantly and I can't
see how to properly maintain mod_perl, deep internal compatibility.


Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, May 22, 2009 at 4:21 PM, Jeff Trawick <tr...@gmail.com> wrote:

>
>
> On Fri, May 22, 2009 at 2:59 PM, William A. Rowe, Jr. <wrowe@rowe-clan.net
> > wrote:
>
>> Joe Orton wrote:
>> >
>> > Having thought about this longer, I do agree that it would be reasonable
>> > to provide OPT_INCNOEXEC as a noop integer for back-compat, but, it
>> > turns out we're out of bits - allow_options_t is an unsigned char and
>> > we're using 2^0 through 2^7 already. :(
>>
>> The C langauge promotes char -> int for comparison.  256 should work fine,
>> no?  It would devolve to 0, of course, but 256 & 255 should test fine.
>>
>> Thoughts?
>
>
> Backing up a bit...
>
> I originally thought we could map bit values in 2.2.x to avoid affecting
> modules, but that isn't possible since includes-with-exec is two bits
> instead of one.
>
> Mapping OPT_INCNOEXEC to a no-op integer is something that takes place at
> compile time, and helps applications which reference the symbol but don't
> use it in any important way.  (IOW, let mod_perl and other similar tarballs
> compile.)  It is good in that it lets mod_perl compile, but bad in that
> mod_perl continues to export the Perl mapping of OPT_INCNOEXEC even after
> httpd has been upgraded and at some point later mod_perl is upgraded.
>
> Failing the compile is our only opportunity to catch some affected modules
> (though it is a rather late opportunity since the modules will likely be
> rebuilt later since they're supposed to work as-is when upgrading httpd;
> somebody will grumble though).
>
> I don't think we should try to preserve compilability if we can't preserve
> compatibility.
>
>
>>
>> > The only available option is to #define OPT_INCNOEXEC to some bogus
>> > string or something; not sure I like that much better than just a clean
>> > break.
>>
>
> /*
>  * #define OPT_INCNOEXEC  32
>  * Apache 2.2.12 and later no longer provide this.
>  * Applications which distinguish between includes-without-exec and
> includes-with-exec
>  * must use different logic for 2.2.<12 and 2.2.>=12.
>  * Prior to 2.2.12:
>  *   includes-without-exec: OPT_INCNOEXEC flag on, OPT_INCLUDES flag off
>

oops, both flags were on here


>
>  *   includes-with-exec:     OPT_INCNOEXEC flag off, OPT_INCLUDES flag on
>  * As of 2.2.12:
>  *   includes-without-exec: OPT_INCLUDES flag on, OPT_INC_WITH_EXEC flag
> off
>  *   includes-with-exec: OPT_INCLUDES flag on, OPT_INC_WITH_EXEC flag on
> *
> */
>

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, May 22, 2009 at 4:21 PM, Jeff Trawick <tr...@gmail.com> wrote:

>
>
> On Fri, May 22, 2009 at 2:59 PM, William A. Rowe, Jr. <wrowe@rowe-clan.net
> > wrote:
>
>> Joe Orton wrote:
>> >
>> > Having thought about this longer, I do agree that it would be reasonable
>> > to provide OPT_INCNOEXEC as a noop integer for back-compat, but, it
>> > turns out we're out of bits - allow_options_t is an unsigned char and
>> > we're using 2^0 through 2^7 already. :(
>>
>> The C langauge promotes char -> int for comparison.  256 should work fine,
>> no?  It would devolve to 0, of course, but 256 & 255 should test fine.
>>
>> Thoughts?
>
>
> Backing up a bit...
>
> I originally thought we could map bit values in 2.2.x to avoid affecting
> modules, but that isn't possible since includes-with-exec is two bits
> instead of one.
>
> Mapping OPT_INCNOEXEC to a no-op integer is something that takes place at
> compile time, and helps applications which reference the symbol but don't
> use it in any important way.  (IOW, let mod_perl and other similar tarballs
> compile.)  It is good in that it lets mod_perl compile, but bad in that
> mod_perl continues to export the Perl mapping of OPT_INCNOEXEC even after
> httpd has been upgraded and at some point later mod_perl is upgraded.
>
> Failing the compile is our only opportunity to catch some affected modules
> (though it is a rather late opportunity since the modules will likely be
> rebuilt later since they're supposed to work as-is when upgrading httpd;
> somebody will grumble though).
>
> I don't think we should try to preserve compilability if we can't preserve
> compatibility.
>
>
>>
>> > The only available option is to #define OPT_INCNOEXEC to some bogus
>> > string or something; not sure I like that much better than just a clean
>> > break.
>>
>
> /*
>  * #define OPT_INCNOEXEC  32
>  * Apache 2.2.12 and later no longer provide this.
>  * Applications which distinguish between includes-without-exec and
> includes-with-exec
>  * must use different logic for 2.2.<12 and 2.2.>=12.
>  * Prior to 2.2.12:
>  *   includes-without-exec: OPT_INCNOEXEC flag on, OPT_INCLUDES flag off
>

oops, both flags were on here


>
>  *   includes-with-exec:     OPT_INCNOEXEC flag off, OPT_INCLUDES flag on
>  * As of 2.2.12:
>  *   includes-without-exec: OPT_INCLUDES flag on, OPT_INC_WITH_EXEC flag
> off
>  *   includes-with-exec: OPT_INCLUDES flag on, OPT_INC_WITH_EXEC flag on
> *
> */
>

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, May 22, 2009 at 5:10 PM, William A. Rowe, Jr.
<wr...@rowe-clan.net>wrote:

> Jeff Trawick wrote:
> >
> > Backing up a bit...
> >
> > I originally thought we could map bit values in 2.2.x to avoid affecting
> > modules, but that isn't possible since includes-with-exec is two bits
> > instead of one.
>
> Hold on... I think this can still work;
>
>  * Retain new true 'Includes' bit as old IncludesNoExec macro value
>    Keep ancient Includes flag bit as 256, never true.
>
>  - all httpd modules testing for including but not executing
>    permission see the permission as allowed
>
>  - old httpd modules testing for includes with exec permission
>    see the permission as denied, until they update the module
>
>  - httpd modules which force/override the includes without exec
>    permission would still work
>
>  - httpd modules which force/override the includes exec behavior
>    would just fail to update anything (256 & 0xff == 00), so it
>    becomes a noop until they update the module
>
> So it has no negative security consequences, still would require
> an update to the rare module, but lets us ship something without
> really nasty side effects.
>

I'll think harder about this once my latest proposal gets shot down ;)

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, May 22, 2009 at 5:10 PM, William A. Rowe, Jr.
<wr...@rowe-clan.net>wrote:

> Jeff Trawick wrote:
> >
> > Backing up a bit...
> >
> > I originally thought we could map bit values in 2.2.x to avoid affecting
> > modules, but that isn't possible since includes-with-exec is two bits
> > instead of one.
>
> Hold on... I think this can still work;
>
>  * Retain new true 'Includes' bit as old IncludesNoExec macro value
>    Keep ancient Includes flag bit as 256, never true.
>
>  - all httpd modules testing for including but not executing
>    permission see the permission as allowed
>
>  - old httpd modules testing for includes with exec permission
>    see the permission as denied, until they update the module
>
>  - httpd modules which force/override the includes without exec
>    permission would still work
>
>  - httpd modules which force/override the includes exec behavior
>    would just fail to update anything (256 & 0xff == 00), so it
>    becomes a noop until they update the module
>
> So it has no negative security consequences, still would require
> an update to the rare module, but lets us ship something without
> really nasty side effects.
>

I'll think harder about this once my latest proposal gets shot down ;)

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Jeff Trawick wrote:
> 
> Backing up a bit...
> 
> I originally thought we could map bit values in 2.2.x to avoid affecting
> modules, but that isn't possible since includes-with-exec is two bits
> instead of one.

Hold on... I think this can still work;

  * Retain new true 'Includes' bit as old IncludesNoExec macro value
    Keep ancient Includes flag bit as 256, never true.

  - all httpd modules testing for including but not executing
    permission see the permission as allowed

  - old httpd modules testing for includes with exec permission
    see the permission as denied, until they update the module

  - httpd modules which force/override the includes without exec
    permission would still work

  - httpd modules which force/override the includes exec behavior
    would just fail to update anything (256 & 0xff == 00), so it
    becomes a noop until they update the module

So it has no negative security consequences, still would require
an update to the rare module, but lets us ship something without
really nasty side effects.




Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Jeff Trawick wrote:
> 
> Backing up a bit...
> 
> I originally thought we could map bit values in 2.2.x to avoid affecting
> modules, but that isn't possible since includes-with-exec is two bits
> instead of one.

Hold on... I think this can still work;

  * Retain new true 'Includes' bit as old IncludesNoExec macro value
    Keep ancient Includes flag bit as 256, never true.

  - all httpd modules testing for including but not executing
    permission see the permission as allowed

  - old httpd modules testing for includes with exec permission
    see the permission as denied, until they update the module

  - httpd modules which force/override the includes without exec
    permission would still work

  - httpd modules which force/override the includes exec behavior
    would just fail to update anything (256 & 0xff == 00), so it
    becomes a noop until they update the module

So it has no negative security consequences, still would require
an update to the rare module, but lets us ship something without
really nasty side effects.




Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, May 22, 2009 at 2:59 PM, William A. Rowe, Jr.
<wr...@rowe-clan.net>wrote:

> Joe Orton wrote:
> >
> > Having thought about this longer, I do agree that it would be reasonable
> > to provide OPT_INCNOEXEC as a noop integer for back-compat, but, it
> > turns out we're out of bits - allow_options_t is an unsigned char and
> > we're using 2^0 through 2^7 already. :(
>
> The C langauge promotes char -> int for comparison.  256 should work fine,
> no?  It would devolve to 0, of course, but 256 & 255 should test fine.
>
> Thoughts?


Backing up a bit...

I originally thought we could map bit values in 2.2.x to avoid affecting
modules, but that isn't possible since includes-with-exec is two bits
instead of one.

Mapping OPT_INCNOEXEC to a no-op integer is something that takes place at
compile time, and helps applications which reference the symbol but don't
use it in any important way.  (IOW, let mod_perl and other similar tarballs
compile.)  It is good in that it lets mod_perl compile, but bad in that
mod_perl continues to export the Perl mapping of OPT_INCNOEXEC even after
httpd has been upgraded and at some point later mod_perl is upgraded.

Failing the compile is our only opportunity to catch some affected modules
(though it is a rather late opportunity since the modules will likely be
rebuilt later since they're supposed to work as-is when upgrading httpd;
somebody will grumble though).

I don't think we should try to preserve compilability if we can't preserve
compatibility.


>
> > The only available option is to #define OPT_INCNOEXEC to some bogus
> > string or something; not sure I like that much better than just a clean
> > break.
>

/*
 * #define OPT_INCNOEXEC  32
 * Apache 2.2.12 and later no longer provide this.
 * Applications which distinguish between includes-without-exec and
includes-with-exec
 * must use different logic for 2.2.<12 and 2.2.>=12.
 * Prior to 2.2.12:
 *   includes-without-exec: OPT_INCNOEXEC flag on, OPT_INCLUDES flag off
 *   includes-with-exec:     OPT_INCNOEXEC flag off, OPT_INCLUDES flag on
 * As of 2.2.12:
 *   includes-without-exec: OPT_INCLUDES flag on, OPT_INC_WITH_EXEC flag off
 *   includes-with-exec: OPT_INCLUDES flag on, OPT_INC_WITH_EXEC flag on
*
*/

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, May 22, 2009 at 2:59 PM, William A. Rowe, Jr.
<wr...@rowe-clan.net>wrote:

> Joe Orton wrote:
> >
> > Having thought about this longer, I do agree that it would be reasonable
> > to provide OPT_INCNOEXEC as a noop integer for back-compat, but, it
> > turns out we're out of bits - allow_options_t is an unsigned char and
> > we're using 2^0 through 2^7 already. :(
>
> The C langauge promotes char -> int for comparison.  256 should work fine,
> no?  It would devolve to 0, of course, but 256 & 255 should test fine.
>
> Thoughts?


Backing up a bit...

I originally thought we could map bit values in 2.2.x to avoid affecting
modules, but that isn't possible since includes-with-exec is two bits
instead of one.

Mapping OPT_INCNOEXEC to a no-op integer is something that takes place at
compile time, and helps applications which reference the symbol but don't
use it in any important way.  (IOW, let mod_perl and other similar tarballs
compile.)  It is good in that it lets mod_perl compile, but bad in that
mod_perl continues to export the Perl mapping of OPT_INCNOEXEC even after
httpd has been upgraded and at some point later mod_perl is upgraded.

Failing the compile is our only opportunity to catch some affected modules
(though it is a rather late opportunity since the modules will likely be
rebuilt later since they're supposed to work as-is when upgrading httpd;
somebody will grumble though).

I don't think we should try to preserve compilability if we can't preserve
compatibility.


>
> > The only available option is to #define OPT_INCNOEXEC to some bogus
> > string or something; not sure I like that much better than just a clean
> > break.
>

/*
 * #define OPT_INCNOEXEC  32
 * Apache 2.2.12 and later no longer provide this.
 * Applications which distinguish between includes-without-exec and
includes-with-exec
 * must use different logic for 2.2.<12 and 2.2.>=12.
 * Prior to 2.2.12:
 *   includes-without-exec: OPT_INCNOEXEC flag on, OPT_INCLUDES flag off
 *   includes-with-exec:     OPT_INCNOEXEC flag off, OPT_INCLUDES flag on
 * As of 2.2.12:
 *   includes-without-exec: OPT_INCLUDES flag on, OPT_INC_WITH_EXEC flag off
 *   includes-with-exec: OPT_INCLUDES flag on, OPT_INC_WITH_EXEC flag on
*
*/

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Joe Orton wrote:
> 
> Having thought about this longer, I do agree that it would be reasonable 
> to provide OPT_INCNOEXEC as a noop integer for back-compat, but, it 
> turns out we're out of bits - allow_options_t is an unsigned char and 
> we're using 2^0 through 2^7 already. :(

The C langauge promotes char -> int for comparison.  256 should work fine,
no?  It would devolve to 0, of course, but 256 & 255 should test fine.

Thoughts?

> The only available option is to #define OPT_INCNOEXEC to some bogus 
> string or something; not sure I like that much better than just a clean 
> break.


Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Joe Orton <jo...@redhat.com>.
On Fri, May 22, 2009 at 05:26:07PM +0100, Joe Orton wrote:
> Attaching my original analysis for security@ which hopefully answers 
> that question ;)

attempt 2

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Joe Orton wrote:
> 
> Having thought about this longer, I do agree that it would be reasonable 
> to provide OPT_INCNOEXEC as a noop integer for back-compat, but, it 
> turns out we're out of bits - allow_options_t is an unsigned char and 
> we're using 2^0 through 2^7 already. :(

The C langauge promotes char -> int for comparison.  256 should work fine,
no?  It would devolve to 0, of course, but 256 & 255 should test fine.

Thoughts?

> The only available option is to #define OPT_INCNOEXEC to some bogus 
> string or something; not sure I like that much better than just a clean 
> break.


Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Joe Orton <jo...@redhat.com>.
On Fri, May 22, 2009 at 05:26:07PM +0100, Joe Orton wrote:
> Attaching my original analysis for security@ which hopefully answers 
> that question ;)

attempt 2

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Joe Orton <jo...@redhat.com>.
On Thu, May 21, 2009 at 02:39:57PM -0400, Jeff Trawick wrote:
> On Wed, May 20, 2009 at 8:53 AM, Joe Orton <jo...@redhat.com> wrote:
> > Given that the semantics of the options has changed, I don't think it's
> > worth changing httpd to maintain any pretence of compile-time or
> > run-time compatibility here.  Any code using the OPT_* constants as
> > exposed by mod_perl cannot work as expected any more.
> 
> Is the change in semantics required to fix the bug, or is it simply the
> current implementation?

Attaching my original analysis for security@ which hopefully answers 
that question ;)

Having thought about this longer, I do agree that it would be reasonable 
to provide OPT_INCNOEXEC as a noop integer for back-compat, but, it 
turns out we're out of bits - allow_options_t is an unsigned char and 
we're using 2^0 through 2^7 already. :(

The only available option is to #define OPT_INCNOEXEC to some bogus 
string or something; not sure I like that much better than just a clean 
break.

Worth noting: for any mod_perl-based code which tests only OPT_INCLUDE 
for "is SSI enabled", that will continue to be compatible with the new 
implementation, modulo mod_perl build failures.  The only issue is with 
code which needs to differentiate between SSI-with-exec and -without.

Regards, Joe

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by "William A. Rowe, Jr." <wr...@rowe-clan.net>.
Jeff Trawick wrote:
> Does somebody else care to share their opinion on this?  Which of these
> are okay?
> 
> - existing mod_perl releases (and potentially other third-party modules)
> won't compile with 2.2.12

CORE_PRIVATE may be broken from release to release, it's a necessary
concession to prevent utter stagnation :(

I believe it was a mistake that this particular symbol/this particular
directive is not a part of the mod_includes internals :(

So given we have a .23 mmn bump, perhaps document this in that section.
But the actual behavior of this flag changes significantly and I can't
see how to properly maintain mod_perl, deep internal compatibility.


Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Joe Orton <jo...@redhat.com>.
On Thu, May 21, 2009 at 02:39:57PM -0400, Jeff Trawick wrote:
> On Wed, May 20, 2009 at 8:53 AM, Joe Orton <jo...@redhat.com> wrote:
> > Given that the semantics of the options has changed, I don't think it's
> > worth changing httpd to maintain any pretence of compile-time or
> > run-time compatibility here.  Any code using the OPT_* constants as
> > exposed by mod_perl cannot work as expected any more.
> 
> Is the change in semantics required to fix the bug, or is it simply the
> current implementation?

Attaching my original analysis for security@ which hopefully answers 
that question ;)

Having thought about this longer, I do agree that it would be reasonable 
to provide OPT_INCNOEXEC as a noop integer for back-compat, but, it 
turns out we're out of bits - allow_options_t is an unsigned char and 
we're using 2^0 through 2^7 already. :(

The only available option is to #define OPT_INCNOEXEC to some bogus 
string or something; not sure I like that much better than just a clean 
break.

Worth noting: for any mod_perl-based code which tests only OPT_INCLUDE 
for "is SSI enabled", that will continue to be compatible with the new 
implementation, modulo mod_perl build failures.  The only issue is with 
code which needs to differentiate between SSI-with-exec and -without.

Regards, Joe

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Thu, May 21, 2009 at 4:18 PM, Eric Covener <co...@gmail.com> wrote:

> On Thu, May 21, 2009 at 2:39 PM, Jeff Trawick <tr...@gmail.com> wrote:
> > - existing Perl modules (and potentially other third-party modules) will
> > confuse include-with-exec and include-without-exec
>
> Any better to leave the old values of SSI/SSINOEXEC as unused  in
> 2.2.x and choose high numbers for the new macros?
>

I think that is something of an improvement in that some modules which would
make the wrong decision with the current code would flat out fail with your
suggestion (it wouldn't think includes were enabled at all).

Then somebody complains to the module author and we tell them that they need
to use different bit checks based on the 2.2.x version.  (This really makes
no sense.)

(Oh, and no swapping levels of mod_include.so as a diagnostic step.)

(I should shut up now until/unless I roll up my sleeves; it is disrespectful
of the effort that went into this.)

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Eric Covener <co...@gmail.com>.
On Thu, May 21, 2009 at 2:39 PM, Jeff Trawick <tr...@gmail.com> wrote:
> - existing Perl modules (and potentially other third-party modules) will
> confuse include-with-exec and include-without-exec

Any better to leave the old values of SSI/SSINOEXEC as unused  in
2.2.x and choose high numbers for the new macros?


-- 
Eric Covener
covener@gmail.com

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, May 20, 2009 at 8:53 AM, Joe Orton <jo...@redhat.com> wrote:

> On Sun, May 17, 2009 at 11:15:00AM -0400, Jeff Trawick wrote:
> > On Tue, May 12, 2009 at 9:17 AM, <co...@apache.org> wrote:
> >
> > > Author: covener
> > > Date: Tue May 12 13:17:29 2009
> > > New Revision: 773881
> > >
> > > URL: http://svn.apache.org/viewvc?rev=773881&view=rev
> > > Log:
> > > backport 772997, 773322, 773342 from trunk.
> > > Reviewed By: jorton, rpluem, covener
> > >
> > > 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=.
> >
> >
> > Current mod_perl tarballs reference OPT_INC_WITH_EXEC as part of mapping
> the
> > httpd API into perl, and the mod_perl build fails because of this.
> >
> > ("modperl_config.c", line 525: undefined symbol: OPT_INCNOEXEC)
>
> Ick :( For some reason I thought this was hidden by CORE_PRIVATE, for
> what little that's worth.
>
> > While I don't understand why the mod_perl mappings are created at release
> > time against who knows what httpd, it brings up an interesting httpd
> issue
> > anyway.
> >
> > If some module does have OPT_INCNOEXEC baked in (32), it matches what
> > 2.2.12+ thinks is OPT_INC_WITH_EXEC.  Similarly, the old
> OPT_INC_WITH_EXEC
> > (previously called OPT_INCLUDES), maps what 2.2.12+ thinks is
> > OPT_INCLUDES-without-exec.
> >
> > We could swap the values of OPT_INCLUDES and OPT_INC_WITH_EXEC to lessen
> the
> > chance of some theoretical module making the wrong decision.
> >
> > We can also define OPT_INCNOEXEC to something (either the new
> OPT_INCLUDES
> > or "Get your mod_perl patch at XXX").
>
> Given that the semantics of the options has changed, I don't think it's
> worth changing httpd to maintain any pretence of compile-time or
> run-time compatibility here.  Any code using the OPT_* constants as
> exposed by mod_perl cannot work as expected any more.
>
> Regards, Joe
>

Is the change in semantics required to fix the bug, or is it simply the
current implementation?

As these constants and the related ap_allow_options() have been exposed to
the C API for eons, and passed through in API mappings such as mod_perl, it
is worth making an alternate fix to avoid breaking module compiles and
(potentially) module misbehavior when upgrading from 2.2.11 to 2.2.12.

Unfortunately I don't have a patch :(

Does somebody else care to share their opinion on this?  Which of these are
okay?

- existing mod_perl releases (and potentially other third-party modules)
won't compile with 2.2.12
- existing Perl modules (and potentially other third-party modules) will
confuse include-with-exec and include-without-exec

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules/filters/mod_include.c server/config.c server/core.c

Posted by Jeff Trawick <tr...@gmail.com>.
On Wed, May 20, 2009 at 8:53 AM, Joe Orton <jo...@redhat.com> wrote:

> On Sun, May 17, 2009 at 11:15:00AM -0400, Jeff Trawick wrote:
> > On Tue, May 12, 2009 at 9:17 AM, <co...@apache.org> wrote:
> >
> > > Author: covener
> > > Date: Tue May 12 13:17:29 2009
> > > New Revision: 773881
> > >
> > > URL: http://svn.apache.org/viewvc?rev=773881&view=rev
> > > Log:
> > > backport 772997, 773322, 773342 from trunk.
> > > Reviewed By: jorton, rpluem, covener
> > >
> > > 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=.
> >
> >
> > Current mod_perl tarballs reference OPT_INC_WITH_EXEC as part of mapping
> the
> > httpd API into perl, and the mod_perl build fails because of this.
> >
> > ("modperl_config.c", line 525: undefined symbol: OPT_INCNOEXEC)
>
> Ick :( For some reason I thought this was hidden by CORE_PRIVATE, for
> what little that's worth.
>
> > While I don't understand why the mod_perl mappings are created at release
> > time against who knows what httpd, it brings up an interesting httpd
> issue
> > anyway.
> >
> > If some module does have OPT_INCNOEXEC baked in (32), it matches what
> > 2.2.12+ thinks is OPT_INC_WITH_EXEC.  Similarly, the old
> OPT_INC_WITH_EXEC
> > (previously called OPT_INCLUDES), maps what 2.2.12+ thinks is
> > OPT_INCLUDES-without-exec.
> >
> > We could swap the values of OPT_INCLUDES and OPT_INC_WITH_EXEC to lessen
> the
> > chance of some theoretical module making the wrong decision.
> >
> > We can also define OPT_INCNOEXEC to something (either the new
> OPT_INCLUDES
> > or "Get your mod_perl patch at XXX").
>
> Given that the semantics of the options has changed, I don't think it's
> worth changing httpd to maintain any pretence of compile-time or
> run-time compatibility here.  Any code using the OPT_* constants as
> exposed by mod_perl cannot work as expected any more.
>
> Regards, Joe
>

Is the change in semantics required to fix the bug, or is it simply the
current implementation?

As these constants and the related ap_allow_options() have been exposed to
the C API for eons, and passed through in API mappings such as mod_perl, it
is worth making an alternate fix to avoid breaking module compiles and
(potentially) module misbehavior when upgrading from 2.2.11 to 2.2.12.

Unfortunately I don't have a patch :(

Does somebody else care to share their opinion on this?  Which of these are
okay?

- existing mod_perl releases (and potentially other third-party modules)
won't compile with 2.2.12
- existing Perl modules (and potentially other third-party modules) will
confuse include-with-exec and include-without-exec