From 9695f2b8160bc5d9a662f5670ec6caccc94526fa Mon Sep 17 00:00:00 2001 From: brectanus Date: Fri, 3 Aug 2007 20:25:30 +0000 Subject: [PATCH] Improvements in transformation cache (add options, document). Update CHANGES. --- CHANGES | 4 +- apache2/apache2_config.c | 60 ++++++++++++++++++++-- apache2/modsecurity.h | 2 + apache2/re.c | 71 ++++++++++++++++----------- doc/modsecurity2-apache-reference.xml | 55 ++++++++++++++++++++- 5 files changed, 158 insertions(+), 34 deletions(-) diff --git a/CHANGES b/CHANGES index 1bed5712..301fa4fe 100644 --- a/CHANGES +++ b/CHANGES @@ -3,7 +3,7 @@ * Cleaned up some documentation. - * Performance improvements in caching transformations. + * Performance improvements and greater control over caching transformations. * Stricter validation for @validateUtf8Encoding. @@ -17,6 +17,8 @@ 27 July 2007 - 2.1.2 -------------------- + * Cleaned up and clarified some documentation. + * Update included core rules to latest version (1.4.3). * Enhanced ability to alert/audit failed requests. diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index b669e5fd..cd16892e 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -95,6 +95,8 @@ void *create_directory_config(apr_pool_t *mp, char *path) { /* Cache */ dcfg->cache_trans = NOT_SET; + dcfg->cache_trans_min = NOT_SET; + dcfg->cache_trans_max = NOT_SET; return dcfg; } @@ -396,6 +398,10 @@ void *merge_directory_configs(apr_pool_t *mp, void *_parent, void *_child) { /* Cache */ merged->cache_trans = (child->cache_trans == NOT_SET ? parent->cache_trans : child->cache_trans); + merged->cache_trans_min = (child->cache_trans_min == (apr_size_t)NOT_SET + ? parent->cache_trans_min : child->cache_trans_min); + merged->cache_trans_max = (child->cache_trans_max == (apr_size_t)NOT_SET + ? parent->cache_trans_max : child->cache_trans_max); return merged; } @@ -471,7 +477,9 @@ void init_directory_config(directory_config *dcfg) { if (dcfg->geo == NOT_SET_P) dcfg->geo = NULL; /* Cache */ - if (dcfg->cache_trans == NOT_SET_P) dcfg->cache_trans = MODSEC_CACHE_ENABLED; + if (dcfg->cache_trans == NOT_SET) dcfg->cache_trans = MODSEC_CACHE_ENABLED; + if (dcfg->cache_trans_min == (apr_size_t)NOT_SET) dcfg->cache_trans_min = 15; + if (dcfg->cache_trans_max == (apr_size_t)NOT_SET) dcfg->cache_trans_max = 0; } /** @@ -1247,9 +1255,8 @@ static const char *cmd_geo_lookups_db(cmd_parms *cmd, void *_dcfg, static const char *cmd_cache_transformations(cmd_parms *cmd, void *_dcfg, const char *p1, const char *p2) { directory_config *dcfg = (directory_config *)_dcfg; - if (dcfg == NULL) return NULL; - // TODO: p2 is options + if (dcfg == NULL) return NULL; if (strcasecmp(p1, "on") == 0) dcfg->cache_trans = MODSEC_CACHE_ENABLED; @@ -1258,6 +1265,53 @@ static const char *cmd_cache_transformations(cmd_parms *cmd, void *_dcfg, const else return apr_psprintf(cmd->pool, "ModSecurity: Invalid value for SecCacheTransformations: %s", p1); + /* Process options */ + if (p2 != NULL) { + apr_table_t *vartable = apr_table_make(cmd->pool, 10); + apr_status_t rc; + char *error_msg = NULL; + const char *charval = NULL; + apr_int64_t intval = 0; + + if (vartable == NULL) { + return apr_psprintf(cmd->pool, "ModSecurity: Unable to process options for SecCacheTransformations"); + } + rc = msre_parse_generic(cmd->pool, p2, vartable, &error_msg); + if (rc < 0) { + return apr_psprintf(cmd->pool, "ModSecurity: Unable to parse options for SecCacheTransformations: %s", error_msg); + } + + /* minval */ + charval = apr_table_get(vartable, "minlen"); + if (charval != NULL) { + intval = apr_atoi64(charval); + if (intval < 0) { + return apr_psprintf(cmd->pool, "ModSecurity: SecCacheTransformations minlen must be positive: %s", charval); + } + if (intval >= (apr_size_t)NOT_SET) { + return apr_psprintf(cmd->pool, "ModSecurity: SecCacheTransformations minlen must be less than: %u", (apr_size_t)NOT_SET); + } + dcfg->cache_trans_min = (apr_size_t)intval; + } + + /* maxval */ + charval = apr_table_get(vartable, "maxlen"); + if (charval != NULL) { + intval = apr_atoi64(charval); + if (intval < 0) { + return apr_psprintf(cmd->pool, "ModSecurity: SecCacheTransformations maxlen must be positive: %s", charval); + } + if (intval >= (apr_size_t)NOT_SET) { + return apr_psprintf(cmd->pool, "ModSecurity: SecCacheTransformations maxlen must be less than: %u", (apr_size_t)NOT_SET); + } + if ((intval != 0) && (intval < dcfg->cache_trans_min)) { + return apr_psprintf(cmd->pool, "ModSecurity: SecCacheTransformations maxlen must not be less than minlen: %u < %u", (apr_size_t)intval, dcfg->cache_trans_min); + } + dcfg->cache_trans_max = (apr_size_t)intval; + + } + } + return NULL; } diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h index af833c91..f9d157c1 100644 --- a/apache2/modsecurity.h +++ b/apache2/modsecurity.h @@ -420,6 +420,8 @@ struct directory_config { /* Cache */ int cache_trans; + apr_size_t cache_trans_min; + apr_size_t cache_trans_max; }; struct error_message { diff --git a/apache2/re.c b/apache2/re.c index f2428b9d..b97853f7 100644 --- a/apache2/re.c +++ b/apache2/re.c @@ -1363,30 +1363,45 @@ apr_status_t msre_rule_process(msre_rule *rule, modsec_rec *msr) { /* Is this var cacheable? */ if (msr->txcfg->cache_trans != MODSEC_CACHE_DISABLED) { - msr_log(msr, 9, "CACHE: Enabled"); - if (var->metadata->is_cacheable == VAR_CACHE) { - usecache = 1; + usecache = 1; - /* Fetch cache table for this target */ - carr = (apr_table_t **)apr_hash_get(msr->tcache, var->name, APR_HASH_KEY_STRING); - if (carr != NULL) { - cachetab = carr[msr->phase]; + /* check the cache options */ + if (var->value_len < msr->txcfg->cache_trans_min) { + msr_log(msr, 9, "CACHE: Disabled - %s value length=%d, smaller than minlen=%d", var->name, var->value_len, msr->txcfg->cache_trans_min); + usecache = 0; + } + if ((msr->txcfg->cache_trans_max != 0) && (var->value_len > msr->txcfg->cache_trans_max)) { + msr_log(msr, 9, "CACHE: Disabled - %s value length=%d, larger than maxlen=%d", var->name, var->value_len, msr->txcfg->cache_trans_max); + usecache = 0; + } + + /* if cache is still enabled, check the VAR for cacheablity */ + if (usecache) { + if (var->metadata->is_cacheable == VAR_CACHE) { + msr_log(msr, 9, "CACHE: Enabled"); + + /* Fetch cache table for this target */ + carr = (apr_table_t **)apr_hash_get(msr->tcache, var->name, APR_HASH_KEY_STRING); + if (carr != NULL) { + cachetab = carr[msr->phase]; + } + else { + /* Create an array of cache tables (one table per phase) */ + carr = (apr_table_t **)apr_pcalloc(msr->mp, (sizeof(apr_table_t *) * (PHASE_LAST + 1))); + if (carr == NULL) return -1; + memset(carr, 0, (sizeof(apr_table_t *) * (PHASE_LAST + 1))); + apr_hash_set(msr->tcache, var->name, APR_HASH_KEY_STRING, carr); + } + + /* Create an empty cache table if this is the first time */ + if (cachetab == NULL) { + cachetab = carr[msr->phase] = apr_table_make(msr->mp, 5); + } } else { - /* Create an array of cache tables (one table per phase) */ - carr = (apr_table_t **)apr_pcalloc(msr->mp, (sizeof(apr_table_t *) * (PHASE_LAST + 1))); - if (carr == NULL) return -1; - memset(carr, 0, (sizeof(apr_table_t *) * (PHASE_LAST + 1))); - apr_hash_set(msr->tcache, var->name, APR_HASH_KEY_STRING, carr); + usecache = 0; + msr_log(msr, 9, "CACHE: %s transformations are not cacheable", var->name); } - - /* Create an empty cache table if this is the first time */ - if (cachetab == NULL) { - cachetab = carr[msr->phase] = apr_table_make(msr->mp, 5); - } - } - else { - msr_log(msr, 9, "CACHE: %s transformations are not cacheable", var->name); } } else { @@ -1579,9 +1594,9 @@ apr_status_t msre_rule_process(msre_rule *rule, modsec_rec *msr) { crec->changed = changed; crec->num = k + 1; crec->path = tfnspath; - crec->val = changed ? apr_pmemdup(msr->mp, rval, rval_length) : NULL; - crec->val_len = changed ? rval_length : -1; - msr_log(msr, 9, "CACHE: Caching %s=\"%.*s\"", tfnskey, crec->val_len, crec->val); + crec->val = changed ? apr_pmemdup(msr->mp, var->value, var->value_len) : NULL; + crec->val_len = changed ? var->value_len : 0; + msr_log(msr, 9, "CACHE: Caching %s=\"%.*s\"", tfnskey, var->value_len, var->value); apr_table_setn(cachetab, tfnskey, (void *)crec); } @@ -1619,8 +1634,6 @@ apr_status_t msre_rule_process(msre_rule *rule, modsec_rec *msr) { } } - - msr_log(msr, 9, "CACHE: size=%u", apr_hash_count(msr->tcache)); #ifdef CACHE_DEBUG if (msr->txcfg->debuglog_level >= 9) { apr_hash_index_t *hi; @@ -1629,11 +1642,10 @@ apr_status_t msre_rule_process(msre_rule *rule, modsec_rec *msr) { const apr_array_header_t *ctarr; const apr_table_entry_t *ctelts; msre_cache_rec *rec; - int hn = 0; + int cn = 0; int ti, ri; for (hi = apr_hash_first(msr->mp, msr->tcache); hi; hi = apr_hash_next(hi)) { - hn++; apr_hash_this(hi, NULL, NULL, &dummy); tab = (apr_table_t **)dummy; if (tab == NULL) continue; @@ -1643,12 +1655,13 @@ apr_status_t msre_rule_process(msre_rule *rule, modsec_rec *msr) { ctarr = apr_table_elts(tab[ti]); ctelts = (const apr_table_entry_t*)ctarr->elts; for (ri = 0; ri < ctarr->nelts; ri++) { + cn++; rec = (msre_cache_rec *)ctelts[ri].val; if (rec->changed) { - msr_log(msr, 9, "CACHE: %5d) phase=%d hits=%d %x;%s=\"%s\"", hn, msr->phase, rec->hits, rec->num, rec->path, log_escape_nq_ex(mptmp, rec->val, rec->val_len)); + msr_log(msr, 9, "CACHE: %5d) phase=%d hits=%d %x;%s=\"%s\"", cn, msr->phase, rec->hits, rec->num, rec->path, log_escape_nq_ex(mptmp, rec->val, rec->val_len)); } else { - msr_log(msr, 9, "CACHE: %5d) phase=%d hits=%d %x;%s=", hn, msr->phase, rec->hits, rec->num, rec->path); + msr_log(msr, 9, "CACHE: %5d) phase=%d hits=%d %x;%s=", cn, msr->phase, rec->hits, rec->num, rec->path); } } } diff --git a/doc/modsecurity2-apache-reference.xml b/doc/modsecurity2-apache-reference.xml index a9a17639..68af551e 100644 --- a/doc/modsecurity2-apache-reference.xml +++ b/doc/modsecurity2-apache-reference.xml @@ -783,6 +783,59 @@ SecAuditLogStorageDir logs/audit +
+ <literal>SecCacheTransformations</literal> + + Description: Controls caching of + transformations. + + Syntax: SecCacheTransformations On|Off + [options] + + Example Usage: SecCacheTransformations On + "minlen:64,maxlen:0" + + Processing Phase: N/A + + Scope: Any + + Dependencies/Notes: N/A + + First parameter: + + + + On - cache transformations + (per transaction, per phase) allowing identical transformations to + be performed only once. (default) + + + + Off - do not cache any + transformations, forcing all transformations to be performed for + each rule executed. + + + + The following options are allowed (comma seperated): + + + + minlen:N - do not cache the + transformation if the value's length is less than N bytes. (default: + 15) + + + + maxlen:N - do not cache the + transformation if the value's length is more than N bytes. A zero + value is interpreted as "unlimited". (default: 0) + + +
+
<literal>SecChrootDir</literal> @@ -4666,4 +4719,4 @@ SecAction "pass,setvar:'tx.allowed_methods=get,post,head'" SecRule REQUEST_METHOD "!@within %{tx.allowed_methods}" t:lowercase,deny,status:403
- + \ No newline at end of file