Edit report at https://bugs.php.net/bug.php?id=52312&edit=1
ID: 52312
Comment by: phpdotnet at hostultra dot com
Reported by: v dot damore at gmail dot com
Summary: PHP safe_mode/open_basedir - lstat performance
problem
Status: Analyzed
Type: Bug
Package: Safe Mode/open_basedir
Operating System: Linux
PHP Version: 5.2.13
Block user comment: N
Private report: N
New Comment:
This bug is a real performance killer.
I propose this solution...
1. Modify symlink() php function so that if open_basedir or safe_mode is on, it
disallows relative symlinks with .. components, instead it creates absolute
symlink.
This prevents attacker from exploiting CVE-2006-5178.
2. Allow the realpath cache to work even if open_basedir or safe_mode is on.
I did this with my own php code.
Before
---------
last pid: 30437; load averages: 32.93, 24.67, 15.62
98 processes: 43 running, 46 sleeping, 9 lock
CPU: 2.7% user, 0.0% nice, 75.3% system, 0.5% interrupt, 21.5% idle
After
---------
last pid: 30582; load averages: 2.06, 3.57, 10.58
68 processes: 6 running, 62 sleeping
CPU: 6.8% user, 0.0% nice, 1.7% system, 0.9% interrupt, 90.6% idle
Previous Comments:
------------------------------------------------------------------------
[2013-03-05 13:11:32] Terry at ellisons dot org dot uk
Rasmus, picking up our 2013-02-22 23:17 / 23:26 UTC conversation, I've thought
about this some further and gone through some test cases on the debugger.
Having walked through these call stacks, my view is that the PHP / Zend path
scanning, file checking and opening is a tangled mess. A typical open goes
through sometimes 7 seven wrapping layers each of which can do compound path
resolution. The only reason that this doesn't end up slugging performance is
because the lowest level tsrm_realparth_r() uses a resolution cache and
short-circuits the actual I/O requests 95+% of the time.
The CVE-2006-5178 advisory really to a vulerability in the open_basedir checks.
The root issue was that a key check in php_check_specific_open_basedir() was
using this cached path resolution; this introduced the vulnerability because
the caching enabled a race condition. The fix was to turn of ALL realpath
resolution caching. Yes this addressed the vulnerability, but at the price of
killing performance in the typical usecase where open_basedir might be used.
This was unnecessary overkill.
If you think about it realpath caching itself doesn't introduce any
vulnerability. The issue is that the open itself -- or in this case the
preceding php_check_specific_open_basedir() check must disable any caching for
that one check alone. Consider an example there the base dir is /a/b/c and
some path needs to be checked by php_check_specific_open_basedir(). There's
not vulnerability introduced by any previous resolution being used. This will
result in one of two scenarios:
* The resolved path is not of the form /a/b/c.... in which case the error is
thrown
* The resolved path is of the form /a/b/c... but the actual path might
contain raced symlinks, so it must be scanned (by tsrm_realparth_r) to ensure
that no links are extant immediately prior to the open. There is also a sound
case for curtailing this scan on a root-owned directory, but this of second
order.
Implementing this is a small code change.
(i) First drop the open_basedir predicated clearing of
CWDG(realpath_cache_size_limit) = 0 in
main/main.c:php_module_startup().
(ii) introduce a per-call mechanism for disabling the cache. This could be
done by adding a flag to realpath, but the two variants (ZTS and none-ZTS) and
the wide use of the wrapper VCWD_REALPATH() macro might complicate this. An
alternative would be to add another flag to the PG() structure, checking this
in tsrm_realparth_r() and setting it in php_check_specific_open_basedir()
around the VCWD_REALPATH(path_tmp, resolved_name) call.
Reactions / Thoughts? Is it worth me proposing a patch?
------------------------------------------------------------------------
[2013-02-23 16:06:50] Terry at ellisons dot org dot uk
Yes Rasmus. We both know that; but this won't be address without something
like an LPC-style file-based cache to preserve context across image
activations, but all this isn't that relevant to #52312 -- "PHP
safe_mode/open_basedir - lstat performance problem".
What is relevant are my points about a require_once 6 sub-directories down
taking 13 stats and 1 open with open_basedir unset and 60 stats and 1 open if
it is set, and that the security requirement could still be implemented within
the former stat number if done correctly.
This isn't a material problem for single source file scripts, but MediaWiki,
Wordpress and the like typically load in ~100 modules generating ~6K stats per
request. And this does become a problem.
------------------------------------------------------------------------
[2013-02-23 15:22:59] [email protected]
First-request cli isn't going to have a populated realpath cache no matter what
we do since this cache is per-process and in no way shared nor persistent
across
different processes.
------------------------------------------------------------------------
[2013-02-23 13:20:03] Terry at ellisons dot org dot uk
The above discussion was largely about the I/O overheads with open_basedir
specified, so my figures where in that context, and dd2.inc is just an empty
class, but this isnt relevant to its inclusion. Try:
echo "">dummy.inc
pwd
strace -tt -o /tmp/strace.log \
php -d realpath_cache_ttl=9999 -d open_basedir=$(pwd) \
-r '$x="./dummy.inc"; require_once($x);'
to see what I mean (I did this from 6 dir levels down which is pretty typical
of web set ups.) since you can do this yourself I won't dump the log output but:
for vb in lstat fstat stat open; do
echo -n "$vb "
sed -ne '/dummy.inc/,/close(/p' /tmp/strace | grep -c " $vb"
done
lstat 54
fstat 5
stat 1
open 1 (*) I removed the open for "/etc/ld.so.cache" which isn't relevant
here
whereas dropping the open_basedir directive and repeating gives:
lstat 8
fstat 5
stat 1
open 1
so the open_basedir can have a severe impact on performance. I am still at loss
as to why the PHP cache is disabled if open_basedir is set. Surely the security
objectives would be entirely achieved by doing the real walk only during the
open routine itself?
I realise that NFS tuning can mitigate these issues -- e.g. using noatime and
setting the actimeo,... parameters or even using a local fscache (in Ubuntu
this is the cachefilesd package).
I agree that running mod_php5 with APC or O+ help a lot here, but as we've
discussed in the past, APC and O+ do not support php-cgi or php-cli. (I know
that APC has apc.enable_cli, but this seems to be a functional no-op.) We
should have acceptable PHP performance over all common usecases.
------------------------------------------------------------------------
[2013-02-23 03:44:04] [email protected]
I don't know what your dd2.inc is doing, but I went through and optimized
syscalls a
couple of years ago, and in normal no-openbasedir mode things are pretty clean.
Here
are a few scenarios:
a.php does a require on ./b.php and a require_once on ./c.php
PHP 5.4/APC-HEAD/apc.stat=1/apc.include_once_override=0/open_basedir=off
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
stat("/var/www/./b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
open("/var/www/c.php", O_RDONLY) = 29
fstat(29, {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
fstat(29, {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
fstat(29, {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
stat("/var/www/c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
The initial stat on a.php is actually done by Apache and we ask Apache for the
stat
struct to avoid an extra stat there. You can see the effect of the open and
then the
(quick) fstat calls on fd=29. This is where APC tries to optimize things a bit
using
the include_once_override. If we turn that on we get:
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
stat("/var/www/./b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
stat("/var/www/./c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
stat("/var/www/./c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
Not ideal since there is still an extra stat for the require_once case, but the
open()
is gone. And you can eliminate those stats by turning off apc.stat which means
only
the initial top-page stat from Apache on a.php will be done.
Now, of course, if we turn on open_basedir we get:
www-data 1799 0.8 0.2 310232 22560 pts/6 S+ 19:26 0:00
/usr/sbin/apache2 -X
root 1887 0.0 0.0 10892 916 pts/12 R+ 19:26 0:00 grep apache
7:26pm x220:~> strace -o out -p 1799
Process 1799 attached - interrupt to quit
^CProcess 1799 detached
7:26pm x220:~> egrep "stat|open" out
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
lstat("/var/www/./b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/var/www/b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat("/var/www/./b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www/./c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/var/www/c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat("/var/www/./c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www/./c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/var/www/c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat("/var/www/./c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
which is far from ideal, of course, because we are not hitting our realpath
cache at
all and there is an oddity there with ./ getting resolved and re-statted and
this re-
realpathed. Not sure I see where your 7 times thing is though.
With ZO+ with this config
zend_optimizerplus.revalidate_freq=0
zend_optimizerplus.enable_file_override=0
open_basedir=
we see this:
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
stat("/var/www/b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
stat("/var/www/c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
Here we see that ZO+ doesn't have the Apache stat optimization that APC has,
which is
something we should obviously add, but it handles the require_once better and
the
open() call and fstats are gone.
When we turn on open_basedir we get:
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
lstat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
lstat("/var/www/./b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
lstat("/var/www", {st_mode=S_IFDIR|0777, st_size=4096, ...}) = 0
lstat("/var", {st_mode=S_IFDIR|0755, st_size=4096, ...}) = 0
stat("/var/www/b.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
stat("/var/www/c.php", {st_mode=S_IFREG|0664, st_size=10, ...}) = 0
Better than APC, still not great, but there are no open() calls here, so the
NFS
getattr issue should be gone.
And finally, ZO+ has a TTL version of apc.stat so we can set it to only stat
every 60
seconds, so even with open_basedir enabled, with this config:
zend_optimizerplus.revalidate_freq=60
zend_optimizerplus.enable_file_override=1
we get this:
stat("/var/www/a.php", {st_mode=S_IFREG|0664, st_size=49, ...}) = 0
which is just the initial stat done by Apache which we can't do anything about.
So, in
general, assuming the 5.5 integrated opcode cache is based on ZO+ and we add
the one
missing Apache stat optimization, I really don't think the situation is all
that dire,
even for NFS-mounted scripts.
------------------------------------------------------------------------
The remainder of the comments for this report are too long. To view
the rest of the comments, please view the bug report online at
https://bugs.php.net/bug.php?id=52312
--
Edit this bug report at https://bugs.php.net/bug.php?id=52312&edit=1
