Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Sign up
Appearance settings

Commit 62639fa

Browse files
committed
Fix #41 
1 parent ef64996 commit 62639fa

7 files changed

+129
-78
lines changed

‎src/ngx_http_modsecurity_body_filter.c‎

Lines changed: 77 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,14 @@ ngx_http_modsecurity_body_filter_init(void)
3333

3434
return NGX_OK;
3535
}
36-
3736
ngx_int_t
3837
ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
3938
{
40-
ngx_chain_t *chain = in;
4139
ngx_http_modsecurity_ctx_t *ctx = NULL;
40+
ngx_chain_t *chain = in;
41+
ngx_int_t ret;
42+
ngx_pool_t *old_pool;
43+
ngx_int_t is_request_processed = 0;
4244
#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS)
4345
ngx_http_modsecurity_conf_t *mcf;
4446
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)
4749
#endif
4850

4951
if (in == NULL) {
52+
ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "MDS input chain is null");
53+
5054
return ngx_http_next_body_filter(r, in);
5155
}
5256

57+
/* get context for request */
5358
ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module);
54-
5559
dd("body filter, recovering ctx: %p", ctx);
5660

57-
if (ctx == NULL) {
61+
if (ctx == NULL || r->filter_finalize || ctx->response_body_filtered) {
62+
if (ctx && ctx->response_body_filtered)
63+
r->filter_finalize = 1;
5864
return ngx_http_next_body_filter(r, in);
5965
}
6066

@@ -140,47 +146,81 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in)
140146
}
141147
#endif
142148

143-
int is_request_processed = 0;
144-
for (; chain != NULL; chain = chain->next)
145-
{
146-
u_char *data = chain->buf->pos;
147-
int ret;
149+
for (chain = in; chain != NULL; chain = chain->next) {
148150

149-
msc_append_response_body(ctx->modsec_transaction, data, chain->buf->last - data);
150-
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
151+
ngx_buf_t *copy_buf;
152+
ngx_chain_t* copy_chain;
153+
is_request_processed = chain->buf->last_buf;
154+
u_char *data = chain->buf->pos;
155+
msc_append_response_body(ctx->modsec_transaction, data,
156+
chain->buf->last - data);
157+
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction,
158+
r, 0);
151159
if (ret > 0) {
152160
return ngx_http_filter_finalize_request(r,
153161
&ngx_http_modsecurity_module, ret);
154162
}
155-
156-
/* XXX: chain->buf->last_buf || chain->buf->last_in_chain */
157-
is_request_processed = chain->buf->last_buf;
158-
159-
if (is_request_processed) {
160-
ngx_pool_t *old_pool;
161-
162-
old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
163-
msc_process_response_body(ctx->modsec_transaction);
164-
ngx_http_modsecurity_pcre_malloc_done(old_pool);
165-
166-
/* XXX: I don't get how body from modsec being transferred to nginx's buffer. If so - after adjusting of nginx's
167-
XXX: body we can proceed to adjust body size (content-length). see xslt_body_filter() for example */
168-
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
169-
if (ret > 0) {
170-
return ret;
163+
if (!chain->buf->last_buf){
164+
copy_chain = ngx_alloc_chain_link(r->pool);
165+
if (copy_chain == NULL) {
166+
return NGX_ERROR;
171167
}
172-
else if (ret < 0) {
173-
return ngx_http_filter_finalize_request(r,
174-
&ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR);
175-
168+
copy_buf = ngx_calloc_buf(r->pool);
169+
if (copy_buf == NULL) {
170+
return NGX_ERROR;
176171
}
172+
copy_buf->pos = chain->buf->pos ;
173+
copy_buf->end = chain->buf->end;
174+
copy_buf->last = chain->buf->last;
175+
copy_buf->temporary = (chain->buf->temporary == 1) ? 1 : 0;
176+
copy_buf->memory = (chain->buf->memory == 1) ? 1 : 0;
177+
copy_chain->buf = copy_buf;
178+
copy_chain->buf->last_buf = chain->buf->last_buf;
179+
copy_chain->next = NULL;
180+
chain->buf->pos = chain->buf->last;
177181
}
178-
}
179-
if (!is_request_processed)
180-
{
181-
dd("buffer was not fully loaded! ctx: %p", ctx);
182+
else
183+
copy_chain = chain;
184+
if (ctx->temp_chain == NULL) {
185+
ctx->temp_chain = copy_chain;
186+
} else {
187+
if (ctx->current_chain == NULL) {
188+
ctx->temp_chain->next = copy_chain;
189+
ctx->temp_chain->buf->last_buf = 0;
190+
} else {
191+
ctx->current_chain->next = copy_chain;
192+
ctx->current_chain->buf->last_buf = 0;
193+
}
194+
ctx->current_chain = copy_chain;
195+
}
196+
182197
}
183198

184-
/* XXX: xflt_filter() -- return NGX_OK here */
185-
return ngx_http_next_body_filter(r, in);
199+
if (is_request_processed) {
200+
old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool);
201+
msc_process_response_body(ctx->modsec_transaction);
202+
ngx_http_modsecurity_pcre_malloc_done(old_pool);
203+
ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0);
204+
if (ret > 0) {
205+
if (ret < NGX_HTTP_BAD_REQUEST && ctx->header_pt != NULL){
206+
ctx->header_pt(r);
207+
}
208+
else {
209+
ctx->response_body_filtered = 1;
210+
return ngx_http_filter_finalize_request(r,
211+
&ngx_http_modsecurity_module
212+
, ret);
213+
}
214+
} else if (ret < 0) {
215+
ctx->response_body_filtered = 1;
216+
return ngx_http_filter_finalize_request(r,
217+
&ngx_http_modsecurity_module, NGX_HTTP_INTERNAL_SERVER_ERROR);
218+
}
219+
ctx->response_body_filtered = 1;
220+
if (ctx->header_pt != NULL)
221+
ctx->header_pt(r);
222+
return ngx_http_next_body_filter(r, ctx->temp_chain);
223+
} else {
224+
return NGX_AGAIN;
225+
}
186226
}

‎src/ngx_http_modsecurity_common.h‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -76,7 +76,6 @@ typedef struct {
7676
ngx_str_t value;
7777
} ngx_http_modsecurity_header_t;
7878

79-
8079
typedef struct {
8180
ngx_http_request_t *r;
8281
Transaction *modsec_transaction;
@@ -97,6 +96,10 @@ typedef struct {
9796
unsigned waiting_more_body:1;
9897
unsigned body_requested:1;
9998
unsigned processed:1;
99+
ngx_http_output_header_filter_pt header_pt;
100+
ngx_chain_t* temp_chain;
101+
ngx_chain_t* current_chain;
102+
unsigned response_body_filtered:1;
100103
unsigned logged:1;
101104
unsigned intervention_triggered:1;
102105
} ngx_http_modsecurity_ctx_t;

‎src/ngx_http_modsecurity_header_filter.c‎

Lines changed: 13 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -551,10 +551,17 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r)
551551
*
552552
*/
553553

554-
/*
555-
* The line below is commented to make the spdy test to work
556-
*/
557-
//r->headers_out.content_length_n = -1;
558-
559-
return ngx_http_next_header_filter(r);
554+
/*
555+
* The line below is commented to make the spdy test to work
556+
*/
557+
//r->headers_out.content_length_n = -1;
558+
559+
if (ctx->response_body_filtered){
560+
return ngx_http_next_header_filter(r);
561+
}
562+
else {
563+
ctx->header_pt = ngx_http_next_header_filter;
564+
r->headers_out.content_length_n = -1;
565+
return NGX_AGAIN;
566+
}
560567
}

‎src/ngx_http_modsecurity_module.c‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -292,6 +292,7 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r)
292292

293293
dd("transaction created");
294294

295+
ctx->response_body_filtered = 0;
295296
ngx_http_set_ctx(r, ctx, ngx_http_modsecurity_module);
296297

297298
cln = ngx_pool_cleanup_add(r->pool, sizeof(ngx_http_modsecurity_ctx_t));

‎tests/modsecurity-proxy.t‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -114,28 +114,28 @@ unlike(http_head('/'), qr/SEE-THIS/, 'proxy head request');
114114

115115

116116
# Redirect (302)
117-
like(http_get('/phase1?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 1');
118-
like(http_get('/phase2?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 2');
119-
like(http_get('/phase3?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 3');
120-
is(http_get('/phase4?what=redirect302'), '', 'redirect 302 - phase 4');
117+
like(http_get('/phase1?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 1');
118+
like(http_get('/phase2?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 2');
119+
like(http_get('/phase3?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 3');
120+
like(http_get('/phase4?what=redirect302'), qr/404/, 'redirect 302 - phase 4');
121121

122122
# Redirect (301)
123-
like(http_get('/phase1?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 1');
124-
like(http_get('/phase2?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 2');
125-
like(http_get('/phase3?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 3');
126-
is(http_get('/phase4?what=redirect301'), '', 'redirect 301 - phase 4');
123+
like(http_get('/phase1?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 1');
124+
like(http_get('/phase2?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 2');
125+
like(http_get('/phase3?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 3');
126+
like(http_get('/phase4?what=redirect301'), qr/404/, 'redirect 301 - phase 4');
127127

128128
# Block (401)
129-
like(http_get('/phase1?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 1');
130-
like(http_get('/phase2?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 2');
131-
like(http_get('/phase3?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 3');
132-
is(http_get('/phase4?what=block401'), '', 'block 401 - phase 4');
129+
like(http_get('/phase1?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 1');
130+
like(http_get('/phase2?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 2');
131+
like(http_get('/phase3?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 3');
132+
like(http_get('/phase4?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 4');
133133

134134
# Block (403)
135-
like(http_get('/phase1?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 1');
136-
like(http_get('/phase2?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 2');
137-
like(http_get('/phase3?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 3');
138-
is(http_get('/phase4?what=block403'), '', 'block 403 - phase 4');
135+
like(http_get('/phase1?what=block403'), qr/403 Forbidden/, 'block 403 - phase 1');
136+
like(http_get('/phase2?what=block403'), qr/403 Forbidden/, 'block 403 - phase 2');
137+
like(http_get('/phase3?what=block403'), qr/403 Forbidden/, 'block 403 - phase 3');
138+
like(http_get('/phase4?what=block403'), qr/403 Forbidden/, 'block 403 - phase 4');
139139

140140
# Nothing to detect
141141
like(http_get('/phase1?what=nothing'), qr/phase1\?what=nothing\' not found/, 'nothing phase 1');

‎tests/modsecurity-scoring.t‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ http {
4646
SecRuleEngine On
4747
SecRule ARGS "@streq badarg1" "id:11,phase:2,setvar:tx.score=1"
4848
SecRule ARGS "@streq badarg2" "id:12,phase:2,setvar:tx.score=2"
49-
SecRule TX:SCORE "@ge 2" "id:199,phase:request,deny,log,status:403"
49+
SecRule tx:score "@ge 2" "id:199,phase:request,deny,log,status:403"
5050
';
5151
}
5252
@@ -56,7 +56,7 @@ http {
5656
SecRule ARGS "@streq badarg1" "id:21,phase:2,setvar:tx.score=+1"
5757
SecRule ARGS "@streq badarg2" "id:22,phase:2,setvar:tx.score=+1"
5858
SecRule ARGS "@streq badarg3" "id:23,phase:2,setvar:tx.score=+1"
59-
SecRule TX:SCORE "@ge 3" "id:299,phase:request,deny,log,status:403"
59+
SecRule tx:score "@ge 3" "id:299,phase:request,deny,log,status:403"
6060
';
6161
}
6262
}

‎tests/modsecurity.t‎

Lines changed: 16 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -139,28 +139,28 @@ $t->plan(21);
139139

140140

141141
# Redirect (302)
142-
like(http_get('/phase1?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 1');
143-
like(http_get('/phase2?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 2');
144-
like(http_get('/phase3?what=redirect302'), qr/^HTTP.*302/, 'redirect 302 - phase 3');
145-
is(http_get('/phase4?what=redirect302'), '', 'redirect 302 - phase 4');
142+
like(http_get('/phase1?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 1');
143+
like(http_get('/phase2?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 2');
144+
like(http_get('/phase3?what=redirect302'), qr/302 Moved Temporarily/, 'redirect 302 - phase 3');
145+
like(http_get('/phase4?what=redirect302'), qr/200/, 'redirect 302 - phase 4');
146146

147147
# Redirect (301)
148-
like(http_get('/phase1?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 1');
149-
like(http_get('/phase2?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 2');
150-
like(http_get('/phase3?what=redirect301'), qr/^HTTP.*301/, 'redirect 301 - phase 3');
151-
is(http_get('/phase4?what=redirect301'), '', 'redirect 301 - phase 4');
148+
like(http_get('/phase1?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 1');
149+
like(http_get('/phase2?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 2');
150+
like(http_get('/phase3?what=redirect301'), qr/301 Moved Permanently/, 'redirect 301 - phase 3');
151+
like(http_get('/phase4?what=redirect301'), qr/200/, 'redirect 301 - phase 4');
152152

153153
# Block (401)
154-
like(http_get('/phase1?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 1');
155-
like(http_get('/phase2?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 2');
156-
like(http_get('/phase3?what=block401'), qr/^HTTP.*401/, 'block 401 - phase 3');
157-
is(http_get('/phase4?what=block401'), '', 'block 401 - phase 4');
154+
like(http_get('/phase1?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 1');
155+
like(http_get('/phase2?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 2');
156+
like(http_get('/phase3?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 3');
157+
like(http_get('/phase4?what=block401'), qr/401 Unauthorized/, 'block 401 - phase 4');
158158

159159
# Block (403)
160-
like(http_get('/phase1?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 1');
161-
like(http_get('/phase2?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 2');
162-
like(http_get('/phase3?what=block403'), qr/^HTTP.*403/, 'block 403 - phase 3');
163-
is(http_get('/phase4?what=block403'), '', 'block 403 - phase 4');
160+
like(http_get('/phase1?what=block403'), qr/403 Forbidden/, 'block 403 - phase 1');
161+
like(http_get('/phase2?what=block403'), qr/403 Forbidden/, 'block 403 - phase 2');
162+
like(http_get('/phase3?what=block403'), qr/403 Forbidden/, 'block 403 - phase 3');
163+
like(http_get('/phase4?what=block403'), qr/403 Forbidden/, 'block 403 - phase 4');
164164

165165
# Nothing to detect
166166
like(http_get('/phase1?what=nothing'), qr/should be moved\/blocked before this./, 'nothing phase 1');

0 commit comments

Comments
(0)

AltStyle によって変換されたページ (->オリジナル) /