From 62639fa2be3839e14e284d13c16516889feb8afb Mon Sep 17 00:00:00 2001 From: Serg Date: Tue, 18 Jun 2024 20:52:29 +0000 Subject: [PATCH 1/2] Fix #41 --- src/ngx_http_modsecurity_body_filter.c | 114 +++++++++++++++-------- src/ngx_http_modsecurity_common.h | 5 +- src/ngx_http_modsecurity_header_filter.c | 19 ++-- src/ngx_http_modsecurity_module.c | 1 + tests/modsecurity-proxy.t | 32 +++---- tests/modsecurity-scoring.t | 4 +- tests/modsecurity.t | 32 +++---- 7 files changed, 129 insertions(+), 78 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 86bccc74..27ea8d84 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -33,12 +33,14 @@ ngx_http_modsecurity_body_filter_init(void) return NGX_OK; } - ngx_int_t 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; + ngx_chain_t *chain = in; + ngx_int_t ret; + ngx_pool_t *old_pool; + ngx_int_t is_request_processed = 0; #if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) ngx_http_modsecurity_conf_t *mcf; ngx_list_part_t *part = &r->headers_out.headers.part; @@ -47,14 +49,18 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) #endif if (in == NULL) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "MDS input chain is null"); + return ngx_http_next_body_filter(r, in); } + /* get context for request */ ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); - dd("body filter, recovering ctx: %p", ctx); - if (ctx == NULL) { + if (ctx == NULL || r->filter_finalize || ctx->response_body_filtered) { + if (ctx && ctx->response_body_filtered) + r->filter_finalize = 1; return ngx_http_next_body_filter(r, in); } @@ -140,47 +146,81 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) } #endif - int is_request_processed = 0; - for (; chain != NULL; chain = chain->next) - { - u_char *data = chain->buf->pos; - int ret; + for (chain = in; chain != NULL; chain = chain->next) { - msc_append_response_body(ctx->modsec_transaction, data, chain->buf->last - data); - ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0); + ngx_buf_t *copy_buf; + ngx_chain_t* copy_chain; + is_request_processed = chain->buf->last_buf; + u_char *data = chain->buf->pos; + msc_append_response_body(ctx->modsec_transaction, data, + chain->buf->last - data); + ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, + r, 0); if (ret > 0) { return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity_module, ret); } - -/* XXX: chain->buf->last_buf || chain->buf->last_in_chain */ - is_request_processed = chain->buf->last_buf; - - if (is_request_processed) { - ngx_pool_t *old_pool; - - old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool); - msc_process_response_body(ctx->modsec_transaction); - ngx_http_modsecurity_pcre_malloc_done(old_pool); - -/* 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); - if (ret > 0) { - return ret; + if (!chain->buf->last_buf){ + copy_chain = ngx_alloc_chain_link(r->pool); + if (copy_chain == NULL) { + return NGX_ERROR; } - else if (ret < 0) { - return ngx_http_filter_finalize_request(r, - &ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR); - + copy_buf = ngx_calloc_buf(r->pool); + if (copy_buf == NULL) { + return NGX_ERROR; } + copy_buf->pos = chain->buf->pos ; + copy_buf->end = chain->buf->end; + copy_buf->last = chain->buf->last; + copy_buf->temporary = (chain->buf->temporary == 1) ? 1 : 0; + copy_buf->memory = (chain->buf->memory == 1) ? 1 : 0; + copy_chain->buf = copy_buf; + copy_chain->buf->last_buf = chain->buf->last_buf; + copy_chain->next = NULL; + chain->buf->pos = chain->buf->last; } - } - if (!is_request_processed) - { - dd("buffer was not fully loaded! ctx: %p", ctx); + else + copy_chain = chain; + if (ctx->temp_chain == NULL) { + ctx->temp_chain = copy_chain; + } else { + if (ctx->current_chain == NULL) { + ctx->temp_chain->next = copy_chain; + ctx->temp_chain->buf->last_buf = 0; + } else { + ctx->current_chain->next = copy_chain; + ctx->current_chain->buf->last_buf = 0; + } + ctx->current_chain = copy_chain; + } + } -/* XXX: xflt_filter() -- return NGX_OK here */ - return ngx_http_next_body_filter(r, in); + if (is_request_processed) { + old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool); + msc_process_response_body(ctx->modsec_transaction); + ngx_http_modsecurity_pcre_malloc_done(old_pool); + ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0); + if (ret > 0) { + if (ret < NGX_HTTP_BAD_REQUEST && ctx->header_pt != NULL){ + ctx->header_pt(r); + } + else { + ctx->response_body_filtered = 1; + return ngx_http_filter_finalize_request(r, + &ngx_http_modsecurity_module + , ret); + } + } else if (ret < 0) { + ctx->response_body_filtered = 1; + return ngx_http_filter_finalize_request(r, + &ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR); + } + ctx->response_body_filtered = 1; + if (ctx->header_pt != NULL) + ctx->header_pt(r); + return ngx_http_next_body_filter(r, ctx->temp_chain); + } else { + return NGX_AGAIN; + } } diff --git a/src/ngx_http_modsecurity_common.h b/src/ngx_http_modsecurity_common.h index 11fdc2d7..da926c71 100644 --- a/src/ngx_http_modsecurity_common.h +++ b/src/ngx_http_modsecurity_common.h @@ -76,7 +76,6 @@ typedef struct { ngx_str_t value; } ngx_http_modsecurity_header_t; - typedef struct { ngx_http_request_t *r; Transaction *modsec_transaction; @@ -97,6 +96,10 @@ typedef struct { unsigned waiting_more_body:1; unsigned body_requested:1; unsigned processed:1; + ngx_http_output_header_filter_pt header_pt; + ngx_chain_t* temp_chain; + ngx_chain_t* current_chain; + unsigned response_body_filtered:1; unsigned logged:1; unsigned intervention_triggered:1; } ngx_http_modsecurity_ctx_t; diff --git a/src/ngx_http_modsecurity_header_filter.c b/src/ngx_http_modsecurity_header_filter.c index 257e7fdb..3cd3c971 100644 --- a/src/ngx_http_modsecurity_header_filter.c +++ b/src/ngx_http_modsecurity_header_filter.c @@ -551,10 +551,17 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r) * */ - /* - * The line below is commented to make the spdy test to work - */ - //r->headers_out.content_length_n = -1; - - return ngx_http_next_header_filter(r); + /* + * The line below is commented to make the spdy test to work + */ + //r->headers_out.content_length_n = -1; + + if (ctx->response_body_filtered){ + return ngx_http_next_header_filter(r); + } + else { + ctx->header_pt = ngx_http_next_header_filter; + r->headers_out.content_length_n = -1; + return NGX_AGAIN; + } } diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index c90b3f61..42146d50 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -292,6 +292,7 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) dd("transaction created"); + ctx->response_body_filtered = 0; ngx_http_set_ctx(r, ctx, ngx_http_modsecurity_module); cln = ngx_pool_cleanup_add(r->pool, sizeof(ngx_http_modsecurity_ctx_t)); diff --git a/tests/modsecurity-proxy.t b/tests/modsecurity-proxy.t index a412f5ea..e97800e2 100644 --- a/tests/modsecurity-proxy.t +++ b/tests/modsecurity-proxy.t @@ -114,28 +114,28 @@ unlike(http_head('/'), qr/SEE-THIS/, 'proxy head request'); # Redirect (302) -like(http_get('/phase1?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 1'); -like(http_get('/phase2?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 2'); -like(http_get('/phase3?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 3'); -is(http_get('/phase4?what=redirect302'), '', 'redirect 302 - phase 4'); +like(http_get('/phase1?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 1'); +like(http_get('/phase2?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 2'); +like(http_get('/phase3?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 3'); +like(http_get('/phase4?what=redirect302'), qr/404/, 'redirect 302 - phase 4'); # Redirect (301) -like(http_get('/phase1?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 1'); -like(http_get('/phase2?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 2'); -like(http_get('/phase3?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 3'); -is(http_get('/phase4?what=redirect301'), '', 'redirect 301 - phase 4'); +like(http_get('/phase1?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 1'); +like(http_get('/phase2?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 2'); +like(http_get('/phase3?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 3'); +like(http_get('/phase4?what=redirect301'), qr/404/, 'redirect 301 - phase 4'); # Block (401) -like(http_get('/phase1?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 1'); -like(http_get('/phase2?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 2'); -like(http_get('/phase3?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 3'); -is(http_get('/phase4?what=block401'), '', 'block 401 - phase 4'); +like(http_get('/phase1?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 1'); +like(http_get('/phase2?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 2'); +like(http_get('/phase3?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 3'); +like(http_get('/phase4?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 4'); # Block (403) -like(http_get('/phase1?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 1'); -like(http_get('/phase2?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 2'); -like(http_get('/phase3?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 3'); -is(http_get('/phase4?what=block403'), '', 'block 403 - phase 4'); +like(http_get('/phase1?what=block403'), qr/403 Forbidden/, 'block 403 - phase 1'); +like(http_get('/phase2?what=block403'), qr/403 Forbidden/, 'block 403 - phase 2'); +like(http_get('/phase3?what=block403'), qr/403 Forbidden/, 'block 403 - phase 3'); +like(http_get('/phase4?what=block403'), qr/403 Forbidden/, 'block 403 - phase 4'); # Nothing to detect like(http_get('/phase1?what=nothing'), qr/phase1\?what=nothing\' not found/, 'nothing phase 1'); diff --git a/tests/modsecurity-scoring.t b/tests/modsecurity-scoring.t index 65fcb13d..5404c471 100644 --- a/tests/modsecurity-scoring.t +++ b/tests/modsecurity-scoring.t @@ -46,7 +46,7 @@ http { SecRuleEngine On SecRule ARGS "@streq badarg1" "id:11,phase:2,setvar:tx.score=1" SecRule ARGS "@streq badarg2" "id:12,phase:2,setvar:tx.score=2" - SecRule TX:SCORE "@ge 2" "id:199,phase:request,deny,log,status:403" + SecRule tx:score "@ge 2" "id:199,phase:request,deny,log,status:403" '; } @@ -56,7 +56,7 @@ http { SecRule ARGS "@streq badarg1" "id:21,phase:2,setvar:tx.score=+1" SecRule ARGS "@streq badarg2" "id:22,phase:2,setvar:tx.score=+1" SecRule ARGS "@streq badarg3" "id:23,phase:2,setvar:tx.score=+1" - SecRule TX:SCORE "@ge 3" "id:299,phase:request,deny,log,status:403" + SecRule tx:score "@ge 3" "id:299,phase:request,deny,log,status:403" '; } } diff --git a/tests/modsecurity.t b/tests/modsecurity.t index fcb26f4f..cad4f092 100644 --- a/tests/modsecurity.t +++ b/tests/modsecurity.t @@ -139,28 +139,28 @@ $t->plan(21); # Redirect (302) -like(http_get('/phase1?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 1'); -like(http_get('/phase2?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 2'); -like(http_get('/phase3?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 3'); -is(http_get('/phase4?what=redirect302'), '', 'redirect 302 - phase 4'); +like(http_get('/phase1?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 1'); +like(http_get('/phase2?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 2'); +like(http_get('/phase3?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 3'); +like(http_get('/phase4?what=redirect302'), qr/200/, 'redirect 302 - phase 4'); # Redirect (301) -like(http_get('/phase1?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 1'); -like(http_get('/phase2?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 2'); -like(http_get('/phase3?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 3'); -is(http_get('/phase4?what=redirect301'), '', 'redirect 301 - phase 4'); +like(http_get('/phase1?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 1'); +like(http_get('/phase2?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 2'); +like(http_get('/phase3?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 3'); +like(http_get('/phase4?what=redirect301'), qr/200/, 'redirect 301 - phase 4'); # Block (401) -like(http_get('/phase1?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 1'); -like(http_get('/phase2?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 2'); -like(http_get('/phase3?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 3'); -is(http_get('/phase4?what=block401'), '', 'block 401 - phase 4'); +like(http_get('/phase1?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 1'); +like(http_get('/phase2?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 2'); +like(http_get('/phase3?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 3'); +like(http_get('/phase4?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 4'); # Block (403) -like(http_get('/phase1?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 1'); -like(http_get('/phase2?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 2'); -like(http_get('/phase3?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 3'); -is(http_get('/phase4?what=block403'), '', 'block 403 - phase 4'); +like(http_get('/phase1?what=block403'), qr/403 Forbidden/, 'block 403 - phase 1'); +like(http_get('/phase2?what=block403'), qr/403 Forbidden/, 'block 403 - phase 2'); +like(http_get('/phase3?what=block403'), qr/403 Forbidden/, 'block 403 - phase 3'); +like(http_get('/phase4?what=block403'), qr/403 Forbidden/, 'block 403 - phase 4'); # Nothing to detect like(http_get('/phase1?what=nothing'), qr/should be moved\/blocked before this./, 'nothing phase 1'); From ae30826ac09271574e5a11eda551d8c34d5c17e3 Mon Sep 17 00:00:00 2001 From: Ervin Hegedus Date: Tue, 19 Nov 2024 21:25:59 +0100 Subject: [PATCH 2/2] Picked up code from PR #273 --- src/ngx_http_modsecurity_body_filter.c | 2 +- src/ngx_http_modsecurity_common.h | 2 ++ src/ngx_http_modsecurity_header_filter.c | 20 ++++++++++---------- src/ngx_http_modsecurity_log.c | 6 +++--- src/ngx_http_modsecurity_module.c | 23 ++++++++++++++++++++++- src/ngx_http_modsecurity_pre_access.c | 10 ++++++++-- src/ngx_http_modsecurity_rewrite.c | 2 +- 7 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 27ea8d84..77c144ad 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -55,7 +55,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) } /* get context for request */ - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); dd("body filter, recovering ctx: %p", ctx); if (ctx == NULL || r->filter_finalize || ctx->response_body_filtered) { diff --git a/src/ngx_http_modsecurity_common.h b/src/ngx_http_modsecurity_common.h index da926c71..1bb243bb 100644 --- a/src/ngx_http_modsecurity_common.h +++ b/src/ngx_http_modsecurity_common.h @@ -102,6 +102,7 @@ typedef struct { unsigned response_body_filtered:1; unsigned logged:1; unsigned intervention_triggered:1; + unsigned request_body_processed:1; } ngx_http_modsecurity_ctx_t; @@ -142,6 +143,7 @@ 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); ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_create_ctx(ngx_http_request_t *r); +ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_get_module_ctx(ngx_http_request_t *r); char *ngx_str_to_char(ngx_str_t a, ngx_pool_t *p); #if (NGX_PCRE2) #define ngx_http_modsecurity_pcre_malloc_init(x) NULL diff --git a/src/ngx_http_modsecurity_header_filter.c b/src/ngx_http_modsecurity_header_filter.c index 3cd3c971..129e633f 100644 --- a/src/ngx_http_modsecurity_header_filter.c +++ b/src/ngx_http_modsecurity_header_filter.c @@ -109,7 +109,7 @@ ngx_http_modsecurity_store_ctx_header(ngx_http_request_t *r, ngx_str_t *name, ng ngx_http_modsecurity_conf_t *mcf; ngx_http_modsecurity_header_t *hdr; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (ctx == NULL || ctx->sanity_headers_out == NULL) { return NGX_ERROR; } @@ -152,7 +152,7 @@ ngx_http_modsecurity_resolv_header_server(ngx_http_request_t *r, ngx_str_t name, ngx_str_t value; clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (r->headers_out.server == NULL) { if (clcf->server_tokens) { @@ -186,7 +186,7 @@ ngx_http_modsecurity_resolv_header_date(ngx_http_request_t *r, ngx_str_t name, o ngx_http_modsecurity_ctx_t *ctx = NULL; ngx_str_t date; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (r->headers_out.date == NULL) { date.data = ngx_cached_http_time.data; @@ -216,7 +216,7 @@ ngx_http_modsecurity_resolv_header_content_length(ngx_http_request_t *r, ngx_str ngx_str_t value; char buf[NGX_INT64_LEN+2]; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (r->headers_out.content_length_n > 0) { @@ -243,7 +243,7 @@ ngx_http_modsecurity_resolv_header_content_type(ngx_http_request_t *r, ngx_str_t { ngx_http_modsecurity_ctx_t *ctx = NULL; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (r->headers_out.content_type.len > 0) { @@ -270,7 +270,7 @@ ngx_http_modsecurity_resolv_header_last_modified(ngx_http_request_t *r, ngx_str_ u_char buf[1024], *p; ngx_str_t value; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (r->headers_out.last_modified_time == -1) { return 1; @@ -302,7 +302,7 @@ ngx_http_modsecurity_resolv_header_connection(ngx_http_request_t *r, ngx_str_t n ngx_str_t value; clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (r->headers_out.status == NGX_HTTP_SWITCHING_PROTOCOLS) { connection = "upgrade"; @@ -353,7 +353,7 @@ ngx_http_modsecurity_resolv_header_transfer_encoding(ngx_http_request_t *r, ngx_ if (r->chunked) { ngx_str_t value = ngx_string("chunked"); - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); #if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) ngx_http_modsecurity_store_ctx_header(r, &name, &value); @@ -380,7 +380,7 @@ ngx_http_modsecurity_resolv_header_vary(ngx_http_request_t *r, ngx_str_t name, o if (r->gzip_vary && clcf->gzip_vary) { ngx_str_t value = ngx_string("Accept-Encoding"); - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); #if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) ngx_http_modsecurity_store_ctx_header(r, &name, &value); @@ -422,7 +422,7 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r) /* XXX: if NOT_MODIFIED, do we need to process it at all? see xslt_header_filter() */ - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); dd("header filter, recovering ctx: %p", ctx); diff --git a/src/ngx_http_modsecurity_log.c b/src/ngx_http_modsecurity_log.c index 167b2d39..3223ed96 100644 --- a/src/ngx_http_modsecurity_log.c +++ b/src/ngx_http_modsecurity_log.c @@ -60,13 +60,13 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r) return NGX_OK; } */ - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); dd("recovering ctx: %p", ctx); if (ctx == NULL) { - dd("something really bad happened here. returning NGX_ERROR"); - return NGX_ERROR; + dd("ModSecurity not enabled or error occurred"); + return NGX_OK; } if (ctx->logged) { diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 42146d50..00ee0636 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -149,7 +149,7 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re dd("processing intervention"); - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (ctx == NULL) { return NGX_HTTP_INTERNAL_SERVER_ERROR; @@ -314,6 +314,27 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) return ctx; } +ngx_inline ngx_http_modsecurity_ctx_t * +ngx_http_modsecurity_get_module_ctx(ngx_http_request_t *r) +{ + ngx_http_modsecurity_ctx_t *ctx; + ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + if (ctx == NULL) { + /* + * refer /src/http/modules/ngx_http_realip_module.c + * if module context was reset, the original address + * can still be found in the cleanup handler + */ + ngx_pool_cleanup_t *cln; + for (cln = r->pool->cleanup; cln; cln = cln->next) { + if (cln->handler == ngx_http_modsecurity_cleanup) { + ctx = cln->data; + break; + } + } + } + return ctx; +} char * ngx_conf_set_rules(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) diff --git a/src/ngx_http_modsecurity_pre_access.c b/src/ngx_http_modsecurity_pre_access.c index ea5c0215..75ac45d4 100644 --- a/src/ngx_http_modsecurity_pre_access.c +++ b/src/ngx_http_modsecurity_pre_access.c @@ -27,7 +27,7 @@ ngx_http_modsecurity_request_read(ngx_http_request_t *r) { ngx_http_modsecurity_ctx_t *ctx; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); #if defined(nginx_version) && nginx_version >= 8011 r->main->count--; @@ -70,7 +70,7 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r) } */ - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); dd("recovering ctx: %p", ctx); @@ -80,6 +80,11 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r) return NGX_HTTP_INTERNAL_SERVER_ERROR; } + if (ctx->request_body_processed) { + // should we use r->internal or r->filter_finalize? + return NGX_DECLINED; + } + if (ctx->intervention_triggered) { return NGX_DECLINED; } @@ -212,6 +217,7 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r) old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool); msc_process_request_body(ctx->modsec_transaction); + ctx->request_body_processed = 1; ngx_http_modsecurity_pcre_malloc_done(old_pool); ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0); diff --git a/src/ngx_http_modsecurity_rewrite.c b/src/ngx_http_modsecurity_rewrite.c index 926cf701..eaff1cc8 100644 --- a/src/ngx_http_modsecurity_rewrite.c +++ b/src/ngx_http_modsecurity_rewrite.c @@ -46,7 +46,7 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r) dd("catching a new _rewrite_ phase handler"); - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); dd("recovering ctx: %p", ctx);