Skip to content

Commit 2c061d0

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 2c061d0

8 files changed

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

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

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

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

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

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

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

‎sapi/fpm/tests/tester.inc

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -367,7 +367,9 @@ class Tester
367367
$this->trace('Testing config using command', $cmd, true);
368368
exec($cmd, $output, $code);
369369
if ($code) {
370-
return preg_replace("/\[.+?\]/", "", $output[0]);
370+
foreach ($output as $outputLine) {
371+
$this->error(preg_replace("/\[.+?\]/", "", $outputLine, 1), null, false);
372+
}
371373
}
372374

373375
return null;
@@ -1231,14 +1233,15 @@ class Tester
12311233
/**
12321234
* Display error.
12331235
*
1234-
* @param string $msg
1235-
* @param \Exception|null $exception
1236+
* @param string $msg Error message.
1237+
* @param \Exception|null $exception If there is an exception, log its message
1238+
* @param bool $prefix Whether to prefix the error message
12361239
*
12371240
* @return false
12381241
*/
1239-
private function error($msg, \Exception $exception = null): bool
1242+
private function error(string $msg, \Exception $exception = null, bool $prefix = true): bool
12401243
{
1241-
$this->error = 'ERROR: ' . $msg;
1244+
$this->error = $prefix ? 'ERROR: ' . $msg : ltrim($msg);
12421245
if ($exception) {
12431246
$this->error .= '; EXCEPTION: ' . $exception->getMessage();
12441247
}

0 commit comments

Comments
 (0)