Implement "block" pseudo-action. See #441.

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.
 

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);

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_action_add(actionset, action);
-            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);
-        }
 
         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;
+        msre_actionset_action_add(merged, (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);
-        }
     }
 
     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;

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);

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",

doc/modsecurity2-apache-reference.xml

@@ -3,7 +3,7 @@
   <title>ModSecurity Reference Manual</title>
 
   <articleinfo>
-    <releaseinfo>Version 2.5.0-rc2/ (January 21, 2008)</releaseinfo>
+    <releaseinfo>Version 2.5.0-rc2/ (January 23, 2008)</releaseinfo>
 
     <copyright>
       <year>2004-2008</year>
@@ -3792,6 +3792,59 @@ SecAction phase:3,allow</programlisting>
       specified.</para>
     </section>
 
+    <section>
+      <title><literal>block</literal></title>
+
+      <para><emphasis>Description:</emphasis> Performs the default disruptive
+      action.</para>
+
+      <para><emphasis>Action Group:</emphasis> Disruptive</para>
+
+      <para>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 <literal>SecDefaultAction</literal> in the same
+      context. Using the <literal>block</literal> action with the
+      <literal>SecRuleUpdateActionById</literal> directive allows a rule to be
+      reverted back to the previous <literal>SecDefaultAction</literal>
+      disruptive action.</para>
+
+      <para>In future versions of ModSecurity, more control and functionality
+      will be added to define "how" to block.</para>
+
+      <para>Examples:</para>
+
+      <para>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.</para>
+
+      <programlisting format="linespecific">### 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,<emphasis>pass</emphasis>,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,<emphasis>block</emphasis>,msg:'Nikto Scanners Identified'"</programlisting>
+
+      <para>In the following example, The rule is reverted back to the
+      <literal>pass</literal> action defined in the SecDefaultAction directive
+      by using the <literal>SecRuleUpdateActionById</literal> directive in
+      conjuction with the <literal>block</literal> action. This allows an
+      administrator to override an action in a 3rd party rule without
+      modifying the rule itself.</para>
+
+      <programlisting format="linespecific">### 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,<emphasis>deny</emphasis>,msg:'Nikto Scanners Identified'"
+
+### Added by the administrator
+SecRuleUpdateActionById 1 "<emphasis>block</emphasis>"</programlisting>
+    </section>
+
     <section>
       <title><literal>capture</literal></title>