Skip to content

Commit 274f9e5

Browse files
author
Felipe Zimmerle
committed
Refactoring on RuleMessage class, now accepting http code as parameter
1 parent 39fb75c commit 274f9e5

File tree

6 files changed

+64
-114
lines changed

6 files changed

+64
-114
lines changed

‎CHANGES

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22
v3.0.????? - ?
33
---------------------------
44

5+
- Refactoring on RuleMessage class, now accepting http code as parameter.
6+
[@zimmerle]
57
- Having disruptive msgs as disruptive [instead of warnings] on audit log
68
[Issue #1592 - @zimmerle, @nobodysz]
79
- Parser: Pipes are no longer welcomed inside regex dict element selection.

‎headers/modsecurity/rule_message.h

Lines changed: 23 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -32,13 +32,19 @@
3232

3333
namespace modsecurity {
3434

35+
36+
3537
class RuleMessage {
3638
public:
39+
enum LogMessageInfo {
40+
ErrorLogTailLogMessageInfo = 2,
41+
ClientLogMessageInfo = 4
42+
};
43+
3744
explicit RuleMessage(Rule *rule, Transaction *trans) :
3845
m_accuracy(rule->m_accuracy),
3946
m_clientIpAddress(trans->m_clientIpAddress),
4047
m_data(""),
41-
m_disruptiveMessage(""),
4248
m_id(trans->m_id),
4349
m_isDisruptive(false),
4450
m_match(""),
@@ -59,35 +65,31 @@ class RuleMessage {
5965
m_ver(rule->m_ver)
6066
{ }
6167

62-
std::string errorLog() {
63-
return RuleMessage::errorLog(this);
64-
}
65-
std::string disruptiveErrorLog() {
66-
return RuleMessage::disruptiveErrorLog(this);
68+
69+
std::string log() {
70+
return RuleMessage::log(this, 0);
6771
}
68-
std::string noClientErrorLog() {
69-
return RuleMessage::noClientErrorLog(this);
72+
std::string log(int props) {
73+
return RuleMessage::log(this, props);
7074
}
71-
std::string noClientErrorLog(bool disrupt) {
72-
return RuleMessage::noClientErrorLog(this, disrupt);
75+
std::string errorLog() {
76+
return RuleMessage::log(this, ClientLogMessageInfo | ErrorLogTailLogMessageInfo);
7377
}
74-
std::string errorLogTail() {
75-
return RuleMessage::errorLogTail(this);
78+
79+
static std::string log(const RuleMessage *rm, int props, int code);
80+
static std::string log(const RuleMessage *rm, int props) {
81+
return RuleMessage::log(rm, props, -1);
7682
}
77-
std::string log() {
78-
return RuleMessage::log(this);
83+
static std::string log(const RuleMessage *rm) {
84+
return RuleMessage::log(rm, 0);
7985
}
80-
static std::string disruptiveErrorLog(const RuleMessage *rm);
81-
static std::string noClientErrorLog(const RuleMessage *rm);
82-
static std::string noClientErrorLog(const RuleMessage *rm, bool disrupt);
83-
static std::string errorLogTail(const RuleMessage *rm);
84-
static std::string errorLog(const RuleMessage *rm);
85-
static std::string log(const RuleMessage *rm);
86+
87+
static std::string _details(const RuleMessage *rm);
88+
static std::string _errorLogTail(const RuleMessage *rm);
8689

8790
int m_accuracy;
8891
std::string m_clientIpAddress;
8992
std::string m_data;
90-
std::string m_disruptiveMessage;
9193
std::string m_id;
9294
bool m_isDisruptive;
9395
std::string m_match;

‎src/actions/disruptive/deny.cc

Lines changed: 1 addition & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,21 +33,15 @@ bool Deny::evaluate(Rule *rule, Transaction *transaction,
3333
#ifndef NO_LOGS
3434
transaction->debug(8, "Running action deny");
3535
#endif
36-
std::string log;
3736

3837
if (transaction->m_it.status == 200) {
3938
transaction->m_it.status = 403;
4039
}
4140

42-
log.append("Access denied with code %d");
43-
log.append(" (phase ");
44-
log.append(std::to_string(rm->m_rule->m_phase - 1) + "). ");
45-
46-
rm->m_disruptiveMessage.assign(log);
4741
transaction->m_it.disruptive = true;
4842
intervention::freeLog(&transaction->m_it);
4943
transaction->m_it.log = strdup(
50-
rm->disruptiveErrorLog().c_str());
44+
rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
5145

5246
rm->m_isDisruptive = true;
5347
return true;

‎src/actions/disruptive/redirect.cc

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -40,23 +40,18 @@ bool Redirect::init(std::string *error) {
4040
bool Redirect::evaluate(Rule *rule, Transaction *transaction,
4141
std::shared_ptr<RuleMessage> rm) {
4242
m_urlExpanded = MacroExpansion::expand(m_url, transaction);
43-
std::string log;
4443

4544
/* if it was changed before, lets keep it. */
4645
if (transaction->m_it.status == 200) {
4746
transaction->m_it.status = m_status;
4847
}
49-
log.append("Access denied with code %d");
50-
log.append(" (phase ");
51-
log.append(std::to_string(rm->m_rule->m_phase - 1) + "). ");
5248

53-
rm->m_disruptiveMessage.assign(log);
5449
intervention::freeUrl(&transaction->m_it);
5550
transaction->m_it.url = strdup(m_urlExpanded.c_str());
5651
transaction->m_it.disruptive = true;
5752
intervention::freeLog(&transaction->m_it);
5853
transaction->m_it.log = strdup(
59-
rm->disruptiveErrorLog().c_str());
54+
rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
6055

6156
rm->m_isDisruptive = true;
6257
return true;

‎src/rule_message.cc

Lines changed: 36 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -23,13 +23,10 @@
2323

2424
namespace modsecurity {
2525

26-
std::string RuleMessage::disruptiveErrorLog(const RuleMessage *rm) {
26+
27+
std::string RuleMessage::_details(const RuleMessage *rm) {
2728
std::string msg;
2829

29-
msg.append("[client " + std::string(rm->m_clientIpAddress) + "]");
30-
msg.append(" ModSecurity: ");
31-
msg.append(rm->m_disruptiveMessage);
32-
msg.append(rm->m_match);
3330
msg.append(" [file \"" + std::string(rm->m_ruleFile) + "\"]");
3431
msg.append(" [line \"" + std::to_string(rm->m_ruleLine) + "\"]");
3532
msg.append(" [id \"" + std::to_string(rm->m_ruleId) + "\"]");
@@ -50,91 +47,55 @@ std::string RuleMessage::disruptiveErrorLog(const RuleMessage *rm) {
5047
msg.append(" [unique_id \"" + rm->m_id + "\"]");
5148
msg.append(" [ref \"" + rm->m_reference + "\"]");
5249

53-
return modsecurity::utils::string::toHexIfNeeded(msg);
50+
return msg;
5451
}
5552

56-
std::string RuleMessage::noClientErrorLog(const RuleMessage *rm, bool disruptive) {
57-
std::string msg;
58-
if (disruptive == false) {
59-
return RuleMessage::noClientErrorLog(rm);
60-
}
6153

62-
msg.append("Message: ");
63-
msg.append(rm->m_disruptiveMessage);
64-
msg.append(rm->m_match);
65-
msg.append(" [file \"" + std::string(rm->m_ruleFile) + "\"]");
66-
msg.append(" [line \"" + std::to_string(rm->m_ruleLine) + "\"]");
67-
msg.append(" [id \"" + std::to_string(rm->m_ruleId) + "\"]");
68-
msg.append(" [rev \"" + rm->m_rev + "\"]");
69-
msg.append(" [msg \"" + rm->m_message + "\"]");
70-
msg.append(" [data \"" + rm->m_data + "\"]");
71-
msg.append(" [severity \"" +
72-
std::to_string(rm->m_severity) + "\"]");
73-
msg.append(" [ver \"" + rm->m_ver + "\"]");
74-
msg.append(" [maturity \"" + std::to_string(rm->m_maturity) + "\"]");
75-
msg.append(" [accuracy \"" + std::to_string(rm->m_accuracy) + "\"]");
76-
for (auto &a : rm->m_tags) {
77-
msg.append(" [tag \"" + a + "\"]");
78-
}
79-
msg.append(" [ref \"" + rm->m_reference + "\"]");
80-
81-
return modsecurity::utils::string::toHexIfNeeded(msg);
82-
}
83-
84-
std::string RuleMessage::noClientErrorLog(const RuleMessage *rm) {
54+
std::string RuleMessage::_errorLogTail(const RuleMessage *rm) {
8555
std::string msg;
8656

87-
msg.append("ModSecurity: Warning. ");
88-
msg.append(rm->m_match);
89-
msg.append(" [file \"" + std::string(rm->m_ruleFile) + "\"]");
90-
msg.append(" [line \"" + std::to_string(rm->m_ruleLine) + "\"]");
91-
msg.append(" [id \"" + std::to_string(rm->m_ruleId) + "\"]");
92-
msg.append(" [rev \"" + rm->m_rev + "\"]");
93-
msg.append(" [msg \"" + rm->m_message + "\"]");
94-
msg.append(" [data \"" + rm->m_data + "\"]");
95-
msg.append(" [severity \"" +
96-
std::to_string(rm->m_severity) + "\"]");
97-
msg.append(" [ver \"" + rm->m_ver + "\"]");
98-
msg.append(" [maturity \"" + std::to_string(rm->m_maturity) + "\"]");
99-
msg.append(" [accuracy \"" + std::to_string(rm->m_accuracy) + "\"]");
100-
for (auto &a : rm->m_tags) {
101-
msg.append(" [tag \"" + a + "\"]");
102-
}
103-
msg.append(" [ref \"" + rm->m_reference + "\"]");
104-
105-
return modsecurity::utils::string::toHexIfNeeded(msg);
106-
}
107-
108-
std::string RuleMessage::errorLogTail(const RuleMessage *rm) {
109-
std::string msg;
110-
111-
msg.append("[hostname \"" + std::string(rm->m_serverIpAddress) \
112-
+ "\"]");
57+
msg.append("[hostname \"" + std::string(rm->m_serverIpAddress) + "\"]");
11358
msg.append(" [uri \"" + rm->m_uriNoQueryStringDecoded + "\"]");
11459
msg.append(" [unique_id \"" + rm->m_id + "\"]");
11560

116-
return modsecurity::utils::string::toHexIfNeeded(msg);
61+
return msg;
11762
}
11863

119-
std::string RuleMessage::errorLog(const RuleMessage *rm) {
120-
std::string msg;
12164

122-
msg.append("[client " + std::string(rm->m_clientIpAddress) + "] ");
123-
msg.append(noClientErrorLog(rm));
124-
msg.append(" " + errorLogTail(rm));
65+
std::string RuleMessage::log(const RuleMessage *rm, int props, int code) {
66+
std::string msg("");
12567

126-
return msg;
127-
}
68+
if (props & ClientLogMessageInfo) {
69+
msg.append("[client " + std::string(rm->m_clientIpAddress) + "] ");
70+
}
12871

129-
std::string RuleMessage::log(const RuleMessage *rm) {
130-
std::string msg("");
131-
if (rm->m_isDisruptive) {
132-
msg.append(disruptiveErrorLog(rm));
133-
} else {
134-
msg.append(errorLog(rm));
72+
if (rm->m_isDisruptive)
73+
{
74+
msg.append("ModSecurity: Access denied with code ");
75+
if (code == -1) {
76+
msg.append("%d");
77+
}
78+
else
79+
{
80+
msg.append(std::to_string(code));
81+
}
82+
msg.append(" (phase ");
83+
msg.append(std::to_string(rm->m_rule->m_phase - 1) + "). ");
84+
}
85+
else
86+
{
87+
msg.append("ModSecurity: Warning. ");
13588
}
13689

137-
return msg;
90+
msg.append(rm->m_match);
91+
msg.append(_details(rm));
92+
93+
if (props & ErrorLogTailLogMessageInfo) {
94+
msg.append(" " + _errorLogTail(rm));
95+
}
96+
97+
return modsecurity::utils::string::toHexIfNeeded(msg);
13898
}
13999

100+
140101
} // namespace modsecurity

‎src/transaction.cc

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1485,11 +1485,7 @@ std::string Transaction::toOldAuditLogFormat(int parts,
14851485
if (parts & audit_log::AuditLog::HAuditLogPart) {
14861486
audit_log << "--" << trailer << "-" << "H--" << std::endl;
14871487
for (auto a : m_rulesMessages) {
1488-
if (a.m_isDisruptive == true) {
1489-
audit_log << a.noClientErrorLog(true) << std::endl;
1490-
} else {
1491-
audit_log << a.noClientErrorLog() << std::endl;
1492-
}
1488+
audit_log << a.log() << std::endl;
14931489
}
14941490
audit_log << std::endl;
14951491
/** TODO: write audit_log H part. */

0 commit comments

Comments
 (0)