Skip to content

Commit

Permalink
Fix phase 2 and 3 audit logging in case of internal redirect
Browse files Browse the repository at this point in the history
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 <[email protected]>
Co-authored-by: Thanos Giannopoulos <[email protected]>
  • Loading branch information
3 people committed Jun 3, 2024
1 parent d90eed5 commit 02249da
Show file tree
Hide file tree
Showing 7 changed files with 13 additions and 15 deletions.
4 changes: 2 additions & 2 deletions src/ngx_http_modsecurity_body_filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,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);
Expand All @@ -165,7 +165,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;
}
Expand Down
2 changes: 1 addition & 1 deletion src/ngx_http_modsecurity_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/ngx_http_modsecurity_header_filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -528,7 +528,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);
}
Expand Down
1 change: 1 addition & 0 deletions src/ngx_http_modsecurity_log.c
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,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;
}
9 changes: 3 additions & 6 deletions src/ngx_http_modsecurity_module.c
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ ngx_inline char *ngx_str_to_char(ngx_str_t a, ngx_pool_t *p)


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;
Expand Down Expand Up @@ -222,11 +222,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)
{
Expand Down
4 changes: 2 additions & 2 deletions src/ngx_http_modsecurity_pre_access.c
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,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;
}
Expand All @@ -214,7 +214,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;
}
Expand Down
6 changes: 3 additions & 3 deletions src/ngx_http_modsecurity_rewrite.c
Original file line number Diff line number Diff line change
Expand Up @@ -117,7 +117,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;
Expand Down Expand Up @@ -166,7 +166,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;
Expand Down Expand Up @@ -215,7 +215,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;
}
Expand Down

0 comments on commit 02249da

Please sign in to comment.