|
Bugzilla – Full Text Bug Listing |
| Summary: | php7+8-fpm: session variables broken when running under user identities | ||
|---|---|---|---|
| Product: | [openSUSE] openSUSE Tumbleweed | Reporter: | Jan Engelhardt <jengelh> |
| Component: | Development | Assignee: | Arjen de Korte <suse+build> |
| Status: | VERIFIED WONTFIX | QA Contact: | E-mail List <qa-bugs> |
| Severity: | Normal | ||
| Priority: | P5 - None | CC: | dimstar, mrueckert, pgajdos |
| Version: | Current | ||
| Target Milestone: | --- | ||
| Hardware: | x86-64 | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Found By: | --- | Services Priority: | |
| Business Priority: | Blocker: | --- | |
| Marketing QA Status: | --- | IT Deployment: | --- |
| Bug Depends on: | |||
| Bug Blocks: | 1192672, 1194544 | ||
Isch blame mislead you - in oS:F pretty much all changes are by me - as i accept all SRs The change came in via https://build.opensuse.org/request/show/935380 This is due to a patch we make to the default upstream php.ini, which changes the session.save_path to /var/lib/php8. Before the above mentioned change, this would work because we didn't provide a php.ini for FPM at all, so it would run with the builtin default value /tmp. I'm not sure when we started explicitly setting the session.save_path, but something like this was mentioned in boo#229200 at a time when at startup PHP would write session files to / if this parameter was not set. I doubt this is still needed though. Since both Apache and PHP-FPM use PrivateTmp=true nowadays, this directory will not be shared with any other application anyway, so we could probably leave this at the default /tmp. In that case, it would behave the same as previously for PHP-FPM. Quick workaround would be to comment out this line in the /etc/php8/fpm/php.ini, but I'm sure you already figured that out. >at a time when at startup PHP would write session files to / if this parameter was not set
Hm. apache-mod_php/php-fpm ran and runs under wwwrun, so couldn't write to /… unless it really did so before Apache even switched identities, which I find a little hard to believe. Same for FPM.
Only php-cli would be affected (possibly run from any identity, including root). Which would explain why php.ini was only ever put to /etc/phpX/cli/.
(In reply to Jan Engelhardt from comment #3) > Hm. apache-mod_php/php-fpm ran and runs under wwwrun, so couldn't write to > /… unless it really did so before Apache even switched identities, which I > find a little hard to believe. Same for FPM. Yet this was exactly why this was changed as far as I can see. See boo#229200#c1 which decribes how a file (owned by root, so this is indeed before switching identities) is written in the / directory at startup. But this PHP bug has been fixed long ago, so I'm not sure if there is any merit in using a session.save_path directory other than /tmp. Alternatively, we might change the ownership of the /var/lib/php8 directory to root:root and set the permissions to 1777 (like /tmp). Setting session.save_path was probably needed because a long time ago, PHP wouldn't handle this properly if it was not set (see https://bugs.php.net/bug.php?id=26757). Nowadays it will work just fine if the upstream default is used (don't set session.save_path) and /tmp will be used. Because both Apache and PHP-FPM are started with PrivateTmp=true, the session files will end up there. This is an autogenerated message for OBS integration: This bug (1194414) was mentioned in https://build.opensuse.org/request/show/945331 Factory / php7 https://build.opensuse.org/request/show/945332 Factory / php8 (In reply to Arjen de Korte from comment #5) > Setting session.save_path was probably needed because a long time ago, PHP > wouldn't handle this properly if it was not set (see > https://bugs.php.net/bug.php?id=26757). > > Nowadays it will work just fine if the upstream default is used (don't set > session.save_path) and /tmp will be used. Because both Apache and PHP-FPM > are started with PrivateTmp=true, the session files will end up there. Sounds reasonable. Thanks! using /tmp is wrong. the clean solution is to create per user session directories: something like this https://nordisch.org/posts/php-fpm-apparmor/ in related news https://bugzilla.opensuse.org/show_bug.cgi?id=1194544 (In reply to Marcus Rückert from comment #9) > using /tmp is wrong. I don't see a major difference between using /var/lib/php{7,8} and /tmp. The only thing that changes here, is that we don't set an explicit default anymore. This doesn't mean there aren't better options, it's just that the default changes. > the clean solution is to create per user session directories: > > something like this https://nordisch.org/posts/php-fpm-apparmor/ This is definitely cleaner, but I don't see how this is related to this. My point would be the following: why should we revert a good setting to a bad setting as a workaround. When they are cleaner solutions for the user to follow so we could leave the default sessions directory in a clean place? (In reply to Marcus Rückert from comment #12) > My point would be the following: > > why should we revert a good setting to a bad setting as a workaround. Please elaborate on why /var/lib/php{7,8} is a good setting and /tmp a bad setting. Note that for apache-mod_php{7,8} and php-fpm, /tmp is actually a PrivateTmp. > When > they are cleaner solutions for the user to follow so we could leave the > default sessions directory in a clean place? Until recently, we never packaged a php.ini file for PHP-FPM and if users didn't provide one, the session.save_path would not be set and default to the built-in default of '/tmp'. We still don't set session.save_path after the latest patch, so really nothing has changed here, besides setting a default /etc/php{7,8}/fpm/php.ini now (to fix boo#1192672). So historically, the session.save_path has been empty for years already. For apache-mod_php{7,8} php.ini would set the session.save_path to /var/lib/php{7,8} and this directory was created with permissions 0755, owner wwwrun. Since the last changes, the session.save_path is no longer set (per upstream defaults) so it will default to '/tmp' too. The difference between these too, is that sessions stored in /var/lib/php{7,8} will survive a restart of apache, while sessions stored in PrivateTmp will be removed by a restart of apache. Other than this, I see no major difference between setting the default session.save_path to /var/lib/php{7,8} and not setting it and defaulting to /tmp (which will be a PrivateTmp). (In reply to Arjen de Korte from comment #13) > (In reply to Marcus Rückert from comment #12) > > My point would be the following: > > > > why should we revert a good setting to a bad setting as a workaround. > > Please elaborate on why /var/lib/php{7,8} is a good setting and /tmp a bad Darix, I value your expertise a lot. Could you please just explain what we had not considered when we wanted to stick with the upstream default? 1. would the new behavior be consistent among mod_php, php-fpm, php cli? They should behave the same 2. php-fpm/apache2 uses privatetmp. are we losing session files between reboots/restarts? 3. I would argue that the default to /tmp upstream is only because "that dir will always be there if someone builds from source". having it is a specific dir makes it cleaner and easier to clean up. 4. the complaint was "it breaks with different users" and my blog post shows a cleaner solution to that. 5. you need to fix openQA for this change. a minor nitpick: In the past we had /var/lib/phpX/sessions. Not sure why this dir is no longer provided in the packages. (In reply to Marcus Rückert from comment #15) > 1. would the new behavior be consistent among mod_php, php-fpm, php cli? > They should behave the same The change was for all php.ini files, yes. But again, as Arjen said earlier, php.ini for fpm had not existed until recently. > 2. php-fpm/apache2 uses privatetmp. are we losing session files between > reboots/restarts? I guess Arjen is aware, see comment 13. Certainly something for discussion. > 3. I would argue that the default to /tmp upstream is only because "that dir > will always be there if someone builds from source". having it is a specific > dir makes it cleaner and easier to clean up. Perhaps, yes. > 4. the complaint was "it breaks with different users" and my blog post shows > a cleaner solution to that. Hmm, I do not see this from comment 0. Of course there can be cleaner solutions, right, but am not sure this fits the scope of this bug. Perhaps a Hackweek topic? > 5. you need to fix openQA for this change. Yes, this is tracked as bug 1194544. And I had forgot: thanks darix for the input! (In reply to Marcus Rückert from comment #15) > 3. I would argue that the default to /tmp upstream is only because "that dir > will always be there if someone builds from source". having it is a specific > dir makes it cleaner and easier to clean up. A reason to store session data in /tmp might be that this nowadays usually is a mounted on a tmpfs volume and /var/lib/php{7,8} will be on located on persistent storage. Some applications might generate lots of session files, which might unexpectedly wear out the drive if /var is mounted on an SSD drive. On the other hand, on /tmp they will eat up RAM. tmpfs sounds nice on paper, but it only takes two tmpfs instances to fill up to their individual default maximum to bring the machine into an uncomfortable situation. That may be, but so far we have not seen any complaints about that for php-fpm users. Which means that either php-fpm is 1) not very popular amongst (open)SUSE users 2) they mitigate this problem by providing a different location in by setting php_admin_value[session.save_path] 3) the problem is not as big as you may think The session files on my systems are a few 10's of kB, you can fit a lot of those in 1 GB. If you're constrained in memory, the space these session files take up is probably not your biggest worry. Even if I value all opinions here, we need to move forward briskly. Fedora has /etc/httpd/conf.d/php.conf: php_value session.save_path "/var/lib/php/session" /etc/php-fpm.d/www.conf:php_value[session.save_path] = /var/lib/php/session And thus, because of convergence argument, comment 19 and comment 15 2., I vote to stay with /var/lib/phpX for now. (In reply to Petr Gajdos from comment #21) > Even if I value all opinions here, we need to move forward briskly. Fedora > has > > /etc/httpd/conf.d/php.conf: php_value session.save_path > "/var/lib/php/session" > /etc/php-fpm.d/www.conf:php_value[session.save_path] = > /var/lib/php/session > > And thus, because of convergence argument, comment 19 and comment 15 2., I > vote to stay with /var/lib/phpX for now. I don't want to sound pedantic, but this all started out with changing to the upstream default for the php.ini file. For most of the time, we never packaged a php.ini file for PHP-FPM (so we'd use the upstream default). Fedora uses the upstream default for php.ini too, but overrides this in /etc/httpd/conf.d/php.conf and /etc/php-fpm.d/www.conf. See also the link in comment 9 which also suggests to setup per user session.save_path in which case the value in php.ini is moot. (In reply to Arjen de Korte from comment #22) > (In reply to Petr Gajdos from comment #21) > > Even if I value all opinions here, we need to move forward briskly. Fedora > > has > > > > /etc/httpd/conf.d/php.conf: php_value session.save_path > > "/var/lib/php/session" > > /etc/php-fpm.d/www.conf:php_value[session.save_path] = > > /var/lib/php/session > > > > And thus, because of convergence argument, comment 19 and comment 15 2., I > > vote to stay with /var/lib/phpX for now. > > I don't want to sound pedantic, but this all started out with changing to > the upstream default for the php.ini file. For most of the time, we never > packaged a php.ini file for PHP-FPM (so we'd use the upstream default). > Fedora uses the upstream default for php.ini too, but overrides this in > /etc/httpd/conf.d/php.conf and /etc/php-fpm.d/www.conf. See also the link in > comment 9 which also suggests to setup per user session.save_path in which > case the value in php.ini is moot. Don't worry, you do not sound pedantic :). Simply for fpm; do s/stay/start/; done. And yes, packaging fpm/php.ini could be considered redundant, as you say, defaults would be used with the possibility to 'override' in fpm/php-fpm.conf. And we can start to set default session.save_path there, yes. I think it would be fair to discuss this broadly. https://lists.opensuse.org/archives/list/factory@lists.opensuse.org/thread/ZCKLGWLFU7SOZYSORYDG677V2OLYHTSE/ There is no big audience in the thread, unfortunately. From my side, I consider this issue resolved by current state in Factory. Thanks to everyone! No further opinions received for well over a year after proposed closing of issue. Yeah, wasn't a big issue in the end. Thanks. This is an autogenerated message for OBS integration: This bug (1194414) was mentioned in https://build.opensuse.org/request/show/1120490 Backports:SLE-15-SP5 / php81 |
I found that my PHP-FPM installation recently started losing $_SESSION variables across two requests. strace shows that the FPM process group has no permissions writing to /var/lib/php8. We find this path in /etc: # grep -r var/lib/php8 /etc/php8/ /etc/php8/fpm/php.ini:session.save_path = "/var/lib/php8" /etc/php8/cli/php.ini:session.save_path = "/var/lib/php8" /etc/php8/apache2/php.ini:session.save_path = "/var/lib/php8" (Tumbleweed) Comparison with Leap 15.3 with php7 (I also remember a time TW+PHP8 "worked"): # grep -r save_path php* php7/cli/php.ini:; session.save_path = "N;/path" php7/cli/php.ini:; session.save_path = "N;MODE;/path" php7/cli/php.ini:session.save_path = "/var/lib/php7" php7/cli/php.ini:; (see session.save_path above), then garbage collection does *not* php7/conf.d/memcached.ini:; of session.save_path after the execution of the script ends. php7/fpm/php-fpm.d/www.conf.default:; (error_log, sessions.save_path, ...). » osc blame openSUSE:Factory/php8/php8.spec 1 (dimstar_suse 2021-01-20 17:26:36 1209) %if "%{flavor}" == "fpm" 1 (dimstar_suse 2021-01-20 17:26:36 1210) make install-binaries INSTALL_ROOT=%{buildroot} 1 (dimstar_suse 2021-01-20 17:26:36 1211) install -dm 755 %{buildroot}%{php_sysconf}/fpm 16 (dimstar_suse 2021-12-06 22:59:07 1212) sed "s=@extdir@=%{extension_dir}=" php.ini-production > %{buildroot}%{php_sysconf}/fpm/php.ini 1 (dimstar_suse 2021-01-20 17:26:36 1213) #install fpm init script. By the looks of it, you made all pool workers shift from /tmp (which was writable) to /var/lib/php8 (writable only if the pool worker happened to be wwwrun, which has an elevated chance not to be the case with FPM, quite unlike Apache).