]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
detect/alert: ensure reject action is applied
authorJuliana Fajardini <jufajardini@oisf.net>
Thu, 28 Jul 2022 23:05:47 +0000 (20:05 -0300)
committerJuliana Fajardini <jufajardini@oisf.net>
Wed, 31 Aug 2022 16:19:41 +0000 (13:19 -0300)
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

(cherry picked from commit 1f54e8611ab39ce3509280574d137df23c325658)

src/detect-engine-alert.c
src/detect-engine-threshold.c

index ec362932e0b0d9dfbfa494b8a6012f4d95294047..bff6078aa5543e4ca5e4d9c98099ddd028b3a5a4 100644 (file)
@@ -28,6 +28,7 @@
 #include "flow-private.h"
 
 #include "util-profiling.h"
+#include "util-validate.h"
 
 /** tag signature we use for tag alerts */
 static Signature g_tag_signature;
@@ -179,7 +180,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) {
@@ -190,6 +193,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 {
         PACKET_UPDATE_ACTION(p, s->action);
 
@@ -402,4 +407,3 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx
 
 }
 
-
index 8400c93912609b0ed82a8ddd6b8bf4a97dd1470d..16e77b2b0dbff44bc2df33ee4ecd77e16557ceda 100644 (file)
@@ -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: