You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by Jeff Trawick <tr...@gmail.com> on 2012/08/01 19:58:16 UTC

Re: mpm-itk and upstream Apache, once again

On Thu, Jul 19, 2012 at 11:38 AM, Steinar H. Gunderson
<sg...@bigfoot.com> wrote:
> On Thu, Jul 19, 2012 at 11:27:04AM -0400, Jeff Trawick wrote:
>> What changes are needed to httpd trunk so that you can build mpm-itk
>> with apxs and enable it via LoadModule, such that mpm-itk is fully
>> functional?  As I'm sure you're aware, prefork, worker, and event are
>> all untied from core enough to support that in httpd >= 2.4.
>
> We'd need:
>
>   1. A hook right after merging the perdir config.
>   2. Fixes to get Apache to drop the connection if it detects
>      (during .htaccess lookup) that it would need to change the uid.
>
> Both patches are simple, although for #2 to be truly generic (ie. be usable
> by mod_privileges as well) we'd need some sort of signalling mechanism saying
> “we have switched uids and cannot switch back”, which then both
> mod_privileges (in secure mode) and mpm-itk could set.
>
> I've attached the current versions of both patches from my current Apache 2.4
> patch set; you can see the “ap_running_under_mpm_itk” variable which would
> probably need to be replaced by ap_mpm_query() or similar.
>
> /* Steinar */
> --
> Homepage: http://www.sesse.net/
>

Your post-perdir-config patch has been committed to trunk with r1368121.

(http://svn.apache.org/viewvc?view=revision&revision=r1368121)

Attached is a patch to trunk that allows you to hook in to the stat
calls from directory walk.  Call apr_stat() like core_dirwalk_stat()
but check for APR_STATUS_IS_EACCES(rv) and decide whether to run
lingering close and exit.  Let us know how that goes.

You still need the parse-htaccess patch for now.

-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Re: mpm-itk and upstream Apache, once again

Posted by "William A. Rowe Jr." <wr...@rowe-clan.net>.
On 8/5/2012 8:32 AM, Steinar H. Gunderson wrote:
> On Sun, Aug 05, 2012 at 11:05:59AM -0400, Jeff Trawick wrote:
>> Great!  I'll do something about the remaining patch "before long".
> 
> When the time comes, do we have any hopes of getting this back from trunk to
> 2.4, or would it need to wait for 2.6/3.0?
> 
> FWIW, the mpm-itk security hardening that was discussed (running with uid != 0,
> and limiting setuid/setgid ranges through seccomp) is starting to come quite
> nicely along, although the problem of initgroups() remains (a rogue process
> with CAP_SETGID can add any supplementary group it pleases, and seccomp is
> unable to check it), and there's been very limited user testing so far.
> I guess we can't get fully down to the level of prefork, but it can get
> pretty close.

Steinar,

I solved a very similar problem by spinning off a low-numbered port daemon
which accesses resources (in this case, port 21 or similar) and returns the
fd to the caller after it evaluates whether that request is permitted by the
configuration parsed when it was launched.

The solution might give you some ideas on how this mpm could have very limited
root privileges with very specific purposes, and not at risk from any remote
code execution flaws discovered in the future;
http://svn.apache.org/repos/asf/httpd/mod_ftp/trunk/modules/ftp/ftp_lowportd.c

Hope it inspires some interesting improvements :)

Bill

Re: mpm-itk and upstream Apache, once again

Posted by Jeff Trawick <tr...@gmail.com>.
On Sun, Aug 5, 2012 at 11:32 AM, Steinar H. Gunderson
<sg...@bigfoot.com> wrote:
> On Sun, Aug 05, 2012 at 11:05:59AM -0400, Jeff Trawick wrote:
>> Great!  I'll do something about the remaining patch "before long".
>
> When the time comes, do we have any hopes of getting this back from trunk to
> 2.4, or would it need to wait for 2.6/3.0?

2.4.small-number

>
> FWIW, the mpm-itk security hardening that was discussed (running with uid != 0,
> and limiting setuid/setgid ranges through seccomp) is starting to come quite
> nicely along, although the problem of initgroups() remains (a rogue process
> with CAP_SETGID can add any supplementary group it pleases, and seccomp is
> unable to check it), and there's been very limited user testing so far.
> I guess we can't get fully down to the level of prefork, but it can get
> pretty close.
>
> /* Steinar */
> --
> Homepage: http://www.sesse.net/



-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Re: mpm-itk and upstream Apache, once again

Posted by "Steinar H. Gunderson" <sg...@bigfoot.com>.
On Sun, Aug 05, 2012 at 11:05:59AM -0400, Jeff Trawick wrote:
> Great!  I'll do something about the remaining patch "before long".

When the time comes, do we have any hopes of getting this back from trunk to
2.4, or would it need to wait for 2.6/3.0?

FWIW, the mpm-itk security hardening that was discussed (running with uid != 0,
and limiting setuid/setgid ranges through seccomp) is starting to come quite
nicely along, although the problem of initgroups() remains (a rogue process
with CAP_SETGID can add any supplementary group it pleases, and seccomp is
unable to check it), and there's been very limited user testing so far.
I guess we can't get fully down to the level of prefork, but it can get
pretty close.

/* Steinar */
-- 
Homepage: http://www.sesse.net/

Re: mpm-itk and upstream Apache, once again

Posted by Michael Felt <ma...@gmail.com>.
A very interesting read for myself. The hat I am wearing is
"admin/implementer" of AIX RBAC (role based access control) where the whole
application is running as a non-root id (or non-superuser if you prefer).

The kernel privileges granted include setting "privileges" and accessing
"all" files - which I shall be looking at to limit/manage removing the
privileges and seeing what "breaks".

If a seteuid() call is made - could it do that to root? must look into that
(not so much httpd, but RBAC mechanisms in general).

My goal with an RBACed configuration of httpd is that a non-superuser could
fully administer httpd - start/stop service; edit key config files.

So the interesting part of this read - maybe httpd has "unknown to me"
needs to be "more super". And I may even have a reason to actually look at
parts of the code :)
On Fri, Sep 21, 2012 at 2:29 PM, Jeff Trawick <tr...@gmail.com> wrote:

> On Sun, Aug 5, 2012 at 11:05 AM, Jeff Trawick <tr...@gmail.com> wrote:
> > On Sun, Aug 5, 2012 at 11:00 AM, Steinar H. Gunderson
> > <sg...@bigfoot.com> wrote:
> >> On Wed, Aug 01, 2012 at 01:58:16PM -0400, Jeff Trawick wrote:
> >>> Your post-perdir-config patch has been committed to trunk with
> r1368121.
> >>
> >> Thanks!
> >>
> >>> Attached is a patch to trunk that allows you to hook in to the stat
> >>> calls from directory walk.  Call apr_stat() like core_dirwalk_stat()
> >>> but check for APR_STATUS_IS_EACCES(rv) and decide whether to run
> >>> lingering close and exit.  Let us know how that goes.
> >>>
> >>> You still need the parse-htaccess patch for now.
> >>
> >> I backported this to 2.4.2, and changed mpm-itk to hook into that
> function
> >> with the following hook:
> >>
> >>   static apr_status_t itk_dirwalk_stat(apr_finfo_t *finfo, request_rec
> *r,
> >>                                        apr_int32_t wanted)
> >>   {
> >>       apr_status_t status = apr_stat(finfo, r->filename, wanted,
> r->pool);
> >>       if (ap_has_irreversibly_setuid && APR_STATUS_IS_EACCES(status)) {
> >>            ap_log_rerror(APLOG_MARK, APLOG_WARNING, status, r,
> >>                          "Couldn't read %s, closing connection.",
> >>                          r->filename);
> >>            ap_lingering_close(r->connection);
> >>            clean_child_exit(0);
> >>       }
> >>       return status;
> >>   }
> >>
> >> Seems to work great, from my limited testing. As an extra bonus, I can
> easily
> >> call clean_child_exit() (which runs more cleanup hooks) instead of
> exit(),
> >> since this is in the MPM's own .c file.
> >
> > Great!  I'll do something about the remaining patch "before long".
>
> It has been a while :)
>
> The dirwalk_stat hook has now been committed:
> http://svn.apache.org/viewvc?view=revision&revision=1388447
>
> Attached is a patch that adds a hook called just before htaccess is
> opened.  See if you can use that to resolve the remaining issue.
>
> >
> >>
> >> /* Steinar */
> >> --
> >> Homepage: http://www.sesse.net/
> >
> >
> >
> > --
> > Born in Roswell... married an alien...
> > http://emptyhammock.com/
>
>
>
> --
> Born in Roswell... married an alien...
> http://emptyhammock.com/
>

Re: mpm-itk and upstream Apache, once again

Posted by "Steinar H. Gunderson" <sg...@bigfoot.com>.
On Sun, Nov 11, 2012 at 08:25:08AM -0500, Jeff Trawick wrote:
> I'll have a look again "soon."

Like my “soon”s, this :-)

> Earlier I couldn't think of a more efficient or direct mechanism that makes
> sense as an API

I guess it depends on how mpm-itk specific you want it to be. One could maybe
have a post-htaccess hook that gets run with the status, but that would
probably be less useful for a hypothetical other module?

> IIRC, mpm-itk only needs to call stat() on the file to check for
> permissions, and since (in the successful case) the file is about to be
> opened I assume that the performance degradation is minimal.

I could probably change to stat, but it would be a bit complicated to get
right, since often you can stat a file without opening it, and
ap_pcfg_* had lots of stuff I didn't want to delve into there and then :-)
Maybe access() would be better than stat(), and good enough. (There's a small
race window if the permissions are changed between access() and open(), but I
guess we can live with that.)

Do you think it would be possible to get these hooks into 2.4.4 or 2.4.5?

/* Steinar */
-- 
Homepage: http://www.sesse.net/

Re: mpm-itk and upstream Apache, once again

Posted by Jeff Trawick <tr...@gmail.com>.
I'll have a look again "soon."

Earlier I couldn't think of a more efficient or direct mechanism that makes
sense as an API

IIRC, mpm-itk only needs to call stat() on the file to check for
permissions, and since (in the successful case) the file is about to be
opened I assume that the performance degradation is minimal.

(It has been a while since I looked at the code.)



On Thu, Nov 8, 2012 at 4:22 PM, Steinar H. Gunderson <sgunderson@bigfoot.com
> wrote:

> On Thu, Nov 08, 2012 at 08:53:12PM +0100, Steinar H. Gunderson wrote:
> >> I've looked at this now; sorry for the long delay. It would seem it is
> not
> >> sufficient for removing the patches from server/config.c (which exit if
> >> .htaccess files cannot be opened); or am I misunderstanding something?
> > Sorry, I'm just confused. Back to coding...
>
> Yes, indeed, it works. Out-of-tree-built mpm-itk, yay!
>
> But it's not ideal, since I don't get the status of the ap_pcfg_openfile()
> call in the htaccess hook (since it's before that), and I also can't
> return a
> file pointer. The only way I can see to do this in mpm-itk is to run
> ap_pcfg_openfile() myself to see if it goes through or not, and then close
> it
> and return DECLINED if the call was successful. That would seem somewhat
> inefficient.
>
> /* Steinar */
> --
> Homepage: http://www.sesse.net/
>



-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Re: mpm-itk and upstream Apache, once again

Posted by "Steinar H. Gunderson" <sg...@bigfoot.com>.
On Thu, Nov 08, 2012 at 08:53:12PM +0100, Steinar H. Gunderson wrote:
>> I've looked at this now; sorry for the long delay. It would seem it is not
>> sufficient for removing the patches from server/config.c (which exit if
>> .htaccess files cannot be opened); or am I misunderstanding something?
> Sorry, I'm just confused. Back to coding...

Yes, indeed, it works. Out-of-tree-built mpm-itk, yay!

But it's not ideal, since I don't get the status of the ap_pcfg_openfile()
call in the htaccess hook (since it's before that), and I also can't return a
file pointer. The only way I can see to do this in mpm-itk is to run
ap_pcfg_openfile() myself to see if it goes through or not, and then close it
and return DECLINED if the call was successful. That would seem somewhat
inefficient.

/* Steinar */
-- 
Homepage: http://www.sesse.net/

Re: mpm-itk and upstream Apache, once again

Posted by "Steinar H. Gunderson" <sg...@bigfoot.com>.
On Thu, Nov 08, 2012 at 08:51:50PM +0100, Steinar H. Gunderson wrote:
> I've looked at this now; sorry for the long delay. It would seem it is not
> sufficient for removing the patches from server/config.c (which exit if
> .htaccess files cannot be opened); or am I misunderstanding something?

Sorry, I'm just confused. Back to coding...

/* Steinar */
-- 
Homepage: http://www.sesse.net/

Re: mpm-itk and upstream Apache, once again

Posted by "Steinar H. Gunderson" <sg...@bigfoot.com>.
On Mon, Sep 24, 2012 at 08:44:21AM -0400, Jeff Trawick wrote:
> I went ahead and committed this to trunk as r1389339.  Hopefully this
> completes the ability to enable mpm-itk without patches to httpd core.

I've looked at this now; sorry for the long delay. It would seem it is not
sufficient for removing the patches from server/config.c (which exit if
.htaccess files cannot be opened); or am I misunderstanding something?

/* Steinar */
-- 
Homepage: http://www.sesse.net/

Re: mpm-itk and upstream Apache, once again

Posted by "Steinar H. Gunderson" <sg...@bigfoot.com>.
On Mon, Sep 24, 2012 at 08:44:21AM -0400, Jeff Trawick wrote:
>> Attached is a patch that adds a hook called just before htaccess is
>> opened.  See if you can use that to resolve the remaining issue.
> I went ahead and committed this to trunk as r1389339.  Hopefully this
> completes the ability to enable mpm-itk without patches to httpd core.

Hi,

Thanks for doing this. I'm currently a bit hindered from testing it out,
but I'll get back to you with the results once things settle down a bit :-)

/* Steinar */
-- 
Homepage: http://www.sesse.net/


Re: mpm-itk and upstream Apache, once again

Posted by Jeff Trawick <tr...@gmail.com>.
On Fri, Sep 21, 2012 at 8:29 AM, Jeff Trawick <tr...@gmail.com> wrote:
> On Sun, Aug 5, 2012 at 11:05 AM, Jeff Trawick <tr...@gmail.com> wrote:
>> On Sun, Aug 5, 2012 at 11:00 AM, Steinar H. Gunderson
>> <sg...@bigfoot.com> wrote:
>>> On Wed, Aug 01, 2012 at 01:58:16PM -0400, Jeff Trawick wrote:
>>>> Your post-perdir-config patch has been committed to trunk with r1368121.
>>>
>>> Thanks!
>>>
>>>> Attached is a patch to trunk that allows you to hook in to the stat
>>>> calls from directory walk.  Call apr_stat() like core_dirwalk_stat()
>>>> but check for APR_STATUS_IS_EACCES(rv) and decide whether to run
>>>> lingering close and exit.  Let us know how that goes.
>>>>
>>>> You still need the parse-htaccess patch for now.
>>>
>>> I backported this to 2.4.2, and changed mpm-itk to hook into that function
>>> with the following hook:
>>>
>>>   static apr_status_t itk_dirwalk_stat(apr_finfo_t *finfo, request_rec *r,
>>>                                        apr_int32_t wanted)
>>>   {
>>>       apr_status_t status = apr_stat(finfo, r->filename, wanted, r->pool);
>>>       if (ap_has_irreversibly_setuid && APR_STATUS_IS_EACCES(status)) {
>>>            ap_log_rerror(APLOG_MARK, APLOG_WARNING, status, r,
>>>                          "Couldn't read %s, closing connection.",
>>>                          r->filename);
>>>            ap_lingering_close(r->connection);
>>>            clean_child_exit(0);
>>>       }
>>>       return status;
>>>   }
>>>
>>> Seems to work great, from my limited testing. As an extra bonus, I can easily
>>> call clean_child_exit() (which runs more cleanup hooks) instead of exit(),
>>> since this is in the MPM's own .c file.
>>
>> Great!  I'll do something about the remaining patch "before long".
>
> It has been a while :)
>
> The dirwalk_stat hook has now been committed:
> http://svn.apache.org/viewvc?view=revision&revision=1388447
>
> Attached is a patch that adds a hook called just before htaccess is
> opened.  See if you can use that to resolve the remaining issue.

I went ahead and committed this to trunk as r1389339.  Hopefully this
completes the ability to enable mpm-itk without patches to httpd core.

>
>>
>>>
>>> /* Steinar */
>>> --
>>> Homepage: http://www.sesse.net/
>>
>>
>>
>> --
>> Born in Roswell... married an alien...
>> http://emptyhammock.com/
>
>
>
> --
> Born in Roswell... married an alien...
> http://emptyhammock.com/



-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Re: mpm-itk and upstream Apache, once again

Posted by Jeff Trawick <tr...@gmail.com>.
On Sun, Aug 5, 2012 at 11:05 AM, Jeff Trawick <tr...@gmail.com> wrote:
> On Sun, Aug 5, 2012 at 11:00 AM, Steinar H. Gunderson
> <sg...@bigfoot.com> wrote:
>> On Wed, Aug 01, 2012 at 01:58:16PM -0400, Jeff Trawick wrote:
>>> Your post-perdir-config patch has been committed to trunk with r1368121.
>>
>> Thanks!
>>
>>> Attached is a patch to trunk that allows you to hook in to the stat
>>> calls from directory walk.  Call apr_stat() like core_dirwalk_stat()
>>> but check for APR_STATUS_IS_EACCES(rv) and decide whether to run
>>> lingering close and exit.  Let us know how that goes.
>>>
>>> You still need the parse-htaccess patch for now.
>>
>> I backported this to 2.4.2, and changed mpm-itk to hook into that function
>> with the following hook:
>>
>>   static apr_status_t itk_dirwalk_stat(apr_finfo_t *finfo, request_rec *r,
>>                                        apr_int32_t wanted)
>>   {
>>       apr_status_t status = apr_stat(finfo, r->filename, wanted, r->pool);
>>       if (ap_has_irreversibly_setuid && APR_STATUS_IS_EACCES(status)) {
>>            ap_log_rerror(APLOG_MARK, APLOG_WARNING, status, r,
>>                          "Couldn't read %s, closing connection.",
>>                          r->filename);
>>            ap_lingering_close(r->connection);
>>            clean_child_exit(0);
>>       }
>>       return status;
>>   }
>>
>> Seems to work great, from my limited testing. As an extra bonus, I can easily
>> call clean_child_exit() (which runs more cleanup hooks) instead of exit(),
>> since this is in the MPM's own .c file.
>
> Great!  I'll do something about the remaining patch "before long".

It has been a while :)

The dirwalk_stat hook has now been committed:
http://svn.apache.org/viewvc?view=revision&revision=1388447

Attached is a patch that adds a hook called just before htaccess is
opened.  See if you can use that to resolve the remaining issue.

>
>>
>> /* Steinar */
>> --
>> Homepage: http://www.sesse.net/
>
>
>
> --
> Born in Roswell... married an alien...
> http://emptyhammock.com/



-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Re: mpm-itk and upstream Apache, once again

Posted by Jeff Trawick <tr...@gmail.com>.
On Sun, Aug 5, 2012 at 11:00 AM, Steinar H. Gunderson
<sg...@bigfoot.com> wrote:
> On Wed, Aug 01, 2012 at 01:58:16PM -0400, Jeff Trawick wrote:
>> Your post-perdir-config patch has been committed to trunk with r1368121.
>
> Thanks!
>
>> Attached is a patch to trunk that allows you to hook in to the stat
>> calls from directory walk.  Call apr_stat() like core_dirwalk_stat()
>> but check for APR_STATUS_IS_EACCES(rv) and decide whether to run
>> lingering close and exit.  Let us know how that goes.
>>
>> You still need the parse-htaccess patch for now.
>
> I backported this to 2.4.2, and changed mpm-itk to hook into that function
> with the following hook:
>
>   static apr_status_t itk_dirwalk_stat(apr_finfo_t *finfo, request_rec *r,
>                                        apr_int32_t wanted)
>   {
>       apr_status_t status = apr_stat(finfo, r->filename, wanted, r->pool);
>       if (ap_has_irreversibly_setuid && APR_STATUS_IS_EACCES(status)) {
>            ap_log_rerror(APLOG_MARK, APLOG_WARNING, status, r,
>                          "Couldn't read %s, closing connection.",
>                          r->filename);
>            ap_lingering_close(r->connection);
>            clean_child_exit(0);
>       }
>       return status;
>   }
>
> Seems to work great, from my limited testing. As an extra bonus, I can easily
> call clean_child_exit() (which runs more cleanup hooks) instead of exit(),
> since this is in the MPM's own .c file.

Great!  I'll do something about the remaining patch "before long".

>
> /* Steinar */
> --
> Homepage: http://www.sesse.net/



-- 
Born in Roswell... married an alien...
http://emptyhammock.com/

Re: mpm-itk and upstream Apache, once again

Posted by "Steinar H. Gunderson" <sg...@bigfoot.com>.
On Wed, Aug 01, 2012 at 01:58:16PM -0400, Jeff Trawick wrote:
> Your post-perdir-config patch has been committed to trunk with r1368121.

Thanks!

> Attached is a patch to trunk that allows you to hook in to the stat
> calls from directory walk.  Call apr_stat() like core_dirwalk_stat()
> but check for APR_STATUS_IS_EACCES(rv) and decide whether to run
> lingering close and exit.  Let us know how that goes.
> 
> You still need the parse-htaccess patch for now.

I backported this to 2.4.2, and changed mpm-itk to hook into that function
with the following hook:

  static apr_status_t itk_dirwalk_stat(apr_finfo_t *finfo, request_rec *r,
                                       apr_int32_t wanted)
  {
      apr_status_t status = apr_stat(finfo, r->filename, wanted, r->pool);
      if (ap_has_irreversibly_setuid && APR_STATUS_IS_EACCES(status)) {
           ap_log_rerror(APLOG_MARK, APLOG_WARNING, status, r,
                         "Couldn't read %s, closing connection.",
                         r->filename);
           ap_lingering_close(r->connection);
           clean_child_exit(0);
      }
      return status;
  }

Seems to work great, from my limited testing. As an extra bonus, I can easily
call clean_child_exit() (which runs more cleanup hooks) instead of exit(),
since this is in the MPM's own .c file.

/* Steinar */
-- 
Homepage: http://www.sesse.net/