diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c index fc462304..ef2c42bf 100644 --- a/apache2/apache2_config.c +++ b/apache2/apache2_config.c @@ -27,7 +27,7 @@ void *create_directory_config(apr_pool_t *mp, char *path) { if (dcfg == NULL) return NULL; #ifdef DEBUG_CONF - fprintf(stderr, "Created directory config %p path %s\n", dcfg, path); + ap_log_perror(APLOG_MARK, APLOG_STARTUP|APLOG_NOERRNO, 0, mp, "Created directory config %pp path %s", dcfg, path); #endif dcfg->mp = mp; @@ -75,6 +75,7 @@ void *create_directory_config(apr_pool_t *mp, char *path) { /* These are only used during the configuration process. */ dcfg->tmp_chain_starter = NULL; dcfg->tmp_default_actionset = NULL; + dcfg->tmp_rule_placeholders = NULL; /* Misc */ dcfg->data_dir = NOT_SET_P; @@ -195,7 +196,7 @@ void *merge_directory_configs(apr_pool_t *mp, void *_parent, void *_child) { directory_config *merged = create_directory_config(mp, NULL); #ifdef DEBUG_CONF - fprintf(stderr, "Merge parent %p child %p RESULT %p\n", _parent, _child, merged); + ap_log_perror(APLOG_MARK, APLOG_STARTUP|APLOG_NOERRNO, 0, mp, "Merge parent %pp child %pp RESULT %pp", _parent, _child, merged); #endif if (merged == NULL) return NULL; @@ -580,11 +581,48 @@ static const char *add_rule(cmd_parms *cmd, directory_config *dcfg, const char * dcfg->upload_validates_files = 1; } + /* Create skip table if one does not already exist. */ + if (dcfg->tmp_rule_placeholders == NULL) { + dcfg->tmp_rule_placeholders = apr_table_make(cmd->pool, 10); + if (dcfg->tmp_rule_placeholders == NULL) return FATAL_ERROR; + } + + /* Keep track of any rule IDs we need to skip after */ + if (rule->actionset->skip_after != NOT_SET_P) { + char *tmp_id = apr_pstrdup(cmd->pool, rule->actionset->skip_after); + apr_table_setn(dcfg->tmp_rule_placeholders, tmp_id, tmp_id); + + #ifdef DEBUG_CONF + ap_log_perror(APLOG_MARK, APLOG_STARTUP|APLOG_NOERRNO, 0, cmd->pool, "Watching for skipafter target rule id=\"%s\".", tmp_id); + #endif + + } + /* Add rule to the recipe. */ if (msre_ruleset_rule_add(dcfg->ruleset, rule, rule->actionset->phase) < 0) { return "Internal Error: Failed to add rule to the ruleset."; } + /* Add an additional placeholder if this rule ID is on the list */ + if ((rule->actionset->id != NULL) && apr_table_get(dcfg->tmp_rule_placeholders, rule->actionset->id)) { + msre_rule *phrule = apr_palloc(rule->ruleset->mp, sizeof(msre_rule)); + + #ifdef DEBUG_CONF + ap_log_perror(APLOG_MARK, APLOG_STARTUP|APLOG_NOERRNO, 0, cmd->pool, "Adding placeholder for rule id=\"%s\".", rule->actionset->id); + #endif + + /* shallow copy of original rule with placeholder marked as target */ + memcpy(phrule, rule, sizeof(msre_rule)); + phrule->placeholder = RULE_PH_TARGET; + + /* Add placeholder. */ + if (msre_ruleset_rule_add(dcfg->ruleset, phrule, phrule->actionset->phase) < 0) { + return "Internal Error: Failed to add placeholder to the ruleset."; + } + + apr_table_unset(dcfg->tmp_rule_placeholders, rule->actionset->id); + } + return NULL; } @@ -1132,7 +1170,7 @@ static const char *cmd_rule_remove_by_msg(cmd_parms *cmd, void *_dcfg, const cha msre_ruleset_rule_remove_with_exception(dcfg->ruleset, re); #ifdef DEBUG_CONF - fprintf(stderr, "Added exception %p (%d %s) to dcfg %p.\n", re, re->type, re->param, dcfg); + ap_log_perror(APLOG_MARK, APLOG_STARTUP|APLOG_NOERRNO, 0, cmd->pool, "Added exception %pp (%d %s) to dcfg %pp.", re, re->type, re->param, dcfg); #endif return NULL; diff --git a/apache2/mod_security2.c b/apache2/mod_security2.c index 3ae85d8a..a82685e6 100644 --- a/apache2/mod_security2.c +++ b/apache2/mod_security2.c @@ -418,6 +418,10 @@ static int hook_post_config(apr_pool_t *mp, apr_pool_t *mp_log, apr_pool_t *mp_t void *init_flag = NULL; int first_time = 0; + /* ENH Figure out a way to validate config before we start + * - skipafter: need to make sure we found all of our targets + */ + /* Figure out if we are here for the first time */ apr_pool_userdata_get(&init_flag, "modsecurity-init-flag", s->process->pool); if (init_flag == NULL) { diff --git a/apache2/modsecurity.h b/apache2/modsecurity.h index 135ff7cd..91ceb486 100644 --- a/apache2/modsecurity.h +++ b/apache2/modsecurity.h @@ -423,6 +423,7 @@ struct directory_config { /* Used only in the configuration phase. */ msre_rule *tmp_chain_starter; msre_actionset *tmp_default_actionset; + apr_table_t *tmp_rule_placeholders; /* Misc */ const char *data_dir; diff --git a/apache2/re.c b/apache2/re.c index 908a2bdf..82a802f0 100644 --- a/apache2/re.c +++ b/apache2/re.c @@ -673,27 +673,34 @@ apr_status_t msre_ruleset_process_phase(msre_ruleset *ruleset, modsec_rec *msr) apr_time_t time1 = 0; #endif - // TODO: Still need to skip over placeholders - /* SKIP_RULES is used to skip all rules until we hit a placeholder * with the specified rule ID and then resume execution after that. */ if (mode == SKIP_RULES) { /* Go to the next rule if we have not yet hit the skip_after ID */ - // TODO: must be a placeholder as well - if ((rule->actionset->id == NULL) || (strcmp(skip_after, rule->actionset->id) != 0)) { + if ((rule->placeholder != RULE_PH_NONE) && ((rule->actionset->id == NULL) || (strcmp(skip_after, rule->actionset->id) != 0))) { if (msr->txcfg->debuglog_level >= 9) { - msr_log(msr, 9, "Skipping rule id=\"%s\" while looking for id=\"%s\"", (rule->actionset->id ? rule->actionset->id : "(none)"), skip_after); + msr_log(msr, 9, "Skipping rule id=\"%s\": Skipping until after id=\"%s\"", (rule->actionset->id ? rule->actionset->id : "(none)"), skip_after); + } continue; } - if (msr->txcfg->debuglog_level >= 4) { - msr_log(msr, 4, "Continuing execution after rule id=\"%s\"", skip_after); + if (msr->txcfg->debuglog_level >= 9) { + msr_log(msr, 9, "Found rule id=\"%s\"%s.", skip_after, (rule->placeholder ? " placeholder" : "")); } - skip_after = NULL; - mode = NEXT_RULE; /* Go to the rule *after* this one to continue execution. */ + if (msr->txcfg->debuglog_level >= 4) { + msr_log(msr, 4, "Continuing execution after rule id=\"%s\".", skip_after); + } + + skip_after = NULL; + mode = NEXT_RULE; + continue; + } + + /* Skip any rule marked as a placeholder */ + if (rule->placeholder != RULE_PH_NONE) { continue; } diff --git a/apache2/re.h b/apache2/re.h index 9d9dc1f1..0be1a760 100644 --- a/apache2/re.h +++ b/apache2/re.h @@ -109,6 +109,9 @@ int DSOLOCAL msre_ruleset_phase_rule_remove_with_exception(msre_ruleset *ruleset #define RULE_NO_MATCH 0 #define RULE_MATCH 1 +#define RULE_PH_NONE 0 /* Not a placeholder */ +#define RULE_PH_TARGET 1 /* Placeholder for skipafter targets */ + struct msre_rule { apr_array_header_t *targets; const char *op_name; @@ -120,6 +123,7 @@ struct msre_rule { const char *p1; const char *filename; int line_num; + int placeholder; msre_ruleset *ruleset; msre_rule *chain_starter; diff --git a/doc/modsecurity2-apache-reference.xml b/doc/modsecurity2-apache-reference.xml index f90d4a99..95c932f8 100644 --- a/doc/modsecurity2-apache-reference.xml +++ b/doc/modsecurity2-apache-reference.xml @@ -1939,7 +1939,7 @@ SecRule REQUEST_HEADERS:Host "!^$" "deny,phase:1"skip - action. + and skipAfter actions.
Phase Request Headers @@ -2333,8 +2333,8 @@ SecRule TX:MYMATCH "@eq ARGS:param" deny intended to be used to check the build number prior to using a feature that is available only in a certain build. Example: - SecRule MODSEC_BUILD "!@ge 02050102" skip:1 -SecRule ARGS "@pm some key words" deny,status:500 + SecRule MODSEC_BUILD "!@ge 02050102" skipAfter:12345 +SecRule ARGS "@pm some key words" id:12345,deny,status:500
@@ -3106,10 +3106,10 @@ SecRule REQUEST_HEADERS:Transfer-Encoding "!^$" SecDefaultAction log,deny,status:403,phase:2 SecRule REQUEST_HEADERS:Content-Type ^text/xml$ \ phase:1,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML -SecRule REQBODY_PROCESSOR "!^XML$" skip:2 +SecRule REQBODY_PROCESSOR "!^XML$" skipAfter:12345 SecRule XML:/employees/employee/name/text() Fred SecRule XML:/xq:employees/employee/name/text() Fred \ - xmlns:xq=http://www.example.com/employees + id:12345,xmlns:xq=http://www.example.com/employees The first XPath expression does not use namespaces. It would match against payload such as this one: @@ -3576,8 +3576,8 @@ SecRule REQUEST_HEADER:Content-Length ^$ first portion of the chained rule will only be triggered if all of the variable checks return positive hits. If one aspect of the chained rule is negative, then the entire rule chain is negative. Also note that - disruptive actions, execution phases, metadata actions (id, rev, msg) - and skip actions can only be specified on by the chain starter + disruptive actions, execution phases, metadata actions (id, rev, msg), + skip and skipAfter actions can only be specified on by the chain starter rule.
@@ -4443,6 +4443,34 @@ SecRule &REQUEST_HEADERS:Accept "@eq 0" \ skip. +
+ <literal>skipAfter</literal> + + Description: Skips rules (or chains) on + successful match resuming rule execution after the specified rule id is + found. + + Action Group: Non-Disruptive + + Example: + + SecRule REQUEST_URI "^/$" "chain,skipAfter:960015" +SecRule REMOTE_ADDR "^127\.0\.0\.1$" "chain" +SecRule REQUEST_HEADERS:User-Agent "^Apache \(internal dummy connection\)$" "t:none" +SecRule &REQUEST_HEADERS:Host "@eq 0" \ + "deny,log,status:400,id:960008,severity:4,msg:'Request Missing a Host Header'" +SecRule &REQUEST_HEADERS:Accept "@eq 0" \ + "log,deny,log,status:400,id:960015,msg:'Request Missing an Accept Header'" + + Note + + SkipAfter only applies to the current processing phase and not + necessarily the order in which the rules appear in the configuration + file. If you group rules by processing phases, then skip should work as + expected. This action can not be used to skip rules within one chain. + Accepts a single paramater denoting the last rule ID to skip. +
+
<literal>status</literal> @@ -4837,8 +4865,8 @@ SecRule REQUEST_HEADERS:Ip-Address "!@streq %{TX.1}"SecDefaultAction log,deny,status:403,phase:2 SecRule REQUEST_HEADERS:Content-Type ^text/xml$ \ phase:1,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML -SecRule REQBODY_PROCESSOR "!^XML$" nolog,pass,skip:1 -SecRule XML "@validateDTD /path/to/apache2/conf/xml.dtd" +SecRule REQBODY_PROCESSOR "!^XML$" nolog,pass,skipAfter:12345 +SecRule XML "@validateDTD /path/to/apache2/conf/xml.dtd,id:12345"
@@ -4852,8 +4880,8 @@ SecRule XML "@validateDTD /path/to/apache2/conf/xml.dtd"

SecDefaultAction log,deny,status:403,phase:2 SecRule REQUEST_HEADERS:Content-Type ^text/xml$ \ phase:1,t:lowercase,nolog,pass,ctl:requestBodyProcessor=XML -SecRule REQBODY_PROCESSOR "!^XML$" nolog,pass,skip:1 -SecRule XML "@validateSchema /path/to/apache2/conf/xml.xsd" +SecRule REQBODY_PROCESSOR "!^XML$" nolog,pass,skipAfter:12345 +SecRule XML "@validateSchema /path/to/apache2/conf/xml.xsd,id:12345" This operator requires request body to be processed as XML.