Req #69090 [Com]: add prefix/xor to cache keys/check permissions or separate caches

From: Date: Sun, 06 Nov 2016 20:54:55 +0000
Subject: Req #69090 [Com]: add prefix/xor to cache keys/check permissions or separate caches
References: 1  Groups: php.bugs 
Request: Send a blank email to php-bugs+get-205212@lists.php.net to get a copy of this message
Edit report at https://bugs.php.net/bug.php?id=69090&edit=1

 ID:                 69090
 Comment by:         php-dev at coydogsoftware dot net
 Reported by:        simon at ikanobori dot jp
 Summary:            add prefix/xor to cache keys/check permissions or
                     separate caches
 Status:             Analyzed
 Type:               Feature/Change Request
 Package:            opcache
 Operating System:   linux/debian
 PHP Version:        5.6.5
 Block user comment: N
 Private report:     N

 New Comment:

bjh438-git, thank you for your comments. I had previously read that article. The setup described
mitigates the security concerns I've outlined for one reason: It recommends disabling OPCache
completely.

Such a multi-user setup is generally recommended, but as I mentioned in my response to jpauli,
it's dangerous when combined with a single SHM cache with a simplistic hash keying scheme which
makes no attempt to segregate users.

My readings of php-internals archives suggest that OPCache was indeed meant to be usable in
multi-user setups so perhaps the maintainers simply haven't thought through the all
implications of the key scheme, though they have discussed its other shortcomimgs. I'm working
on a brief but more comprehensive advisory which I'll post here and on php-internals soon if we
don't get more meaningful engagement on this issue.


Previous Comments:
------------------------------------------------------------------------
[2016-11-06 17:27:01] bjh438-git at yahoo dot com

php-dev - would it not be best (at least when using fpm) to run each pool under a separate user /
group account as discussed here: https://www.digitalocean.com/community/tutorials/how-to-host-multiple-websites-securely-with-nginx-and-php-fpm-on-ubuntu-14-04


This would seem to satisfy the original suggestions earlier in the thread to leave as much as
possible to the OS level security controls.

I mention because I was wondering if your patches (which are warmly welcomed...I've been
posting on this issue for a while now on SF & SE) presume that these types of controls would
also be implemented or perhaps obviate them.

------------------------------------------------------------------------
[2016-11-04 20:11:41] php-dev at coydogsoftware dot net

I'll address jpauli's points:

Not everyone is using FPM. My patch fixes this problem on both FPM and apache2handler, where we can
separate users with mod_ruid2.

My experience with FPM is limited so I may be speaking out of turn, but when I tried separate FPM
pools with separate users, they were still forked from the same parent FPM master process. Correct
me if I'm wrong but the OPCache SHM segment is opened in this master process and inherited by
the pools as a file descriptor. The multi-user pools still share a single OPCache, and thus they
actually aid in bypassing file permissions, rather than fixing the problem.

I agree 100% with your points about relying on the lower stack to isolate vhosts and enforce
permissions, but the entire point of this bug report is that OPCache breaks this isolation in
real-world configurations since it uses a shared cache passed from Apache parent to children, or
from FPM master to pools.

You stated "Obviously, the OPCache SHM wouldn't be shared here" in a multi-pool
configuration. What would such a configuration look like? My testing shows the opposite to be true.
Perhaps you meant to suggest multiple FPM master daemons instead of multiple pools? If the OPCache
SHM can be initialized at the pool level instead of in the master process (I don't think this
is how it works today; I'd love to be proven wrong on this), this is great for FPM users but
does nothing for users of other SAPI's.

The big caveat in my testing is that most of it was done under cPanel. Both mod_ruid2/mod_php and
FPM have the shared OPCache problem under both EA3 and EA4. This is true for both PHP5 and PHP7. If
you think this is a problem with cPanel's apache2handler and FPM configurations then I can take
the issue up with cPanel, but I'd love to see how they could fix this for all SAPI's where
OPCache is useful. Clearly the problem isn't limited to cPanel though, based on the other users
commenting on this bug.

Hopefully this will clarify users' concerns with this bug; from your response I'm frankly
not sure you fully understand the problem we're reporting. I agree with "jeff at mcneill
dot io" that to isolate vhosts with PHP as it stands today, you'd need entirely separate
Apache parents (for apache2handler) or FPM master processes (not just multiple pools).

------------------------------------------------------------------------
[2016-11-04 17:11:34] jeff at mcneill dot io

The suggestion as one user per site would have to apply all the way across the stack. So Apache
would have to have a single user, and no multi-site, no virtual sites, no resource sharing at all?
This would mean full resources for each and every site, which would be extremely wasteful. I have a
client with four websites. Does the single client need four users? I have several clients but I want
to run all clients and all their sites with a single configuration. Apache can do this, PHP can do
this, therefore opcache should be able to support such a configuration.

------------------------------------------------------------------------
[2016-11-04 16:54:30] jpauli@php.net

Excuse me but...

Wouldn't it be safer, more reliable, less hackish ... to separate the PHP pools of the website
? One pool per site, each pool listening on one port for CGI requests.

I mean, we do know for sure, that PHP cannot replace the OS and the configuration to secure
webservers in a shared architecture (shared hosting).
We've tried things such as safe_mode, open_basedir etc... since the beginning, with no success.
Nowadays, with mature OS, mature stacks, and a mature FCGI handler (PHP-FPM), it is easy to build a
shared hosting architecture not relying on the PHP language itself to isolate the virtualhosts.

Security - of the filesystem here - is the matter of the OS , not PHP.

Why don't you create several PHP-FPM pools, and secure them with one Unix user per pool ?
Obviously, the OPCache SHM wouldn't be shared here, but that is not what you want : OPCache SHM
shouldn't be crossed against several websites.

One pool per website = one SHM per website = one unix user per website = we solved every low level
security problems, right ?

------------------------------------------------------------------------
[2016-11-04 10:32:56] php-dev at coydogsoftware dot net

At this point two separate but related issues are being discussed:

 1. OPCache is prone to filename collisions across chroots. Adding inode number to the key would fix
this.

 2. All child PHP processes have access to all scripts in the cache, regardless of file permissions.
This renders OPCache unusable and dangerous on shared hosting servers, where OPCache may be used by
malicious users to bypass file permissions (and open_basedir FWIW). Adding inode to the cache key
would *not* fix this vulnerability, although in practice it would help in cases where parent
directory is unreadable.

I'll focus on the second issue: The obvious real-world example is a shared hosting server with
multiple CMS sites. Most PHP CMS's store database credentials and other sensitive information
in PHP scripts. Thus one malicious user (translation: compromised CMS) typically has full access to
databases for other users' CMS's if OPCache is enabled. If anyone doubts this, I can
provide working proof of concept exploit scripts targeted at WordPress. Fortunately this
doesn't seem to be exploited in the wild on any large scale, but I anticipate PHP malware will
start incorporating this technique as soon as it's more widely known.

I'm attaching a patch against the 5.6 branch which prepends a unique user identifier (username
in Windows, euid elsewhere) to the cache key. This should fix issue 2 in all situations where dl()
is not allowed (PHP5 with FPM would still in theory be vulnerable unless dl() is disabled).

It's not a perfect fix because it requires the PHP process to act as gatekeeper, essentially a
substitute for kernel enforcement of filesystem permissions. This is a problem if any PHP child
process with a descriptor for the shared opcache can be subverted, for example with a malicious
extension, hence my concern about dl()).
	
It will also fix issue 1 *only* in cases where the chroot environments are running PHP scripts with
separate user accounts. If the chroots use a shared web server user for PHP, issue 1 is still a
problem.

I agree with Rasmus that the script inode should also be added to the key, but I wasn't sure if
it would be appropriate to stat the script for its inode in the key generation function due to
performance concerns, and being unfamiliar with the extension and PHP itself I wasn't prepared
to do this in a more appropriate place.

I started with 5.6 because I believe this is a fairly serious security vulnerability which many
users are unaware of, which isn't adequately explained in the OPCache documentation, and which
should IMHO be fixed in a patch release. I'm willing to port it forward to 7.x if there's
interest and time allows, but I feel strongly that this patch or something similar should be
included in a 5.6 patch release ASAP.

Code is only minimally tested. Use at own risk. Apologies for any indentation issues; I did my best
to follow the style guide, but existing OPCache code did not. Feedback is welcome.

------------------------------------------------------------------------


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=69090


--
Edit this bug report at https://bugs.php.net/bug.php?id=69090&edit=1


Thread (38 messages)

« previous php.bugs (#205212) next »