detect: set detect table for non-firewall mode as well

This also exposed a difference between the handling of TD alerts in
firewall vs non-firewall mode. In firewall mode the table/hook is also
part of the alert ordering to make sure actions from packet:td are
applied before app:td. Handle that explicitly for now.

diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c
index 571b8aede88ff1e9032b7913504a9413bf253f9f..6988b2825b226c217f6425c66a16e7797f457824 100644 (file)
--- a/src/detect-engine-alert.c
+++ b/src/detect-engine-alert.c
@@ -192,12 +192,10 @@ static void PacketApplySignatureActions(Packet *p, const Signature *s, const Pac
     /* REJECT also sets ACTION_DROP, just make it more visible with this check */
     if (pa->action & ACTION_DROP_REJECT) {
         uint8_t drop_reason = PKT_DROP_REASON_RULES;
-        if (s->flags & SIG_FLAG_FIREWALL) {
-            if (s->firewall_table == FIREWALL_TABLE_PACKET_PRE_STREAM) {
-                drop_reason = PKT_DROP_REASON_STREAM_PRE_HOOK;
-            } else if (s->firewall_table == FIREWALL_TABLE_PACKET_PRE_FLOW) {
-                drop_reason = PKT_DROP_REASON_FLOW_PRE_HOOK;
-            }
+        if (s->detect_table == DETECT_TABLE_PACKET_PRE_STREAM) {
+            drop_reason = PKT_DROP_REASON_STREAM_PRE_HOOK;
+        } else if (s->detect_table == DETECT_TABLE_PACKET_PRE_FLOW) {
+            drop_reason = PKT_DROP_REASON_FLOW_PRE_HOOK;
         }
 
         /* PacketDrop will update the packet action, too */
@@ -336,11 +334,11 @@ void AlertQueueAppend(DetectEngineThreadCtx *det_ctx, const Signature *s, Packet
  * 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)
+static int AlertQueueSortHelperFirewall(const void *a, const void *b)
 {
     const PacketAlert *pa0 = a;
     const PacketAlert *pa1 = b;
-    if (pa0->s->firewall_table == pa1->s->firewall_table) {
+    if (pa0->s->detect_table == pa1->s->detect_table) {
         if (pa1->iid == pa0->iid) {
             if (pa1->tx_id == PACKET_ALERT_NOTX) {
                 return -1;
@@ -352,7 +350,23 @@ static int AlertQueueSortHelper(const void *a, const void *b)
             return pa0->iid < pa1->iid ? -1 : 1;
         }
     }
-    return pa0->s->firewall_table < pa1->s->firewall_table ? -1 : 1;
+    return pa0->s->detect_table < pa1->s->detect_table ? -1 : 1;
+}
+
+static int AlertQueueSortHelper(const void *a, const void *b)
+{
+    const PacketAlert *pa0 = a;
+    const PacketAlert *pa1 = b;
+    if (pa1->iid == pa0->iid) {
+        if (pa1->tx_id == PACKET_ALERT_NOTX) {
+            return -1;
+        } else if (pa0->tx_id == PACKET_ALERT_NOTX) {
+            return 1;
+        }
+        return pa0->tx_id < pa1->tx_id ? 1 : -1;
+    } else {
+        return pa0->iid < pa1->iid ? -1 : 1;
+    }
 }
 
 /** \internal
@@ -400,7 +414,8 @@ static inline void PacketAlertFinalizeProcessQueue(
     if (det_ctx->alert_queue_size > 1) {
         /* sort the alert queue before thresholding and appending to Packet */
         qsort(det_ctx->alert_queue, det_ctx->alert_queue_size, sizeof(PacketAlert),
-                AlertQueueSortHelper);
+                (de_ctx->flags & DE_HAS_FIREWALL) ? AlertQueueSortHelperFirewall
+                                                  : AlertQueueSortHelper);
     }
 
     bool dropped = false;

diff --git a/src/detect-engine.c b/src/detect-engine.c
index 999c96c002c17cd000eefa921886fb4a842d5600..42b28f711af9fa6c1ca2aefdf5a2fe1a2c92a55d 100644 (file)
--- a/src/detect-engine.c
+++ b/src/detect-engine.c
@@ -126,22 +126,22 @@ const struct SignatureProperties signature_properties[SIG_TYPE_MAX] = {
 // rule types documentation tag end: SignatureProperties
 // clang-format on
 
-const char *DetectTableToString(enum FirewallTable table)
+const char *DetectTableToString(enum DetectTable table)
 {
     switch (table) {
-        case FIREWALL_TABLE_NOT_SET:
+        case DETECT_TABLE_NOT_SET:
             return "not_set";
-        case FIREWALL_TABLE_PACKET_PRE_FLOW:
+        case DETECT_TABLE_PACKET_PRE_FLOW:
             return "pre_flow";
-        case FIREWALL_TABLE_PACKET_PRE_STREAM:
+        case DETECT_TABLE_PACKET_PRE_STREAM:
             return "pre_stream";
-        case FIREWALL_TABLE_PACKET_FILTER:
+        case DETECT_TABLE_PACKET_FILTER:
             return "packet_filter";
-        case FIREWALL_TABLE_PACKET_TD:
+        case DETECT_TABLE_PACKET_TD:
             return "packet_td";
-        case FIREWALL_TABLE_APP_FILTER:
+        case DETECT_TABLE_APP_FILTER:
             return "app_filter";
-        case FIREWALL_TABLE_APP_TD:
+        case DETECT_TABLE_APP_TD:
             return "app_td";
         default:
             return "unknown";

diff --git a/src/detect-engine.h b/src/detect-engine.h
index 4d22b8075b412acaf12bb8c038f75a906b57ff9c..f6b233ba03e97435347a434e74d04a7e8c562df6 100644 (file)
--- a/src/detect-engine.h
+++ b/src/detect-engine.h
@@ -27,7 +27,7 @@
 #include "detect.h"
 #include "suricata.h"
 
-const char *DetectTableToString(enum FirewallTable table);
+const char *DetectTableToString(enum DetectTable table);
 
 /* start up registry funcs */
 

diff --git a/src/detect-parse.c b/src/detect-parse.c
index bb304e2f7aeeda39cfed2e15c3012954942c101e..47bd49d563719e104951194fe788183663c2c3fc 100644 (file)
--- a/src/detect-parse.c
+++ b/src/detect-parse.c
@@ -2460,16 +2460,16 @@ static void SigSetupPrefilter(DetectEngineCtx *de_ctx, Signature *s)
  */
 static bool DetectRuleValidateTable(const Signature *s)
 {
-    if (s->firewall_table == 0)
+    if (s->detect_table == 0)
         return true;
 
-    const uint8_t table_as_flag = BIT_U8(s->firewall_table);
+    const uint8_t table_as_flag = BIT_U8(s->detect_table);
 
     for (SigMatch *sm = s->init_data->smlists[DETECT_SM_LIST_MATCH]; sm != NULL; sm = sm->next) {
         const uint8_t kw_tables_supported = sigmatch_table[sm->type].tables;
         if (kw_tables_supported != 0 && (kw_tables_supported & table_as_flag) == 0) {
             SCLogError("rule %u uses hook \"%s\", but keyword \"%s\" doesn't support this hook",
-                    s->id, DetectTableToString(s->firewall_table), sigmatch_table[sm->type].name);
+                    s->id, DetectTableToString(s->detect_table), sigmatch_table[sm->type].name);
             return false;
         }
     }
@@ -2487,33 +2487,34 @@ static bool DetectFirewallRuleValidate(const DetectEngineCtx *de_ctx, const Sign
     return true;
 }
 
-static void DetectFirewallRuleSetTable(Signature *s)
+static void DetectRuleSetTable(Signature *s)
 {
-    enum FirewallTable table;
+    enum DetectTable table;
     if (s->flags & SIG_FLAG_FIREWALL) {
         if (s->type == SIG_TYPE_PKT) {
             if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_PKT &&
                     s->init_data->hook.t.pkt.ph == SIGNATURE_HOOK_PKT_PRE_STREAM)
-                table = FIREWALL_TABLE_PACKET_PRE_STREAM;
+                table = DETECT_TABLE_PACKET_PRE_STREAM;
             else if (s->init_data->hook.type == SIGNATURE_HOOK_TYPE_PKT &&
                      s->init_data->hook.t.pkt.ph == SIGNATURE_HOOK_PKT_PRE_FLOW)
-                table = FIREWALL_TABLE_PACKET_PRE_FLOW;
+                table = DETECT_TABLE_PACKET_PRE_FLOW;
             else
-                table = FIREWALL_TABLE_PACKET_FILTER;
+                table = DETECT_TABLE_PACKET_FILTER;
         } else if (s->type == SIG_TYPE_APP_TX) {
-            table = FIREWALL_TABLE_APP_FILTER;
+            table = DETECT_TABLE_APP_FILTER;
         } else {
             BUG_ON(1);
         }
     } else {
+        // TODO pre_flow/pre_stream
         if (s->type != SIG_TYPE_APP_TX) {
-            table = FIREWALL_TABLE_PACKET_TD;
+            table = DETECT_TABLE_PACKET_TD;
         } else {
-            table = FIREWALL_TABLE_APP_TD;
+            table = DETECT_TABLE_APP_TD;
         }
     }
 
-    s->firewall_table = (uint8_t)table;
+    s->detect_table = (uint8_t)table;
 }
 
 static int SigValidateFirewall(const DetectEngineCtx *de_ctx, const Signature *s)
@@ -2829,9 +2830,7 @@ static int SigValidateConsolidate(
     SigConsolidateTcpBuffer(s);
 
     SignatureSetType(de_ctx, s);
-    if (de_ctx->flags & DE_HAS_FIREWALL) {
-        DetectFirewallRuleSetTable(s);
-    }
+    DetectRuleSetTable(s);
 
     int r = SigValidateFileHandling(s);
     if (r == 0) {

diff --git a/src/detect.h b/src/detect.h
index d6370ca603dc04d969abbd65c216641b7539c66f..dca1a0e16f771c957ccb3ffe94646ee0077f1d9b 100644 (file)
--- a/src/detect.h
+++ b/src/detect.h
@@ -550,22 +550,21 @@ enum SignatureHookType {
     SIGNATURE_HOOK_TYPE_APP,
 };
 
-// TODO should probably be renamed to DetectTable, similar for values
-enum FirewallTable {
-    FIREWALL_TABLE_NOT_SET = 0,
-    FIREWALL_TABLE_PACKET_PRE_FLOW,
-    FIREWALL_TABLE_PACKET_PRE_STREAM,
-    FIREWALL_TABLE_PACKET_FILTER,
-    FIREWALL_TABLE_PACKET_TD,
-    FIREWALL_TABLE_APP_FILTER,
-    FIREWALL_TABLE_APP_TD,
-
-#define DETECT_TABLE_PACKET_PRE_FLOW_FLAG   BIT_U8(FIREWALL_TABLE_PACKET_PRE_FLOW)
-#define DETECT_TABLE_PACKET_PRE_STREAM_FLAG BIT_U8(FIREWALL_TABLE_PACKET_PRE_STREAM)
-#define DETECT_TABLE_PACKET_FILTER_FLAG     BIT_U8(FIREWALL_TABLE_PACKET_FILTER)
-#define DETECT_TABLE_PACKET_TD_FLAG         BIT_U8(FIREWALL_TABLE_PACKET_TD)
-#define DETECT_TABLE_APP_FILTER_FLAG        BIT_U8(FIREWALL_TABLE_APP_FILTER)
-#define DETECT_TABLE_APP_TD_FLAG            BIT_U8(FIREWALL_TABLE_APP_TD)
+enum DetectTable {
+    DETECT_TABLE_NOT_SET = 0,
+    DETECT_TABLE_PACKET_PRE_FLOW,
+    DETECT_TABLE_PACKET_PRE_STREAM,
+    DETECT_TABLE_PACKET_FILTER,
+    DETECT_TABLE_PACKET_TD,
+    DETECT_TABLE_APP_FILTER,
+    DETECT_TABLE_APP_TD,
+
+#define DETECT_TABLE_PACKET_PRE_FLOW_FLAG   BIT_U8(DETECT_TABLE_PACKET_PRE_FLOW)
+#define DETECT_TABLE_PACKET_PRE_STREAM_FLAG BIT_U8(DETECT_TABLE_PACKET_PRE_STREAM)
+#define DETECT_TABLE_PACKET_FILTER_FLAG     BIT_U8(DETECT_TABLE_PACKET_FILTER)
+#define DETECT_TABLE_PACKET_TD_FLAG         BIT_U8(DETECT_TABLE_PACKET_TD)
+#define DETECT_TABLE_APP_FILTER_FLAG        BIT_U8(DETECT_TABLE_APP_FILTER)
+#define DETECT_TABLE_APP_TD_FLAG            BIT_U8(DETECT_TABLE_APP_TD)
 };
 
 // dns:request_complete should add DetectBufferTypeGetByName("dns:request_complete");
@@ -700,8 +699,8 @@ typedef struct Signature_ {
     /** classification id **/
     uint16_t class_id;
 
-    /** firewall: pseudo table this rule is part of (enum FirewallTable) */
-    uint8_t firewall_table;
+    /** detect: pseudo table this rule is part of (enum DetectTable) */
+    uint8_t detect_table;
 
     /** firewall: progress value for this signature */
     uint8_t app_progress_hook;