diff --git a/NEWS b/NEWS index 26ce34b3cea..2c52262b7ff 100644 --- a/NEWS +++ b/NEWS @@ -6,6 +6,8 @@ PHP NEWS . Fixed bug #77106 (Missing separator in FPM FastCGI errors). (Jakub Zelenka) . Fixed bug GH-9981 (FPM does not reset fastcgi.error_header). (Jakub Zelenka) + . Fixed bug #68591 (Configuration test does not perform UID lookups). + (Jakub Zelenka) - LDAP: . Fixed bug GH-10112 (LDAP\Connection::__construct() refers to ldap_create()). diff --git a/sapi/fpm/fpm/fpm_conf.c b/sapi/fpm/fpm/fpm_conf.c index f2d9d3c088a..ef9b35a9994 100644 --- a/sapi/fpm/fpm/fpm_conf.c +++ b/sapi/fpm/fpm/fpm_conf.c @@ -34,6 +34,7 @@ #include "fpm_status.h" #include "fpm_log.h" #include "fpm_events.h" +#include "fpm_unix.h" #include "zlog.h" #ifdef HAVE_SYSTEMD #include "fpm_systemd.h" @@ -1854,6 +1855,12 @@ int fpm_conf_init_main(int test_conf, int force_daemon) /* {{{ */ } if (test_conf) { + for (struct fpm_worker_pool_s *wp = fpm_worker_all_pools; wp; wp = wp->next) { + if (!fpm_unix_test_config(wp)) { + return -1; + } + } + if (test_conf > 1) { fpm_conf_dump(); } diff --git a/sapi/fpm/fpm/fpm_unix.c b/sapi/fpm/fpm/fpm_unix.c index b1127b35949..d10a6f3254b 100644 --- a/sapi/fpm/fpm/fpm_unix.c +++ b/sapi/fpm/fpm/fpm_unix.c @@ -46,6 +46,55 @@ size_t fpm_pagesize; + +static inline bool fpm_unix_is_id(const char* name) +{ + return strlen(name) == strspn(name, "0123456789"); +} + +static struct passwd *fpm_unix_get_passwd(struct fpm_worker_pool_s *wp, const char *name, int flags) +{ + struct passwd *pwd = getpwnam(name); + if (!pwd) { + zlog(flags, "[pool %s] cannot get uid for user '%s'", wp->config->name, name); + return NULL; + } + + return pwd; +} + +static inline bool fpm_unix_check_passwd(struct fpm_worker_pool_s *wp, const char *name, int flags) +{ + return !name || fpm_unix_is_id(name) || fpm_unix_get_passwd(wp, name, flags); +} + +static struct group *fpm_unix_get_group(struct fpm_worker_pool_s *wp, const char *name, int flags) +{ + struct group *group = getgrnam(name); + if (!group) { + zlog(flags, "[pool %s] cannot get gid for group '%s'", wp->config->name, name); + return NULL; + } + + return group; +} + +static inline bool fpm_unix_check_group(struct fpm_worker_pool_s *wp, const char *name, int flags) +{ + return !name || fpm_unix_is_id(name) || fpm_unix_get_group(wp, name, flags); +} + +bool fpm_unix_test_config(struct fpm_worker_pool_s *wp) +{ + struct fpm_worker_pool_config_s *config = wp->config; + return ( + fpm_unix_check_passwd(wp, config->user, ZLOG_ERROR) && + fpm_unix_check_group(wp, config->group, ZLOG_ERROR) && + fpm_unix_check_passwd(wp, config->listen_owner, ZLOG_SYSERROR) && + fpm_unix_check_group(wp, config->listen_group, ZLOG_SYSERROR) + ); +} + int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp) /* {{{ */ { struct fpm_worker_pool_config_s *c = wp->config; @@ -105,11 +154,10 @@ int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp) /* {{{ */ if ((end = strchr(p, ','))) { *end++ = 0; } - pwd = getpwnam(p); + pwd = fpm_unix_get_passwd(wp, p, ZLOG_SYSERROR); if (pwd) { zlog(ZLOG_DEBUG, "[pool %s] user '%s' have uid=%d", wp->config->name, p, pwd->pw_uid); } else { - zlog(ZLOG_SYSERROR, "[pool %s] cannot get uid for user '%s'", wp->config->name, p); acl_free(acl); efree(tmp); return -1; @@ -138,11 +186,10 @@ int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp) /* {{{ */ if ((end = strchr(p, ','))) { *end++ = 0; } - grp = getgrnam(p); + grp = fpm_unix_get_group(wp, p, ZLOG_SYSERROR); if (grp) { zlog(ZLOG_DEBUG, "[pool %s] group '%s' have gid=%d", wp->config->name, p, grp->gr_gid); } else { - zlog(ZLOG_SYSERROR, "[pool %s] cannot get gid for group '%s'", wp->config->name, p); acl_free(acl); efree(tmp); return -1; @@ -175,14 +222,13 @@ int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp) /* {{{ */ #endif if (c->listen_owner && *c->listen_owner) { - if (strlen(c->listen_owner) == strspn(c->listen_owner, "0123456789")) { + if (fpm_unix_is_id(c->listen_owner)) { wp->socket_uid = strtoul(c->listen_owner, 0, 10); } else { struct passwd *pwd; - pwd = getpwnam(c->listen_owner); + pwd = fpm_unix_get_passwd(wp, c->listen_owner, ZLOG_SYSERROR); if (!pwd) { - zlog(ZLOG_SYSERROR, "[pool %s] cannot get uid for user '%s'", wp->config->name, c->listen_owner); return -1; } @@ -192,14 +238,13 @@ int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp) /* {{{ */ } if (c->listen_group && *c->listen_group) { - if (strlen(c->listen_group) == strspn(c->listen_group, "0123456789")) { + if (fpm_unix_is_id(c->listen_group)) { wp->socket_gid = strtoul(c->listen_group, 0, 10); } else { struct group *grp; - grp = getgrnam(c->listen_group); + grp = fpm_unix_get_group(wp, c->listen_group, ZLOG_SYSERROR); if (!grp) { - zlog(ZLOG_SYSERROR, "[pool %s] cannot get gid for group '%s'", wp->config->name, c->listen_group); return -1; } wp->socket_gid = grp->gr_gid; @@ -279,7 +324,7 @@ static int fpm_unix_conf_wp(struct fpm_worker_pool_s *wp) /* {{{ */ if (is_root) { if (wp->config->user && *wp->config->user) { - if (strlen(wp->config->user) == strspn(wp->config->user, "0123456789")) { + if (fpm_unix_is_id(wp->config->user)) { wp->set_uid = strtoul(wp->config->user, 0, 10); pwd = getpwuid(wp->set_uid); if (pwd) { @@ -289,9 +334,8 @@ static int fpm_unix_conf_wp(struct fpm_worker_pool_s *wp) /* {{{ */ } else { struct passwd *pwd; - pwd = getpwnam(wp->config->user); + pwd = fpm_unix_get_passwd(wp, wp->config->user, ZLOG_ERROR); if (!pwd) { - zlog(ZLOG_ERROR, "[pool %s] cannot get uid for user '%s'", wp->config->name, wp->config->user); return -1; } @@ -304,14 +348,13 @@ static int fpm_unix_conf_wp(struct fpm_worker_pool_s *wp) /* {{{ */ } if (wp->config->group && *wp->config->group) { - if (strlen(wp->config->group) == strspn(wp->config->group, "0123456789")) { + if (fpm_unix_is_id(wp->config->group)) { wp->set_gid = strtoul(wp->config->group, 0, 10); } else { struct group *grp; - grp = getgrnam(wp->config->group); + grp = fpm_unix_get_group(wp, wp->config->group, ZLOG_ERROR); if (!grp) { - zlog(ZLOG_ERROR, "[pool %s] cannot get gid for group '%s'", wp->config->name, wp->config->group); return -1; } wp->set_gid = grp->gr_gid; diff --git a/sapi/fpm/fpm/fpm_unix.h b/sapi/fpm/fpm/fpm_unix.h index 0bb22687b02..6fc9e5e8450 100644 --- a/sapi/fpm/fpm/fpm_unix.h +++ b/sapi/fpm/fpm/fpm_unix.h @@ -5,6 +5,8 @@ #include "fpm_worker_pool.h" +bool fpm_unix_test_config(struct fpm_worker_pool_s *wp); + int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp); int fpm_unix_set_socket_permissions(struct fpm_worker_pool_s *wp, const char *path); int fpm_unix_free_socket_permissions(struct fpm_worker_pool_s *wp); diff --git a/sapi/fpm/tests/bug68591-conf-test-group.phpt b/sapi/fpm/tests/bug68591-conf-test-group.phpt new file mode 100644 index 00000000000..14d63908011 --- /dev/null +++ b/sapi/fpm/tests/bug68591-conf-test-group.phpt @@ -0,0 +1,38 @@ +--TEST-- +FPM: bug68591 - config test group existence +--SKIPIF-- + +--FILE-- +testConfig(); + +?> +Done +--EXPECT-- +ERROR: [pool unconfined] cannot get gid for group 'aaaaaa' +ERROR: FPM initialization failed +Done +--CLEAN-- + diff --git a/sapi/fpm/tests/bug68591-conf-test-listen-group.phpt b/sapi/fpm/tests/bug68591-conf-test-listen-group.phpt new file mode 100644 index 00000000000..a98f32aa40a --- /dev/null +++ b/sapi/fpm/tests/bug68591-conf-test-listen-group.phpt @@ -0,0 +1,38 @@ +--TEST-- +FPM: bug68591 - config test listen group existence +--SKIPIF-- + +--FILE-- +testConfig(); + +?> +Done +--EXPECTF-- +ERROR: [pool unconfined] cannot get gid for group 'aaaaaa': %s +ERROR: FPM initialization failed +Done +--CLEAN-- + diff --git a/sapi/fpm/tests/bug68591-conf-test-listen-owner.phpt b/sapi/fpm/tests/bug68591-conf-test-listen-owner.phpt new file mode 100644 index 00000000000..e6491170267 --- /dev/null +++ b/sapi/fpm/tests/bug68591-conf-test-listen-owner.phpt @@ -0,0 +1,38 @@ +--TEST-- +FPM: bug68591 - config test listen owner existence +--SKIPIF-- + +--FILE-- +testConfig(); + +?> +Done +--EXPECTF-- +ERROR: [pool unconfined] cannot get uid for user 'aaaaaa': %s +ERROR: FPM initialization failed +Done +--CLEAN-- + diff --git a/sapi/fpm/tests/bug68591-conf-test-user.phpt b/sapi/fpm/tests/bug68591-conf-test-user.phpt new file mode 100644 index 00000000000..5627151f3f1 --- /dev/null +++ b/sapi/fpm/tests/bug68591-conf-test-user.phpt @@ -0,0 +1,38 @@ +--TEST-- +FPM: bug68591 - config test user existence +--SKIPIF-- + +--FILE-- +testConfig(); + +?> +Done +--EXPECT-- +ERROR: [pool unconfined] cannot get uid for user 'aaaaaa' +ERROR: FPM initialization failed +Done +--CLEAN-- + diff --git a/sapi/fpm/tests/tester.inc b/sapi/fpm/tests/tester.inc index 50de960aa8d..db8f0c04f4a 100644 --- a/sapi/fpm/tests/tester.inc +++ b/sapi/fpm/tests/tester.inc @@ -269,10 +269,11 @@ class Tester static public function skipIfConfigFails(string $configTemplate) { $tester = new self($configTemplate, '', [], self::getCallerFileName()); - $testResult = $tester->testConfig(); + $testResult = $tester->testConfig(true); if ($testResult !== null) { self::clean(2); - die("skip $testResult"); + $message = $testResult[0] ?? 'Config failed'; + die("skip $message"); } } @@ -378,17 +379,26 @@ class Tester /** * Test configuration file. * - * @return null|string + * @return null|array * @throws \Exception */ - public function testConfig() + public function testConfig($silent = false) { $configFile = $this->createConfig(); $cmd = self::findExecutable() . ' -tt -y ' . $configFile . ' 2>&1'; $this->trace('Testing config using command', $cmd, true); exec($cmd, $output, $code); if ($code) { - return preg_replace("/\[.+?\]/", "", $output[0]); + $messages = []; + foreach ($output as $outputLine) { + $message = preg_replace("/\[.+?\]/", "", $outputLine, 1); + $messages[] = $message; + if ( ! $silent) { + $this->error($message, null, false); + } + } + + return $messages; } return null; @@ -1259,14 +1269,15 @@ class Tester /** * Display error. * - * @param string $msg - * @param \Exception|null $exception + * @param string $msg Error message. + * @param \Exception|null $exception If there is an exception, log its message + * @param bool $prefix Whether to prefix the error message * * @return false */ - private function error($msg, \Exception $exception = null): bool + private function error(string $msg, \Exception $exception = null, bool $prefix = true): bool { - $this->error = 'ERROR: ' . $msg; + $this->error = $prefix ? 'ERROR: ' . $msg : ltrim($msg); if ($exception) { $this->error .= '; EXCEPTION: ' . $exception->getMessage(); }