I'm not sure it is reasonable to expect from PHP to control system-level functions like ptrace. There are system-level means - like https://linux-audit.com/protect-ptrace-processes-kernel-yama-ptrace_scope/ - to control this. If you run a multi-user shared service with shell access that's what you probably want to do.

The kernel already doesn't allow this behavior. PHP-FPM is specifically asking for it to be allowed by calling prctl(PR_SET_DUMPABLE...) in fpm_unix.c.

Hmm in that case it indeed seems wrong. I am not sure why it is doing that, I would like to hear from the maintainer, but it looks like this code can be removed.

Any updates on this? The fix is rather trivial, so it's not clear what is holding up progress on this bug.

I think that the main issue is that the opcache doesn't consider the concept of separate pools and share resources across the pools which causes other issues like for example https://bugs.php.net/bug.php?id=74709 . However the solution of this problem might be quite complex and needs a bit more thinking. For example we could consider doing MINIT on the pool level but that might have some consequences or having new module entry callbacks for that which would require some ABI changes. It needs a bit more thinking and some discussion though.

If the opcache didn't have access to the shared memory of other pools, it wouldn't be an issue to have PR_SET_DUMPABLE set. It can be actually quite useful for tracing and some applications may rely on it already.

I think that the question here is if we consider pools as a secure environment that can be for example safely used for shared hosting. I don't think that it's the case until we resolve the separation issue above as there might be more such issues. Also I wouldn't like to break some apps relying on the current behavior. What we could do is to introduce a configuration setting that will however default to the current behaviour.

@Jakub I understand these complications, but still not sure - is there a legit reason why FPM does prctl(PR_SET_DUMPABLE) for child processes? I have hard time imagining any PHP application needing this low-level system flag or even knowing about it, but we could allow it for some special cases with FPM config. Do we need it in the default case though?

@stac That flag is there to make FPM tracing available which uses ptrace to get execution related data (see fpm_php_trace_dump) which is then logged to slowlog. The tracing is part of FPM and it's integrate into it so I think removing that could potentially break something...

If the trace is just a debug facility then I think we can safely put it under a flag, for those who need it, and set default to the secure setting which is not allowing everybody the access.

Well I would say it's probably more than just a special debugging facility. It's the whole slowlog functionality. Basically in FPM you can specify request_slowlog_timeout for a pool. If this is specified, the child run time is checked using heartbeat event and the time run is compared with the allowed timeout. If it exceeds the timeout specified in request_slowlog_timeout, the ptrace is attached which stops the child and log all the information to the slowlog file (another pool configuration). It means that this might be quite important functionality for some users. Also there is not much point to disable it for users that don't use different users for pools
 (meaning having at least 2 pools with different users) as there is no security risk IMO. If this should be default, then just for the configurations have such risk. I 
think something like this:

request_slowlog_trace_enabled = (auto|on|off)

The auto would be default and disable it for for the risky configurations where more than two different users are in pools.

What do you think?

I might be misunderstanding how the slowlog functionality works, but in the scenario where the FPM child process changes UID and GID (and thus loses it's DUMPABLE flag so that the PR_SET_DUMPABLE call has some effect), wont the process calling PTRACE_ATTACH (fpm_trace_signal) be running as root?

If that is the case, the kernel wont stop the parent from attaching to the child, regardless of the DUMPABLE flag on the child process. The parent process in this case will have the CAP_SYS_PTRACE capability since it's running as root, and calling PR_SET_DUMPABLE in the child is redundant.

Ah it looks you are right actually. Yes it will be root that attaches the ptrace. I thought that it applies on any user from reading the docs in http://man7.org/linux/man-pages/man2/prctl.2.html that states just:

  Processes that are not dumpable can not be attached via
  ptrace(2) PTRACE_ATTACH; see ptrace(2) for further details.

However it's extended in the ptrace docs (which I didn't read :) ):

4. Deny access if the target process "dumpable" attribute has a value
   other than 1 (SUID_DUMP_USER; see the discussion of
   PR_SET_DUMPABLE in prctl(2)), and the caller does not have the
   CAP_SYS_PTRACE capability in the user namespace of the target
   process.

It actually makes sense as root should be allowed to do ptrace in any case. I should have read it properly or at least try it :)

In that case I think it makes sense to disable it by default. Will prepare a patch and experiment with that once I get little bit of time.

I just attached initial patch for 5.6. I will need to test a bit more and also check if it applies to higher branches. Of course I will be more than happy if you can also test it and have some feedback! Thanks.

lgtm, I'd propose to merge it to the next release.

I tested on 5.6 and 7.0 only. Will try to test on 7.1+ over the weekend but it should be fine IMO.

Just did a quick test with 7.1 and 7.2 and seems good. I think it's good to go to to the next version. Cheers.

Open to discussion, how the "shared hosting" issue should be managed ?

https://wiki.php.net/security should list this explicitly, and IMHO, all "shared hosting" should be marked as "Low" (so going throw the next RC)

Patch applied.

Thanks.