Skip to content

Fix response body (based on #84) #326

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 1 commit into from
Jan 10, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
114 changes: 77 additions & 37 deletions src/ngx_http_modsecurity_body_filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
}

Expand Down Expand Up @@ -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;
}
}
5 changes: 4 additions & 1 deletion src/ngx_http_modsecurity_common.h
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand Down
19 changes: 13 additions & 6 deletions src/ngx_http_modsecurity_header_filter.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
1 change: 1 addition & 0 deletions src/ngx_http_modsecurity_module.c
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand Down
32 changes: 16 additions & 16 deletions tests/modsecurity-proxy.t
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down
4 changes: 2 additions & 2 deletions tests/modsecurity-scoring.t
Original file line number Diff line number Diff line change
Expand Up @@ -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"
';
}

Expand All @@ -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"
';
}
}
Expand Down
32 changes: 16 additions & 16 deletions tests/modsecurity.t
Original file line number Diff line number Diff line change
Expand Up @@ -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');
Expand Down