discussion on fd leak problematic

["Bjoern A. Zeeb" <bz...@zabbadoz.net>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Hi,

this is a summary for further discussion on the fd leak problematic in
httpd-2.0 and related apr (inherit) code.


History:
- ----------------------------------------

Christian Kratzer noticed a lot of open fds in http-2.0.44 and we
started to dig into this[1].

Some other people noticed those too which we got to know after a
posting to vuln-dev[2].

The Problem has existed for a along time in httpd code but came not up
before 2.0.40 because of another bug in apr which has been fixed before
2.0.40 release[3].

- From then on APR_IMPLEMENT_INHERIT_SET and APR_IMPLEMENT_INHERIT_UNSET
worked the way they should but they had already been ""misused"" the wrong
way for unknown reasons in httpd code. See cvs commits around the versions
of diff in [4].

Our first patch intended as a starting for a complete fix is also in
bugzilla [1].

After further discussions with Steve Grubb I also traced the open pipes
identified as pod (pipe of death) and after about a week later also
provided a diff.

There also had been some mails to security.

Steve Grubb worked/works together with some people at redhat which
started intensively working on the problem: [5]
Joe Orton then came up with two patches on dev@apr[6] and dev@httpd[7].

I also had some private mails with William A. Rowe, Jr.


Problems needing discussion:
- ----------------------------------------

APR
- - - - - - - - - - - - - - - - - - - - -

with APR we already had one commit[8]. The main question here is from
what I can see:

is it possible to use FD_CLOEXEC somewhere in the API of
APR_IMPLEMENT_INHERIT_(UN)SET so that we might have some double checks
for closing file descriptors on execs.

Another question from me is if it would be possible to always register
a child chlean_fn not apr_pool_cleanup_null for pipes - resp. s.th. like
in my diff[9].

Last point on this would be that developers that use APR need to somehow
recognnize that they can change the behaviour for pipes like they can
for files with apr_file_inherit_(un)set. not having apr_pipe_inherit_(un)set
seems to also cause irritations amog apr/httpd-developers.


For open there is the (yet) unused(?) flag APR_FILE_NOCLEANUP. Can some
have a look at this and either document it as yet unused or remove it
from the code or correct me if I am wrong. [file_io/unix/open.c]


HTTPD
- - - - - - - - - - - - - - - - - - - - -

because of the ""missuse"" of apr_file_inherit_set and not registered
child cleanup_fn we have various fd leaks for pipes and logs.


After my initial patch [1][10] Joe Orton came up with another patch
to dev@httpd[7]. My problem with this is:
is it good enough to simply remove the apr_file_inherit_set() calls
or explicitly use apr_file_inherit_unset() so that we can be sure
that there is always a child cleanup_fn registered.
In apr there still is the flag APR_FILE_NOCLEANUP that might prevent
registering a child cleanup_fn as the deault for now is. This flag seems
to be unused at the moment for open() calls.

Joe Orton does this in his patch for pipes (as they do not have a child
cleanup_fn registered by default).


Another thing I lately discovered after some discussion with Steve Grubb
on lseek()ing on open file descriptors was that for me only error logs
had been readable but not access logs. I think we do not need to open
error_logs for reading. A fix for at least one place has been posted to
dev@httpd[11]. There might be other places where files are opened with
more priviledges than needed. Please look at this.



Why ?
- ----------------------------------------

Some might say that CGIs are only as secure as one trusts or verifies
the code that runs on a webserver. Heard that multiple times.
This is correct.

But there are other reasons why these things are security relevant:

a) as stated by multiple persons reading and writing on open file
descriptors is no good for multiple reasons. See p.ex. posting to
vuln-dev by Steve Grubb[2].

As stated by some persons: one may open those files in any case. That
is not fully true. I am using suexec and the CGI is not able to open
the log files (even not the ones from the virtual host it is running
on). It for sure can as long as the fds are leaked to it.


b) the open pipes can be abused for a partly DoS attack or one can
at least cause performance impacts when running apache with mpm=worker
or mpm=threadpool by simply writing the correct 'restart' or
'graceful' char to them which causes a drop/restart on some of the
processes/threads from what I could see on linux.


There is at least one more reason why this needs to be fixed:

a lot of open file descriptors are problems for resource limits. Small
hosters or people with few virtual hosts will perhaps not log to pipes
but have an error and an access log open for each virtual host.
If they have set resource limits by p.ex. patching suexec they run into
problems. This is btw. why this had been discovered by Christian
Kratzer from what I know.
It is fully impractical to loop through fd 3..rlim.rlim_cur and close
all of them in p.ex. suexec as "some systems such as AIX have a huge
per-process open file limit."[12]



TODO:
- ----------------------------------------

APR: check FD_CLOEXEC and check if it would be possible to always
register a child cleanup_fn[9] as this (yet) is only used for execs
on unix.


HTTPD: we need further discussion on wheter to explicitly call
apr_file_inherit_unset() on those. This can be best decided together
with apr people as they might better know if the default behavior
will change/break somewhen in future again.

check wether files etc. are opened with too many priviledges and if
those orivs can be withdrwn w/o breaking the code in other places. See
p.ex. [11].



Please have a look at the code as you all should know it far better than
me and discuss those points.



LAST:
- ----------------------------------------

some more comments/details might either be in the security@httpd account
or are also archived on

	http://sources.zabbadoz.net/patches/apache/

along with some sample CGIs etc. that show some of the problematics
described above.




References:
- ----------------------------------------

[1]	bug report by Christian Kratzer
	http://nagoya.apache.org/bugzilla/show_bug.cgi?id=17206

[2]	vuln-dev posting by Steve Grubb
	http://www.securityfocus.com/archive/82/312747/2003-02-20/2003-02-26/0
	or
	http://marc.theaimsgroup.com/?l=vuln-dev&m=104585997219471&w=2

[3]	apr 24^2 vs. 24<<2 bug
	http://cvs.apache.org/viewcvs.cgi/apr/include/arch/unix/Attic/inherit.h.diff?r1=1.10&r2=1.11&diff_format=h

[4]	httpd inherit commits which cause problmes
	http://cvs.apache.org/viewcvs.cgi/httpd-2.0/server/log.c.diff?r1=1.94&r2=1.95&diff_format=h

[5]	redhat bugzilla report
	https://bugzilla.redhat.com/bugzilla/show_bug.cgi?id=82142

[6]	apr patch by Joe Orton
	http://marc.theaimsgroup.com/?t=104688218100008&r=1&w=2

[7]	httpd diff by Joe Orton
	http://marc.theaimsgroup.com/?t=104696493400002&r=1&w=2
	resend: http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=104755245104369&w=2

[8]	apr pipe inherit commit
	http://cvs.apache.org/viewcvs.cgi/apr/file_io/unix/pipe.c.diff?r1=1.61&r2=1.62&diff_format=h

[9]	suggestion for registering child cleanup_fn for pipes.
	http://sources.zabbadoz.net/patches/apache/pipe-20030302-01.diff

[10]	suggestion for a fix that "works for me" for fd leaks of logs
	included in
	http://sources.zabbadoz.net/patches/apache/bug-report.txt

[11]	suggestion for not opening error_logs readable
	http://marc.theaimsgroup.com/?l=apache-httpd-dev&m=104758986221626&w=2
	http://sources.zabbadoz.net/patches/apache/httpd-2.0-error-log-open-for-reading-20030313-01.diff

[12]	postfix master source code by Wietse Venema

# End;

- -- 
Bjoern A. Zeeb				bzeeb at Zabbadoz dot NeT
56 69 73 69 74				http://www.zabbadoz.net/
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (FreeBSD)

iD8DBQE+cllYIcUJFg5KeHURAnRgAJwOrTEmFyVaCEERYoWgV9kJk9bUJwCeK5kf
6f4FNCdWnC+3RnvzUHCKE+8=
=rk5r
-----END PGP SIGNATURE-----