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 185b43edff7f3f9db0c919663eb02ceb49787a8f)

diff --git a/src/decode.h b/src/decode.h
index 82bdfd567dc342dd28d6a0a1145cf1a2e791f0b8..2506a1d4b8043aec8c888d5138ce4776cc8ff3a5 100644 (file)
--- a/src/decode.h
+++ b/src/decode.h
@@ -275,10 +275,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 4634afc192abe04d8bcabcb9048dc20f2d6bc668..8d8a062474475a973c95d2b17ae3570f324d6239 100644 (file)
--- 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