From: Juliana Fajardini <jufajardini@gmail.com>
Date: Tue, 19 Apr 2022 20:43:10 +0000 (-0300)
Subject: detect/alert: preprocess then append alert queue
X-Git-Tag: suricata-5.0.10~43
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5872f7b7dc5d305ac7beeebe831eae56e00f682e;p=thirdparty%2Fsuricata.git

detect/alert: preprocess then append alert queue

Do all alert queue processing before actually appending
the PacketAlerts to the Packet's alert queue.

Adjusted changes to use macro instead of functions, in cases where the
latter didn't exist in this branch.

Task #4943

(cherry picked from commit faea583d9b34cf8f88b9e40f6e8feed6c4d70758)
---

diff --git a/src/decode.h b/src/decode.h
index 87600e5a7d..60949a8203 100644
--- a/src/decode.h
+++ b/src/decode.h
@@ -271,10 +271,10 @@ typedef uint16_t Port;
  * found in this packet */
 typedef struct PacketAlert_ {
     SigIntId num; /* Internal num, used for sorting */
-    uint8_t action; /* Internal num, used for sorting */
+    uint8_t action; /* Internal num, used for thresholding */
     uint8_t flags;
     const struct Signature_ *s;
-    uint64_t tx_id;
+    uint64_t tx_id; /* Used for sorting */
 } PacketAlert;
 
 /* flag to indicate the rule action (drop/pass) needs to be applied to the flow */
diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c
index da905ea674..4b3cd01283 100644
--- a/src/detect-engine-alert.c
+++ b/src/detect-engine-alert.c
@@ -352,6 +352,23 @@ void AlertQueueAppend(DetectEngineThreadCtx *det_ctx, const Signature *s, Packet
     return;
 }
 
+/** \internal
+ * \brief sort helper for sorting alerts by priority
+ *
+ * Sorting is done first based on num and then using tx_id, if nums are equal.
+ * The Signature::num field is set based on internal priority. Higher priority
+ * rules have lower nums.
+ */
+static int AlertQueueSortHelper(const void *a, const void *b)
+{
+    const PacketAlert *pa0 = a;
+    const PacketAlert *pa1 = b;
+    if (pa1->num == pa0->num)
+        return pa0->tx_id < pa1->tx_id ? 1 : -1;
+    else
+        return pa0->num > pa1->num ? 1 : -1;
+}
+
 /**
  * \brief Check the threshold of the sigs that match, set actions, break on pass action
  *        This function iterate the packet alerts array, removing those that didn't match
@@ -364,13 +381,18 @@ void AlertQueueAppend(DetectEngineThreadCtx *det_ctx, const Signature *s, Packet
 void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx, Packet *p)
 {
     SCEnter();
+
+    /* sort the alert queue before thresholding and appending to Packet */
+    qsort(det_ctx->alert_queue, det_ctx->alert_queue_size, sizeof(PacketAlert),
+            AlertQueueSortHelper);
+
     int i = 0;
+    uint16_t max_pos = det_ctx->alert_queue_size;
 
-    while (i < p->alerts.cnt) {
-        const Signature *s = de_ctx->sig_array[p->alerts.alerts[i].num];
-        SCLogDebug("Sig->num: %" PRIu32 " SID %u", p->alerts.alerts[i].num, s->id);
+    while (i < max_pos) {
+        const Signature *s = de_ctx->sig_array[det_ctx->alert_queue[i].num];
+        uint8_t res = PacketAlertHandle(de_ctx, det_ctx, s, p, &det_ctx->alert_queue[i]);
 
-        int res = PacketAlertHandle(de_ctx, det_ctx, s, p, &p->alerts.alerts[i]);
         if (res > 0) {
             /* Now, if we have an alert, we have to check if we want
              * to tag this session or src/dst host */
@@ -393,36 +415,35 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx
              * - match is in applayer
              * - match is in stream */
             if (s->action & (ACTION_DROP | ACTION_PASS)) {
-                if ((p->alerts.alerts[i].flags &
+                if ((det_ctx->alert_queue[i].flags &
                             (PACKET_ALERT_FLAG_STATE_MATCH | PACKET_ALERT_FLAG_STREAM_MATCH)) ||
                         (s->flags & (SIG_FLAG_IPONLY | SIG_FLAG_PDONLY | SIG_FLAG_APPLAYER))) {
-                    p->alerts.alerts[i].flags |= PACKET_ALERT_FLAG_APPLY_ACTION_TO_FLOW;
+                    det_ctx->alert_queue[i].flags |= PACKET_ALERT_FLAG_APPLY_ACTION_TO_FLOW;
                     SCLogDebug("packet %" PRIu64 " sid %u action %02x alert_flags %02x (set "
                                "PACKET_ALERT_FLAG_APPLY_ACTION_TO_FLOW)",
-                            p->pcap_cnt, s->id, s->action, p->alerts.alerts[i].flags);
+                            p->pcap_cnt, s->id, s->action, det_ctx->alert_queue[i].flags);
                 }
             }
 
             /* set actions on packet */
-            PacketApplySignatureActions(p, p->alerts.alerts[i].s, p->alerts.alerts[i].flags);
-
-            if (PACKET_TEST_ACTION(p, ACTION_PASS)) {
-                /* Ok, reset the alert cnt to end in the previous of pass
-                 * so we ignore the rest with less prio */
-                p->alerts.cnt = i;
-                break;
-            }
+            PacketApplySignatureActions(p, s, det_ctx->alert_queue[i].flags);
         }
 
         /* Thresholding removes this alert */
         if (res == 0 || res == 2 || (s->flags & SIG_FLAG_NOALERT)) {
-            PacketAlertRemove(p, i);
+            /* we will not copy this to the AlertQueue */
+        } else if (p->alerts.cnt < packet_alert_max) {
+            p->alerts.alerts[p->alerts.cnt] = det_ctx->alert_queue[i];
+            SCLogDebug("Appending sid %" PRIu32 " alert to Packet::alerts at pos %u", s->id, i);
 
-            if (p->alerts.cnt == 0)
+            if (PACKET_TEST_ACTION(p, ACTION_PASS)) {
+                /* Ok, reset the alert cnt to end in the previous of pass
+                 * so we ignore the rest with less prio */
                 break;
-        } else {
-            i++;
+            }
+            p->alerts.cnt++;
         }
+        i++;
     }
 
     /* At this point, we should have all the new alerts. Now check the tag