diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 86bccc7..3e5fdd1 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -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); @@ -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; } 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 257e7fd..7f9b6ad 100644 --- a/src/ngx_http_modsecurity_header_filter.c +++ b/src/ngx_http_modsecurity_header_filter.c @@ -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); } diff --git a/src/ngx_http_modsecurity_log.c b/src/ngx_http_modsecurity_log.c index 167b2d3..e1e0e4c 100644 --- a/src/ngx_http_modsecurity_log.c +++ b/src/ngx_http_modsecurity_log.c @@ -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; } diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index c90b3f6..e6f4be3 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -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; @@ -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) { diff --git a/src/ngx_http_modsecurity_pre_access.c b/src/ngx_http_modsecurity_pre_access.c index ea5c021..7805ee6 100644 --- a/src/ngx_http_modsecurity_pre_access.c +++ b/src/ngx_http_modsecurity_pre_access.c @@ -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; } @@ -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; } diff --git a/src/ngx_http_modsecurity_rewrite.c b/src/ngx_http_modsecurity_rewrite.c index 926cf70..f8c3ae4 100644 --- a/src/ngx_http_modsecurity_rewrite.c +++ b/src/ngx_http_modsecurity_rewrite.c @@ -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; @@ -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; @@ -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; } diff --git a/tests/modsecurity-config-custom-error-page.t b/tests/modsecurity-config-custom-error-page.t index a8f5862..b8f6214 100644 --- a/tests/modsecurity-config-custom-error-page.t +++ b/tests/modsecurity-config-custom-error-page.t @@ -55,7 +55,7 @@ http { error_page 403 /403.html; location /403.html { - root %%TESTDIR%%/http; + alias %%TESTDIR%%/403.html; internal; } @@ -63,7 +63,11 @@ http { modsecurity on; modsecurity_rules ' SecRuleEngine On - SecRule ARGS "@streq root" "id:10,phase:1,auditlog,status:403,deny" + SecResponseBodyAccess On + SecRule ARGS:phase1 "@streq BAD" "id:10,phase:1,auditlog,status:403,deny" + SecRule ARGS:phase2 "@streq BAD" "id:11,phase:2,auditlog,status:403,deny" + SecRule ARGS:phase3 "@streq BAD" "id:12,phase:3,auditlog,status:403,deny" + SecRule ARGS:phase4 "@streq BAD" "id:13,phase:4,auditlog,status:403,drop" SecDebugLog %%TESTDIR%%/auditlog-debug-local.txt SecDebugLogLevel 9 SecAuditEngine RelevantOnly @@ -82,7 +86,11 @@ http { modsecurity on; modsecurity_rules ' SecRuleEngine On - SecRule ARGS "@streq root" "id:10,phase:1,auditlog,status:403,deny" + SecResponseBodyAccess On + SecRule ARGS:phase1 "@streq BAD" "id:10,phase:1,auditlog,status:403,deny" + SecRule ARGS:phase2 "@streq BAD" "id:11,phase:2,auditlog,status:403,deny" + SecRule ARGS:phase3 "@streq BAD" "id:12,phase:3,auditlog,status:403,deny" + SecRule ARGS:phase4 "@streq BAD" "id:13,phase:4,auditlog,status:403,drop" SecDebugLog %%TESTDIR%%/auditlog-debug-global.txt SecDebugLogLevel 9 SecAuditEngine RelevantOnly @@ -96,7 +104,7 @@ http { location /403.html { modsecurity off; - root %%TESTDIR%%/http; + alias %%TESTDIR%%/403.html; internal; } @@ -107,31 +115,28 @@ http { EOF my $index_txt = "This is the index page."; -my $custom_txt = "This is a custom error page."; +my $error_txt = "This is a custom error page."; $t->write_file("/index.html", $index_txt); -mkdir($t->testdir() . '/http'); -$t->write_file("/http/403.html", $custom_txt); +$t->write_file("/403.html", $error_txt); +$t->todo_alerts(); $t->run(); -$t->plan(10); +$t->plan(32); ############################################################################### my $d = $t->testdir(); -my $t1; -my $t2; -my $t3; -my $t4; - # Performing requests to a server with ModSecurity enabled at location context -$t1 = http_get_host('s1', '/index.html?what=root'); -$t2 = http_get_host('s1', '/index.html?what=other'); - -# Performing requests to a server with ModSecurity enabled at server context -$t3 = http_get_host('s2', '/index.html?what=root'); -$t4 = http_get_host('s2', '/index.html?what=other'); +like(http_get_host('s1', '/?phase1=BAD'), qr/$error_txt/, 'location context, phase 1, error page'); +like(http_get_host('s1', '/?phase1=GOOD'), qr/$index_txt/, 'location context, phase 1, index page'); +like(http_get_host('s1', '/?phase2=BAD'), qr/$error_txt/, 'location context, phase 2, error page'); +like(http_get_host('s1', '/?phase2=GOOD'), qr/$index_txt/, 'location context, phase 2, index page'); +like(http_get_host('s1', '/?phase3=BAD'), qr/$error_txt/, 'location context, phase 3, error page'); +like(http_get_host('s1', '/?phase3=GOOD'), qr/$index_txt/, 'location context, phase 3, index page'); +is(http_get_host('s1', '/?phase4=BAD'), '', 'location context, phase 4, drop'); +like(http_get_host('s1', '/?phase4=GOOD'), qr/$index_txt/, 'location context, phase 4, index page'); my $local = do { local $/ = undef; @@ -140,6 +145,25 @@ my $local = do { <$fh>; }; +like($local, qr/phase1=BAD/, 'location context, phase 1, BAD in auditlog'); +unlike($local, qr/phase1=GOOD/, 'location context, phase 1, GOOD not in auditlog'); +like($local, qr/phase2=BAD/, 'location context, phase 2, BAD in auditlog'); +unlike($local, qr/phase2=GOOD/, 'location context, phase 2, GOOD not in auditlog'); +like($local, qr/phase3=BAD/, 'location context, phase 3, BAD in auditlog'); +unlike($local, qr/phase3=GOOD/, 'location context, phase 3, GOOD not in auditlog'); +like($local, qr/phase4=BAD/, 'location context, phase 4, BAD in auditlog'); +unlike($local, qr/phase4=GOOD/, 'location context, phase 4, GOOD not in auditlog'); + +# Performing requests to a server with ModSecurity enabled at server context +like(http_get_host('s2', '/?phase1=BAD'), qr/$error_txt/, 'server context, phase 1, error page'); +like(http_get_host('s2', '/?phase1=GOOD'), qr/$index_txt/, 'server context, phase 1, index page'); +like(http_get_host('s2', '/?phase2=BAD'), qr/$error_txt/, 'server context, phase 2, error page'); +like(http_get_host('s2', '/?phase2=GOOD'), qr/$index_txt/, 'server context, phase 2, index page'); +like(http_get_host('s2', '/?phase3=BAD'), qr/$error_txt/, 'server context, phase 3, error page'); +like(http_get_host('s2', '/?phase3=GOOD'), qr/$index_txt/, 'server context, phase 3, index page'); +is(http_get_host('s2', '/?phase4=BAD'), '', 'server context, phase 4, drop'); +like(http_get_host('s2', '/?phase4=GOOD'), qr/$index_txt/, 'server context, phase 4, index page'); + my $global = do { local $/ = undef; open my $fh, "<", "$d/auditlog-global.txt" @@ -147,18 +171,14 @@ my $global = do { <$fh>; }; -like($t1, qr/$custom_txt/, 'ModSecurity at location / root'); -like($t2, qr/$index_txt/, 'ModSecurity at location / other'); -like($local, qr/what=root/, 'ModSecurity at location / root present in auditlog'); -unlike($local, qr/what=other/, 'ModSecurity at location / other not present in auditlog'); - -like($t3, qr/$custom_txt/, 'ModSecurity at server / root'); -like($t4, qr/$index_txt/, 'ModSecurity at server / other'); -like($global, qr/what=root/, 'ModSecurity at server / root present in auditlog'); -unlike($global, qr/what=other/, 'ModSecurity at server / other not present in auditlog'); - -like($local, qr/Access denied with code 403/, 'ModSecurity at location / 403 in auditlog'); -like($global, qr/Access denied with code 403/, 'ModSecurity at server / 403 in auditlog'); +like($global, qr/phase1=BAD/, 'server context, phase 1, BAD in auditlog'); +unlike($global, qr/phase1=GOOD/, 'server context, phase 1, GOOD not in auditlog'); +like($global, qr/phase2=BAD/, 'server context, phase 2, BAD in auditlog'); +unlike($global, qr/phase2=GOOD/, 'server context, phase 2, GOOD not in auditlog'); +like($global, qr/phase3=BAD/, 'server context, phase 3, BAD in auditlog'); +unlike($global, qr/phase3=GOOD/, 'server context, phase 3, GOOD not in auditlog'); +like($global, qr/phase4=BAD/, 'server context, phase 4, BAD in auditlog'); +unlike($global, qr/phase4=GOOD/, 'server context, phase 4, GOOD not in auditlog'); ###############################################################################