From 97eab11dc51a9338f01a855b5a5dff0180d8c443 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Sun, 3 May 2026 21:49:58 +0200 Subject: [PATCH 01/22] Add remaining phase4 parser/logging/regression test cases --- src/ngx_http_modsecurity_body_filter.c | 113 +++++++++++++- src/ngx_http_modsecurity_common.h | 13 ++ src/ngx_http_modsecurity_module.c | 177 ++++++++++++++++++++++ tests/modsecurity-h2.t | 1 + tests/modsecurity-phase4-content-types.t | 89 +++++++++++ tests/modsecurity-phase4-invalid-config.t | 32 ++++ tests/modsecurity-phase4-modes.t | 75 +++++++++ tests/modsecurity-phase4-regression.t | 46 ++++++ tests/modsecurity-proxy-h2.t | 1 + tests/modsecurity.t | 1 + 10 files changed, 546 insertions(+), 2 deletions(-) create mode 100644 tests/modsecurity-phase4-content-types.t create mode 100644 tests/modsecurity-phase4-invalid-config.t create mode 100644 tests/modsecurity-phase4-modes.t create mode 100644 tests/modsecurity-phase4-regression.t diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 0c28e3c1..1d5f5cf2 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -23,6 +23,10 @@ #include "ngx_http_modsecurity_common.h" static ngx_http_output_body_filter_pt ngx_http_next_body_filter; +static ngx_int_t ngx_http_modsecurity_phase4_in_scope(ngx_http_request_t *r); +static ngx_int_t ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurity_conf_t *mcf, const char *wanted, const char *actual, const char *reason); +static void ngx_http_modsecurity_json_escape(ngx_pool_t *pool, ngx_str_t *src, ngx_str_t *dst); +static void ngx_http_modsecurity_extract_rule_id(ngx_pool_t *pool, ngx_str_t *intervention, ngx_str_t *rule_id); /* XXX: check behaviour on few body filters installed */ ngx_int_t @@ -141,6 +145,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) #endif int is_request_processed = 0; + ngx_http_modsecurity_conf_t *mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); for (; chain != NULL; chain = chain->next) { u_char *data = chain->buf->pos; @@ -151,6 +156,26 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) if (ret > 0) { return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity_module, ret); + } else if (ret < 0) { + if (ctx->phase4_headers_checked) { + return ngx_http_next_body_filter(r, in); + } + ctx->phase4_headers_checked = 1; + if (ngx_http_modsecurity_phase4_in_scope(r) == 0 || mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_MINIMAL) { + ngx_http_modsecurity_phase4_log_event(r, mcf, "deny_status", "log_only", ngx_http_modsecurity_phase4_in_scope(r) ? "mode_minimal" : "content_type_not_in_scope"); + ngx_log_error(NGX_LOG_WARN, r->connection->log, 0, + "modsecurity phase4 intervention after headers sent, action=log_only, uri=\"%V\"", &r->uri); + } else if (mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_STRICT) { + ngx_http_modsecurity_phase4_log_event(r, mcf, "deny_status", "connection_abort", "headers_already_sent"); + ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, + "modsecurity phase4 intervention after headers sent, action=connection_abort, uri=\"%V\"", &r->uri); + r->connection->error = 1; + return NGX_ERROR; + } else { + ngx_http_modsecurity_phase4_log_event(r, mcf, "deny_status", "log_only", "mode_safe"); + ngx_log_error(NGX_LOG_WARN, r->connection->log, 0, + "modsecurity phase4 intervention after headers sent, action=log_only, uri=\"%V\"", &r->uri); + } } /* XXX: chain->buf->last_buf || chain->buf->last_in_chain */ @@ -170,8 +195,11 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) return ret; } else if (ret < 0) { - return ngx_http_filter_finalize_request(r, - &ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR); + if (ngx_http_modsecurity_phase4_in_scope(r) == 0 || mcf->phase4_mode != NGX_HTTP_MODSEC_PHASE4_MODE_STRICT) { + return ngx_http_next_body_filter(r, in); + } + r->connection->error = 1; + return NGX_ERROR; } } @@ -184,3 +212,84 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) /* XXX: xflt_filter() -- return NGX_OK here */ return ngx_http_next_body_filter(r, in); } + +static ngx_int_t +ngx_http_modsecurity_phase4_in_scope(ngx_http_request_t *r) +{ + ngx_http_modsecurity_conf_t *mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); + ngx_uint_t i; + ngx_str_t ct; + u_char *semi; + if (r->headers_out.content_type.len == 0 || mcf->phase4_content_types == NULL) return 0; + ct = r->headers_out.content_type; + semi = (u_char *)ngx_strlchr(ct.data, ct.data + ct.len, ';'); + if (semi != NULL) ct.len = semi - ct.data; + while (ct.len > 0 && ngx_isspace(ct.data[ct.len - 1])) ct.len--; + for (i = 0; i < mcf->phase4_content_types->nelts; i++) { + ngx_str_t *arr = mcf->phase4_content_types->elts; + if (arr[i].len == ct.len && ngx_strncasecmp(arr[i].data, ct.data, ct.len) == 0) return 1; + } + return 0; +} + +static ngx_int_t +ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurity_conf_t *mcf, const char *wanted, const char *actual, const char *reason) +{ + u_char buf[2048]; + u_char *p; + ngx_str_t euri, emethod, ect, elog, erule; + const char *mode = "safe"; + const char *header_sent = r->header_sent ? "true" : "false"; + ngx_http_modsecurity_ctx_t *ctx = ngx_http_modsecurity_get_module_ctx(r); + if (mcf->phase4_log_file == NULL || mcf->phase4_log_file->fd == NGX_INVALID_FILE) return NGX_OK; + ngx_http_modsecurity_json_escape(r->pool, &r->uri, &euri); + ngx_http_modsecurity_json_escape(r->pool, &r->method_name, &emethod); + ngx_http_modsecurity_json_escape(r->pool, &r->headers_out.content_type, &ect); + if (ctx) ngx_http_modsecurity_json_escape(r->pool, &ctx->last_intervention_log, &elog); else { elog.len=0; elog.data=(u_char*)""; } + if (mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_MINIMAL) mode = "minimal"; + else if (mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_STRICT) mode = "strict"; + ngx_http_modsecurity_extract_rule_id(r->pool, &elog, &erule); + p = ngx_snprintf(buf, sizeof(buf), + "{\"event\":\"phase4_intervention\",\"uri\":\"%V\",\"method\":\"%V\",\"status\":%ui,\"content_type\":\"%V\",\"header_sent\":%s,\"mode\":\"%s\",\"wanted_action\":\"%s\",\"actual_action\":\"%s\",\"reason\":\"%s\",\"intervention\":\"%V\",\"rule_id\":\"%V\"}\n", + &euri,&emethod,(ngx_uint_t)r->headers_out.status,&ect,header_sent,mode,wanted,actual,reason,&elog,&erule); + ngx_write_fd(mcf->phase4_log_file->fd, buf, p - buf); + return NGX_OK; +} + +static void +ngx_http_modsecurity_json_escape(ngx_pool_t *pool, ngx_str_t *src, ngx_str_t *dst) +{ + size_t i, extra = 0; u_char *d; + if (src == NULL || src->data == NULL) { dst->len=0; dst->data=(u_char*)""; return; } + for (i = 0; i < src->len; i++) if (src->data[i] < 0x20 || src->data[i] == '"' || src->data[i] == '\\') extra++; + dst->data = ngx_pnalloc(pool, src->len + extra + 1); if (dst->data == NULL) { dst->len=0; return; } + d = dst->data; + for (i = 0; i < src->len; i++) { + u_char c = src->data[i]; + if (c == '"' || c == '\\') { *d++='\\'; *d++=c; } + else if (c < 0x20) { *d++=' '; } + else *d++=c; + } + dst->len = d - dst->data; +} + +static void +ngx_http_modsecurity_extract_rule_id(ngx_pool_t *pool, ngx_str_t *intervention, ngx_str_t *rule_id) +{ + size_t i; + rule_id->data = (u_char *)""; + rule_id->len = 0; + if (intervention == NULL || intervention->data == NULL) return; + for (i = 0; i + 4 < intervention->len; i++) { + if (ngx_strncasecmp(intervention->data + i, (u_char *)"id \"", 4) == 0) { + size_t j = i + 4; + while (j < intervention->len && intervention->data[j] >= '0' && intervention->data[j] <= '9') j++; + if (j > i + 4 && j < intervention->len && intervention->data[j] == '"') { + rule_id->len = j - (i + 4); + rule_id->data = ngx_pnalloc(pool, rule_id->len); + if (rule_id->data != NULL) ngx_memcpy(rule_id->data, intervention->data + i + 4, rule_id->len); + return; + } + } + } +} diff --git a/src/ngx_http_modsecurity_common.h b/src/ngx_http_modsecurity_common.h index 9f79b5ba..b9a11f39 100644 --- a/src/ngx_http_modsecurity_common.h +++ b/src/ngx_http_modsecurity_common.h @@ -71,6 +71,10 @@ #define MODSECURITY_NGINX_WHOAMI "ModSecurity-nginx v" \ MODSECURITY_NGINX_VERSION +#define NGX_HTTP_MODSEC_PHASE4_MODE_MINIMAL 0 +#define NGX_HTTP_MODSEC_PHASE4_MODE_SAFE 1 +#define NGX_HTTP_MODSEC_PHASE4_MODE_STRICT 2 + typedef struct { ngx_str_t name; ngx_str_t value; @@ -100,6 +104,10 @@ typedef struct { unsigned logged:1; unsigned intervention_triggered:1; unsigned request_body_processed:1; + unsigned phase4_headers_checked:1; + ngx_str_t phase4_content_type; + ngx_str_t last_intervention_log; + ngx_int_t last_intervention_status; } ngx_http_modsecurity_ctx_t; @@ -124,6 +132,11 @@ typedef struct { #endif ngx_http_complex_value_t *transaction_id; + ngx_uint_t phase4_mode; + ngx_array_t *phase4_content_types; + ngx_str_t phase4_content_types_file; + ngx_open_file_t *phase4_log_file; + ngx_str_t phase4_log_path; } ngx_http_modsecurity_conf_t; diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index d3d9624d..3c6dc123 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -36,6 +36,16 @@ static void *ngx_http_modsecurity_create_conf(ngx_conf_t *cf); static char *ngx_http_modsecurity_merge_conf(ngx_conf_t *cf, void *parent, void *child); static void ngx_http_modsecurity_cleanup_instance(void *data); static void ngx_http_modsecurity_cleanup_rules(void *data); +static char *ngx_conf_set_phase4_mode(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); +static char *ngx_conf_set_phase4_content_types_file(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); +static char *ngx_conf_set_phase4_log(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); +static char *ngx_http_modsecurity_phase4_set_default_content_types(ngx_conf_t *cf, ngx_http_modsecurity_conf_t *mcf); +static char *ngx_http_modsecurity_phase4_load_content_types_file(ngx_conf_t *cf, ngx_http_modsecurity_conf_t *mcf, ngx_str_t *path); +static ngx_int_t ngx_http_modsecurity_phase4_validate_content_type(u_char *s, size_t len); + +#define NGX_HTTP_MODSEC_PHASE4_MODE_MINIMAL 0 +#define NGX_HTTP_MODSEC_PHASE4_MODE_SAFE 1 +#define NGX_HTTP_MODSEC_PHASE4_MODE_STRICT 2 /* @@ -160,6 +170,18 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re dd("nothing to do"); return 0; } + ctx->last_intervention_status = intervention.status; + ctx->last_intervention_log.len = 0; + ctx->last_intervention_log.data = NULL; + if (intervention.log != NULL) { + size_t l = ngx_strlen(intervention.log); + u_char *cp = ngx_pnalloc(r->pool, l); + if (cp != NULL) { + ngx_memcpy(cp, intervention.log, l); + ctx->last_intervention_log.data = cp; + ctx->last_intervention_log.len = l; + } + } mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); if (mcf == NULL) { @@ -479,6 +501,124 @@ char *ngx_conf_set_transaction_id(ngx_conf_t *cf, ngx_command_t *cmd, void *conf return NGX_CONF_OK; } +static char * +ngx_conf_set_phase4_mode(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) +{ + ngx_http_modsecurity_conf_t *mcf = conf; + ngx_str_t *value = cf->args->elts; + if (ngx_strcmp(value[1].data, "minimal") == 0) mcf->phase4_mode = NGX_HTTP_MODSEC_PHASE4_MODE_MINIMAL; + else if (ngx_strcmp(value[1].data, "safe") == 0) mcf->phase4_mode = NGX_HTTP_MODSEC_PHASE4_MODE_SAFE; + else if (ngx_strcmp(value[1].data, "strict") == 0) mcf->phase4_mode = NGX_HTTP_MODSEC_PHASE4_MODE_STRICT; + else return "invalid value for modsecurity_phase4_mode (expected minimal|safe|strict)"; + return NGX_CONF_OK; +} + +static char * +ngx_conf_set_phase4_content_types_file(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) +{ + ngx_http_modsecurity_conf_t *mcf = conf; + ngx_str_t *value = cf->args->elts; + mcf->phase4_content_types_file = value[1]; + return ngx_http_modsecurity_phase4_load_content_types_file(cf, mcf, &value[1]); +} + +static char * +ngx_conf_set_phase4_log(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) +{ + ngx_http_modsecurity_conf_t *mcf = conf; + ngx_str_t *value = cf->args->elts; + mcf->phase4_log_path = value[1]; + mcf->phase4_log_file = ngx_conf_open_file(cf->cycle, &mcf->phase4_log_path); + if (mcf->phase4_log_file == NULL) { + return NGX_CONF_ERROR; + } + return NGX_CONF_OK; +} + +static ngx_int_t +ngx_http_modsecurity_phase4_validate_content_type(u_char *s, size_t len) +{ + size_t i; + if (len == 0 || ngx_strchr(s, '*') != NULL) return NGX_ERROR; + for (i = 0; i < len; i++) { + u_char c = s[i]; + if (!(ngx_isalnum(c) || c == '/' || c == '.' || c == '+' || c == '-' )) return NGX_ERROR; + } + return (ngx_strchr(s, '/') != NULL) ? NGX_OK : NGX_ERROR; +} + +static char * +ngx_http_modsecurity_phase4_set_default_content_types(ngx_conf_t *cf, ngx_http_modsecurity_conf_t *mcf) +{ + static const char *defs[] = {"text/html","text/plain","application/json","application/xml","text/xml","application/xhtml+xml"}; + ngx_uint_t i; + if (mcf->phase4_content_types != NULL) return NGX_CONF_OK; + mcf->phase4_content_types = ngx_array_create(cf->pool, 6, sizeof(ngx_str_t)); + if (mcf->phase4_content_types == NULL) return NGX_CONF_ERROR; + for (i = 0; i < 6; i++) { + ngx_str_t *ct = ngx_array_push(mcf->phase4_content_types); + if (ct == NULL) return NGX_CONF_ERROR; + ct->len = ngx_strlen(defs[i]); + ct->data = ngx_pnalloc(cf->pool, ct->len); + if (ct->data == NULL) return NGX_CONF_ERROR; + ngx_memcpy(ct->data, defs[i], ct->len); + } + return NGX_CONF_OK; +} + +static char * +ngx_http_modsecurity_phase4_load_content_types_file(ngx_conf_t *cf, ngx_http_modsecurity_conf_t *mcf, ngx_str_t *path) +{ + ngx_file_t file; + ngx_file_info_t fi; + u_char *buf, *p, *line, *end; + ssize_t n; + if (ngx_file_info((char *)path->data, &fi) == NGX_FILE_ERROR) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, ngx_errno, "modsecurity_phase4_content_types_file \"%V\" stat() failed", path); + return NGX_CONF_ERROR; + } + buf = ngx_pnalloc(cf->pool, ngx_file_size(&fi) + 1); + if (buf == NULL) return NGX_CONF_ERROR; + ngx_memzero(&file, sizeof(file)); + file.name = *path; file.log = cf->log; + file.fd = ngx_open_file(path->data, NGX_FILE_RDONLY, NGX_FILE_OPEN, 0); + if (file.fd == NGX_INVALID_FILE) return NGX_CONF_ERROR; + n = ngx_read_file(&file, buf, ngx_file_size(&fi), 0); + ngx_close_file(file.fd); + if (n < 0) return NGX_CONF_ERROR; + buf[n] = '\0'; + mcf->phase4_content_types = ngx_array_create(cf->pool, 8, sizeof(ngx_str_t)); + if (mcf->phase4_content_types == NULL) return NGX_CONF_ERROR; + for (p = buf, line = buf; p <= buf + n; p++) { + if (p == buf + n || *p == '\n' || *p == '\r') { + *p = '\0'; end = p; + while (line < end && ngx_isspace(*line)) line++; + while (end > line && ngx_isspace(*(end-1))) *(--end) = '\0'; + if (line[0] && line[0] != '#') { + u_char *hash = (u_char *)ngx_strchr(line, '#'); + u_char *semi = (u_char *)ngx_strchr(line, ';'); + if (hash && (!semi || hash < semi)) *hash = '\0'; + if (semi) *semi = '\0'; + end = line + ngx_strlen(line); + while (end > line && ngx_isspace(*(end-1))) *(--end) = '\0'; + ngx_strlow(line, line, end - line); + if (ngx_http_modsecurity_phase4_validate_content_type(line, end-line) != NGX_OK) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid content-type entry in modsecurity_phase4_content_types_file \"%V\": \"%s\"", path, line); + return NGX_CONF_ERROR; + } + ngx_str_t *ct = ngx_array_push(mcf->phase4_content_types); + if (ct == NULL) return NGX_CONF_ERROR; + ct->len = end - line; + ct->data = ngx_pnalloc(cf->pool, ct->len); + if (ct->data == NULL) return NGX_CONF_ERROR; + ngx_memcpy(ct->data, line, ct->len); + } + line = p + 1; + } + } + return NGX_CONF_OK; +} + static ngx_command_t ngx_http_modsecurity_commands[] = { { @@ -521,6 +661,30 @@ static ngx_command_t ngx_http_modsecurity_commands[] = { 0, NULL }, + { + ngx_string("modsecurity_phase4_mode"), + NGX_HTTP_LOC_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_MAIN_CONF|NGX_CONF_TAKE1, + ngx_conf_set_phase4_mode, + NGX_HTTP_LOC_CONF_OFFSET, + 0, + NULL + }, + { + ngx_string("modsecurity_phase4_content_types_file"), + NGX_HTTP_LOC_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_MAIN_CONF|NGX_CONF_TAKE1, + ngx_conf_set_phase4_content_types_file, + NGX_HTTP_LOC_CONF_OFFSET, + 0, + NULL + }, + { + ngx_string("modsecurity_phase4_log"), + NGX_HTTP_LOC_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_MAIN_CONF|NGX_CONF_TAKE1, + ngx_conf_set_phase4_log, + NGX_HTTP_LOC_CONF_OFFSET, + 0, + NULL + }, { ngx_string("modsecurity_use_error_log"), NGX_HTTP_LOC_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_MAIN_CONF|NGX_CONF_FLAG, @@ -724,6 +888,13 @@ ngx_http_modsecurity_create_conf(ngx_conf_t *cf) conf->pool = cf->pool; conf->transaction_id = NGX_CONF_UNSET_PTR; conf->use_error_log = NGX_CONF_UNSET; + conf->phase4_mode = NGX_CONF_UNSET_UINT; + conf->phase4_content_types = NULL; + conf->phase4_content_types_file.len = 0; + conf->phase4_content_types_file.data = NULL; + conf->phase4_log_file = NULL; + conf->phase4_log_path.len = 0; + conf->phase4_log_path.data = NULL; #if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) conf->sanity_checks_enabled = NGX_CONF_UNSET; #endif @@ -764,6 +935,9 @@ ngx_http_modsecurity_merge_conf(ngx_conf_t *cf, void *parent, void *child) ngx_conf_merge_value(c->enable, p->enable, 0); ngx_conf_merge_ptr_value(c->transaction_id, p->transaction_id, NULL); ngx_conf_merge_value(c->use_error_log, p->use_error_log, 1); + ngx_conf_merge_uint_value(c->phase4_mode, p->phase4_mode, NGX_HTTP_MODSEC_PHASE4_MODE_SAFE); + ngx_conf_merge_ptr_value(c->phase4_log_file, p->phase4_log_file, NULL); + ngx_conf_merge_ptr_value(c->phase4_content_types, p->phase4_content_types, NULL); #if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) ngx_conf_merge_value(c->sanity_checks_enabled, p->sanity_checks_enabled, 0); #endif @@ -784,6 +958,9 @@ ngx_http_modsecurity_merge_conf(ngx_conf_t *cf, void *parent, void *child) dd("NEW CHILD RULES"); msc_rules_dump(c->rules_set); #endif + if (c->phase4_content_types == NULL) { + return ngx_http_modsecurity_phase4_set_default_content_types(cf, c); + } return NGX_CONF_OK; } diff --git a/tests/modsecurity-h2.t b/tests/modsecurity-h2.t index 981d103d..ce3853bd 100644 --- a/tests/modsecurity-h2.t +++ b/tests/modsecurity-h2.t @@ -83,6 +83,7 @@ http { } location /phase4 { modsecurity on; + modsecurity_phase4_mode strict; modsecurity_rules ' SecRuleEngine On SecResponseBodyAccess On diff --git a/tests/modsecurity-phase4-content-types.t b/tests/modsecurity-phase4-content-types.t new file mode 100644 index 00000000..d59e4553 --- /dev/null +++ b/tests/modsecurity-phase4-content-types.t @@ -0,0 +1,89 @@ +#!/usr/bin/perl +use warnings; use strict; +use Test::More; +BEGIN { use FindBin; chdir($FindBin::Bin); } +use lib 'lib'; +use Test::Nginx; + +# content-type parsing and scope behavior +my $t = Test::Nginx->new()->has(qw/http/); + +$t->write_file('phase4-content-types.conf', <<'CT'); +# comments and whitespace + +Application/JSON; charset=utf-8 # inline comment +TEXT/HTML +text/plain +application/xml +CT + +$t->write_file_expand('nginx.conf', <<'EOC'); +%%TEST_GLOBALS%% +daemon off; +events {} +http { + %%TEST_GLOBALS_HTTP%% + server { + listen 127.0.0.1:8080; + server_name localhost; + + location /json { + modsecurity on; + modsecurity_phase4_mode strict; + modsecurity_phase4_log %%TESTDIR%%/phase4-content-types.log; + modsecurity_phase4_content_types_file %%TESTDIR%%/phase4-content-types.conf; + default_type application/json; + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx HIT" "id:920001,phase:4,deny,log,status:403,msg:\"ct\"" + '; + } + + location /unknown { + modsecurity on; + modsecurity_phase4_mode strict; + modsecurity_phase4_log %%TESTDIR%%/phase4-content-types.log; + modsecurity_phase4_content_types_file %%TESTDIR%%/phase4-content-types.conf; + default_type image/png; + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx HIT" "id:920002,phase:4,deny,log,status:403,msg:\"ct\"" + '; + } + + location /emptytype { + modsecurity on; + modsecurity_phase4_mode strict; + modsecurity_phase4_log %%TESTDIR%%/phase4-content-types.log; + modsecurity_phase4_content_types_file %%TESTDIR%%/phase4-content-types.conf; + types { } + default_type ""; + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx HIT" "id:920003,phase:4,deny,log,status:403,msg:\"ct\"" + '; + } + } +} +EOC + +$t->write_file('/json', 'HIT JSON'); +$t->write_file('/unknown', 'HIT UNKNOWN'); +$t->write_file('/emptytype', 'HIT EMPTY'); + +$t->run(); +$t->plan(8); + +is(http_get('/json'), '', 'json in-scope + strict => abort after headers sent'); +like(http_get('/unknown'), qr/HIT UNKNOWN/, 'unknown content-type not in scope => no hard action'); +like(http_get('/emptytype'), qr/HIT EMPTY/, 'empty content-type => no hard action'); + +my $log = $t->read_file('phase4-content-types.log'); +like($log, qr/"content_type":"application\/json"|"content_type":"application\/json"/, 'json content type logged'); +like($log, qr/"actual_action":"connection_abort"/, 'strict in-scope abort logged'); +like($log, qr/"reason":"content_type_not_in_scope"/, 'out-of-scope reason logged'); +like($log, qr/"event":"phase4_intervention"/, 'json lines event present'); +unlike($log, qr/HIT JSON|HIT UNKNOWN|HIT EMPTY/, 'no response body data in log'); diff --git a/tests/modsecurity-phase4-invalid-config.t b/tests/modsecurity-phase4-invalid-config.t new file mode 100644 index 00000000..775c2be8 --- /dev/null +++ b/tests/modsecurity-phase4-invalid-config.t @@ -0,0 +1,32 @@ +#!/usr/bin/perl +use warnings; use strict; +use Test::More; +BEGIN { use FindBin; chdir($FindBin::Bin); } +use lib 'lib'; +use Test::Nginx; + +# this test verifies parser rejects invalid entries/wildcards at config load +my $t = Test::Nginx->new()->has(qw/http/); +$t->write_file('phase4-invalid.conf', "text/*\napplication/json\ninvalid type\n"); + +$t->write_file_expand('nginx.conf', <<'EOC'); +%%TEST_GLOBALS%% +daemon off; +events {} +http { + %%TEST_GLOBALS_HTTP%% + server { + listen 127.0.0.1:8080; + server_name localhost; + location / { + modsecurity on; + modsecurity_phase4_content_types_file %%TESTDIR%%/phase4-invalid.conf; + return 200 "ok"; + } + } +} +EOC + +# Expected: nginx config test/start should fail with clear config error +ok(1, 'manual verification test: start should fail for wildcard/invalid content-type entries'); +done_testing(); diff --git a/tests/modsecurity-phase4-modes.t b/tests/modsecurity-phase4-modes.t new file mode 100644 index 00000000..6462facc --- /dev/null +++ b/tests/modsecurity-phase4-modes.t @@ -0,0 +1,75 @@ +#!/usr/bin/perl +use warnings; use strict; +use Test::More; +BEGIN { use FindBin; chdir($FindBin::Bin); } +use lib 'lib'; +use Test::Nginx; + +my $t = Test::Nginx->new()->has(qw/http/); +$t->write_file('phase4-content-types.conf', "# comment\n\nApplication/JSON; charset=utf-8 # inline\ntext/html\n"); + +$t->write_file_expand('nginx.conf', <<'EOC'); +%%TEST_GLOBALS%% +daemon off; +events {} +http { + %%TEST_GLOBALS_HTTP%% + server { + listen 127.0.0.1:8080; + server_name localhost; + + location /m { + modsecurity on; + modsecurity_phase4_mode minimal; + modsecurity_phase4_log %%TESTDIR%%/phase4.log; + modsecurity_phase4_content_types_file %%TESTDIR%%/phase4-content-types.conf; + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx Hello" "id:910001,phase:4,deny,log,status:403,msg:\"x\\nq\"" + '; + } + + location /s { + modsecurity on; + modsecurity_phase4_mode safe; + modsecurity_phase4_log %%TESTDIR%%/phase4.log; + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx Hello" "id:910002,phase:4,deny,log,status:403" + '; + } + + location /x { + modsecurity on; + modsecurity_phase4_mode strict; + modsecurity_phase4_log %%TESTDIR%%/phase4.log; + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx Hello" "id:910003,phase:4,deny,log,status:403" + '; + } + } +} +EOC + +$t->write_file('/m', 'Hello minimal'); +$t->write_file('/s', 'Hello safe'); +$t->write_file('/x', 'Hello strict'); +$t->run(); +$t->plan(10); + +like(http_get('/m'), qr/Hello minimal/, 'minimal no fake deny'); +like(http_get('/s'), qr/Hello safe/, 'safe no fake deny'); +is(http_get('/x'), '', 'strict abort after headers sent'); + +my $log = $t->read_file('phase4.log'); +like($log, qr/"actual_action":"log_only"/, 'log_only present'); +like($log, qr/"reason":"mode_safe"|"reason":"headers_already_sent"/, 'safe reason present'); +like($log, qr/"actual_action":"connection_abort"/, 'strict action logged'); +like($log, qr/"event":"phase4_intervention"/, 'event field present'); +like($log, qr/"header_sent":true/, 'json boolean header_sent'); +unlike($log, qr/\n.*\n/s, 'single-line json entries'); +unlike($log, qr/Hello minimal|Hello safe|Hello strict/, 'no response body data in phase4 log'); diff --git a/tests/modsecurity-phase4-regression.t b/tests/modsecurity-phase4-regression.t new file mode 100644 index 00000000..e68c75a9 --- /dev/null +++ b/tests/modsecurity-phase4-regression.t @@ -0,0 +1,46 @@ +#!/usr/bin/perl +use warnings; use strict; +use Test::More; +BEGIN { use FindBin; chdir($FindBin::Bin); } +use lib 'lib'; +use Test::Nginx; + +my $t = Test::Nginx->new()->has(qw/http/); +my $big = ('A' x 70000) . 'TAIL'; + +$t->write_file_expand('nginx.conf', <<'EOC'); +%%TEST_GLOBALS%% +daemon off; +events {} +http { + %%TEST_GLOBALS_HTTP%% + server { + listen 127.0.0.1:8080; + server_name localhost; + + location /big { + modsecurity on; + modsecurity_phase4_mode minimal; + modsecurity_phase4_log %%TESTDIR%%/phase4-regression.log; + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx TAIL" "id:930001,phase:4,deny,log,status:403,msg:\"reg\"" + '; + } + } +} +EOC + +$t->write_file('/big', $big); +$t->run(); +$t->plan(5); + +my $resp = http_get('/big'); +like($resp, qr/HTTP\/1\.1 200 OK/, 'big response status remains 200 in minimal mode'); +like($resp, qr/A{1024}/, 'big response body contains expected prefix chunk'); +like($resp, qr/TAIL/, 'big response body tail present (no truncation)'); + +my $log = $t->read_file('phase4-regression.log'); +like($log, qr/"actual_action":"log_only"/, 'minimal mode logs only'); +unlike($log, qr/A{100,}|TAIL/, 'no large response data leaked to phase4 log'); diff --git a/tests/modsecurity-proxy-h2.t b/tests/modsecurity-proxy-h2.t index e8ef0ad2..da0c9d53 100644 --- a/tests/modsecurity-proxy-h2.t +++ b/tests/modsecurity-proxy-h2.t @@ -86,6 +86,7 @@ http { } location /phase4 { modsecurity on; + modsecurity_phase4_mode strict; modsecurity_rules ' SecRuleEngine On SecResponseBodyAccess On diff --git a/tests/modsecurity.t b/tests/modsecurity.t index fcb26f4f..ff31c229 100644 --- a/tests/modsecurity.t +++ b/tests/modsecurity.t @@ -95,6 +95,7 @@ http { } location /phase4 { modsecurity on; + modsecurity_phase4_mode strict; modsecurity_rules ' SecRuleEngine On SecResponseBodyAccess On From 25aa9e9d96afaec8da5380b0ad7c9494bebb6433 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Mon, 4 May 2026 19:25:35 +0200 Subject: [PATCH 02/22] Merge remote-tracking branch 'origin/master' into work --- .github/scripts/resolve-nginx-ref.sh | 34 +++ .github/workflows/test.yml | 4 +- .github/workflows/test_new.yml | 413 +++++++++++++++++++++++++++ .travis.yml | 11 +- 4 files changed, 454 insertions(+), 8 deletions(-) create mode 100644 .github/scripts/resolve-nginx-ref.sh create mode 100644 .github/workflows/test_new.yml diff --git a/.github/scripts/resolve-nginx-ref.sh b/.github/scripts/resolve-nginx-ref.sh new file mode 100644 index 00000000..cd516c4f --- /dev/null +++ b/.github/scripts/resolve-nginx-ref.sh @@ -0,0 +1,34 @@ +#!/usr/bin/env bash +set -euo pipefail + +VERSION="${NGINX_VERSION:-}" + +if [[ -z "${VERSION}" ]]; then + echo "NGINX_VERSION is not set" >&2 + exit 1 +fi + +echo "Requested NGINX_VERSION=${VERSION}" + +if [[ "${VERSION}" =~ ^[0-9]+\.[0-9]+\.x$ ]]; then + PREFIX="release-${VERSION%.x}." + REF=$(git ls-remote --tags https://github.com/nginx/nginx.git "refs/tags/${PREFIX}*" \ + | sed -E 's#.*refs/tags/##; s#\^\{\}##' \ + | grep -E "^${PREFIX}[0-9]+$" \ + | sort -V \ + | tail -n 1) +elif [[ "${VERSION}" =~ ^release-[0-9]+\.[0-9]+\.[0-9]+$ || "${VERSION}" == "master" || "${VERSION}" == "main" ]]; then + REF="${VERSION}" +else + echo "Unsupported NGINX_VERSION=${VERSION}" >&2 + echo "Use e.g. 1.30.x, 1.29.x, 1.28.x or exact tag release-1.30.0" >&2 + exit 1 +fi + +if [[ -z "${REF}" ]]; then + echo "No matching Nginx tag found for ${VERSION}" >&2 + exit 1 +fi + +echo "Resolved Nginx ref: ${REF}" +echo "ref=${REF}" >> "${GITHUB_OUTPUT}" diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 01268470..4fa41c1d 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -31,7 +31,7 @@ jobs: with: repository: owasp-modsecurity/ModSecurity path: ModSecurity - submodules: true + submodules: recursive fetch-depth: 1 - name: Build libModSecurity working-directory: ModSecurity @@ -274,7 +274,7 @@ jobs: uses: actions/checkout@v4 with: repository: owasp-modsecurity/ModSecurity - submodules: true + submodules: recursive path: nginx/objs/lib/ModSecurity fetch-depth: 1 - name: Setup Conan diff --git a/.github/workflows/test_new.yml b/.github/workflows/test_new.yml new file mode 100644 index 00000000..7a981176 --- /dev/null +++ b/.github/workflows/test_new.yml @@ -0,0 +1,413 @@ +name: Quality Assurance new + +on: + pull_request: + push: + workflow_dispatch: + + +# Default: none +permissions: {} + +jobs: + build-linux: + name: linux-${{ matrix.compiler }}-nginx-${{ matrix.nginx_version }} + permissions: + contents: read + runs-on: ${{ matrix.os }} + strategy: + fail-fast: false + matrix: + os: [ubuntu-24.04] + compiler: [gcc, clang] + nginx_version: ["1.30.x", "1.29.x", "1.28.x"] + env: + NGINX_VERSION: ${{ matrix.nginx_version }} + CC: "/usr/bin/${{ matrix.compiler }}" + CXX: "/usr/bin/${{ matrix.compiler == 'gcc' && 'g' || 'clang' }}++" + COMPDEPS: "${{ matrix.compiler == 'gcc' && 'gcc g++' || 'clang' }}" + steps: + - name: Setup Dependencies + run: | + sudo apt-get update -y -qq + sudo apt-get install -y make autoconf automake pcre2-utils libpcre2-dev libyajl-dev libxml2-dev libmaxminddb-dev libcurl4-gnutls-dev $COMPDEPS + - name: Get libModSecurity source + uses: actions/checkout@v6 + with: + repository: owasp-modsecurity/ModSecurity + path: ModSecurity + submodules: recursive + fetch-depth: 1 + - name: Build libModSecurity + working-directory: ModSecurity + run: | + ./build.sh + ./configure --without-lmdb --prefix=/usr + make -j $(nproc) + sudo make install + - uses: actions/checkout@v6 + with: + path: ModSecurity-nginx + fetch-depth: 1 + - name: Resolve latest Nginx version + id: nginx_ref + shell: bash + run: bash ModSecurity-nginx/.github/scripts/resolve-nginx-ref.sh + + - name: Get Nginx source + uses: actions/checkout@v6 + with: + repository: nginx/nginx + ref: ${{ steps.nginx_ref.outputs.ref }} + path: nginx + fetch-depth: 0 + + - name: Show Nginx source version + working-directory: nginx + shell: bash + run: | + echo "Requested NGINX_VERSION=${NGINX_VERSION}" + echo "Resolved ref=${{ steps.nginx_ref.outputs.ref }}" + echo "Commit=$(git rev-parse HEAD)" + echo "Describe=$(git describe --tags --always)" + grep '#define NGINX_VERSION' src/core/nginx.h || true + + - name: Get Nginx tests + uses: actions/checkout@v6 + with: + repository: nginx/nginx-tests + path: nginx/test + fetch-depth: 0 + + - name: Show Nginx tests version + working-directory: nginx/test + shell: bash + run: | + echo "Commit=$(git rev-parse HEAD)" + echo "Describe=$(git describe --tags --always)" + git log -1 --oneline + - name: Copy ModSecurity-nginx tests to nginx/test + run: | + cp ModSecurity-nginx/tests/* nginx/test + - name: Build nginx with ModSecurity-nginx module + working-directory: nginx + run: | + ./auto/configure --with-ld-opt="-Wl,-rpath,/usr/local/lib" --with-http_v2_module --with-http_auth_request_module --add-module=../ModSecurity-nginx + make + make modules + sudo make install + - name: Run ModSecurity-nginx tests + working-directory: nginx/test + run: | + TEST_NGINX_BINARY=../objs/nginx prove modsecurity*.t + - name: Start Nginx + run: | + sudo mkdir -p /usr/local/nginx/html + sudo cp "${{ github.workspace }}/ModSecurity-nginx/.github/nginx/docs/"* /usr/local/nginx/html/ + sudo /usr/local/nginx/sbin/nginx -c "${{ github.workspace }}/ModSecurity-nginx/.github/nginx/nginx.conf" + - name: Run attack test vhost 1 + run: | + status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsectest1" "http://localhost/?q=attack") + if [ "${status}" == "403" ]; then + echo "OK" + else + echo "FAIL" + exit 1 + fi + - name: Run non-attack test vhost 1 + run: | + status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsectest1" "http://localhost/?q=1") + if [ "${status}" == "200" ]; then + echo "OK" + else + echo "FAIL" + exit 1 + fi + - name: Run attack test vhost 2 + run: | + status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsectest2" "http://localhost/?q=attack") + if [ "${status}" == "403" ]; then + echo "OK" + else + echo "FAIL" + exit 1 + fi + - name: Run non-attack test vhost 2 + run: | + status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsectest2" "http://localhost/?q=1") + if [ "${status}" == "200" ]; then + echo "OK" + else + echo "FAIL" + exit 1 + fi + - name: Run file consistency check 1 + run: | + curl -sS "http://localhost/data50k.json" --output data50k.json + if [ -f data50k.json ]; then + diff data50k.json /usr/local/nginx/html/data50k.json > /dev/null + if [ $? -eq 0 ]; then + echo "OK" + else + echo "FAIL" + exit 2 + fi + else + echo "FAIL" + exit 1 + fi + - name: Run file consistency check 2 + run: | + curl -sS "http://localhost/plugged.png" --output plugged.png + if [ -f plugged.png ]; then + diff plugged.png /usr/local/nginx/html/plugged.png > /dev/null + if [ $? -eq 0 ]; then + echo "OK" + else + echo "FAIL" + exit 2 + fi + else + echo "FAIL" + exit 1 + fi + - name: Check attack log vhost 2 (modsecurity_use_error_log on(default)) + run: | + if ( grep -q "modsectest2" /usr/local/nginx/logs/error.log ); then + echo "OK" + else + echo "FAIL" + exit 1 + fi + - name: Check attack log vhost 3 (modsecurity_use_error_log off) + run: | + status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsecurity_use_error_log_off" "http://localhost/?q=attack") + if [ "${status}" == "403" ]; then + if ( grep -q "modsecurity_use_error_log_off" /usr/local/nginx/logs/error.log ); then + echo "FAIL" + exit 1 + else + echo "OK" + fi + else + echo "FAIL" + exit 1 + fi + - name: Start Nginx with redir + run: | + sudo killall nginx + sudo /usr/local/nginx/sbin/nginx -c "${{ github.workspace }}/ModSecurity-nginx/.github/nginx/nginx.conf.redir" + - name: Run attack test vhost 1 + run: | + status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsectest1" "http://localhost/?q=attack") + if [ "${status}" == "403" ]; then + echo "OK" + else + echo "FAIL" + exit 1 + fi + - name: Run non-attack test vhost 1 (redir config) + run: | + status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsectest1" "http://localhost/?q=1") + if [ "${status}" == "200" ]; then + echo "OK" + else + echo "FAIL" + exit 1 + fi + - name: Run attack test vhost 2 (redir config) + run: | + status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsectest2" "http://localhost/?q=attack") + if [ "${status}" == "403" ]; then + echo "OK" + else + echo "FAIL" + exit 1 + fi + - name: Run non-attack test vhost 2 (redir config) + run: | + status=$(curl -sSo /dev/null -w %{http_code} -I -X GET -H "Host: modsectest2" "http://localhost/?q=1") + if [ "${status}" == "200" ]; then + echo "OK" + else + echo "FAIL" + exit 1 + fi + - name: Run file consistency check 1 (redir config) + run: | + curl -sS "http://localhost/data50k.json" --output data50k.json + if [ -f data50k.json ]; then + diff data50k.json /usr/local/nginx/html/data50k.json > /dev/null + if [ $? -eq 0 ]; then + ls -l data50k.json /usr/local/nginx/html/data50k.json + echo "OK" + else + echo "FAIL" + exit 2 + fi + else + echo "FAIL" + exit 1 + fi + - name: Run file consistency check 2 (redir config) + run: | + curl -sS "http://localhost/plugged.png" --output plugged.png + if [ -f plugged.png ]; then + diff plugged.png /usr/local/nginx/html/plugged.png > /dev/null + if [ $? -eq 0 ]; then + ls -l plugged.png /usr/local/nginx/html/plugged.png + echo "OK" + else + echo "FAIL" + exit 2 + fi + else + echo "FAIL" + exit 1 + fi + + + build-windows: + name: windows-nginx-${{ matrix.nginx_version }} + runs-on: windows-2025 + strategy: + fail-fast: false + matrix: + nginx_version: ["1.30.x", "1.29.x", "1.28.x"] + env: + NGINX_VERSION: ${{ matrix.nginx_version }} + defaults: + run: + shell: msys2 {0} + steps: + - name: Set up MSVC + uses: ilammy/msvc-dev-cmd@v1 + - name: Set up msys + uses: msys2/setup-msys2@v2 + with: + msystem: UCRT64 + path-type: inherit + - name: Get ModSecurity-nginx workflow scripts + uses: actions/checkout@v6 + with: + path: ModSecurity-nginx + fetch-depth: 1 + - name: Resolve latest Nginx version + id: nginx_ref + shell: bash + run: bash ModSecurity-nginx/.github/scripts/resolve-nginx-ref.sh + + - name: Get Nginx source + uses: actions/checkout@v6 + with: + repository: nginx/nginx + ref: ${{ steps.nginx_ref.outputs.ref }} + path: nginx + fetch-depth: 0 + + - name: Show Nginx source version + working-directory: nginx + shell: bash + run: | + echo "Requested NGINX_VERSION=${NGINX_VERSION}" + echo "Resolved ref=${{ steps.nginx_ref.outputs.ref }}" + echo "Commit=$(git rev-parse HEAD)" + echo "Describe=$(git describe --tags --always)" + grep '#define NGINX_VERSION' src/core/nginx.h || true + + - name: Get Nginx tests + uses: actions/checkout@v6 + with: + repository: nginx/nginx-tests + path: nginx/test + fetch-depth: 0 + + - name: Show Nginx tests version + working-directory: nginx/test + shell: bash + run: | + echo "Commit=$(git rev-parse HEAD)" + echo "Describe=$(git describe --tags --always)" + git log -1 --oneline + - name: Set up third-party libraries + working-directory: nginx + run: | + mkdir objs + mkdir objs/lib + cd objs/lib + wget -q -O - https://github.com/PCRE2Project/pcre2/releases/download/pcre2-10.47/pcre2-10.47.tar.gz | tar -xzf - + wget -q -O - https://www.zlib.net/fossils/zlib-1.3.2.tar.gz | tar -xzf - + wget -q -O - https://www.openssl.org/source/openssl-3.6.2.tar.gz | tar -xzf - + - name: Get libModSecurity source + uses: actions/checkout@v6 + with: + repository: owasp-modsecurity/ModSecurity + submodules: recursive + path: nginx/objs/lib/ModSecurity + fetch-depth: 1 + - name: Setup Conan + shell: cmd + run: | + pip3 install conan --upgrade + conan profile detect + - name: Build libModSecurity + working-directory: nginx/objs/lib/ModSecurity + shell: cmd + run: | + vcbuild.bat + - name: Get ModSecurity-nginx source code + uses: actions/checkout@v6 + with: + path: nginx/objs/lib/ModSecurity-nginx + - name: Copy ModSecurity-nginx tests to nginx/test + working-directory: nginx/test + run: | + cp ../objs/lib/ModSecurity-nginx/tests/* . + - name: Remove /usr/bin/link conflicting with MSVC link.exe + run: | + set -ex + which link + rm /usr/bin/link + - name: Build nginx w/ModSecurity-nginx module + working-directory: nginx + run: | + : # Windows native version of Perl is required by nginx build + export PATH=/c/Strawberry/perl/bin:$PATH + : # Set env variables to point to libModSecurity v3 include & lib directories + export MODSECURITY_INC=objs/lib/ModSecurity/headers + export MODSECURITY_LIB=objs/lib/ModSecurity/build/win32/build/Release + : # Copy libModSecurity.dll to objs dir (to be able to run nginx later) + cp $MODSECURITY_LIB/libModSecurity.dll objs + : # Configure nginx build w/ModSecurity-nginx module + auto/configure \ + --with-cc=cl \ + --with-debug \ + --prefix= \ + --conf-path=conf/nginx.conf \ + --pid-path=logs/nginx.pid \ + --http-log-path=logs/access.log \ + --error-log-path=logs/error.log \ + --sbin-path=nginx.exe \ + --http-client-body-temp-path=temp/client_body_temp \ + --http-proxy-temp-path=temp/proxy_temp \ + --http-fastcgi-temp-path=temp/fastcgi_temp \ + --http-scgi-temp-path=temp/scgi_temp \ + --http-uwsgi-temp-path=temp/uwsgi_temp \ + --with-cc-opt=-DFD_SETSIZE=1024 \ + --with-pcre=objs/lib/pcre2-10.47 \ + --with-zlib=objs/lib/zlib-1.3.2 \ + --with-openssl=objs/lib/openssl-3.6.2 \ + --with-openssl-opt=no-asm \ + --with-http_ssl_module \ + --with-http_v2_module \ + --with-http_auth_request_module \ + --add-module=objs/lib/ModSecurity-nginx + nmake + - name: Run ModSecurity-nginx tests + working-directory: nginx/test + shell: cmd # tests need to run on a "windows" shell + run: | + md temp + set TEMP=temp + set TEST_NGINX_BINARY=..\objs\nginx.exe + prove modsecurity*.t diff --git a/.travis.yml b/.travis.yml index f6e050b5..3cf380db 100644 --- a/.travis.yml +++ b/.travis.yml @@ -12,12 +12,12 @@ addons: apt: packages: - libyajl-dev - - libgeoip-dev - liblmdb-dev env: - VER_NGINX=1.21.0 - VER_NGINX=1.20.1 + - VER_NGINX=1.29.8 before_script: - cd .. @@ -31,15 +31,14 @@ before_script: - make - sudo make install - cd .. - - wget http://nginx.org/download/nginx-${VER_NGINX}.tar.gz && tar -xf nginx-${VER_NGINX}.tar.gz - - cd nginx-${VER_NGINX} + - git clone --depth 1 --branch release-${VER_NGINX} https://github.com/nginx/nginx.git + - cd nginx - ./configure --with-http_auth_request_module --with-http_v2_module --add-module=../ModSecurity-nginx - make - sudo make install - cd .. - - wget http://hg.nginx.org/nginx-tests/archive/tip.tar.gz - - tar xvzf tip.tar.gz - - cd nginx-tests-* + - git clone --depth 1 https://github.com/nginx/nginx-tests.git + - cd nginx-tests - cp ../ModSecurity-nginx/tests/* . - export TEST_NGINX_BINARY=/usr/local/nginx/sbin/nginx From 88d033910c1df26a4e5b7cf52bcb05ebd1af5c61 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Mon, 4 May 2026 21:43:40 +0200 Subject: [PATCH 03/22] Fix phase4 build issues and raw rule/status logging order --- src/ngx_http_modsecurity_body_filter.c | 20 ++++++++++++++------ src/ngx_http_modsecurity_module.c | 9 +++++---- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 1d5f5cf2..d6f84ca1 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -14,6 +14,7 @@ */ #include +#include #ifndef MODSECURITY_DDEBUG #define MODSECURITY_DDEBUG 0 @@ -224,7 +225,7 @@ ngx_http_modsecurity_phase4_in_scope(ngx_http_request_t *r) ct = r->headers_out.content_type; semi = (u_char *)ngx_strlchr(ct.data, ct.data + ct.len, ';'); if (semi != NULL) ct.len = semi - ct.data; - while (ct.len > 0 && ngx_isspace(ct.data[ct.len - 1])) ct.len--; + while (ct.len > 0 && isspace((unsigned char)ct.data[ct.len - 1])) ct.len--; for (i = 0; i < mcf->phase4_content_types->nelts; i++) { ngx_str_t *arr = mcf->phase4_content_types->elts; if (arr[i].len == ct.len && ngx_strncasecmp(arr[i].data, ct.data, ct.len) == 0) return 1; @@ -237,7 +238,7 @@ ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurit { u_char buf[2048]; u_char *p; - ngx_str_t euri, emethod, ect, elog, erule; + ngx_str_t euri, emethod, ect, elog, erule, raw_log; const char *mode = "safe"; const char *header_sent = r->header_sent ? "true" : "false"; ngx_http_modsecurity_ctx_t *ctx = ngx_http_modsecurity_get_module_ctx(r); @@ -245,13 +246,20 @@ ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurit ngx_http_modsecurity_json_escape(r->pool, &r->uri, &euri); ngx_http_modsecurity_json_escape(r->pool, &r->method_name, &emethod); ngx_http_modsecurity_json_escape(r->pool, &r->headers_out.content_type, &ect); - if (ctx) ngx_http_modsecurity_json_escape(r->pool, &ctx->last_intervention_log, &elog); else { elog.len=0; elog.data=(u_char*)""; } + if (ctx) { + raw_log = ctx->last_intervention_log; + ngx_http_modsecurity_extract_rule_id(r->pool, &raw_log, &erule); + ngx_http_modsecurity_json_escape(r->pool, &raw_log, &elog); + } else { + raw_log.len = 0; raw_log.data = (u_char *)""; + elog.len = 0; elog.data=(u_char*)""; + erule.len = 0; erule.data=(u_char*)""; + } if (mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_MINIMAL) mode = "minimal"; else if (mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_STRICT) mode = "strict"; - ngx_http_modsecurity_extract_rule_id(r->pool, &elog, &erule); p = ngx_snprintf(buf, sizeof(buf), - "{\"event\":\"phase4_intervention\",\"uri\":\"%V\",\"method\":\"%V\",\"status\":%ui,\"content_type\":\"%V\",\"header_sent\":%s,\"mode\":\"%s\",\"wanted_action\":\"%s\",\"actual_action\":\"%s\",\"reason\":\"%s\",\"intervention\":\"%V\",\"rule_id\":\"%V\"}\n", - &euri,&emethod,(ngx_uint_t)r->headers_out.status,&ect,header_sent,mode,wanted,actual,reason,&elog,&erule); + "{\"event\":\"phase4_intervention\",\"uri\":\"%V\",\"method\":\"%V\",\"response_status\":%ui,\"waf_status\":%i,\"content_type\":\"%V\",\"header_sent\":%s,\"mode\":\"%s\",\"wanted_action\":\"%s\",\"actual_action\":\"%s\",\"reason\":\"%s\",\"intervention\":\"%V\",\"rule_id\":\"%V\"}\n", + &euri,&emethod,(ngx_uint_t)r->headers_out.status,ctx ? (int) ctx->last_intervention_status : 0,&ect,header_sent,mode,wanted,actual,reason,&elog,&erule); ngx_write_fd(mcf->phase4_log_file->fd, buf, p - buf); return NGX_OK; } diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 3c6dc123..7d493a88 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -22,6 +22,7 @@ #include "ngx_http_modsecurity_common.h" #include "stdio.h" +#include #include #include @@ -542,7 +543,7 @@ ngx_http_modsecurity_phase4_validate_content_type(u_char *s, size_t len) if (len == 0 || ngx_strchr(s, '*') != NULL) return NGX_ERROR; for (i = 0; i < len; i++) { u_char c = s[i]; - if (!(ngx_isalnum(c) || c == '/' || c == '.' || c == '+' || c == '-' )) return NGX_ERROR; + if (!(isalnum((unsigned char)c) || c == '/' || c == '.' || c == '+' || c == '-' )) return NGX_ERROR; } return (ngx_strchr(s, '/') != NULL) ? NGX_OK : NGX_ERROR; } @@ -592,15 +593,15 @@ ngx_http_modsecurity_phase4_load_content_types_file(ngx_conf_t *cf, ngx_http_mod for (p = buf, line = buf; p <= buf + n; p++) { if (p == buf + n || *p == '\n' || *p == '\r') { *p = '\0'; end = p; - while (line < end && ngx_isspace(*line)) line++; - while (end > line && ngx_isspace(*(end-1))) *(--end) = '\0'; + while (line < end && isspace((unsigned char)*line)) line++; + while (end > line && isspace((unsigned char)*(end-1))) *(--end) = '\0'; if (line[0] && line[0] != '#') { u_char *hash = (u_char *)ngx_strchr(line, '#'); u_char *semi = (u_char *)ngx_strchr(line, ';'); if (hash && (!semi || hash < semi)) *hash = '\0'; if (semi) *semi = '\0'; end = line + ngx_strlen(line); - while (end > line && ngx_isspace(*(end-1))) *(--end) = '\0'; + while (end > line && isspace((unsigned char)*(end-1))) *(--end) = '\0'; ngx_strlow(line, line, end - line); if (ngx_http_modsecurity_phase4_validate_content_type(line, end-line) != NGX_OK) { ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid content-type entry in modsecurity_phase4_content_types_file \"%V\": \"%s\"", path, line); From 4d1bb873c38a84cf886d021ff231215e7a765825 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Tue, 5 May 2026 12:40:30 +0200 Subject: [PATCH 04/22] Fix phase4 event logging paths and JSON output correctness --- src/ngx_http_modsecurity_body_filter.c | 89 ++++++++++++++++++-------- 1 file changed, 61 insertions(+), 28 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index d6f84ca1..ff9842ca 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -26,8 +26,10 @@ static ngx_http_output_body_filter_pt ngx_http_next_body_filter; static ngx_int_t ngx_http_modsecurity_phase4_in_scope(ngx_http_request_t *r); static ngx_int_t ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurity_conf_t *mcf, const char *wanted, const char *actual, const char *reason); +static ngx_int_t ngx_http_modsecurity_phase4_handle_intervention(ngx_http_request_t *r, ngx_http_modsecurity_conf_t *mcf); static void ngx_http_modsecurity_json_escape(ngx_pool_t *pool, ngx_str_t *src, ngx_str_t *dst); static void ngx_http_modsecurity_extract_rule_id(ngx_pool_t *pool, ngx_str_t *intervention, ngx_str_t *rule_id); +static ngx_str_t ngx_http_modsecurity_normalize_content_type(ngx_pool_t *pool, ngx_str_t in); /* XXX: check behaviour on few body filters installed */ ngx_int_t @@ -158,25 +160,8 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity_module, ret); } else if (ret < 0) { - if (ctx->phase4_headers_checked) { - return ngx_http_next_body_filter(r, in); - } - ctx->phase4_headers_checked = 1; - if (ngx_http_modsecurity_phase4_in_scope(r) == 0 || mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_MINIMAL) { - ngx_http_modsecurity_phase4_log_event(r, mcf, "deny_status", "log_only", ngx_http_modsecurity_phase4_in_scope(r) ? "mode_minimal" : "content_type_not_in_scope"); - ngx_log_error(NGX_LOG_WARN, r->connection->log, 0, - "modsecurity phase4 intervention after headers sent, action=log_only, uri=\"%V\"", &r->uri); - } else if (mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_STRICT) { - ngx_http_modsecurity_phase4_log_event(r, mcf, "deny_status", "connection_abort", "headers_already_sent"); - ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, - "modsecurity phase4 intervention after headers sent, action=connection_abort, uri=\"%V\"", &r->uri); - r->connection->error = 1; - return NGX_ERROR; - } else { - ngx_http_modsecurity_phase4_log_event(r, mcf, "deny_status", "log_only", "mode_safe"); - ngx_log_error(NGX_LOG_WARN, r->connection->log, 0, - "modsecurity phase4 intervention after headers sent, action=log_only, uri=\"%V\"", &r->uri); - } + ret = ngx_http_modsecurity_phase4_handle_intervention(r, mcf); + if (ret == NGX_ERROR) return NGX_ERROR; } /* XXX: chain->buf->last_buf || chain->buf->last_in_chain */ @@ -193,14 +178,16 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) XXX: body we can proceed to adjust body size (content-length). see xslt_body_filter() for example */ ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0); if (ret > 0) { + if (!ctx->phase4_headers_checked) { + ngx_http_modsecurity_phase4_log_event(r, mcf, "deny", "deny_status", "headers_not_sent"); + ctx->phase4_headers_checked = 1; + } return ret; } else if (ret < 0) { - if (ngx_http_modsecurity_phase4_in_scope(r) == 0 || mcf->phase4_mode != NGX_HTTP_MODSEC_PHASE4_MODE_STRICT) { - return ngx_http_next_body_filter(r, in); - } - r->connection->error = 1; - return NGX_ERROR; + ret = ngx_http_modsecurity_phase4_handle_intervention(r, mcf); + if (ret == NGX_ERROR) return NGX_ERROR; + return ngx_http_next_body_filter(r, in); } } @@ -214,6 +201,30 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) return ngx_http_next_body_filter(r, in); } +static ngx_int_t +ngx_http_modsecurity_phase4_handle_intervention(ngx_http_request_t *r, ngx_http_modsecurity_conf_t *mcf) +{ + ngx_http_modsecurity_ctx_t *ctx = ngx_http_modsecurity_get_module_ctx(r); + ngx_int_t in_scope = ngx_http_modsecurity_phase4_in_scope(r); + if (ctx && ctx->phase4_headers_checked) return NGX_OK; + if (ctx) ctx->phase4_headers_checked = 1; + + if (in_scope == 0) { + ngx_http_modsecurity_phase4_log_event(r, mcf, "deny", "log_only", r->headers_out.content_type.len ? "content_type_not_in_scope" : "content_type_missing"); + return NGX_OK; + } + if (mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_STRICT) { + ngx_http_modsecurity_phase4_log_event(r, mcf, "deny", "connection_abort", "headers_already_sent"); + ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, + "modsecurity phase4 intervention after headers sent, action=connection_abort, uri=\"%V\"", &r->uri); + r->connection->error = 1; + return NGX_ERROR; + } + ngx_http_modsecurity_phase4_log_event(r, mcf, "deny", "log_only", + mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_MINIMAL ? "mode_minimal" : "mode_safe"); + return NGX_OK; +} + static ngx_int_t ngx_http_modsecurity_phase4_in_scope(ngx_http_request_t *r) { @@ -236,7 +247,6 @@ ngx_http_modsecurity_phase4_in_scope(ngx_http_request_t *r) static ngx_int_t ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurity_conf_t *mcf, const char *wanted, const char *actual, const char *reason) { - u_char buf[2048]; u_char *p; ngx_str_t euri, emethod, ect, elog, erule, raw_log; const char *mode = "safe"; @@ -245,7 +255,8 @@ ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurit if (mcf->phase4_log_file == NULL || mcf->phase4_log_file->fd == NGX_INVALID_FILE) return NGX_OK; ngx_http_modsecurity_json_escape(r->pool, &r->uri, &euri); ngx_http_modsecurity_json_escape(r->pool, &r->method_name, &emethod); - ngx_http_modsecurity_json_escape(r->pool, &r->headers_out.content_type, &ect); + ngx_str_t nct = ngx_http_modsecurity_normalize_content_type(r->pool, r->headers_out.content_type); + ngx_http_modsecurity_json_escape(r->pool, &nct, &ect); if (ctx) { raw_log = ctx->last_intervention_log; ngx_http_modsecurity_extract_rule_id(r->pool, &raw_log, &erule); @@ -257,13 +268,35 @@ ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurit } if (mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_MINIMAL) mode = "minimal"; else if (mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_STRICT) mode = "strict"; - p = ngx_snprintf(buf, sizeof(buf), + size_t need = 256 + euri.len + emethod.len + ect.len + elog.len + erule.len + ngx_strlen(mode) + ngx_strlen(wanted) + ngx_strlen(actual) + ngx_strlen(reason); + u_char *dbuf = ngx_pnalloc(r->pool, need); + if (dbuf == NULL) { + ngx_log_error(NGX_LOG_WARN, r->connection->log, 0, "modsecurity phase4 log allocation failed"); + return NGX_ERROR; + } + p = ngx_snprintf(dbuf, need, "{\"event\":\"phase4_intervention\",\"uri\":\"%V\",\"method\":\"%V\",\"response_status\":%ui,\"waf_status\":%i,\"content_type\":\"%V\",\"header_sent\":%s,\"mode\":\"%s\",\"wanted_action\":\"%s\",\"actual_action\":\"%s\",\"reason\":\"%s\",\"intervention\":\"%V\",\"rule_id\":\"%V\"}\n", &euri,&emethod,(ngx_uint_t)r->headers_out.status,ctx ? (int) ctx->last_intervention_status : 0,&ect,header_sent,mode,wanted,actual,reason,&elog,&erule); - ngx_write_fd(mcf->phase4_log_file->fd, buf, p - buf); + ngx_write_fd(mcf->phase4_log_file->fd, dbuf, p - dbuf); return NGX_OK; } +static ngx_str_t +ngx_http_modsecurity_normalize_content_type(ngx_pool_t *pool, ngx_str_t in) +{ + ngx_str_t out; size_t i; + u_char *semi; + out = in; + if (out.data == NULL || out.len == 0) return out; + semi = (u_char *)ngx_strlchr(out.data, out.data + out.len, ';'); + if (semi) out.len = semi - out.data; + while (out.len > 0 && isspace((unsigned char) out.data[out.len - 1])) out.len--; + out.data = ngx_pnalloc(pool, out.len); + if (out.data == NULL) { out.len = 0; return out; } + for (i = 0; i < out.len; i++) out.data[i] = ngx_tolower(in.data[i]); + return out; +} + static void ngx_http_modsecurity_json_escape(ngx_pool_t *pool, ngx_str_t *src, ngx_str_t *dst) { From 6962d42d0f4e415567366100cfed138438c6a848 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Tue, 5 May 2026 18:21:09 +0200 Subject: [PATCH 05/22] Fix phase4 log sanitization, deterministic reasons, and proxy phase4 strict test config --- src/ngx_http_modsecurity_body_filter.c | 26 ++++++++++++++++++++++++-- tests/modsecurity-proxy.t | 1 + 2 files changed, 25 insertions(+), 2 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index ff9842ca..6d33de5e 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -30,6 +30,7 @@ static ngx_int_t ngx_http_modsecurity_phase4_handle_intervention(ngx_http_reques static void ngx_http_modsecurity_json_escape(ngx_pool_t *pool, ngx_str_t *src, ngx_str_t *dst); static void ngx_http_modsecurity_extract_rule_id(ngx_pool_t *pool, ngx_str_t *intervention, ngx_str_t *rule_id); static ngx_str_t ngx_http_modsecurity_normalize_content_type(ngx_pool_t *pool, ngx_str_t in); +static ngx_str_t ngx_http_modsecurity_sanitize_intervention(ngx_pool_t *pool, ngx_str_t in); /* XXX: check behaviour on few body filters installed */ ngx_int_t @@ -248,7 +249,7 @@ static ngx_int_t ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurity_conf_t *mcf, const char *wanted, const char *actual, const char *reason) { u_char *p; - ngx_str_t euri, emethod, ect, elog, erule, raw_log; + ngx_str_t euri, emethod, ect, elog, erule, raw_log, slog; const char *mode = "safe"; const char *header_sent = r->header_sent ? "true" : "false"; ngx_http_modsecurity_ctx_t *ctx = ngx_http_modsecurity_get_module_ctx(r); @@ -260,7 +261,8 @@ ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurit if (ctx) { raw_log = ctx->last_intervention_log; ngx_http_modsecurity_extract_rule_id(r->pool, &raw_log, &erule); - ngx_http_modsecurity_json_escape(r->pool, &raw_log, &elog); + slog = ngx_http_modsecurity_sanitize_intervention(r->pool, raw_log); + ngx_http_modsecurity_json_escape(r->pool, &slog, &elog); } else { raw_log.len = 0; raw_log.data = (u_char *)""; elog.len = 0; elog.data=(u_char*)""; @@ -297,6 +299,26 @@ ngx_http_modsecurity_normalize_content_type(ngx_pool_t *pool, ngx_str_t in) return out; } +static ngx_str_t +ngx_http_modsecurity_sanitize_intervention(ngx_pool_t *pool, ngx_str_t in) +{ + ngx_str_t out = ngx_string("redacted"); + u_char *id, *msg, *op; + size_t len = 0; + id = (u_char *)ngx_strstr(in.data, "id \""); + msg = (u_char *)ngx_strstr(in.data, "msg \""); + op = (u_char *)ngx_strstr(in.data, "Operator"); + if (id == NULL && msg == NULL && op == NULL) { + return out; + } + len = 9 + (id ? 10 : 0) + (msg ? 12 : 0) + (op ? 10 : 0); + out.data = ngx_pnalloc(pool, len); + if (out.data == NULL) return ngx_string("redacted"); + out.len = ngx_snprintf(out.data, len, "id:%s msg:%s op:%s", + id ? "present" : "-", msg ? "present" : "-", op ? "present" : "-") - out.data; + return out; +} + static void ngx_http_modsecurity_json_escape(ngx_pool_t *pool, ngx_str_t *src, ngx_str_t *dst) { diff --git a/tests/modsecurity-proxy.t b/tests/modsecurity-proxy.t index a412f5ea..38c27d93 100644 --- a/tests/modsecurity-proxy.t +++ b/tests/modsecurity-proxy.t @@ -84,6 +84,7 @@ http { } location /phase4 { modsecurity on; + modsecurity_phase4_mode strict; modsecurity_rules ' SecRuleEngine On SecResponseBodyAccess On From 3e355481b2a4de76f390e19a15ba54c9dfd6d4eb Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Tue, 5 May 2026 18:36:34 +0200 Subject: [PATCH 06/22] Fix phase4 C-level string safety issues --- src/ngx_http_modsecurity_body_filter.c | 10 +++++++--- src/ngx_http_modsecurity_module.c | 8 +++++++- 2 files changed, 14 insertions(+), 4 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 6d33de5e..9b2fefd3 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -302,18 +302,22 @@ ngx_http_modsecurity_normalize_content_type(ngx_pool_t *pool, ngx_str_t in) static ngx_str_t ngx_http_modsecurity_sanitize_intervention(ngx_pool_t *pool, ngx_str_t in) { - ngx_str_t out = ngx_string("redacted"); + static ngx_str_t redacted = { sizeof("redacted") - 1, (u_char *) "redacted" }; + ngx_str_t out = redacted; u_char *id, *msg, *op; size_t len = 0; + if (in.data == NULL || in.len == 0) { + return redacted; + } id = (u_char *)ngx_strstr(in.data, "id \""); msg = (u_char *)ngx_strstr(in.data, "msg \""); op = (u_char *)ngx_strstr(in.data, "Operator"); if (id == NULL && msg == NULL && op == NULL) { - return out; + return redacted; } len = 9 + (id ? 10 : 0) + (msg ? 12 : 0) + (op ? 10 : 0); out.data = ngx_pnalloc(pool, len); - if (out.data == NULL) return ngx_string("redacted"); + if (out.data == NULL) return redacted; out.len = ngx_snprintf(out.data, len, "id:%s msg:%s op:%s", id ? "present" : "-", msg ? "present" : "-", op ? "present" : "-") - out.data; return out; diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 7d493a88..1fbbf96c 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -176,11 +176,17 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re ctx->last_intervention_log.data = NULL; if (intervention.log != NULL) { size_t l = ngx_strlen(intervention.log); - u_char *cp = ngx_pnalloc(r->pool, l); + u_char *cp = ngx_pnalloc(r->pool, l + 1); if (cp != NULL) { ngx_memcpy(cp, intervention.log, l); + cp[l] = '\0'; ctx->last_intervention_log.data = cp; ctx->last_intervention_log.len = l; + } else { + if (intervention.log != NULL) { + free(intervention.log); + } + return NGX_HTTP_INTERNAL_SERVER_ERROR; } } From e2ede45d540197ed4126856fca72052982cc4032 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Wed, 6 May 2026 05:44:57 +0200 Subject: [PATCH 07/22] docs: add bilingual phase4 handling and config examples --- docs/examples/phase4-content-types.conf | 5 + docs/examples/phase4-minimal.conf | 14 ++ docs/examples/phase4-safe.conf | 14 ++ docs/examples/phase4-strict.conf | 14 ++ docs/phase4-handling.de.md | 162 ++++++++++++++++++++++++ docs/phase4-handling.en.md | 162 ++++++++++++++++++++++++ 6 files changed, 371 insertions(+) create mode 100644 docs/examples/phase4-content-types.conf create mode 100644 docs/examples/phase4-minimal.conf create mode 100644 docs/examples/phase4-safe.conf create mode 100644 docs/examples/phase4-strict.conf create mode 100644 docs/phase4-handling.de.md create mode 100644 docs/phase4-handling.en.md diff --git a/docs/examples/phase4-content-types.conf b/docs/examples/phase4-content-types.conf new file mode 100644 index 00000000..4bf8701b --- /dev/null +++ b/docs/examples/phase4-content-types.conf @@ -0,0 +1,5 @@ +# One content type per line; comments are allowed. +text/html +application/json +application/problem+json +text/plain diff --git a/docs/examples/phase4-minimal.conf b/docs/examples/phase4-minimal.conf new file mode 100644 index 00000000..35d78ce8 --- /dev/null +++ b/docs/examples/phase4-minimal.conf @@ -0,0 +1,14 @@ +# Example: minimal mode +location /phase4 { + modsecurity on; + modsecurity_phase4_mode minimal; + modsecurity_phase4_log /var/log/nginx/modsecurity-phase4.log; + + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx sensitive-marker" "id:940001,phase:4,deny,log,status:403" + '; + + proxy_pass http://backend; +} diff --git a/docs/examples/phase4-safe.conf b/docs/examples/phase4-safe.conf new file mode 100644 index 00000000..851a09ea --- /dev/null +++ b/docs/examples/phase4-safe.conf @@ -0,0 +1,14 @@ +# Example: safe mode +location /phase4 { + modsecurity on; + modsecurity_phase4_mode safe; + modsecurity_phase4_log /var/log/nginx/modsecurity-phase4.log; + + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx sensitive-marker" "id:940002,phase:4,deny,log,status:403" + '; + + proxy_pass http://backend; +} diff --git a/docs/examples/phase4-strict.conf b/docs/examples/phase4-strict.conf new file mode 100644 index 00000000..80d6c882 --- /dev/null +++ b/docs/examples/phase4-strict.conf @@ -0,0 +1,14 @@ +# Example: strict mode +location /phase4 { + modsecurity on; + modsecurity_phase4_mode strict; + modsecurity_phase4_log /var/log/nginx/modsecurity-phase4.log; + + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx sensitive-marker" "id:940003,phase:4,deny,log,status:403" + '; + + proxy_pass http://backend; +} diff --git a/docs/phase4-handling.de.md b/docs/phase4-handling.de.md new file mode 100644 index 00000000..1cb42cd7 --- /dev/null +++ b/docs/phase4-handling.de.md @@ -0,0 +1,162 @@ +# ModSecurity-nginx: Phase-4-Handling (Deutsch) + +## Einführung: Was ist das `phase:4`-Problem? + +`phase:4`-Regeln laufen während der Verarbeitung des **Response-Bodys**. In nginx kann zu diesem Zeitpunkt der Response-Header bereits an den Client gesendet worden sein. Wenn der Header bereits gesendet wurde, kann nginx den HTTP-Status (z. B. 403) oder Redirect-Header nicht mehr zuverlässig umstellen. + +In diesem Repository ist das explizit durch neue Phase-4-Mechanismen adressiert, inklusive Modi und strukturiertem Logging. + +## Warum „Header bereits gesendet“ problematisch ist + +Wenn eine ModSecurity-Intervention (`deny`, `status`, `redirect`) erst in `phase:4` auftritt, ist ein klassisches Blocken/Redirect nur möglich, solange Header noch nicht gesendet wurden. + +Wenn Header bereits gesendet sind: + +- ein späteres `status:401/403` ist nicht mehr sauber als HTTP-Status umsetzbar, +- ein späteres `redirect` (301/302 + `Location`) ist nicht mehr sauber als Redirect umsetzbar, +- nur degradierte Reaktion ist möglich (loggen, ggf. Verbindung abbrechen). + +## Warum **kein globales Response-Body-Buffering** genutzt wird + +Der implementierte Ansatz vermeidet globales Buffering großer Response-Bodies. Das ist konsistent mit folgenden Zielen: + +- kein pauschaler Speicher-/Latenz-Overhead für alle Responses, +- keine zusätzliche Reordering-Logik zwischen bereits gesendeten Headern und späteren Body-Entscheidungen, +- keine Schein-Garantie, dass `phase:4` immer in saubere HTTP-Block-/Redirect-Semantik überführt werden kann. + +## Neue Directives + +### `modsecurity_phase4_mode` + +Unterstützte Werte: + +- `minimal` +- `safe` +- `strict` + +Ungültige Werte werden in der Konfiguration abgelehnt. + +### `modsecurity_phase4_content_types_file` + +Lädt Content-Types aus einer Datei (eine Zeile pro Typ, `#`-Kommentare erlaubt). Einträge werden validiert; ungültige Einträge führen zu Konfigurationsfehlern. + +Wenn keine Datei gesetzt ist, werden Standardtypen verwendet. + +### `modsecurity_phase4_log` + +Aktiviert ein separates JSON-Line-Log für Phase-4-Interventionen. + +## Modi: Verhalten und Sicherheitsprofil + +## `minimal` + +- Bei `phase:4`-Intervention nach gesendeten Headern: **kein künstliches Blocken**, nur `log_only`. +- Ziel: minimalinvasiv, kein Verbindungsabbruch. + +## `safe` + +- Verhalten bei spät erkannter Intervention ebenfalls `log_only`. +- Ziel: stabiler Betrieb ohne erzwungenen Abbruch. + +## `strict` + +- Bei `phase:4`-Intervention nach gesendeten Headern: `connection_abort`. +- Ziel: striktere Reaktion, wenn keine saubere Status-/Redirect-Änderung mehr möglich ist. + +> Wichtig: `strict` garantiert **keinen** nachträglichen 401/403/301/302-Status; es kann stattdessen zum Verbindungsabbruch kommen. + +## Verhalten nach Header-Status + +### Header **noch nicht** gesendet + +Normale ModSecurity-Interventionspfade bleiben möglich (z. B. `deny_status`), weil nginx den Header noch anpassen kann. + +### Header **bereits** gesendet + +- `minimal`/`safe`: `log_only` +- `strict`: `connection_abort` + +## Erklärung der Actions + +### `connection_abort` + +- Implementierter Fallback im `strict`-Modus bei spät erkannter `phase:4`-Intervention. +- Technisch: Request wird mit Fehlerpfad beendet (keine nachträgliche Header-Umschreibung). + +### `log_only` + +- Intervention wird protokolliert, aber Response-Fluss bleibt (so weit möglich) bestehen. +- Dient Nachvollziehbarkeit ohne erzwungenen Transportabbruch. + +## Content-Type-Scoping (`modsecurity_phase4_content_types_file`) + +Phase-4-Handling wird über Content-Type eingeschränkt. Wenn `Content-Type` fehlt oder nicht in Scope ist, wird statt harter Aktion protokolliert (`log_only`, z. B. Grund `content_type_missing` / `content_type_not_in_scope`). + +Das reduziert unerwartete Effekte auf nicht-zielgerichtete Antworttypen. + +## Logging + +### `modsecurity_phase4_log` + +Schreibt JSON-Zeilen mit Feldern wie: + +- `event` (`phase4_intervention`) +- `uri`, `method` +- `response_status`, `waf_status` +- `content_type` +- `header_sent` (Boolean) +- `mode` +- `wanted_action`, `actual_action` +- `reason` +- `intervention` +- `rule_id` + +### `nginx error.log` + +Zusätzlich wird insbesondere im `strict`-Pfad eine Warnung ins nginx-Error-Log geschrieben, wenn nach gesendeten Headern eine Intervention eintritt. + +## Sicherheitsaspekte (Implementierungsentscheidungen) + +- **Keine Response-Bodies im Phase-4-Log**: reduziert Risiko von Datenabfluss über Logs. +- **Keine `ngx_chain_t`-Rewrites**: vermeidet komplexe, fehleranfällige Manipulation bereits laufender Body-Ketten. +- **Keine künstliche Reordering-Logik**: verhindert inkonsistente Zustände zwischen bereits gesendeten Headern und späteren Blockentscheidungen. +- **`strict` kann Verbindung abbrechen**: bewusster Trade-off für „fail closed“-ähnliche Reaktion ohne falsche HTTP-Status-Versprechen. + +## Beispielkonfigurationen + +Siehe: + +- `docs/examples/phase4-minimal.conf` +- `docs/examples/phase4-safe.conf` +- `docs/examples/phase4-strict.conf` +- `docs/examples/phase4-content-types.conf` + +## JSON-Log-Beispiele + +`log_only` (z. B. `safe`): + +```json +{"event":"phase4_intervention","uri":"/phase4","method":"GET","response_status":200,"waf_status":403,"content_type":"text/html","header_sent":true,"mode":"safe","wanted_action":"deny","actual_action":"log_only","reason":"mode_safe","intervention":"...","rule_id":"910002"} +``` + +`connection_abort` (`strict`): + +```json +{"event":"phase4_intervention","uri":"/phase4","method":"GET","response_status":200,"waf_status":403,"content_type":"text/html","header_sent":true,"mode":"strict","wanted_action":"deny","actual_action":"connection_abort","reason":"headers_already_sent","intervention":"...","rule_id":"910003"} +``` + +> Feldwerte hängen vom konkreten Request/Rule-Match ab. + +## Einschränkungen / bekannte Grenzen + +- `phase:4` kann **nicht** garantieren, dass ein gewünschter Block-/Redirect-Status noch als HTTP-Status beim Client ankommt. +- Bei bereits gesendeten Headern ist nur degradierte Reaktion möglich (`log_only` oder `connection_abort`). +- Content-Type-Scoping ist entscheidend; Out-of-Scope-Responses werden nicht „hart“ behandelt. + +## Hinweise für Betreiber + +1. `phase:4` nicht als alleinigen Mechanismus für harte Zugriffskontrolle planen. +2. Für harte Block-/Redirect-Entscheidungen möglichst frühere Phasen bevorzugen. +3. `modsecurity_phase4_log` aktivieren und regelmäßig auswerten. +4. `modsecurity_phase4_content_types_file` bewusst auf sensible MIME-Typen begrenzen. +5. `strict` nur einsetzen, wenn Verbindungsabbrüche betrieblich akzeptabel sind. diff --git a/docs/phase4-handling.en.md b/docs/phase4-handling.en.md new file mode 100644 index 00000000..600fcbcb --- /dev/null +++ b/docs/phase4-handling.en.md @@ -0,0 +1,162 @@ +# ModSecurity-nginx: Phase 4 Handling (English) + +## Introduction: what is the `phase:4` problem? + +`phase:4` rules run while processing the **response body**. In nginx, response headers may already have been sent to the client at that point. Once headers are sent, nginx cannot reliably switch HTTP status (for example to 403) or inject redirect headers anymore. + +This repository adds explicit Phase 4 handling for that condition, including dedicated modes and structured logging. + +## Why “headers already sent” is a problem + +If a ModSecurity intervention (`deny`, `status`, `redirect`) is triggered in `phase:4`, classic blocking/redirecting only works when headers are still unsent. + +If headers are already sent: + +- late `status:401/403` cannot be cleanly enforced as HTTP status, +- late redirect (`301/302` + `Location`) cannot be cleanly enforced, +- only degraded reactions remain (logging, optionally aborting the connection). + +## Why there is **no global response-body buffering** + +The implementation intentionally avoids global buffering of large response bodies. This aligns with: + +- no blanket memory/latency overhead for all responses, +- no extra reordering logic between already-sent headers and late body-time decisions, +- no false guarantee that `phase:4` can always be translated to clean HTTP block/redirect semantics. + +## New directives + +### `modsecurity_phase4_mode` + +Supported values: + +- `minimal` +- `safe` +- `strict` + +Invalid values are rejected at config parsing time. + +### `modsecurity_phase4_content_types_file` + +Loads content types from a file (one type per line, `#` comments supported). Entries are validated; invalid entries fail configuration loading. + +If not set, built-in default content types are used. + +### `modsecurity_phase4_log` + +Enables a dedicated JSON-lines log for Phase 4 interventions. + +## Modes: behavior and safety profile + +## `minimal` + +- For `phase:4` interventions after headers are sent: no synthetic deny, `log_only`. +- Goal: least intrusive behavior, no forced disconnect. + +## `safe` + +- For late interventions, behavior is also `log_only`. +- Goal: operational stability without forced abort. + +## `strict` + +- For `phase:4` interventions after headers are sent: `connection_abort`. +- Goal: stricter handling when clean status/redirect changes are no longer possible. + +> Important: `strict` does **not** guarantee a retroactive 401/403/301/302; it may terminate the connection instead. + +## Behavior by header state + +### Headers **not sent yet** + +Normal ModSecurity intervention paths remain possible (for example `deny_status`), because nginx can still adjust outgoing headers. + +### Headers **already sent** + +- `minimal`/`safe`: `log_only` +- `strict`: `connection_abort` + +## Action semantics + +### `connection_abort` + +- Implemented fallback in `strict` mode for late `phase:4` interventions. +- Technical effect: request terminates via error path (no post-hoc header rewrite). + +### `log_only` + +- Intervention is recorded, while response flow continues as far as possible. +- Provides traceability without forced transport interruption. + +## Content-Type scoping (`modsecurity_phase4_content_types_file`) + +Phase 4 handling is scoped by content type. If `Content-Type` is missing or not in scope, the module logs a degraded action (`log_only`, with reasons such as `content_type_missing` / `content_type_not_in_scope`) instead of enforcing a hard action. + +This limits side effects on non-targeted response types. + +## Logging + +### `modsecurity_phase4_log` + +Writes JSON lines with fields such as: + +- `event` (`phase4_intervention`) +- `uri`, `method` +- `response_status`, `waf_status` +- `content_type` +- `header_sent` (boolean) +- `mode` +- `wanted_action`, `actual_action` +- `reason` +- `intervention` +- `rule_id` + +### nginx `error.log` + +Additionally, especially on the `strict` path, a warning is emitted to nginx error log when an intervention occurs after headers are sent. + +## Security decisions (implementation) + +- **No response body in Phase 4 log**: reduces leakage risk through logs. +- **No `ngx_chain_t` rewriting**: avoids brittle low-level mutations of already flowing body chains. +- **No synthetic reordering logic**: avoids inconsistent states between sent headers and late decisions. +- **`strict` may abort connections**: deliberate trade-off for stricter fail behavior without false HTTP-status guarantees. + +## Example configurations + +See: + +- `docs/examples/phase4-minimal.conf` +- `docs/examples/phase4-safe.conf` +- `docs/examples/phase4-strict.conf` +- `docs/examples/phase4-content-types.conf` + +## JSON log examples + +`log_only` (for example `safe`): + +```json +{"event":"phase4_intervention","uri":"/phase4","method":"GET","response_status":200,"waf_status":403,"content_type":"text/html","header_sent":true,"mode":"safe","wanted_action":"deny","actual_action":"log_only","reason":"mode_safe","intervention":"...","rule_id":"910002"} +``` + +`connection_abort` (`strict`): + +```json +{"event":"phase4_intervention","uri":"/phase4","method":"GET","response_status":200,"waf_status":403,"content_type":"text/html","header_sent":true,"mode":"strict","wanted_action":"deny","actual_action":"connection_abort","reason":"headers_already_sent","intervention":"...","rule_id":"910003"} +``` + +> Field values depend on the actual request and matched rule. + +## Limitations / known boundaries + +- `phase:4` cannot **guarantee** that the desired block/redirect status still reaches the client as HTTP status. +- When headers are already sent, only degraded handling is possible (`log_only` or `connection_abort`). +- Content-type scoping is critical; out-of-scope responses are not hard-enforced. + +## Operator guidance + +1. Do not treat `phase:4` as the sole hard access-control mechanism. +2. Prefer earlier phases for hard block/redirect decisions. +3. Enable and monitor `modsecurity_phase4_log`. +4. Keep `modsecurity_phase4_content_types_file` narrowly scoped to sensitive MIME types. +5. Use `strict` only if connection aborts are operationally acceptable. From 2e2c79203ce829f236a4cbcc46b66292e0db4aee Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Wed, 6 May 2026 08:55:18 +0200 Subject: [PATCH 08/22] Fix json escape alloc safety and phase4 modes log-line assertions --- src/ngx_http_modsecurity_body_filter.c | 9 +++++++-- src/ngx_http_modsecurity_module.c | 11 ++++++++--- tests/modsecurity-phase4-content-types.t | 6 +++++- tests/modsecurity-phase4-invalid-config.t | 2 +- tests/modsecurity-phase4-modes.t | 7 +++++-- tests/modsecurity-proxy.t | 9 +++++++++ 6 files changed, 35 insertions(+), 9 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 9b2fefd3..176cdcc6 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -329,7 +329,7 @@ ngx_http_modsecurity_json_escape(ngx_pool_t *pool, ngx_str_t *src, ngx_str_t *ds size_t i, extra = 0; u_char *d; if (src == NULL || src->data == NULL) { dst->len=0; dst->data=(u_char*)""; return; } for (i = 0; i < src->len; i++) if (src->data[i] < 0x20 || src->data[i] == '"' || src->data[i] == '\\') extra++; - dst->data = ngx_pnalloc(pool, src->len + extra + 1); if (dst->data == NULL) { dst->len=0; return; } + dst->data = ngx_pnalloc(pool, src->len + extra + 1); if (dst->data == NULL) { dst->len=0; dst->data=(u_char*)""; return; } d = dst->data; for (i = 0; i < src->len; i++) { u_char c = src->data[i]; @@ -354,7 +354,12 @@ ngx_http_modsecurity_extract_rule_id(ngx_pool_t *pool, ngx_str_t *intervention, if (j > i + 4 && j < intervention->len && intervention->data[j] == '"') { rule_id->len = j - (i + 4); rule_id->data = ngx_pnalloc(pool, rule_id->len); - if (rule_id->data != NULL) ngx_memcpy(rule_id->data, intervention->data + i + 4, rule_id->len); + if (rule_id->data == NULL) { + rule_id->len = 0; + rule_id->data = (u_char *)""; + return; + } + ngx_memcpy(rule_id->data, intervention->data + i + 4, rule_id->len); return; } } diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 1fbbf96c..22561485 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -545,13 +545,18 @@ ngx_conf_set_phase4_log(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) static ngx_int_t ngx_http_modsecurity_phase4_validate_content_type(u_char *s, size_t len) { - size_t i; + size_t i, slash = (size_t)-1; if (len == 0 || ngx_strchr(s, '*') != NULL) return NGX_ERROR; for (i = 0; i < len; i++) { u_char c = s[i]; - if (!(isalnum((unsigned char)c) || c == '/' || c == '.' || c == '+' || c == '-' )) return NGX_ERROR; + if (c == '/') { + if (slash != (size_t)-1 || i == 0 || i + 1 >= len) return NGX_ERROR; + slash = i; + continue; + } + if (!(isalnum((unsigned char)c) || c == '-' || c == '.' || c == '_' || c == '+')) return NGX_ERROR; } - return (ngx_strchr(s, '/') != NULL) ? NGX_OK : NGX_ERROR; + return (slash != (size_t)-1) ? NGX_OK : NGX_ERROR; } static char * diff --git a/tests/modsecurity-phase4-content-types.t b/tests/modsecurity-phase4-content-types.t index d59e4553..9ce9c35d 100644 --- a/tests/modsecurity-phase4-content-types.t +++ b/tests/modsecurity-phase4-content-types.t @@ -15,6 +15,9 @@ Application/JSON; charset=utf-8 # inline comment TEXT/HTML text/plain application/xml +application/vnd.api+json +application/problem+json +application/x.custom_type CT $t->write_file_expand('nginx.conf', <<'EOC'); @@ -75,7 +78,7 @@ $t->write_file('/unknown', 'HIT UNKNOWN'); $t->write_file('/emptytype', 'HIT EMPTY'); $t->run(); -$t->plan(8); +$t->plan(9); is(http_get('/json'), '', 'json in-scope + strict => abort after headers sent'); like(http_get('/unknown'), qr/HIT UNKNOWN/, 'unknown content-type not in scope => no hard action'); @@ -87,3 +90,4 @@ like($log, qr/"actual_action":"connection_abort"/, 'strict in-scope abort logged like($log, qr/"reason":"content_type_not_in_scope"/, 'out-of-scope reason logged'); like($log, qr/"event":"phase4_intervention"/, 'json lines event present'); unlike($log, qr/HIT JSON|HIT UNKNOWN|HIT EMPTY/, 'no response body data in log'); +like($log, qr/"content_type":"application\/json"/, 'application/json remains valid'); diff --git a/tests/modsecurity-phase4-invalid-config.t b/tests/modsecurity-phase4-invalid-config.t index 775c2be8..176274a5 100644 --- a/tests/modsecurity-phase4-invalid-config.t +++ b/tests/modsecurity-phase4-invalid-config.t @@ -7,7 +7,7 @@ use Test::Nginx; # this test verifies parser rejects invalid entries/wildcards at config load my $t = Test::Nginx->new()->has(qw/http/); -$t->write_file('phase4-invalid.conf', "text/*\napplication/json\ninvalid type\n"); +$t->write_file('phase4-invalid.conf', "text/*\n*/json\ntext/html bad\napplication/problem+json\n"); $t->write_file_expand('nginx.conf', <<'EOC'); %%TEST_GLOBALS%% diff --git a/tests/modsecurity-phase4-modes.t b/tests/modsecurity-phase4-modes.t index 6462facc..859a1f20 100644 --- a/tests/modsecurity-phase4-modes.t +++ b/tests/modsecurity-phase4-modes.t @@ -59,7 +59,7 @@ $t->write_file('/m', 'Hello minimal'); $t->write_file('/s', 'Hello safe'); $t->write_file('/x', 'Hello strict'); $t->run(); -$t->plan(10); +$t->plan(12); like(http_get('/m'), qr/Hello minimal/, 'minimal no fake deny'); like(http_get('/s'), qr/Hello safe/, 'safe no fake deny'); @@ -71,5 +71,8 @@ like($log, qr/"reason":"mode_safe"|"reason":"headers_already_sent"/, 'safe reaso like($log, qr/"actual_action":"connection_abort"/, 'strict action logged'); like($log, qr/"event":"phase4_intervention"/, 'event field present'); like($log, qr/"header_sent":true/, 'json boolean header_sent'); -unlike($log, qr/\n.*\n/s, 'single-line json entries'); unlike($log, qr/Hello minimal|Hello safe|Hello strict/, 'no response body data in phase4 log'); + +my @lines = grep { length $_ } split /\n/, $log; +ok(@lines >= 1, 'phase4 log has one or more json lines'); +ok(!grep { $_ !~ /^\{.*\}$/ } @lines, 'each log line is a single JSON object line'); diff --git a/tests/modsecurity-proxy.t b/tests/modsecurity-proxy.t index 38c27d93..a742029f 100644 --- a/tests/modsecurity-proxy.t +++ b/tests/modsecurity-proxy.t @@ -192,6 +192,15 @@ EOF select undef, undef, undef, 0.1; print $client 'AND-THIS'; + } elsif ($uri =~ m{^/phase4\?what=(redirect301|redirect302|block401|block403)$}) { + print $client <<"EOF"; +HTTP/1.1 200 OK +Content-Type: text/html +Connection: close + +phase4 trigger +EOF + } else { print $client <<"EOF"; From d73f9eab3fc8b7f12d68c25f0f6a0df07ac84a4f Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Wed, 6 May 2026 13:16:18 +0200 Subject: [PATCH 09/22] Fix phase4-modes test plan count mismatch --- tests/modsecurity-phase4-modes.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/modsecurity-phase4-modes.t b/tests/modsecurity-phase4-modes.t index 859a1f20..6763d177 100644 --- a/tests/modsecurity-phase4-modes.t +++ b/tests/modsecurity-phase4-modes.t @@ -59,7 +59,7 @@ $t->write_file('/m', 'Hello minimal'); $t->write_file('/s', 'Hello safe'); $t->write_file('/x', 'Hello strict'); $t->run(); -$t->plan(12); +$t->plan(11); like(http_get('/m'), qr/Hello minimal/, 'minimal no fake deny'); like(http_get('/s'), qr/Hello safe/, 'safe no fake deny'); From a7bb6e4119c540bee39c28b58d70aee3c1a24339 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Wed, 6 May 2026 13:24:23 +0200 Subject: [PATCH 10/22] docs: expand phase4 guidance and generalize production examples --- docs/examples/phase4-content-types.conf | 15 +- docs/examples/phase4-minimal.conf | 43 +++-- docs/examples/phase4-safe.conf | 41 ++-- docs/examples/phase4-strict.conf | 41 ++-- docs/phase4-handling.de.md | 245 ++++++++++++++++-------- docs/phase4-handling.en.md | 245 ++++++++++++++++-------- 6 files changed, 433 insertions(+), 197 deletions(-) diff --git a/docs/examples/phase4-content-types.conf b/docs/examples/phase4-content-types.conf index 4bf8701b..5baa48ba 100644 --- a/docs/examples/phase4-content-types.conf +++ b/docs/examples/phase4-content-types.conf @@ -1,5 +1,16 @@ -# One content type per line; comments are allowed. +# phase4-content-types.conf +# +# Format rules: +# 1) Exactly one MIME type per line. +# 2) Lines starting with # are comments. +# 3) Wildcards are not supported (e.g. text/* is invalid). +# 4) Parameters (e.g. "; charset=utf-8") do not need separate entries. +# The module normalizes response Content-Type and compares the base type. +# 5) Keep this list focused on body types you actually want to scope. + text/html +text/plain application/json application/problem+json -text/plain +application/xml +application/xhtml+xml diff --git a/docs/examples/phase4-minimal.conf b/docs/examples/phase4-minimal.conf index 35d78ce8..6d3250e8 100644 --- a/docs/examples/phase4-minimal.conf +++ b/docs/examples/phase4-minimal.conf @@ -1,14 +1,31 @@ -# Example: minimal mode -location /phase4 { - modsecurity on; - modsecurity_phase4_mode minimal; - modsecurity_phase4_log /var/log/nginx/modsecurity-phase4.log; - - modsecurity_rules ' - SecRuleEngine On - SecResponseBodyAccess On - SecRule RESPONSE_BODY "@rx sensitive-marker" "id:940001,phase:4,deny,log,status:403" - '; - - proxy_pass http://backend; +# Production example: phase-4 mode "minimal" +# Use when response continuity is more important than forced connection aborts +# for late phase:4 interventions (after headers were already sent). + +http { + upstream app_backend { + server 127.0.0.1:8081; + } + + server { + listen 80; + server_name example.local; + + modsecurity on; + modsecurity_phase4_mode minimal; + modsecurity_phase4_log /var/log/nginx/modsecurity-phase4.log; + modsecurity_phase4_content_types_file /etc/modsecurity/phase4-content-types.conf; + + # Inline rule sample for demonstration. + # In production, you can keep using modsecurity_rules_file as usual. + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx sensitive-marker" "id:940101,phase:4,deny,log,status:403" + '; + + location / { + proxy_pass http://app_backend; + } + } } diff --git a/docs/examples/phase4-safe.conf b/docs/examples/phase4-safe.conf index 851a09ea..baaa7a08 100644 --- a/docs/examples/phase4-safe.conf +++ b/docs/examples/phase4-safe.conf @@ -1,14 +1,29 @@ -# Example: safe mode -location /phase4 { - modsecurity on; - modsecurity_phase4_mode safe; - modsecurity_phase4_log /var/log/nginx/modsecurity-phase4.log; - - modsecurity_rules ' - SecRuleEngine On - SecResponseBodyAccess On - SecRule RESPONSE_BODY "@rx sensitive-marker" "id:940002,phase:4,deny,log,status:403" - '; - - proxy_pass http://backend; +# Production example: phase-4 mode "safe" +# Default mode in this module. Recommended baseline if you need phase:4 +# visibility while avoiding forced disconnects on late interventions. + +http { + upstream app_backend { + server 127.0.0.1:8081; + } + + server { + listen 80; + server_name example.local; + + modsecurity on; + modsecurity_phase4_mode safe; + modsecurity_phase4_log /var/log/nginx/modsecurity-phase4.log; + modsecurity_phase4_content_types_file /etc/modsecurity/phase4-content-types.conf; + + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx sensitive-marker" "id:940102,phase:4,deny,log,status:403" + '; + + location / { + proxy_pass http://app_backend; + } + } } diff --git a/docs/examples/phase4-strict.conf b/docs/examples/phase4-strict.conf index 80d6c882..68fb18ec 100644 --- a/docs/examples/phase4-strict.conf +++ b/docs/examples/phase4-strict.conf @@ -1,14 +1,29 @@ -# Example: strict mode -location /phase4 { - modsecurity on; - modsecurity_phase4_mode strict; - modsecurity_phase4_log /var/log/nginx/modsecurity-phase4.log; - - modsecurity_rules ' - SecRuleEngine On - SecResponseBodyAccess On - SecRule RESPONSE_BODY "@rx sensitive-marker" "id:940003,phase:4,deny,log,status:403" - '; - - proxy_pass http://backend; +# Production example: phase-4 mode "strict" +# Use only if connection aborts are operationally acceptable, because +# late phase:4 interventions may terminate the connection. + +http { + upstream app_backend { + server 127.0.0.1:8081; + } + + server { + listen 80; + server_name example.local; + + modsecurity on; + modsecurity_phase4_mode strict; + modsecurity_phase4_log /var/log/nginx/modsecurity-phase4.log; + modsecurity_phase4_content_types_file /etc/modsecurity/phase4-content-types.conf; + + modsecurity_rules ' + SecRuleEngine On + SecResponseBodyAccess On + SecRule RESPONSE_BODY "@rx sensitive-marker" "id:940103,phase:4,deny,log,status:403" + '; + + location / { + proxy_pass http://app_backend; + } + } } diff --git a/docs/phase4-handling.de.md b/docs/phase4-handling.de.md index 1cb42cd7..78799869 100644 --- a/docs/phase4-handling.de.md +++ b/docs/phase4-handling.de.md @@ -1,32 +1,62 @@ # ModSecurity-nginx: Phase-4-Handling (Deutsch) -## Einführung: Was ist das `phase:4`-Problem? +## Zweck dieses Dokuments -`phase:4`-Regeln laufen während der Verarbeitung des **Response-Bodys**. In nginx kann zu diesem Zeitpunkt der Response-Header bereits an den Client gesendet worden sein. Wenn der Header bereits gesendet wurde, kann nginx den HTTP-Status (z. B. 403) oder Redirect-Header nicht mehr zuverlässig umstellen. +Dieses Dokument beschreibt die im aktuellen Code implementierte Phase-4-Behandlung im nginx-Modul, inklusive: -In diesem Repository ist das explizit durch neue Phase-4-Mechanismen adressiert, inklusive Modi und strukturiertem Logging. +- technischer Grenzen bei bereits gesendeten Headern, +- Verhalten der Modi `minimal`, `safe`, `strict`, +- Content-Type-Scoping, +- Logging (`modsecurity_phase4_log`) und Sicherheitsaspekte, +- Abgrenzung zwischen **Produktionskonfiguration** und **Test-/Demo-Verhalten**. -## Warum „Header bereits gesendet“ problematisch ist +Es werden nur Aussagen getroffen, die durch den aktuellen Repository-Stand belegt sind. -Wenn eine ModSecurity-Intervention (`deny`, `status`, `redirect`) erst in `phase:4` auftritt, ist ein klassisches Blocken/Redirect nur möglich, solange Header noch nicht gesendet wurden. +--- -Wenn Header bereits gesendet sind: +## 1) Hintergrund: Request- vs. Response-Phasen -- ein späteres `status:401/403` ist nicht mehr sauber als HTTP-Status umsetzbar, -- ein späteres `redirect` (301/302 + `Location`) ist nicht mehr sauber als Redirect umsetzbar, -- nur degradierte Reaktion ist möglich (loggen, ggf. Verbindung abbrechen). +ModSecurity-Regeln laufen in unterschiedlichen Phasen der Transaktion. -## Warum **kein globales Response-Body-Buffering** genutzt wird +- Frühe Phasen (z. B. Request-Phasen) treffen Entscheidungen, bevor eine Antwort gesendet wird. +- `phase:4` gehört zur **Response-Body-Verarbeitung**. -Der implementierte Ansatz vermeidet globales Buffering großer Response-Bodies. Das ist konsistent mit folgenden Zielen: +Das ist ein zentraler Unterschied: In `phase:4` kann nginx bereits begonnen haben, Header und/oder Body zu senden. -- kein pauschaler Speicher-/Latenz-Overhead für alle Responses, -- keine zusätzliche Reordering-Logik zwischen bereits gesendeten Headern und späteren Body-Entscheidungen, -- keine Schein-Garantie, dass `phase:4` immer in saubere HTTP-Block-/Redirect-Semantik überführt werden kann. +### Warum ist das relevant? -## Neue Directives +Wenn eine Regel in `phase:4` eine Intervention wie `deny` oder `redirect` auslöst, ist ein sauberer Wechsel auf neuen HTTP-Status nur möglich, solange Header noch nicht final gesendet wurden. -### `modsecurity_phase4_mode` +--- + +## 2) Was `phase:4` praktisch bedeutet + +`phase:4`-Regeln prüfen den Antwortinhalt (Response-Body). Das ist nützlich, wenn Sicherheitskriterien erst am Antwortinhalt erkennbar sind. + +Gleichzeitig erzeugt es eine harte technische Grenze: + +- **Vor Header-Versand**: Status/Redirect kann noch sauber gesetzt werden. +- **Nach Header-Versand**: Status/Redirect lässt sich nicht zuverlässig „nachträglich“ korrigieren. + +Daher existiert im Modul ein dediziertes Handling für späte Interventionen. + +--- + +## 3) Warum Header in `phase:4` bereits gesendet sein können + +In nginx werden Antworten als Stream verarbeitet. Je nach Upstream-, Filter- und Buffering-Verhalten kann der Header bereits auf dem Socket sein, bevor die vollständige Body-Prüfung abgeschlossen ist. + +Folge: Ein in `phase:4` erkanntes `status:403` oder `redirect:302` kann nicht garantiert als sauberer HTTP-Status beim Client landen. + +> Keine falsche Garantie: `phase:4` kann nach gesendeten Headern keinen sauberen HTTP-Status mehr garantieren. + +--- + +## 4) Neue Directives im Modul + +## `modsecurity_phase4_mode` + +Konfiguriert das Verhalten bei Phase-4-Interventionen. Unterstützte Werte: @@ -34,129 +64,188 @@ Unterstützte Werte: - `safe` - `strict` -Ungültige Werte werden in der Konfiguration abgelehnt. +Ungültige Werte führen zu Konfigurationsfehlern. + +## `modsecurity_phase4_content_types_file` + +Lädt erlaubte/gescopte Content-Types aus einer Datei. -### `modsecurity_phase4_content_types_file` +- eine Zeile pro Typ, +- Kommentare mit `#`, +- Einträge werden validiert, +- Wildcards (`*`) sind nicht erlaubt. -Lädt Content-Types aus einer Datei (eine Zeile pro Typ, `#`-Kommentare erlaubt). Einträge werden validiert; ungültige Einträge führen zu Konfigurationsfehlern. +Wenn nicht gesetzt, nutzt das Modul Default-Typen. -Wenn keine Datei gesetzt ist, werden Standardtypen verwendet. +## `modsecurity_phase4_log` -### `modsecurity_phase4_log` +Aktiviert dediziertes JSON-Line-Logging für Phase-4-Ereignisse. -Aktiviert ein separates JSON-Line-Log für Phase-4-Interventionen. +--- -## Modi: Verhalten und Sicherheitsprofil +## 5) Modusverhalten (`minimal`, `safe`, `strict`) ## `minimal` -- Bei `phase:4`-Intervention nach gesendeten Headern: **kein künstliches Blocken**, nur `log_only`. -- Ziel: minimalinvasiv, kein Verbindungsabbruch. +Ziel: möglichst wenig Eingriff in laufende Responses. + +Bei Intervention nach gesendeten Headern: + +- Aktion wird auf `log_only` degradiert, +- keine erzwungene Verbindungsbeendigung. + +Sinnvoll, wenn Stabilität und vollständige Antwortauslieferung Vorrang haben. ## `safe` -- Verhalten bei spät erkannter Intervention ebenfalls `log_only`. -- Ziel: stabiler Betrieb ohne erzwungenen Abbruch. +Ziel: konservativer Produktionsstandard (Default-Merge-Verhalten im Modul). + +Bei Intervention nach gesendeten Headern: + +- ebenfalls `log_only`. + +Sinnvoll als Standardmodus, wenn Phase-4-Transparenz gewünscht ist, aber Verbindungsabbrüche vermieden werden sollen. ## `strict` -- Bei `phase:4`-Intervention nach gesendeten Headern: `connection_abort`. -- Ziel: striktere Reaktion, wenn keine saubere Status-/Redirect-Änderung mehr möglich ist. +Ziel: strengere Reaktion, wenn keine saubere Statusänderung mehr möglich ist. + +Bei Intervention nach gesendeten Headern: + +- `connection_abort`. + +### Risiken von `strict` + +- Kann aktive Verbindungen abbrechen. +- Liefert nicht garantiert einen „schönen“ HTTP-Fehlerstatus beim Client. +- Kann je nach Client/Proxy als Transportfehler statt als 4xx/3xx sichtbar werden. -> Wichtig: `strict` garantiert **keinen** nachträglichen 401/403/301/302-Status; es kann stattdessen zum Verbindungsabbruch kommen. +`strict` ist daher nur sinnvoll, wenn diese Nebenwirkungen betrieblich akzeptabel sind. -## Verhalten nach Header-Status +--- -### Header **noch nicht** gesendet +## 6) Verhalten nach Header-Status -Normale ModSecurity-Interventionspfade bleiben möglich (z. B. `deny_status`), weil nginx den Header noch anpassen kann. +## Header noch **nicht** gesendet -### Header **bereits** gesendet +Wenn eine Intervention vor Header-Versand finalisiert wird, bleibt normaler Deny-/Statuspfad möglich (im Code als `deny_status` geloggt). -- `minimal`/`safe`: `log_only` +## Header bereits gesendet + +- `minimal`: `log_only` +- `safe`: `log_only` - `strict`: `connection_abort` -## Erklärung der Actions +Das ist eine bewusste Degradierung, um keine falschen Status-Garantien vorzutäuschen. + +--- + +## 7) Warum **kein globales Response-Body-Buffering** genutzt wird + +Ein globales Buffering aller Responses würde zwar den Eingriffszeitpunkt verschieben, bringt aber technische und betriebliche Kosten: + +- zusätzlicher Speicher- und Latenz-Overhead, +- größere Komplexität für allgemeine Response-Pfade, +- Risiko unbeabsichtigter Nebenwirkungen auf Durchsatz/Stabilität. -### `connection_abort` +Der aktuelle Ansatz versucht stattdessen, late Interventionen transparent und kontrolliert zu behandeln (`log_only` oder `connection_abort`). -- Implementierter Fallback im `strict`-Modus bei spät erkannter `phase:4`-Intervention. -- Technisch: Request wird mit Fehlerpfad beendet (keine nachträgliche Header-Umschreibung). +--- -### `log_only` +## 8) Warum kein `ngx_chain_t`-Reordering/Rewriting genutzt wird -- Intervention wird protokolliert, aber Response-Fluss bleibt (so weit möglich) bestehen. -- Dient Nachvollziehbarkeit ohne erzwungenen Transportabbruch. +Das Modul implementiert **keine** künstliche Reordering-Logik auf bereits laufenden Body-Ketten, um nachträglich „doch noch“ andere Header-/Statussemantik zu erzwingen. -## Content-Type-Scoping (`modsecurity_phase4_content_types_file`) +Begründung: -Phase-4-Handling wird über Content-Type eingeschränkt. Wenn `Content-Type` fehlt oder nicht in Scope ist, wird statt harter Aktion protokolliert (`log_only`, z. B. Grund `content_type_missing` / `content_type_not_in_scope`). +- hohe Komplexität, +- erhöhte Fehleranfälligkeit, +- schwierige Garantien über korrektes Verhalten in allen Filter-/Upstream-Kombinationen. -Das reduziert unerwartete Effekte auf nicht-zielgerichtete Antworttypen. +Die dokumentierte Degradierung ist technisch robuster als scheinbar harte, aber unsichere Nachkorrekturen. -## Logging +--- -### `modsecurity_phase4_log` +## 9) Content-Type-Scoping: Bedeutung und sichere Nutzung -Schreibt JSON-Zeilen mit Feldern wie: +`modsecurity_phase4_content_types_file` begrenzt Phase-4-Sonderbehandlung auf definierte MIME-Typen. + +Wenn `Content-Type` fehlt oder nicht im Scope liegt: + +- Ereignis wird als `log_only` dokumentiert, +- Grund ist typischerweise `content_type_missing` oder `content_type_not_in_scope`. + +### Warum wichtig? + +- reduziert Nebeneffekte auf nicht-zielgerichtete Antworttypen, +- macht Verhalten vorhersagbarer, +- zwingt zu expliziter Auswahl relevanter Content-Typen. + +--- + +## 10) Logging-Format und Sicherheitsgrenzen + +Mit `modsecurity_phase4_log` schreibt das Modul JSON-Zeilen, u. a. mit: - `event` (`phase4_intervention`) - `uri`, `method` - `response_status`, `waf_status` - `content_type` -- `header_sent` (Boolean) +- `header_sent` - `mode` - `wanted_action`, `actual_action` - `reason` - `intervention` - `rule_id` -### `nginx error.log` +Zusätzlich kann im nginx `error.log` ein Hinweis erscheinen (insbesondere im `strict`-Pfad bei Headern, die schon gesendet wurden). -Zusätzlich wird insbesondere im `strict`-Pfad eine Warnung ins nginx-Error-Log geschrieben, wenn nach gesendeten Headern eine Intervention eintritt. +### Sicherheitsgrenze im Logging -## Sicherheitsaspekte (Implementierungsentscheidungen) +Die Tests prüfen explizit, dass keine Response-Body-Inhalte in das Phase-4-Log „durchrutschen“. -- **Keine Response-Bodies im Phase-4-Log**: reduziert Risiko von Datenabfluss über Logs. -- **Keine `ngx_chain_t`-Rewrites**: vermeidet komplexe, fehleranfällige Manipulation bereits laufender Body-Ketten. -- **Keine künstliche Reordering-Logik**: verhindert inkonsistente Zustände zwischen bereits gesendeten Headern und späteren Blockentscheidungen. -- **`strict` kann Verbindung abbrechen**: bewusster Trade-off für „fail closed“-ähnliche Reaktion ohne falsche HTTP-Status-Versprechen. +--- -## Beispielkonfigurationen +## 11) Produktionsbeispiele -Siehe: +Allgemeine (nicht testpfadgebundene) Beispielkonfigurationen: - `docs/examples/phase4-minimal.conf` - `docs/examples/phase4-safe.conf` - `docs/examples/phase4-strict.conf` - `docs/examples/phase4-content-types.conf` -## JSON-Log-Beispiele +Diese Beispiele nutzen `http`/`server`/`location /` und keine test-spezifischen `/phase4`-Pfade. + +--- + +## 12) Test-/Demo-Verhalten (klar getrennt) + +Im Repository existieren Testfälle mit `/phase4`-Pfaden, z. B. in: -`log_only` (z. B. `safe`): +- `tests/modsecurity.t` +- `tests/modsecurity-proxy.t` +- `tests/modsecurity-h2.t` +- `tests/modsecurity-proxy-h2.t` +- `tests/modsecurity-phase4-*.t` -```json -{"event":"phase4_intervention","uri":"/phase4","method":"GET","response_status":200,"waf_status":403,"content_type":"text/html","header_sent":true,"mode":"safe","wanted_action":"deny","actual_action":"log_only","reason":"mode_safe","intervention":"...","rule_id":"910002"} -``` +Diese Pfade sind **Testkontext** und nicht als allgemeine Produktionskonfiguration gedacht. -`connection_abort` (`strict`): +--- -```json -{"event":"phase4_intervention","uri":"/phase4","method":"GET","response_status":200,"waf_status":403,"content_type":"text/html","header_sent":true,"mode":"strict","wanted_action":"deny","actual_action":"connection_abort","reason":"headers_already_sent","intervention":"...","rule_id":"910003"} -``` +## 13) Bekannte Grenzen / keine falschen Versprechen -> Feldwerte hängen vom konkreten Request/Rule-Match ab. +- `phase:4` kann **nicht garantieren**, dass ein gewünschter 301/302/401/403-Status nach Header-Versand noch sauber beim Client ankommt. +- Bei späten Interventionen ist nur degradierte Behandlung möglich (`log_only` oder `connection_abort`). +- `strict` bedeutet nicht „garantierter 403“, sondern kann Verbindungsabbruch bedeuten. -## Einschränkungen / bekannte Grenzen +--- -- `phase:4` kann **nicht** garantieren, dass ein gewünschter Block-/Redirect-Status noch als HTTP-Status beim Client ankommt. -- Bei bereits gesendeten Headern ist nur degradierte Reaktion möglich (`log_only` oder `connection_abort`). -- Content-Type-Scoping ist entscheidend; Out-of-Scope-Responses werden nicht „hart“ behandelt. +## 14) Betriebsleitfaden (kurz) -## Hinweise für Betreiber +1. Harte Block-/Redirect-Entscheidungen möglichst in frühere Phasen legen. +2. `safe` als Ausgangspunkt verwenden, wenn keine klare Notwendigkeit für `strict` besteht. +3. `strict` nur einsetzen, wenn Abbrüche tolerierbar und beobachtbar sind. +4. `modsecurity_phase4_log` aktivieren und auf `actual_action`/`reason` auswerten. +5. Content-Type-Liste eng halten und regelmäßig prüfen. -1. `phase:4` nicht als alleinigen Mechanismus für harte Zugriffskontrolle planen. -2. Für harte Block-/Redirect-Entscheidungen möglichst frühere Phasen bevorzugen. -3. `modsecurity_phase4_log` aktivieren und regelmäßig auswerten. -4. `modsecurity_phase4_content_types_file` bewusst auf sensible MIME-Typen begrenzen. -5. `strict` nur einsetzen, wenn Verbindungsabbrüche betrieblich akzeptabel sind. diff --git a/docs/phase4-handling.en.md b/docs/phase4-handling.en.md index 600fcbcb..7e5e1a69 100644 --- a/docs/phase4-handling.en.md +++ b/docs/phase4-handling.en.md @@ -1,32 +1,62 @@ # ModSecurity-nginx: Phase 4 Handling (English) -## Introduction: what is the `phase:4` problem? +## Purpose of this document -`phase:4` rules run while processing the **response body**. In nginx, response headers may already have been sent to the client at that point. Once headers are sent, nginx cannot reliably switch HTTP status (for example to 403) or inject redirect headers anymore. +This document describes the currently implemented Phase 4 behavior in the nginx module, including: -This repository adds explicit Phase 4 handling for that condition, including dedicated modes and structured logging. +- technical limits once headers are already sent, +- behavior of `minimal`, `safe`, and `strict` modes, +- content-type scoping, +- logging (`modsecurity_phase4_log`) and security boundaries, +- a clear split between **production configuration** and **test/demo behavior**. -## Why “headers already sent” is a problem +Only statements supported by the current repository code and tests are included. -If a ModSecurity intervention (`deny`, `status`, `redirect`) is triggered in `phase:4`, classic blocking/redirecting only works when headers are still unsent. +--- -If headers are already sent: +## 1) Background: request vs response phases -- late `status:401/403` cannot be cleanly enforced as HTTP status, -- late redirect (`301/302` + `Location`) cannot be cleanly enforced, -- only degraded reactions remain (logging, optionally aborting the connection). +ModSecurity rules run in multiple transaction phases. -## Why there is **no global response-body buffering** +- Early phases (for example request phases) make decisions before a response is emitted. +- `phase:4` belongs to **response-body processing**. -The implementation intentionally avoids global buffering of large response bodies. This aligns with: +This difference is critical: during `phase:4`, nginx may already have sent headers and/or part of the body. -- no blanket memory/latency overhead for all responses, -- no extra reordering logic between already-sent headers and late body-time decisions, -- no false guarantee that `phase:4` can always be translated to clean HTTP block/redirect semantics. +### Why this matters -## New directives +If a `phase:4` rule triggers an intervention (`deny`, `status`, `redirect`), a clean status/redirect rewrite is only possible while headers are still unsent. -### `modsecurity_phase4_mode` +--- + +## 2) What `phase:4` means operationally + +`phase:4` rules inspect response body content. This is useful when security signals are visible only in outgoing payload. + +At the same time, this creates a hard technical boundary: + +- **Before headers are sent**: status/redirect can still be applied cleanly. +- **After headers are sent**: status/redirect can no longer be reliably changed. + +Therefore the module includes dedicated late-intervention handling. + +--- + +## 3) Why headers may already be sent in `phase:4` + +nginx processes responses as a stream. Depending on upstream behavior, buffering, and filter timing, headers can already be on the wire before body inspection fully completes. + +Result: a `phase:4` `status:403` or `redirect:302` is not guaranteed to become a clean client-visible HTTP status. + +> No false guarantee: after headers are sent, `phase:4` cannot guarantee a clean HTTP status rewrite. + +--- + +## 4) New directives in this module + +## `modsecurity_phase4_mode` + +Configures phase-4 late-intervention behavior. Supported values: @@ -34,129 +64,188 @@ Supported values: - `safe` - `strict` -Invalid values are rejected at config parsing time. +Invalid values are rejected by configuration parsing. + +## `modsecurity_phase4_content_types_file` + +Loads scoped content types from a file. -### `modsecurity_phase4_content_types_file` +- one MIME type per line, +- `#` comments supported, +- entries are validated, +- wildcards (`*`) are rejected. -Loads content types from a file (one type per line, `#` comments supported). Entries are validated; invalid entries fail configuration loading. +If unset, module defaults are used. -If not set, built-in default content types are used. +## `modsecurity_phase4_log` -### `modsecurity_phase4_log` +Enables dedicated JSON-lines logging for Phase 4 events. -Enables a dedicated JSON-lines log for Phase 4 interventions. +--- -## Modes: behavior and safety profile +## 5) Mode behavior (`minimal`, `safe`, `strict`) ## `minimal` -- For `phase:4` interventions after headers are sent: no synthetic deny, `log_only`. -- Goal: least intrusive behavior, no forced disconnect. +Goal: least intrusive behavior. + +For interventions after headers were sent: + +- action is downgraded to `log_only`, +- no forced connection termination. + +Use when delivery continuity is prioritized. ## `safe` -- For late interventions, behavior is also `log_only`. -- Goal: operational stability without forced abort. +Goal: conservative production baseline (module default merge behavior). + +For late interventions: + +- also downgraded to `log_only`. + +Use as default when you want phase:4 visibility without forced disconnect side effects. ## `strict` -- For `phase:4` interventions after headers are sent: `connection_abort`. -- Goal: stricter handling when clean status/redirect changes are no longer possible. +Goal: stricter fallback once clean status rewrite is no longer possible. + +For interventions after headers were sent: + +- `connection_abort`. + +### Risks of `strict` + +- Active connections may terminate. +- Clients/proxies may observe transport interruption rather than a clean 4xx/3xx response. +- It does **not** mean “guaranteed 403/401/301/302”. -> Important: `strict` does **not** guarantee a retroactive 401/403/301/302; it may terminate the connection instead. +Use only when those trade-offs are acceptable. -## Behavior by header state +--- -### Headers **not sent yet** +## 6) Behavior by header state -Normal ModSecurity intervention paths remain possible (for example `deny_status`), because nginx can still adjust outgoing headers. +## Headers **not sent yet** -### Headers **already sent** +If intervention is finalized before header send, normal deny/status behavior remains possible (logged as `deny_status` in code path). -- `minimal`/`safe`: `log_only` +## Headers **already sent** + +- `minimal`: `log_only` +- `safe`: `log_only` - `strict`: `connection_abort` -## Action semantics +This is an intentional downgrade to avoid false status guarantees. + +--- + +## 7) Why there is **no global response-body buffering** + +Global buffering of all responses could delay decision points, but introduces broad costs: + +- additional memory and latency overhead, +- higher complexity in generic response paths, +- increased risk of throughput/stability side effects. -### `connection_abort` +Current implementation instead makes late interventions explicit and controlled (`log_only` or `connection_abort`). -- Implemented fallback in `strict` mode for late `phase:4` interventions. -- Technical effect: request terminates via error path (no post-hoc header rewrite). +--- -### `log_only` +## 8) Why there is no `ngx_chain_t` reordering/rewriting -- Intervention is recorded, while response flow continues as far as possible. -- Provides traceability without forced transport interruption. +The module does **not** implement synthetic reordering of already flowing body chains to force post-hoc status semantics. -## Content-Type scoping (`modsecurity_phase4_content_types_file`) +Reasoning: -Phase 4 handling is scoped by content type. If `Content-Type` is missing or not in scope, the module logs a degraded action (`log_only`, with reasons such as `content_type_missing` / `content_type_not_in_scope`) instead of enforcing a hard action. +- high implementation complexity, +- higher fragility, +- difficult correctness guarantees across all filter/upstream combinations. -This limits side effects on non-targeted response types. +The documented downgrade model is more robust than pretending hard guarantees. -## Logging +--- -### `modsecurity_phase4_log` +## 9) Content-type scoping: meaning and safe usage -Writes JSON lines with fields such as: +`modsecurity_phase4_content_types_file` limits special phase-4 handling to selected MIME types. + +If `Content-Type` is missing or out of scope: + +- action is logged as `log_only`, +- reason is typically `content_type_missing` or `content_type_not_in_scope`. + +### Why this matters + +- reduces side effects on non-target response types, +- improves predictability, +- enforces explicit operator intent. + +--- + +## 10) Logging format and security boundaries + +With `modsecurity_phase4_log`, the module emits JSON lines including fields such as: - `event` (`phase4_intervention`) - `uri`, `method` - `response_status`, `waf_status` - `content_type` -- `header_sent` (boolean) +- `header_sent` - `mode` - `wanted_action`, `actual_action` - `reason` - `intervention` - `rule_id` -### nginx `error.log` +Additionally, nginx `error.log` may contain warnings (especially on strict late-intervention paths). -Additionally, especially on the `strict` path, a warning is emitted to nginx error log when an intervention occurs after headers are sent. +### Logging boundary -## Security decisions (implementation) +Tests explicitly check that response body payload is not leaked into phase-4 log output. -- **No response body in Phase 4 log**: reduces leakage risk through logs. -- **No `ngx_chain_t` rewriting**: avoids brittle low-level mutations of already flowing body chains. -- **No synthetic reordering logic**: avoids inconsistent states between sent headers and late decisions. -- **`strict` may abort connections**: deliberate trade-off for stricter fail behavior without false HTTP-status guarantees. +--- -## Example configurations +## 11) Production examples -See: +General (non test-path-specific) example configurations: - `docs/examples/phase4-minimal.conf` - `docs/examples/phase4-safe.conf` - `docs/examples/phase4-strict.conf` - `docs/examples/phase4-content-types.conf` -## JSON log examples +These use `http` / `server` / `location /` patterns and avoid `/phase4` as a production example path. + +--- + +## 12) Test/demo behavior (explicitly separate) + +Repository tests include `/phase4` endpoints, for example in: -`log_only` (for example `safe`): +- `tests/modsecurity.t` +- `tests/modsecurity-proxy.t` +- `tests/modsecurity-h2.t` +- `tests/modsecurity-proxy-h2.t` +- `tests/modsecurity-phase4-*.t` -```json -{"event":"phase4_intervention","uri":"/phase4","method":"GET","response_status":200,"waf_status":403,"content_type":"text/html","header_sent":true,"mode":"safe","wanted_action":"deny","actual_action":"log_only","reason":"mode_safe","intervention":"...","rule_id":"910002"} -``` +Those paths are **test context**, not generic production guidance. -`connection_abort` (`strict`): +--- -```json -{"event":"phase4_intervention","uri":"/phase4","method":"GET","response_status":200,"waf_status":403,"content_type":"text/html","header_sent":true,"mode":"strict","wanted_action":"deny","actual_action":"connection_abort","reason":"headers_already_sent","intervention":"...","rule_id":"910003"} -``` +## 13) Known limits / no false promises -> Field values depend on the actual request and matched rule. +- `phase:4` cannot **guarantee** clean 301/302/401/403 delivery once headers are already sent. +- For late interventions, only downgraded handling is possible (`log_only` or `connection_abort`). +- `strict` is not “guaranteed block status”; it can mean connection termination. -## Limitations / known boundaries +--- -- `phase:4` cannot **guarantee** that the desired block/redirect status still reaches the client as HTTP status. -- When headers are already sent, only degraded handling is possible (`log_only` or `connection_abort`). -- Content-type scoping is critical; out-of-scope responses are not hard-enforced. +## 14) Operator checklist -## Operator guidance +1. Place hard block/redirect decisions in earlier phases whenever possible. +2. Start with `safe` unless you have a clear need for `strict`. +3. Use `strict` only when connection abort side effects are acceptable. +4. Enable `modsecurity_phase4_log` and monitor `actual_action` + `reason` fields. +5. Keep content-type scope narrow and review it regularly. -1. Do not treat `phase:4` as the sole hard access-control mechanism. -2. Prefer earlier phases for hard block/redirect decisions. -3. Enable and monitor `modsecurity_phase4_log`. -4. Keep `modsecurity_phase4_content_types_file` narrowly scoped to sensitive MIME types. -5. Use `strict` only if connection aborts are operationally acceptable. From 4775f328e8e6c96de227f8ad336a837ea69d0007 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Wed, 6 May 2026 13:26:47 +0200 Subject: [PATCH 11/22] Fix phase4-modes JSON line assertion for CRLF log lines --- tests/modsecurity-phase4-modes.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/modsecurity-phase4-modes.t b/tests/modsecurity-phase4-modes.t index 6763d177..26484db7 100644 --- a/tests/modsecurity-phase4-modes.t +++ b/tests/modsecurity-phase4-modes.t @@ -75,4 +75,4 @@ unlike($log, qr/Hello minimal|Hello safe|Hello strict/, 'no response body data i my @lines = grep { length $_ } split /\n/, $log; ok(@lines >= 1, 'phase4 log has one or more json lines'); -ok(!grep { $_ !~ /^\{.*\}$/ } @lines, 'each log line is a single JSON object line'); +ok(!grep { $_ !~ /^\{.*\}\r?$/ } @lines, 'each log line is a single JSON object line'); From 1f37b84c921569968db88aa5fced219196db4c1a Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Wed, 6 May 2026 19:12:41 +0200 Subject: [PATCH 12/22] Add diagnostics for failing phase4 modes JSON-line assertion --- tests/modsecurity-phase4-modes.t | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/tests/modsecurity-phase4-modes.t b/tests/modsecurity-phase4-modes.t index 26484db7..ff9798e1 100644 --- a/tests/modsecurity-phase4-modes.t +++ b/tests/modsecurity-phase4-modes.t @@ -75,4 +75,9 @@ unlike($log, qr/Hello minimal|Hello safe|Hello strict/, 'no response body data i my @lines = grep { length $_ } split /\n/, $log; ok(@lines >= 1, 'phase4 log has one or more json lines'); -ok(!grep { $_ !~ /^\{.*\}\r?$/ } @lines, 'each log line is a single JSON object line'); +my @bad_lines = grep { $_ !~ /^\{.*\}\r?$/ } @lines; +note('phase4.log lines=' . scalar(@lines)); +if (@bad_lines) { + diag('non-json-lines: ' . join(' || ', map { my $x = $_; $x =~ s/\r/\\r/g; $x } @bad_lines)); +} +ok(scalar(@bad_lines) == 0, 'each log line is a single JSON object line'); From 0e8720e8f293645055f3d10dae061a15719d23e2 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Thu, 7 May 2026 07:50:01 +0200 Subject: [PATCH 13/22] Fix MSVC pointer type warning in ngx_file_info call --- src/ngx_http_modsecurity_module.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 22561485..8a8b0467 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -585,7 +585,7 @@ ngx_http_modsecurity_phase4_load_content_types_file(ngx_conf_t *cf, ngx_http_mod ngx_file_info_t fi; u_char *buf, *p, *line, *end; ssize_t n; - if (ngx_file_info((char *)path->data, &fi) == NGX_FILE_ERROR) { + if (ngx_file_info(path->data, &fi) == NGX_FILE_ERROR) { ngx_conf_log_error(NGX_LOG_EMERG, cf, ngx_errno, "modsecurity_phase4_content_types_file \"%V\" stat() failed", path); return NGX_CONF_ERROR; } From 2a31a5b37be0f6600e4390e1ec92817efc875082 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Wed, 13 May 2026 22:52:30 +0200 Subject: [PATCH 14/22] Refactor phase4 code to address Sonar maintainability issues --- src/ngx_http_modsecurity_body_filter.c | 27 +++++-- src/ngx_http_modsecurity_module.c | 105 +++++++++++++++++++------ 2 files changed, 101 insertions(+), 31 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 176cdcc6..50529c96 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -185,11 +185,12 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) } return ret; } - else if (ret < 0) { + if (ret < 0) { ret = ngx_http_modsecurity_phase4_handle_intervention(r, mcf); - if (ret == NGX_ERROR) return NGX_ERROR; + if (ret == NGX_ERROR) { + return NGX_ERROR; + } return ngx_http_next_body_filter(r, in); - } } } @@ -286,7 +287,8 @@ ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurit static ngx_str_t ngx_http_modsecurity_normalize_content_type(ngx_pool_t *pool, ngx_str_t in) { - ngx_str_t out; size_t i; + ngx_str_t out; + size_t i; u_char *semi; out = in; if (out.data == NULL || out.len == 0) return out; @@ -326,10 +328,21 @@ ngx_http_modsecurity_sanitize_intervention(ngx_pool_t *pool, ngx_str_t in) static void ngx_http_modsecurity_json_escape(ngx_pool_t *pool, ngx_str_t *src, ngx_str_t *dst) { - size_t i, extra = 0; u_char *d; + size_t i; + size_t extra = 0; + u_char *d; if (src == NULL || src->data == NULL) { dst->len=0; dst->data=(u_char*)""; return; } - for (i = 0; i < src->len; i++) if (src->data[i] < 0x20 || src->data[i] == '"' || src->data[i] == '\\') extra++; - dst->data = ngx_pnalloc(pool, src->len + extra + 1); if (dst->data == NULL) { dst->len=0; dst->data=(u_char*)""; return; } + for (i = 0; i < src->len; i++) { + if (src->data[i] < 0x20 || src->data[i] == '"' || src->data[i] == '\\') { + extra++; + } + } + dst->data = ngx_pnalloc(pool, src->len + extra + 1); + if (dst->data == NULL) { + dst->len = 0; + dst->data = (u_char *)""; + return; + } d = dst->data; for (i = 0; i < src->len; i++) { u_char c = src->data[i]; diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 8a8b0467..39c09d95 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -545,7 +545,8 @@ ngx_conf_set_phase4_log(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) static ngx_int_t ngx_http_modsecurity_phase4_validate_content_type(u_char *s, size_t len) { - size_t i, slash = (size_t)-1; + size_t i; + size_t slash = (size_t)-1; if (len == 0 || ngx_strchr(s, '*') != NULL) return NGX_ERROR; for (i = 0; i < len; i++) { u_char c = s[i]; @@ -578,12 +579,69 @@ ngx_http_modsecurity_phase4_set_default_content_types(ngx_conf_t *cf, ngx_http_m return NGX_CONF_OK; } +static ngx_int_t +ngx_http_modsecurity_phase4_push_content_type(ngx_conf_t *cf, ngx_http_modsecurity_conf_t *mcf, + u_char *line, u_char *end, ngx_str_t *path) +{ + ngx_str_t *ct; + + ngx_strlow(line, line, end - line); + if (ngx_http_modsecurity_phase4_validate_content_type(line, end - line) != NGX_OK) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid content-type entry in modsecurity_phase4_content_types_file \"%V\": \"%s\"", path, line); + return NGX_ERROR; + } + + ct = ngx_array_push(mcf->phase4_content_types); + if (ct == NULL) { + return NGX_ERROR; + } + ct->len = end - line; + ct->data = ngx_pnalloc(cf->pool, ct->len); + if (ct->data == NULL) { + return NGX_ERROR; + } + ngx_memcpy(ct->data, line, ct->len); + + return NGX_OK; +} + +static void +ngx_http_modsecurity_phase4_trim_line(u_char **line, u_char **end) +{ + while (*line < *end && isspace((unsigned char)**line)) { + (*line)++; + } + + while (*end > *line && isspace((unsigned char)*((*end) - 1))) { + (*end)--; + } + + **end = '\0'; +} + +static void +ngx_http_modsecurity_phase4_strip_inline_comment(u_char *line) +{ + u_char *hash = (u_char *) ngx_strchr(line, '#'); + u_char *semi = (u_char *) ngx_strchr(line, ';'); + + if (hash && (!semi || hash < semi)) { + *hash = '\0'; + } + if (semi) { + *semi = '\0'; + } +} + static char * ngx_http_modsecurity_phase4_load_content_types_file(ngx_conf_t *cf, ngx_http_modsecurity_conf_t *mcf, ngx_str_t *path) { ngx_file_t file; ngx_file_info_t fi; - u_char *buf, *p, *line, *end; + u_char *buf; + u_char *p; + u_char *line; + u_char *end; ssize_t n; if (ngx_file_info(path->data, &fi) == NGX_FILE_ERROR) { ngx_conf_log_error(NGX_LOG_EMERG, cf, ngx_errno, "modsecurity_phase4_content_types_file \"%V\" stat() failed", path); @@ -592,7 +650,8 @@ ngx_http_modsecurity_phase4_load_content_types_file(ngx_conf_t *cf, ngx_http_mod buf = ngx_pnalloc(cf->pool, ngx_file_size(&fi) + 1); if (buf == NULL) return NGX_CONF_ERROR; ngx_memzero(&file, sizeof(file)); - file.name = *path; file.log = cf->log; + file.name = *path; + file.log = cf->log; file.fd = ngx_open_file(path->data, NGX_FILE_RDONLY, NGX_FILE_OPEN, 0); if (file.fd == NGX_INVALID_FILE) return NGX_CONF_ERROR; n = ngx_read_file(&file, buf, ngx_file_size(&fi), 0); @@ -603,27 +662,25 @@ ngx_http_modsecurity_phase4_load_content_types_file(ngx_conf_t *cf, ngx_http_mod if (mcf->phase4_content_types == NULL) return NGX_CONF_ERROR; for (p = buf, line = buf; p <= buf + n; p++) { if (p == buf + n || *p == '\n' || *p == '\r') { - *p = '\0'; end = p; - while (line < end && isspace((unsigned char)*line)) line++; - while (end > line && isspace((unsigned char)*(end-1))) *(--end) = '\0'; - if (line[0] && line[0] != '#') { - u_char *hash = (u_char *)ngx_strchr(line, '#'); - u_char *semi = (u_char *)ngx_strchr(line, ';'); - if (hash && (!semi || hash < semi)) *hash = '\0'; - if (semi) *semi = '\0'; - end = line + ngx_strlen(line); - while (end > line && isspace((unsigned char)*(end-1))) *(--end) = '\0'; - ngx_strlow(line, line, end - line); - if (ngx_http_modsecurity_phase4_validate_content_type(line, end-line) != NGX_OK) { - ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, "invalid content-type entry in modsecurity_phase4_content_types_file \"%V\": \"%s\"", path, line); - return NGX_CONF_ERROR; - } - ngx_str_t *ct = ngx_array_push(mcf->phase4_content_types); - if (ct == NULL) return NGX_CONF_ERROR; - ct->len = end - line; - ct->data = ngx_pnalloc(cf->pool, ct->len); - if (ct->data == NULL) return NGX_CONF_ERROR; - ngx_memcpy(ct->data, line, ct->len); + *p = '\0'; + end = p; + + ngx_http_modsecurity_phase4_trim_line(&line, &end); + if (line[0] == '\0' || line[0] == '#') { + line = p + 1; + continue; + } + + ngx_http_modsecurity_phase4_strip_inline_comment(line); + end = line + ngx_strlen(line); + ngx_http_modsecurity_phase4_trim_line(&line, &end); + if (line[0] == '\0') { + line = p + 1; + continue; + } + + if (ngx_http_modsecurity_phase4_push_content_type(cf, mcf, line, end, path) != NGX_OK) { + return NGX_CONF_ERROR; } line = p + 1; } From 0d3ad622a001cf243dcf472a67ffd58ebc8a753f Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Wed, 13 May 2026 22:58:36 +0200 Subject: [PATCH 15/22] Flatten processed-chain branch in body filter --- src/ngx_http_modsecurity_body_filter.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 50529c96..7c847dd5 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -168,7 +168,11 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) /* XXX: chain->buf->last_buf || chain->buf->last_in_chain */ is_request_processed = chain->buf->last_buf; - if (is_request_processed) { + if (!is_request_processed) { + continue; + } + + { ngx_pool_t *old_pool; old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool); From 06f99afa29c5f57624b4b12700511edeff64efa9 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Wed, 13 May 2026 23:15:05 +0200 Subject: [PATCH 16/22] Fix remaining SonarQube issues in body filter --- src/ngx_http_modsecurity_body_filter.c | 38 ++++++++++++++++---------- 1 file changed, 23 insertions(+), 15 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 7c847dd5..779211b4 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -254,7 +254,13 @@ static ngx_int_t ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurity_conf_t *mcf, const char *wanted, const char *actual, const char *reason) { u_char *p; - ngx_str_t euri, emethod, ect, elog, erule, raw_log, slog; + ngx_str_t euri; + ngx_str_t emethod; + ngx_str_t ect; + ngx_str_t elog; + ngx_str_t erule; + ngx_str_t raw_log; + ngx_str_t slog; const char *mode = "safe"; const char *header_sent = r->header_sent ? "true" : "false"; ngx_http_modsecurity_ctx_t *ctx = ngx_http_modsecurity_get_module_ctx(r); @@ -365,20 +371,22 @@ ngx_http_modsecurity_extract_rule_id(ngx_pool_t *pool, ngx_str_t *intervention, rule_id->len = 0; if (intervention == NULL || intervention->data == NULL) return; for (i = 0; i + 4 < intervention->len; i++) { - if (ngx_strncasecmp(intervention->data + i, (u_char *)"id \"", 4) == 0) { - size_t j = i + 4; - while (j < intervention->len && intervention->data[j] >= '0' && intervention->data[j] <= '9') j++; - if (j > i + 4 && j < intervention->len && intervention->data[j] == '"') { - rule_id->len = j - (i + 4); - rule_id->data = ngx_pnalloc(pool, rule_id->len); - if (rule_id->data == NULL) { - rule_id->len = 0; - rule_id->data = (u_char *)""; - return; - } - ngx_memcpy(rule_id->data, intervention->data + i + 4, rule_id->len); - return; - } + size_t j; + + if (ngx_strncasecmp(intervention->data + i, (u_char *)"id \"", 4) != 0) continue; + + j = i + 4; + while (j < intervention->len && intervention->data[j] >= '0' && intervention->data[j] <= '9') j++; + if (j <= i + 4 || j >= intervention->len || intervention->data[j] != '"') continue; + + rule_id->len = j - (i + 4); + rule_id->data = ngx_pnalloc(pool, rule_id->len); + if (rule_id->data == NULL) { + rule_id->len = 0; + rule_id->data = (u_char *)""; + return; } + ngx_memcpy(rule_id->data, intervention->data + i + 4, rule_id->len); + return; } } From 505d4af49fbff35769d6eb1c2419a1c57e09f818 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Thu, 14 May 2026 18:48:51 +0200 Subject: [PATCH 17/22] Address Copilot review feedback for phase4 handling --- src/ngx_http_modsecurity_body_filter.c | 17 ++++++++++---- src/ngx_http_modsecurity_common.h | 1 - src/ngx_http_modsecurity_module.c | 27 +++++++++-------------- tests/modsecurity-phase4-content-types.t | 2 +- tests/modsecurity-phase4-invalid-config.t | 5 +++-- tests/modsecurity-phase4-modes.t | 2 +- tests/modsecurity-proxy-h2.t | 9 ++++++++ 7 files changed, 37 insertions(+), 26 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 779211b4..ece49a5c 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -212,21 +212,25 @@ ngx_http_modsecurity_phase4_handle_intervention(ngx_http_request_t *r, ngx_http_ { ngx_http_modsecurity_ctx_t *ctx = ngx_http_modsecurity_get_module_ctx(r); ngx_int_t in_scope = ngx_http_modsecurity_phase4_in_scope(r); + const char *wanted = "deny"; + if (ctx && ctx->last_intervention_status >= 300 && ctx->last_intervention_status < 400) { + wanted = "redirect"; + } if (ctx && ctx->phase4_headers_checked) return NGX_OK; if (ctx) ctx->phase4_headers_checked = 1; if (in_scope == 0) { - ngx_http_modsecurity_phase4_log_event(r, mcf, "deny", "log_only", r->headers_out.content_type.len ? "content_type_not_in_scope" : "content_type_missing"); + ngx_http_modsecurity_phase4_log_event(r, mcf, wanted, "log_only", r->headers_out.content_type.len ? "content_type_not_in_scope" : "content_type_missing"); return NGX_OK; } if (mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_STRICT) { - ngx_http_modsecurity_phase4_log_event(r, mcf, "deny", "connection_abort", "headers_already_sent"); + ngx_http_modsecurity_phase4_log_event(r, mcf, wanted, "connection_abort", "headers_already_sent"); ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, "modsecurity phase4 intervention after headers sent, action=connection_abort, uri=\"%V\"", &r->uri); r->connection->error = 1; return NGX_ERROR; } - ngx_http_modsecurity_phase4_log_event(r, mcf, "deny", "log_only", + ngx_http_modsecurity_phase4_log_event(r, mcf, wanted, "log_only", mcf->phase4_mode == NGX_HTTP_MODSEC_PHASE4_MODE_MINIMAL ? "mode_minimal" : "mode_safe"); return NGX_OK; } @@ -290,7 +294,12 @@ ngx_http_modsecurity_phase4_log_event(ngx_http_request_t *r, ngx_http_modsecurit p = ngx_snprintf(dbuf, need, "{\"event\":\"phase4_intervention\",\"uri\":\"%V\",\"method\":\"%V\",\"response_status\":%ui,\"waf_status\":%i,\"content_type\":\"%V\",\"header_sent\":%s,\"mode\":\"%s\",\"wanted_action\":\"%s\",\"actual_action\":\"%s\",\"reason\":\"%s\",\"intervention\":\"%V\",\"rule_id\":\"%V\"}\n", &euri,&emethod,(ngx_uint_t)r->headers_out.status,ctx ? (int) ctx->last_intervention_status : 0,&ect,header_sent,mode,wanted,actual,reason,&elog,&erule); - ngx_write_fd(mcf->phase4_log_file->fd, dbuf, p - dbuf); + ssize_t n = ngx_write_fd(mcf->phase4_log_file->fd, dbuf, p - dbuf); + if (n < 0 || (size_t) n != (size_t) (p - dbuf)) { + ngx_log_error(NGX_LOG_WARN, r->connection->log, ngx_errno, + "modsecurity phase4 log write failed"); + return NGX_ERROR; + } return NGX_OK; } diff --git a/src/ngx_http_modsecurity_common.h b/src/ngx_http_modsecurity_common.h index b9a11f39..de9941cb 100644 --- a/src/ngx_http_modsecurity_common.h +++ b/src/ngx_http_modsecurity_common.h @@ -105,7 +105,6 @@ typedef struct { unsigned intervention_triggered:1; unsigned request_body_processed:1; unsigned phase4_headers_checked:1; - ngx_str_t phase4_content_type; ngx_str_t last_intervention_log; ngx_int_t last_intervention_status; } ngx_http_modsecurity_ctx_t; diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 39c09d95..339ea8da 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -44,11 +44,6 @@ static char *ngx_http_modsecurity_phase4_set_default_content_types(ngx_conf_t *c static char *ngx_http_modsecurity_phase4_load_content_types_file(ngx_conf_t *cf, ngx_http_modsecurity_conf_t *mcf, ngx_str_t *path); static ngx_int_t ngx_http_modsecurity_phase4_validate_content_type(u_char *s, size_t len); -#define NGX_HTTP_MODSEC_PHASE4_MODE_MINIMAL 0 -#define NGX_HTTP_MODSEC_PHASE4_MODE_SAFE 1 -#define NGX_HTTP_MODSEC_PHASE4_MODE_STRICT 2 - - /* * PCRE malloc/free workaround, based on * https://github.com/openresty/lua-nginx-module/blob/master/src/ngx_http_lua_pcrefix.c @@ -174,7 +169,12 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re ctx->last_intervention_status = intervention.status; ctx->last_intervention_log.len = 0; ctx->last_intervention_log.data = NULL; - if (intervention.log != NULL) { + mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); + if (mcf == NULL) { + return NGX_HTTP_INTERNAL_SERVER_ERROR; + } + + if (mcf->phase4_log_file != NULL && intervention.log != NULL) { size_t l = ngx_strlen(intervention.log); u_char *cp = ngx_pnalloc(r->pool, l + 1); if (cp != NULL) { @@ -182,19 +182,9 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re cp[l] = '\0'; ctx->last_intervention_log.data = cp; ctx->last_intervention_log.len = l; - } else { - if (intervention.log != NULL) { - free(intervention.log); - } - return NGX_HTTP_INTERNAL_SERVER_ERROR; } } - mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); - if (mcf == NULL) { - return NGX_HTTP_INTERNAL_SERVER_ERROR; - } - // logging to nginx error log can be disable by setting `modsecurity_use_error_log` to off if (mcf->use_error_log) { log = intervention.log; @@ -653,7 +643,10 @@ ngx_http_modsecurity_phase4_load_content_types_file(ngx_conf_t *cf, ngx_http_mod file.name = *path; file.log = cf->log; file.fd = ngx_open_file(path->data, NGX_FILE_RDONLY, NGX_FILE_OPEN, 0); - if (file.fd == NGX_INVALID_FILE) return NGX_CONF_ERROR; + if (file.fd == NGX_INVALID_FILE) { + ngx_conf_log_error(NGX_LOG_EMERG, cf, ngx_errno, "modsecurity_phase4_content_types_file \"%V\" open() failed", path); + return NGX_CONF_ERROR; + } n = ngx_read_file(&file, buf, ngx_file_size(&fi), 0); ngx_close_file(file.fd); if (n < 0) return NGX_CONF_ERROR; diff --git a/tests/modsecurity-phase4-content-types.t b/tests/modsecurity-phase4-content-types.t index 9ce9c35d..412e9fea 100644 --- a/tests/modsecurity-phase4-content-types.t +++ b/tests/modsecurity-phase4-content-types.t @@ -85,7 +85,7 @@ like(http_get('/unknown'), qr/HIT UNKNOWN/, 'unknown content-type not in scope = like(http_get('/emptytype'), qr/HIT EMPTY/, 'empty content-type => no hard action'); my $log = $t->read_file('phase4-content-types.log'); -like($log, qr/"content_type":"application\/json"|"content_type":"application\/json"/, 'json content type logged'); +like($log, qr/"content_type":"application\/json"/, 'json content type logged'); like($log, qr/"actual_action":"connection_abort"/, 'strict in-scope abort logged'); like($log, qr/"reason":"content_type_not_in_scope"/, 'out-of-scope reason logged'); like($log, qr/"event":"phase4_intervention"/, 'json lines event present'); diff --git a/tests/modsecurity-phase4-invalid-config.t b/tests/modsecurity-phase4-invalid-config.t index 176274a5..061cde6b 100644 --- a/tests/modsecurity-phase4-invalid-config.t +++ b/tests/modsecurity-phase4-invalid-config.t @@ -27,6 +27,7 @@ http { } EOC -# Expected: nginx config test/start should fail with clear config error -ok(1, 'manual verification test: start should fail for wildcard/invalid content-type entries'); +my $failed = eval { $t->run(); 1 }; +ok(!$failed, 'nginx startup fails for invalid phase4 content-type entries'); +like($@, qr/invalid content-type entry in modsecurity_phase4_content_types_file/, 'error points to invalid content-type entry'); done_testing(); diff --git a/tests/modsecurity-phase4-modes.t b/tests/modsecurity-phase4-modes.t index ff9798e1..99c2988c 100644 --- a/tests/modsecurity-phase4-modes.t +++ b/tests/modsecurity-phase4-modes.t @@ -67,7 +67,7 @@ is(http_get('/x'), '', 'strict abort after headers sent'); my $log = $t->read_file('phase4.log'); like($log, qr/"actual_action":"log_only"/, 'log_only present'); -like($log, qr/"reason":"mode_safe"|"reason":"headers_already_sent"/, 'safe reason present'); +like($log, qr/"reason":"mode_safe"/, 'safe reason present'); like($log, qr/"actual_action":"connection_abort"/, 'strict action logged'); like($log, qr/"event":"phase4_intervention"/, 'event field present'); like($log, qr/"header_sent":true/, 'json boolean header_sent'); diff --git a/tests/modsecurity-proxy-h2.t b/tests/modsecurity-proxy-h2.t index da0c9d53..df0e43ba 100644 --- a/tests/modsecurity-proxy-h2.t +++ b/tests/modsecurity-proxy-h2.t @@ -271,6 +271,15 @@ EOF select undef, undef, undef, 0.1; print $client 'AND-THIS'; + } elsif ($uri =~ m{^/phase4\?what=(redirect301|redirect302|block401|block403)$}) { + print $client <<"EOF"; +HTTP/1.1 200 OK +Content-Type: text/html +Connection: close + +phase4 trigger +EOF + } else { print $client <<"EOF"; From cfc7e226ab27bf287e054b34b32043cb97d3f00a Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Thu, 14 May 2026 19:04:23 +0200 Subject: [PATCH 18/22] Fix sanity-check redeclaration and narrow phase4 log capture --- src/ngx_http_modsecurity_body_filter.c | 4 ++-- src/ngx_http_modsecurity_module.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index ece49a5c..0b7223e8 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -47,8 +47,8 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) { ngx_chain_t *chain = in; ngx_http_modsecurity_ctx_t *ctx = NULL; -#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) ngx_http_modsecurity_conf_t *mcf; +#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) ngx_list_part_t *part = &r->headers_out.headers.part; ngx_table_elt_t *data = part->elts; ngx_uint_t i = 0; @@ -149,7 +149,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) #endif int is_request_processed = 0; - ngx_http_modsecurity_conf_t *mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); + mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); for (; chain != NULL; chain = chain->next) { u_char *data = chain->buf->pos; diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 339ea8da..e397dfd1 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -174,7 +174,7 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re return NGX_HTTP_INTERNAL_SERVER_ERROR; } - if (mcf->phase4_log_file != NULL && intervention.log != NULL) { + if (mcf->phase4_log_file != NULL && r->header_sent && intervention.log != NULL) { size_t l = ngx_strlen(intervention.log); u_char *cp = ngx_pnalloc(r->pool, l + 1); if (cp != NULL) { From 00519756540d1dbb9bd21ddac3307463363a9cb2 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Thu, 14 May 2026 19:04:28 +0200 Subject: [PATCH 19/22] Rewrite phase4 invalid-config test to use nginx -t output --- tests/modsecurity-phase4-invalid-config.t | 30 ++++++++++++----------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/tests/modsecurity-phase4-invalid-config.t b/tests/modsecurity-phase4-invalid-config.t index 061cde6b..e37956d6 100644 --- a/tests/modsecurity-phase4-invalid-config.t +++ b/tests/modsecurity-phase4-invalid-config.t @@ -1,33 +1,35 @@ #!/usr/bin/perl use warnings; use strict; -use Test::More; +use Test::More tests => 1; BEGIN { use FindBin; chdir($FindBin::Bin); } use lib 'lib'; -use Test::Nginx; +use Test::Nginx::Socket -Base; -# this test verifies parser rejects invalid entries/wildcards at config load -my $t = Test::Nginx->new()->has(qw/http/); -$t->write_file('phase4-invalid.conf', "text/*\n*/json\ntext/html bad\napplication/problem+json\n"); +my $t = Test::Nginx::Socket->new()->has(qw/http/); +$t->write_file('phase4-invalid.conf', "text/*\n"); -$t->write_file_expand('nginx.conf', <<'EOC'); +$t->write_file_expand('nginx.conf', <<'EOF'); %%TEST_GLOBALS%% -daemon off; + events {} + http { %%TEST_GLOBALS_HTTP%% + server { - listen 127.0.0.1:8080; + listen 127.0.0.1:%%PORT%%; server_name localhost; location / { modsecurity on; modsecurity_phase4_content_types_file %%TESTDIR%%/phase4-invalid.conf; - return 200 "ok"; + return 200 "ok\n"; } } } -EOC +EOF + +my $cmd = "$t->{_testdir}/../nginx -p $t->{_testdir}/ -c nginx.conf -t 2>&1"; +my $out = `$cmd`; -my $failed = eval { $t->run(); 1 }; -ok(!$failed, 'nginx startup fails for invalid phase4 content-type entries'); -like($@, qr/invalid content-type entry in modsecurity_phase4_content_types_file/, 'error points to invalid content-type entry'); -done_testing(); +like($out, qr/invalid content-type entry in modsecurity_phase4_content_types_file/, + 'error points to invalid content-type entry'); From 329f269977b6f914d819751b659976011aeb02bc Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Thu, 14 May 2026 19:18:27 +0200 Subject: [PATCH 20/22] Use repository Test::Nginx harness in phase4 invalid-config test --- tests/modsecurity-phase4-invalid-config.t | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/modsecurity-phase4-invalid-config.t b/tests/modsecurity-phase4-invalid-config.t index e37956d6..c62f53b4 100644 --- a/tests/modsecurity-phase4-invalid-config.t +++ b/tests/modsecurity-phase4-invalid-config.t @@ -3,9 +3,9 @@ use warnings; use strict; use Test::More tests => 1; BEGIN { use FindBin; chdir($FindBin::Bin); } use lib 'lib'; -use Test::Nginx::Socket -Base; +use Test::Nginx; -my $t = Test::Nginx::Socket->new()->has(qw/http/); +my $t = Test::Nginx->new()->has(qw/http/); $t->write_file('phase4-invalid.conf', "text/*\n"); $t->write_file_expand('nginx.conf', <<'EOF'); @@ -28,7 +28,8 @@ http { } EOF -my $cmd = "$t->{_testdir}/../nginx -p $t->{_testdir}/ -c nginx.conf -t 2>&1"; +my $cmd = $ENV{TEST_NGINX_BINARY} || ($t->testdir() . '/../nginx'); +$cmd .= " -p " . $t->testdir() . "/ -c nginx.conf -t 2>&1"; my $out = `$cmd`; like($out, qr/invalid content-type entry in modsecurity_phase4_content_types_file/, From 8b766e44038642390593c6a0d2a6d13b64247fc9 Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Thu, 14 May 2026 19:26:58 +0200 Subject: [PATCH 21/22] Stabilize phase4 invalid-config test nginx -t setup --- tests/modsecurity-phase4-invalid-config.t | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/modsecurity-phase4-invalid-config.t b/tests/modsecurity-phase4-invalid-config.t index c62f53b4..cc695551 100644 --- a/tests/modsecurity-phase4-invalid-config.t +++ b/tests/modsecurity-phase4-invalid-config.t @@ -7,6 +7,8 @@ use Test::Nginx; my $t = Test::Nginx->new()->has(qw/http/); $t->write_file('phase4-invalid.conf', "text/*\n"); +mkdir($t->testdir() . '/logs') unless -d $t->testdir() . '/logs'; +$t->write_file('logs/error.log', ''); $t->write_file_expand('nginx.conf', <<'EOF'); %%TEST_GLOBALS%% @@ -17,7 +19,7 @@ http { %%TEST_GLOBALS_HTTP%% server { - listen 127.0.0.1:%%PORT%%; + listen 127.0.0.1:19849; server_name localhost; location / { modsecurity on; From 20d7aca643db680455a3baea9e33bb0e1ae739cb Mon Sep 17 00:00:00 2001 From: Easton97-Jens <66330090+Easton97-Jens@users.noreply.github.com> Date: Thu, 14 May 2026 20:17:29 +0200 Subject: [PATCH 22/22] docs: document phase4 nginx directives in README --- README.md | 27 ++++++++++++++++++++++++++- 1 file changed, 26 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index a93e5df0..ed1aa335 100644 --- a/README.md +++ b/README.md @@ -45,7 +45,7 @@ Further information about nginx third-party add-ons support are available [here] # Usage ModSecurity for nginx extends your nginx configuration directives. -It adds four new directives and they are: +It adds eight directives and they are: modsecurity ----------- @@ -175,6 +175,31 @@ using the same unique identificator. String can contain variables. + +modsecurity_phase4_mode +------------------------ +**syntax:** *modsecurity_phase4_mode minimal | safe | strict* + +**context:** *http, server, location* + +Controls how phase 4 interventions are handled when response headers were already sent. + +modsecurity_phase4_content_types_file +-------------------------------------- +**syntax:** *modsecurity_phase4_content_types_file <path>* + +**context:** *http, server, location* + +Loads the list of response content types that are in scope for phase 4 handling from a file. + +modsecurity_phase4_log +---------------------- +**syntax:** *modsecurity_phase4_log <path>* + +**context:** *http, server, location* + +Sets the file used for phase 4 JSON event logging. + modsecurity_use_error_log ----------- **syntax:** *modsecurity_use_error_log on | off*