session.gc_probablity=1 in /etc/php5/apache2/php.ini conflicts with permissions on /var/lib/php5

[Jon Barber]

Binary package hint: php5

Description: Ubuntu 10.04 LTS
Release: 10.04

php5-common:
  Installed: 5.3.2-1ubuntu4.2
  Candidate: 5.3.2-1ubuntu4.2
  Version table:
 *** 5.3.2-1ubuntu4.2 0
        500 http://gb.archive.ubuntu.com/ubuntu/ lucid-updates/main Packages
        100 /var/lib/dpkg/status
     5.3.2-1ubuntu4 0
        500 http://gb.archive.ubuntu.com/ubuntu/ lucid/main Packages

/var/log/apache2/error.log: [Wed Aug 18 13:17:39 2010] [error] [client 192.168.0.103] PHP Notice: session_start(): ps_files_cleanup_dir: opendir(/var/lib/php5) failed: Permission denied (13) in /var/www/a.php on line 1

This seems to be a regression introduced by upstream packagers. php.ini used to have session.gc_probability=0 with the comment "This is disabled in the Debian packages, due to the strict permissions on /var/lib/php5". The strict permissions remain, but session.gc_probability is now enabled.

By default there's a 0.1% chance that a call to session_start() will trigger this, but setting session.gc_divisor=1 makes this easily reproducible.

The problem was originally spotted by a developer colleague, who referenced this blog entry: http://somethingemporium.com/2007/06/obscure-error-with-php5-on-debian-ubuntu-session-phpini-garbage

[Serge Hallyn (serge-hallyn)]

Confirmed - when I installed php5, the /usr/share/php5/* files had gc_probability=1.

[Serge Hallyn (serge-hallyn)]

Clint, please confirm whether there is not a good reason to set
gc_probability=0 as the package is being installed.

[Clint Byrum (clint-fewbar)]

Hi Serge,

Per this page:

http://www.php.net/manual/en/session.configuration.php#ini.session.gc-probability

The algorithm basically divides this by session.gc_divisor to decide on the percentage of the time to kick off session garbage collection. By setting it to 0, you're saying never do session garbage collection. That could be a huge problem if you have large sessions, as the dir will never be cleaned out by PHP's garbage collection.

However, the default cron job installed with php5-common cleans out /var/lib/php based on the maxlifetime. This makes a lot more sense than having 1% of your web requests pause to delete a bunch of files right before exit. Its not just about the permissions on /var/lib/php5. So that makes sense why it was 0.

However, the reasons for setting it to 1 instead of 0 are explained here:

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=388808

and here

http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=321460

Basically, its confusing. We're disabling core functionality (even if its misguided functionality) without any way to tell application developers that it is turned off.

I think the best course of action is to just make the path different for each SAPI (cgi, cli, fcgi, apache2, etc). I can see that breaking some things where maybe people are mixing fcgi and apache2, but thats a fringe case, far less common than people registering their own handlers or using the default install w/ local session files.

[Adam Conrad (adconrad)]

What does having different paths per SAPI buy you? The reason we did it this way in the first place (setting gc to 0, strict/sticky permissions, and a cronjob) was so that users all running as the same user (say, all www-data) couldn't sniff each other's session.

Both those Debian bugs were closed by documenting WHY it was set to 0, not setting it back to 1. With our setup, it should be turned off, plain and simple. Document it, sure, but don't turn it back on because some people can't read. :P

[Clint Byrum (clint-fewbar)]

Adam, thanks very much for your insight on this,

Point taken, if we are not going to relax the permissions then having multiple directories is pointless. After reading your comment and thinking about it, I have to agree that the permissions should remain so tight. My thinking was that web SAPI's could be loosened while the others stayed tight.

I think the solution is actually to do nothing. While its unfortunate that one in 1000 requests (not 100, new php.ini-production has a default gc_divisor=1000) will fail an opendir, and produce an E_NOTICE. E_NOTICE should only be turned on during development[1], so while this might be annoying, it shouldn't break anything. Meanwhile users can opt to turn it back off, and/or leave E_NOTICE suppressed.

Changing to Wont Fix, and lowering Importance to Low (since only users who have enabled E_NOTICE and left gc turned on will see this)

--

[1] http://php.net/manual/en/errorfunc.configuration.php
"In PHP 4 and PHP 5 the default value is E_ALL & ~E_NOTICE. This setting does not show E_NOTICE level errors. You may want to show them during development."

[Agustín Fernández (yo-agustinfernandez)]

I don't understand the reasoning for closing this as won't fix.

Now we have an inconsistency between the configuration and the permissions of the directory.

Either the directory permissions should be changed (and the /etc/cron,d/php5 script which deletes the files should be deleted) or the settings should be changed.

We do not turn of notices on production since they are a very good tool to find information about hard to reproduce bugs. We simply log them instead of showing them in the page. I think that usage must be pretty common.

[Clint Byrum (clint-fewbar)]

Agustín, I understand your confusion, this is a somewhat confusing topic and no one solution will work perfectly for everyone.

We're err'ing on the side of security, performance, and not breaking applications that depend on garbage collection in custom handlers in this case. If you'd like to enable E_NOTICE, you'll need to disable garbage collection or loosen the permissions on that directory. This is purely an attempt to have sane defaults.

Its probably a good idea to request that a more descriptive error message be added that suggests this. Something like "tight permissions on session storage directory prevent garbage collection. Disable garbage collection to suppress this message. Please see <url to launchpad bug> for more information."

That should probably be a separate feature request though.