Edit report at https://bugs.php.net/bug.php?id=52312&edit=1
ID: 52312 Updated by: [email protected] 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: 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. Previous Comments: ------------------------------------------------------------------------ [2013-02-23 00:49:33] Terry at ellisons dot org dot uk Incidentally I've just straced opening a require_once on /home/terry/work/ext/lpc/tests/dd2.inc and PHP 5.3.17 does the 7 x lstat walk that you indicate is necessary to meet CVE-2006-5178, but it then repeats this another 5 times and does another 3 fstats after the file is opened 45 f/lstats all in 2.5μS (on a local disk clearly). PHP 5.4.6 is worse, it does 5x7 walks, opens the file then does 4 fstatsand reads the file, before doing another 3x7 lstat walks and a last stat for luck, giving a grand total of 60 f/lstats in 4μS though this one includes mmapping the contents from the file cache. OK, I can understand -- though question -- mandating one walk but why do it seven times ???? ------------------------------------------------------------------------ [2013-02-23 00:08:51] Terry at ellisons dot org dot uk For the purposes of this bug, let's document the advisory which triggered this change: CVE-2006-5178 see http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2006-5178. Let's be honest, even with this patch there is still a minuscule chance that the exploit described could succeed, because we can't go into kernel mode during the recursive descent and prevent other processes with a higher nice being scheduled in and doing the dirty. So it replaces "very small" by "even smaller" at the same time killing NFS performance. As I said don't remove this change, just make it elective by adding an extra variant of the open_basedir parameter, and openly documenting the issues so that sysadmins can make their own judgement call on security vs performance. Let's face it on a typical shared service I can think of easier ways to do this exploit. E.g. if exec and the proc functions aren't disabled then there is no point in having a strong open_basedir. If you are an enterprise and just want to lock down your apps programmers from breaching good practice / infrastructure standards over a NAS infrastructure then the weak form is perfectly appropriate. Also IMO, PHP is trying to do a better job here than the standard Linux realpath function and failing. ------------------------------------------------------------------------ [2013-02-22 23:26:14] [email protected] There is no such thing as a "miniscule race" when it comes to computers. Either there is a race condition or there isn't. In this case there is. So if we remove this check open_basedir will be much less secure. Something along the lines of Marcin's idea of separate caches might be feasible, but this is not a small change. ------------------------------------------------------------------------ [2013-02-22 23:17:59] Terry at ellisons dot org dot uk This bug is extant in 5.3 and 5.4 (up to 5.4.9). The flaw is the logic in main/main.c:php_module_startup(): /* Disable realpath cache if safe_mode or open_basedir are set */ if (PG(safe_mode) || (PG(open_basedir) && *PG(open_basedir))) { CWDG(realpath_cache_size_limit) = 0; } Similar logic validly exists in code such as suEXEC and suPHP to prevent a race condition allowing and unscrupulous script author to execute a script in another UID in certain circumstances. This simply doesn't apply in this case. Which technically a similar race could allow a script author to break the basedir contain with miniscule probability by exploiting such a race, the performance impacts are significant, for no material increase in security strength. Get rid of this rule or at least split open_basedir into two separate directives open_basedir_paranoid which disables the cache as above and open_basedir which does the rational checks and doesn't disable the cache. This has been lying around for nearly 3 years. How can we progress this? ------------------------------------------------------------------------ [2012-07-31 20:00:48] marcin at 6irc dot net How about having concurrent cache tables for each basedir setting? For instance, when open_basedir is set to '/home/teh1234;/tmp', then the lstat populates only cache table "0", realpath_cache_* also uses exclusively this cache, and when open_basedir is set to '/home/klaczy;/tmp' then it populates and uses only cache table "1"? Are there any security considerations that I don't notice here? ------------------------------------------------------------------------ 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
