From 0c8a5b2af0501ccd7bb5b426f65f2e2da5fb7417 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Tue, 29 Apr 2014 07:23:37 -0700 Subject: [PATCH] nginx: copies the req body chain to be processed instead of move Add a check for the definition MOVE_REQUEST_CHAIN_TO_MODSEC, whenever it is set the chain will be moved into the brigade. If it was not set the chain will be only copied. Moving was causing segfaults on the following regression tests: #15 - SecRequestBodyInMemoryLimit #16 - SecRequestBodyInMemoryLimit (greater) #19 - SecRequestBodyLimitAction ProcessPartial (multipart/greater - chunked) (from: regression/config/10-request-directives.t) --- nginx/modsecurity/apr_bucket_nginx.c | 30 ++++++ nginx/modsecurity/apr_bucket_nginx.h | 3 + nginx/modsecurity/ngx_http_modsecurity.c | 111 ++++++++++++++++------- 3 files changed, 113 insertions(+), 31 deletions(-) diff --git a/nginx/modsecurity/apr_bucket_nginx.c b/nginx/modsecurity/apr_bucket_nginx.c index 8dc98d50..e113774b 100644 --- a/nginx/modsecurity/apr_bucket_nginx.c +++ b/nginx/modsecurity/apr_bucket_nginx.c @@ -154,6 +154,36 @@ ngx_buf_t * apr_bucket_to_ngx_buf(apr_bucket *e, ngx_pool_t *pool) { } ngx_int_t +copy_chain_to_brigade(ngx_chain_t *chain_orig, apr_bucket_brigade *bb, ngx_pool_t *pool, ngx_int_t last_buf) { + apr_bucket *e; + + ngx_chain_t *chain = chain_orig; + while (chain) { + e = ngx_buf_to_apr_bucket(chain->buf, bb->p, bb->bucket_alloc); + if (e == NULL) { + return NGX_ERROR; + } + + APR_BRIGADE_INSERT_TAIL(bb, e); + if (chain->buf->last_buf) { + e = apr_bucket_eos_create(bb->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(bb, e); + return NGX_OK; + } + chain = chain->next; + } + + if (last_buf) { + e = apr_bucket_eos_create(bb->bucket_alloc); + APR_BRIGADE_INSERT_TAIL(bb, e); + return NGX_OK; + } + + return NGX_AGAIN; +} + + +ngx_int_t move_chain_to_brigade(ngx_chain_t *chain_orig, apr_bucket_brigade *bb, ngx_pool_t *pool, ngx_int_t last_buf) { apr_bucket *e; ngx_chain_t *cl; diff --git a/nginx/modsecurity/apr_bucket_nginx.h b/nginx/modsecurity/apr_bucket_nginx.h index e37f9f78..a3dd5598 100644 --- a/nginx/modsecurity/apr_bucket_nginx.h +++ b/nginx/modsecurity/apr_bucket_nginx.h @@ -13,6 +13,9 @@ apr_bucket * apr_bucket_nginx_make(apr_bucket *e, ngx_buf_t *buf, ngx_buf_t * apr_bucket_to_ngx_buf(apr_bucket *e, ngx_pool_t *pool); +ngx_int_t copy_chain_to_brigade(ngx_chain_t *chain_orig, + apr_bucket_brigade *bb, ngx_pool_t *pool, ngx_int_t last_buf); + ngx_int_t move_chain_to_brigade(ngx_chain_t *chain, apr_bucket_brigade *bb, ngx_pool_t *pool, ngx_int_t last_buf); ngx_int_t move_brigade_to_chain(apr_bucket_brigade *bb, ngx_chain_t **chain, ngx_pool_t *pool); diff --git a/nginx/modsecurity/ngx_http_modsecurity.c b/nginx/modsecurity/ngx_http_modsecurity.c index 8f17ea30..eff0fa51 100644 --- a/nginx/modsecurity/ngx_http_modsecurity.c +++ b/nginx/modsecurity/ngx_http_modsecurity.c @@ -18,7 +18,6 @@ #include - /* Those are defined twice, lets keep it defined just once by `undef` * the first one. */ @@ -31,6 +30,29 @@ #define NOTE_NGINX_REQUEST_CTX "nginx-ctx" #define TXID_SIZE 25 +/* + * If MOVE_REQUEST_CHAIN_TO_MODSEC is set, the chain will be moved into the + * ModSecurity brigade, after be processed by ModSecurity, it needs to be moved + * to nginx chains again, in order to be processed by other modules. It seems + * that this is not working well whenever the request body is delivered into + * chunks. Resulting in segfaults. + * + * If MOVE_REQUEST_CHAIN_TO_MODSEC is _not_ set, a copy of the request body + * will be delivered to ModSecurity and after processed it will be released, + * not modifying the nginx chain. + * + * The malfunctioning can be observer while running the following regression + * tests: + * #15 - SecRequestBodyInMemoryLimit + * #16 - SecRequestBodyInMemoryLimit (greater) + * #19 - SecRequestBodyLimitAction ProcessPartial (multipart/greater - chunked) + * (from: regression/config/10-request-directives.t) + */ +/* +#define MOVE_REQUEST_CHAIN_TO_MODSEC +*/ + + #define tuxb /* @@ -492,8 +514,8 @@ ngx_http_modsecurity_save_headers_in_visitor(void *data, const char *key, const static ngx_inline ngx_int_t ngx_http_modsecurity_load_request_body(ngx_http_request_t *r) { - ngx_http_modsecurity_ctx_t *ctx; - + ngx_http_modsecurity_ctx_t *ctx; + ngx_chain_t *chain; ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSec: loading request body."); @@ -502,14 +524,16 @@ ngx_http_modsecurity_load_request_body(ngx_http_request_t *r) modsecSetBodyBrigade(ctx->req, ctx->brigade); - if (r->request_body == NULL || r->request_body->bufs == NULL) { - ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "ModSec: request was null."); - - return move_chain_to_brigade(NULL, ctx->brigade, r->pool, 1); + if (r->request_body == NULL) { + chain = NULL; + } + else { + chain = r->request_body->bufs; } - if (move_chain_to_brigade(r->request_body->bufs, ctx->brigade, r->pool, 1) != NGX_OK) { + +#ifdef MOVE_REQUEST_CHAIN_TO_MODSEC + if (move_chain_to_brigade(chain, ctx->brigade, r->pool, 1) != NGX_OK) { ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSec: failed to move chain to brigade."); @@ -517,6 +541,14 @@ ngx_http_modsecurity_load_request_body(ngx_http_request_t *r) } r->request_body = NULL; +#else + if (copy_chain_to_brigade(chain, ctx->brigade, r->pool, 1) != NGX_OK) { + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: failed to copy chain to brigade."); + + return NGX_ERROR; + } +#endif return NGX_OK; } @@ -524,11 +556,15 @@ ngx_http_modsecurity_load_request_body(ngx_http_request_t *r) static ngx_inline ngx_int_t ngx_http_modsecurity_save_request_body(ngx_http_request_t *r) { - ngx_http_modsecurity_ctx_t *ctx; - apr_off_t content_length; - ngx_buf_t *buf; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity); + ngx_http_modsecurity_ctx_t *ctx; +#ifdef MOVE_REQUEST_CHAIN_TO_MODSEC + apr_off_t content_length; + ngx_buf_t *buf; +#endif + ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity); + +#ifdef MOVE_REQUEST_CHAIN_TO_MODSEC apr_brigade_length(ctx->brigade, 0, &content_length); buf = ngx_create_temp_buf(ctx->r->pool, (size_t) content_length); @@ -550,10 +586,15 @@ ngx_http_modsecurity_save_request_body(ngx_http_request_t *r) } - r->headers_in.content_length_n = content_length; - ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSec: Content length: %O, Content length n: %O", content_length, r->headers_in.content_length_n); + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: Content length: %O, Content length n: %O", content_length, + r->headers_in.content_length_n); +#else + apr_brigade_cleanup(ctx->brigade); +#endif + return NGX_OK; } @@ -787,7 +828,7 @@ ngx_http_modsecurity_status(ngx_http_request_t *r, int status) * @param r pointer to ngx_http_request_t. * */ -static void +ngx_int_t ngx_http_modsecurity_process_request(ngx_http_request_t *r) { ngx_int_t rc = 0; @@ -802,15 +843,13 @@ ngx_http_modsecurity_process_request(ngx_http_request_t *r) if (ngx_http_modsecurity_load_request(r) != NGX_OK) { ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSec: failed while loading the request."); - ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); - goto terminate; + return NGX_HTTP_INTERNAL_SERVER_ERROR; } if (ngx_http_modsecurity_load_headers_in(r) != NGX_OK) { ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSec: failed while loading the headers."); - ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); - goto terminate; + return NGX_HTTP_INTERNAL_SERVER_ERROR; } rc = modsecProcessRequestHeaders(ctx->req); @@ -818,7 +857,7 @@ ngx_http_modsecurity_process_request(ngx_http_request_t *r) if (rc != NGX_DECLINED) { ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSec: failed while processing the request headers"); - ngx_http_finalize_request(r, rc); + return rc; } /* Here we check if ModSecurity is enabled or disabled again. @@ -841,12 +880,15 @@ ngx_http_modsecurity_process_request(ngx_http_request_t *r) } if (load_request_body == 1) { + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: loading request body..."); + rc = ngx_http_modsecurity_load_request_body(r); if (rc != NGX_OK) { ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSec: failed while loading the request body."); - ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); + return NGX_HTTP_INTERNAL_SERVER_ERROR; } @@ -862,24 +904,23 @@ ngx_http_modsecurity_process_request(ngx_http_request_t *r) "ModSec: finalizing the connection after process the " \ "request body."); - ngx_http_finalize_request(r, rc); + return rc; } if (load_request_body == 1) { if (ngx_http_modsecurity_save_request_body(r) != NGX_OK) { ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSec: failed while saving the request body"); - ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); + return NGX_HTTP_INTERNAL_SERVER_ERROR; } if (ngx_http_modsecurity_save_headers_in(r) != NGX_OK) { ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSec: failed while saving the headers in"); - ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); + return NGX_HTTP_INTERNAL_SERVER_ERROR; } } -terminate: - return; + return NGX_OK; } @@ -1102,14 +1143,22 @@ ngx_http_modsecurity_handler(ngx_http_request_t *r) { ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSec: request is ready to be processed."); - ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "ModSec: chuncked? %d", r->chunked); - ngx_http_modsecurity_process_request(r); + rc = ngx_http_modsecurity_process_request(r); ctx->request_processed = 1; + + if (rc == NGX_ERROR || rc >= NGX_HTTP_SPECIAL_RESPONSE) { + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: returning a special response after process " \ + "a request: %d", rc); + + return rc; + } + + } ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "ModSec: returning NGX_OK. Count++ :P" ); + "ModSec: returning NGX_DECLINED." ); return NGX_DECLINED; }