Skip to content

Commit b2cddc6

Browse files
bukkabukka
committed
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.
1 parent 5f1311a commit b2cddc6

8 files changed

+240
-25
lines changed

‎sapi/fpm/fpm/fpm_conf.c

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@
3434
#include "fpm_status.h"
3535
#include "fpm_log.h"
3636
#include "fpm_events.h"
37+
#include "fpm_unix.h"
3738
#include "zlog.h"
3839
#ifdef HAVE_SYSTEMD
3940
#include "fpm_systemd.h"
@@ -1818,6 +1819,12 @@ int fpm_conf_init_main(int test_conf, int force_daemon) /* {{{ */
18181819
}
18191820

18201821
if (test_conf) {
1822+
for (struct fpm_worker_pool_s *wp = fpm_worker_all_pools; wp; wp = wp->next) {
1823+
if (!fpm_unix_test_config(wp)) {
1824+
return -1;
1825+
}
1826+
}
1827+
18211828
if (test_conf > 1) {
18221829
fpm_conf_dump();
18231830
}

‎sapi/fpm/fpm/fpm_unix.c

Lines changed: 59 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,55 @@
3434

3535
size_t fpm_pagesize;
3636

37+
38+
static inline bool fpm_unix_is_id(const char* name)
39+
{
40+
return strlen(name) == strspn(name, "0123456789");
41+
}
42+
43+
static struct passwd *fpm_unix_get_passwd(struct fpm_worker_pool_s *wp, const char *name, int flags)
44+
{
45+
struct passwd *pwd = getpwnam(name);
46+
if (!pwd) {
47+
zlog(flags, "[pool %s] cannot get uid for user '%s'", wp->config->name, name);
48+
return NULL;
49+
}
50+
51+
return pwd;
52+
}
53+
54+
static inline bool fpm_unix_check_passwd(struct fpm_worker_pool_s *wp, const char *name, int flags)
55+
{
56+
return !name || fpm_unix_is_id(name) || fpm_unix_get_passwd(wp, name, flags);
57+
}
58+
59+
static struct group *fpm_unix_get_group(struct fpm_worker_pool_s *wp, const char *name, int flags)
60+
{
61+
struct group *group = getgrnam(name);
62+
if (!group) {
63+
zlog(flags, "[pool %s] cannot get gid for group '%s'", wp->config->name, name);
64+
return NULL;
65+
}
66+
67+
return group;
68+
}
69+
70+
static inline bool fpm_unix_check_group(struct fpm_worker_pool_s *wp, const char *name, int flags)
71+
{
72+
return !name || fpm_unix_is_id(name) || fpm_unix_get_group(wp, name, flags);
73+
}
74+
75+
bool fpm_unix_test_config(struct fpm_worker_pool_s *wp)
76+
{
77+
struct fpm_worker_pool_config_s *config = wp->config;
78+
return (
79+
fpm_unix_check_passwd(wp, config->user, ZLOG_ERROR) &&
80+
fpm_unix_check_group(wp, config->group, ZLOG_ERROR) &&
81+
fpm_unix_check_passwd(wp, config->listen_owner, ZLOG_SYSERROR) &&
82+
fpm_unix_check_group(wp, config->listen_group, ZLOG_SYSERROR)
83+
);
84+
}
85+
3786
int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp) /* {{{ */
3887
{
3988
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) /* {{{ */
93142
if ((end = strchr(p, ','))) {
94143
*end++ = 0;
95144
}
96-
pwd = getpwnam(p);
145+
pwd = fpm_unix_get_passwd(wp, p, ZLOG_SYSERROR);
97146
if (pwd) {
98147
zlog(ZLOG_DEBUG, "[pool %s] user '%s' have uid=%d", wp->config->name, p, pwd->pw_uid);
99148
} else {
100-
zlog(ZLOG_SYSERROR, "[pool %s] cannot get uid for user '%s'", wp->config->name, p);
101149
acl_free(acl);
102150
efree(tmp);
103151
return -1;
@@ -126,11 +174,10 @@ int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp) /* {{{ */
126174
if ((end = strchr(p, ','))) {
127175
*end++ = 0;
128176
}
129-
grp = getgrnam(p);
177+
grp = fpm_unix_get_group(wp, p, ZLOG_SYSERROR);
130178
if (grp) {
131179
zlog(ZLOG_DEBUG, "[pool %s] group '%s' have gid=%d", wp->config->name, p, grp->gr_gid);
132180
} else {
133-
zlog(ZLOG_SYSERROR, "[pool %s] cannot get gid for group '%s'", wp->config->name, p);
134181
acl_free(acl);
135182
efree(tmp);
136183
return -1;
@@ -163,14 +210,13 @@ int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp) /* {{{ */
163210
#endif
164211

165212
if (c->listen_owner && *c->listen_owner) {
166-
if (strlen(c->listen_owner) == strspn(c->listen_owner, "0123456789")) {
213+
if (fpm_unix_is_id(c->listen_owner)) {
167214
wp->socket_uid = strtoul(c->listen_owner, 0, 10);
168215
} else {
169216
struct passwd *pwd;
170217

171-
pwd = getpwnam(c->listen_owner);
218+
pwd = fpm_unix_get_passwd(wp, c->listen_owner, ZLOG_SYSERROR);
172219
if (!pwd) {
173-
zlog(ZLOG_SYSERROR, "[pool %s] cannot get uid for user '%s'", wp->config->name, c->listen_owner);
174220
return -1;
175221
}
176222

@@ -180,14 +226,13 @@ int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp) /* {{{ */
180226
}
181227

182228
if (c->listen_group && *c->listen_group) {
183-
if (strlen(c->listen_group) == strspn(c->listen_group, "0123456789")) {
229+
if (fpm_unix_is_id(c->listen_group)) {
184230
wp->socket_gid = strtoul(c->listen_group, 0, 10);
185231
} else {
186232
struct group *grp;
187233

188-
grp = getgrnam(c->listen_group);
234+
grp = fpm_unix_get_group(wp, c->listen_group, ZLOG_SYSERROR);
189235
if (!grp) {
190-
zlog(ZLOG_SYSERROR, "[pool %s] cannot get gid for group '%s'", wp->config->name, c->listen_group);
191236
return -1;
192237
}
193238
wp->socket_gid = grp->gr_gid;
@@ -267,14 +312,13 @@ static int fpm_unix_conf_wp(struct fpm_worker_pool_s *wp) /* {{{ */
267312

268313
if (is_root) {
269314
if (wp->config->user && *wp->config->user) {
270-
if (strlen(wp->config->user) == strspn(wp->config->user, "0123456789")) {
315+
if (fpm_unix_is_id(wp->config->user)) {
271316
wp->set_uid = strtoul(wp->config->user, 0, 10);
272317
} else {
273318
struct passwd *pwd;
274319

275-
pwd = getpwnam(wp->config->user);
320+
pwd = fpm_unix_get_passwd(wp, wp->config->user, ZLOG_ERROR);
276321
if (!pwd) {
277-
zlog(ZLOG_ERROR, "[pool %s] cannot get uid for user '%s'", wp->config->name, wp->config->user);
278322
return -1;
279323
}
280324

@@ -287,14 +331,13 @@ static int fpm_unix_conf_wp(struct fpm_worker_pool_s *wp) /* {{{ */
287331
}
288332

289333
if (wp->config->group && *wp->config->group) {
290-
if (strlen(wp->config->group) == strspn(wp->config->group, "0123456789")) {
334+
if (fpm_unix_is_id(wp->config->group)) {
291335
wp->set_gid = strtoul(wp->config->group, 0, 10);
292336
} else {
293337
struct group *grp;
294338

295-
grp = getgrnam(wp->config->group);
339+
grp = fpm_unix_get_group(wp, wp->config->group, ZLOG_ERROR);
296340
if (!grp) {
297-
zlog(ZLOG_ERROR, "[pool %s] cannot get gid for group '%s'", wp->config->name, wp->config->group);
298341
return -1;
299342
}
300343
wp->set_gid = grp->gr_gid;

‎sapi/fpm/fpm/fpm_unix.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,8 @@
55

66
#include "fpm_worker_pool.h"
77

8+
bool fpm_unix_test_config(struct fpm_worker_pool_s *wp);
9+
810
int fpm_unix_resolve_socket_permissions(struct fpm_worker_pool_s *wp);
911
int fpm_unix_set_socket_permissions(struct fpm_worker_pool_s *wp, const char *path);
1012
int fpm_unix_free_socket_permissions(struct fpm_worker_pool_s *wp);

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
FPM: bug68591 - config test group existence
3+
--SKIPIF--
4+
<?php
5+
include "skipif.inc";
6+
?>
7+
--FILE--
8+
<?php
9+
10+
require_once "tester.inc";
11+
12+
$cfg = <<<EOT
13+
[global]
14+
error_log = {{FILE:LOG}}
15+
[unconfined]
16+
listen = {{ADDR:UDS}}
17+
group = aaaaaa
18+
pm = dynamic
19+
pm.max_children = 5
20+
pm.start_servers = 2
21+
pm.min_spare_servers = 1
22+
pm.max_spare_servers = 3
23+
EOT;
24+
25+
$tester = new FPM\Tester($cfg);
26+
$tester->testConfig();
27+
28+
?>
29+
Done
30+
--EXPECT--
31+
ERROR: [pool unconfined] cannot get gid for group 'aaaaaa'
32+
ERROR: FPM initialization failed
33+
Done
34+
--CLEAN--
35+
<?php
36+
require_once "tester.inc";
37+
FPM\Tester::clean();
38+
?>

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
FPM: bug68591 - config test listen group existence
3+
--SKIPIF--
4+
<?php
5+
include "skipif.inc";
6+
?>
7+
--FILE--
8+
<?php
9+
10+
require_once "tester.inc";
11+
12+
$cfg = <<<EOT
13+
[global]
14+
error_log = {{FILE:LOG}}
15+
[unconfined]
16+
listen = {{ADDR:UDS}}
17+
listen.group = aaaaaa
18+
pm = dynamic
19+
pm.max_children = 5
20+
pm.start_servers = 2
21+
pm.min_spare_servers = 1
22+
pm.max_spare_servers = 3
23+
EOT;
24+
25+
$tester = new FPM\Tester($cfg);
26+
$tester->testConfig();
27+
28+
?>
29+
Done
30+
--EXPECT--
31+
ERROR: [pool unconfined] cannot get gid for group 'aaaaaa': Success (0)
32+
ERROR: FPM initialization failed
33+
Done
34+
--CLEAN--
35+
<?php
36+
require_once "tester.inc";
37+
FPM\Tester::clean();
38+
?>

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
FPM: bug68591 - config test listen owner existence
3+
--SKIPIF--
4+
<?php
5+
include "skipif.inc";
6+
?>
7+
--FILE--
8+
<?php
9+
10+
require_once "tester.inc";
11+
12+
$cfg = <<<EOT
13+
[global]
14+
error_log = {{FILE:LOG}}
15+
[unconfined]
16+
listen = {{ADDR:UDS}}
17+
listen.owner = aaaaaa
18+
pm = dynamic
19+
pm.max_children = 5
20+
pm.start_servers = 2
21+
pm.min_spare_servers = 1
22+
pm.max_spare_servers = 3
23+
EOT;
24+
25+
$tester = new FPM\Tester($cfg);
26+
$tester->testConfig();
27+
28+
?>
29+
Done
30+
--EXPECT--
31+
ERROR: [pool unconfined] cannot get uid for user 'aaaaaa': Success (0)
32+
ERROR: FPM initialization failed
33+
Done
34+
--CLEAN--
35+
<?php
36+
require_once "tester.inc";
37+
FPM\Tester::clean();
38+
?>

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

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
--TEST--
2+
FPM: bug68591 - config test user existence
3+
--SKIPIF--
4+
<?php
5+
include "skipif.inc";
6+
?>
7+
--FILE--
8+
<?php
9+
10+
require_once "tester.inc";
11+
12+
$cfg = <<<EOT
13+
[global]
14+
error_log = {{FILE:LOG}}
15+
[unconfined]
16+
listen = {{ADDR:UDS}}
17+
user = aaaaaa
18+
pm = dynamic
19+
pm.max_children = 5
20+
pm.start_servers = 2
21+
pm.min_spare_servers = 1
22+
pm.max_spare_servers = 3
23+
EOT;
24+
25+
$tester = new FPM\Tester($cfg);
26+
$tester->testConfig();
27+
28+
?>
29+
Done
30+
--EXPECT--
31+
ERROR: [pool unconfined] cannot get uid for user 'aaaaaa'
32+
ERROR: FPM initialization failed
33+
Done
34+
--CLEAN--
35+
<?php
36+
require_once "tester.inc";
37+
FPM\Tester::clean();
38+
?>

‎sapi/fpm/tests/tester.inc

Lines changed: 20 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -259,10 +259,11 @@ class Tester
259259
static public function skipIfConfigFails(string $configTemplate)
260260
{
261261
$tester = new self($configTemplate, '', [], self::getCallerFileName());
262-
$testResult = $tester->testConfig();
262+
$testResult = $tester->testConfig(true);
263263
if ($testResult !== null) {
264264
self::clean(2);
265-
die("skip $testResult");
265+
$message = $testResult[0] ?? 'Config failed';
266+
die("skip $message");
266267
}
267268
}
268269

@@ -357,17 +358,26 @@ class Tester
357358
/**
358359
* Test configuration file.
359360
*
360-
* @return null|string
361+
* @return null|array
361362
* @throws \Exception
362363
*/
363-
public function testConfig()
364+
public function testConfig($silent = false)
364365
{
365366
$configFile = $this->createConfig();
366367
$cmd = self::findExecutable() . ' -t -y ' . $configFile . ' 2>&1';
367368
$this->trace('Testing config using command', $cmd, true);
368369
exec($cmd, $output, $code);
369370
if ($code) {
370-
return preg_replace("/\[.+?\]/", "", $output[0]);
371+
$messages = [];
372+
foreach ($output as $outputLine) {
373+
$message = preg_replace("/\[.+?\]/", "", $outputLine, 1);
374+
$messages[] = $message;
375+
if ( ! $silent) {
376+
$this->error($message, null, false);
377+
}
378+
}
379+
380+
return $messages;
371381
}
372382

373383
return null;
@@ -1231,14 +1241,15 @@ class Tester
12311241
/**
12321242
* Display error.
12331243
*
1234-
* @param string $msg
1235-
* @param \Exception|null $exception
1244+
* @param string $msg Error message.
1245+
* @param \Exception|null $exception If there is an exception, log its message
1246+
* @param bool $prefix Whether to prefix the error message
12361247
*
12371248
* @return false
12381249
*/
1239-
private function error($msg, \Exception $exception = null): bool
1250+
private function error(string $msg, \Exception $exception = null, bool $prefix = true): bool
12401251
{
1241-
$this->error = 'ERROR: ' . $msg;
1252+
$this->error = $prefix ? 'ERROR: ' . $msg : ltrim($msg);
12421253
if ($exception) {
12431254
$this->error .= '; EXCEPTION: ' . $exception->getMessage();
12441255
}

0 commit comments

Comments
 (0)