Adjustments on the error log

i/o operations are expensive. Making sure that they were not
being executed/printed more than necessary.

Author: zimmerle

src/rule_with_actions.cc

@@ -236,10 +236,12 @@ void RuleWithActions::executeActionsAfterFullMatch(Transaction *trans) const {
         }
     }
     if (!disruptiveAlreadyExecuted && m_actionDisruptiveAction != nullptr) {
+        trans->messageGetLast()->setRule(this);
         executeAction(trans,
             m_actionDisruptiveAction.get(), false);
     } else if  (!disruptiveAlreadyExecuted && hasBlockAction()
         && m_defaultActions.m_actionDisruptiveAction != nullptr) {
+        trans->messageGetLast()->setRule(this);
         executeAction(trans,
             m_defaultActions.m_actionDisruptiveAction.get(), false);
     }

src/rule_with_actions.h

@@ -294,25 +294,27 @@ class RuleWithActions : public Rule, public RuleWithActionsProperties {
             return false;
         }
 
+        if (!hasDisruptiveAction() && !hasBlockAction()) {
+            return false;
+        }
+
         return true;
     }
 
-
     inline bool isItToBeAuditLogged() const noexcept {
-        if (!isItToBeLogged() && !m_containsAuditLogAction
-            && !m_defaultActions.m_containsAuditLogAction) {
-            return false;
+        if (m_containsAuditLogAction) {
+            return true;
         }
 
-        if (m_containsNoAuditLogAction) {
-            return false;
+        if (m_defaultActions.m_containsAuditLogAction && !m_containsNoAuditLogAction) {
+            return true;
         }
 
-        if (m_defaultActions.m_containsAuditLogAction && !m_containsAuditLogAction) {
-            return false;
+        if (isItToBeLogged()) {
+            return true;
         }
 
-        return true;
+        return false;
     }
 
 

src/transaction.cc

@@ -72,12 +72,12 @@ void TransactionRuleMessageManagement::logMatchLastRuleOnTheChain(const RuleWith
 
     rm->setRule(rule);
 
-    if (rule->hasDisruptiveAction() && rule->isItToBeLogged() &&
-        (m_transaction->getRuleEngineState() == RulesSet::DetectionOnlyRuleEngine)) {
+    if (rule->isItToBeLogged() &&
+        (m_transaction->getRuleEngineState() == RulesSet::EnabledRuleEngine)) {
         /* error */
         // The error goes over the disruptive massage. We don't need it here.
         //m_transaction->serverLog(rm);
-    } else if (rule->hasBlockAction() && rule->isItToBeLogged()) {
+    } else if (rule->isItToBeLogged()) {
         /* Log as warning. */
         m_transaction->serverLog(rm);
     }

test/test-cases/regression/config-secdefaultaction.json

@@ -277,7 +277,13 @@
     },
     "rules":[
       "SecRuleEngine On",
-      "SecAuditEngine On",
+      "SecAuditEngine RelevantOnly",
+      "SecAuditLogParts ABCFHZ",
+      "SecAuditLog /tmp/test/modsec_audit_auditlog_1.log",
+      "SecAuditLogDirMode 0766",
+      "SecAuditLogFileMode 0666",
+      "SecAuditLogType Serial",
+      "SecAuditLogRelevantStatus \"^(?:3|4(?!04))\"",
       "SecDefaultAction \"phase:2,log,auditlog,status:302,redirect:'http://www.google.com'\"",
       "SecRule REQUEST_HEADERS \"@contains PHPSESSID\" \"phase:2,id:1,block\"",
       "SecRule TX \"@contains to_test\" \"id:2,t:lowercase,t:none,block\""

test/test-cases/regression/issue-1528.json

@@ -27,12 +27,13 @@
   },
   "expected": {
     "debug_log": "Rule returned 1",
-    "error_log": "Matched \"Operator `Rx' with parameter `\\^attack\\$'"
+    "error_log": "Matched \"Operator `Rx' with parameter `\\^attack\\$'",
+    "http_code": 403
   },
   "rules": [
     "SecRuleEngine On",
     "SecAction \"id:1, setvar:tx.bad_value=attack\"",
-    "SecRule ARGS:param \"@rx ^%{tx.bad_value}$\" \"id:2,log\""
+    "SecRule ARGS:param \"@rx ^%{tx.bad_value}$\" \"id:2,log,deny\""
   ]
 }
 ]

test/test-cases/regression/issue-1844.json

@@ -85,10 +85,12 @@
       ]
     },
     "expected":{
-      "error_log":"line \"55\""
+      "error_log":"line \"55\"",
+      "http_code": 403
     },
     "rules":[
       "SecRuleEngine On",
+      "SecDefaultAction \"phase:2,deny\"",
       "SecRule WEBAPPID \"@contains test2\" \"id:1,phase:3,pass,t:trim\"",
       "Include test-cases/data/big-file.conf"
     ]