diff --git a/src/Makefile.am b/src/Makefile.am index ea561751..13f9045c 100644 --- a/src/Makefile.am +++ b/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 \ diff --git a/src/actions/action.cc b/src/actions/action.cc index 3579f55e..d49d2874 100644 --- a/src/actions/action.cc +++ b/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); } diff --git a/src/actions/block.cc b/src/actions/block.cc index 2ff1ae30..59367669 100644 --- a/src/actions/block.cc +++ b/src/actions/block.cc @@ -19,6 +19,7 @@ #include #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 diff --git a/src/actions/block.h b/src/actions/block.h index 371b7447..94884e75 100644 --- a/src/actions/block.h +++ b/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 diff --git a/src/actions/deny.cc b/src/actions/deny.cc new file mode 100644 index 00000000..bf1178f5 --- /dev/null +++ b/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 +#include + +#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 diff --git a/src/actions/deny.h b/src/actions/deny.h new file mode 100644 index 00000000..f6fe63c5 --- /dev/null +++ b/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 + +#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_ diff --git a/src/actions/pass.cc b/src/actions/pass.cc new file mode 100644 index 00000000..82c31f7f --- /dev/null +++ b/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 +#include + +#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 diff --git a/src/actions/pass.h b/src/actions/pass.h new file mode 100644 index 00000000..5f89e317 --- /dev/null +++ b/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 + +#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_ diff --git a/src/actions/redirect.h b/src/actions/redirect.h index 39fbe8f8..3da9d736 100644 --- a/src/actions/redirect.h +++ b/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 diff --git a/src/assay.cc b/src/assay.cc index fd7e0563..96c66208 100644 --- a/src/assay.cc +++ b/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); } diff --git a/src/rule.cc b/src/rule.cc index c95b4c2a..13b91907 100644 --- a/src/rule.cc +++ b/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,14 +337,31 @@ 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); - a->evaluate(this, assay); + 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); - a->evaluate(this, assay); + if (a->isDisruptive()) { + assay->debug(4, "Running (disruptive) action: " + a->action); + a->evaluate(this, assay); + } } } diff --git a/test/test-cases/regression/action-disruptive.json b/test/test-cases/regression/action-disruptive.json new file mode 100644 index 00000000..25e82be5 --- /dev/null +++ b/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\"" + ] + } +] \ No newline at end of file diff --git a/test/test-cases/regression/actions.json b/test/test-cases/regression/actions.json index cea5ec67..e293aa50 100644 --- a/test/test-cases/regression/actions.json +++ b/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\"" ] }, { diff --git a/test/test-cases/regression/auditlog.json b/test/test-cases/regression/auditlog.json index 19111c77..c62ba571 100644 --- a/test/test-cases/regression/auditlog.json +++ b/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", diff --git a/test/test-cases/regression/sec_component_signature.json b/test/test-cases/regression/sec_component_signature.json index e05c3d4e..e7668939 100644 --- a/test/test-cases/regression/sec_component_signature.json +++ b/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\"" ] } ] \ No newline at end of file