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 87600e5a7d7a8e5ccbcf0c6f47b878a5fdf58937..60949a82038ee6fc2a58c374b7c14ee5090fa4fd 100644 (file)
--- 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 da905ea6744a6e72fa1cd232d8046260e50d7547..4b3cd0128361a385fb70a89502fb6973eac31cbd 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