From: Victor Julien <vjulien@oisf.net>
Date: Fri, 11 Apr 2025 04:43:48 +0000 (+0200)
Subject: firewall: detect: set per rule table
X-Git-Tag: suricata-8.0.0-rc1~470
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e6bd69b4190e0f441c39f1d71c8d58771bc66437;p=thirdparty%2Fsuricata.git

firewall: detect: set per rule table

For firewall mode, set the pseudo table in the rule and use this
in alert queue ordering, so that rule actions are applied in the
expected order:

        packet:filter -> packet:td -> app:filter -> app:td

This makes sure that a packet:td drop is applied before a app:filter
accept.
---

diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c
index 0cb10012c3..d803e28fd8 100644
--- a/src/detect-engine-alert.c
+++ b/src/detect-engine-alert.c
@@ -330,15 +330,19 @@ static int AlertQueueSortHelper(const void *a, const void *b)
 {
     const PacketAlert *pa0 = a;
     const PacketAlert *pa1 = b;
-    if (pa1->num == pa0->num) {
-        if (pa1->tx_id == PACKET_ALERT_NOTX) {
-            return -1;
-        } else if (pa0->tx_id == PACKET_ALERT_NOTX) {
-            return 1;
+    if (pa0->s->firewall_table == pa1->s->firewall_table) {
+        if (pa1->num == pa0->num) {
+            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->num < pa1->num ? -1 : 1;
         }
-        return pa0->tx_id < pa1->tx_id ? 1 : -1;
     }
-    return pa0->num > pa1->num ? 1 : -1;
+    return pa0->s->firewall_table < pa1->s->firewall_table ? -1 : 1;
 }
 
 /** \internal
diff --git a/src/detect-engine-loader.c b/src/detect-engine-loader.c
index 7b63b29b40..f58433bdf1 100644
--- a/src/detect-engine-loader.c
+++ b/src/detect-engine-loader.c
@@ -293,6 +293,7 @@ static int LoadFirewallRuleFiles(DetectEngineCtx *de_ctx)
         int32_t skipped_sigs = 0;
 
         SCLogNotice("fw: rule file full path \"%s\"", de_ctx->firewall_rule_file_exclusive);
+        de_ctx->flags |= DE_HAS_FIREWALL;
 
         int ret = DetectLoadSigFile(de_ctx, de_ctx->firewall_rule_file_exclusive, &good_sigs,
                 &bad_sigs, &skipped_sigs, true);
@@ -304,8 +305,6 @@ static int LoadFirewallRuleFiles(DetectEngineCtx *de_ctx)
             exit(EXIT_FAILURE);
         }
 
-        de_ctx->flags |= DE_HAS_FIREWALL;
-
         if (good_sigs == 0) {
             SCLogNotice("fw: No rules loaded from %s.", de_ctx->firewall_rule_file_exclusive);
         } else {
diff --git a/src/detect-parse.c b/src/detect-parse.c
index 009106f13e..36ba235a32 100644
--- a/src/detect-parse.c
+++ b/src/detect-parse.c
@@ -2451,6 +2451,28 @@ static bool DetectFirewallRuleValidate(const DetectEngineCtx *de_ctx, const Sign
     return true;
 }
 
+static void DetectFirewallRuleSetTable(Signature *s)
+{
+    enum FirewallTable table;
+    if (s->flags & SIG_FLAG_FIREWALL) {
+        if (s->type == SIG_TYPE_PKT) {
+            table = FIREWALL_TABLE_PACKET_FILTER;
+        } else if (s->type == SIG_TYPE_APP_TX) {
+            table = FIREWALL_TABLE_APP_FILTER;
+        } else {
+            BUG_ON(1);
+        }
+    } else {
+        if (s->type != SIG_TYPE_APP_TX) {
+            table = FIREWALL_TABLE_PACKET_TD;
+        } else {
+            table = FIREWALL_TABLE_APP_TD;
+        }
+    }
+
+    s->firewall_table = (uint8_t)table;
+}
+
 /**
  *  \internal
  *  \brief validate a just parsed signature for internal inconsistencies
@@ -2869,6 +2891,10 @@ static Signature *SigInitHelper(
     /* check what the type of this sig is */
     SignatureSetType(de_ctx, sig);
 
+    if (de_ctx->flags & DE_HAS_FIREWALL) {
+        DetectFirewallRuleSetTable(sig);
+    }
+
     if (sig->type == SIG_TYPE_IPONLY) {
         /* For IPOnly */
         if (IPOnlySigParseAddress(de_ctx, sig, parser.src, SIG_DIREC_SRC ^ dir) < 0)
diff --git a/src/detect.h b/src/detect.h
index f424d40153..b65cdaee48 100644
--- a/src/detect.h
+++ b/src/detect.h
@@ -560,6 +560,13 @@ enum SignatureHookType {
     SIGNATURE_HOOK_TYPE_APP,
 };
 
+enum FirewallTable {
+    FIREWALL_TABLE_PACKET_FILTER,
+    FIREWALL_TABLE_PACKET_TD,
+    FIREWALL_TABLE_APP_FILTER,
+    FIREWALL_TABLE_APP_TD,
+};
+
 // dns:request_complete should add DetectBufferTypeGetByName("dns:request_complete");
 // TODO to json
 typedef struct SignatureHook_ {
@@ -692,6 +699,9 @@ typedef struct Signature_ {
     /** classification id **/
     uint16_t class_id;
 
+    /** firewall: pseudo table this rule is part of (enum FirewallTable) */
+    uint8_t firewall_table;
+
     /** firewall: progress value for this signature */
     uint8_t app_progress_hook;