Skip to content

Commit 768a76a

Browse files
author
Felipe Zimmerle
committed
perf. improvement/rx: Only compute dynamic regex in case of macro
On #1528 was added the support for macro expansion on @rx operator. The performance improvement suggested on the pull request was not thread safe, therefore removed. This patch adds a performance improvement on top of #1528. The benchmarks points to 10x faster results on OWASP CRS.
1 parent 4a23891 commit 768a76a

File tree

8 files changed

+56
-34
lines changed

8 files changed

+56
-34
lines changed

src/macro_expansion.cc

+13-1
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,20 @@ std::string MacroExpansion::expand(const std::string& input,
4848
}
4949

5050

51+
bool MacroExpansion::containsMacro(std::string& input) {
52+
size_t pos = input.find("%{");
53+
size_t end = input.find("}");
54+
55+
if (pos == std::string::npos || end == std::string::npos) {
56+
return false;
57+
}
58+
59+
return true;
60+
}
61+
62+
5163
std::string MacroExpansion::expand(const std::string& input,
52-
modsecurity::Rule *rule, Transaction *transaction) {
64+
modsecurity::Rule *rule, Transaction *transaction) {
5365
std::string res;
5466
size_t pos = input.find("%{");
5567

src/macro_expansion.h

+2
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,8 @@ class MacroExpansion {
4747
return toupper(aa) == bb;
4848
});
4949
}
50+
51+
static bool containsMacro(std::string& input);
5052
};
5153

5254

src/operators/rx.cc

+18-3
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,15 @@ namespace modsecurity {
2828
namespace operators {
2929

3030

31+
bool Rx::init(const std::string &arg, std::string *error) {
32+
m_containsMacro = MacroExpansion::containsMacro(m_param);
33+
if (m_containsMacro == false) {
34+
m_re = new Regex(m_param);
35+
}
36+
37+
return true;
38+
}
39+
3140

3241
bool Rx::evaluate(Transaction *transaction, Rule *rule,
3342
const std::string& input, std::shared_ptr<RuleMessage> ruleMessage) {
@@ -39,8 +48,12 @@ bool Rx::evaluate(Transaction *transaction, Rule *rule,
3948
return true;
4049
}
4150

42-
std::string eparam = MacroExpansion::expand(m_param, transaction);
43-
re = new Regex(eparam);
51+
if (m_containsMacro) {
52+
std::string eparam = MacroExpansion::expand(m_param, transaction);
53+
re = new Regex(eparam);
54+
} else {
55+
re = m_re;
56+
}
4457

4558
matches = re->searchAll(input);
4659
if (rule && rule->getActionsByName("capture").size() > 0 && transaction) {
@@ -62,7 +75,9 @@ bool Rx::evaluate(Transaction *transaction, Rule *rule,
6275
logOffset(ruleMessage, i.m_offset, i.m_length);
6376
}
6477

65-
delete re;
78+
if (m_containsMacro) {
79+
delete re;
80+
}
6681

6782
if (matches.size() > 0) {
6883
return true;

src/operators/rx.h

+16-3
Original file line numberDiff line numberDiff line change
@@ -36,17 +36,25 @@ class Rx : public Operator {
3636
public:
3737
/** @ingroup ModSecurity_Operator */
3838
Rx(std::string op, std::string param, bool negation)
39-
: Operator(op, param, negation) {
39+
: Operator(op, param, negation),
40+
m_containsMacro(false) {
4041
}
4142
Rx(std::string name, std::string param)
42-
: Operator(name, param) {
43+
: Operator(name, param),
44+
m_containsMacro(false) {
4345
}
4446
explicit Rx(std::string param)
45-
: Operator("Rx", param) {
47+
: Operator("Rx", param),
48+
m_containsMacro(false) {
4649
}
4750

4851
~Rx() {
52+
if (m_containsMacro == false && m_re != NULL) {
53+
delete m_re;
54+
m_re = NULL;
55+
}
4956
}
57+
5058
bool evaluate(Transaction *transaction, Rule *rule,
5159
const std::string &input) override {
5260
return evaluate(transaction, NULL, input, NULL);
@@ -58,6 +66,11 @@ class Rx : public Operator {
5866
bool evaluate(Transaction *transaction, Rule *rule,
5967
const std::string& input,
6068
std::shared_ptr<RuleMessage> ruleMessage) override;
69+
70+
bool init(const std::string &arg, std::string *error);
71+
private:
72+
bool m_containsMacro;
73+
Regex *m_re;
6174
};
6275

6376

src/parser/seclang-parser.yy

+5
Original file line numberDiff line numberDiff line change
@@ -1027,6 +1027,11 @@ op_before_init:
10271027
| OPERATOR_RX FREE_TEXT
10281028
{
10291029
OPERATOR_CONTAINER($$, new operators::Rx($2));
1030+
std::string error;
1031+
if ($$->init(driver.ref.back(), &error) == false) {
1032+
driver.error(@0, error);
1033+
YYERROR;
1034+
}
10301035
}
10311036
| OPERATOR_STR_EQ FREE_TEXT
10321037
{

test/benchmark/basic_rules.conf

-2
Original file line numberDiff line numberDiff line change
@@ -3,5 +3,3 @@ Include "../../modsecurity.conf-recommended"
33

44
Include "owasp-v3/crs-setup.conf.example"
55
Include "owasp-v3/rules/*.conf"
6-
Include "owasp-v3/crs-setup.conf.example"
7-
Include "owasp-v3/rules/*.conf"

test/benchmark/benchmark.cc

+1-24
Original file line numberDiff line numberDiff line change
@@ -24,31 +24,8 @@
2424

2525
using modsecurity::Transaction;
2626

27-
char request_header[] = "" \
28-
"GET /tutorials/other/top-20-mysql-best-practices/ HTTP/1.1\n\r" \
29-
"Host: net.tutsplus.com\n\r" \
30-
"User-Agent: Mozilla/5.0 (Windows; U; Windows NT 6.1; en-US; rv:1.9.1.5)" \
31-
" Gecko/20091102 Firefox/3.5.5 (.NET CLR 3.5.30729)\n\r" \
32-
"Accept: text/html,application/xhtml+xml,application/xml; " \
33-
"q=0.9,*/*;q=0.8\n\r" \
34-
"Accept-Language: en-us,en;q=0.5\n\r" \
35-
"Accept-Encoding: gzip,deflate\n\r" \
36-
"Accept-Charset: ISO-8859-1,utf-8;q=0.7,*;q=0.7\n\r" \
37-
"Keep-Alive: 300\n\r" \
38-
"Connection: keep-alive\n\r" \
39-
"Cookie: PHPSESSID=r2t5uvjq435r4q7ib3vtdjq120\n\r" \
40-
"Pragma: no-cache\n\r" \
41-
"Cache-Control: no-cache\n\r";
42-
4327
char request_uri[] = "/test.pl?param1=test&para2=test2";
4428

45-
char request_body[] = "";
46-
47-
char response_headers[] = "" \
48-
"HTTP/1.1 200 OK\n\r" \
49-
"Content-Type: text/xml; charset=utf-8\n\r" \
50-
"Content-Length: length\n\r";
51-
5229
unsigned char response_body[] = "" \
5330
"<?xml version=\"1.0\" encoding=\"utf-8\"?>\n\r" \
5431
"<soap:Envelope xmlns:xsi=\"http://www.w3.org/2001/XMLSchema-instance\" " \
@@ -69,7 +46,7 @@ const char* const help_message = "Usage: benchmark [num_iterations|-h|-?|--help]
6946

7047
int main(int argc, char *argv[]) {
7148

72-
unsigned long long NUM_REQUESTS(10000);
49+
unsigned long long NUM_REQUESTS(1000000);
7350

7451
if (argc > 1) {
7552
if (0 == strcmp(argv[1], "-h") ||

test/benchmark/download-owasp-v3-rules.sh

+1-1
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33

44
git clone https://github.com/SpiderLabs/owasp-modsecurity-crs.git owasp-v3
55
cd owasp-v3
6-
git checkout v3.0.0 -b tag3.0.0
6+
git checkout v3.0.2 -b tag3.0.2
77
cd -
88

99
echo 'Include "owasp-v3/crs-setup.conf.example"' >> basic_rules.conf

0 commit comments

Comments
 (0)