From d3cae36a21a806356c3cd86b50af09bdef580b09 Mon Sep 17 00:00:00 2001 From: Jakub Zelenka Date: Fri, 23 Dec 2022 19:24:57 +0000 Subject: [PATCH] Fix bug #68591: Configuration test does not perform UID lookups Addition check in fpm config test to verify existence of user, group, listen.owner and listen.group. --- sapi/fpm/fpm/fpm_conf.c | 7 ++ sapi/fpm/fpm/fpm_unix.c | 75 +++++++++++++++---- sapi/fpm/fpm/fpm_unix.h | 2 + sapi/fpm/tests/bug68591-conf-test-group.phpt | 38 ++++++++++ .../bug68591-conf-test-listen-group.phpt | 38 ++++++++++ .../bug68591-conf-test-listen-owner.phpt | 38 ++++++++++ sapi/fpm/tests/bug68591-conf-test-user.phpt | 38 ++++++++++ sapi/fpm/tests/tester.inc | 29 ++++--- 8 files changed, 240 insertions(+), 25 deletions(-) create mode 100644 sapi/fpm/tests/bug68591-conf-test-group.phpt create mode 100644 sapi/fpm/tests/bug68591-conf-test-listen-group.phpt create mode 100644 sapi/fpm/tests/bug68591-conf-test-listen-owner.phpt create mode 100644 sapi/fpm/tests/bug68591-conf-test-user.phpt diff --git a/sapi/fpm/fpm/fpm_conf.c b/sapi/fpm/fpm/fpm_conf.c index cc7102eabbc0..5be11fa9e414 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" @@ -1818,6 +1819,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 ed5b66294fb0..1df71178c26b 100644 --- a/sapi/fpm/fpm/fpm_unix.c +++ b/sapi/fpm/fpm/fpm_unix.c @@ -34,6 +34,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; @@ -93,11 +142,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; @@ -126,11 +174,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; @@ -163,14 +210,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; } @@ -180,14 +226,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; @@ -267,14 +312,13 @@ 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); } 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; } @@ -287,14 +331,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 0bb22687b02f..6fc9e5e8450d 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 000000000000..14d639080116 --- /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 000000000000..a98f32aa40a6 --- /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 000000000000..e64911702675 --- /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 000000000000..5627151f3f13 --- /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 c1f090ab145e..7b82431d838c 100644 --- a/sapi/fpm/tests/tester.inc +++ b/sapi/fpm/tests/tester.inc @@ -259,10 +259,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"); } } @@ -357,17 +358,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() . ' -t -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; @@ -1231,14 +1241,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(); }