From 06af583ae372ee14ab12bfc903e0e8d3a6a293fd Mon Sep 17 00:00:00 2001
From: Mikhail Galanin <mikhail.galanin@team.bumble.com>
Date: Fri, 14 Jul 2023 18:40:03 +0100
Subject: [PATCH] Set CLOEXEC on listened/accepted sockets in the FPM children

Co-authored-by: Jakub Zelenka <bukka@php.net>
---
 main/fastcgi.c                           | 10 +++
 sapi/fpm/fpm/fpm_children.c              | 21 ++++++-
 sapi/fpm/tests/socket-close-on-exec.phpt | 77 ++++++++++++++++++++++++
 sapi/fpm/tests/tester.inc                | 13 ++++
 4 files changed, 120 insertions(+), 1 deletion(-)
 create mode 100644 sapi/fpm/tests/socket-close-on-exec.phpt

diff --git a/main/fastcgi.c b/main/fastcgi.c
index a77491f1bf23d..9c66830137f10 100644
--- a/main/fastcgi.c
+++ b/main/fastcgi.c
@@ -1423,6 +1423,16 @@ int fcgi_accept_request(fcgi_request *req)
 					return -1;
 				}
 
+#if defined(F_SETFD) && defined(FD_CLOEXEC)
+				int fd_attrs = fcntl(req->fd, F_GETFD);
+				if (0 > fd_attrs) {
+					fcgi_log(FCGI_WARNING, "failed to get attributes of the connection socket");
+				}
+				if (0 > fcntl(req->fd, F_SETFD, fd_attrs | FD_CLOEXEC)) {
+					fcgi_log(FCGI_WARNING, "failed to change attribute of the connection socket");
+				}
+#endif
+
 #ifdef _WIN32
 				break;
 #else
diff --git a/sapi/fpm/fpm/fpm_children.c b/sapi/fpm/fpm/fpm_children.c
index 1c9780e3de3c1..285df91a9b690 100644
--- a/sapi/fpm/fpm/fpm_children.c
+++ b/sapi/fpm/fpm/fpm_children.c
@@ -167,6 +167,24 @@ struct fpm_child_s *fpm_child_find(pid_t pid) /* {{{ */
 }
 /* }}} */
 
+static int fpm_child_cloexec(void)
+{
+	/* get listening socket attributes so it can be extended */
+	int attrs = fcntl(fpm_globals.listening_socket, F_GETFD);
+	if (0 > attrs) {
+		zlog(ZLOG_WARNING, "failed to get attributes of listening socket, errno: %d", errno);
+		return -1;
+	}
+
+	/* set CLOEXEC to prevent the descriptor leaking to child processes */
+	if (0 > fcntl(fpm_globals.listening_socket, F_SETFD, attrs | FD_CLOEXEC)) {
+		zlog(ZLOG_WARNING, "failed to change attribute of listening socket");
+		return -1;
+	}
+
+	return 0;
+}
+
 static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */
 {
 	fpm_globals.max_requests = wp->config->pm_max_requests;
@@ -178,7 +196,8 @@ static void fpm_child_init(struct fpm_worker_pool_s *wp) /* {{{ */
 	    0 > fpm_unix_init_child(wp)   ||
 	    0 > fpm_signals_init_child()  ||
 	    0 > fpm_env_init_child(wp)    ||
-	    0 > fpm_php_init_child(wp)) {
+	    0 > fpm_php_init_child(wp)    ||
+	    0 > fpm_child_cloexec()) {
 
 		zlog(ZLOG_ERROR, "[pool %s] child failed to initialize", wp->config->name);
 		exit(FPM_EXIT_SOFTWARE);
diff --git a/sapi/fpm/tests/socket-close-on-exec.phpt b/sapi/fpm/tests/socket-close-on-exec.phpt
new file mode 100644
index 0000000000000..b879854059f21
--- /dev/null
+++ b/sapi/fpm/tests/socket-close-on-exec.phpt
@@ -0,0 +1,77 @@
+--TEST--
+FPM: Set CLOEXEC on the listen and connection socket
+--SKIPIF--
+<?php
+require_once "skipif.inc";
+FPM\Tester::skipIfShellCommandFails('lsof -v 2> /dev/null');
+?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<'EOT'
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR}}
+pm = static
+pm.max_children = 1
+pm.status_listen = {{ADDR[status]}}
+pm.status_path = /status
+env[PATH] = $PATH
+EOT;
+
+$code = <<<'EOT'
+<?php
+
+$fpmPort = $_GET['fpm_port'];
+
+echo "My sockets (expect to see 2 of them - one ESTABLISHED and one LISTEN):\n";
+
+$mypid = getmypid();
+$ph = popen("/bin/sh -c 'lsof -Pn -p$mypid' 2>&1 | grep TCP | grep '127.0.0.1:$fpmPort'", 'r');
+echo stream_get_contents($ph);
+pclose($ph);
+
+echo "\n";
+
+/*
+We expect that both LISTEN (inherited from the master process) and ACCEPTED (ESTABLISHED)
+have F_CLOEXEC and should not appear in the created process.
+
+We grep out sockets from non-FPM port, because they may appear in the environment,
+e.g. PHP-FPM can inherit them from the test-runner. We cannot control the runner,
+but we can test how the FPM behaves.
+*/
+
+echo "Sockets after exec(), expected to be empty:\n";
+$ph = popen("/bin/sh -c 'lsof -Pn -p\$\$' 2>&1 | grep TCP | grep '127.0.0.1:$fpmPort'", 'r');
+var_dump(stream_get_contents($ph));
+pclose($ph);
+
+EOT;
+
+$tester = new FPM\Tester($cfg, $code);
+$tester->start();
+$tester->expectLogStartNotices();
+$tester->request(query: 'fpm_port='.$tester->getPort())->printBody();
+$tester->terminate();
+$tester->expectLogTerminatingNotices();
+$tester->close();
+
+?>
+Done
+--EXPECTF--
+My sockets (expect to see 2 of them - one ESTABLISHED and one LISTEN):
+%S 127.0.0.1:%d->127.0.0.1:%d (ESTABLISHED)
+%S 127.0.0.1:%d (LISTEN)
+
+Sockets after exec(), expected to be empty:
+string(0) ""
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>
diff --git a/sapi/fpm/tests/tester.inc b/sapi/fpm/tests/tester.inc
index 0493adfd483ae..533bebea899a6 100644
--- a/sapi/fpm/tests/tester.inc
+++ b/sapi/fpm/tests/tester.inc
@@ -330,6 +330,19 @@ class Tester
         }
     }
 
+    /**
+     * Skip test if supplied shell command fails.
+     *
+     * @param string $command
+     */
+    static public function skipIfShellCommandFails($command)
+    {
+        $output = system($command, $code);
+        if ($code) {
+            die("skip command '$command' faield with code $code and output: $output");
+        }
+    }
+
     /**
      * Skip if posix extension not loaded.
      */