diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index fd286d0..05fb571 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -31,12 +31,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 *loc_cf = NULL; ngx_list_part_t *part = &r->headers_out.headers.part; @@ -44,18 +46,23 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) ngx_uint_t i = 0; #endif - if (in == NULL) { +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); } + #if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) loc_cf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); if (loc_cf != NULL && loc_cf->sanity_checks_enabled != NGX_CONF_UNSET) @@ -129,52 +136,86 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) { dd("%d header(s) were not inspected by ModSecurity, so exiting", worth_to_fail); return ngx_http_filter_finalize_request(r, - &ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR); + &ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR); } } #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); + 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); if (ret > 0) { return ngx_http_filter_finalize_request(r, - &ngx_http_modsecurity_module, ret); + &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); - 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; } + 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; + } + } - if (!is_request_processed) - { - dd("buffer was not fully loaded! ctx: %p", ctx); + + 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); + 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; } - -/* XXX: xflt_filter() -- return NGX_OK here */ - return ngx_http_next_body_filter(r, in); } diff --git a/src/ngx_http_modsecurity_common.h b/src/ngx_http_modsecurity_common.h index 7108347..7bf0283 100644 --- a/src/ngx_http_modsecurity_common.h +++ b/src/ngx_http_modsecurity_common.h @@ -62,7 +62,6 @@ typedef struct { ngx_str_t value; } ngx_http_modsecurity_header_t; - typedef struct { ngx_http_request_t *r; Transaction *modsec_transaction; @@ -83,6 +82,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; } ngx_http_modsecurity_ctx_t; diff --git a/src/ngx_http_modsecurity_header_filter.c b/src/ngx_http_modsecurity_header_filter.c index 7728d82..e4d646c 100644 --- a/src/ngx_http_modsecurity_header_filter.c +++ b/src/ngx_http_modsecurity_header_filter.c @@ -542,10 +542,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 0c491cc..ab67451 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -253,7 +253,8 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) ctx->modsec_transaction = msc_new_transaction(cf->modsec, loc_cf->rules_set, r->connection->log); 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 9bd0102..50f613c 100644 --- a/tests/modsecurity-proxy.t +++ b/tests/modsecurity-proxy.t @@ -86,7 +86,8 @@ http { modsecurity on; modsecurity_rules ' SecRuleEngine On - SecDefaultAction "phase:4,log,deny,status:403" + SecResponseBodyAccess On + SecDefaultAction "phase:4,log,deny,status:403" SecRule ARGS "@streq redirect301" "id:1,phase:4,status:301,redirect:http://www.modsecurity.org" SecRule ARGS "@streq redirect302" "id:2,phase:4,status:302,redirect:http://www.modsecurity.org" SecRule ARGS "@streq block401" "id:3,phase:4,status:401,block" @@ -116,25 +117,25 @@ unlike(http_head('/'), qr/SEE-THIS/, 'proxy head request'); 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'); -is(http_get('/phase4?what=redirect302'), '', 'redirect 302 - phase 4'); +like(http_get('/phase4?what=redirect302'), qr/404/, 'redirect 302 - phase 4'); # Redirect (301) 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'); -is(http_get('/phase4?what=redirect301'), '', 'redirect 301 - phase 4'); +like(http_get('/phase4?what=redirect301'), qr/404/, 'redirect 301 - phase 4'); # Block (401) 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'); -is(http_get('/phase4?what=block401'), '', 'block 401 - phase 4'); +like(http_get('/phase4?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 4'); # Block (403) 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'); -is(http_get('/phase4?what=block403'), '', 'block 403 - phase 4'); +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 ddd3b61..33a3612 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 7079447..ca341ad 100644 --- a/tests/modsecurity.t +++ b/tests/modsecurity.t @@ -97,7 +97,8 @@ http { modsecurity on; modsecurity_rules ' SecRuleEngine On - SecDefaultAction "phase:4,log,deny,status:403" + SecResponseBodyAccess On + SecDefaultAction "phase:4,log,deny,status:403" SecRule ARGS "@streq redirect301" "id:1,phase:4,status:301,redirect:http://www.modsecurity.org" SecRule ARGS "@streq redirect302" "id:2,phase:4,status:302,redirect:http://www.modsecurity.org" SecRule ARGS "@streq block401" "id:3,phase:4,status:401,block" @@ -123,25 +124,25 @@ $t->plan(20); 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'); -is(http_get('/phase4?what=redirect302'), '', 'redirect 302 - phase 4'); +like(http_get('/phase4?what=redirect302'), qr/200/, 'redirect 302 - phase 4'); # Redirect (301) 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'); -is(http_get('/phase4?what=redirect301'), '', 'redirect 301 - phase 4'); +like(http_get('/phase4?what=redirect301'), qr/200/, 'redirect 301 - phase 4'); # Block (401) 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'); -is(http_get('/phase4?what=block401'), '', 'block 401 - phase 4'); +like(http_get('/phase4?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 4'); # Block (403) 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'); -is(http_get('/phase4?what=block403'), '', 'block 403 - phase 4'); +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');