From e00e2cc3119dc7863cdb67591a178a206f0199a9 Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Thu, 29 Nov 2018 18:08:54 +0300 Subject: [PATCH 1/6] Do not use main/server contexts for creating/merging configuration --- src/ngx_http_modsecurity_module.c | 168 +++++++++--------------------- 1 file changed, 50 insertions(+), 118 deletions(-) diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 199d992..e9218b1 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -25,12 +25,9 @@ #include static ngx_int_t ngx_http_modsecurity_init(ngx_conf_t *cf); -static void *ngx_http_modsecurity_create_main_conf(ngx_conf_t *cf); static void *ngx_http_modsecurity_create_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_srv_conf(ngx_conf_t *cf, void *parent, void *child); +static char *ngx_http_modsecurity_merge_conf(ngx_conf_t *cf, void *parent, void *child); static void ngx_http_modsecurity_config_cleanup(void *data); -static char *ngx_http_modsecurity_init_main_conf(ngx_conf_t *cf, void *conf); /* @@ -234,11 +231,10 @@ ngx_http_modsecurity_cleanup(void *data) ngx_inline ngx_http_modsecurity_ctx_t * ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) { - ngx_http_modsecurity_ctx_t *ctx; - ngx_http_modsecurity_conf_t *loc_cf = NULL; - ngx_http_modsecurity_conf_t *cf = NULL; - ngx_pool_cleanup_t *cln = NULL; - ngx_str_t s; + ngx_str_t s; + ngx_pool_cleanup_t *cln; + ngx_http_modsecurity_ctx_t *ctx; + ngx_http_modsecurity_conf_t *mcf; ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_modsecurity_ctx_t)); if (ctx == NULL) @@ -246,19 +242,19 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) dd("failed to allocate memory for the context."); return NULL; } - cf = ngx_http_get_module_main_conf(r, ngx_http_modsecurity_module); - loc_cf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); - dd("creating transaction with the following rules: '%p' -- ms: '%p'", loc_cf->rules_set, cf->modsec); + mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); + + dd("creating transaction with the following rules: '%p' -- ms: '%p'", mcf->rules_set, mcf->modsec); - if (loc_cf->transaction_id) { - if (ngx_http_complex_value(r, loc_cf->transaction_id, &s) != NGX_OK) { + if (mcf->transaction_id) { + if (ngx_http_complex_value(r, mcf->transaction_id, &s) != NGX_OK) { return NGX_CONF_ERROR; } - ctx->modsec_transaction = msc_new_transaction_with_id(cf->modsec, loc_cf->rules_set, (char *) s.data, r->connection->log); + ctx->modsec_transaction = msc_new_transaction_with_id(mcf->modsec, mcf->rules_set, (char *) s.data, r->connection->log); } else { - ctx->modsec_transaction = msc_new_transaction(cf->modsec, loc_cf->rules_set, r->connection->log); + ctx->modsec_transaction = msc_new_transaction(mcf->modsec, mcf->rules_set, r->connection->log); } dd("transaction created"); @@ -437,32 +433,32 @@ static ngx_command_t ngx_http_modsecurity_commands[] = { static ngx_http_module_t ngx_http_modsecurity_ctx = { - NULL, /* preconfiguration */ - ngx_http_modsecurity_init, /* postconfiguration */ + NULL, /* preconfiguration */ + ngx_http_modsecurity_init, /* postconfiguration */ - ngx_http_modsecurity_create_main_conf, /* create main configuration */ - ngx_http_modsecurity_init_main_conf, /* init main configuration */ + NULL, /* create main configuration */ + NULL, /* init main configuration */ - ngx_http_modsecurity_create_conf, /* create server configuration */ - ngx_http_modsecurity_merge_srv_conf, /* merge server configuration */ + NULL, /* create server configuration */ + NULL, /* merge server configuration */ - ngx_http_modsecurity_create_conf, /* create location configuration */ - ngx_http_modsecurity_merge_loc_conf /* merge location configuration */ + ngx_http_modsecurity_create_conf, /* create location configuration */ + ngx_http_modsecurity_merge_conf /* merge location configuration */ }; ngx_module_t ngx_http_modsecurity_module = { NGX_MODULE_V1, - &ngx_http_modsecurity_ctx, /* module context */ - ngx_http_modsecurity_commands, /* module directives */ - NGX_HTTP_MODULE, /* module type */ - NULL, /* init master */ - NULL, /* init module */ - NULL, /* init process */ - NULL, /* init thread */ - NULL, /* exit thread */ - NULL, /* exit process */ - NULL, /* exit master */ + &ngx_http_modsecurity_ctx, /* module context */ + ngx_http_modsecurity_commands, /* module directives */ + NGX_HTTP_MODULE, /* module type */ + NULL, /* init master */ + NULL, /* init module */ + NULL, /* init process */ + NULL, /* init thread */ + NULL, /* exit thread */ + NULL, /* exit process */ + NULL, /* exit master */ NGX_MODULE_V1_PADDING }; @@ -545,51 +541,15 @@ ngx_http_modsecurity_init(ngx_conf_t *cf) static void * -ngx_http_modsecurity_create_main_conf(ngx_conf_t *cf) +ngx_http_modsecurity_create_conf(ngx_conf_t *cf) { - ngx_http_modsecurity_conf_t *conf; + ngx_pool_cleanup_t *cln; + ngx_http_modsecurity_conf_t *conf; ngx_log_error(NGX_LOG_NOTICE, cf->log, 0, MODSECURITY_NGINX_WHOAMI); - /* ngx_pcalloc already sets all of this scructure to zeros. */ - conf = ngx_http_modsecurity_create_conf(cf); - - if (conf == NULL || conf == NGX_CONF_ERROR) { - dd("failed to allocate space for the ModSecurity configuration"); - return NGX_CONF_ERROR; - } - - dd ("conf crated at: '%p'", conf); - - /* Create our ModSecurity instace */ - conf->modsec = msc_init(); - if (conf->modsec == NULL) - { - dd("failed to create the ModSecurity instance"); - return NGX_CONF_ERROR; - } - - /* Provide our connector information to LibModSecurity */ - msc_set_connector_info(conf->modsec, MODSECURITY_NGINX_WHOAMI); - msc_set_log_cb(conf->modsec, ngx_http_modsecurity_log); - - return conf; -} - - -static char *ngx_http_modsecurity_init_main_conf(ngx_conf_t *cf, void *conf) -{ - dd("modsec main conf init. Loaded rules:"); - - return NGX_CONF_OK; -} - - -static void *ngx_http_modsecurity_create_conf(ngx_conf_t *cf) -{ - ngx_pool_cleanup_t *cln = NULL; - ngx_http_modsecurity_conf_t *conf = (ngx_http_modsecurity_conf_t *) - ngx_pcalloc(cf->pool, sizeof(ngx_http_modsecurity_conf_t)); + conf = (ngx_http_modsecurity_conf_t *) ngx_pcalloc(cf->pool, + sizeof(ngx_http_modsecurity_conf_t)); if (conf == NULL) { @@ -619,58 +579,30 @@ static void *ngx_http_modsecurity_create_conf(ngx_conf_t *cf) dd("failed to create the ModSecurity configuration cleanup"); return NGX_CONF_ERROR; } + cln->handler = ngx_http_modsecurity_config_cleanup; cln->data = conf; - return conf; -} - + dd ("conf created at: '%p'", conf); -static char * -ngx_http_modsecurity_merge_srv_conf(ngx_conf_t *cf, void *parent, void *child) -{ - ngx_http_modsecurity_conf_t *p = parent; - ngx_http_modsecurity_conf_t *c = child; -#if defined(MODSECURITY_DDEBUG) && (MODSECURITY_DDEBUG) - ngx_http_core_srv_conf_t *clcf = ngx_http_conf_get_module_srv_conf(cf, ngx_http_core_module); -#endif - int rules; - const char *error = NULL; - - dd("merging srv config [%s] - parent: '%p' child: '%p'", - ngx_str_to_char(clcf->server_name, cf->pool), parent, - child); - dd(" state - parent: '%d' child: '%d'", - (int) p->enable, (int) c->enable); - - ngx_conf_merge_value(c->enable, p->enable, 0); - ngx_conf_merge_value(c->sanity_checks_enabled, p->sanity_checks_enabled, 0); - ngx_conf_merge_ptr_value(c->transaction_id, p->transaction_id, NULL); - -#if defined(MODSECURITY_DDEBUG) && (MODSECURITY_DDEBUG) - dd("PARENT RULES"); - msc_rules_dump(p->rules_set); - dd("CHILD RULES"); - msc_rules_dump(c->rules_set); -#endif + /* Create our ModSecurity instance */ + conf->modsec = msc_init(); + if (conf->modsec == NULL) + { + dd("failed to create the ModSecurity instance"); + return NGX_CONF_ERROR; + } - rules = msc_rules_merge(c->rules_set, p->rules_set, &error); + /* Provide our connector information to LibModSecurity */ + msc_set_connector_info(conf->modsec, MODSECURITY_NGINX_WHOAMI); + msc_set_log_cb(conf->modsec, ngx_http_modsecurity_log); - if (rules < 0) { - return strdup(error); - } - dd(" state - this: '%d'", - (int) c->enable); -#if defined(MODSECURITY_DDEBUG) && (MODSECURITY_DDEBUG) - dd("NEW CHIELD RULES"); - msc_rules_dump(c->rules_set); -#endif - return NGX_CONF_OK; + return conf; } static char * -ngx_http_modsecurity_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) +ngx_http_modsecurity_merge_conf(ngx_conf_t *cf, void *parent, void *child) { ngx_http_modsecurity_conf_t *p = parent; ngx_http_modsecurity_conf_t *c = child; @@ -704,7 +636,7 @@ ngx_http_modsecurity_merge_loc_conf(ngx_conf_t *cf, void *parent, void *child) } #if defined(MODSECURITY_DDEBUG) && (MODSECURITY_DDEBUG) - dd("NEW CHIELD RULES"); + dd("NEW CHILD RULES"); msc_rules_dump(c->rules_set); #endif return NGX_CONF_OK; From ae765f2c1385c136eada7ae905355948a1c11bea Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Fri, 30 Nov 2018 13:58:11 +0300 Subject: [PATCH 2/6] Emit connector version just once (broken in e00e2cc) --- src/ngx_http_modsecurity_module.c | 14 +++++++++++--- 1 file changed, 11 insertions(+), 3 deletions(-) diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index e9218b1..0367627 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -25,6 +25,7 @@ #include static ngx_int_t ngx_http_modsecurity_init(ngx_conf_t *cf); +static char *ngx_http_modsecurity_init_main_conf(ngx_conf_t *cf, void *conf); static void *ngx_http_modsecurity_create_conf(ngx_conf_t *cf); static char *ngx_http_modsecurity_merge_conf(ngx_conf_t *cf, void *parent, void *child); static void ngx_http_modsecurity_config_cleanup(void *data); @@ -437,7 +438,7 @@ static ngx_http_module_t ngx_http_modsecurity_ctx = { ngx_http_modsecurity_init, /* postconfiguration */ NULL, /* create main configuration */ - NULL, /* init main configuration */ + ngx_http_modsecurity_init_main_conf, /* init main configuration */ NULL, /* create server configuration */ NULL, /* merge server configuration */ @@ -540,14 +541,21 @@ ngx_http_modsecurity_init(ngx_conf_t *cf) } +static char * +ngx_http_modsecurity_init_main_conf(ngx_conf_t *cf, void *conf) +{ + ngx_log_error(NGX_LOG_NOTICE, cf->log, 0, MODSECURITY_NGINX_WHOAMI); + + return NGX_CONF_OK; +} + + static void * ngx_http_modsecurity_create_conf(ngx_conf_t *cf) { ngx_pool_cleanup_t *cln; ngx_http_modsecurity_conf_t *conf; - ngx_log_error(NGX_LOG_NOTICE, cf->log, 0, MODSECURITY_NGINX_WHOAMI); - conf = (ngx_http_modsecurity_conf_t *) ngx_pcalloc(cf->pool, sizeof(ngx_http_modsecurity_conf_t)); From 7b8f1ef3e2b5654c90211d55f5f86b7d7b83f2a7 Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Fri, 30 Nov 2018 17:22:28 +0300 Subject: [PATCH 3/6] Introduce separate configuration for main context It is being used for creating ModSecurity instance via msc_init() call, as well as setting up necessary cleanups for the instance. While here: - refactored cleanup handler for common context to highlight that its primary goal is to release memory consumed by rules; - made all sanity checks related code to be included only when MODSECURITY_SANITY_CHECKS is true. --- src/ngx_http_modsecurity_common.h | 17 ++-- src/ngx_http_modsecurity_module.c | 136 ++++++++++++++++++++++-------- 2 files changed, 110 insertions(+), 43 deletions(-) diff --git a/src/ngx_http_modsecurity_common.h b/src/ngx_http_modsecurity_common.h index 79355d1..8a2962a 100644 --- a/src/ngx_http_modsecurity_common.h +++ b/src/ngx_http_modsecurity_common.h @@ -68,7 +68,7 @@ typedef struct { Transaction *modsec_transaction; ModSecurityIntervention *delayed_intervention; -#ifdef MODSECURITY_SANITY_CHECKS +#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) /* * Should be filled with the headers that were sent to ModSecurity. * @@ -87,14 +87,19 @@ typedef struct { typedef struct { - ModSecurity *modsec; + void *pool; + ModSecurity *modsec; +} ngx_http_modsecurity_main_conf_t; - ngx_flag_t enable; - ngx_flag_t sanity_checks_enabled; - Rules *rules_set; +typedef struct { + void *pool; + Rules *rules_set; - void *pool; + ngx_flag_t enable; +#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) + ngx_flag_t sanity_checks_enabled; +#endif ngx_http_complex_value_t *transaction_id; } ngx_http_modsecurity_conf_t; diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 0367627..4816d81 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -25,10 +25,12 @@ #include static ngx_int_t ngx_http_modsecurity_init(ngx_conf_t *cf); +static void *ngx_http_modsecurity_create_main_conf(ngx_conf_t *cf); static char *ngx_http_modsecurity_init_main_conf(ngx_conf_t *cf, void *conf); static void *ngx_http_modsecurity_create_conf(ngx_conf_t *cf); static char *ngx_http_modsecurity_merge_conf(ngx_conf_t *cf, void *parent, void *child); -static void ngx_http_modsecurity_config_cleanup(void *data); +static void ngx_http_modsecurity_cleanup_instance(void *data); +static void ngx_http_modsecurity_cleanup_rules(void *data); /* @@ -232,10 +234,11 @@ ngx_http_modsecurity_cleanup(void *data) ngx_inline ngx_http_modsecurity_ctx_t * ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) { - ngx_str_t s; - ngx_pool_cleanup_t *cln; - ngx_http_modsecurity_ctx_t *ctx; - ngx_http_modsecurity_conf_t *mcf; + ngx_str_t s; + ngx_pool_cleanup_t *cln; + ngx_http_modsecurity_ctx_t *ctx; + ngx_http_modsecurity_conf_t *mlcf; + ngx_http_modsecurity_main_conf_t *mmcf; ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_modsecurity_ctx_t)); if (ctx == NULL) @@ -244,18 +247,19 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) return NULL; } - mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); + mmcf = ngx_http_get_module_main_conf(r, ngx_http_modsecurity_module); + mlcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); - dd("creating transaction with the following rules: '%p' -- ms: '%p'", mcf->rules_set, mcf->modsec); + dd("creating transaction with the following rules: '%p' -- ms: '%p'", mlcf->rules_set, mmcf->modsec); - if (mcf->transaction_id) { - if (ngx_http_complex_value(r, mcf->transaction_id, &s) != NGX_OK) { + if (mlcf->transaction_id) { + if (ngx_http_complex_value(r, mlcf->transaction_id, &s) != NGX_OK) { return NGX_CONF_ERROR; } - ctx->modsec_transaction = msc_new_transaction_with_id(mcf->modsec, mcf->rules_set, (char *) s.data, r->connection->log); + ctx->modsec_transaction = msc_new_transaction_with_id(mmcf->modsec, mlcf->rules_set, (char *) s.data, r->connection->log); } else { - ctx->modsec_transaction = msc_new_transaction(mcf->modsec, mcf->rules_set, r->connection->log); + ctx->modsec_transaction = msc_new_transaction(mmcf->modsec, mlcf->rules_set, r->connection->log); } dd("transaction created"); @@ -437,7 +441,7 @@ static ngx_http_module_t ngx_http_modsecurity_ctx = { NULL, /* preconfiguration */ ngx_http_modsecurity_init, /* postconfiguration */ - NULL, /* create main configuration */ + ngx_http_modsecurity_create_main_conf, /* create main configuration */ ngx_http_modsecurity_init_main_conf, /* init main configuration */ NULL, /* create server configuration */ @@ -541,6 +545,55 @@ ngx_http_modsecurity_init(ngx_conf_t *cf) } +static void * +ngx_http_modsecurity_create_main_conf(ngx_conf_t *cf) +{ + ngx_pool_cleanup_t *cln; + ngx_http_modsecurity_main_conf_t *conf; + + conf = (ngx_http_modsecurity_main_conf_t *) ngx_pcalloc(cf->pool, + sizeof(ngx_http_modsecurity_main_conf_t)); + + if (conf == NULL) + { + return NGX_CONF_ERROR; + } + + /* + * set by ngx_pcalloc(): + * + * conf->modsec = NULL; + * conf->pool = NULL; + */ + + cln = ngx_pool_cleanup_add(cf->pool, 0); + if (cln == NULL) { + return NGX_CONF_ERROR; + } + + cln->handler = ngx_http_modsecurity_cleanup_instance; + cln->data = conf; + + conf->pool = cf->pool; + + /* Create our ModSecurity instance */ + conf->modsec = msc_init(); + if (conf->modsec == NULL) + { + dd("failed to create the ModSecurity instance"); + return NGX_CONF_ERROR; + } + + /* Provide our connector information to LibModSecurity */ + msc_set_connector_info(conf->modsec, MODSECURITY_NGINX_WHOAMI); + msc_set_log_cb(conf->modsec, ngx_http_modsecurity_log); + + dd ("main conf created at: '%p', instance is: '%p'", conf, conf->modsec); + + return conf; +} + + static char * ngx_http_modsecurity_init_main_conf(ngx_conf_t *cf, void *conf) { @@ -568,7 +621,6 @@ ngx_http_modsecurity_create_conf(ngx_conf_t *cf) /* * set by ngx_pcalloc(): * - * conf->modsec = NULL; * conf->enable = 0; * conf->sanity_checks_enabled = 0; * conf->rules_set = NULL; @@ -577,10 +629,12 @@ ngx_http_modsecurity_create_conf(ngx_conf_t *cf) */ conf->enable = NGX_CONF_UNSET; - conf->sanity_checks_enabled = NGX_CONF_UNSET; conf->rules_set = msc_create_rules_set(); conf->pool = cf->pool; conf->transaction_id = NGX_CONF_UNSET_PTR; +#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) + conf->sanity_checks_enabled = NGX_CONF_UNSET; +#endif cln = ngx_pool_cleanup_add(cf->pool, 0); if (cln == NULL) { @@ -588,23 +642,11 @@ ngx_http_modsecurity_create_conf(ngx_conf_t *cf) return NGX_CONF_ERROR; } - cln->handler = ngx_http_modsecurity_config_cleanup; + cln->handler = ngx_http_modsecurity_cleanup_rules; cln->data = conf; dd ("conf created at: '%p'", conf); - /* Create our ModSecurity instance */ - conf->modsec = msc_init(); - if (conf->modsec == NULL) - { - dd("failed to create the ModSecurity instance"); - return NGX_CONF_ERROR; - } - - /* Provide our connector information to LibModSecurity */ - msc_set_connector_info(conf->modsec, MODSECURITY_NGINX_WHOAMI); - msc_set_log_cb(conf->modsec, ngx_http_modsecurity_log); - return conf; } @@ -628,8 +670,10 @@ ngx_http_modsecurity_merge_conf(ngx_conf_t *cf, void *parent, void *child) (int) c->enable, (int) p->enable); ngx_conf_merge_value(c->enable, p->enable, 0); - ngx_conf_merge_value(c->sanity_checks_enabled, p->sanity_checks_enabled, 0); ngx_conf_merge_ptr_value(c->transaction_id, p->transaction_id, NULL); +#if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) + ngx_conf_merge_value(c->sanity_checks_enabled, p->sanity_checks_enabled, 0); +#endif #if defined(MODSECURITY_DDEBUG) && (MODSECURITY_DDEBUG) dd("PARENT RULES"); @@ -652,20 +696,38 @@ ngx_http_modsecurity_merge_conf(ngx_conf_t *cf, void *parent, void *child) static void -ngx_http_modsecurity_config_cleanup(void *data) +ngx_http_modsecurity_cleanup_instance(void *data) { - ngx_pool_t *old_pool; - ngx_http_modsecurity_conf_t *t = (ngx_http_modsecurity_conf_t *) data; + ngx_pool_t *old_pool; + ngx_http_modsecurity_main_conf_t *conf; + + conf = (ngx_http_modsecurity_main_conf_t *) data; + + dd("deleting a main conf -- instance is: \"%p\"", conf->modsec); + + old_pool = ngx_http_modsecurity_pcre_malloc_init(conf->pool); + msc_cleanup(conf->modsec); + ngx_http_modsecurity_pcre_malloc_done(old_pool); + + conf->modsec = NULL; +} + + +static void +ngx_http_modsecurity_cleanup_rules(void *data) +{ + ngx_pool_t *old_pool; + ngx_http_modsecurity_conf_t *conf; + + conf = (ngx_http_modsecurity_conf_t *) data; - dd("deleting a loc conf -- RuleSet is: \"%p\"", t->rules_set); + dd("deleting a loc conf -- RuleSet is: \"%p\"", conf->rules_set); - old_pool = ngx_http_modsecurity_pcre_malloc_init(t->pool); - msc_rules_cleanup(t->rules_set); - msc_cleanup(t->modsec); + old_pool = ngx_http_modsecurity_pcre_malloc_init(conf->pool); + msc_rules_cleanup(conf->rules_set); ngx_http_modsecurity_pcre_malloc_done(old_pool); - t->rules_set = NULL; - t->modsec = NULL; + conf->rules_set = NULL; } From 3e256b0de98ec85f41bfbd5cfc7cbbbb4a96a9b7 Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Fri, 30 Nov 2018 17:44:13 +0300 Subject: [PATCH 4/6] Small rewording in cleanup handlers Also, there is no need to set corresponding pointers to NULL as cleanup handler should be called only once (per context). --- src/ngx_http_modsecurity_module.c | 24 ++++++++++-------------- 1 file changed, 10 insertions(+), 14 deletions(-) diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 4816d81..04c4e1d 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -699,17 +699,15 @@ static void ngx_http_modsecurity_cleanup_instance(void *data) { ngx_pool_t *old_pool; - ngx_http_modsecurity_main_conf_t *conf; + ngx_http_modsecurity_main_conf_t *mmcf; - conf = (ngx_http_modsecurity_main_conf_t *) data; + mmcf = (ngx_http_modsecurity_main_conf_t *) data; - dd("deleting a main conf -- instance is: \"%p\"", conf->modsec); + dd("deleting a main conf -- instance is: \"%p\"", mmcf->modsec); - old_pool = ngx_http_modsecurity_pcre_malloc_init(conf->pool); - msc_cleanup(conf->modsec); + old_pool = ngx_http_modsecurity_pcre_malloc_init(mmcf->pool); + msc_cleanup(mmcf->modsec); ngx_http_modsecurity_pcre_malloc_done(old_pool); - - conf->modsec = NULL; } @@ -717,17 +715,15 @@ static void ngx_http_modsecurity_cleanup_rules(void *data) { ngx_pool_t *old_pool; - ngx_http_modsecurity_conf_t *conf; + ngx_http_modsecurity_conf_t *mcf; - conf = (ngx_http_modsecurity_conf_t *) data; + mcf = (ngx_http_modsecurity_conf_t *) data; - dd("deleting a loc conf -- RuleSet is: \"%p\"", conf->rules_set); + dd("deleting a loc conf -- RuleSet is: \"%p\"", mcf->rules_set); - old_pool = ngx_http_modsecurity_pcre_malloc_init(conf->pool); - msc_rules_cleanup(conf->rules_set); + old_pool = ngx_http_modsecurity_pcre_malloc_init(mcf->pool); + msc_rules_cleanup(mcf->rules_set); ngx_http_modsecurity_pcre_malloc_done(old_pool); - - conf->rules_set = NULL; } From 6b7e59e33442231724d901cf4a6668e1f0e15e0f Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Fri, 30 Nov 2018 18:03:49 +0300 Subject: [PATCH 5/6] Use consistent naming of configuration structures No functional changes. --- src/ngx_http_modsecurity_body_filter.c | 6 +++--- src/ngx_http_modsecurity_header_filter.c | 10 +++++----- src/ngx_http_modsecurity_log.c | 10 +++++----- src/ngx_http_modsecurity_module.c | 14 +++++++------- src/ngx_http_modsecurity_pre_access.c | 10 +++++----- src/ngx_http_modsecurity_rewrite.c | 10 +++++----- 6 files changed, 30 insertions(+), 30 deletions(-) diff --git a/src/ngx_http_modsecurity_body_filter.c b/src/ngx_http_modsecurity_body_filter.c index fd286d0..6118a94 100644 --- a/src/ngx_http_modsecurity_body_filter.c +++ b/src/ngx_http_modsecurity_body_filter.c @@ -38,7 +38,7 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) ngx_chain_t *chain = in; ngx_http_modsecurity_ctx_t *ctx = NULL; #if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) - ngx_http_modsecurity_conf_t *loc_cf = NULL; + ngx_http_modsecurity_conf_t *mcf; ngx_list_part_t *part = &r->headers_out.headers.part; ngx_table_elt_t *data = part->elts; ngx_uint_t i = 0; @@ -57,8 +57,8 @@ ngx_http_modsecurity_body_filter(ngx_http_request_t *r, ngx_chain_t *in) } #if defined(MODSECURITY_SANITY_CHECKS) && (MODSECURITY_SANITY_CHECKS) - loc_cf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); - if (loc_cf != NULL && loc_cf->sanity_checks_enabled != NGX_CONF_UNSET) + mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); + if (mcf != NULL && mcf->sanity_checks_enabled != NGX_CONF_UNSET) { #if 0 dd("dumping stored ctx headers"); diff --git a/src/ngx_http_modsecurity_header_filter.c b/src/ngx_http_modsecurity_header_filter.c index 7728d82..3f9f748 100644 --- a/src/ngx_http_modsecurity_header_filter.c +++ b/src/ngx_http_modsecurity_header_filter.c @@ -103,17 +103,17 @@ ngx_http_modsecurity_header_out_t ngx_http_modsecurity_headers_out[] = { int ngx_http_modescurity_store_ctx_header(ngx_http_request_t *r, ngx_str_t *name, ngx_str_t *value) { - ngx_http_modsecurity_ctx_t *ctx = NULL; - ngx_http_modsecurity_header_t *hdr = NULL; - ngx_http_modsecurity_conf_t *loc_cf = NULL; + ngx_http_modsecurity_ctx_t *ctx; + ngx_http_modsecurity_conf_t *mcf; + ngx_http_modsecurity_header_t *hdr; ctx = ngx_http_get_module_ctx(r, ngx_http_modsecurity_module); if (ctx == NULL || ctx->sanity_headers_out == NULL) { return NGX_ERROR; } - loc_cf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); - if (loc_cf == NULL || loc_cf->sanity_checks_enabled == NGX_CONF_UNSET) + mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); + if (mcf == NULL || mcf->sanity_checks_enabled == NGX_CONF_UNSET) { return NGX_OK; } diff --git a/src/ngx_http_modsecurity_log.c b/src/ngx_http_modsecurity_log.c index f6454b5..5546596 100644 --- a/src/ngx_http_modsecurity_log.c +++ b/src/ngx_http_modsecurity_log.c @@ -37,14 +37,14 @@ ngx_http_modsecurity_log(void *log, const void* data) ngx_int_t ngx_http_modsecurity_log_handler(ngx_http_request_t *r) { - ngx_http_modsecurity_ctx_t *ctx = NULL; - ngx_http_modsecurity_conf_t *cf; - ngx_pool_t *old_pool; + ngx_pool_t *old_pool; + ngx_http_modsecurity_ctx_t *ctx; + ngx_http_modsecurity_conf_t *mcf; dd("catching a new _log_ phase handler"); - cf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); - if (cf == NULL || cf->enable != 1) + 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; diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 04c4e1d..819552e 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -237,7 +237,7 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) ngx_str_t s; ngx_pool_cleanup_t *cln; ngx_http_modsecurity_ctx_t *ctx; - ngx_http_modsecurity_conf_t *mlcf; + ngx_http_modsecurity_conf_t *mcf; ngx_http_modsecurity_main_conf_t *mmcf; ctx = ngx_pcalloc(r->pool, sizeof(ngx_http_modsecurity_ctx_t)); @@ -248,18 +248,18 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) } mmcf = ngx_http_get_module_main_conf(r, ngx_http_modsecurity_module); - mlcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); + mcf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); - dd("creating transaction with the following rules: '%p' -- ms: '%p'", mlcf->rules_set, mmcf->modsec); + dd("creating transaction with the following rules: '%p' -- ms: '%p'", mcf->rules_set, mmcf->modsec); - if (mlcf->transaction_id) { - if (ngx_http_complex_value(r, mlcf->transaction_id, &s) != NGX_OK) { + if (mcf->transaction_id) { + if (ngx_http_complex_value(r, mcf->transaction_id, &s) != NGX_OK) { return NGX_CONF_ERROR; } - ctx->modsec_transaction = msc_new_transaction_with_id(mmcf->modsec, mlcf->rules_set, (char *) s.data, r->connection->log); + ctx->modsec_transaction = msc_new_transaction_with_id(mmcf->modsec, mcf->rules_set, (char *) s.data, r->connection->log); } else { - ctx->modsec_transaction = msc_new_transaction(mmcf->modsec, mlcf->rules_set, r->connection->log); + ctx->modsec_transaction = msc_new_transaction(mmcf->modsec, mcf->rules_set, r->connection->log); } dd("transaction created"); diff --git a/src/ngx_http_modsecurity_pre_access.c b/src/ngx_http_modsecurity_pre_access.c index f863cf8..05d7140 100644 --- a/src/ngx_http_modsecurity_pre_access.c +++ b/src/ngx_http_modsecurity_pre_access.c @@ -44,14 +44,14 @@ ngx_int_t ngx_http_modsecurity_pre_access_handler(ngx_http_request_t *r) { #if 1 - ngx_http_modsecurity_ctx_t *ctx = NULL; - ngx_http_modsecurity_conf_t *cf; - ngx_pool_t *old_pool; + ngx_pool_t *old_pool; + ngx_http_modsecurity_ctx_t *ctx; + ngx_http_modsecurity_conf_t *mcf; dd("catching a new _preaccess_ phase handler"); - cf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); - if (cf == NULL || cf->enable != 1) + 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_DECLINED; diff --git a/src/ngx_http_modsecurity_rewrite.c b/src/ngx_http_modsecurity_rewrite.c index a0d9196..cd02438 100644 --- a/src/ngx_http_modsecurity_rewrite.c +++ b/src/ngx_http_modsecurity_rewrite.c @@ -23,12 +23,12 @@ ngx_int_t ngx_http_modsecurity_rewrite_handler(ngx_http_request_t *r) { - ngx_http_modsecurity_ctx_t *ctx = NULL; - ngx_http_modsecurity_conf_t *cf; - ngx_pool_t *old_pool; + ngx_pool_t *old_pool; + ngx_http_modsecurity_ctx_t *ctx; + ngx_http_modsecurity_conf_t *mcf; - cf = ngx_http_get_module_loc_conf(r, ngx_http_modsecurity_module); - if (cf == NULL || cf->enable != 1) { + 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_DECLINED; } From 2ae2a1011bf1450a85acc14d02c406025d93657d Mon Sep 17 00:00:00 2001 From: Andrei Belov Date: Fri, 30 Nov 2018 19:11:41 +0300 Subject: [PATCH 6/6] Include number of loaded rules in log message at start --- src/ngx_http_modsecurity_common.h | 3 ++ src/ngx_http_modsecurity_module.c | 86 ++++++++++++++++++++++--------- 2 files changed, 66 insertions(+), 23 deletions(-) diff --git a/src/ngx_http_modsecurity_common.h b/src/ngx_http_modsecurity_common.h index 8a2962a..52443fb 100644 --- a/src/ngx_http_modsecurity_common.h +++ b/src/ngx_http_modsecurity_common.h @@ -89,6 +89,9 @@ typedef struct { typedef struct { void *pool; ModSecurity *modsec; + ngx_uint_t rules_inline; + ngx_uint_t rules_file; + ngx_uint_t rules_remote; } ngx_http_modsecurity_main_conf_t; diff --git a/src/ngx_http_modsecurity_module.c b/src/ngx_http_modsecurity_module.c index 819552e..420b917 100644 --- a/src/ngx_http_modsecurity_module.c +++ b/src/ngx_http_modsecurity_module.c @@ -286,13 +286,19 @@ ngx_http_modsecurity_create_ctx(ngx_http_request_t *r) } -char *ngx_conf_set_rules(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) { - ngx_str_t *value = cf->args->elts; - int res; - const char *error = NULL; - char *rules = ngx_str_to_char(value[1], cf->pool); - ngx_pool_t *old_pool; - ngx_http_modsecurity_conf_t *mcf = conf; +char * +ngx_conf_set_rules(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) +{ + int res; + char *rules; + ngx_str_t *value; + const char *error; + ngx_pool_t *old_pool; + ngx_http_modsecurity_conf_t *mcf = conf; + ngx_http_modsecurity_main_conf_t *mmcf; + + value = cf->args->elts; + rules = ngx_str_to_char(value[1], cf->pool); if (rules == (char *)-1) { return NGX_CONF_ERROR; @@ -301,22 +307,32 @@ char *ngx_conf_set_rules(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) { old_pool = ngx_http_modsecurity_pcre_malloc_init(cf->pool); res = msc_rules_add(mcf->rules_set, rules, &error); ngx_http_modsecurity_pcre_malloc_done(old_pool); + if (res < 0) { dd("Failed to load the rules: '%s' - reason: '%s'", rules, error); return strdup(error); } + mmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_modsecurity_module); + mmcf->rules_inline += res; + return NGX_CONF_OK; } -char *ngx_conf_set_rules_file(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) { - ngx_str_t *value = cf->args->elts; - int res; - const char *error = NULL; - ngx_pool_t *old_pool; - ngx_http_modsecurity_conf_t *mcf = conf; - char *rules_set = ngx_str_to_char(value[1], cf->pool); +char * +ngx_conf_set_rules_file(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) +{ + int res; + char *rules_set; + ngx_str_t *value; + const char *error; + ngx_pool_t *old_pool; + ngx_http_modsecurity_conf_t *mcf = conf; + ngx_http_modsecurity_main_conf_t *mmcf; + + value = cf->args->elts; + rules_set = ngx_str_to_char(value[1], cf->pool); if (rules_set == (char *)-1) { return NGX_CONF_ERROR; @@ -325,27 +341,38 @@ char *ngx_conf_set_rules_file(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) { old_pool = ngx_http_modsecurity_pcre_malloc_init(cf->pool); res = msc_rules_add_file(mcf->rules_set, rules_set, &error); ngx_http_modsecurity_pcre_malloc_done(old_pool); + if (res < 0) { dd("Failed to load the rules from: '%s' - reason: '%s'", rules_set, error); return strdup(error); } + mmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_modsecurity_module); + mmcf->rules_file += res; + return NGX_CONF_OK; } -char *ngx_conf_set_rules_remote(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) { - ngx_str_t *value = cf->args->elts; - int res; - const char *error = NULL; - const char *rules_remote_key = ngx_str_to_char(value[1], cf->pool); - const char *rules_remote_server = ngx_str_to_char(value[2], cf->pool); - ngx_pool_t *old_pool; - ngx_http_modsecurity_conf_t *mcf = conf; +char * +ngx_conf_set_rules_remote(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) +{ + int res; + ngx_str_t *value; + const char *error; + const char *rules_remote_key, *rules_remote_server; + ngx_pool_t *old_pool; + ngx_http_modsecurity_conf_t *mcf = conf; + ngx_http_modsecurity_main_conf_t *mmcf; + + value = cf->args->elts; + rules_remote_key = ngx_str_to_char(value[1], cf->pool); + rules_remote_server = ngx_str_to_char(value[2], cf->pool); if (rules_remote_server == (char *)-1) { return NGX_CONF_ERROR; } + if (rules_remote_key == (char *)-1) { return NGX_CONF_ERROR; } @@ -353,11 +380,15 @@ char *ngx_conf_set_rules_remote(ngx_conf_t *cf, ngx_command_t *cmd, void *conf) old_pool = ngx_http_modsecurity_pcre_malloc_init(cf->pool); res = msc_rules_add_remote(mcf->rules_set, rules_remote_key, rules_remote_server, &error); ngx_http_modsecurity_pcre_malloc_done(old_pool); + if (res < 0) { dd("Failed to load the rules from: '%s' - reason: '%s'", rules_remote_server, error); return strdup(error); } + mmcf = ngx_http_conf_get_module_main_conf(cf, ngx_http_modsecurity_module); + mmcf->rules_remote += res; + return NGX_CONF_OK; } @@ -564,6 +595,9 @@ ngx_http_modsecurity_create_main_conf(ngx_conf_t *cf) * * conf->modsec = NULL; * conf->pool = NULL; + * conf->rules_inline = 0; + * conf->rules_file = 0; + * conf->rules_remote = 0; */ cln = ngx_pool_cleanup_add(cf->pool, 0); @@ -597,7 +631,13 @@ ngx_http_modsecurity_create_main_conf(ngx_conf_t *cf) static char * ngx_http_modsecurity_init_main_conf(ngx_conf_t *cf, void *conf) { - ngx_log_error(NGX_LOG_NOTICE, cf->log, 0, MODSECURITY_NGINX_WHOAMI); + ngx_http_modsecurity_main_conf_t *mmcf; + mmcf = (ngx_http_modsecurity_main_conf_t *) conf; + + ngx_log_error(NGX_LOG_NOTICE, cf->log, 0, + "%s (rules loaded inline/local/remote: %ui/%ui/%ui)", + MODSECURITY_NGINX_WHOAMI, mmcf->rules_inline, + mmcf->rules_file, mmcf->rules_remote); return NGX_CONF_OK; }