Fix disruptive actions execution

src/Makefile.am

@@ -57,9 +57,11 @@ ACTIONS = \
 	actions/capture.cc \
 	actions/chain.cc \
 	actions/ctl_audit_log_parts.cc \
+	actions/deny.cc \
 	actions/log_data.cc \
 	actions/msg.cc \
 	actions/no_audit_log.cc \
+	actions/pass.cc \
 	actions/phase.cc \
 	actions/redirect.cc \
 	actions/rev.cc \

src/actions/action.cc

@@ -23,12 +23,14 @@
 
 #include "actions/block.h"
 #include "actions/chain.h"
+#include "actions/deny.h"
 #include "actions/redirect.h"
 #include "actions/status.h"
 #include "actions/rule_id.h"
 #include "actions/phase.h"
 #include "actions/severity.h"
 #include "actions/capture.h"
+#include "actions/pass.h"
 
 
 
@@ -82,6 +84,12 @@ Action *Action::instantiate(const std::string& name) {
     if (name == "capture") {
         return new Capture(name);
     }
+    if (name == "pass") {
+        return new Pass(name);
+    }
+    if (name == "deny") {
+        return new Deny(name);
+    }
     return new Action(name);
 }
 

src/actions/block.cc

@@ -19,6 +19,7 @@
 #include <string>
 
 #include "modsecurity/assay.h"
+#include "src/rule.h"
 
 namespace ModSecurity {
 namespace actions {
@@ -31,13 +32,16 @@ Block::Block(std::string action)
 
 
 bool Block::evaluate(Rule *rule, Assay *assay) {
-    assay->actions.push_back(this);
+    for (Action *a : rule->actions_runtime_pos) {
+        if (a->isDisruptive() == true) {
+            assay->actions.push_back(a);
+        }
+    }
     return true;
 }
 
 void Block::fill_intervention(ModSecurityIntervention *i) {
-    i->status = 403;
+
-    i->log = "Blocked request!";
 }
 
 }  // namespace actions

src/actions/block.h

@@ -35,6 +35,7 @@ class Block : public Action {
 
     bool evaluate(Rule *rule, Assay *assay) override;
     void fill_intervention(ModSecurityIntervention *i) override;
+    bool isDisruptive() override { return true; }
 };
 
 }  // namespace actions

src/actions/deny.cc

@@ -0,0 +1,44 @@
+/*
+ * ModSecurity, http://www.modsecurity.org/
+ * Copyright (c) 2015 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ *
+ * You may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * If any of the files related to licensing are missing or if you have any
+ * other questions related to licensing please contact Trustwave Holdings, Inc.
+ * directly using the email address security@modsecurity.org.
+ *
+ */
+
+#include "actions/deny.h"
+
+#include <iostream>
+#include <string>
+
+#include "modsecurity/assay.h"
+
+namespace ModSecurity {
+namespace actions {
+
+Deny::Deny(std::string action)
+    : Action(action) {
+    this->action = action;
+    this->action_kind = 2;
+}
+
+
+bool Deny::evaluate(Rule *rule, Assay *assay) {
+    assay->actions.push_back(this);
+    return true;
+}
+
+void Deny::fill_intervention(ModSecurityIntervention *i) {
+    i->status = 403;
+    i->log = "Deny action";
+}
+
+}  // namespace actions
+}  // namespace ModSecurity

src/actions/deny.h

@@ -0,0 +1,41 @@
+/*
+ * ModSecurity, http://www.modsecurity.org/
+ * Copyright (c) 2015 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ *
+ * You may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * If any of the files related to licensing are missing or if you have any
+ * other questions related to licensing please contact Trustwave Holdings, Inc.
+ * directly using the email address security@modsecurity.org.
+ *
+ */
+
+#include <string>
+
+#include "actions/action.h"
+#include "modsecurity/assay.h"
+
+#ifndef SRC_ACTIONS_DENY_H_
+#define SRC_ACTIONS_DENY_H_
+
+namespace ModSecurity {
+namespace actions {
+
+
+class Deny : public Action {
+ public:
+    explicit Deny(std::string action);
+
+    bool evaluate(Rule *rule, Assay *assay) override;
+    void fill_intervention(ModSecurityIntervention *i) override;
+    bool isDisruptive() override { return true; }
+};
+
+
+}  // namespace actions
+}  // namespace ModSecurity
+
+#endif  // SRC_ACTIONS_DENY_H_

src/actions/pass.cc

@@ -0,0 +1,44 @@
+/*
+ * ModSecurity, http://www.modsecurity.org/
+ * Copyright (c) 2015 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ *
+ * You may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * If any of the files related to licensing are missing or if you have any
+ * other questions related to licensing please contact Trustwave Holdings, Inc.
+ * directly using the email address security@modsecurity.org.
+ *
+ */
+
+#include "actions/pass.h"
+
+#include <iostream>
+#include <string>
+
+#include "modsecurity/assay.h"
+#include "src/rule.h"
+
+namespace ModSecurity {
+namespace actions {
+
+Pass::Pass(std::string action)
+    : Action(action) {
+    this->action = action;
+    this->action_kind = 2;
+}
+
+
+bool Pass::evaluate(Rule *rule, Assay *assay) {
+    assay->actions.clear();
+    return true;
+}
+
+void Pass::fill_intervention(ModSecurityIntervention *i) {
+
+}
+
+}  // namespace actions
+}  // namespace ModSecurity

src/actions/pass.h

@@ -0,0 +1,41 @@
+/*
+ * ModSecurity, http://www.modsecurity.org/
+ * Copyright (c) 2015 Trustwave Holdings, Inc. (http://www.trustwave.com/)
+ *
+ * You may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * If any of the files related to licensing are missing or if you have any
+ * other questions related to licensing please contact Trustwave Holdings, Inc.
+ * directly using the email address security@modsecurity.org.
+ *
+ */
+
+#include <string>
+
+#include "actions/action.h"
+#include "modsecurity/assay.h"
+
+#ifndef SRC_ACTIONS_PASS_H_
+#define SRC_ACTIONS_PASS_H_
+
+namespace ModSecurity {
+namespace actions {
+
+
+class Pass : public Action {
+ public:
+    explicit Pass(std::string action);
+
+    bool evaluate(Rule *rule, Assay *assay) override;
+    void fill_intervention(ModSecurityIntervention *i) override;
+    bool isDisruptive() override { return true; }
+};
+
+}  // namespace actions
+}  // namespace ModSecurity
+
+
+#endif  // SRC_ACTIONS_PASS_H_

src/actions/redirect.h

@@ -37,6 +37,7 @@ class Redirect : public Action {
     int status;
     std::string url;
     void fill_intervention(ModSecurityIntervention *i) override;
+    bool isDisruptive() override { return true; }
 };
 
 }  // namespace actions

src/assay.cc

@@ -32,7 +32,7 @@
 #include "modsecurity/modsecurity.h"
 #include "modsecurity/intervention.h"
 #include "actions/action.h"
-#include "actions/block.h"
+#include "actions/deny.h"
 #include "src/utils.h"
 #include "src/audit_log.h"
 #include "src/unique_id.h"
@@ -710,7 +710,7 @@ int Assay::appendRequestBody(const unsigned char *buf, size_t len) {
                 Rules::BodyLimitAction::RejectBodyLimitAction) {
                 debug(5, "Request body limit is marked to reject the " \
                     "request");
-                Action *a = new actions::Block("block");
+                Action *a = new actions::Deny("deny");
                 a->temporaryAction = true;
                 actions.push_back(a);
             }
@@ -906,7 +906,7 @@ int Assay::appendResponseBody(const unsigned char *buf, size_t len) {
                 Rules::BodyLimitAction::RejectBodyLimitAction) {
                 debug(5, "Response body limit is marked to reject the " \
                     "request");
-                Action *a = new actions::Block("block");
+                Action *a = new actions::Deny("deny");
                 a->temporaryAction = true;
                 actions.push_back(a);
             }

src/rule.cc

@@ -297,9 +297,20 @@ bool Rule::evaluate(Assay *assay) {
                 std::to_string(elapsed_secs) + " seconds");
 
             if (ret) {
+                bool containsDisruptive = false;
                 bool chainResult = false;
                 assay->debug(4, "Rule returned 1.");
 
+                for (Action *a :
+                    this->actions_runtime_pos) {
+                    if (a->isDisruptive() == false) {
+                        assay->debug(4, "Running (_non_ disruptive) action: " + a->action);
+                        a->evaluate(this, assay);
+                    } else {
+                        containsDisruptive = true;
+                    }
+                }
+
                 if (this->chained && this->chainedRule == NULL) {
                     assay->debug(4, "Rule is marked as chained but there " \
                         "isn't a subsequent rule.");
@@ -326,16 +337,33 @@ bool Rule::evaluate(Assay *assay) {
                 if (this->chained && chainResult == true || !this->chained) {
                     for (Action *a : assay->m_rules->defaultActions[this->phase]) {
                         if (a->action_kind == actions::Action::RunTimeOnlyIfMatchKind) {
-                            assay->debug(4, "(SecDefaultAction) Running action: " + a->action);
+                            if (a->isDisruptive()) {
+                                if (containsDisruptive) {
+                                    assay->debug(4, "(SecDefaultAction) " \
+                                        "_ignoring_ action: " + a->action + \
+                                        " (rule contains a disruptive action)");
+                                } else {
+                                    assay->debug(4, "(SecDefaultAction) " \
+                                        "Running action: " + a->action + \
+                                        " (rule _does not_ contains a " \
+                                        "disruptive action)");
                                     a->evaluate(this, assay);
                                 }
+                            } else {
+                                assay->debug(4, "(SecDefaultAction) Running " \
+                                    "action: " + a->action);
+                                a->evaluate(this, assay);
+                            }
+                        }
                     }
                     for (Action *a :
                         this->actions_runtime_pos) {
-                        assay->debug(4, "Running action: " + a->action);
+                        if (a->isDisruptive()) {
+                            assay->debug(4, "Running (disruptive) action: " + a->action);
                             a->evaluate(this, assay);
                         }
                     }
+                }
 
             } else {
                 assay->debug(4, "Rule returned 0.");

test/test-cases/regression/action-disruptive.json

@@ -0,0 +1,74 @@
+[
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing Disruptive actions (1/n)",
+    "expected":{
+      "debug_log": " Running action: deny",
+      "http_code":403
+    },
+    "rules":[
+      "SecRuleEngine On",
+      "SecDebugLog \/tmp\/modsec_debug.log",
+      "SecDefaultAction \"phase:2,deny,status:404\"",
+      "SecAction \"id:'900001',phase:request,nolog,status:403,t:none\""
+    ]
+  },
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing Disruptive actions (2/n)",
+    "expected":{
+      "debug_log": " Running action: deny",
+      "http_code":404
+    },
+    "rules":[
+      "SecRuleEngine On",
+      "SecDebugLog \/tmp\/modsec_debug.log",
+      "SecDefaultAction \"phase:2,deny,status:404\"",
+      "SecAction \"'id:'1',phase:request,nolog,t:none\""
+    ]
+  },
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing Disruptive actions (3/n)",
+    "expected":{
+      "debug_log": " Running action: deny",
+      "http_code":404
+    },
+    "rules":[
+      "SecRuleEngine On",
+      "SecDebugLog \/tmp\/modsec_debug.log",
+      "SecDefaultAction \"phase:2,deny,status:404\"",
+      "SecAction \"id:'1',phase:request,nolog,block,t:none\""
+    ]
+  },
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing Disruptive actions (4/n)",
+    "expected":{
+      "http_code":200
+    },
+    "rules":[
+      "SecRuleEngine On",
+      "SecDebugLog \/tmp\/modsec_debug.log",
+      "SecAction \"id:'1',phase:request,nolog,t:none\""
+    ]
+  },
+  {
+    "enabled":1,
+    "version_min":300000,
+    "title":"Testing Disruptive actions (5/n)",
+    "expected":{
+      "http_code":200
+    },
+    "rules":[
+      "SecRuleEngine On",
+      "SecDebugLog \/tmp\/modsec_debug.log",
+      "SecDefaultAction \"phase:2,deny,status:404\"",
+      "SecAction \"id:'1',phase:request,nolog,pass,t:none\""
+    ]
+  }
+]

test/test-cases/regression/actions.json

@@ -3,7 +3,7 @@
     "enabled": 1,
     "version_min": 300000,
     "version_max": 0,
-    "title": "actions :: trim,block",
+    "title": "actions :: trim,deny",
     "client": {
       "ip": "200.249.12.31",
       "port": 2313
@@ -57,7 +57,7 @@
       "SecRuleEngine On",
       "SecDebugLog \/tmp\/modsec_debug.log",
       "SecDebugLogLevel 9",
-      "SecRule ARGS \"@contains test\" \"id:1,t:trim,block\""
+      "SecRule ARGS \"@contains test\" \"id:1,t:trim,deny\""
     ]
   },
     {

test/test-cases/regression/auditlog.json

@@ -48,7 +48,7 @@
       "SecRuleEngine On",
       "SecDebugLog \/tmp\/modsec_debug.log",
       "SecDebugLogLevel 9",
-      "SecRule ARGS \"@contains test\" \"id:1,t:trim,block,auditlog\"",
+      "SecRule ARGS \"@contains test\" \"id:1,t:trim,deny,auditlog\"",
       "SecAuditEngine RelevantOnly",
       "SecAuditLogParts ABCFHZ",
       "SecAuditLogStorageDir /tmp/test",
@@ -107,7 +107,7 @@
       "SecRuleEngine On",
       "SecDebugLog \/tmp\/modsec_debug.log",
       "SecDebugLogLevel 9",
-      "SecRule ARGS \"@contains test\" \"id:1,t:trim,block,auditlog\"",
+      "SecRule ARGS \"@contains test\" \"id:1,t:trim,deny,auditlog\"",
       "SecAuditEngine RelevantOnly",
       "SecAuditLogParts ABCFHZ",
       "SecAuditLogStorageDir /tmp/test",
@@ -167,7 +167,7 @@
       "SecRuleEngine On",
       "SecDebugLog \/tmp\/modsec_debug.log",
       "SecDebugLogLevel 9",
-      "SecRule ARGS \"@contains test\" \"id:1,t:trim,block,auditlog\"",
+      "SecRule ARGS \"@contains test\" \"id:1,t:trim,deny,auditlog\"",
       "SecAuditEngine RelevantOnly",
       "SecAuditLogParts ABCFHZ",
       "SecAuditLogStorageDir /tmp/test",

test/test-cases/regression/sec_component_signature.json

@@ -49,7 +49,7 @@
       "SecDebugLog \/tmp\/modsec_debug.log",
       "SecDebugLogLevel 9",
       "SecComponentSignature \"OWASP_CRS/2.2.9\"",
-      "SecRule ARGS \"@contains test\" \"id:1,t:trim,block,auditlog\""
+      "SecRule ARGS \"@contains test\" \"id:1,t:trim,deny,status:403,auditlog\""
     ]
   }
 ]