php54-fpm "warning: user apache does not exist - using root"

Bug #1312972 reported by kustodian
6
This bug affects 1 person
Affects Status Importance Assigned to Milestone
IUS Community Project
Fix Released
Undecided
Carl George

Bug Description

If you install/upgrade php54-fpm and Apache is not installed (which is usually the case if you are using Nginx), the php54-fpm prints this warning:

"warning: user apache does not exist - using root"

What I noticed is that this sets the owner of /var/log/php-fpm directory to owner root and 770 mode:

$ ls -ld /var/log/php-fpm/
drwxrwx---. 2 root root 4096 Apr 4 15:58 /var/log/php-fpm/

This is a problem because php-fpm cannot write anything into that directory because of this. And the biggest problem is that if you set permissions and owner to the user which runs php-fpm (which is nginx in my case) after the package is upgraded, it overwrites those permission again.

This same thing happens with php55u-fpm.

Revision history for this message
Carl George (carl.george) wrote :

I was able to reproduce the issue you saw. We are looking into an appropriate fix.

Revision history for this message
Carl George (carl.george) wrote :

From what I can tell, the logging is all done by the master fpm process, which runs as root. I agree that the directory should not be owned by the apache user, however I think it should be OK for the directory to be owned by root. As far as manual changes being overwritten, that is because the SPEC file currently specifies the directory owner.

https://github.com/iuscommunity-pkg/php54/blob/master/SPECS/php54.spec#L1396

%attr(770,apache,root) %dir %{_localstatedir}/log/php-fpm

Would it be acceptable to just remove the %attr macro? The directory would then default to (755,root,root), and future changes of ownership would not be overwritten.

Changed in ius:
status: New → In Progress
assignee: nobody → Carl George (carl.george)
Revision history for this message
kustodian (kustodian) wrote :

You are correct, all the logging is done by root, so it would be a lot better to remove the %attr macro and leave that directory to default, or whatever is later set by the system admin.

The problem is that in /etc/php-fpm.d/www.conf at the end of the file php error_log is set to that same directory:

php_admin_value[error_log] = /var/log/php-fpm/www-error.log

and this logging is done by the php-fpm worker processes, which are run by a none root user (this is set to apache in www.conf), which is probably the reason why the owner of /var/log/php-fpm/ is set to apache. So this is a problem.

Why is 'php_admin_value[error_log]' even set in www.conf by default? It would probably be easier to comment that and add a comment that the directory should belong to the process user.

Also why are any 'php_(admin_)value' even set, why not leave those to the admin to decide in either php.ini, or wherever. I'm especially talking about setting session handler and path:

; Set session path to a directory owned by process user
php_value[session.save_handler] = files
php_value[session.save_path] = /var/lib/php/session

Anyway, I would recommend to comment all 'php_(admin_)value' and even change the error_log to a different directory like /var/log/php/www-error.log, since those are not php-fpm errors, but actually php errors. I would set the file like this:

;php_admin_value[sendmail_path] = /usr/sbin/sendmail -t -i -f <email address hidden>
;php_flag[display_errors] = off
; Set error_log directory to a path owned by process user
;php_admin_value[error_log] = /var/log/php/www-error.log
;php_admin_flag[log_errors] = on
;php_admin_value[memory_limit] = 128M

; Set session path to a directory owned by process user
;php_value[session.save_handler] = files
;php_value[session.save_path] = /var/lib/php/session

Revision history for this message
kustodian (kustodian) wrote :

Carl do you have any updates about this issue?

Revision history for this message
Carl George (carl.george) wrote :

Sorry for the delay, I've been sidetracked with other work.

All the current values in the file are pulled directly from our upstream, Fedora [0]. There are some updates to this configuration file that we have not kept up to date with, specifically I can see that they added a '%pre fpm' section to add the apache user if necessary. That will prevent the situation you recognized where /var/log/php-fpm is owned by root.

As far as the php_admin_values, we again strive to be as close to Fedora as possible. The purpose of these values in a pool configuration file is to have values specific to just that pool. While researching these settings, and comparing to the upstream php source [1], the state of this in Fedora seems inconsistent to me. It is immediately obvious in the configuration file that you can customize the running user for a pool, and it mentions setting the same user for the session.save_path and soap.wsdl_cache_dir. It doesn't point out the ownership of the log dir, or the fact that if you change that ownership, the RPM will change it back on the next upgrade. At the very least I think this merits more explict comments, and possible changing some of the values.

I'm still reviewing the exact way to go about these changes. I hope to have some new RPMs in the testing repos soon that will address your concerns.

[0] http://pkgs.fedoraproject.org/cgit/php.git/tree/php-fpm-www.conf
[1] https://github.com/php/php-src/blob/master/sapi/fpm/php-fpm.conf.in

Revision history for this message
Carl George (carl.george) wrote :

We have corrected the issue where the apache user does not exist [0]. We are still evaluating the other changes.

[0] https://github.com/iuscommunity-pkg/php54/commit/471e861215193650fc595e453c106adbef13174e

Revision history for this message
kustodian (kustodian) wrote :

Creating the apache user if it doesn't exists is an ok solution.

There is still a problem with setting the permissions of the /var/log/php-fpm directory after each package upgrade which is done in the line you mentioned:

%attr(770,apache,root) %dir %{_localstatedir}/log/php-fpm

Since root is writing into that directory, it's not a problem to remove the %attr macro and just leave the default permissions (root:root 755). If someone wants to tighten the permissions, they can change them manually and they will stay like that. That is far better than to set the new permissions each time the package is upgraded.

Revision history for this message
kustodian (kustodian) wrote :

Hi Carl, every time I upgrade php-fpm I remember your name :)

Are there any news about this issue? I don't know what is a big problem about removing the part of setting permissions to 770 on each upgrade, since currently this doesn't allow the admins to set their own permissions for the log directory, for example to make it readable to the monitoring system.

Revision history for this message
Carl George (carl.george) wrote :

Sorry for the delay. We've had our hands full with the RHEL 7 and CentOS 7 releases.

The best solution I can come up with is to change the ownership to apache:apache, and then you can add users to the apache group to grant access. That will maintain the security of 770 permissions, without the headache of determining what user should own the directory, or changing the defaults Red Hat sets for the log directory. It will keep us very close to the Red Hat settings settings while still allowing us to accommodate your use case.

Does this sound like a reasonable solution to you?

Revision history for this message
kustodian (kustodian) wrote :

As I said before, I would rather like it that the package doesn't change directory permissions to package defaults after every update, but that looks like a limitation of the RPM format. Also if that is how Red Hat does it (even though I think it is not the right way), I can't questions you decision of following their rules.

Giving the log directory apache:apache ownership is a reasonable fix, especially for the PHP error_log settings, since it would be very hard to make that log work if php-fpm isn't run by the apache user.

Revision history for this message
Carl George (carl.george) wrote :

I have made the change for the group ownership of the log directory.

https://github.com/iuscommunity-pkg/php54/commit/b4d2123a1662d420537dda2d96a96aa3cf66edb1
https://github.com/iuscommunity-pkg/php55u/commit/f3a1c78afc972123a0498c201b5e7f820a4067dd

The new builds php54-5.4.31-2.ius and php55u-5.5.15-3.ius will move to the testing repos tonight. It may take up to 24 hours to sync to all mirrors.

Changed in ius:
status: In Progress → Fix Committed
Revision history for this message
Carl George (carl.george) wrote :

This fix will move to stable tonight with php54-5.4.32-1 and php55u-5.5.16-1.

Changed in ius:
status: Fix Committed → Fix Released
To post a comment you must log in.
This report contains Public information  
Everyone can see this information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.