Re: Feedback needed: suexec different-owner patch

2016-04-01 Thread monttyle

On 2016-03-30 16:35, Jacob Champion wrote:

Sorry, but that is not a good approach. You must assume that a local
attacker calls suexec directly and passes arguments of his liking.
That is the attack vector that suexec's rather annoying restrictions
try to avoid.


Checking my own understanding... so if an attacker is able to make
arbitrary calls to suexec as the httpd user (which requires another
vulnerability to begin with) then with this patch, they are able to
execute any scripts in the DocumentRoot as any user. Whereas before 
they
would have been limited to running scripts only as their owners. Is 
that

correct?


Yes, it's a plausible exploit.


Other possible issues, not necessarily security-related:

- It looks like the args passed to execv[e]() begin with the original
argv[5] (the trusted group name) instead of the original argv[3] (the
command to execute).


I was quite careful to add the new arguments to the end.  That way,
suexec can tell the difference between the new and old syntax
by the number of arguments.



- There is also a compatibility change for suexec itself:

suexec target_uname target_gname cmd arg1 arg2 arg3

will now interpret "arg1" and "arg2" as the trusted user and group,
respectively. I don't know if suexec has any compatibility guarantees
from release to release.

(monttyle, please correct me if I've misread the patch.)


CGI being CGI, you don't really get arg1 arg2 arg3 in practice as I 
understand it.

Data is passed into stdin instead.


A third approach from the original IRC discussion was to simply compile
the "trusted" user/group directly into suexec, as another configured 
option.


--Jacob


You thought of it first...  That's probably what I'll try next.


Re: Feedback needed: suexec different-owner patch

2016-04-01 Thread monttyle

On 2016-03-30 14:49, Stefan Fritsch wrote:

On Saturday 19 March 2016 11:09:40, montt...@heavyspace.ca wrote:

Since its been a while since this issue was mentioned, this patch
allows Apache to suexec files by a different (but still restricted
by UID) owner, to avoid the security issue where apache forces you
to suexec to files it has full chmod access to.

 Original Message 
Subject: suexec different-owner patch



Criticisms, please?


You are doing the configuration parsing in httpd, and then pass the
allowed uid/group to suexec as command line arguments.
Sorry, but that is not a good approach. You must assume that a local
attacker calls suexec directly and passes arguments of his liking.
That is the attack vector that suexec's rather annoying restrictions
try to avoid.


I begin to see.  Instead of a situation that can't be exploited
without changing a file's ownership, it's a situation which can be
exploited by poking it until the right values are fed to it.  Not good.


So the config file parsing would have to be done inside suexec, with
the config file path being compiled into the suexec utility. Of
course, this would cause some slowdown because suexec would need to
parse its config file on every request.


My interest in suexec was for keeping this info inside the apache
vhost config, instead of farming it out external things which
aren't organized this way.

How about a hardcoded owner which is different from what's being
setuid to?  That would prevent someone exploiting it, but still
insulate the files from being chmod-ed.


Re: Any one doing it?

2016-04-01 Thread William A Rowe Jr
On Fri, Apr 1, 2016 at 11:49 AM, Yann Ylavic  wrote:

> On Fri, Apr 1, 2016 at 6:36 PM, Luca Toscano 
> wrote:
> >
> > +1, let's concentrate on 2.4 :)
>
> You know, there is still many 2.2 around, and some people may have
> backported 2.4 fixes to 2.2 and decide to propose them some days.
> If three PMC members (at least) think a 2.2.32 is worth it, it may
> well be out ;)
>

As an RM of many 2.2 releases, my inclination at this point is to wait for
the point where we have 'something to fix' (vis a vis Security patches) and
hold a vote to announce at the same time that we are starting a 12 month
EOL clock, broadcast widely with a 2.2.32 security release.  That could
happen at any point, but the amount of activity on 2.2 branch since the
last release doesn't seem to merit a parallel release with 2.4.20 this
coming week.  Just my 2c...


> But I agree that we should mainly reference 2.4 in the "generic" docs.
>

We actually have an http://httpd.apache.org/docs/current/ tree that
reflects the now-current release, and needs not be changed when
we finally release a 2.6 or 3.0 flavor.


Re: svn commit: r1737382 - /httpd/httpd/trunk/docs/manual/mod/core.xml

2016-04-01 Thread Luca Toscano
2016-04-01 16:32 GMT+02:00 Eric Covener :

> On Fri, Apr 1, 2016 at 10:05 AM, Luca Toscano 
> wrote:
> > Yep I thought what was best too and decided to commit on trunk to gather
> > some feedback. The Bug is low severity but as stated in users@ the
> > documentation says X and httpd does the opposite, getting people
> confused.
> > IMHO we should advertise directives that are not working properly with
> > specific versions of httpd since most of the people does not read CHANGES
> > and Bugzilla (also they tend to be a bit too dense for whoever is not
> > familiar with httpd development/code), but I don't have also a lot of
> > experience within the project to make a strong argument (plus I still
> need
> > to learn a lot).
>
>
> A potential compromise would be a brief  note.  Still a
> bit of a fudge.   Not sure how I feel about it.
>

Change reverted, will follow your advice to avoid these changes in the
future.

Thanks!

Luca


Re: svn commit: r1737382 - /httpd/httpd/trunk/docs/manual/mod/core.xml

2016-04-01 Thread Eric Covener
On Fri, Apr 1, 2016 at 10:05 AM, Luca Toscano  wrote:
> Yep I thought what was best too and decided to commit on trunk to gather
> some feedback. The Bug is low severity but as stated in users@ the
> documentation says X and httpd does the opposite, getting people confused.
> IMHO we should advertise directives that are not working properly with
> specific versions of httpd since most of the people does not read CHANGES
> and Bugzilla (also they tend to be a bit too dense for whoever is not
> familiar with httpd development/code), but I don't have also a lot of
> experience within the project to make a strong argument (plus I still need
> to learn a lot).


A potential compromise would be a brief  note.  Still a
bit of a fudge.   Not sure how I feel about it.


Re: svn commit: r1737382 - /httpd/httpd/trunk/docs/manual/mod/core.xml

2016-04-01 Thread Luca Toscano
Hi Eric and Rüdiger,

2016-04-01 15:59 GMT+02:00 Rüdiger Plüm :

>
>
> On 04/01/2016 03:48 PM, Eric Covener wrote:
> > I am -0.9 on this info in the manual, for a relatively low severity bug.
>
> +1. We don't do this kind of stuff in the documentation. This is what
> CHANGES and Bugzilla are for.
>


Yep I thought what was best too and decided to commit on trunk to gather
some feedback. The Bug is low severity but as stated in users@ the
documentation says X and httpd does the opposite, getting people confused.
IMHO we should advertise directives that are not working properly with
specific versions of httpd since most of the people does not read CHANGES
and Bugzilla (also they tend to be a bit too dense for whoever is not
familiar with httpd development/code), but I don't have also a lot of
experience within the project to make a strong argument (plus I still need
to learn a lot).

Is there any compromise that we can think of to avoid a misleading
documentation entry like this one?

Thanks!

Luca


Re: svn commit: r1737382 - /httpd/httpd/trunk/docs/manual/mod/core.xml

2016-04-01 Thread Rüdiger Plüm


On 04/01/2016 03:48 PM, Eric Covener wrote:
> I am -0.9 on this info in the manual, for a relatively low severity bug.

+1. We don't do this kind of stuff in the documentation. This is what CHANGES 
and Bugzilla are for.

Regards

Rüdiger



> 
> On Fri, Apr 1, 2016 at 9:31 AM,   wrote:
>> Author: elukey
>> Date: Fri Apr  1 13:31:28 2016
>> New Revision: 1737382
>>
>> URL: http://svn.apache.org/viewvc?rev=1737382=rev
>> Log:
>> Added warning for AllowOverrideList None in the mod-core doc (PR 58528)
>>
>> Modified:
>> httpd/httpd/trunk/docs/manual/mod/core.xml
>>
>> Modified: httpd/httpd/trunk/docs/manual/mod/core.xml
>> URL: 
>> http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/core.xml?rev=1737382=1737381=1737382=diff
>> ==
>> --- httpd/httpd/trunk/docs/manual/mod/core.xml (original)
>> +++ httpd/httpd/trunk/docs/manual/mod/core.xml Fri Apr  1 13:31:28 2016
>> @@ -328,6 +328,14 @@ NoDecode option available in 2.3.12 and
>>  completely ignored. In this case, the server will not even attempt
>>  to read .htaccess files in the filesystem.
>>
>> +AllowOverrideList bug in versions = 2.4.18
>> +AllowOverrideList will not 
>> prevent
>> +.htaccess files processing when explicitly set with None in httpd 
>> versions = 2.4.18 due to a
>> +https://bz.apache.org/bugzilla/show_bug.cgi?id=58528;>bug.
>> +Set only AllowOverride to None as 
>> workaround if
>> +you are unable to upgrade to httpd  2.4.18
>> +
>> +
>>  When this directive is set to All, then any
>>  directive which has the .htaccess >  href="directive-dict.html#Context">Context is allowed in
>> @@ -520,6 +528,14 @@ AllowOverride AuthConfig Indexes
>>  ignored.  In this case, the server will not even attempt to read
>>  .htaccess files in the filesystem.
>>
>> +AllowOverrideList bug in versions = 2.4.18
>> +AllowOverrideList will not 
>> prevent
>> +.htaccess files processing when explicitly set with None in httpd 
>> versions = 2.4.18 due to a
>> +https://bz.apache.org/bugzilla/show_bug.cgi?id=58528;>bug.
>> +Set only AllowOverride to None as 
>> workaround if
>> +you are unable to upgrade to httpd  2.4.18
>> +
>> +
>>  Example:
>>
>>  
>>
>>
> 
> 
> 


Re: svn commit: r1737382 - /httpd/httpd/trunk/docs/manual/mod/core.xml

2016-04-01 Thread Eric Covener
I am -0.9 on this info in the manual, for a relatively low severity bug.

On Fri, Apr 1, 2016 at 9:31 AM,   wrote:
> Author: elukey
> Date: Fri Apr  1 13:31:28 2016
> New Revision: 1737382
>
> URL: http://svn.apache.org/viewvc?rev=1737382=rev
> Log:
> Added warning for AllowOverrideList None in the mod-core doc (PR 58528)
>
> Modified:
> httpd/httpd/trunk/docs/manual/mod/core.xml
>
> Modified: httpd/httpd/trunk/docs/manual/mod/core.xml
> URL: 
> http://svn.apache.org/viewvc/httpd/httpd/trunk/docs/manual/mod/core.xml?rev=1737382=1737381=1737382=diff
> ==
> --- httpd/httpd/trunk/docs/manual/mod/core.xml (original)
> +++ httpd/httpd/trunk/docs/manual/mod/core.xml Fri Apr  1 13:31:28 2016
> @@ -328,6 +328,14 @@ NoDecode option available in 2.3.12 and
>  completely ignored. In this case, the server will not even attempt
>  to read .htaccess files in the filesystem.
>
> +AllowOverrideList bug in versions = 2.4.18
> +AllowOverrideList will not 
> prevent
> +.htaccess files processing when explicitly set with None in httpd 
> versions = 2.4.18 due to a
> +https://bz.apache.org/bugzilla/show_bug.cgi?id=58528;>bug.
> +Set only AllowOverride to None as 
> workaround if
> +you are unable to upgrade to httpd  2.4.18
> +
> +
>  When this directive is set to All, then any
>  directive which has the .htaccess   href="directive-dict.html#Context">Context is allowed in
> @@ -520,6 +528,14 @@ AllowOverride AuthConfig Indexes
>  ignored.  In this case, the server will not even attempt to read
>  .htaccess files in the filesystem.
>
> +AllowOverrideList bug in versions = 2.4.18
> +AllowOverrideList will not 
> prevent
> +.htaccess files processing when explicitly set with None in httpd 
> versions = 2.4.18 due to a
> +https://bz.apache.org/bugzilla/show_bug.cgi?id=58528;>bug.
> +Set only AllowOverride to None as 
> workaround if
> +you are unable to upgrade to httpd  2.4.18
> +
> +
>  Example:
>
>  
>
>



-- 
Eric Covener
cove...@gmail.com