@remicollet You recently committed d53fa7f, which looks related. What do you think about this PR?

FYI: I have been running php-fpm for more than a year with ProtectHome / ProtectSystem / PrivateDevices and never experienced any issues

I'm mostly fine with this change, excepted for ProtectSystem=full which also protect /etc, and can break packaged applications (downstream) which save their configuration there (and so may need write access)

Perhaps a good idea to add comment about each option, see recent addition in https://github.com/memcached/memcached/blob/master/scripts/memcached.service

@candrews please rebase it.

@remicollet Do you have an example of packages that would need write-access to /etc? I find that highly unlikely. Also; if there would be a distro that needs that, they can always modify / overload the default .service file :)

I've rebased and added comments describing the settings. I'm also doubtful that ProtectSystem=full would break anything; it doesn't seem like php should be writing to /etc. I agree with @SjonHortensius that if a distro/user wants to allow writing to /etc (or anything else restricted by these settings) they can override the settings - php should ship secure defaults.

Any updates?

I've rebased and added additional hardening options added in systemd since I initial created this pull request:
ProtectKernelModules=true
ProtectKernelTunables=true
ProtectControlGroups=true
RestrictRealtime=true
RestrictAddressFamilies=AF_INET AF_INET6 AF_UNIX
RestrictNamespaces=true

Since namespaces in particular have been the source of security issues recently, these additions should be good improvements to PHP's security.

I want to move on this for you, but @remicollet is our authority on this sort of thing, and is saying he doesn't like some of it ... I hear the arguments against it, but I don't hear his acceptance of them ... so I feel my hands are tied ...

Perhaps @remicollet could come back to us on this ?

Just as a side note ... you're not going to want to hear this ... "secure defaults" is not achievable in most realms of PHP: What we ship is working defaults. It has to be left to package maintainers like Remi, and whoever does windows packages to make sure that they are installed securely (or as securely as possible) ... still, I think in this case we can do something about making it more secure, I'm just offering an explanation as to why your aspiration to ship secure defaults is not met with the kind of applause you were probably hoping for.

@krakjoe : I definitely understand why changing the defaults is bad due to a lot of existing code, but having a blessed set of recommendations would be nice.

Remi is now one of the release managers for 7.2, he is also on holiday this week.

The decision for 7.2 will be left to him and @sgolemon. I'm not happy for it to be merged into 7.1 at this stage in release cycle whatever.

Can someone review this PR, please? cc @remicollet @bukka

I agree with @remicollet and would prefer setting ProtectSystem=true instead. If changed, I think it can be merged to master.

Actually thinking about it, I don't really think that there is any need for a write access to /etc as fpm doesn't change any configuration when running. It just needs a read access unless someone wants to store error log or uds file in it which seems quite strange.

So is this good to be merged into master, @bukka?

Ping?

I think it should be fine for master.

Thanks, @candrews! Applied as 40c4d7f.