Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:96890
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: pass (pb1.pair.com: domain zend.com designates 104.47.42.117 as permitted sender)
To: "php-dev@coydogsoftware.net" <php-dev@coydogsoftware.net>
CC: "rasmus@lerdorf.com" <rasmus@lerdorf.com>, "internals@lists.php.net"
	<internals@lists.php.net>, "Anatol Belski (ab@php.net)" <ab@php.net>, "Zeev
 Suraski" <zeev@zend.com>, Nikita Popov <nikic@php.net>, Julien Pauli
	<julienpauli@gmail.com>, Joe Watkins <pthreads@pthreads.org>
Thread-Topic: [PATCH] opcache bug #69090, prepend user identifier to keys
Thread-Index: AQHSO+UMD4h3upkAiEqzeDgxtnu8QqDTwDSAgAYxXDE=
Date: Tue, 15 Nov 2016 11:41:47 +0000
Message-ID: <MWHPR02MB2477215967D9F5D543749791BFBF0@MWHPR02MB2477.namprd02.prod.outlook.com>
References: <MWHPR02MB247797DCC52957382A6E120BBFBB0@MWHPR02MB2477.namprd02.prod.outlook.com>,<20161111130329.GA22968@sliver.coydogsoftware.net>
In-Reply-To: <20161111130329.GA22968@sliver.coydogsoftware.net>
Accept-Language: en-US
Content-Language: en-US
x-ms-exchange-messagesentrepresentingtype: 1
x-originating-ip: [25.164.163.132]
x-microsoft-exchange-diagnostics: 1;BY2PR02MB297;7:XNkSBPabk16wxX5hCgdlo4TBnRx6qDRuc99SWkYj7Pxmb1qpgfk/ZLNqAelaP4MRkG/ql8l4NA4gcHQZ7G5MHzxbT+WWMONhL4AXbhjnnnpFKN2GwTSQBBJiM7swDpfvAhZshORIpvKay7lTJCee5v3KiLwjsLlkl4IlJgFe8MoPXNt2NfClSgaUAHZYmTNEznX6lbAD/6fTzFsDDN1XPHMj/bRqs60dXb+OZclAwtFx2aYL1qu5IJZ9QARBknqHHvhErQfxy2Z/5/ylGuZ2u7GxGD7X5HYiXgowwiJDTyabRShSz2yjwGM1wXPWF5fC+ZuwWBOuNUpZbFIXnwR4s20eWLCTAPrdL1OrTgD2voI=
x-forefront-antispam-report: SFV:SKI;SCL:-1SFV:NSPM;SFS:(10019020)(7916002)(189002)(46034005)(199003)(24454002)(377454003)(81156014)(2900100001)(8676002)(4326007)(122556002)(3846002)(92566002)(6116002)(345774005)(8936002)(2906002)(68736007)(102836003)(87936001)(76176999)(86362001)(7906003)(5640700001)(6916009)(16799955002)(3660700001)(229853002)(15188155005)(7846002)(2950100002)(7696004)(110136003)(5660300001)(7736002)(74316002)(50986999)(66066001)(101416001)(99286002)(2501003)(106356001)(105586002)(106116001)(54356999)(76576001)(9686002)(2351001)(3280700002)(97736004)(77096005)(81166006)(5890100001)(189998001)(33656002)(15395725005);DIR:OUT;SFP:1102;SCL:1;SRVR:BY2PR02MB297;H:MWHPR02MB2477.namprd02.prod.outlook.com;FPR:;SPF:None;PTR:InfoNoRecords;MX:1;A:1;LANG:en;
x-ms-office365-filtering-correlation-id: 3e060963-2bbd-4551-cce1-08d40d4c6209
x-microsoft-antispam: UriScan:;BCL:0;PCL:0;RULEID:(22001);SRVR:BY2PR02MB297;
x-microsoft-antispam-prvs: <BY2PR02MB2975342DFD0464044610D1EBFBF0@BY2PR02MB297.namprd02.prod.outlook.com>
x-exchange-antispam-report-test: UriScan:(158342451672863);
x-exchange-antispam-report-cfa-test: BCL:0;PCL:0;RULEID:(6060326)(601004)(2401047)(8121501046)(5005006)(10201501046)(3002001)(6061324);SRVR:BY2PR02MB297;BCL:0;PCL:0;RULEID:;SRVR:BY2PR02MB297;
x-forefront-prvs: 012792EC17
received-spf: None (protection.outlook.com: zend.com does not designate
 permitted sender hosts)
spamdiagnosticoutput: 1:99
spamdiagnosticmetadata: NSPM
Content-Type: multipart/alternative;
	boundary="_000_MWHPR02MB2477215967D9F5D543749791BFBF0MWHPR02MB2477namp_"
MIME-Version: 1.0
X-OriginatorOrg: zend.com
X-MS-Exchange-CrossTenant-originalarrivaltime: 15 Nov 2016 11:41:47.5584
 (UTC)
X-MS-Exchange-CrossTenant-fromentityheader: Hosted
X-MS-Exchange-CrossTenant-id: 32210298-c08b-4829-8097-6b12c025a892
X-MS-Exchange-Transport-CrossTenantHeadersStamped: BY2PR02MB297
Subject: Re: [PATCH] opcache bug #69090, prepend user identifier to keys
From: dmitry@zend.com (Dmitry Stogov)

--_000_MWHPR02MB2477215967D9F5D543749791BFBF0MWHPR02MB2477namp_
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: quoted-printable

hi,


I've solved one of the problems described at https://bugs.php.net/bug.php?i=
d=3D69090

The patch and explanation are attached to bug report.

Please, review.


Julien, Anatol, Joe, are you OK to commit this into 5.6 and above?


I'm going to think about the second problem.


Thanks. Dmitry.

________________________________
From: php-dev@coydogsoftware.net <php-dev@coydogsoftware.net>
Sent: Friday, November 11, 2016 4:03:29 PM
To: Dmitry Stogov
Cc: rasmus@lerdorf.com; internals@lists.php.net; Anatol Belski (ab@php.net)=
; Zeev Suraski
Subject: Re: [PATCH] opcache bug #69090, prepend user identifier to keys

Dmitry,

Thank you for taking the time to answer my questions. Time allowing,
I'll be taking a closer look at the code this weekend. I do have a
couple of quick comments, see below:

On Fri, Nov 11, 2016 at 07:31:03AM +0000, Dmitry Stogov wrote:
> On Nov 10, 2016 5:10 PM, php-dev@coydogsoftware.net wrote:
> >
> > Can you confirm that you see the permissions bypass problem? I've seen
> > the chroot filename collision problem acknowledged in the bugtracker an=
d
> > in old php-internals posts, but I've seen nobody from the PHP Project
> > explicitly acknowledge the permissions bypass vulnerability. If my
> > meaning isn't clear I can provide proof of concept off-list. The
> > permissions bypass affects both apache2handler (even with mod_ruid2) an=
d
> > FPM (even with user pools).
>
> I didn't see the problem in real life, but it's clear, that serving of
> few chroot environments using the same cache may lead to duplicate
> keys. FPM with separate pools shouldn't be affected.

FPM with separate user pools under a single master is affected by the
permissions bypass issue. To avoid the issue, separate user pools isn't
sufficient; you would need separate FPM master daemons.

Many users read about the ability to run separate pools with separate
users under a single master and think this provides adequate user
separation:
http://php.net/manual/en/install.fpm.configuration.php

But when OPCache is enabled, the user pools under a single master all
share a common cache, with disastrous results if a single user is
compromised.

Unfortunately this is the way the popular shared hosting control panels
have started implementing FPM: with a single master.

Was "single master, multiple pools with separate users" not intended for
a shared hosting environment? If not, what is the point of the 'user'
and 'chroot' directives? Were shared hosts using FPM always expected to
use separate FPM master daemons? If so, the documentation might be more
explicit IMHO (I'd be willing to put in some work here if you feel a
documentation fix is what's needed).

> > But again I should stress that *chroot filename collisions are not the
> > only bad behavior here.* They're not the bug I'm most concerned with.
>
> Do you talk about executing "unreadable" PHP scripts of different
> users?  I think, the proper way to fix this, whould executing access()
> check on each cached script access (this might be enabled/disabled
> through php.ini)

Yes! This is exactly my concern. And you're absolutely right, a check of
access() at script compile time is a better solution than my patch. I
think it should be the default behavior though.

I'm starting to think I should have opened a separate bug for the
permissions bypass issue and the chroot filename collision; in the
bug tracker users were already lumping these issues together but they're
really separate concerns.

Off-list I'm going to send you a proof of concept script which
demonstrates the problem with a typical FPM/OPCache deployment in a
shared environment.

-php-dev at coydogsoftware dot net

--_000_MWHPR02MB2477215967D9F5D543749791BFBF0MWHPR02MB2477namp_--