Skip to content

Commit 89c8d3b

Browse files
mlevogianniskolotourosthanosgn
committed
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 <[email protected]> Co-authored-by: Thanos Giannopoulos <[email protected]>
1 parent 4dc6f25 commit 89c8d3b

7 files changed

+13
-15
lines changed

Diff for: src/ngx_http_modsecurity_body_filter.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -145,7 +145,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
145145
int ret;
146146

147147
msc_append_response_body(ctx->modsec_transaction, data, chain->buf->last - data);
148-
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
148+
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r);
149149
if (ret > 0) {
150150
return ngx_http_filter_finalize_request(r,
151151
&ngx_http_modsecurity_module, ret);
@@ -163,7 +163,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
163163

164164
/* XXX: I don't get how body from modsec being transferred to nginx's buffer. If so - after adjusting of nginx's
165165
XXX: body we can proceed to adjust body size (content-length). see xslt_body_filter() for example */
166-
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
166+
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r);
167167
if (ret > 0) {
168168
return ret;
169169
}

Diff for: src/ngx_http_modsecurity_common.h

+1-1
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ typedef struct {
137137
extern ngx_module_t ngx_http_modsecurity_module;
138138

139139
/* ngx_http_modsecurity_module.c */
140-
int ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_request_t *r, ngx_int_t early_log);
140+
int ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_request_t *r);
141141
ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_create_ctx(ngx_http_request_t *r);
142142
char *ngx_str_to_char(ngx_str_t a, ngx_pool_t *p);
143143
#if (NGX_PCRE2)

Diff for: src/ngx_http_modsecurity_header_filter.c

+1-1
Original file line numberDiff line numberDiff line change
@@ -526,7 +526,7 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r)
526526
old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
527527
msc_process_response_headers(ctx->modsec_transaction, status, http_response_ver);
528528
ngx_http_modsecurity_pcre_malloc_done(old_pool);
529-
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
529+
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r);
530530
if (r->error_page) {
531531
return ngx_http_next_header_filter(r);
532532
}

Diff for: src/ngx_http_modsecurity_log.c

+1
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,7 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r)
7676
old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
7777
msc_process_logging(ctx->modsec_transaction);
7878
ngx_http_modsecurity_pcre_malloc_done(old_pool);
79+
ctx->logged = 1;
7980

8081
return NGX_OK;
8182
}

Diff for: src/ngx_http_modsecurity_module.c

+3-6
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,7 @@ ngx_inline char *ngx_str_to_char(ngx_str_t a, ngx_pool_t *p)
132132

133133

134134
ngx_inline int
135-
ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_request_t *r, ngx_int_t early_log)
135+
ngx_http_modsecurity_process_intervention(Transaction *transaction, ngx_http_request_t *r)
136136
{
137137
char *log = NULL;
138138
ModSecurityIntervention intervention;
@@ -217,11 +217,8 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re
217217
*/
218218
msc_update_status_code(ctx->modsec_transaction, intervention.status);
219219

220-
if (early_log) {
221-
dd("intervention -- calling log handler manually with code: %d", intervention.status);
222-
ngx_http_modsecurity_log_handler(r);
223-
ctx->logged = 1;
224-
}
220+
dd("intervention -- calling log handler manually with code: %d", intervention.status);
221+
ngx_http_modsecurity_log_handler(r);
225222

226223
if (r->header_sent)
227224
{

Diff for: src/ngx_http_modsecurity_pre_access.c

+2-2
Original file line numberDiff line numberDiff line change
@@ -193,7 +193,7 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r)
193193
* it may ask for a intervention in consequence of that.
194194
*
195195
*/
196-
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
196+
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r);
197197
if (ret > 0) {
198198
return ret;
199199
}
@@ -212,7 +212,7 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r)
212212
msc_process_request_body(ctx->modsec_transaction);
213213
ngx_http_modsecurity_pcre_malloc_done(old_pool);
214214

215-
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
215+
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r);
216216
if (r->error_page) {
217217
return NGX_DECLINED;
218218
}

Diff for: src/ngx_http_modsecurity_rewrite.c

+3-3
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r)
115115
*
116116
*/
117117
dd("Processing intervention with the connection information filled in");
118-
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 1);
118+
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r);
119119
if (ret > 0) {
120120
ctx->intervention_triggered = 1;
121121
return ret;
@@ -164,7 +164,7 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r)
164164
ngx_http_modsecurity_pcre_malloc_done(old_pool);
165165

166166
dd("Processing intervention with the transaction information filled in (uri, method and version)");
167-
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 1);
167+
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r);
168168
if (ret > 0) {
169169
ctx->intervention_triggered = 1;
170170
return ret;
@@ -213,7 +213,7 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r)
213213
msc_process_request_headers(ctx->modsec_transaction);
214214
ngx_http_modsecurity_pcre_malloc_done(old_pool);
215215
dd("Processing intervention with the request headers information filled in");
216-
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 1);
216+
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r);
217217
if (r->error_page) {
218218
return NGX_DECLINED;
219219
}

0 commit comments

Comments
 (0)