Skip to content

Commit 327bb21

Browse files
maaarghkbukka
maaarghk
authored and
bukka
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 5ad056c commit 327bb21

15 files changed

+638
-29
lines changed

‎NEWS

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ PHP NEWS
1212

1313
- FPM:
1414
. Added listen.setfib pool option to set route FIB on FreeBSD. (David Carlier)
15+
. Added access.suppress_path pool option to filter access log entries.
16+
(Mark Gallagher)
1517

1618
- Standard:
1719
. Fixed empty array returned by str_split on empty input. (Michael Vorisek)

‎sapi/fpm/fpm/fpm_conf.c

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

562562
memset(kv, 0, sizeof(*kv));
563-
kv->key = strdup(Z_STRVAL_P(key));
563+
if (key) {
564+
kv->key = strdup(Z_STRVAL_P(key));
564565

565-
if (!kv->key) {
566-
free(kv);
567-
return "fpm_conf_set_array: strdup(key) failed";
566+
if (!kv->key) {
567+
free(kv);
568+
return "fpm_conf_set_array: strdup(key) failed";
569+
}
568570
}
569571

570572
if (convert_to_bool) {
@@ -669,6 +671,11 @@ int fpm_worker_pool_config_free(struct fpm_worker_pool_config_s *wpc) /* {{{ */
669671
free(wpc->apparmor_hat);
670672
#endif
671673

674+
for (kv = wpc->access_suppress_paths; kv; kv = kv_next) {
675+
kv_next = kv->next;
676+
free(kv->value);
677+
free(kv);
678+
}
672679
for (kv = wpc->php_values; kv; kv = kv_next) {
673680
kv_next = kv->next;
674681
free(kv->key);
@@ -1503,11 +1510,24 @@ static void fpm_conf_ini_parser_array(zval *name, zval *key, zval *value, void *
15031510
char *err = NULL;
15041511
void *config;
15051512

1506-
if (!*Z_STRVAL_P(key)) {
1507-
zlog(ZLOG_ERROR, "[%s:%d] Misspelled array ?", ini_filename, ini_lineno);
1513+
if (zend_string_equals_literal(Z_STR_P(name), "access.suppress_path")) {
1514+
if (!(*Z_STRVAL_P(key) == '\0')) {
1515+
zlog(ZLOG_ERROR, "[%s:%d] Keys provided to field 'access.suppress_path' are ignored", ini_filename, ini_lineno);
1516+
*error = 1;
1517+
}
1518+
if (!(*Z_STRVAL_P(value)) || (*Z_STRVAL_P(value) != '/')) {
1519+
zlog(ZLOG_ERROR, "[%s:%d] Values provided to field 'access.suppress_path' must begin with '/'", ini_filename, ini_lineno);
1520+
*error = 1;
1521+
}
1522+
if (*error) {
1523+
return;
1524+
}
1525+
} else if (!*Z_STRVAL_P(key)) {
1526+
zlog(ZLOG_ERROR, "[%s:%d] You must provide a key for field '%s'", ini_filename, ini_lineno, Z_STRVAL_P(name));
15081527
*error = 1;
15091528
return;
15101529
}
1530+
15111531
if (!current_wp || !current_wp->config) {
15121532
zlog(ZLOG_ERROR, "[%s:%d] Array are not allowed in the global section", ini_filename, ini_lineno);
15131533
*error = 1;
@@ -1539,6 +1559,10 @@ static void fpm_conf_ini_parser_array(zval *name, zval *key, zval *value, void *
15391559
config = (char *)current_wp->config + WPO(php_admin_values);
15401560
err = fpm_conf_set_array(key, value, &config, 1);
15411561

1562+
} else if (zend_string_equals_literal(Z_STR_P(name), "access.suppress_path")) {
1563+
config = (char *)current_wp->config + WPO(access_suppress_paths);
1564+
err = fpm_conf_set_array(NULL, value, &config, 0);
1565+
15421566
} else {
15431567
zlog(ZLOG_ERROR, "[%s:%d] unknown directive '%s'", ini_filename, ini_lineno, Z_STRVAL_P(name));
15441568
*error = 1;
@@ -1747,6 +1771,9 @@ static void fpm_conf_dump(void) /* {{{ */
17471771
zlog(ZLOG_NOTICE, "\tping.response = %s", STR2STR(wp->config->ping_response));
17481772
zlog(ZLOG_NOTICE, "\taccess.log = %s", STR2STR(wp->config->access_log));
17491773
zlog(ZLOG_NOTICE, "\taccess.format = %s", STR2STR(wp->config->access_format));
1774+
for (kv = wp->config->access_suppress_paths; kv; kv = kv->next) {
1775+
zlog(ZLOG_NOTICE, "\taccess.suppress_path[] = %s", kv->value);
1776+
}
17501777
zlog(ZLOG_NOTICE, "\tslowlog = %s", STR2STR(wp->config->slowlog));
17511778
zlog(ZLOG_NOTICE, "\trequest_slowlog_timeout = %ds", wp->config->request_slowlog_timeout);
17521779
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)