From 89c8d3bbf77d85b385323050dfa8ee084941c5a5 Mon Sep 17 00:00:00 2001 From: Marios Levogiannis Date: Wed, 24 Nov 2021 12:36:28 +0200 Subject: [PATCH] Fix phase 2 and 3 audit logging in case of internal redirect Phase 2 and 3 log entries were not logged in the audit log in case of an internal redirect. Only phase 1 and 4 ones were logged, the former because early logging was explicitly enabled and the latter because the internal redirect does not work in this phase. This commit unconditionally enables early logging in all ModSecurity phases. Since the Nginx log phase may not be executed in case of an intervention, the process intervention function is the only place which is guaranteed to call the log handler in such a case. Co-authored-by: Dimitris Kolotouros Co-authored-by: Thanos Giannopoulos --- src/ngx_http_modsecurity_body_filter.c | 4 ++-- src/ngx_http_modsecurity_common.h | 2 +- src/ngx_http_modsecurity_header_filter.c | 2 +- src/ngx_http_modsecurity_log.c | 1 + src/ngx_http_modsecurity_module.c | 9 +++------ src/ngx_http_modsecurity_pre_access.c | 4 ++-- src/ngx_http_modsecurity_rewrite.c | 6 +++--- 7 files changed, 13 insertions(+), 15 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 725f986..1b85df5 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -145,7 +145,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) int ret; msc_append_response_body(ctx->modsec_transaction, data, chain->buf->last - data); - ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0); + ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (ret > 0) { return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity_module, ret); @@ -163,7 +163,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) /* XXX: I don't get how body from modsec being transferred to nginx's buffer. If so - after adjusting of nginx's 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); + ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (ret > 0) { return ret; } diff --git a/src/ngx_http_modsecurity_common.h b/src/ngx_http_modsecurity_common.h index 11fdc2d..3b91cca 100644 --- a/src/ngx_http_modsecurity_common.h +++ b/src/ngx_http_modsecurity_common.h @@ -137,7 +137,7 @@ typedef struct { extern ngx_module_t ngx_http_modsecurity_module; /* ngx_http_modsecurity_module.c */ -int ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_request_t *r, ngx_int_t early_log); +int ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_request_t *r); ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_create_ctx(ngx_http_request_t *r); char *ngx_str_to_char(ngx_str_t a, ngx_pool_t *p); #if (NGX_PCRE2) diff --git a/src/ngx_http_modsecurity_header_filter.c b/src/ngx_http_modsecurity_header_filter.c index c36be19..51990c7 100644 --- a/src/ngx_http_modsecurity_header_filter.c +++ b/src/ngx_http_modsecurity_header_filter.c @@ -526,7 +526,7 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r) old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool); msc_process_response_headers(ctx->modsec_transaction, status, http_response_ver); ngx_http_modsecurity_pcre_malloc_done(old_pool); - ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0); + ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (r->error_page) { return ngx_http_next_header_filter(r); } diff --git a/src/ngx_http_modsecurity_log.c b/src/ngx_http_modsecurity_log.c index d713a65..6f1bcc0 100644 --- a/src/ngx_http_modsecurity_log.c +++ b/src/ngx_http_modsecurity_log.c @@ -76,6 +76,7 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r) old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool); msc_process_logging(ctx->modsec_transaction); ngx_http_modsecurity_pcre_malloc_done(old_pool); + ctx->logged = 1; return NGX_OK; } diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 5c341e2..2691f1d 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -132,7 +132,7 @@ ngx_inline char *ngx_str_to_char(ngx_str_t a, ngx_pool_t *p) ngx_inline int -ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_request_t *r, ngx_int_t early_log) +ngx_http_modsecurity_process_intervention(Transaction *transaction, ngx_http_request_t *r) { char *log = NULL; ModSecurityIntervention intervention; @@ -217,11 +217,8 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re */ msc_update_status_code(ctx->modsec_transaction, intervention.status); - if (early_log) { - dd("intervention -- calling log handler manually with code: %d", intervention.status); - ngx_http_modsecurity_log_handler(r); - ctx->logged = 1; - } + dd("intervention -- calling log handler manually with code: %d", intervention.status); + ngx_http_modsecurity_log_handler(r); if (r->header_sent) { diff --git a/src/ngx_http_modsecurity_pre_access.c b/src/ngx_http_modsecurity_pre_access.c index abb7d3e..779b71e 100644 --- a/src/ngx_http_modsecurity_pre_access.c +++ b/src/ngx_http_modsecurity_pre_access.c @@ -193,7 +193,7 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r) * it may ask for a intervention in consequence of that. * */ - ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0); + ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (ret > 0) { return ret; } @@ -212,7 +212,7 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r) msc_process_request_body(ctx->modsec_transaction); ngx_http_modsecurity_pcre_malloc_done(old_pool); - ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0); + ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (r->error_page) { return NGX_DECLINED; } diff --git a/src/ngx_http_modsecurity_rewrite.c b/src/ngx_http_modsecurity_rewrite.c index ebf115e..3425a0b 100644 --- a/src/ngx_http_modsecurity_rewrite.c +++ b/src/ngx_http_modsecurity_rewrite.c @@ -115,7 +115,7 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r) * */ dd("Processing intervention with the connection information filled in"); - ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 1); + ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (ret > 0) { ctx->intervention_triggered = 1; return ret; @@ -164,7 +164,7 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r) ngx_http_modsecurity_pcre_malloc_done(old_pool); dd("Processing intervention with the transaction information filled in (uri, method and version)"); - ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 1); + ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (ret > 0) { ctx->intervention_triggered = 1; return ret; @@ -213,7 +213,7 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r) msc_process_request_headers(ctx->modsec_transaction); ngx_http_modsecurity_pcre_malloc_done(old_pool); dd("Processing intervention with the request headers information filled in"); - ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 1); + ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r); if (r->error_page) { return NGX_DECLINED; }