Skip to content

Commit 2ecc7ca

Browse files
maaarghkmaaarghk
committed
fpm: Implement access log filtering
Adds a setting "access.suppress_path" to php-fpm pool configurations which causes successful GET requests to the specified URIs to be excluded from the access log. This is to reduce noise caused by automated health checks. Requests with response codes outwith the successful range 200 - 299, requests made with query parameters and requests which have a Content-Length other than 0 will ignore this setting as a security precaution. Closes GH-8174, #80428 [1] [1] https://bugs.php.net/bug.php?id=80428
1 parent 0ad8b64 commit 2ecc7ca

14 files changed

+636
-29
lines changed

‎sapi/fpm/fpm/fpm_conf.c

Lines changed: 33 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -557,11 +557,13 @@ static char *fpm_conf_set_array(zval *key, zval *value, void **config, int conve
557557
}
558558

559559
memset(kv, 0, sizeof(*kv));
560-
kv->key = strdup(Z_STRVAL_P(key));
560+
if (key) {
561+
kv->key = strdup(Z_STRVAL_P(key));
561562

562-
if (!kv->key) {
563-
free(kv);
564-
return "fpm_conf_set_array: strdup(key) failed";
563+
if (!kv->key) {
564+
free(kv);
565+
return "fpm_conf_set_array: strdup(key) failed";
566+
}
565567
}
566568

567569
if (convert_to_bool) {
@@ -663,6 +665,11 @@ int fpm_worker_pool_config_free(struct fpm_worker_pool_config_s *wpc) /* {{{ */
663665
free(wpc->apparmor_hat);
664666
#endif
665667

668+
for (kv = wpc->access_suppress_paths; kv; kv = kv_next) {
669+
kv_next = kv->next;
670+
free(kv->value);
671+
free(kv);
672+
}
666673
for (kv = wpc->php_values; kv; kv = kv_next) {
667674
kv_next = kv->next;
668675
free(kv->key);
@@ -1497,11 +1504,24 @@ static void fpm_conf_ini_parser_array(zval *name, zval *key, zval *value, void *
14971504
char *err = NULL;
14981505
void *config;
14991506

1500-
if (!*Z_STRVAL_P(key)) {
1501-
zlog(ZLOG_ERROR, "[%s:%d] Misspelled array ?", ini_filename, ini_lineno);
1507+
if (zend_string_equals_literal(Z_STR_P(name), "access.suppress_path")) {
1508+
if (!(*Z_STRVAL_P(key) == '\0')) {
1509+
zlog(ZLOG_ERROR, "[%s:%d] Keys provided to field 'access.suppress_path' are ignored", ini_filename, ini_lineno);
1510+
*error = 1;
1511+
}
1512+
if (!(*Z_STRVAL_P(value)) || (*Z_STRVAL_P(value) != '/')) {
1513+
zlog(ZLOG_ERROR, "[%s:%d] Values provided to field 'access.suppress_path' must begin with '/'", ini_filename, ini_lineno);
1514+
*error = 1;
1515+
}
1516+
if (*error) {
1517+
return;
1518+
}
1519+
} else if (!*Z_STRVAL_P(key)) {
1520+
zlog(ZLOG_ERROR, "[%s:%d] You must provide a key for field '%s'", ini_filename, ini_lineno, Z_STRVAL_P(name));
15021521
*error = 1;
15031522
return;
15041523
}
1524+
15051525
if (!current_wp || !current_wp->config) {
15061526
zlog(ZLOG_ERROR, "[%s:%d] Array are not allowed in the global section", ini_filename, ini_lineno);
15071527
*error = 1;
@@ -1533,6 +1553,10 @@ static void fpm_conf_ini_parser_array(zval *name, zval *key, zval *value, void *
15331553
config = (char *)current_wp->config + WPO(php_admin_values);
15341554
err = fpm_conf_set_array(key, value, &config, 1);
15351555

1556+
} else if (zend_string_equals_literal(Z_STR_P(name), "access.suppress_path")) {
1557+
config = (char *)current_wp->config + WPO(access_suppress_paths);
1558+
err = fpm_conf_set_array(NULL, value, &config, 0);
1559+
15361560
} else {
15371561
zlog(ZLOG_ERROR, "[%s:%d] unknown directive '%s'", ini_filename, ini_lineno, Z_STRVAL_P(name));
15381562
*error = 1;
@@ -1735,6 +1759,9 @@ static void fpm_conf_dump(void) /* {{{ */
17351759
zlog(ZLOG_NOTICE, "\tping.response = %s", STR2STR(wp->config->ping_response));
17361760
zlog(ZLOG_NOTICE, "\taccess.log = %s", STR2STR(wp->config->access_log));
17371761
zlog(ZLOG_NOTICE, "\taccess.format = %s", STR2STR(wp->config->access_format));
1762+
for (kv = wp->config->access_suppress_paths; kv; kv = kv->next) {
1763+
zlog(ZLOG_NOTICE, "\taccess.suppress_path[] = %s", kv->value);
1764+
}
17381765
zlog(ZLOG_NOTICE, "\tslowlog = %s", STR2STR(wp->config->slowlog));
17391766
zlog(ZLOG_NOTICE, "\trequest_slowlog_timeout = %ds", wp->config->request_slowlog_timeout);
17401767
zlog(ZLOG_NOTICE, "\trequest_slowlog_trace_depth = %d", wp->config->request_slowlog_trace_depth);

‎sapi/fpm/fpm/fpm_conf.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,7 @@ struct fpm_worker_pool_config_s {
7979
char *ping_response;
8080
char *access_log;
8181
char *access_format;
82+
struct key_value_s *access_suppress_paths;
8283
char *slowlog;
8384
int request_slowlog_timeout;
8485
int request_slowlog_trace_depth;

‎sapi/fpm/fpm/fpm_log.c

Lines changed: 53 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,9 @@
2727

2828
static char *fpm_log_format = NULL;
2929
static int fpm_log_fd = -1;
30+
static struct key_value_s *fpm_access_suppress_paths = NULL;
31+
32+
static int fpm_access_log_suppress(struct fpm_scoreboard_proc_s *proc);
3033

3134
int fpm_log_open(int reopen) /* {{{ */
3235
{
@@ -79,6 +82,17 @@ int fpm_log_init_child(struct fpm_worker_pool_s *wp) /* {{{ */
7982
}
8083
}
8184

85+
for (struct key_value_s *kv = wp->config->access_suppress_paths; kv; kv = kv->next) {
86+
struct key_value_s *kvcopy = calloc(1, sizeof(*kvcopy));
87+
if (kvcopy == NULL) {
88+
zlog(ZLOG_ERROR, "unable to allocate memory while opening the access log");
89+
return -1;
90+
}
91+
kvcopy->value = strdup(kv->value);
92+
kvcopy->next = fpm_access_suppress_paths;
93+
fpm_access_suppress_paths = kvcopy;
94+
}
95+
8296
if (fpm_log_fd == -1) {
8397
fpm_log_fd = wp->log_fd;
8498
}
@@ -136,6 +150,10 @@ int fpm_log_write(char *log_format) /* {{{ */
136150
}
137151
proc = *proc_p;
138152
fpm_scoreboard_proc_release(proc_p);
153+
154+
if (UNEXPECTED(fpm_access_log_suppress(&proc))) {
155+
return -1;
156+
}
139157
}
140158

141159
token = 0;
@@ -474,3 +492,38 @@ int fpm_log_write(char *log_format) /* {{{ */
474492
return 0;
475493
}
476494
/* }}} */
495+
496+
static int fpm_access_log_suppress(struct fpm_scoreboard_proc_s *proc)
497+
{
498+
// Never suppress when query string is passed
499+
if (proc->query_string[0] != '\0') {
500+
return 0;
501+
}
502+
503+
// Never suppress if request method is not GET or HEAD
504+
if (
505+
strcmp(proc->request_method, "GET") != 0
506+
&& strcmp(proc->request_method, "HEAD") != 0
507+
) {
508+
return 0;
509+
}
510+
511+
// Never suppress when response code does not indicate success
512+
if (SG(sapi_headers).http_response_code < 200 || SG(sapi_headers).http_response_code > 299) {
513+
return 0;
514+
}
515+
516+
// Never suppress when a body has been sent
517+
if (SG(request_info).content_length > 0) {
518+
return 0;
519+
}
520+
521+
// Suppress when request URI is an exact match for one of our entries
522+
for (struct key_value_s *kv = fpm_access_suppress_paths; kv; kv = kv->next) {
523+
if (kv->value && strcmp(kv->value, proc->request_uri) == 0) {
524+
return 1;
525+
}
526+
}
527+
528+
return 0;
529+
}

‎sapi/fpm/tests/config-array-validation-php-value-key.phpt

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
FPM: Validates arrays in configuration are correctly set - php_value array must be passed a key
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$cfg = <<<EOT
11+
[global]
12+
error_log = {{FILE:LOG}}
13+
pid = {{FILE:PID}}
14+
[unconfined]
15+
listen = {{ADDR}}
16+
php_value[] = E_ALL
17+
pm = static
18+
pm.max_children = 5
19+
EOT;
20+
21+
$tester = new FPM\Tester($cfg);
22+
$tester->start(['-tt']);
23+
$tester->expectLogError("\[%s:%d\] You must provide a key for field 'php_value'");
24+
25+
?>
26+
Done
27+
--EXPECT--
28+
29+
Done
30+
--CLEAN--
31+
<?php
32+
require_once "tester.inc";
33+
FPM\Tester::clean();
34+
?>

‎sapi/fpm/tests/config-array-validation-suppress-path-key-2.phpt

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
FPM: Validates arrays in configuration are correctly set - access.suppress_path doesn't accept key with forward slash
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$cfg = <<<EOT
11+
[global]
12+
error_log = {{FILE:LOG}}
13+
pid = {{FILE:PID}}
14+
[unconfined]
15+
listen = {{ADDR}}
16+
access.suppress_path[/] = test
17+
pm = static
18+
pm.max_children = 5
19+
EOT;
20+
21+
$tester = new FPM\Tester($cfg);
22+
$tester->start(['-tt']);
23+
$tester->expectLogError("\[%s:%d\] Keys provided to field 'access.suppress_path' are ignored");
24+
25+
?>
26+
Done
27+
--EXPECT--
28+
29+
Done
30+
--CLEAN--
31+
<?php
32+
require_once "tester.inc";
33+
FPM\Tester::clean();
34+
?>

‎sapi/fpm/tests/config-array-validation-suppress-path-key.phpt

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
FPM: Validates arrays in configuration are correctly set - access.suppress_path doesn't allow key
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$cfg = <<<EOT
11+
[global]
12+
error_log = {{FILE:LOG}}
13+
pid = {{FILE:PID}}
14+
[unconfined]
15+
listen = {{ADDR}}
16+
access.suppress_path[pingpath] = /ping
17+
pm = static
18+
pm.max_children = 5
19+
EOT;
20+
21+
$tester = new FPM\Tester($cfg);
22+
$tester->start(['-tt']);
23+
$tester->expectLogError("\[%s:%d\] Keys provided to field 'access.suppress_path' are ignored");
24+
25+
?>
26+
Done
27+
--EXPECT--
28+
29+
Done
30+
--CLEAN--
31+
<?php
32+
require_once "tester.inc";
33+
FPM\Tester::clean();
34+
?>

‎sapi/fpm/tests/config-array-validation-suppress-path-starts-slash.phpt

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
--TEST--
2+
FPM: Validates arrays in configuration are correctly set - access.suppress_path begins with forward slash
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$cfg = <<<EOT
11+
[global]
12+
error_log = {{FILE:LOG}}
13+
pid = {{FILE:PID}}
14+
[unconfined]
15+
listen = {{ADDR}}
16+
access.suppress_path[] = needs / to start with a slash
17+
pm = static
18+
pm.max_children = 5
19+
EOT;
20+
21+
$tester = new FPM\Tester($cfg);
22+
$tester->start(['-tt']);
23+
$tester->expectLogError("\[%s:%d\] Values provided to field 'access.suppress_path' must begin with '\/'");
24+
25+
?>
26+
Done
27+
--EXPECT--
28+
29+
Done
30+
--CLEAN--
31+
<?php
32+
require_once "tester.inc";
33+
FPM\Tester::clean();
34+
?>

‎sapi/fpm/tests/config-array.phpt

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
--TEST--
2+
FPM: Set arrays in configuration
3+
--SKIPIF--
4+
<?php include "skipif.inc"; ?>
5+
--FILE--
6+
<?php
7+
8+
require_once "tester.inc";
9+
10+
$cfg = <<<EOT
11+
[global]
12+
error_log = {{FILE:LOG}}
13+
pid = {{FILE:PID}}
14+
[unconfined]
15+
listen = {{ADDR}}
16+
access.suppress_path[] = /ping
17+
access.suppress_path[] = /health_check.php
18+
pm = static
19+
pm.max_children = 5
20+
php_value[error_reporting] = E_ALL
21+
php_value[date.timezone] = Europe/London
22+
php_admin_value[disable_functions] = eval
23+
php_flag[display_errors] = On
24+
php_admin_flag[log_errors] = 1
25+
EOT;
26+
27+
$tester = new FPM\Tester($cfg);
28+
$tester->start(['-tt']);
29+
$tester->expectLogConfigOptions([
30+
'access.suppress_path[] = /ping',
31+
'access.suppress_path[] = /health_check.php',
32+
'php_value[error_reporting] = 32767',
33+
'php_value[date.timezone] = Europe/London',
34+
'php_value[display_errors] = 1',
35+
'php_admin_value[disable_functions] = eval',
36+
'php_admin_value[log_errors] = 1',
37+
]);
38+
39+
40+
?>
41+
Done
42+
--EXPECT--
43+
44+
Done
45+
--CLEAN--
46+
<?php
47+
require_once "tester.inc";
48+
FPM\Tester::clean();
49+
?>

0 commit comments

Comments
 (0)