From e53f989142ca0e755c03deab09df400b0e7f23d5 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Mon, 28 Apr 2014 06:28:11 -0700 Subject: [PATCH 01/16] nginx refactoring Refactoring on the nginx module, including: - Better handling larger posts; - Now using nginx echo module during the regression tests. - Better interacting with neginx chain rules - Separation of the request handling and content filters. - Better handling nginx sessions and resource counts to allow a more efficient garbage collector. - Handling both http/1.0 and 1.1, including keep-alive. - Tests are now capable to test nginx as a proxy or end-server. - Tested agains nginx 1.6 and 1.7. --- nginx/modsecurity/apr_bucket_nginx.c | 11 +- nginx/modsecurity/ngx_http_modsecurity.c | 510 ++++++++++-------- tests/regression/misc/00-phases.t | 10 +- tests/regression/misc/25-libinjection.t | 8 +- .../regression/nginx/conf/nginx.conf.template | 41 +- 5 files changed, 351 insertions(+), 229 deletions(-) diff --git a/nginx/modsecurity/apr_bucket_nginx.c b/nginx/modsecurity/apr_bucket_nginx.c index 62eb59a733..8dc98d5053 100644 --- a/nginx/modsecurity/apr_bucket_nginx.c +++ b/nginx/modsecurity/apr_bucket_nginx.c @@ -154,9 +154,11 @@ ngx_buf_t * apr_bucket_to_ngx_buf(apr_bucket *e, ngx_pool_t *pool) { } ngx_int_t -move_chain_to_brigade(ngx_chain_t *chain, apr_bucket_brigade *bb, ngx_pool_t *pool, ngx_int_t last_buf) { +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; + ngx_chain_t *cl; + + ngx_chain_t *chain = chain_orig; while (chain) { e = ngx_buf_to_apr_bucket(chain->buf, bb->p, bb->bucket_alloc); @@ -168,7 +170,6 @@ move_chain_to_brigade(ngx_chain_t *chain, apr_bucket_brigade *bb, ngx_pool_t *po if (chain->buf->last_buf) { e = apr_bucket_eos_create(bb->bucket_alloc); APR_BRIGADE_INSERT_TAIL(bb, e); - chain->buf->last_buf = 0; return NGX_OK; } cl = chain; @@ -181,7 +182,6 @@ move_chain_to_brigade(ngx_chain_t *chain, apr_bucket_brigade *bb, ngx_pool_t *po APR_BRIGADE_INSERT_TAIL(bb, e); return NGX_OK; } - return NGX_AGAIN; } @@ -215,8 +215,10 @@ move_brigade_to_chain(apr_bucket_brigade *bb, ngx_chain_t **ll, ngx_pool_t *pool } cl->buf->last_buf = 1; + cl->next = NULL; *ll = cl; } else { + cl->next = NULL; cl->buf->last_buf = 1; } apr_brigade_cleanup(bb); @@ -243,6 +245,7 @@ move_brigade_to_chain(apr_bucket_brigade *bb, ngx_chain_t **ll, ngx_pool_t *pool ll = &cl->next; } + apr_brigade_cleanup(bb); /* no eos or error */ return NGX_ERROR; diff --git a/nginx/modsecurity/ngx_http_modsecurity.c b/nginx/modsecurity/ngx_http_modsecurity.c index 7c1395315b..fc8555f52b 100644 --- a/nginx/modsecurity/ngx_http_modsecurity.c +++ b/nginx/modsecurity/ngx_http_modsecurity.c @@ -18,6 +18,10 @@ #include + +/* Those are defined twice, lets keep it defined just once by `undef` + * the first one. + */ #undef CR #undef LF #undef CRLF @@ -25,22 +29,42 @@ #include "api.h" #define NOTE_NGINX_REQUEST_CTX "nginx-ctx" +#define TXID_SIZE 25 + +#define tuxb + +/* + * If NGX_HTTP_MODSECURITY_PREACCESS_HANDLER_ONLY is defined, this module will + * not process the body and header filter, instead it will only process the + * preaccess request handler. This is useful to debug the module and check if + * reference counters (r->main->count) and connections are being handled as + * expected. However, do not expect to have ModSecurity fully workable with + * this variable set. Use only for debugging. + * + */ +/* +#define NGX_HTTP_MODSECURITY_PREACCESS_HANDLER_ONLY + */ typedef struct { - ngx_flag_t enable; - directory_config *config; + ngx_flag_t enable; + directory_config *config; - ngx_str_t *file; - ngx_uint_t line; + ngx_str_t *file; + ngx_uint_t line; } ngx_http_modsecurity_loc_conf_t; typedef struct { - ngx_http_request_t *r; - conn_rec *connection; - request_rec *req; + ngx_http_request_t *r; + conn_rec *connection; + request_rec *req; + + apr_bucket_brigade *brigade; - apr_bucket_brigade *brigade; - unsigned complete; + unsigned complete:1; + unsigned request_processed:1; + unsigned waiting_more_body:1; + unsigned body_requested:1; } ngx_http_modsecurity_ctx_t; @@ -48,14 +72,19 @@ typedef struct { ** Module's registred function/handlers. */ static ngx_int_t ngx_http_modsecurity_handler(ngx_http_request_t *r); -static void ngx_http_modsecurity_body_handler(ngx_http_request_t *r); +static void ngx_http_modsecurity_request_read(ngx_http_request_t *r); + +#ifndef NGX_HTTP_MODSECURITY_PREACCESS_HANDLER_ONLY static ngx_int_t ngx_http_modsecurity_header_filter(ngx_http_request_t *r); static ngx_int_t ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in); +#endif static ngx_int_t ngx_http_modsecurity_preconfiguration(ngx_conf_t *cf); static ngx_int_t ngx_http_modsecurity_init(ngx_conf_t *cf); static ngx_int_t ngx_http_modsecurity_init_process(ngx_cycle_t *cycle); + static void *ngx_http_modsecurity_create_loc_conf(ngx_conf_t *cf); static char *ngx_http_modsecurity_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child); + static char *ngx_http_modsecurity_config(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); static char *ngx_http_modsecurity_enable(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); @@ -67,9 +96,25 @@ static void ngx_http_modsecurity_cleanup(void *data); static int ngx_http_modsecurity_save_headers_in_visitor(void *data, const char *key, const char *value); static int ngx_http_modsecurity_save_headers_out_visitor(void *data, const char *key, const char *value); +/* +** This is a temporary hack to make PCRE work with ModSecurity +** nginx hijacks pcre_malloc and pcre_free, so we have to re-hijack them +*/ +extern apr_pool_t *pool; +static void * +modsec_pcre_malloc(size_t size) +{ + return apr_palloc(pool, size); +} + +static void +modsec_pcre_free(void *ptr) +{ +} + +static server_rec *modsec_server = NULL; -/* command handled by the module */ -static ngx_command_t ngx_http_modsecurity_commands[] = { +static ngx_command_t ngx_http_modsecurity_commands[] = { { ngx_string("ModSecurityConfig"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE1, ngx_http_modsecurity_config, @@ -86,10 +131,6 @@ static ngx_command_t ngx_http_modsecurity_commands[] = { ngx_null_command }; -/* -** handlers for configuration phases of the module -*/ - static ngx_http_module_t ngx_http_modsecurity_ctx = { ngx_http_modsecurity_preconfiguration, /* preconfiguration */ ngx_http_modsecurity_init, /* postconfiguration */ @@ -104,7 +145,6 @@ static ngx_http_module_t ngx_http_modsecurity_ctx = { ngx_http_modsecurity_merge_loc_conf /* merge location configuration */ }; - ngx_module_t ngx_http_modsecurity = { NGX_MODULE_V1, &ngx_http_modsecurity_ctx, /* module context */ @@ -120,9 +160,11 @@ ngx_module_t ngx_http_modsecurity = { NGX_MODULE_V1_PADDING }; -static ngx_http_output_header_filter_pt ngx_http_next_header_filter; -static ngx_http_output_body_filter_pt ngx_http_next_body_filter; +#ifndef NGX_HTTP_MODSECURITY_PREACCESS_HANDLER_ONLY +static ngx_http_output_header_filter_pt ngx_http_next_header_filter; +static ngx_http_output_body_filter_pt ngx_http_next_body_filter; +#endif static ngx_http_upstream_t ngx_http_modsecurity_upstream; static struct { @@ -140,7 +182,6 @@ static struct { {NULL, ngx_null_string} }; - static inline u_char * ngx_pstrdup0(ngx_pool_t *pool, ngx_str_t *src) { @@ -148,7 +189,7 @@ ngx_pstrdup0(ngx_pool_t *pool, ngx_str_t *src) dst = ngx_pnalloc(pool, src->len + 1); if (dst == NULL) { - return NULL; + NGX_HTTP_MODSECURITY_ return NULL; } ngx_memcpy(dst, src->data, src->len); @@ -157,7 +198,6 @@ ngx_pstrdup0(ngx_pool_t *pool, ngx_str_t *src) return dst; } - static inline int ngx_http_modsecurity_method_number(unsigned int nginx) { @@ -329,8 +369,6 @@ ngx_http_modsecurity_load_headers_in(ngx_http_request_t *r) req->user = (char *)ngx_pstrdup0(r->pool, &r->headers_in.user); - - ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSecurity: load headers in done"); @@ -346,7 +384,6 @@ ngx_http_modsecurity_save_headers_in(ngx_http_request_t *r) /* clean up headers_in */ ngx_memzero(&r->headers_in, sizeof(ngx_http_headers_in_t)); - if (ngx_list_init(&r->headers_in.headers, r->pool, 20, sizeof(ngx_table_elt_t)) != NGX_OK) @@ -457,16 +494,25 @@ ngx_http_modsecurity_load_request_body(ngx_http_request_t *r) { ngx_http_modsecurity_ctx_t *ctx; + + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: loading request body."); + ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity); 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 (move_chain_to_brigade(r->request_body->bufs, 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."); + return NGX_ERROR; } @@ -475,106 +521,25 @@ ngx_http_modsecurity_load_request_body(ngx_http_request_t *r) return NGX_OK; } - 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; - ngx_http_core_srv_conf_t *cscf; - size_t size; - ngx_http_connection_t *hc; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity); apr_brigade_length(ctx->brigade, 0, &content_length); - if (r->header_in->end - r->header_in->last >= content_length) { - /* use r->header_in */ - - if (ngx_buf_size(r->header_in)) { - /* move to the end */ - ngx_memmove(r->header_in->pos + content_length, - r->header_in->pos, - ngx_buf_size(r->header_in)); - } - - if (apr_brigade_flatten(ctx->brigade, - (char *)r->header_in->pos, - (apr_size_t *)&content_length) != APR_SUCCESS) { - return NGX_ERROR; - } - - apr_brigade_cleanup(ctx->brigade); - - r->header_in->last += content_length; - - return NGX_OK; - } - - if (ngx_buf_size(r->header_in)) { - - /* - * ngx_http_set_keepalive will reuse r->header_in if - * (r->header_in != c->buffer && r->header_in.last != r->header_in.end), - * so we need this code block. - * see ngx_http_set_keepalive, ngx_http_alloc_large_header_buffer - */ - cscf = ngx_http_get_module_srv_conf(r, ngx_http_core_module); - - size = ngx_max(cscf->large_client_header_buffers.size, - (size_t)content_length + ngx_buf_size(r->header_in)); - - hc = r->http_connection; - - if (hc->nfree && size == cscf->large_client_header_buffers.size) { - - buf = hc->free[--hc->nfree]; - - ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "ModSecurity: use http free large header buffer: %p %uz", - buf->pos, buf->end - buf->last); - - } else if (hc->nbusy < cscf->large_client_header_buffers.num) { - - if (hc->busy == NULL) { - hc->busy = ngx_palloc(r->connection->pool, - cscf->large_client_header_buffers.num * sizeof(ngx_buf_t *)); - } - - if (hc->busy == NULL) { - return NGX_ERROR; - } else { - buf = ngx_create_temp_buf(r->connection->pool, size); - } - } else { - /* TODO: how to deal this case ? */ - return NGX_ERROR; - } - - } else { - - buf = ngx_create_temp_buf(r->pool, (size_t) content_length); - } - - if (buf == NULL) { - return NGX_ERROR; - } - - if (apr_brigade_flatten(ctx->brigade, (char *)buf->pos, - (apr_size_t *)&content_length) != APR_SUCCESS) { - return NGX_ERROR; - } - + buf = ngx_create_temp_buf(ctx->r->pool, (size_t) content_length); + apr_brigade_flatten(ctx->brigade, (char *)buf->pos, (apr_size_t *)&content_length); apr_brigade_cleanup(ctx->brigade); buf->last += content_length; - - ngx_memcpy(buf->last, r->header_in->pos, ngx_buf_size(r->header_in)); - buf->last += ngx_buf_size(r->header_in); - r->header_in = buf; + 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); return NGX_OK; } @@ -797,6 +762,114 @@ ngx_http_modsecurity_status(ngx_http_request_t *r, int status) } +/* + * @brief Process request whenever `request header` and `body` are ready for. + * + * This function provides calls ModSecurity standalone library in the necessary + * order to analisys: `request headers` and `request body`. + * + * @note ModSecurity state can be changed by ctl:ruleEngine=off + * + * @param r pointer to ngx_http_request_t. + * + */ +static void +ngx_http_modsecurity_process_request(ngx_http_request_t *r) +{ + ngx_int_t rc = 0; + ngx_http_modsecurity_ctx_t *ctx; + int load_request_body = 0; + + ctx = ngx_http_get_module_pool_ctx(r, ngx_http_modsecurity); + + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: starting to process the request..."); + + 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; + } + + 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; + } + + rc = modsecProcessRequestHeaders(ctx->req); + rc = ngx_http_modsecurity_status(r, rc); + 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); + } + + /* Here we check if ModSecurity is enabled or disabled again. + * This is a kind of hack. ModSecurity can be disable by: + * ctl:ruleEngine=off, which shouldn't be a problem. However, due to + * the way that standalone is handling this situation this lead us to + * a problem that may end up by freezing the connection. As we don't + * want that we verify if it was disabled prior to load the request body. + * + * Changes on standalone should be coordenated with others like IIS, so + * leaving this dirty hack here. + */ + if (r->method == NGX_HTTP_POST && + modsecIsRequestBodyAccessEnabled(ctx->req) && + modsecContextState(ctx->req) != MODSEC_DISABLED) { + + load_request_body = 1; + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: request body will be processed..."); + } + + if (load_request_body == 1) { + 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); + } + + + } + + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: processing request body..."); + rc = modsecProcessRequestBody(ctx->req); + rc = ngx_http_modsecurity_status(r, rc); + if (rc != NGX_DECLINED) + { + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: finalizing the connection after process the " \ + "request body."); + + ngx_http_finalize_request(r, 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); + } + 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); + } + } +terminate: + return; +} + + + /* create loc conf struct */ static void * ngx_http_modsecurity_create_loc_conf(ngx_conf_t *cf) @@ -849,24 +922,6 @@ modsecLog(void *obj, int level, char *str) } } -/* -** This is a temporary hack to make PCRE work with ModSecurity -** nginx hijacks pcre_malloc and pcre_free, so we have to re-hijack them -*/ -extern apr_pool_t *pool; - -static void * -modsec_pcre_malloc(size_t size) -{ - return apr_palloc(pool, size); -} - -static void -modsec_pcre_free(void *ptr) -{ -} - -static server_rec *modsec_server = NULL; static ngx_int_t ngx_http_modsecurity_preconfiguration(ngx_conf_t *cf) @@ -906,7 +961,6 @@ ngx_http_modsecurity_terminate(ngx_cycle_t *cycle) } } - static ngx_int_t ngx_http_modsecurity_init(ngx_conf_t *cf) { @@ -917,24 +971,29 @@ ngx_http_modsecurity_init(ngx_conf_t *cf) cmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_core_module); + h = ngx_array_push(&cmcf->phases[NGX_HTTP_PREACCESS_PHASE].handlers); if (h == NULL) { return NGX_ERROR; } + *h = ngx_http_modsecurity_handler; +#ifndef NGX_HTTP_MODSECURITY_PREACCESS_HANDLER_ONLY ngx_http_next_header_filter = ngx_http_top_header_filter; ngx_http_top_header_filter = ngx_http_modsecurity_header_filter; ngx_http_next_body_filter = ngx_http_top_body_filter; ngx_http_top_body_filter = ngx_http_modsecurity_body_filter; +#endif ngx_memzero(&ngx_http_modsecurity_upstream, sizeof(ngx_http_upstream_t)); +#ifdef MODSEC_UPSTREAM_CACHABLE ngx_http_modsecurity_upstream.cacheable = 1; +#endif return NGX_OK; } - static ngx_int_t ngx_http_modsecurity_init_process(ngx_cycle_t *cycle) { @@ -944,119 +1003,122 @@ ngx_http_modsecurity_init_process(ngx_cycle_t *cycle) return NGX_OK; } - -/* -** [ENTRY POINT] does : this function called by nginx from the request handler -*/ static ngx_int_t -ngx_http_modsecurity_handler(ngx_http_request_t *r) -{ - ngx_http_modsecurity_loc_conf_t *cf; - ngx_http_modsecurity_ctx_t *ctx; - ngx_int_t rc; +ngx_http_modsecurity_handler(ngx_http_request_t *r) { + ngx_int_t rc = NULL; + ngx_http_modsecurity_ctx_t *ctx = NULL; + ngx_http_modsecurity_loc_conf_t *cf = NULL; + + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: Catching a new access phase handler. Count: %d", r->main->count); cf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity); /* Process only main request */ if (r != r->main || !cf->enable) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: ModSecurity is not enabled or not a main request."); + return NGX_DECLINED; } - ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "modSecurity: handler"); + ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity); + + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: Recovering ctx: %p", ctx); + + if (ctx == NULL) { + ctx = ngx_http_modsecurity_create_ctx(r); - /* create / retrive request ctx */ - if (r->internal) { - - ctx = ngx_http_get_module_pool_ctx(r, ngx_http_modsecurity); + ngx_http_set_ctx(r, ctx, ngx_http_modsecurity); - if (ctx) { - /* we have already processed the request headers */ - ngx_http_set_ctx(r, ctx, ngx_http_modsecurity); - return NGX_DECLINED; + if (ngx_http_set_pool_ctx(r, ctx, ngx_http_modsecurity) != NGX_OK) { + return NGX_ERROR; } - ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "modSecurity: request pool ctx empty"); + } - ctx = ngx_http_modsecurity_create_ctx(r); if (ctx == NULL) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: ctx is null, nothing we can do, returning."); return NGX_HTTP_INTERNAL_SERVER_ERROR; } - ngx_http_set_ctx(r, ctx, ngx_http_modsecurity); - - if (ngx_http_set_pool_ctx(r, ctx, ngx_http_modsecurity) != NGX_OK) { - return NGX_ERROR; - } + ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: ctx is now: %p / count: %d", ctx, r->main->count); - /* load request to request rec */ - if (ngx_http_modsecurity_load_request(r) != NGX_OK - || ngx_http_modsecurity_load_headers_in(r) != NGX_OK) { + if (modsecContextState(ctx->req) == MODSEC_DISABLED) { + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: ModSecurity was disabled, returning....", ctx); - return NGX_HTTP_INTERNAL_SERVER_ERROR; + return NGX_DECLINED; } - /* processing request headers */ - rc = ngx_http_modsecurity_status(r, modsecProcessRequestHeaders(ctx->req)); + if (ctx->waiting_more_body == 1) { + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: waiting for more data before proceed. / count: %d", r->main->count); - if (rc != NGX_DECLINED) { - return rc; + return NGX_DONE; } - if (modsecContextState(ctx->req) == MODSEC_DISABLED) { - return NGX_DECLINED; - } + if (ctx->body_requested == 0) { + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: asking for the request body, if any."); - if (r->method == NGX_HTTP_POST - && modsecIsRequestBodyAccessEnabled(ctx->req) ) { + ctx->body_requested = 1; + rc = ngx_http_read_client_request_body(r, + ngx_http_modsecurity_request_read); - /* read POST request body, should we process PUT? */ - rc = ngx_http_read_client_request_body(r, ngx_http_modsecurity_body_handler); - if (rc >= NGX_HTTP_SPECIAL_RESPONSE) { - return rc; + if (rc == NGX_ERROR || rc >= NGX_HTTP_SPECIAL_RESPONSE) { + return rc; } - return NGX_DONE; + if (rc == NGX_AGAIN) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: nginx is asking us to wait for more data."); + ctx->waiting_more_body = 1; + return NGX_DONE; + } } - - /* other method */ - return ngx_http_modsecurity_status(r, modsecProcessRequestBody(ctx->req)); -} + if (ctx->waiting_more_body == 0 && ctx->request_processed == 0) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: request is ready to be processed."); + ngx_http_modsecurity_process_request(r); + ctx->request_processed = 1; + } -static void -ngx_http_modsecurity_body_handler(ngx_http_request_t *r) -{ - ngx_http_modsecurity_ctx_t *ctx; - ngx_int_t rc; + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: returning NGX_OK. Count++ :P" ); - ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "modSecurity: body handler"); + return NGX_DECLINED; +} - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity); +/* post read callback for rewrite and access phases */ +void +ngx_http_modsecurity_request_read(ngx_http_request_t *r) +{ - if (ngx_http_modsecurity_load_request_body(r) != NGX_OK) { - return ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); - } + ngx_http_modsecurity_ctx_t *ctx; - rc = ngx_http_modsecurity_status(r, modsecProcessRequestBody(ctx->req)); + r->read_event_handler = ngx_http_request_empty_handler; - if (rc != NGX_DECLINED) { - return ngx_http_finalize_request(r, rc); - } + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: reading request body..."); - if (ngx_http_modsecurity_save_request_body(r) != NGX_OK - || ngx_http_modsecurity_save_headers_in(r) != NGX_OK ) { + ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity); - return ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); + r->main->count--; + if (ctx->waiting_more_body) { + ctx->waiting_more_body = 0; + ngx_http_core_run_phases(r); } - r->phase_handler++; - ngx_http_core_run_phases(r); - ngx_http_finalize_request(r, NGX_DONE); } - +#ifndef NGX_HTTP_MODSECURITY_PREACCESS_HANDLER_ONLY static ngx_int_t ngx_http_modsecurity_header_filter(ngx_http_request_t *r) { ngx_http_modsecurity_loc_conf_t *cf; @@ -1102,8 +1164,8 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r) { r->filter_need_in_memory = 1; return NGX_OK; } - - +#endif +#ifndef NGX_HTTP_MODSECURITY_PREACCESS_HANDLER_ONLY static ngx_int_t ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) { @@ -1182,17 +1244,32 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) rc = move_brigade_to_chain(ctx->brigade, &out, r->pool); if (rc == NGX_ERROR) { + ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSec: Problems moving brigade to chain"); + return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity, NGX_HTTP_INTERNAL_SERVER_ERROR); } + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSecurity: save headers out done: before"); + + if (ngx_http_modsecurity_save_headers_in(r) != NGX_OK ||ngx_http_modsecurity_save_headers_out(r) != NGX_OK) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSecurity: save headers out done _inside_"); + + return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity, NGX_HTTP_INTERNAL_SERVER_ERROR); } + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSecurity: save headers out done: done"); + + if (r->headers_out.content_length_n != -1) { r->headers_out.content_length_n = content_length; @@ -1200,16 +1277,18 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) } r->header_sent = 0; - rc = ngx_http_next_header_filter(r); + rc = ngx_http_next_header_filter(r); if (rc == NGX_ERROR || rc > NGX_OK) { + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSecurity: ngx_http_next_header_filter not NGX_OK."); + return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity, rc); } return ngx_http_next_body_filter(r, out); } - -#define TXID_SIZE 25 +#endif static ngx_http_modsecurity_ctx_t * ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) @@ -1232,6 +1311,7 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, "modSecurity: ctx memory allocation error"); return NULL; } + cln = ngx_pool_cleanup_add(r->pool, sizeof(ngx_http_modsecurity_ctx_t)); if (cln == NULL) { ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, "modSecurity: ctx memory allocation error"); @@ -1242,6 +1322,15 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) ctx->r = r; + /* + + Using pcalloc already set to zero. + + ctx->request_processed = 0; + ctx->waiting_more_body = 0; + ctx->body_requested = 0; + */ + if (r->connection->requests == 0 || ctx->connection == NULL) { /* TODO: set server_rec, why igonre return value? */ @@ -1321,14 +1410,15 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) if (ctx->brigade == NULL) { return NULL; } - return ctx; } - static void +static void ngx_http_modsecurity_cleanup(void *data) { - ngx_http_modsecurity_ctx_t *ctx = data; + ngx_http_modsecurity_ctx_t *ctx = data; + + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, ctx->r->connection->log, 0, "ModSec: Cleaning up: %p ", data); if (ctx->req != NULL) { (void) modsecFinishRequest(ctx->req); @@ -1336,7 +1426,7 @@ ngx_http_modsecurity_cleanup(void *data) if (ctx->connection != NULL) { (void) modsecFinishConnection(ctx->connection); } - + } static char * @@ -1372,7 +1462,6 @@ ngx_http_modsecurity_config(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) return NGX_CONF_OK; } - static char * ngx_http_modsecurity_enable(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) { @@ -1390,8 +1479,7 @@ ngx_http_modsecurity_enable(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) return NGX_CONF_OK; } - - static int +static int ngx_http_modsecurity_drop_action(request_rec *r) { ngx_http_modsecurity_ctx_t *ctx; diff --git a/tests/regression/misc/00-phases.t b/tests/regression/misc/00-phases.t index 2f1c4e5935..b4c7e037d1 100644 --- a/tests/regression/misc/00-phases.t +++ b/tests/regression/misc/00-phases.t @@ -11,7 +11,7 @@ SecResponseBodyMimeType text/plain null SecRule REQUEST_LINE "^POST" "phase:1,pass,log,auditlog,id:500169" SecRule ARGS "val1" "phase:1,pass,log,auditlog,id:500170" - SecRule RESPONSE_HEADERS:Last-Modified "." "phase:1,pass,log,auditlog,id:500171" + SecRule RESPONSE_HEADERS:Last-Modified|RESPONSE_HEADERS:Content-type "." "phase:1,pass,log,auditlog,id:500171" SecRule RESPONSE_BODY "TEST" "phase:1,pass,log,auditlog,id:500172" ), match_log => { @@ -41,7 +41,7 @@ SecResponseBodyMimeType text/plain null SecRule REQUEST_LINE "^POST" "phase:2,pass,log,auditlog,id:500173" SecRule ARGS "val1" "phase:2,pass,log,auditlog,id:500174" - SecRule RESPONSE_HEADERS:Last-Modified "." "phase:2,pass,log,auditlog,id:500175" + SecRule RESPONSE_HEADERS:Last-Modified|RESPONSE_HEADERS:Content-type "." "phase:2,pass,log,auditlog,id:500175" SecRule RESPONSE_BODY "TEST" "phase:2,pass,log,auditlog,id:500176" ), match_log => { @@ -71,7 +71,7 @@ SecResponseBodyMimeType text/plain null SecRule REQUEST_LINE "^POST" "phase:3,pass,log,auditlog,id:500177" SecRule ARGS "val1" "phase:3,pass,log,auditlog,id:500178" - SecRule RESPONSE_HEADERS:Last-Modified "." "phase:3,pass,log,auditlog,id:500179" + SecRule RESPONSE_HEADERS:Last-Modified|RESPONSE_HEADERS:Content-type "." "phase:3,pass,log,auditlog,id:500179" SecRule RESPONSE_BODY "TEST" "phase:3,pass,log,auditlog,id:500180" ), match_log => { @@ -103,7 +103,7 @@ SecDebugLogLevel 9 SecRule REQUEST_LINE "^POST" "phase:4,pass,log,auditlog,id:500181" SecRule ARGS "val1" "phase:4,pass,log,auditlog,id:500182" - SecRule RESPONSE_HEADERS:Last-Modified "." "phase:4,pass,log,auditlog,id:500183" + SecRule RESPONSE_HEADERS:Last-Modified|RESPONSE_HEADERS:Content-Type "." "phase:4,pass,log,auditlog,id:500183" SecRule RESPONSE_BODY "TEST" "phase:4,pass,log,auditlog,id:500184" ), match_log => { @@ -132,7 +132,7 @@ SecResponseBodyMimeType text/plain null SecRule REQUEST_LINE "^POST" "phase:5,pass,log,auditlog,id:500185" SecRule ARGS "val1" "phase:5,pass,log,auditlog,id:500186" - SecRule RESPONSE_HEADERS:Last-Modified "." "phase:5,pass,log,auditlog,id:500187" + SecRule RESPONSE_HEADERS:Last-Modified|RESPONSE_HEADERS:Content-type "." "phase:5,pass,log,auditlog,id:500187" SecRule RESPONSE_BODY "TEST" "phase:5,pass,log,auditlog,id:500188" ), match_log => { diff --git a/tests/regression/misc/25-libinjection.t b/tests/regression/misc/25-libinjection.t index 28b133fdf9..b527beb8dc 100644 --- a/tests/regression/misc/25-libinjection.t +++ b/tests/regression/misc/25-libinjection.t @@ -19,7 +19,7 @@ status => qr/^403$/, }, request => new HTTP::Request( - POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/index.html", + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", [ "Content-Type" => "application/x-www-form-urlencoded", ], @@ -46,7 +46,7 @@ status => qr/^200$/, }, request => new HTTP::Request( - POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/index.html", + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", [ "Content-Type" => "application/x-www-form-urlencoded", ], @@ -73,7 +73,7 @@ status => qr/^403$/, }, request => new HTTP::Request( - POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/index.html", + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", [ "Content-Type" => "application/x-www-form-urlencoded", ], @@ -100,7 +100,7 @@ status => qr/^200$/, }, request => new HTTP::Request( - POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/index.html", + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", [ "Content-Type" => "application/x-www-form-urlencoded", ], diff --git a/tests/regression/nginx/conf/nginx.conf.template b/tests/regression/nginx/conf/nginx.conf.template index f5cb5e9783..b6bf005438 100644 --- a/tests/regression/nginx/conf/nginx.conf.template +++ b/tests/regression/nginx/conf/nginx.conf.template @@ -1,8 +1,10 @@ -user root; -worker_processes 1; +worker_processes 1; daemon on; -error_log logs/error.log debug; +worker_rlimit_core 500M; +working_directory /tmp/; +error_log logs/error.log debug; + events { worker_connections 1024; } @@ -10,13 +12,42 @@ events { http { ModSecurityEnabled [% enable %]; ModSecurityConfig [% config %]; - server { + client_body_buffer_size 1024M; + server { + client_max_body_size 30M; listen [% listen %]; server_name localhost; + + + + location /no-proxy/test.txt { + echo "TEST"; + } + + location /no-proxy/test2.txt { + echo "TEST 2"; + } + + location /proxy/test.txt { + proxy_pass http://localhost:[% listen %]/more/test.txt; + } + + location /proxy/test2.txt { + proxy_pass http://localhost:[% listen %]/more/test2.txt; + } + + location /test.txt { + echo "TEST"; + } + + location /test2.txt { + echo "TEST 2"; + } + location / { - error_page 405 = $uri; } + } } From 03952c70c1aae64cadc81f0123bdada93fd3e5cb Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Mon, 28 Apr 2014 11:54:23 -0700 Subject: [PATCH 02/16] nginx: looking for segfaults on the regression test. If nginx segfaults it will return, warning that the test failed. --- tests/run-regression-tests-nginx.pl | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/run-regression-tests-nginx.pl b/tests/run-regression-tests-nginx.pl index 6e4af85847..6d1ee4369f 100755 --- a/tests/run-regression-tests-nginx.pl +++ b/tests/run-regression-tests-nginx.pl @@ -204,6 +204,16 @@ sub runfile { my $rc = 0; my $conf_fn; + # watch for segfaults + if ($t and !$t->{match_log}) { + $t->{match_log} = {}; + } + if ($t and $t->{match_log} and !$t->{match_log}{-error}) { + $t->{match_log}{-error} = []; + } + push $t->{match_log}{-error}, qr/(core dump)/; + push $t->{match_log}{-error}, 1; + # Startup nginx with optionally included conf. if (exists $t{conf} and defined $t{conf}) { $conf_fn = sprintf "%s/%s_%s_%06d.conf", @@ -498,7 +508,6 @@ sub match_log { #dbg("Match \"$re\" in $name \"$$rbuf\" ($n)"); if ($$rbuf =~ m/$re/m) { $rc = $&; -# print "bonga\n"; last; } # TODO: Use select()/poll() @@ -695,6 +704,7 @@ sub nginx_reset_fd { return undef; } + # Any extras listed in "match_log" if ($t and exists $t->{match_log}) { for my $k (keys %{ $t->{match_log} || {} }) { From 5d8fa16ff40d21dbb1c7eb39d80b252e94aa70b3 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Mon, 28 Apr 2014 19:11:59 -0700 Subject: [PATCH 03/16] nginx: better dealing with chunked request body --- nginx/modsecurity/ngx_http_modsecurity.c | 28 ++++++++++++++++--- .../regression/nginx/conf/nginx.conf.template | 4 ++- 2 files changed, 27 insertions(+), 5 deletions(-) diff --git a/nginx/modsecurity/ngx_http_modsecurity.c b/nginx/modsecurity/ngx_http_modsecurity.c index fc8555f52b..8f17ea30d9 100644 --- a/nginx/modsecurity/ngx_http_modsecurity.c +++ b/nginx/modsecurity/ngx_http_modsecurity.c @@ -537,9 +537,23 @@ ngx_http_modsecurity_save_request_body(ngx_http_request_t *r) buf->last += content_length; r->header_in = buf; + if (r->headers_in.content_length) { + ngx_str_t *str = NULL; + + str = &r->headers_in.content_length->value; + str->data = ngx_palloc(r->pool, NGX_OFF_T_LEN); + if (str->data == NULL) { + ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); + return NGX_OK; + } + str->len = ngx_snprintf(str->data, NGX_OFF_T_LEN, "%O", content_length) - str->data; + + } + + 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); return NGX_OK; } @@ -635,7 +649,7 @@ ngx_http_modsecurity_load_headers_out(ngx_http_request_t *r) *(const char **)apr_array_push(ctx->req->content_languages) = data; } - /* req->chunked = r->chunked; may be useless */ + req->chunked = r->chunked; req->clength = r->headers_out.content_length_n; req->mtime = apr_time_make(r->headers_out.last_modified_time, 0); @@ -654,7 +668,7 @@ ngx_http_modsecurity_save_headers_out(ngx_http_request_t *r) ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity); - /* r->chunked = ctx->req->chunked; */ + r->chunked = ctx->req->chunked; ngx_http_clean_header(r); @@ -1055,7 +1069,6 @@ ngx_http_modsecurity_handler(ngx_http_request_t *r) { return NGX_DECLINED; } - if (ctx->waiting_more_body == 1) { ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSec: waiting for more data before proceed. / count: %d", r->main->count); @@ -1068,6 +1081,8 @@ ngx_http_modsecurity_handler(ngx_http_request_t *r) { "ModSec: asking for the request body, if any."); ctx->body_requested = 1; + r->request_body_in_single_buf = 1; + rc = ngx_http_read_client_request_body(r, ngx_http_modsecurity_request_read); @@ -1084,8 +1099,11 @@ ngx_http_modsecurity_handler(ngx_http_request_t *r) { } if (ctx->waiting_more_body == 0 && ctx->request_processed == 0) { + 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); ctx->request_processed = 1; } @@ -1287,6 +1305,8 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) } return ngx_http_next_body_filter(r, out); + + return NGX_OK; } #endif diff --git a/tests/regression/nginx/conf/nginx.conf.template b/tests/regression/nginx/conf/nginx.conf.template index b6bf005438..a543a0b531 100644 --- a/tests/regression/nginx/conf/nginx.conf.template +++ b/tests/regression/nginx/conf/nginx.conf.template @@ -18,7 +18,9 @@ http { client_max_body_size 30M; listen [% listen %]; server_name localhost; - + client_body_in_single_buffer on; + client_body_in_file_only on; + proxy_buffering off; location /no-proxy/test.txt { From b4058507a56f55596e85ceb9c6ce5afc201cd8c2 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Tue, 29 Apr 2014 07:23:37 -0700 Subject: [PATCH 04/16] 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 8dc98d5053..e113774b90 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 e37f9f78c8..a3dd559816 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 8f17ea30d9..eff0fa512b 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; } From 8a722e8554f90aa9c3c59a8b86d1d7d694a7a91a Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Wed, 30 Apr 2014 11:06:21 -0700 Subject: [PATCH 05/16] nginx: cosmetics: Splits lines longer than 80 characters --- nginx/modsecurity/apr_bucket_nginx.c | 19 ++-- nginx/modsecurity/apr_bucket_nginx.h | 7 +- nginx/modsecurity/ngx_http_modsecurity.c | 135 +++++++++++++++-------- nginx/modsecurity/ngx_pool_context.c | 9 +- nginx/modsecurity/ngx_pool_context.h | 6 +- 5 files changed, 117 insertions(+), 59 deletions(-) diff --git a/nginx/modsecurity/apr_bucket_nginx.c b/nginx/modsecurity/apr_bucket_nginx.c index e113774b90..cbdc7ef516 100644 --- a/nginx/modsecurity/apr_bucket_nginx.c +++ b/nginx/modsecurity/apr_bucket_nginx.c @@ -62,7 +62,9 @@ static apr_status_t nginx_bucket_read(apr_bucket *b, const char **str, return APR_EGENERAL; } - size = ngx_read_file(buf->file, data, ngx_buf_size(buf), buf->file_pos); + size = ngx_read_file(buf->file, data, ngx_buf_size(buf), + buf->file_pos); + if (size != ngx_buf_size(buf)) { apr_bucket_free(data); return APR_EGENERAL; @@ -153,8 +155,9 @@ ngx_buf_t * apr_bucket_to_ngx_buf(apr_bucket *e, ngx_pool_t *pool) { return buf; } -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 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; @@ -183,8 +186,9 @@ copy_chain_to_brigade(ngx_chain_t *chain_orig, apr_bucket_brigade *bb, ngx_pool_ } -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) { +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; @@ -215,8 +219,9 @@ move_chain_to_brigade(ngx_chain_t *chain_orig, apr_bucket_brigade *bb, ngx_pool_ return NGX_AGAIN; } -ngx_int_t -move_brigade_to_chain(apr_bucket_brigade *bb, ngx_chain_t **ll, ngx_pool_t *pool) { +ngx_int_t move_brigade_to_chain(apr_bucket_brigade *bb, + ngx_chain_t **ll, ngx_pool_t *pool) +{ apr_bucket *e; ngx_buf_t *buf; ngx_chain_t *cl; diff --git a/nginx/modsecurity/apr_bucket_nginx.h b/nginx/modsecurity/apr_bucket_nginx.h index a3dd559816..3f7004feb8 100644 --- a/nginx/modsecurity/apr_bucket_nginx.h +++ b/nginx/modsecurity/apr_bucket_nginx.h @@ -16,6 +16,9 @@ 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); +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 eff0fa512b..5abb769b2f 100644 --- a/nginx/modsecurity/ngx_http_modsecurity.c +++ b/nginx/modsecurity/ngx_http_modsecurity.c @@ -98,25 +98,33 @@ static void ngx_http_modsecurity_request_read(ngx_http_request_t *r); #ifndef NGX_HTTP_MODSECURITY_PREACCESS_HANDLER_ONLY static ngx_int_t ngx_http_modsecurity_header_filter(ngx_http_request_t *r); -static ngx_int_t ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in); +static ngx_int_t ngx_http_modsecurity_body_filter(ngx_http_request_t *r, + ngx_chain_t *in); #endif static ngx_int_t ngx_http_modsecurity_preconfiguration(ngx_conf_t *cf); static ngx_int_t ngx_http_modsecurity_init(ngx_conf_t *cf); static ngx_int_t ngx_http_modsecurity_init_process(ngx_cycle_t *cycle); static void *ngx_http_modsecurity_create_loc_conf(ngx_conf_t *cf); -static char *ngx_http_modsecurity_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child); +static char *ngx_http_modsecurity_merge_loc_conf(ngx_conf_t *cf, + void *parent, void *child); -static char *ngx_http_modsecurity_config(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); -static char *ngx_http_modsecurity_enable(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); +static char *ngx_http_modsecurity_config(ngx_conf_t *cf, + ngx_command_t *cmd, void *conf); +static char *ngx_http_modsecurity_enable(ngx_conf_t *cf, + ngx_command_t *cmd, void *conf); + +static ngx_http_modsecurity_ctx_t + *ngx_http_modsecurity_create_ctx(ngx_http_request_t *r); -static ngx_http_modsecurity_ctx_t * ngx_http_modsecurity_create_ctx(ngx_http_request_t *r); static int ngx_http_modsecurity_drop_action(request_rec *r); static void ngx_http_modsecurity_terminate(ngx_cycle_t *cycle); static void ngx_http_modsecurity_cleanup(void *data); -static int ngx_http_modsecurity_save_headers_in_visitor(void *data, const char *key, const char *value); -static int ngx_http_modsecurity_save_headers_out_visitor(void *data, const char *key, const char *value); +static int ngx_http_modsecurity_save_headers_in_visitor(void *data, + const char *key, const char *value); +static int ngx_http_modsecurity_save_headers_out_visitor(void *data, + const char *key, const char *value); /* ** This is a temporary hack to make PCRE work with ModSecurity @@ -211,7 +219,7 @@ ngx_pstrdup0(ngx_pool_t *pool, ngx_str_t *src) dst = ngx_pnalloc(pool, src->len + 1); if (dst == NULL) { - NGX_HTTP_MODSECURITY_ return NULL; + return NULL; } ngx_memcpy(dst, src->data, src->len); @@ -220,13 +228,15 @@ ngx_pstrdup0(ngx_pool_t *pool, ngx_str_t *src) return dst; } + +/* + * MultiplyDeBruijnBitPosition + * http://graphics.stanford.edu/~seander/bithacks.html#ZerosOnRightMultLookup + */ static inline int ngx_http_modsecurity_method_number(unsigned int nginx) { - /* - * http://graphics.stanford.edu/~seander/bithacks.html#ZerosOnRightMultLookup - */ - static const int MultiplyDeBruijnBitPosition[32] = { + static const int MultiplyDeBruijn[32] = { M_INVALID, /* 1 >> 0 */ M_GET, M_INVALID, /* 1 >> 28 */ @@ -261,7 +271,7 @@ ngx_http_modsecurity_method_number(unsigned int nginx) M_OPTIONS }; - return MultiplyDeBruijnBitPosition[((uint32_t)((nginx & -nginx) * 0x077CB531U)) >> 27]; + return MultiplyDeBruijn[((uint32_t)((nginx & -nginx) * 0x077CB531U)) >> 27]; } static ngx_inline ngx_int_t @@ -323,13 +333,23 @@ ngx_http_modsecurity_load_request(ngx_http_request_t *r) req->parsed_uri.dns_looked_up = 0; req->parsed_uri.dns_resolved = 0; - // req->parsed_uri.password = (char *)ngx_pstrdup0(r->pool, &r->headers_in.passwd); - // req->parsed_uri.user = (char *)ngx_pstrdup0(r->pool, &r->headers_in.user); + /* req->parsed_uri.password = (char *)ngx_pstrdup0(r->pool, */ + /* &r->headers_in.passwd); */ + + /* req->parsed_uri.user = (char *)ngx_pstrdup0(r->pool, */ + /* &r->headers_in.user); */ req->parsed_uri.fragment = (char *)ngx_pstrdup0(r->pool, &r->exten); - req->hostname = (char *)ngx_pstrdup0(r->pool, (ngx_str_t *)&ngx_cycle->hostname); + req->hostname = (char *)ngx_pstrdup0(r->pool, + (ngx_str_t *)&ngx_cycle->hostname); - req->header_only = r->header_only ? r->header_only : (r->method == NGX_HTTP_HEAD); + + if (r->header_only) { + req->header_only = r->header_only; + } + else { + req->header_only = (r->method == NGX_HTTP_HEAD); + } return NGX_OK; } @@ -366,7 +386,8 @@ ngx_http_modsecurity_load_headers_in(ngx_http_request_t *r) i = 0; } - apr_table_setn(req->headers_in, (char *)h[i].key.data, (char *)h[i].value.data); + apr_table_setn(req->headers_in, (char *)h[i].key.data, + (char *)h[i].value.data); ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSecurity: load headers in: \"%V: %V\"", &h[i].key, &h[i].value); @@ -382,7 +403,8 @@ ngx_http_modsecurity_load_headers_in(ngx_http_request_t *r) lang = apr_table_get(ctx->req->headers_in, "Content-Languages"); if(lang != NULL) { - ctx->req->content_languages = apr_array_make(ctx->req->pool, 1, sizeof(const char *)); + ctx->req->content_languages = apr_array_make(ctx->req->pool, 1, + sizeof(const char *)); *(const char **)apr_array_push(ctx->req->content_languages) = lang; } @@ -465,8 +487,8 @@ ngx_http_modsecurity_save_headers_in(ngx_http_request_t *r) } -static int -ngx_http_modsecurity_save_headers_in_visitor(void *data, const char *key, const char *value) +static int ngx_http_modsecurity_save_headers_in_visitor(void *data, + const char *key, const char *value) { ngx_http_request_t *r = data; ngx_table_elt_t *h; @@ -568,7 +590,8 @@ ngx_http_modsecurity_save_request_body(ngx_http_request_t *r) apr_brigade_length(ctx->brigade, 0, &content_length); buf = ngx_create_temp_buf(ctx->r->pool, (size_t) content_length); - apr_brigade_flatten(ctx->brigade, (char *)buf->pos, (apr_size_t *)&content_length); + apr_brigade_flatten(ctx->brigade, (char *)buf->pos, + (apr_size_t *)&content_length); apr_brigade_cleanup(ctx->brigade); buf->last += content_length; r->header_in = buf; @@ -582,7 +605,8 @@ ngx_http_modsecurity_save_request_body(ngx_http_request_t *r) ngx_http_finalize_request(r, NGX_HTTP_INTERNAL_SERVER_ERROR); return NGX_OK; } - str->len = ngx_snprintf(str->data, NGX_OFF_T_LEN, "%O", content_length) - str->data; + str->len = ngx_snprintf(str->data, NGX_OFF_T_LEN, "%O", + content_length) - str->data; } @@ -618,7 +642,8 @@ ngx_http_modsecurity_load_headers_out(ngx_http_request_t *r) req = ctx->req; req->status = r->headers_out.status; - req->status_line = (char *)ngx_pstrdup0(r->pool, &r->headers_out.status_line); + req->status_line = (char *)ngx_pstrdup0(r->pool, + &r->headers_out.status_line); /* deep copy */ part = &r->headers_out.headers.part; @@ -659,8 +684,8 @@ ngx_http_modsecurity_load_headers_out(ngx_http_request_t *r) for (i = 0; special_headers_out[i].name; i++) { vv = ngx_http_get_variable(r, &special_headers_out[i].variable_name, - ngx_hash_key(special_headers_out[i].variable_name.data, - special_headers_out[i].variable_name.len)); + ngx_hash_key(special_headers_out[i].variable_name.data, + special_headers_out[i].variable_name.len)); if (vv && !vv->not_found) { @@ -680,13 +705,15 @@ ngx_http_modsecurity_load_headers_out(ngx_http_request_t *r) } req->content_type = apr_table_get(ctx->req->headers_out, "Content-Type"); - req->content_encoding = apr_table_get(ctx->req->headers_out, "Content-Encoding"); + req->content_encoding = apr_table_get(ctx->req->headers_out, + "Content-Encoding"); data = (char *)apr_table_get(ctx->req->headers_out, "Content-Languages"); if(data != NULL) { - ctx->req->content_languages = apr_array_make(ctx->req->pool, 1, sizeof(const char *)); + ctx->req->content_languages = apr_array_make(ctx->req->pool, 1, + sizeof(const char *)); *(const char **)apr_array_push(ctx->req->content_languages) = data; } @@ -751,7 +778,8 @@ ngx_http_modsecurity_save_headers_out(ngx_http_request_t *r) static int -ngx_http_modsecurity_save_headers_out_visitor(void *data, const char *key, const char *value) +ngx_http_modsecurity_save_headers_out_visitor(void *data, + const char *key, const char *value) { ngx_http_request_t *r = data; ngx_table_elt_t *h, he; @@ -798,7 +826,8 @@ ngx_http_modsecurity_save_headers_out_visitor(void *data, const char *key, const static ngx_inline ngx_int_t ngx_http_modsecurity_status(ngx_http_request_t *r, int status) { - ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSecurity: status %d", status); + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "ModSecurity: status %d", status); if (status == DECLINED || status == APR_SUCCESS) { return NGX_DECLINED; @@ -995,11 +1024,13 @@ ngx_http_modsecurity_preconfiguration(ngx_conf_t *cf) } /* set host name */ - modsec_server->server_hostname = ngx_palloc(cf->pool, ngx_cycle->hostname.len + 1); + modsec_server->server_hostname = ngx_palloc(cf->pool, + ngx_cycle->hostname.len + 1); if (modsec_server->server_hostname == NULL) { return NGX_ERROR; } - ngx_memcpy(modsec_server->server_hostname, ngx_cycle->hostname.data, ngx_cycle->hostname.len); + ngx_memcpy(modsec_server->server_hostname, ngx_cycle->hostname.data, + ngx_cycle->hostname.len); modsec_server->server_hostname[ ngx_cycle->hostname.len] = '\0'; modsecStartConfig(); @@ -1065,7 +1096,8 @@ ngx_http_modsecurity_handler(ngx_http_request_t *r) { ngx_http_modsecurity_loc_conf_t *cf = NULL; ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "ModSec: Catching a new access phase handler. Count: %d", r->main->count); + "ModSec: Catching a new access phase handler. Count: %d", + r->main->count); cf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity); @@ -1112,7 +1144,8 @@ ngx_http_modsecurity_handler(ngx_http_request_t *r) { } if (ctx->waiting_more_body == 1) { ngx_log_debug1(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, - "ModSec: waiting for more data before proceed. / count: %d", r->main->count); + "ModSec: waiting for more data before proceed. / count: %d", + r->main->count); return NGX_DONE; } @@ -1201,7 +1234,8 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r) { && r->err_status >= NGX_HTTP_MOVED_PERMANENTLY && r->err_status < 308) { - /* 3XX load redirect location header so that we can do redirect in phase 3,4 */ + /* 3XX load redirect location header so that we can do redirect */ + /* in phase 3,4 */ location = apr_table_get(ctx->req->headers_out, "Location"); if (location == NULL) { @@ -1226,7 +1260,8 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r) { return ngx_http_next_header_filter(r); } - ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "modSecurity: header filter"); + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "modSecurity: header filter"); r->filter_need_in_memory = 1; return NGX_OK; @@ -1250,7 +1285,8 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) return ngx_http_next_body_filter(r, in); } - ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "modSecurity: body filter"); + ngx_log_debug0(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, + "modSecurity: body filter"); for (cl = in; cl; cl = cl->next) { apr_bucket *e; @@ -1377,13 +1413,15 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_modsecurity_ctx_t)); if (ctx == NULL) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, "modSecurity: ctx memory allocation error"); + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "modSecurity: ctx memory allocation error"); return NULL; } cln = ngx_pool_cleanup_add(r->pool, sizeof(ngx_http_modsecurity_ctx_t)); if (cln == NULL) { - ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, "modSecurity: ctx memory allocation error"); + ngx_log_error(NGX_LOG_INFO, r->connection->log, 0, + "modSecurity: ctx memory allocation error"); return NULL; } cln->handler = ngx_http_modsecurity_cleanup; @@ -1408,7 +1446,8 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) /* fill apr_sockaddr_t */ asa = ngx_palloc(r->pool, sizeof(apr_sockaddr_t)); asa->pool = ctx->connection->pool; - asa->hostname = (char *)ngx_pstrdup0(r->pool, &r->connection->addr_text); + asa->hostname = (char *)ngx_pstrdup0(r->pool, + &r->connection->addr_text); asa->servname = asa->hostname; asa->next = NULL; asa->salen = r->connection->socklen; @@ -1472,9 +1511,11 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) salt[TXID_SIZE-1] = '\0'; - apr_table_setn(ctx->req->subprocess_env, "UNIQUE_ID", apr_psprintf(ctx->req->pool, "%s", salt)); + apr_table_setn(ctx->req->subprocess_env, "UNIQUE_ID", + apr_psprintf(ctx->req->pool, "%s", salt)); - ctx->brigade = apr_brigade_create(ctx->req->pool, ctx->req->connection->bucket_alloc); + ctx->brigade = apr_brigade_create(ctx->req->pool, + ctx->req->connection->bucket_alloc); if (ctx->brigade == NULL) { return NULL; @@ -1487,7 +1528,8 @@ ngx_http_modsecurity_cleanup(void *data) { ngx_http_modsecurity_ctx_t *ctx = data; - ngx_log_debug1(NGX_LOG_DEBUG_HTTP, ctx->r->connection->log, 0, "ModSec: Cleaning up: %p ", data); + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, ctx->r->connection->log, 0, + "ModSec: Cleaning up: %p ", data); if (ctx->req != NULL) { (void) modsecFinishRequest(ctx->req); @@ -1523,7 +1565,8 @@ ngx_http_modsecurity_config(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) msg = modsecProcessConfig(mscf->config, (const char *)value[1].data, NULL); if (msg != NULL) { - ngx_log_error(NGX_LOG_EMERG, cf->log, 0, "ModSecurityConfig in %s:%ui: %s", + ngx_log_error(NGX_LOG_EMERG, cf->log, 0, + "ModSecurityConfig in %s:%ui: %s", cf->conf_file->file.name.data, cf->conf_file->line, msg); return NGX_CONF_ERROR; } @@ -1552,11 +1595,13 @@ static int ngx_http_modsecurity_drop_action(request_rec *r) { ngx_http_modsecurity_ctx_t *ctx; - ctx = (ngx_http_modsecurity_ctx_t *) apr_table_get(r->notes, NOTE_NGINX_REQUEST_CTX); + ctx = (ngx_http_modsecurity_ctx_t *) apr_table_get(r->notes, + NOTE_NGINX_REQUEST_CTX); if (ctx == NULL) { return -1; } + ctx->r->connection->error = 1; return 0; } diff --git a/nginx/modsecurity/ngx_pool_context.c b/nginx/modsecurity/ngx_pool_context.c index b1a0607cd0..abd00dde52 100644 --- a/nginx/modsecurity/ngx_pool_context.c +++ b/nginx/modsecurity/ngx_pool_context.c @@ -99,7 +99,8 @@ ngx_pool_get_ctx(ngx_pool_t *pool, ngx_uint_t index) ngx_pool_context_node_t *node; hash = (ngx_uint_t) pool + index; - key = ngx_murmur_hash2((u_char *)&hash, sizeof(hash)) % ngx_pool_context_hash_size; + key = ngx_murmur_hash2((u_char *)&hash, + sizeof(hash)) % ngx_pool_context_hash_size; node = ngx_pool_context_hash[key]; @@ -126,7 +127,8 @@ ngx_pool_set_ctx(ngx_pool_t *pool, ngx_uint_t index, void *data) ngx_pool_cleanup_t *cln; hash = (ngx_uint_t) pool + index; - key = ngx_murmur_hash2((u_char *)&hash, sizeof(hash)) % ngx_pool_context_hash_size; + key = ngx_murmur_hash2((u_char *)&hash, + sizeof(hash)) % ngx_pool_context_hash_size; node = ngx_pool_context_hash[key]; @@ -200,7 +202,8 @@ ngx_pool_context_init_conf(ngx_cycle_t *cycle, void *conf) ngx_pool_context_hash_size = pcf->size; - ngx_pool_context_hash = ngx_pcalloc(cycle->pool, sizeof(ngx_pool_context_node_t *) * ngx_pool_context_hash_size); + ngx_pool_context_hash = ngx_pcalloc(cycle->pool, + sizeof(ngx_pool_context_node_t *) * ngx_pool_context_hash_size); if (ngx_pool_context_hash == NULL) { return NGX_CONF_ERROR; diff --git a/nginx/modsecurity/ngx_pool_context.h b/nginx/modsecurity/ngx_pool_context.h index de8b2f3a2e..f33b27d8cc 100644 --- a/nginx/modsecurity/ngx_pool_context.h +++ b/nginx/modsecurity/ngx_pool_context.h @@ -6,7 +6,9 @@ void* ngx_pool_get_ctx(ngx_pool_t * pool, ngx_uint_t index); ngx_int_t ngx_pool_set_ctx(ngx_pool_t * pool, ngx_uint_t index,void * data); -#define ngx_http_get_module_pool_ctx(r, module) ngx_pool_get_ctx(r->pool, module.index) -#define ngx_http_set_pool_ctx(r, c, module) ngx_pool_set_ctx(r->pool, module.index, c) +#define ngx_http_get_module_pool_ctx(r, module) \ + ngx_pool_get_ctx(r->pool, module.index) +#define ngx_http_set_pool_ctx(r, c, module) \ + ngx_pool_set_ctx(r->pool, module.index, c) #endif /* _NGX_POOL_CONTEXT_H_INCLUDE_ */ From 97a82390635e7d39c2565b9104c9ac0848827343 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Wed, 30 Apr 2014 11:09:03 -0700 Subject: [PATCH 06/16] nginx: cosmetics: Removes trailing whitespace --- nginx/modsecurity/ngx_http_modsecurity.c | 16 ++++++++-------- nginx/modsecurity/ngx_pool_context.c | 2 +- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/nginx/modsecurity/ngx_http_modsecurity.c b/nginx/modsecurity/ngx_http_modsecurity.c index 5abb769b2f..fd8a69fa6b 100644 --- a/nginx/modsecurity/ngx_http_modsecurity.c +++ b/nginx/modsecurity/ngx_http_modsecurity.c @@ -55,7 +55,7 @@ #define tuxb -/* +/* * If NGX_HTTP_MODSECURITY_PREACCESS_HANDLER_ONLY is defined, this module will * not process the body and header filter, instead it will only process the * preaccess request handler. This is useful to debug the module and check if @@ -1230,7 +1230,7 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r) { ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity); /* already processed, checking redirect action. */ - if (ctx && ctx->complete + if (ctx && ctx->complete && r->err_status >= NGX_HTTP_MOVED_PERMANENTLY && r->err_status < 308) { @@ -1296,12 +1296,12 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) if (size) { char *data = apr_pmemdup(bb->p, buf->pos, size); if (data == NULL) { - return ngx_http_filter_finalize_request(r, + return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity, NGX_HTTP_INTERNAL_SERVER_ERROR); } e = apr_bucket_pool_create(data , size, bb->p, bb->bucket_alloc); if (e == NULL) { - return ngx_http_filter_finalize_request(r, + return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity, NGX_HTTP_INTERNAL_SERVER_ERROR); } APR_BRIGADE_INSERT_TAIL(bb, e); @@ -1312,7 +1312,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) buf->last_buf = 0; e = apr_bucket_eos_create(bb->bucket_alloc); if (e == NULL) { - return ngx_http_filter_finalize_request(r, + return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity, NGX_HTTP_INTERNAL_SERVER_ERROR); } APR_BRIGADE_INSERT_TAIL(bb, e); @@ -1333,7 +1333,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) if (ngx_http_modsecurity_load_headers_in(r) != NGX_OK || ngx_http_modsecurity_load_headers_out(r) != NGX_OK) { - return ngx_http_filter_finalize_request(r, + return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity, NGX_HTTP_INTERNAL_SERVER_ERROR); } @@ -1350,7 +1350,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) ngx_log_debug(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSec: Problems moving brigade to chain"); - return ngx_http_filter_finalize_request(r, + return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity, NGX_HTTP_INTERNAL_SERVER_ERROR); } @@ -1365,7 +1365,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) "ModSecurity: save headers out done _inside_"); - return ngx_http_filter_finalize_request(r, + return ngx_http_filter_finalize_request(r, &ngx_http_modsecurity, NGX_HTTP_INTERNAL_SERVER_ERROR); } diff --git a/nginx/modsecurity/ngx_pool_context.c b/nginx/modsecurity/ngx_pool_context.c index abd00dde52..ebf7ce4d15 100644 --- a/nginx/modsecurity/ngx_pool_context.c +++ b/nginx/modsecurity/ngx_pool_context.c @@ -70,7 +70,7 @@ ngx_module_t ngx_pool_context_module = { } \ \ node->prev = NULL; \ - + #define ngx_pool_context_link(queue, node) \ \ From d2a1b61dad9ade6196c1fa53ed877341b55f8d6d Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Wed, 30 Apr 2014 11:25:04 -0700 Subject: [PATCH 07/16] niginx: cosmetics: Changes CRLF to LF --- nginx/modsecurity/apr_bucket_nginx.c | 486 +++++++++++++-------------- nginx/modsecurity/apr_bucket_nginx.h | 32 +- 2 files changed, 259 insertions(+), 259 deletions(-) diff --git a/nginx/modsecurity/apr_bucket_nginx.c b/nginx/modsecurity/apr_bucket_nginx.c index cbdc7ef516..d47737f64b 100644 --- a/nginx/modsecurity/apr_bucket_nginx.c +++ b/nginx/modsecurity/apr_bucket_nginx.c @@ -1,160 +1,160 @@ - -#include - -static apr_status_t nginx_bucket_read(apr_bucket *b, const char **str, - apr_size_t *len, apr_read_type_e block); -static void nginx_bucket_destroy(void *data); - -static const apr_bucket_type_t apr_bucket_type_nginx = { - "NGINX", 5, APR_BUCKET_DATA, - nginx_bucket_destroy, - nginx_bucket_read, - apr_bucket_setaside_noop, - apr_bucket_shared_split, - apr_bucket_shared_copy -}; - - -typedef struct apr_bucket_nginx { - apr_bucket_refcount refcount; - ngx_buf_t *buf; -} apr_bucket_nginx; - -/* ngx_buf_t to apr_bucket */ -apr_bucket * apr_bucket_nginx_create(ngx_buf_t *buf, - apr_pool_t *p, - apr_bucket_alloc_t *list) -{ - - apr_bucket *b = apr_bucket_alloc(sizeof(*b), list); - - APR_BUCKET_INIT(b); /* link */ - b->free = apr_bucket_free; - b->list = list; - return apr_bucket_nginx_make(b, buf, p); -} - -apr_bucket * apr_bucket_nginx_make(apr_bucket *b, ngx_buf_t *buf, - apr_pool_t *pool) -{ - apr_bucket_nginx *n; - - n = apr_bucket_alloc(sizeof(*n), b->list); - - n->buf = buf; - - b = apr_bucket_shared_make(b, n, 0, ngx_buf_size(buf)); - b->type = &apr_bucket_type_nginx; - return b; -} - -static apr_status_t nginx_bucket_read(apr_bucket *b, const char **str, - apr_size_t *len, apr_read_type_e block) -{ - apr_bucket_nginx *n = b->data; - ngx_buf_t *buf = n->buf; - u_char *data; - ssize_t size; - - if (buf->pos == NULL && ngx_buf_size(buf) != 0) { - data = apr_bucket_alloc(ngx_buf_size(buf), b->list); - if (data == NULL) { - return APR_EGENERAL; - } - + +#include + +static apr_status_t nginx_bucket_read(apr_bucket *b, const char **str, + apr_size_t *len, apr_read_type_e block); +static void nginx_bucket_destroy(void *data); + +static const apr_bucket_type_t apr_bucket_type_nginx = { + "NGINX", 5, APR_BUCKET_DATA, + nginx_bucket_destroy, + nginx_bucket_read, + apr_bucket_setaside_noop, + apr_bucket_shared_split, + apr_bucket_shared_copy +}; + + +typedef struct apr_bucket_nginx { + apr_bucket_refcount refcount; + ngx_buf_t *buf; +} apr_bucket_nginx; + +/* ngx_buf_t to apr_bucket */ +apr_bucket * apr_bucket_nginx_create(ngx_buf_t *buf, + apr_pool_t *p, + apr_bucket_alloc_t *list) +{ + + apr_bucket *b = apr_bucket_alloc(sizeof(*b), list); + + APR_BUCKET_INIT(b); /* link */ + b->free = apr_bucket_free; + b->list = list; + return apr_bucket_nginx_make(b, buf, p); +} + +apr_bucket * apr_bucket_nginx_make(apr_bucket *b, ngx_buf_t *buf, + apr_pool_t *pool) +{ + apr_bucket_nginx *n; + + n = apr_bucket_alloc(sizeof(*n), b->list); + + n->buf = buf; + + b = apr_bucket_shared_make(b, n, 0, ngx_buf_size(buf)); + b->type = &apr_bucket_type_nginx; + return b; +} + +static apr_status_t nginx_bucket_read(apr_bucket *b, const char **str, + apr_size_t *len, apr_read_type_e block) +{ + apr_bucket_nginx *n = b->data; + ngx_buf_t *buf = n->buf; + u_char *data; + ssize_t size; + + if (buf->pos == NULL && ngx_buf_size(buf) != 0) { + data = apr_bucket_alloc(ngx_buf_size(buf), b->list); + if (data == NULL) { + return APR_EGENERAL; + } + size = ngx_read_file(buf->file, data, ngx_buf_size(buf), buf->file_pos); - if (size != ngx_buf_size(buf)) { - apr_bucket_free(data); - return APR_EGENERAL; - } - buf->pos = data; - } - - *str = (char *)buf->pos + b->start; - *len = b->length; - - return APR_SUCCESS; -} - - -static void nginx_bucket_destroy(void *data) -{ - apr_bucket_nginx *n = data; - ngx_buf_t *buf = n->buf; - - if (apr_bucket_shared_destroy(n)) { - if (!ngx_buf_in_memory(buf) && buf->pos != NULL) { - apr_bucket_free(buf->pos); - buf->pos = NULL; - } - apr_bucket_free(n); - } -} - -ngx_buf_t * apr_bucket_to_ngx_buf(apr_bucket *e, ngx_pool_t *pool) { - ngx_buf_t *buf, *b; - apr_bucket_nginx *n; - ngx_uint_t len; - u_char *data; - - if (e->type->is_metadata) { - return NULL; - } - - if (e->type == &apr_bucket_type_nginx) { - n = e->data; - b = n->buf; - - /* whole buf */ - if (e->length == (apr_size_t)ngx_buf_size(b)) { - b->last_buf = 0; - return b; - } - - buf = ngx_palloc(pool, sizeof(ngx_buf_t)); - if (buf == NULL) { - return NULL; - } - ngx_memcpy(buf, b, sizeof(ngx_buf_t)); - - if (ngx_buf_in_memory(buf)) { - buf->start = buf->pos = buf->pos + e->start; - buf->end = buf->last = buf->pos + e->length; - } else { - buf->pos = NULL; - buf->file_pos += e->start; - buf->file_last = buf->file_pos + e->length; - } - - buf->last_buf = 0; - return buf; - } - - if (apr_bucket_read(e, (const char **)&data, - &len, APR_BLOCK_READ) != APR_SUCCESS) { - return NULL; - } - - buf = ngx_calloc_buf(pool); - if (buf == NULL) { - return NULL; - } - - if (e->type == &apr_bucket_type_pool) { - buf->start = data; - } else if (len != 0) { - buf->start = ngx_palloc(pool, len); - ngx_memcpy(buf->start, data, len); - } - - buf->pos = buf->start; - buf->end = buf->last = buf->start + len; - buf->temporary = 1; - return buf; -} - + if (size != ngx_buf_size(buf)) { + apr_bucket_free(data); + return APR_EGENERAL; + } + buf->pos = data; + } + + *str = (char *)buf->pos + b->start; + *len = b->length; + + return APR_SUCCESS; +} + + +static void nginx_bucket_destroy(void *data) +{ + apr_bucket_nginx *n = data; + ngx_buf_t *buf = n->buf; + + if (apr_bucket_shared_destroy(n)) { + if (!ngx_buf_in_memory(buf) && buf->pos != NULL) { + apr_bucket_free(buf->pos); + buf->pos = NULL; + } + apr_bucket_free(n); + } +} + +ngx_buf_t * apr_bucket_to_ngx_buf(apr_bucket *e, ngx_pool_t *pool) { + ngx_buf_t *buf, *b; + apr_bucket_nginx *n; + ngx_uint_t len; + u_char *data; + + if (e->type->is_metadata) { + return NULL; + } + + if (e->type == &apr_bucket_type_nginx) { + n = e->data; + b = n->buf; + + /* whole buf */ + if (e->length == (apr_size_t)ngx_buf_size(b)) { + b->last_buf = 0; + return b; + } + + buf = ngx_palloc(pool, sizeof(ngx_buf_t)); + if (buf == NULL) { + return NULL; + } + ngx_memcpy(buf, b, sizeof(ngx_buf_t)); + + if (ngx_buf_in_memory(buf)) { + buf->start = buf->pos = buf->pos + e->start; + buf->end = buf->last = buf->pos + e->length; + } else { + buf->pos = NULL; + buf->file_pos += e->start; + buf->file_last = buf->file_pos + e->length; + } + + buf->last_buf = 0; + return buf; + } + + if (apr_bucket_read(e, (const char **)&data, + &len, APR_BLOCK_READ) != APR_SUCCESS) { + return NULL; + } + + buf = ngx_calloc_buf(pool); + if (buf == NULL) { + return NULL; + } + + if (e->type == &apr_bucket_type_pool) { + buf->start = data; + } else if (len != 0) { + buf->start = ngx_palloc(pool, len); + ngx_memcpy(buf->start, data, len); + } + + buf->pos = buf->start; + buf->end = buf->last = buf->start + len; + buf->temporary = 1; + return buf; +} + 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) { @@ -189,100 +189,100 @@ ngx_int_t copy_chain_to_brigade(ngx_chain_t *chain_orig, 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; + apr_bucket *e; ngx_chain_t *cl; 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; - } - cl = chain; - chain = chain->next; - ngx_free_chain(pool, cl); - } - - if (last_buf) { - e = apr_bucket_eos_create(bb->bucket_alloc); - APR_BRIGADE_INSERT_TAIL(bb, e); - return NGX_OK; - } - return NGX_AGAIN; -} - + + 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; + } + cl = chain; + chain = chain->next; + ngx_free_chain(pool, cl); + } + + 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_brigade_to_chain(apr_bucket_brigade *bb, ngx_chain_t **ll, ngx_pool_t *pool) { - apr_bucket *e; - ngx_buf_t *buf; - ngx_chain_t *cl; - - cl = NULL; - - if (APR_BRIGADE_EMPTY(bb)) { - *ll = NULL; - return NGX_OK; - } - - for (e = APR_BRIGADE_FIRST(bb); - e != APR_BRIGADE_SENTINEL(bb); - e = APR_BUCKET_NEXT(e)) { - - if (APR_BUCKET_IS_EOS(e)) { - if (cl == NULL) { - cl = ngx_alloc_chain_link(pool); - if (cl == NULL) { - break; - } - - cl->buf = ngx_calloc_buf(pool); - if (cl->buf == NULL) { - break; - } - - cl->buf->last_buf = 1; + apr_bucket *e; + ngx_buf_t *buf; + ngx_chain_t *cl; + + cl = NULL; + + if (APR_BRIGADE_EMPTY(bb)) { + *ll = NULL; + return NGX_OK; + } + + for (e = APR_BRIGADE_FIRST(bb); + e != APR_BRIGADE_SENTINEL(bb); + e = APR_BUCKET_NEXT(e)) { + + if (APR_BUCKET_IS_EOS(e)) { + if (cl == NULL) { + cl = ngx_alloc_chain_link(pool); + if (cl == NULL) { + break; + } + + cl->buf = ngx_calloc_buf(pool); + if (cl->buf == NULL) { + break; + } + + cl->buf->last_buf = 1; cl->next = NULL; - *ll = cl; - } else { + *ll = cl; + } else { cl->next = NULL; - cl->buf->last_buf = 1; - } - apr_brigade_cleanup(bb); - return NGX_OK; - } - - if (APR_BUCKET_IS_METADATA(e)) { - continue; - } - - buf = apr_bucket_to_ngx_buf(e, pool); - if (buf == NULL) { - break; - } - - cl = ngx_alloc_chain_link(pool); - if (cl == NULL) { - break; - } - - cl->buf = buf; - cl->next = NULL; - *ll = cl; - ll = &cl->next; - } - - - apr_brigade_cleanup(bb); - /* no eos or error */ - return NGX_ERROR; -} - + cl->buf->last_buf = 1; + } + apr_brigade_cleanup(bb); + return NGX_OK; + } + + if (APR_BUCKET_IS_METADATA(e)) { + continue; + } + + buf = apr_bucket_to_ngx_buf(e, pool); + if (buf == NULL) { + break; + } + + cl = ngx_alloc_chain_link(pool); + if (cl == NULL) { + break; + } + + cl->buf = buf; + cl->next = NULL; + *ll = cl; + ll = &cl->next; + } + + + apr_brigade_cleanup(bb); + /* no eos or error */ + return NGX_ERROR; +} + diff --git a/nginx/modsecurity/apr_bucket_nginx.h b/nginx/modsecurity/apr_bucket_nginx.h index 3f7004feb8..00b2b966d1 100644 --- a/nginx/modsecurity/apr_bucket_nginx.h +++ b/nginx/modsecurity/apr_bucket_nginx.h @@ -1,18 +1,18 @@ -#pragma once -#include -#include "apr_buckets.h" - -apr_bucket * apr_bucket_nginx_create(ngx_buf_t *buf, - apr_pool_t *p, - apr_bucket_alloc_t *list); - -apr_bucket * apr_bucket_nginx_make(apr_bucket *e, ngx_buf_t *buf, - apr_pool_t *pool); - -#define ngx_buf_to_apr_bucket apr_bucket_nginx_create - -ngx_buf_t * apr_bucket_to_ngx_buf(apr_bucket *e, ngx_pool_t *pool); - +#pragma once +#include +#include "apr_buckets.h" + +apr_bucket * apr_bucket_nginx_create(ngx_buf_t *buf, + apr_pool_t *p, + apr_bucket_alloc_t *list); + +apr_bucket * apr_bucket_nginx_make(apr_bucket *e, ngx_buf_t *buf, + apr_pool_t *pool); + +#define ngx_buf_to_apr_bucket apr_bucket_nginx_create + +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); @@ -21,4 +21,4 @@ ngx_int_t move_chain_to_brigade(ngx_chain_t *chain, apr_bucket_brigade *bb, ngx_int_t move_brigade_to_chain(apr_bucket_brigade *bb, ngx_chain_t **chain, ngx_pool_t *pool); - + From a841260ceac2b5ce2b387c3d2aadfb3be477cc6f Mon Sep 17 00:00:00 2001 From: paulyang Date: Wed, 14 May 2014 10:04:10 +0800 Subject: [PATCH 08/16] Bugfix: add -P option in test script Otherwise nginx's installation directory could not be specified. Signed-off-by: paulyang --- tests/run-regression-tests-nginx.pl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/run-regression-tests-nginx.pl b/tests/run-regression-tests-nginx.pl index 6d1ee4369f..98674d7f95 100755 --- a/tests/run-regression-tests-nginx.pl +++ b/tests/run-regression-tests-nginx.pl @@ -52,7 +52,7 @@ my $platform = "nginx"; my %opt; -getopts('A:E:D:C:T:H:a:p:dvh', \%opt); +getopts('A:E:D:C:T:H:P:a:p:dvh', \%opt); if ($opt{d}) { $Data::Dumper::Indent = 1; From 48465283fa912f9fb5606bac653e6ac5ce3f223e Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Wed, 14 May 2014 14:45:24 +0400 Subject: [PATCH 09/16] Removed unneeded and invalid initialization. --- nginx/modsecurity/ngx_http_modsecurity.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nginx/modsecurity/ngx_http_modsecurity.c b/nginx/modsecurity/ngx_http_modsecurity.c index fd8a69fa6b..aa5f8ec755 100644 --- a/nginx/modsecurity/ngx_http_modsecurity.c +++ b/nginx/modsecurity/ngx_http_modsecurity.c @@ -1091,7 +1091,7 @@ ngx_http_modsecurity_init_process(ngx_cycle_t *cycle) static ngx_int_t ngx_http_modsecurity_handler(ngx_http_request_t *r) { - ngx_int_t rc = NULL; + ngx_int_t rc; ngx_http_modsecurity_ctx_t *ctx = NULL; ngx_http_modsecurity_loc_conf_t *cf = NULL; From c7ed4dd26d014acdc74d94266b70cae23c7c77cd Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Thu, 15 May 2014 15:56:44 +0400 Subject: [PATCH 10/16] Obtain port from r->connection->local_sockaddr. This eliminates segfaults caused by unset (NULL) r->port_start and non-NULL r->port_end. In fact, r->port_start is always NULL, so it is useless to rely on this pointer. --- nginx/modsecurity/ngx_http_modsecurity.c | 34 ++++++++++++++++++++---- 1 file changed, 29 insertions(+), 5 deletions(-) diff --git a/nginx/modsecurity/ngx_http_modsecurity.c b/nginx/modsecurity/ngx_http_modsecurity.c index aa5f8ec755..c83eddde9d 100644 --- a/nginx/modsecurity/ngx_http_modsecurity.c +++ b/nginx/modsecurity/ngx_http_modsecurity.c @@ -279,9 +279,13 @@ ngx_http_modsecurity_load_request(ngx_http_request_t *r) { ngx_http_modsecurity_ctx_t *ctx; request_rec *req; - ngx_str_t str; size_t root; ngx_str_t path; + ngx_uint_t port; + struct sockaddr_in *sin; +#if (NGX_HAVE_INET6) + struct sockaddr_in6 *sin6; +#endif ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity); req = ctx->req; @@ -324,10 +328,30 @@ ngx_http_modsecurity_load_request(ngx_http_request_t *r) req->parsed_uri.path = (char *)ngx_pstrdup0(r->pool, &r->uri); req->parsed_uri.is_initialized = 1; - str.data = r->port_start; - str.len = r->port_end - r->port_start; - req->parsed_uri.port = ngx_atoi(str.data, str.len); - req->parsed_uri.port_str = (char *)ngx_pstrdup0(r->pool, &str); + switch (r->connection->local_sockaddr->sa_family) { + +#if (NGX_HAVE_INET6) + case AF_INET6: + sin6 = (struct sockaddr_in6 *) r->connection->local_sockaddr; + port = ntohs(sin6->sin6_port); + break; +#endif + +#if (NGX_HAVE_UNIX_DOMAIN) + case AF_UNIX: + port = 0; + break; +#endif + + default: /* AF_INET */ + sin = (struct sockaddr_in *) r->connection->local_sockaddr; + port = ntohs(sin->sin_port); + break; + } + + req->parsed_uri.port = port; + req->parsed_uri.port_str = ngx_pnalloc(r->pool, sizeof("65535")); + (void) ngx_sprintf((u_char *)req->parsed_uri.port_str, "%ui%c", port, '\0'); req->parsed_uri.query = r->args.len ? req->args : NULL; req->parsed_uri.dns_looked_up = 0; From 1d711162efc622e62ee77a0a6bb205f5dac71959 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Fri, 9 Jan 2015 05:46:09 -0800 Subject: [PATCH 11/16] nginx: fixing fuzzyHash test case for nginx POST was happening on a file that was not allowed by nginx to receive a POST. Nginx was returning 405 instead of 200 making the test to fail. Fixed by change the URL to one that is allowed to receive POST. --- tests/regression/misc/30-fuzzyHash.t | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/regression/misc/30-fuzzyHash.t b/tests/regression/misc/30-fuzzyHash.t index fcd1a899c7..1ef6969f58 100644 --- a/tests/regression/misc/30-fuzzyHash.t +++ b/tests/regression/misc/30-fuzzyHash.t @@ -108,7 +108,7 @@ SecRule REQBODY_ERROR \"!\@eq 0\" \ status => qr/^200$/, }, request => new HTTP::Request( - POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/index.html", + POST => "http://$ENV{SERVER_NAME}:$ENV{SERVER_PORT}/test.txt", [ "Content-Type" => "application/x-www-form-urlencoded", ], From dd61b187ee13a61fbe6b3ac25b9ec879470352f7 Mon Sep 17 00:00:00 2001 From: Ramandeep Singh Date: Mon, 30 Jun 2014 12:41:00 +0530 Subject: [PATCH 12/16] Passthrough the saved Response headers in the response Tickets: https://github.com/SpiderLabs/ModSecurity/issues/735 --- nginx/modsecurity/ngx_http_modsecurity.c | 17 ++++++++++++++++- 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/nginx/modsecurity/ngx_http_modsecurity.c b/nginx/modsecurity/ngx_http_modsecurity.c index c83eddde9d..1fac4f55a8 100644 --- a/nginx/modsecurity/ngx_http_modsecurity.c +++ b/nginx/modsecurity/ngx_http_modsecurity.c @@ -806,7 +806,7 @@ ngx_http_modsecurity_save_headers_out_visitor(void *data, const char *key, const char *value) { ngx_http_request_t *r = data; - ngx_table_elt_t *h, he; + ngx_table_elt_t *h, he, *new_h; ngx_http_upstream_header_t *hh; ngx_http_upstream_main_conf_t *umcf; @@ -837,6 +837,21 @@ ngx_http_modsecurity_save_headers_out_visitor(void *data, if (hh->copy_handler(r, h, hh->conf) != NGX_OK) { return 0; } + } else { + /* Add the response header directly to headers_out if not present in + * the hash. This is done to passthrough such response headers. + * Remember the response headers were cleared earlier using + * ngx_http_clean_header(r) call in ngx_http_modsecurity_save_headers_out. + */ + + new_h = ngx_list_push(&r->headers_out.headers); + if (new_h == NULL) { + return NGX_ERROR; + } + + new_h->hash = h->hash; + new_h->key = h->key; + new_h->value = h->value; } ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, From dc2461de5410b0e4fdc15afa0651292fa180bcbe Mon Sep 17 00:00:00 2001 From: Ramandeep Date: Fri, 8 Aug 2014 13:45:12 +0530 Subject: [PATCH 13/16] Allow non-zero Content-Length for HEAD requests --- nginx/modsecurity/ngx_http_modsecurity.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nginx/modsecurity/ngx_http_modsecurity.c b/nginx/modsecurity/ngx_http_modsecurity.c index 1fac4f55a8..e4e2a2c305 100644 --- a/nginx/modsecurity/ngx_http_modsecurity.c +++ b/nginx/modsecurity/ngx_http_modsecurity.c @@ -1412,7 +1412,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) "ModSecurity: save headers out done: done"); - if (r->headers_out.content_length_n != -1) { + if (r->headers_out.content_length_n != -1 && r->method != NGX_HTTP_HEAD) { r->headers_out.content_length_n = content_length; r->headers_out.content_length = NULL; /* header filter will set this */ From 1f1eea331d5e73dbd5cec36967bd86a7beb9e50c Mon Sep 17 00:00:00 2001 From: Fabricio Oliveira Date: Wed, 25 Feb 2015 15:10:10 -0300 Subject: [PATCH 14/16] Fix missing status_line when logging in nginx. --- apache2/mod_security2.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/apache2/mod_security2.c b/apache2/mod_security2.c index 519f2cc8db..aa2ba38b23 100644 --- a/apache2/mod_security2.c +++ b/apache2/mod_security2.c @@ -1278,7 +1278,7 @@ static int hook_log_transaction(request_rec *r) { msr->r = r; msr->response_status = r->status; - msr->status_line = ((r->status_line != NULL) + msr->status_line = ((r->status_line != NULL) && (*r->status_line != '\0') ? r->status_line : ap_get_status_line(r->status)); msr->response_protocol = get_response_protocol(origr); msr->response_headers = apr_table_copy(msr->mp, r->headers_out); From 226ad1df7d17e24694e838e0112db5f3123ee9a4 Mon Sep 17 00:00:00 2001 From: Felipe Zimmerle Date: Sat, 21 Mar 2015 11:50:48 -0700 Subject: [PATCH 15/16] Avoids segfault while running with proxy_pass Duplicates the headers variables while coping data from/to ModSecurity. This seems to fix the segfault that was happening while using proxy_pass. The variable is later cleaned, which means that we don't have a leak because of that. --- nginx/modsecurity/config | 14 ++++---- nginx/modsecurity/ngx_http_modsecurity.c | 44 ++++++++++++++---------- 2 files changed, 33 insertions(+), 25 deletions(-) diff --git a/nginx/modsecurity/config b/nginx/modsecurity/config index fe8471ba3d..2a1c266476 100644 --- a/nginx/modsecurity/config +++ b/nginx/modsecurity/config @@ -6,21 +6,23 @@ CFLAGS="$CFLAGS \ -I/usr/include/apache2 \ -I/usr/include/libxml2 \ -DWITH_LUA -I/usr/include/lua5.1 \ - -DWITH_PCRE_STUDY -DMODSEC_PCRE_MATCH_LIMIT=1500 -DMODSEC_PCRE_MATCH_LIMIT_RECURSION=1500 -DREQUEST_EARLY \ + -DWITH_PCRE_STUDY -DMODSEC_PCRE_MATCH_LIMIT=1500 -DMODSEC_PCRE_MATCH_LIMIT_RECURSION=1500 -DREQUEST_EARLY -DWITH_APU_CRYPTO -DWITH_REMOTE_RULES \ \ - -DWITH_YAJL -I/usr/include/yajl " + -DWITH_YAJL -I/usr/include/yajl \ + -DWITH_SSDEEP -I/usr/" CORE_LIBS="$CORE_LIBS \ - -L/usr/lib -lapr-1 \ - -L/usr/lib -laprutil-1 \ + -L/usr/lib/x86_64-linux-gnu -lapr-1 \ + -L/usr/lib/x86_64-linux-gnu -laprutil-1 \ -I/usr/include/apache2 \ -L/usr/lib/x86_64-linux-gnu -lcurl \ -lxml2 \ -llua5.1 \ -lpcre \ - -L/usr/lib -lcap \ - -lyajl " + -L/usr/lib \ + -lyajl \ + -lfuzzy" ngx_addon_name=ngx_http_modsecurity diff --git a/nginx/modsecurity/ngx_http_modsecurity.c b/nginx/modsecurity/ngx_http_modsecurity.c index e4e2a2c305..390e938f17 100644 --- a/nginx/modsecurity/ngx_http_modsecurity.c +++ b/nginx/modsecurity/ngx_http_modsecurity.c @@ -832,28 +832,34 @@ ngx_http_modsecurity_save_headers_out_visitor(void *data, hh = ngx_hash_find(&umcf->headers_in_hash, h->hash, h->lowcase_key, h->key.len); - if (hh) { - /* copy all */ - if (hh->copy_handler(r, h, hh->conf) != NGX_OK) { - return 0; - } - } else { - /* Add the response header directly to headers_out if not present in - * the hash. This is done to passthrough such response headers. - * Remember the response headers were cleared earlier using - * ngx_http_clean_header(r) call in ngx_http_modsecurity_save_headers_out. - */ - - new_h = ngx_list_push(&r->headers_out.headers); - if (new_h == NULL) { - return NGX_ERROR; - } + // While using proxy_pass with a combination of other factores + // there seems to be a memory corruption if we use hh->copy_handler. + // Temporary using new_h. This demand a further investigation. + // + //if (hh && 0 == 1) { + // /* copy all */ + // if (hh->copy_handler(r, h, hh->conf) != NGX_OK) { + // return 0; + // } + //} else { + + /* Add the response header directly to headers_out if not present in + * the hash. This is done to passthrough such response headers. + * Remember the response headers were cleared earlier using + * ngx_http_clean_header(r) call in ngx_http_modsecurity_save_headers_out. + */ - new_h->hash = h->hash; - new_h->key = h->key; - new_h->value = h->value; + new_h = ngx_list_push(&r->headers_out.headers); + if (new_h == NULL) { + return NGX_ERROR; } + new_h->hash = h->hash; + new_h->key = h->key; + new_h->value = h->value; + + // } + ngx_log_debug2(NGX_LOG_DEBUG_HTTP, r->connection->log, 0, "ModSecurity: save headers out: \"%V: %V\"", &h->key, &h->value); From 51200fca9df66e1a7cc5ee9a1fce78a1319a0815 Mon Sep 17 00:00:00 2001 From: littlecho Date: Thu, 26 Mar 2015 15:22:45 +0800 Subject: [PATCH 16/16] Update apache2_config.c Change third parameter(which is the apr file permission flag) from CREATEMODE to dcfg->auditlog_fileperms. Due to the user can specify the desired file permission setting for the audit log files with setting the value of SecAuditLogFileMode, we should follow the file permission setting from the config file. Therefore, as the dcfg->auditlog_fileperms will be modified in cmd_audit_log_dirmode function, we can use the value while calling apr_file_open to meet the file permission that specified in modsecurity.conf. --- apache2/apache2_config.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index bfbcb83468..5f602363f2 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -1189,10 +1189,13 @@ static const char *cmd_audit_log(cmd_parms *cmd, void *_dcfg, const char *p1) else { const char *file_name = ap_server_root_relative(cmd->pool, dcfg->auditlog_name); apr_status_t rc; - + + if (dcfg->auditlog_fileperms == NOT_SET) { + dcfg->auditlog_fileperms = CREATEMODE; + } rc = apr_file_open(&dcfg->auditlog_fd, file_name, APR_WRITE | APR_APPEND | APR_CREATE | APR_BINARY, - CREATEMODE, cmd->pool); + dcfg->auditlog_fileperms, cmd->pool); if (rc != APR_SUCCESS) { return apr_psprintf(cmd->pool, "ModSecurity: Failed to open the audit log file: %s",