Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES

--000e0cd25016855441046a70786d
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit

On Wed, May 20, 2009 at 8:53 AM, Joe Orton wrote:

> On Sun, May 17, 2009 at 11:15:00AM -0400, Jeff Trawick wrote:
> > On Tue, May 12, 2009 at 9:17 AM, 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

--000e0cd25016855441046a70786d
Content-Type: text/html; charset=ISO-8859-1
Content-Transfer-Encoding: quoted-printable

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGES STATUS include/http_core.h modules

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

On Thu, May 21, 2009 at 02:39:57PM -0400, Jeff Trawick wrote:
> On Wed, May 20, 2009 at 8:53 AM, Joe Orton 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

--sm4nu43k4a2Rpi4c
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline

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

--sm4nu43k4a2Rpi4c
Content-Type: text/plain; charset=utf-8
Content-Disposition: attachment; filename="ssi.foo"


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


--sm4nu43k4a2Rpi4c--

Re: svn commit: r773881 - in /httpd/httpd/branches/2.2.x: CHANGESSTATUS include/http_core.h modules/

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.