Fix response body

src/ngx_http_modsecurity_body_filter.c

@@ -31,31 +31,38 @@ 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;
     ngx_table_elt_t *data = part->elts;
     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);
 }

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;
 
 

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;
+     }
 }

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));

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');

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"
             ';
         }
     }

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');