From: Juliana Fajardini <jufajardini@oisf.net>
Date: Thu, 28 Jul 2022 23:05:47 +0000 (-0300)
Subject: detect/alert: ensure reject action is applied
X-Git-Tag: suricata-7.0.0-beta1~278
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1f54e8611ab39ce3509280574d137df23c325658;p=thirdparty%2Fsuricata.git

detect/alert: ensure reject action is applied

Bug 5458 states that the reject action is no longer working. While SV
tests that use the reject action still pass, it indeed seems that a
regression has happened with commit aa93984, because while the
function that applies rule actions to the flow (RuleActionToFlow) does
check for the reject action, the newly added function PacketApply
SignatureActions only checks for ACTION_DROP or ACTION_PASS when
deciding to call RuleActionToFlow.

Bug #5458
---

diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c
index f6da8201ca..834b483588 100644
--- a/src/detect-engine-alert.c
+++ b/src/detect-engine-alert.c
@@ -32,6 +32,7 @@
 #endif
 
 #include "util-profiling.h"
+#include "util-validate.h"
 
 /** tag signature we use for tag alerts */
 static Signature g_tag_signature;
@@ -183,7 +184,9 @@ static void PacketApplySignatureActions(Packet *p, const Signature *s, const uin
     SCLogDebug("packet %" PRIu64 " sid %u action %02x alert_flags %02x", p->pcap_cnt, s->id,
             s->action, alert_flags);
 
-    if (s->action & ACTION_DROP) {
+    /* REJECT also sets ACTION_DROP, just make it more visible with this check */
+    if (s->action & (ACTION_DROP | ACTION_REJECT_ANY)) {
+        /* PacketDrop will update the packet action, too */
         PacketDrop(p, s->action, PKT_DROP_REASON_RULES);
 
         if (p->alerts.drop.action == 0) {
@@ -194,6 +197,8 @@ static void PacketApplySignatureActions(Packet *p, const Signature *s, const uin
         if ((p->flow != NULL) && (alert_flags & PACKET_ALERT_FLAG_APPLY_ACTION_TO_FLOW)) {
             RuleActionToFlow(s->action, p->flow);
         }
+
+        DEBUG_VALIDATE_BUG_ON(!PacketTestAction(p, ACTION_DROP));
     } else {
         PacketUpdateAction(p, s->action);
 
@@ -415,4 +420,3 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx
 
 }
 
-
diff --git a/src/detect-engine-threshold.c b/src/detect-engine-threshold.c
index e2a9019abf..2154eecc55 100644
--- a/src/detect-engine-threshold.c
+++ b/src/detect-engine-threshold.c
@@ -300,11 +300,11 @@ static inline void RateFilterSetAction(Packet *p, PacketAlert *pa, uint8_t new_a
             pa->flags |= PACKET_ALERT_RATE_FILTER_MODIFIED;
             break;
         case TH_ACTION_DROP:
-            PacketDrop(p, new_action, PKT_DROP_REASON_RULES_THRESHOLD);
+            PacketDrop(p, ACTION_DROP, PKT_DROP_REASON_RULES_THRESHOLD);
             pa->flags |= PACKET_ALERT_RATE_FILTER_MODIFIED;
             break;
         case TH_ACTION_REJECT:
-            PACKET_REJECT(p);
+            PacketDrop(p, (ACTION_REJECT | ACTION_DROP), PKT_DROP_REASON_RULES_THRESHOLD);
             pa->flags |= PACKET_ALERT_RATE_FILTER_MODIFIED;
             break;
         case TH_ACTION_PASS: