diff --git a/CHANGES b/CHANGES
index 7ce816db..81c2e062 100644
--- a/CHANGES
+++ b/CHANGES
@@ -1,6 +1,8 @@
23 Jan 2008 - 2.5.0-rc2
-----------------------
+ * Implemented "block" action.
+
* No longer log the query portion of the URI in the error log as
it may contain sensitive data.
diff --git a/apache2/apache2_config.c b/apache2/apache2_config.c
index 02551592..9243219b 100644
--- a/apache2/apache2_config.c
+++ b/apache2/apache2_config.c
@@ -609,6 +609,10 @@ static const char *add_rule(cmd_parms *cmd, directory_config *dcfg, int type,
rule->actionset = msre_actionset_merge(modsecurity->msre, dcfg->tmp_default_actionset,
rule->actionset, 1);
+ /* Keep track of the parent action for "block" */
+ rule->actionset->parent_intercept_action_rec = dcfg->tmp_default_actionset->intercept_action_rec;
+ rule->actionset->parent_intercept_action = dcfg->tmp_default_actionset->intercept_action;
+
/* Must NOT specify a disruptive action in logging phase. */
if ((rule->actionset != NULL)
&& (rule->actionset->phase == PHASE_LOGGING)
@@ -761,7 +765,7 @@ static const char *update_rule_action(cmd_parms *cmd, directory_config *dcfg,
#ifdef DEBUG_CONF
ap_log_perror(APLOG_MARK, APLOG_STARTUP|APLOG_NOERRNO, 0, cmd->pool,
- "Looking to update rule id=\"%s\" with \"%s\".", p1, p2);
+ "Update rule id=\"%s\" with action \"%s\".", p1, p2);
#endif
/* Fetch the rule */
@@ -769,7 +773,7 @@ static const char *update_rule_action(cmd_parms *cmd, directory_config *dcfg,
if (rule == NULL) {
#ifdef DEBUG_CONF
ap_log_perror(APLOG_MARK, APLOG_STARTUP|APLOG_NOERRNO, 0, cmd->pool,
- "Failed to update rule id=\"%s\" with \"%s\": Rule not found.", p1, p2);
+ "Update rule id=\"%s\" with action \"%s\" failed: Rule not found.", p1, p2);
#endif
return NULL;
}
@@ -799,7 +803,7 @@ static const char *update_rule_action(cmd_parms *cmd, directory_config *dcfg,
{
char *actions = msre_actionset_generate_action_string(ruleset->mp, rule->actionset);
ap_log_perror(APLOG_MARK, APLOG_STARTUP|APLOG_NOERRNO, 0, cmd->pool,
- "Updating rule %pp id=\"%s\" action: \"%s\"",
+ "Update rule %pp id=\"%s\" old action: \"%s\"",
rule,
(rule->actionset->id == NOT_SET_P ? "(none)" : rule->actionset->id),
actions);
@@ -819,7 +823,7 @@ static const char *update_rule_action(cmd_parms *cmd, directory_config *dcfg,
{
char *actions = msre_actionset_generate_action_string(ruleset->mp, rule->actionset);
ap_log_perror(APLOG_MARK, APLOG_STARTUP|APLOG_NOERRNO, 0, cmd->pool,
- "Updated rule %pp id=\"%s\" action: \"%s\"",
+ "Update rule %pp id=\"%s\" new action: \"%s\"",
rule,
(rule->actionset->id == NOT_SET_P ? "(none)" : rule->actionset->id),
actions);
diff --git a/apache2/re.c b/apache2/re.c
index 8872e71c..470acd7c 100644
--- a/apache2/re.c
+++ b/apache2/re.c
@@ -87,6 +87,35 @@ char *msre_actionset_generate_action_string(apr_pool_t *pool, const msre_actions
return actions;
}
+/**
+ * Add an action to an actionset.
+ */
+static void msre_actionset_action_add(msre_actionset *actionset, msre_action *action)
+{
+ msre_action *add_action = action;
+
+ /**
+ * The "block" action is just a placeholder for the parent action.
+ */
+ if ((actionset->parent_intercept_action_rec != NULL) && (actionset->parent_intercept_action_rec != NOT_SET_P) && (strcmp("block", action->metadata->name) == 0) && (strcmp("block", action->metadata->name) == 0)) {
+ /* revert back to parent */
+ actionset->intercept_action = actionset->parent_intercept_action;
+ add_action = actionset->parent_intercept_action_rec;
+ }
+
+ if (add_action->metadata->cardinality_group != ACTION_CGROUP_NONE) {
+ msre_actionset_cardinality_fixup(actionset, add_action);
+ }
+
+ if (add_action->metadata->cardinality == ACTION_CARDINALITY_ONE) {
+ /* One action per actionlist. */
+ apr_table_setn(actionset->actions, add_action->metadata->name, (void *)add_action);
+ } else {
+ /* Multiple actions per actionlist. */
+ apr_table_addn(actionset->actions, add_action->metadata->name, (void *)add_action);
+ }
+}
+
/**
* Creates msre_var instances (rule variables) out of the
* given text string and places them into the supplied table.
@@ -159,17 +188,7 @@ apr_status_t msre_parse_actions(msre_engine *engine, msre_actionset *actionset,
action->metadata->init(engine, actionset, action);
}
- if (action->metadata->cardinality_group != ACTION_CGROUP_NONE) {
- msre_actionset_cardinality_fixup(actionset, action);
- }
-
- if (action->metadata->cardinality == ACTION_CARDINALITY_ONE) {
- /* One action per actionlist. */
- apr_table_setn(actionset->actions, action->metadata->name, (void *)action);
- } else {
- /* Multiple actions per actionlist. */
- apr_table_addn(actionset->actions, action->metadata->name, (void *)action);
- }
+ msre_actionset_action_add(actionset, action);
count++;
}
@@ -503,6 +522,9 @@ msre_actionset *msre_actionset_create(msre_engine *engine, const char *text,
actionset->skip_after = NOT_SET_P;
/* Disruptive */
+ actionset->parent_intercept_action_rec = NOT_SET_P;
+ actionset->intercept_action_rec = NOT_SET_P;
+ actionset->parent_intercept_action = NOT_SET;
actionset->intercept_action = NOT_SET;
actionset->intercept_uri = NOT_SET_P;
actionset->intercept_status = NOT_SET;
@@ -581,6 +603,7 @@ msre_actionset *msre_actionset_merge(msre_engine *engine, msre_actionset *parent
/* Disruptive */
if (child->intercept_action != NOT_SET) {
+ merged->intercept_action_rec = child->intercept_action_rec;
merged->intercept_action = child->intercept_action;
merged->intercept_uri = child->intercept_uri;
}
@@ -598,17 +621,7 @@ msre_actionset *msre_actionset_merge(msre_engine *engine, msre_actionset *parent
tarr = apr_table_elts(child->actions);
telts = (const apr_table_entry_t*)tarr->elts;
for (i = 0; i < tarr->nelts; i++) {
- msre_action *action = (msre_action *)telts[i].val;
-
- if (action->metadata->cardinality_group != ACTION_CGROUP_NONE) {
- msre_actionset_cardinality_fixup(merged, action);
- }
-
- if (action->metadata->cardinality == ACTION_CARDINALITY_ONE) {
- apr_table_setn(merged->actions, action->metadata->name, (void *)action);
- } else {
- apr_table_addn(merged->actions, action->metadata->name, (void *)action);
- }
+ msre_actionset_action_add(merged, (msre_action *)telts[i].val);
}
return merged;
@@ -643,6 +656,9 @@ void msre_actionset_set_defaults(msre_actionset *actionset) {
if (actionset->skip_after == NOT_SET_P) actionset->skip_after = NULL;
/* Disruptive */
+ if (actionset->parent_intercept_action_rec == NOT_SET_P) actionset->parent_intercept_action_rec = NULL;
+ if (actionset->intercept_action_rec == NOT_SET_P) actionset->intercept_action_rec = NULL;
+ if (actionset->parent_intercept_action == NOT_SET) actionset->parent_intercept_action = ACTION_NONE;
if (actionset->intercept_action == NOT_SET) actionset->intercept_action = ACTION_NONE;
if (actionset->intercept_uri == NOT_SET_P) actionset->intercept_uri = NULL;
if (actionset->intercept_status == NOT_SET) actionset->intercept_status = 403;
diff --git a/apache2/re.h b/apache2/re.h
index 62a20f4b..16bfea8f 100644
--- a/apache2/re.h
+++ b/apache2/re.h
@@ -272,9 +272,15 @@ struct msre_actionset {
int intercept_status;
int intercept_pause;
+ /* "block" needs parent action to reset it */
+ msre_action *parent_intercept_action_rec;
+ msre_action *intercept_action_rec;
+ int parent_intercept_action;
+
/* Other */
int log;
int auditlog;
+ int block;
};
char DSOLOCAL *msre_actionset_generate_action_string(apr_pool_t *pool, const msre_actionset *actionset);
diff --git a/apache2/re_actions.c b/apache2/re_actions.c
index 180ec60a..dcc259b4 100644
--- a/apache2/re_actions.c
+++ b/apache2/re_actions.c
@@ -378,11 +378,21 @@ static apr_status_t msre_action_noauditlog_init(msre_engine *engine, msre_action
return 1;
}
+/* block */
+static apr_status_t msre_action_block_init(msre_engine *engine, msre_actionset *actionset,
+ msre_action *action)
+{
+ /* Right now we just set a flag and inherit the real disruptive action */
+ actionset->block = 1;
+ return 1;
+}
+
/* deny */
static apr_status_t msre_action_deny_init(msre_engine *engine, msre_actionset *actionset,
msre_action *action)
{
actionset->intercept_action = ACTION_DENY;
+ actionset->intercept_action_rec = action;
return 1;
}
@@ -404,6 +414,7 @@ static apr_status_t msre_action_drop_init(msre_engine *engine, msre_actionset *a
msre_action *action)
{
actionset->intercept_action = ACTION_DROP;
+ actionset->intercept_action_rec = action;
return 1;
}
@@ -432,6 +443,7 @@ static apr_status_t msre_action_redirect_init(msre_engine *engine, msre_actionse
{
actionset->intercept_action = ACTION_REDIRECT;
actionset->intercept_uri = action->param;
+ actionset->intercept_action_rec = action;
return 1;
}
@@ -463,6 +475,7 @@ static apr_status_t msre_action_proxy_init(msre_engine *engine, msre_actionset *
{
actionset->intercept_action = ACTION_PROXY;
actionset->intercept_uri = action->param;
+ actionset->intercept_action_rec = action;
return 1;
}
@@ -488,6 +501,7 @@ static apr_status_t msre_action_pass_init(msre_engine *engine, msre_actionset *a
msre_action *action)
{
actionset->intercept_action = ACTION_NONE;
+ actionset->intercept_action_rec = action;
return 1;
}
@@ -526,6 +540,7 @@ static apr_status_t msre_action_allow_init(msre_engine *engine, msre_actionset *
msre_action *action)
{
actionset->intercept_action = ACTION_ALLOW;
+ actionset->intercept_action_rec = action;
if (action->param != NULL) {
if (strcasecmp(action->param, "phase") == 0) {
@@ -1744,6 +1759,19 @@ void msre_engine_register_default_actions(msre_engine *engine) {
NULL
);
+ /* deny */
+ msre_engine_action_register(engine,
+ "block",
+ ACTION_DISRUPTIVE,
+ 0, 0,
+ NO_PLUS_MINUS,
+ ACTION_CARDINALITY_ONE,
+ ACTION_CGROUP_DISRUPTIVE,
+ NULL,
+ msre_action_block_init,
+ NULL
+ );
+
/* deny */
msre_engine_action_register(engine,
"deny",
diff --git a/doc/modsecurity2-apache-reference.xml b/doc/modsecurity2-apache-reference.xml
index a58a0578..52c2be0d 100644
--- a/doc/modsecurity2-apache-reference.xml
+++ b/doc/modsecurity2-apache-reference.xml
@@ -3,7 +3,7 @@
ModSecurity Reference Manual
- Version 2.5.0-rc2/ (January 21, 2008)
+ Version 2.5.0-rc2/ (January 23, 2008)
2004-2008
@@ -3792,6 +3792,59 @@ SecAction phase:3,allow
specified.
+
+ <literal>block</literal>
+
+ Description: Performs the default disruptive
+ action.
+
+ Action Group: Disruptive
+
+ It is intended to be used by ruleset writers to signify that the
+ rule was intended to block and leaves the "how" up to the administrator.
+ This action is currently a placeholder which will just be replaced by
+ the action from the last SecDefaultAction in the same
+ context. Using the block action with the
+ SecRuleUpdateActionById directive allows a rule to be
+ reverted back to the previous SecDefaultAction
+ disruptive action.
+
+ In future versions of ModSecurity, more control and functionality
+ will be added to define "how" to block.
+
+ Examples:
+
+ In the following example, the second rule will "deny" because of
+ the SecDefaultAction disruptive action. The intent being that the
+ administrator could easily change this to another disruptive action
+ without editing the actual rules.
+
+ ### Administrator defines "how" to block (deny,status:403)...
+SecDefaultAction phase:2,deny,status:403,log,auditlog
+
+### Included from a rulest...
+# Intent is to warn for this User Agent
+SecRule REQUEST_HEADERS:User-Agent "perl" "phase:2,pass,msg:'Perl based user agent identified'"
+# Intent is to block for this User Agent, "how" described in SecDefaultAction
+SecRule REQUEST_HEADERS:User-Agent "nikto" "phase:2,block,msg:'Nikto Scanners Identified'"
+
+ In the following example, The rule is reverted back to the
+ pass action defined in the SecDefaultAction directive
+ by using the SecRuleUpdateActionById directive in
+ conjuction with the block action. This allows an
+ administrator to override an action in a 3rd party rule without
+ modifying the rule itself.
+
+ ### Administrator defines "how" to block (deny,status:403)...
+SecDefaultAction phase:2,pass,log,auditlog
+
+### Included from a rulest...
+SecRule REQUEST_HEADERS:User-Agent "nikto" "id:1,phase:2,deny,msg:'Nikto Scanners Identified'"
+
+### Added by the administrator
+SecRuleUpdateActionById 1 "block"
+
+
<literal>capture</literal>