From 2db3f6dd9a781ccfa1eb584d75940275583130b0 Mon Sep 17 00:00:00 2001 From: Liu Dongmiao Date: Sat, 12 Mar 2022 03:05:35 +0800 Subject: [PATCH 1/2] recovery context after internal redirect --- src/ngx_http_modsecurity_body_filter.c | 2 +- src/ngx_http_modsecurity_common.h | 2 ++ src/ngx_http_modsecurity_header_filter.c | 20 ++++++++++---------- src/ngx_http_modsecurity_log.c | 2 +- src/ngx_http_modsecurity_module.c | 23 ++++++++++++++++++++++- src/ngx_http_modsecurity_pre_access.c | 10 ++++++++-- src/ngx_http_modsecurity_rewrite.c | 2 +- 7 files changed, 45 insertions(+), 16 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index 725f9869..82ac8e27 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -48,7 +48,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) return ngx_http_next_body_filter(r, in); } - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); dd("body filter, recovering ctx: %p", ctx); diff --git a/src/ngx_http_modsecurity_common.h b/src/ngx_http_modsecurity_common.h index 60218c4b..70230c69 100644 --- a/src/ngx_http_modsecurity_common.h +++ b/src/ngx_http_modsecurity_common.h @@ -99,6 +99,7 @@ typedef struct { unsigned processed:1; unsigned logged:1; unsigned intervention_triggered:1; + unsigned request_body_processed:1; } ngx_http_modsecurity_ctx_t; @@ -139,6 +140,7 @@ extern ngx_module_t ngx_http_modsecurity_module; /* ngx_http_modsecurity_module.c */ int ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_request_t *r, ngx_int_t early_log); ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_create_ctx(ngx_http_request_t *r); +ngx_http_modsecurity_ctx_t *ngx_http_modsecurity_get_module_ctx(ngx_http_request_t *r); char *ngx_str_to_char(ngx_str_t a, ngx_pool_t *p); ngx_pool_t *ngx_http_modsecurity_pcre_malloc_init(ngx_pool_t *pool); void ngx_http_modsecurity_pcre_malloc_done(ngx_pool_t *old_pool); diff --git a/src/ngx_http_modsecurity_header_filter.c b/src/ngx_http_modsecurity_header_filter.c index c36be19a..cb0ac4d4 100644 --- a/src/ngx_http_modsecurity_header_filter.c +++ b/src/ngx_http_modsecurity_header_filter.c @@ -107,7 +107,7 @@ ngx_http_modsecurity_store_ctx_header(ngx_http_request_t *r, ngx_str_t *name, ng ngx_http_modsecurity_conf_t *mcf; ngx_http_modsecurity_header_t *hdr; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (ctx == NULL || ctx->sanity_headers_out == NULL) { return NGX_ERROR; } @@ -150,7 +150,7 @@ ngx_http_modsecurity_resolv_header_server(ngx_http_request_t *r, ngx_str_t name, ngx_str_t value; clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (r->headers_out.server == NULL) { if (clcf->server_tokens) { @@ -184,7 +184,7 @@ ngx_http_modsecurity_resolv_header_date(ngx_http_request_t *r, ngx_str_t name, o ngx_http_modsecurity_ctx_t *ctx = NULL; ngx_str_t date; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (r->headers_out.date == NULL) { date.data = ngx_cached_http_time.data; @@ -214,7 +214,7 @@ ngx_http_modsecurity_resolv_header_content_length(ngx_http_request_t *r, ngx_str ngx_str_t value; char buf[NGX_INT64_LEN+2]; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (r->headers_out.content_length_n > 0) { @@ -241,7 +241,7 @@ ngx_http_modsecurity_resolv_header_content_type(ngx_http_request_t *r, ngx_str_t { ngx_http_modsecurity_ctx_t *ctx = NULL; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (r->headers_out.content_type.len > 0) { @@ -268,7 +268,7 @@ ngx_http_modsecurity_resolv_header_last_modified(ngx_http_request_t *r, ngx_str_ u_char buf[1024], *p; ngx_str_t value; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (r->headers_out.last_modified_time == -1) { return 1; @@ -300,7 +300,7 @@ ngx_http_modsecurity_resolv_header_connection(ngx_http_request_t *r, ngx_str_t n ngx_str_t value; clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (r->headers_out.status == NGX_HTTP_SWITCHING_PROTOCOLS) { connection = "upgrade"; @@ -351,7 +351,7 @@ ngx_http_modsecurity_resolv_header_transfer_encoding(ngx_http_request_t *r, ngx_ if (r->chunked) { ngx_str_t value = ngx_string("chunked"); - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); #if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) ngx_http_modsecurity_store_ctx_header(r, &name, &value); @@ -378,7 +378,7 @@ ngx_http_modsecurity_resolv_header_vary(ngx_http_request_t *r, ngx_str_t name, o if (r->gzip_vary && clcf->gzip_vary) { ngx_str_t value = ngx_string("Accept-Encoding"); - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); #if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) ngx_http_modsecurity_store_ctx_header(r, &name, &value); @@ -420,7 +420,7 @@ ngx_http_modsecurity_header_filter(ngx_http_request_t *r) /* XXX: if NOT_MODIFIED, do we need to process it at all? see xslt_header_filter() */ - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); dd("header filter, recovering ctx: %p", ctx); diff --git a/src/ngx_http_modsecurity_log.c b/src/ngx_http_modsecurity_log.c index d713a65a..8303dc1c 100644 --- a/src/ngx_http_modsecurity_log.c +++ b/src/ngx_http_modsecurity_log.c @@ -58,7 +58,7 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r) return NGX_OK; } */ - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); dd("recovering ctx: %p", ctx); diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index b6f33f56..7ed2ac59 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -142,7 +142,7 @@ ngx_http_modsecurity_process_intervention (Transaction *transaction, ngx_http_re dd("processing intervention"); - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); if (ctx == NULL) { return NGX_HTTP_INTERNAL_SERVER_ERROR; @@ -306,6 +306,27 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) return ctx; } +ngx_inline ngx_http_modsecurity_ctx_t * +ngx_http_modsecurity_get_module_ctx(ngx_http_request_t *r) +{ + ngx_http_modsecurity_ctx_t *ctx; + ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + if (ctx == NULL) { + /* + * refer /src/http/modules/ngx_http_realip_module.c + * if module context was reset, the original address + * can still be found in the cleanup handler + */ + ngx_pool_cleanup_t *cln; + for (cln = r->pool->cleanup; cln; cln = cln->next) { + if (cln->handler == ngx_http_modsecurity_cleanup) { + ctx = cln->data; + break; + } + } + } + return ctx; +} char * ngx_conf_set_rules(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) diff --git a/src/ngx_http_modsecurity_pre_access.c b/src/ngx_http_modsecurity_pre_access.c index abb7d3e2..c6a56f38 100644 --- a/src/ngx_http_modsecurity_pre_access.c +++ b/src/ngx_http_modsecurity_pre_access.c @@ -25,7 +25,7 @@ ngx_http_modsecurity_request_read(ngx_http_request_t *r) { ngx_http_modsecurity_ctx_t *ctx; - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); #if defined(nginx_version) && nginx_version >= 8011 r->main->count--; @@ -68,7 +68,7 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r) } */ - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); dd("recovering ctx: %p", ctx); @@ -78,6 +78,11 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r) return NGX_HTTP_INTERNAL_SERVER_ERROR; } + if (ctx->request_body_processed) { + // should we use r->internal or r->filter_finalize? + return NGX_DECLINED; + } + if (ctx->intervention_triggered) { return NGX_DECLINED; } @@ -210,6 +215,7 @@ ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r) old_pool = ngx_http_modsecurity_pcre_malloc_init(r->pool); msc_process_request_body(ctx->modsec_transaction); + ctx->request_body_processed = 1; ngx_http_modsecurity_pcre_malloc_done(old_pool); ret = ngx_http_modsecurity_process_intervention(ctx->modsec_transaction, r, 0); diff --git a/src/ngx_http_modsecurity_rewrite.c b/src/ngx_http_modsecurity_rewrite.c index f6f8d413..f7302702 100644 --- a/src/ngx_http_modsecurity_rewrite.c +++ b/src/ngx_http_modsecurity_rewrite.c @@ -44,7 +44,7 @@ ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r) dd("catching a new _rewrite_ phase handler"); - ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); + ctx = ngx_http_modsecurity_get_module_ctx(r); dd("recovering ctx: %p", ctx); From 5b6b4d8159fa2202e4f2bff1b052c827b32267ae Mon Sep 17 00:00:00 2001 From: Liu Dongmiao Date: Wed, 31 Aug 2022 09:45:03 +0800 Subject: [PATCH 2/2] use context to determine whether modsecurity is enabled in log phase log phase would use the final location for redirect, so we should use context to determine whether it should be logged. --- src/ngx_http_modsecurity_log.c | 11 ++--------- 1 file changed, 2 insertions(+), 9 deletions(-) diff --git a/src/ngx_http_modsecurity_log.c b/src/ngx_http_modsecurity_log.c index 8303dc1c..38fbc3a2 100644 --- a/src/ngx_http_modsecurity_log.c +++ b/src/ngx_http_modsecurity_log.c @@ -39,16 +39,9 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r) { ngx_pool_t *old_pool; ngx_http_modsecurity_ctx_t *ctx; - ngx_http_modsecurity_conf_t *mcf; dd("catching a new _log_ phase handler"); - mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); - if (mcf == NULL || mcf->enable != 1) - { - dd("ModSecurity not enabled... returning"); - return NGX_OK; - } /* if (r->method != NGX_HTTP_GET && @@ -63,8 +56,8 @@ ngx_http_modsecurity_log_handler(ngx_http_request_t *r) dd("recovering ctx: %p", ctx); if (ctx == NULL) { - dd("something really bad happened here. returning NGX_ERROR"); - return NGX_ERROR; + dd("ModSecurity not enabled or error occurred"); + return NGX_OK; } if (ctx->logged) {