Avoid passing RuleMessage by std::shared_ptr and use a reference instead.

- Avoids copying std::shared_ptr when lifetime of the RuleMessage
  is controlled by the caller.
  - The RuleMessage instance is created in RuleWithActions::evaluate and
    then used to call the overloaded version of this method that is
    specialized by subclasses.
  - Once the call to the overloaded method returns, the std::shared_ptr
    is destroyed as it's not stored by any of the callers, so it can
    be replaced with a stack variable and avoid paying the cost of
    copying the std::shared_ptr (and its control block that is
    guaranteed to be thread-safe and thus is not a straightforward
    pointer copy)
- Introduced RuleMessage::reset because this is required by
  RuleWithActions::performLogging when it's not the 'last log', the rule
  has multimatch and it's to be logged.
  - The current version is creating allocating another instance of
    RuleMessage on the heap to copy the Rule & Transaction related state
    while all the other members in the RuleMessage are set to their
    default values.
  - The new version leverages the existent, unused and incomplete
    function 'clean' (renamed as 'reset') to do this on the current
    instance.
    - Notice that the current code preserves the value of m_saveMessage,
      so 'reset' provides an argument for the caller to control whether
      this member should be reinitialized.
This commit is contained in:
eduar-hte
2024-05-06 01:24:52 +00:00
committed by Eduardo Arias
parent e313ac7de7
commit 4df297b596
86 changed files with 217 additions and 241 deletions

View File

@@ -27,11 +27,10 @@ namespace modsecurity {
namespace actions {
bool AuditLog::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
rm->m_noAuditLog = false;
bool AuditLog::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
ruleMessage.m_noAuditLog = false;
ms_dbg_a(transaction, 9, "Saving transaction to logs");
rm->m_saveMessage = true;
ruleMessage.m_saveMessage = true;
return true;
}

View File

@@ -35,8 +35,7 @@ class AuditLog : public Action {
explicit AuditLog(const std::string &action)
: Action(action) { }
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
};

View File

@@ -29,15 +29,14 @@ namespace modsecurity {
namespace actions {
bool Block::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
bool Block::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
ms_dbg_a(transaction, 8, "Marking request as disruptive.");
for (auto &a : transaction->m_rules->m_defaultActions[rule->getPhase()]) {
if (a->isDisruptive() == false) {
continue;
}
a->evaluate(rule, transaction, rm);
a->evaluate(rule, transaction, ruleMessage);
}
return true;

View File

@@ -35,8 +35,7 @@ class Block : public Action {
public:
explicit Block(const std::string &action) : Action(action) { }
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
};

View File

@@ -39,7 +39,7 @@ bool Status::init(std::string *error) {
bool Status::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
RuleMessage &ruleMessage) {
transaction->m_it.status = m_status;
return true;
}

View File

@@ -37,8 +37,7 @@ class Status : public Action {
: Action(action), m_status(0) { }
bool init(std::string *error) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
int m_status;
};

View File

@@ -29,7 +29,7 @@ namespace disruptive {
bool Deny::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
RuleMessage &ruleMessage) {
ms_dbg_a(transaction, 8, "Running action deny");
if (transaction->m_it.status == 200) {
@@ -38,9 +38,9 @@ bool Deny::evaluate(RuleWithActions *rule, Transaction *transaction,
transaction->m_it.disruptive = true;
intervention::freeLog(&transaction->m_it);
rm->m_isDisruptive = true;
ruleMessage.m_isDisruptive = true;
transaction->m_it.log = strdup(
rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
ruleMessage.log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
return true;
}

View File

@@ -33,8 +33,7 @@ class Deny : public Action {
public:
explicit Deny(const std::string &action) : Action(action) { }
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
bool isDisruptive() override { return true; }
};

View File

@@ -33,7 +33,7 @@ namespace disruptive {
bool Drop::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
RuleMessage &ruleMessage) {
ms_dbg_a(transaction, 8, "Running action drop " \
"[executing deny instead of drop.]");
@@ -43,9 +43,9 @@ bool Drop::evaluate(RuleWithActions *rule, Transaction *transaction,
transaction->m_it.disruptive = true;
intervention::freeLog(&transaction->m_it);
rm->m_isDisruptive = true;
ruleMessage.m_isDisruptive = true;
transaction->m_it.log = strdup(
rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
ruleMessage.log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
return true;
}

View File

@@ -32,8 +32,7 @@ class Drop : public Action {
public:
explicit Drop(const std::string &action) : Action(action) { }
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
bool isDisruptive() override { return true; }
};

View File

@@ -30,7 +30,7 @@ namespace disruptive {
bool Pass::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
RuleMessage &ruleMessage) {
intervention::free(&transaction->m_it);
intervention::reset(&transaction->m_it);

View File

@@ -31,8 +31,7 @@ class Pass : public Action {
public:
explicit Pass(const std::string &action) : Action(action) { }
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
bool isDisruptive() override { return true; }
};

View File

@@ -35,7 +35,7 @@ bool Redirect::init(std::string *error) {
bool Redirect::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
RuleMessage &ruleMessage) {
std::string m_urlExpanded(m_string->evaluate(transaction));
/* if it was changed before, lets keep it. */
if (transaction->m_it.status == 200
@@ -47,9 +47,9 @@ bool Redirect::evaluate(RuleWithActions *rule, Transaction *transaction,
transaction->m_it.url = strdup(m_urlExpanded.c_str());
transaction->m_it.disruptive = true;
intervention::freeLog(&transaction->m_it);
rm->m_isDisruptive = true;
ruleMessage.m_isDisruptive = true;
transaction->m_it.log = strdup(
rm->log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
ruleMessage.log(RuleMessage::LogMessageInfo::ClientLogMessageInfo).c_str());
return true;
}

View File

@@ -46,8 +46,7 @@ class Redirect : public Action {
m_status(0),
m_string(std::move(z)) { }
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
bool init(std::string *error) override;
bool isDisruptive() override { return true; }

View File

@@ -28,10 +28,9 @@ namespace modsecurity {
namespace actions {
bool Log::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
bool Log::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
ms_dbg_a(transaction, 9, "Saving transaction to logs");
rm->m_saveMessage = true;
ruleMessage.m_saveMessage = true;
return true;
}

View File

@@ -33,8 +33,7 @@ class Log : public Action {
explicit Log(const std::string &action)
: Action(action) { }
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
};
} // namespace actions

View File

@@ -29,9 +29,8 @@ namespace modsecurity {
namespace actions {
bool LogData::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
rm->m_data = data(transaction);
bool LogData::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
ruleMessage.m_data = data(transaction);
return true;
}

View File

@@ -39,8 +39,7 @@ class LogData : public Action {
: Action("logdata"),
m_string(std::move(z)) { }
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
std::string data(Transaction *Transaction);

View File

@@ -46,10 +46,9 @@ namespace modsecurity {
namespace actions {
bool Msg::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
std::string msg = data(transaction);
rm->m_message = msg;
bool Msg::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
const auto msg = data(transaction);
ruleMessage.m_message = msg;
ms_dbg_a(transaction, 9, "Saving msg: " + msg);
return true;

View File

@@ -40,8 +40,7 @@ class Msg : public Action {
: Action("msg"),
m_string(std::move(z)) { }
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
std::string data(Transaction *Transaction);
std::unique_ptr<RunTimeString> m_string;

View File

@@ -26,10 +26,9 @@ namespace modsecurity {
namespace actions {
bool NoAuditLog::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
rm->m_noAuditLog = true;
rm->m_saveMessage = false;
bool NoAuditLog::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
ruleMessage.m_noAuditLog = true;
ruleMessage.m_saveMessage = false;
return true;
}

View File

@@ -35,8 +35,7 @@ class NoAuditLog : public Action {
explicit NoAuditLog(const std::string &action)
: Action(action) { }
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
};
} // namespace actions

View File

@@ -29,9 +29,8 @@ namespace modsecurity {
namespace actions {
bool NoLog::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
rm->m_saveMessage = false;
bool NoLog::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
ruleMessage.m_saveMessage = false;
return true;
}

View File

@@ -33,8 +33,7 @@ class NoLog : public Action {
explicit NoLog(const std::string &action)
: Action(action) { }
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
};
} // namespace actions

View File

@@ -71,13 +71,12 @@ bool Severity::init(std::string *error) {
}
bool Severity::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
bool Severity::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
ms_dbg_a(transaction, 9, "This rule severity is: " + \
std::to_string(this->m_severity) + " current transaction is: " + \
std::to_string(transaction->m_highestSeverityAction));
rm->m_severity = m_severity;
ruleMessage.m_severity = m_severity;
if (transaction->m_highestSeverityAction > this->m_severity) {
transaction->m_highestSeverityAction = this->m_severity;

View File

@@ -35,8 +35,7 @@ class Severity : public Action {
: Action(action),
m_severity(0) { }
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
bool init(std::string *error) override;
int m_severity;

View File

@@ -57,12 +57,11 @@ std::string Tag::getName(Transaction *transaction) {
}
bool Tag::evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) {
bool Tag::evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) {
std::string tag = getName(transaction);
ms_dbg_a(transaction, 9, "Rule tag: " + tag);
rm->m_tags.push_back(tag);
ruleMessage.m_tags.push_back(tag);
return true;
}

View File

@@ -38,8 +38,7 @@ class Tag : public Action {
std::string getName(Transaction *transaction);
bool evaluate(RuleWithActions *rule, Transaction *transaction,
std::shared_ptr<RuleMessage> rm) override;
bool evaluate(RuleWithActions *rule, Transaction *transaction, RuleMessage &ruleMessage) override;
protected:
std::unique_ptr<RunTimeString> m_string;