diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 86bccc7..27ea8d8 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 11fdc2d..da926c7 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 257e7fd..3cd3c97 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 c90b3f6..42146d5 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 a412f5e..e97800e 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 65fcb13..5404c47 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 fcb26f4..cad4f09 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');