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 Thu, May 21, 2009 at 02:39:57PM -0400, Jeff Trawick wrote: On Wed, May 20, 2009 at 8:53 AM, Joe Orton jor...@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
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 I've now had a deeper look into this. I can't see a way to fix the problem without changing the semantics of the OPT_ bits used, as I mentioned briefly in my comment to Vincent. Status quo: a) OPT_INCLUDES is interpreted as SSI is allowed with exec= b) OPT_INCNOEXEC is interperted iff OPT_INCLUDES is also set as meaning SSI is allowed but exec= is not c) setting AllowOverride Options=IncludesNoExec results in both OPT_INCLUDES and OPT_INCNOEXEC being set in the -override_opts bitmask, i.e. either or both options can be overridden in .htaccess files From this leads the fact that an .htaccess file can set simply Options Includes in a context which inherits AllowOverride Options=IncludesNoExec. I'm presuming nobody will argue that's a feature not a bug? If so, I think this is the set of constraints which need to be satisfied: 1) the result of a config merge with only Options IncludesNoEXEC specified must not allow use of exec= in SSI 2) if AllowOverride Options= is used without Includes, notably, use of AllowOverride Options=IncludesNoExec, use of Options Includes in an .htaccess file must be an error 3) if AllowOverride Options=Includes is set, use of both Options Includes and Options IncludesNoExec must succeed and enable SSI with or without exec= respectively 4) if permitted by AllowOverride, setting Options Includes in a context from which Options IncludesNoExec is inherited, then the result must be one where exec= is allowed. Attached is a patch which passes the tests I have so far - Vincent, can you easily re-run your tests used to produce that lovely matrix, with this applied? 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
On Thu, May 21, 2009 at 3:25 PM, Jeff Trawick traw...@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.
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 Fri, May 22, 2009 at 10:17 AM, Jeff Trawick traw...@gmail.com wrote: On Thu, May 21, 2009 at 3:25 PM, Jeff Trawick traw...@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 d...@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
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.foert...@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
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
On Fri, May 22, 2009 at 2:46 PM, Torsten Foertsch torsten.foert...@gmx.netwrote: 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
On Fri, May 22, 2009 at 2:59 PM, William A. Rowe, Jr. wr...@rowe-clan.netwrote: 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
On Fri, May 22, 2009 at 4:21 PM, Jeff Trawick traw...@gmail.com wrote: 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 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
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
On Fri, May 22, 2009 at 5:10 PM, William A. Rowe, Jr. wr...@rowe-clan.netwrote: 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
On Wed, May 20, 2009 at 8:53 AM, Joe Orton jor...@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, cove...@apache.org wrote: Author: covener Date: Tue May 12 13:17:29 2009 New Revision: 773881 URL: http://svn.apache.org/viewvc?rev=773881view=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
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
On Thu, May 21, 2009 at 3:08 PM, William A. Rowe, Jr. wr...@rowe-clan.netwrote: 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
On Sun, May 17, 2009 at 11:15:00AM -0400, Jeff Trawick wrote: On Tue, May 12, 2009 at 9:17 AM, cove...@apache.org wrote: Author: covener Date: Tue May 12 13:17:29 2009 New Revision: 773881 URL: http://svn.apache.org/viewvc?rev=773881view=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
On Tue, May 12, 2009 at 9:17 AM, cove...@apache.org wrote: Author: covener Date: Tue May 12 13:17:29 2009 New Revision: 773881 URL: http://svn.apache.org/viewvc?rev=773881view=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) 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).