Fix bug #68591: Configuration test does not perform UID lookups

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();
 		}

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;

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);

sapi/fpm/tests/bug68591-conf-test-group.phpt

@@ -0,0 +1,38 @@
+--TEST--
+FPM: bug68591 - config test group existence
+--SKIPIF--
+<?php
+include "skipif.inc";
+?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR:UDS}}
+group = aaaaaa
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 2
+pm.min_spare_servers = 1
+pm.max_spare_servers = 3
+EOT;
+
+$tester = new FPM\Tester($cfg);
+$tester->testConfig();
+
+?>
+Done
+--EXPECT--
+ERROR: [pool unconfined] cannot get gid for group 'aaaaaa'
+ERROR: FPM initialization failed
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>

sapi/fpm/tests/bug68591-conf-test-listen-group.phpt

@@ -0,0 +1,38 @@
+--TEST--
+FPM: bug68591 - config test listen group existence
+--SKIPIF--
+<?php
+include "skipif.inc";
+?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR:UDS}}
+listen.group = aaaaaa
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 2
+pm.min_spare_servers = 1
+pm.max_spare_servers = 3
+EOT;
+
+$tester = new FPM\Tester($cfg);
+$tester->testConfig();
+
+?>
+Done
+--EXPECTF--
+ERROR: [pool unconfined] cannot get gid for group 'aaaaaa': %s
+ERROR: FPM initialization failed
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>

sapi/fpm/tests/bug68591-conf-test-listen-owner.phpt

@@ -0,0 +1,38 @@
+--TEST--
+FPM: bug68591 - config test listen owner existence
+--SKIPIF--
+<?php
+include "skipif.inc";
+?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR:UDS}}
+listen.owner = aaaaaa
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 2
+pm.min_spare_servers = 1
+pm.max_spare_servers = 3
+EOT;
+
+$tester = new FPM\Tester($cfg);
+$tester->testConfig();
+
+?>
+Done
+--EXPECTF--
+ERROR: [pool unconfined] cannot get uid for user 'aaaaaa': %s
+ERROR: FPM initialization failed
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>

sapi/fpm/tests/bug68591-conf-test-user.phpt

@@ -0,0 +1,38 @@
+--TEST--
+FPM: bug68591 - config test user existence
+--SKIPIF--
+<?php
+include "skipif.inc";
+?>
+--FILE--
+<?php
+
+require_once "tester.inc";
+
+$cfg = <<<EOT
+[global]
+error_log = {{FILE:LOG}}
+[unconfined]
+listen = {{ADDR:UDS}}
+user = aaaaaa
+pm = dynamic
+pm.max_children = 5
+pm.start_servers = 2
+pm.min_spare_servers = 1
+pm.max_spare_servers = 3
+EOT;
+
+$tester = new FPM\Tester($cfg);
+$tester->testConfig();
+
+?>
+Done
+--EXPECT--
+ERROR: [pool unconfined] cannot get uid for user 'aaaaaa'
+ERROR: FPM initialization failed
+Done
+--CLEAN--
+<?php
+require_once "tester.inc";
+FPM\Tester::clean();
+?>

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();
         }