diff --git a/Makefile.am b/Makefile.am index 85fefaae..b57f23ba 100644 --- a/Makefile.am +++ b/Makefile.am @@ -125,7 +125,6 @@ TESTS+=test/test-cases/regression/config-body_limits.json TESTS+=test/test-cases/regression/config-calling_phases_by_name.json TESTS+=test/test-cases/regression/config-include-bad.json TESTS+=test/test-cases/regression/config-include.json -TESTS+=test/test-cases/regression/config-phases.json TESTS+=test/test-cases/regression/config-remove_by_id.json TESTS+=test/test-cases/regression/config-remove_by_msg.json TESTS+=test/test-cases/regression/config-remove_by_tag.json diff --git a/headers/modsecurity/rules_set.h b/headers/modsecurity/rules_set.h index a6a3c0ff..2ead9a81 100644 --- a/headers/modsecurity/rules_set.h +++ b/headers/modsecurity/rules_set.h @@ -41,87 +41,6 @@ class Driver; } -class RulesSetPhases { - public: - - ~RulesSetPhases() { - /** Cleanup the rules */ - for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { - std::vector rules = m_rules[i]; - while (rules.empty() == false) { - Rule *rule = rules.back(); - rules.pop_back(); - if (rule->refCountDecreaseAndCheck()) { - rule = NULL; - } - } - } - } - - bool insert(Rule *rule) { - if (rule->m_phase >= modsecurity::Phases::NUMBER_OF_PHASES) { - return false; - } - m_rules[rule->m_phase].push_back(rule); - - return true; - } - - int append(RulesSetPhases *from, std::ostringstream *err) { - int amount_of_rules = 0; - std::vector v; - - for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { - v.reserve(m_rules[i].size()); - for (size_t z = 0; z < m_rules[i].size(); z++) { - Rule *rule_ckc = m_rules[i].at(z); - if (rule_ckc->m_secMarker == true) { - continue; - } - v.push_back(rule_ckc->m_ruleId); - } - } - std::sort (v.begin(), v.end()); - - for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { - for (size_t j = 0; j < from->at(i).size(); j++) { - Rule *rule = from->at(i).at(j); - if (std::binary_search(v.begin(), v.end(), rule->m_ruleId)) { - if (err != NULL) { - *err << "Rule id: " << std::to_string(rule->m_ruleId) \ - << " is duplicated" << std::endl; - } - return -1; - } - amount_of_rules++; - rule->refCountIncrease(); - m_rules[i].push_back(rule); - } - } - - return amount_of_rules; - } - - void dump() { - for (int i = 0; i <= modsecurity::Phases::NUMBER_OF_PHASES; i++) { - std::vector rules = m_rules[i]; - std::cout << "Phase: " << std::to_string(i); - std::cout << " (" << std::to_string(rules.size()); - std::cout << " rules)" << std::endl; - for (int j = 0; j < rules.size(); j++) { - std::cout << " Rule ID: " << std::to_string(rules[j]->m_ruleId); - std::cout << "--" << rules[j] << std::endl; - } - } - } - - std::vector operator[](int index) const { return m_rules[index]; } - std::vector at(int index) const { return m_rules[index]; } - - std::vector m_rules[8]; -}; - - /** @ingroup ModSecurity_CPP_API */ class RulesSet : public RulesSetProperties { public: @@ -161,7 +80,6 @@ class RulesSet : public RulesSetProperties { int64_t unicode_codepage; - RulesSetPhases m_rulesSetPhases; private: #ifndef NO_LOGS uint8_t m_secmarker_skipped; diff --git a/headers/modsecurity/rules_set_properties.h b/headers/modsecurity/rules_set_properties.h index 66a09a25..252f51f3 100644 --- a/headers/modsecurity/rules_set_properties.h +++ b/headers/modsecurity/rules_set_properties.h @@ -202,7 +202,17 @@ class RulesSetProperties { ~RulesSetProperties() { int i = 0; - + /** Cleanup the rules */ + for (i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { + std::vector rules = m_rules[i]; + while (rules.empty() == false) { + Rule *rule = rules.back(); + rules.pop_back(); + if (rule->refCountDecreaseAndCheck()) { + rule = NULL; + } + } + } for (i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { std::vector *tmp = &m_defaultActions[i]; while (tmp->empty() == false) { @@ -344,6 +354,12 @@ class RulesSetProperties { static int mergeProperties(RulesSetProperties *from, RulesSetProperties *to, std::ostringstream *err) { + int amount_of_rules = 0; + + amount_of_rules = appendRules(from->m_rules, to->m_rules, err); + if (amount_of_rules < 0) { + return amount_of_rules; + } merge_ruleengine_value(to->m_secRuleEngine, from->m_secRuleEngine, PropertyNotSetRuleEngine); @@ -455,7 +471,56 @@ class RulesSetProperties { } } - return 1; + return amount_of_rules; + } + + + static int appendRules( + std::vector *from, + std::vector *to, + std::ostringstream *err) { + int amount_of_rules = 0; + // TODO: std::vector could be replaced with something more efficient. + std::vector v; + for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { + std::vector *rules_to = to+i; + v.reserve(rules_to->size()); + for (size_t z = 0; z < rules_to->size(); z++) { + Rule *rule_ckc = rules_to->at(z); + if (rule_ckc->m_secMarker == true) { + continue; + } + v.push_back(rule_ckc->m_ruleId); + } + } + std::sort (v.begin(), v.end()); + + for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { + std::vector *rules_from = from+i; + std::vector *rules_to = to+i; + for (size_t j = 0; j < rules_from->size(); j++) { + Rule *rule = rules_from->at(j); + if (std::binary_search(v.begin(), v.end(), rule->m_ruleId)) { + if (err != NULL) { + *err << "Rule id: " << std::to_string(rule->m_ruleId) \ + << " is duplicated" << std::endl; + } + return -1; + } + amount_of_rules++; + rule->refCountIncrease(); + rules_to->push_back(rule); + } + } + return amount_of_rules; + } + + + std::vector *getRulesForPhase(int phase) { + if (phase >= modsecurity::Phases::NUMBER_OF_PHASES) { + return NULL; + } + return &m_rules[phase]; } @@ -486,6 +551,7 @@ class RulesSetProperties { ConfigString m_secArgumentSeparator; ConfigString m_secWebAppId; std::vector m_defaultActions[modsecurity::Phases::NUMBER_OF_PHASES]; + std::vector m_rules[modsecurity::Phases::NUMBER_OF_PHASES]; ConfigUnicodeMap m_unicodeMapTable; }; diff --git a/src/parser/driver.cc b/src/parser/driver.cc index 5de8c29a..90415255 100644 --- a/src/parser/driver.cc +++ b/src/parser/driver.cc @@ -45,7 +45,7 @@ int Driver::addSecMarker(std::string marker) { for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { Rule *rule = new Rule(marker); rule->m_phase = i; - m_rulesSetPhases.insert(rule); + m_rules[i].push_back(rule); } return 0; } @@ -58,15 +58,14 @@ int Driver::addSecAction(Rule *rule) { return false; } - - m_rulesSetPhases.insert(rule); + m_rules[rule->m_phase].push_back(rule); return true; } int Driver::addSecRuleScript(RuleScript *rule) { - m_rulesSetPhases.insert(rule); + m_rules[rule->m_phase].push_back(rule); return true; } @@ -119,7 +118,7 @@ int Driver::addSecRule(Rule *rule) { return false; } for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { - std::vector rules = m_rulesSetPhases[i]; + std::vector rules = m_rules[i]; for (int j = 0; j < rules.size(); j++) { if (rules[j]->m_ruleId == rule->m_ruleId) { m_parserError << "Rule id: " << std::to_string(rule->m_ruleId) \ @@ -130,7 +129,7 @@ int Driver::addSecRule(Rule *rule) { } lastRule = rule; - m_rulesSetPhases.insert(rule); + m_rules[rule->m_phase].push_back(rule); return true; } diff --git a/src/parser/driver.h b/src/parser/driver.h index 72091f0a..9cc49ba7 100644 --- a/src/parser/driver.h +++ b/src/parser/driver.h @@ -90,8 +90,6 @@ class Driver : public RulesSetProperties { std::string buffer; Rule *lastRule; - - RulesSetPhases m_rulesSetPhases; }; diff --git a/src/rules_set.cc b/src/rules_set.cc index 06d49fc0..0e4987ea 100644 --- a/src/rules_set.cc +++ b/src/rules_set.cc @@ -110,7 +110,7 @@ int RulesSet::evaluate(int phase, Transaction *t) { return 0; } - std::vector rules = m_rulesSetPhases[phase]; + std::vector rules = m_rules[phase]; ms_dbg_a(t, 9, "This phase consists of " \ + std::to_string(rules.size()) + " rule(s)."); @@ -222,10 +222,7 @@ int RulesSet::evaluate(int phase, Transaction *t) { int RulesSet::merge(Driver *from) { int amount_of_rules = 0; - - amount_of_rules = m_rulesSetPhases.append(&from->m_rulesSetPhases, - &m_parserError); - mergeProperties( + amount_of_rules = mergeProperties( dynamic_cast(from), dynamic_cast(this), &m_parserError); @@ -236,10 +233,7 @@ int RulesSet::merge(Driver *from) { int RulesSet::merge(RulesSet *from) { int amount_of_rules = 0; - - amount_of_rules = m_rulesSetPhases.append(&from->m_rulesSetPhases, - &m_parserError); - mergeProperties( + amount_of_rules = mergeProperties( dynamic_cast(from), dynamic_cast(this), &m_parserError); @@ -260,7 +254,7 @@ void RulesSet::debug(int level, const std::string &id, void RulesSet::dump() const { std::cout << "Rules: " << std::endl; for (int i = 0; i < modsecurity::Phases::NUMBER_OF_PHASES; i++) { - std::vector rules = m_rulesSetPhases[i]; + std::vector rules = m_rules[i]; std::cout << "Phase: " << std::to_string(i); std::cout << " (" << std::to_string(rules.size()); std::cout << " rules)" << std::endl; diff --git a/test/optimization/optimization.cc b/test/optimization/optimization.cc index 4caf40be..6dd16e02 100644 --- a/test/optimization/optimization.cc +++ b/test/optimization/optimization.cc @@ -67,8 +67,8 @@ int main(int argc, char **argv) { std::cout << std::endl; int nphases = modsecurity::Phases::NUMBER_OF_PHASES; - for (int i = 0; i < nphases; i++) { - std::vector rules = modsecRules->m_rulesSetPhases[i]; + for (int j = 0; j < nphases; j++) { + std::vector rules = modsecRules->m_rules[i]; if (rules.size() == 0) { continue; } diff --git a/test/test-cases/regression/config-phases.json b/test/test-cases/regression/config-phases.json deleted file mode 100644 index 5a620757..00000000 --- a/test/test-cases/regression/config-phases.json +++ /dev/null @@ -1,199 +0,0 @@ -[ - { - "enabled":1, - "version_min":300000, - "title":"Testing Config :: Phases (1/n)", - "client":{ - "ip":"200.249.12.31", - "port":123 - }, - "server":{ - "ip":"200.249.12.31", - "port":80 - }, - "request":{ - "headers":{ - "Host":"localhost", - "User-Agent":"curl/7.38.0", - "Accept":"*/*" - }, - "uri":"/?key=value&key=other_value", - "method":"GET" - }, - "response":{ - "headers":{ - "Date":"Mon, 13 Jul 2015 20:02:41 GMT", - "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT", - "Content-Type":"text/html" - }, - "body":[ - "no need." - ] - }, - "expected":{ - "debug_log":"SecRules 0.*\n.*This phase consists of 1 rule\\(s\\)" - }, - "rules":[ - "SecRuleEngine On", - "SecRule ARGS \"@unconditionalMatch other_value\" \"id:1,phase:0,block,status:404\"" - ] - }, - { - "enabled":1, - "version_min":300000, - "title":"Testing Config :: Phases (2/n)", - "client":{ - "ip":"200.249.12.31", - "port":123 - }, - "server":{ - "ip":"200.249.12.31", - "port":80 - }, - "request":{ - "headers":{ - "Host":"localhost", - "User-Agent":"curl/7.38.0", - "Accept":"*/*" - }, - "uri":"/?key=value&key=other_value", - "method":"GET" - }, - "response":{ - "headers":{ - "Date":"Mon, 13 Jul 2015 20:02:41 GMT", - "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT", - "Content-Type":"text/html" - }, - "body":[ - "no need." - ] - }, - "expected":{ - "debug_log":"SecRules 1.*\n.*This phase consists of 1 rule\\(s\\)" - }, - "rules":[ - "SecRuleEngine On", - "SecRule ARGS \"@unconditionalMatch other_value\" \"id:1,phase:1,block,status:404\"" - ] -}, -{ - "enabled":1, - "version_min":300000, - "title":"Testing Config :: Phases (3/n)", - "client":{ - "ip":"200.249.12.31", - "port":123 - }, - "server":{ - "ip":"200.249.12.31", - "port":80 - }, - "request":{ - "headers":{ - "Host":"localhost", - "User-Agent":"curl/7.38.0", - "Accept":"*/*" - }, - "uri":"/?key=value&key=other_value", - "method":"GET" - }, - "response":{ - "headers":{ - "Date":"Mon, 13 Jul 2015 20:02:41 GMT", - "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT", - "Content-Type":"text/html" - }, - "body":[ - "no need." - ] - }, - "expected":{ - "debug_log":"SecRules 3.*\n.*This phase consists of 1 rule\\(s\\)" - }, - "rules":[ - "SecRuleEngine On", - "SecRule ARGS \"@unconditionalMatch other_value\" \"id:1,phase:3,block,status:404\"" - ] -}, -{ - "enabled":1, - "version_min":300000, - "title":"Testing Config :: Phases (4/n)", - "client":{ - "ip":"200.249.12.31", - "port":123 - }, - "server":{ - "ip":"200.249.12.31", - "port":80 - }, - "request":{ - "headers":{ - "Host":"localhost", - "User-Agent":"curl/7.38.0", - "Accept":"*/*" - }, - "uri":"/?key=value&key=other_value", - "method":"GET" - }, - "response":{ - "headers":{ - "Date":"Mon, 13 Jul 2015 20:02:41 GMT", - "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT", - "Content-Type":"text/html" - }, - "body":[ - "no need." - ] - }, - "expected":{ - "debug_log":"SecRules 4.*\n.*This phase consists of 1 rule\\(s\\)" - }, - "rules":[ - "SecRuleEngine On", - "SecResponseBodyAccess On", - "SecRule ARGS \"@unconditionalMatch other_value\" \"id:1,phase:4,block,status:404\"" - ] -}, -{ - "enabled":1, - "version_min":300000, - "title":"Testing Config :: Phases (5/n)", - "client":{ - "ip":"200.249.12.31", - "port":123 - }, - "server":{ - "ip":"200.249.12.31", - "port":80 - }, - "request":{ - "headers":{ - "Host":"localhost", - "User-Agent":"curl/7.38.0", - "Accept":"*/*" - }, - "uri":"/?key=value&key=other_value", - "method":"GET" - }, - "response":{ - "headers":{ - "Date":"Mon, 13 Jul 2015 20:02:41 GMT", - "Last-Modified":"Sun, 26 Oct 2014 22:33:37 GMT", - "Content-Type":"text/html" - }, - "body":[ - "no need." - ] - }, - "expected":{ - "debug_log":"SecRules 5.*\n.*This phase consists of 1 rule\\(s\\)" - }, - "rules":[ - "SecRuleEngine On", - "SecRule ARGS \"@unconditionalMatch other_value\" \"id:1,phase:5,block,status:404\"" - ] -} -] -