Re: Feedback needed: suexec different-owner patch

2016-04-04 Thread Jacob Champion
On 04/02/2016 12:56 PM, Stefan Fritsch wrote:
> If suexec allowed to suid to a user different than the owner of a 
> script, on that server it would allow any local user to execute any 
> script as any other user. Even if suexec checked that the script is 
> owned by a special "trusted" user, it would still allow to execute 
> that script as any user, without any "opt-in" necessary by the target 
> user.

Ah, this finally made it click for me.

In the case where only the trusted-owner CGI script is compromised (e.g.
an arbitrary code execution vuln), this proposal makes things better,
since the attacker can at least be denied access to the disk. But if
httpd is compromised, it makes things worse, since the attacker can now
run the trusted-owner script as any non-system user. And if both httpd
and the trusted-owner script are compromised, this proposal makes things
*much* worse: an attacker can now run arbitrary code as any non-system user.

Thanks for your feedback on this. Your xattrs suggestion seems like it
might solve the two negative cases, but it uses a much more obscure
(IMO) mechanism to operate... Likewise, having suexec parse a separate
configuration file seems like a lot of complexity to add.

> BTW, using the immutable flag (which can only be done by root) on the 
> scripts is a work-around for your problem that does not involve 
> modifying suexec.

Good point, though I don't think it can be used for the proposed use
case (which was for the trusted user to be able to regularly maintain
the scripts).

--Jacob


Re: Feedback needed: suexec different-owner patch

2016-04-02 Thread Stefan Fritsch
On Friday 01 April 2016 14:03:12, montt...@heavyspace.ca wrote:
> 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. Without the patch, a user would have to "opt-in" to using suexec 
by putting a executable file in one of the paths allowed by suexec. 
With the patch, suexec would allow the apache user to become every 
other user, just by being installed.

Of course people may modify suexec to suit their use cases and the 
situation on their servers. But for a suexec variant that is shipped 
by apache by default, it is important that it does not allow local 
privileg escalation, not in any situation.

So imagine a multi-user server where users are allowed to execute 
cgi/php scripts as httpd run user in their ~/public_html folder. This 
is not a very smart setup, but I am 100% sure that such setups exist. 
If suexec allowed to suid to a user different than the owner of a 
script, on that server it would allow any local user to execute any 
script as any other user. Even if suexec checked that the script is 
owned by a special "trusted" user, it would still allow to execute 
that script as any user, without any "opt-in" necessary by the target 
user.

> Yes, it's a plausible exploit.
> 

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

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

As outlined above, I don't think this would be appropriate to include 
in the suexec shipped with apache httpd.


BTW, using the immutable flag (which can only be done by root) on the 
scripts is a work-around for your problem that does not involve 
modifying suexec. 




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: Feedback needed: suexec different-owner patch

2016-03-30 Thread Jacob Champion
On 03/30/2016 01:49 PM, Stefan Fritsch wrote:
> 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.

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?

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).

- 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.)

> 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.
> 
> A different idea would be to use filesystem xattrs. Maybe check for an 
> xattr APACHE_SUEXEC_ALLOWED, and if a file is owned by root and has 
> that xattr, suexec would allow to change to the user specified in the 
> xattr.
> 
> But these two approaches are just ideas. I don't know if they would be 
> accepted in httpd.

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

--Jacob



Re: Feedback needed: suexec different-owner patch

2016-03-30 Thread Stefan Fritsch
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
> Date: 2016-03-04 18:33
>  From: montt...@heavyspace.ca
> To: dev@httpd.apache.org
> Reply-To: dev@httpd.apache.org
> 
> Here is my first try at a patch for my suggestion, modified from
> httpd 2.2.31.  It works to my satisfaction, able to switch to a UID
> other than the file's owner, while still strictly matching the UID
> and GID of the file against known values.  I make no guarantees of
> correctness or bug-freeness however.  The changes are so simple
> though, I hope there's nothing flagrantly wrong.
> 
> It uses another option, "SuexecFileGroup", which independently
> defines the specific user and group the file must belong to.  If
> you don't define it, it defaults to the old behavior.  I re-used
> suexec's own sanity checking on the new option where it seemed
> appropriate.
> 
> 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.

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.

A different idea would be to use filesystem xattrs. Maybe check for an 
xattr APACHE_SUEXEC_ALLOWED, and if a file is owned by root and has 
that xattr, suexec would allow to change to the user specified in the 
xattr.

But these two approaches are just ideas. I don't know if they would be 
accepted in httpd.



Re: Feedback needed: suexec different-owner patch

2016-03-19 Thread Jim Jagielski
I promise to look deeply into this post 2.4.19 release.

> On Mar 19, 2016, at 1:09 PM, 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
> Date: 2016-03-04 18:33
> From: montt...@heavyspace.ca
> To: dev@httpd.apache.org
> Reply-To: dev@httpd.apache.org
> 
> Here is my first try at a patch for my suggestion, modified from httpd 
> 2.2.31.  It works to my satisfaction, able to switch to a UID other than the 
> file's owner, while still strictly matching the UID and GID of the file 
> against known values.  I make no guarantees of correctness or bug-freeness 
> however.  The changes are so simple though, I hope there's nothing flagrantly 
> wrong.
> 
> It uses another option, "SuexecFileGroup", which independently defines the 
> specific user and group the file must belong to.  If you don't define it, it 
> defaults to the old behavior.  I re-used suexec's own sanity checking on the 
> new option where it seemed appropriate.
> 
> Criticisms, please?



Re: Feedback needed: suexec different-owner patch

2016-03-19 Thread Tim Bannister
On 19 March 2016, 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.


That patch builds on what I'd consider as a legacy feature. I have not used 
suexec for a long time: it is risky, and on the one recent-ish occasion when I 
wanted something like suexec, I also wanted to chroot() / jail() / otherwise 
separate the CGI application from the main system.

httpd's users do sometimes need to have web content served using processes that 
have different privileges to httpd, and perhaps are also isolated from one 
another. suexec achieves some of this albeit not well.
It feels to me as if some kind of FastCGI process manager, combined with a 
privileged helper, could be used to fill the gap that mpm_itk and suexec don't 
completely cover.

I'll add to my To Do list (and maybe also Bugzilla) a task to see what already 
exists and document how to use that in place of suexec.
If nothing out there already works, then my idea is to code that up as well.

I wish I could say when I might get round to that, but the way if these things 
is that it's easy to start this kind of task and rather more difficult to 
complete them.

As to whether to take the suggested patch: +0. I don't think it will make 
things worse; however, I don't feel qualified to comment on security-critical 
code.

Tim


-- 
Tim Bannister – is...@c8h10n4o2.org.uk