Everything
php
Learn Community Releases The PHP Project
Home » Project » Newsgroups » php.internals » [PATCH] make dl() DTRT under safe mode »

[PATCH] make dl() DTRT under safe mode

php.internals

Steve Langasek

23 years ago
The attached patch changes dl() so that, instead of outright refusing to run under safe mode, it performs additional security checks on the value of extension_dir and accepts a filename-only argument (no directories) from the caller. In addition, if the provided argument doesn't return a handle, dl will append the value of PHP_SHLIB_SUFFIX to the filename and try again. Rationale: Most Linux distributions that include PHP ship the majority of extensions as shared objects, to make it easier for the user to get just those features they need. In addition, there are many extensions not provided by Linux distributors that a user may wish to add locally. Unfortunately, current PHP programmer culture leads to all extensions being loaded via php.ini, which -- contrary to some claims made in the past on this list -- often causes instability when used with apache 1.3. Simply put, the current state of libraries on Linux isn't good enough to cope with all possible libraries being loaded into a single apache process at once; and SSL-using extensions seem to have problems of their very own when it comes to Apache's load-unload-load-rinse-spindry handling of DSOs at start time. Supporting a reasonable dl() function under safe mode is the first step toward being able to wean users off of this fragile configuration.
-- Steve Langasek postmodern programmer

Derick Rethans

23 years ago
On Sat, 16 Aug 2003, Steve Langasek wrote:
> The attached patch changes dl() so that, instead of outright refusing to > run under safe mode, it performs additional security checks on the value > of extension_dir and accepts a filename-only argument (no directories) > from the caller. In addition, if the provided argument doesn't return a > handle, dl will append the value of PHP_SHLIB_SUFFIX to the filename and > try again. > > Rationale: Most Linux distributions that include PHP ship the majority > of extensions as shared objects, to make it easier for the user to get > just those features they need. In addition, there are many extensions > not provided by Linux distributors that a user may wish to add locally. > Unfortunately, current PHP programmer culture leads to all extensions > being loaded via php.ini, which -- contrary to some claims made in the > past on this list -- often causes instability when used with apache 1.3.
Loading them through dl() gives much more instability. I don't see where you got this claim from, I never saw a bugreport about this.
> Simply put, the current state of libraries on Linux isn't good enough to > cope with all possible libraries being loaded into a single apache > process at once; and SSL-using extensions seem to have problems of their > very own when it comes to Apache's load-unload-load-rinse-spindry > handling of DSOs at start time. Supporting a reasonable dl() function > under safe mode is the first step toward being able to wean users off of > this fragile configuration.
I don't agree, dl() is kinda deprecated anyway. It's buggy as hell and it causes quite a lot of problems with intra-extension dependencies. If you want something solid, you compile in all extensions. This has nothing to do with safe-mode really. regards, Derick
-- "Interpreting what the GPL actually means is a job best left to those that read the future by examining animal entrails." ------------------------------------------------------------------------- Derick Rethans http://derickrethans.nl/ International PHP Magazine http://php-mag.net/ -------------------------------------------------------------------------

Steve Langasek

23 years ago
On Sun, Aug 17, 2003 at 11:52:13AM +0200, Derick Rethans wrote:
> On Sat, 16 Aug 2003, Steve Langasek wrote:
> > The attached patch changes dl() so that, instead of outright refusing to > > run under safe mode, it performs additional security checks on the value > > of extension_dir and accepts a filename-only argument (no directories) > > from the caller. In addition, if the provided argument doesn't return a > > handle, dl will append the value of PHP_SHLIB_SUFFIX to the filename and > > try again.
> > Rationale: Most Linux distributions that include PHP ship the majority > > of extensions as shared objects, to make it easier for the user to get > > just those features they need. In addition, there are many extensions > > not provided by Linux distributors that a user may wish to add locally. > > Unfortunately, current PHP programmer culture leads to all extensions > > being loaded via php.ini, which -- contrary to some claims made in the > > past on this list -- often causes instability when used with apache 1.3.
> Loading them through dl() gives much more instability. I don't see where > you got this claim from, I never saw a bugreport about this.
http://bugs.debian.org/131080 http://bugs.debian.org/131724 http://bugs.debian.org/165699 http://bugs.debian.org/165718 http://bugs.debian.org/165719 http://bugs.debian.org/166414 http://bugs.debian.org/188014 http://bugs.debian.org/188025 http://bugs.debian.org/188058 http://bugs.debian.org/189202 http://bugs.debian.org/189653 http://bugs.debian.org/200255 http://bugs.debian.org/205592 This claim comes from over a year of fighting with linker/library bugs that impact *nothing else* in Debian besides PHP. They don't get filed against bugs.php.net because they're almost always library bugs, not PHP bugs; indeed, the above list is absolutely *not* definitive, because most such bugs get reassigned to library packages for fixing, and I lose track of them. But even though the libraries have bugs, these bugs are *only* exposed by PHP. No other scripting language has problems like these, not even other languages that are loaded as Apache DSOs (mod_python, mod_perl); and in every case, these bugs could be resolved by removing an 'extension=' line from php.ini and using dl() instead. Last March, I offered some comments on this mailing list in response to dl() discussion suggesting that 35-40% of all showstopper bugs in Debian's PHP packages could be resolved by use of dl(), and that the percentage was falling. Based on the past year, I find that I need to revise that estimate to 70%. Now, the other possibility is that taking libc-client out back and shooting it would also resolve all of the PHP-related crashes, since php4-imap is at the core of most of them. Unfortunately, php4-imap is also one of the most widely used extensions, and porting it to a suitable alternative library is not an endeavor to be undertaken lightly.
> > Simply put, the current state of libraries on Linux isn't good enough to > > cope with all possible libraries being loaded into a single apache > > process at once; and SSL-using extensions seem to have problems of their > > very own when it comes to Apache's load-unload-load-rinse-spindry > > handling of DSOs at start time. Supporting a reasonable dl() function > > under safe mode is the first step toward being able to wean users off of > > this fragile configuration.
> I don't agree, dl() is kinda deprecated anyway. It's buggy as hell and > it causes quite a lot of problems with intra-extension dependencies.
*Not* using dl() causes a lot of problems with inter-extension dependency conflicts. Under the circumstances, I believe it's important to support both modes of operation equally, to the extent possible.
> If you want something solid, you compile in all extensions. This has > nothing to do with safe-mode really.
So you don't intend to support distributors of PHP binaries for Unix? Monolithic binaries are not an option for us: we're actively trying to *reduce* the number of dependencies of any one PHP package in Debian right now in the interest of release management, and there are some extensions we wouldn't be able to compile in if we wanted to, that users may still want to load later. Loading all of the shared extensions at startup causes crashes. That seems to leave dl() as the only option, but you apparently won't accept a patch to even make dl() work for all Apache 1.3 users. I understand from the source that there are concerns about the viability of dl() under threaded webservers; and when the time comes, I'm prepared to work on sorting out such stability problems if need be (it's also possible that Apache2's startup process is nicer to libraries than Apache 1.3's is). But my users are *not* using ZTS today; rather, they *are* seeing crashes that could be resolved through the use of dl().
-- Steve Langasek postmodern programmer

Rasmus Lerdorf

23 years ago
On Sun, 17 Aug 2003, Steve Langasek wrote:
> http://bugs.debian.org/131080 > http://bugs.debian.org/131724 > http://bugs.debian.org/165699 > http://bugs.debian.org/165718 > http://bugs.debian.org/165719 > http://bugs.debian.org/166414 > http://bugs.debian.org/188014 > http://bugs.debian.org/188025 > http://bugs.debian.org/188058 > http://bugs.debian.org/189202 > http://bugs.debian.org/189653 > http://bugs.debian.org/200255 > http://bugs.debian.org/205592
Most of these seem to be symbol clash problems (Ok, I didn't read them all) or problems vaguely attributed to Apache's double init phase on startup. Your conclusion that pushing towards dl() instead of figuring out what is messing up the extension=foo.so style of loading libraries is misguided. dl() is slow because the library is loaded and unloaded on every single request. dl() is clunky because it isn't, and I don't see how it could ever be, threadsafe. And finally it has some nasty security implications. I understand you want to address these with safemode changes, but many servers don't run safemode (because it is not actually very safe and in general it is crap) and the damage a user can do is somewhat limited. By forcing them to allow dl() users can now load arbitrary C code directly into the web server process and really really mess stuff up. Nobody here is saying that we don't want to support folks building binary distributions of PHP. We are just saying that your chosen path is a bad idea and that we need to address the real problem instead. Architecturally it makes absolutely no sense to load and unload libraries on a per-request basis. This has to happen at server startup. One thing we should probably do is not dlclose() any shared extensions on shutdown. I am not sure if your list of bugs included any reports of crashes on shutdown, but if you ever come across one of those it is likely that removing the dlclose() will fix it. -Rasmus

Steve Langasek

23 years ago
On Sun, Aug 17, 2003 at 09:12:57AM -0700, Rasmus Lerdorf wrote:
> On Sun, 17 Aug 2003, Steve Langasek wrote: > > http://bugs.debian.org/131080 > > http://bugs.debian.org/131724 > > http://bugs.debian.org/165699 > > http://bugs.debian.org/165718 > > http://bugs.debian.org/165719 > > http://bugs.debian.org/166414 > > http://bugs.debian.org/188014 > > http://bugs.debian.org/188025 > > http://bugs.debian.org/188058 > > http://bugs.debian.org/189202 > > http://bugs.debian.org/189653 > > http://bugs.debian.org/200255 > > http://bugs.debian.org/205592
> Most of these seem to be symbol clash problems (Ok, I didn't read them > all) or problems vaguely attributed to Apache's double init phase on > startup.
In general, yes.
> Your conclusion that pushing towards dl() instead of figuring out what is > messing up the extension=foo.so style of loading libraries is misguided.
I don't think it's misguided to want to mitigate the impact of library bugs on PHP. Some of these bugs have taken months at a time to iron out, because they require the cooperation of upstream library developers to fix in the case of symbol collisions. (The Kerberos symbol collision bug -- still unresolved -- requires the cooperation of two upstream library maintainers and four Debian package maintainers to fix.) Our users are better served by not being exposed to such bugs in the first place.
> dl() is slow because the library is loaded and unloaded on every single > request. dl() is clunky because it isn't, and I don't see how it could > ever be, threadsafe. And finally it has some nasty security implications. > I understand you want to address these with safemode changes, but many > servers don't run safemode (because it is not actually very safe and in > general it is crap) and the damage a user can do is somewhat limited. By > forcing them to allow dl() users can now load arbitrary C code directly > into the web server process and really really mess stuff up.
Well, I'm not convinced that it couldn't be made threadsafe, at least on the platforms I'm concerned with. I'm told that glibc 2.3's dlopen()/dlclose() is supposed to have thread-intelligent reference counting. And I certainly agree that there are some tradeoffs to consider when enabling dl() support on a server; however, if safe_mode is not enabled, there are lots of other ways you can do nasty things to the server using arbitrary C code -- system(), for example. (Given that a well-constructed [or poorly constructed?] system() can far outlive a single HTTP request, I'd be more concerned about the potential for damage there.) As for performance: I'm sure many of our users would gladly sacrifice a bit of performance in exchange for something that works when they ask it to. A fully functional dl(), together with defensive coding on the part of PHP script authors, would give users the flexibility they need when things don't work right -- in /either/ direction.
> Nobody here is saying that we don't want to support folks building binary > distributions of PHP. We are just saying that your chosen path is a bad > idea and that we need to address the real problem instead. > Architecturally it makes absolutely no sense to load and unload libraries > on a per-request basis. This has to happen at server startup.
Addressing the "real problem" means chasing down each bug individually in a different library. I've spent a year doing that, and I no longer agree that it's the right way to proceed. The imap/ssl bugs have been a game of hide-and-seek from the start, and the most recent one -- http://bugs.debian.org/205553 -- doesn't even appear to be a problem with a mislinked library. Rather, there seems to be something broken deep in the source code of either libssl/libcrypto or libc-client. Since those are the top two hairiest libraries I've ever worked with, I'm not particularly enthusiastic about trying to chase down the one line of hair that happens to host a bug; but if you want to tackle that, I would certainly be grateful to see the bug fixed.
> One thing we should probably do is not dlclose() any shared extensions on > shutdown. I am not sure if your list of bugs included any reports of > crashes on shutdown, but if you ever come across one of those it is likely > that removing the dlclose() will fix it.
I don't recall ever seeing such a bug report. At least in the case of library symbol conflicts, leaving the shared extension open after the end of a request would have exactly the opposite effect, though.
-- Steve Langasek postmodern programmer

Rasmus Lerdorf

23 years ago
On Sun, 17 Aug 2003, Steve Langasek wrote:
> > One thing we should probably do is not dlclose() any shared extensions on > > shutdown. I am not sure if your list of bugs included any reports of > > crashes on shutdown, but if you ever come across one of those it is likely > > that removing the dlclose() will fix it. > > I don't recall ever seeing such a bug report. At least in the case of > library symbol conflicts, leaving the shared extension open after the > end of a request would have exactly the opposite effect, though.
I meant on server shutdown, not request shutdown. There are a couple of problems in this area. The most obvious affects C++ extensions that use any sort of function-scoped static objects. If you are using an older version of gcc/glibc that doesn't support per-shared-library atexit stacks or if you have compiled your extension without --cxa-atexit (or whatever that option is called) then at shutdown we unload the shared extension but the destructor for the function-scoped static object is sitting on the process atexit stack referring to an address in the now unloaded extension code. -Rasmus

Unnamed Person

23 years ago
"Rasmus Lerdorf" <rasmus@lerdorf.com> a écrit dans le message de news:Pine.LNX.4.56.0308170900400.4211@www...
> On Sun, 17 Aug 2003, Steve Langasek wrote: > > http://bugs.debian.org/131080 > > http://bugs.debian.org/131724 > > http://bugs.debian.org/165699 > > http://bugs.debian.org/165718 > > http://bugs.debian.org/165719 > > http://bugs.debian.org/166414 > > http://bugs.debian.org/188014 > > http://bugs.debian.org/188025 > > http://bugs.debian.org/188058 > > http://bugs.debian.org/189202 > > http://bugs.debian.org/189653 > > http://bugs.debian.org/200255 > > http://bugs.debian.org/205592 > > Most of these seem to be symbol clash problems (Ok, I didn't read them > all) or problems vaguely attributed to Apache's double init phase on > startup. > > Your conclusion that pushing towards dl() instead of figuring out what is > messing up the extension=foo.so style of loading libraries is misguided. > dl() is slow because the library is loaded and unloaded on every single > request. dl() is clunky because it isn't, and I don't see how it could > ever be, threadsafe. And finally it has some nasty security implications. > I understand you want to address these with safemode changes, but many > servers don't run safemode (because it is not actually very safe and in > general it is crap) and the damage a user can do is somewhat limited. By > forcing them to allow dl() users can now load arbitrary C code directly > into the web server process and really really mess stuff up. > > Nobody here is saying that we don't want to support folks building binary > distributions of PHP. We are just saying that your chosen path is a bad > idea and that we need to address the real problem instead. > Architecturally it makes absolutely no sense to load and unload libraries > on a per-request basis. This has to happen at server startup. > > One thing we should probably do is not dlclose() any shared extensions on > shutdown. I am not sure if your list of bugs included any reports of > crashes on shutdown, but if you ever come across one of those it is likely > that removing the dlclose() will fix it.
Well sure it will be more efficient, but then anyone will be able to use a shared extension of someone else? What about shared hostings and conflict names?
> > -Rasmus
-- Regards. M.CHAILLAN Nicolas nicos@php.net www.WorldAKT.com Hébergement de sites internets.

Zeev Suraski

23 years ago
At 01:30 18/08/2003, nicos@php.net wrote:
>Well sure it will be more efficient, but then anyone will be able to use a >shared extension of someone else?
Yes, there's nothing wrong with that. You're making it sound as if it's using other people's PHP code, which in almost all cases, it isn't. If you have something private and miraculously you were given permission by the administrator to install it, and you don't want others to use it, password protect it or something.
>What about shared hostings and conflict names?
I don't see a problem here at all. Again, this is not PHP code. Zeev

Unnamed Person

23 years ago
"Zeev Suraski" <zeev@zend.com> a écrit dans le message de news:5.1.0.14.2.20030818104512.055a9968@localhost...
> At 01:30 18/08/2003, nicos@php.net wrote: > >Well sure it will be more efficient, but then anyone will be able to use
a
> >shared extension of someone else? > > Yes, there's nothing wrong with that. You're making it sound as if it's > using other people's PHP code, which in almost all cases, it isn't. If
you
> have something private and miraculously you were given permission by the > administrator to install it, and you don't want others to use it, password > protect it or something. > > >What about shared hostings and conflict names? > > I don't see a problem here at all. Again, this is not PHP code.
Hmm, I thought that if we remove dlclose() after every execution then someone else might use the functions loaded with dl()... But well it looks fine if you say so.
> > Zeev >
nicos.