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 725f986..82ac8e2 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 60218c4..70230c6 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 c36be19..cb0ac4d 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 d713a65..8303dc1 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 b6f33f5..7ed2ac5 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 abb7d3e..c6a56f3 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 f6f8d41..f730270 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 8303dc1..38fbc3a 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) {