detect: set ACTION_ALERT for rules that should alert

Replaces default "alert" logic and removed SIG_FLAG_NOALERT.

Instead, "noalert" unsets ACTION_ALERT. Same for flowbits:noalert and
friends.

In signature ordering rules w/o action are sorted as if they have 'alert',
which is the same behavior as before, but now implemented explicitly.

Ticket: #5466.

diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c
index bf6a760b88974afcd7b6116dfdf9aa8c5673d9d2..750ed6a8be4f20fd706fd441c98337cc1bff4d68 100644 (file)
--- a/src/detect-engine-alert.c
+++ b/src/detect-engine-alert.c
@@ -208,7 +208,7 @@ static void PacketApplySignatureActions(Packet *p, const Signature *s, const Pac
             // nothing to set in the packet
         } else if (pa->action & (ACTION_ALERT | ACTION_CONFIG)) {
             // nothing to set in the packet
-        } else {
+        } else if (pa->action != 0) {
             DEBUG_VALIDATE_BUG_ON(1); // should be unreachable
         }
 
@@ -408,7 +408,9 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx
         }
 
         /* Thresholding removes this alert */
-        if (res == 0 || res == 2 || (s->flags & SIG_FLAG_NOALERT)) {
+        if (res == 0 || res == 2 || (s->action & (ACTION_ALERT | ACTION_PASS)) == 0) {
+            SCLogDebug("sid:%u: skipping alert because of thresholding (res=%d) or NOALERT (%02x)",
+                    s->id, res, s->action);
             /* we will not copy this to the AlertQueue */
             p->alerts.suppressed++;
         } else if (p->alerts.cnt < packet_alert_max) {

diff --git a/src/detect-engine-analyzer.c b/src/detect-engine-analyzer.c
index 0ce0dbc4e810594559266582404841a9a1a0cea8..8e90f7796af5def5b4fc96f96da72f13a926a75c 100644 (file)
--- a/src/detect-engine-analyzer.c
+++ b/src/detect-engine-analyzer.c
@@ -27,6 +27,7 @@
 #include "suricata-common.h"
 #include "suricata.h"
 #include "rust.h"
+#include "action-globals.h"
 #include "detect.h"
 #include "detect-parse.h"
 #include "detect-engine.h"
@@ -1023,7 +1024,7 @@ void EngineAnalysisRules2(const DetectEngineCtx *de_ctx, const Signature *s)
     if (s->flags & SIG_FLAG_DP_ANY) {
         jb_append_string(ctx.js, "dp_any");
     }
-    if (s->flags & SIG_FLAG_NOALERT) {
+    if ((s->action & ACTION_ALERT) == 0) {
         jb_append_string(ctx.js, "noalert");
     }
     if (s->flags & SIG_FLAG_DSIZE) {

diff --git a/src/detect-flowbits.c b/src/detect-flowbits.c
index dce56625ec16de832040774ffefd30f80a4f8644..40f04d75f305f1b5d47731b6812218a8a4f431e8 100644 (file)
--- a/src/detect-flowbits.c
+++ b/src/detect-flowbits.c
@@ -26,6 +26,7 @@
 
 #include "suricata-common.h"
 #include "decode.h"
+#include "action-globals.h"
 #include "detect.h"
 #include "threads.h"
 #include "flow.h"
@@ -287,7 +288,7 @@ int DetectFlowbitSetup (DetectEngineCtx *de_ctx, Signature *s, const char *rawst
     if (strcmp(fb_cmd_str,"noalert") == 0) {
         if (strlen(fb_name) != 0)
             goto error;
-        s->flags |= SIG_FLAG_NOALERT;
+        s->action &= ~ACTION_ALERT;
         return 0;
     } else if (strcmp(fb_cmd_str,"isset") == 0) {
         fb_cmd = DETECT_FLOWBITS_CMD_ISSET;
@@ -927,7 +928,7 @@ static int FlowBitsTestSig05(void)
 
     s = de_ctx->sig_list = SigInit(de_ctx,"alert ip any any -> any any (msg:\"Noalert\"; flowbits:noalert; content:\"GET \"; sid:1;)");
     FAIL_IF_NULL(s);
-    FAIL_IF((s->flags & SIG_FLAG_NOALERT) != SIG_FLAG_NOALERT);
+    FAIL_IF((s->action & ACTION_ALERT) != 0);
 
     SigGroupBuild(de_ctx);
     DetectEngineCtxFree(de_ctx);

diff --git a/src/detect-hostbits.c b/src/detect-hostbits.c
index 571510325aeaeaeacd9fef2e543e18ac2ce89935..86d93da4f3fe72b54f46c5569ee99871dd11f6f1 100644 (file)
--- a/src/detect-hostbits.c
+++ b/src/detect-hostbits.c
@@ -25,6 +25,7 @@
 
 #include "suricata-common.h"
 #include "decode.h"
+#include "action-globals.h"
 #include "detect.h"
 #include "threads.h"
 #include "flow.h"
@@ -377,7 +378,7 @@ int DetectHostbitSetup (DetectEngineCtx *de_ctx, Signature *s, const char *rawst
         case DETECT_XBITS_CMD_NOALERT:
             if (strlen(fb_name) != 0)
                 goto error;
-            s->flags |= SIG_FLAG_NOALERT;
+            s->action &= ~ACTION_ALERT;
             return 0;
         case DETECT_XBITS_CMD_ISNOTSET:
         case DETECT_XBITS_CMD_ISSET:

diff --git a/src/detect-noalert.c b/src/detect-noalert.c
index fce16875e455b00dd598895a7833025e40f4b1f9..c0d90eca2fe39717662f2cb6160f91755c643d88 100644 (file)
--- a/src/detect-noalert.c
+++ b/src/detect-noalert.c
@@ -24,6 +24,7 @@
  */
 
 #include "suricata-common.h"
+#include "action-globals.h"
 #include "detect.h"
 #include "detect-noalert.h"
 #include "util-debug.h"
@@ -33,7 +34,7 @@ static int DetectNoalertSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 {
     DEBUG_VALIDATE_BUG_ON(nullstr != NULL);
 
-    s->flags |= SIG_FLAG_NOALERT;
+    s->action &= ~ACTION_ALERT;
     return 0;
 }
 

diff --git a/src/detect-parse.c b/src/detect-parse.c
index 74082cf521ada824f5ac8eb5a5e0c4f71ba6dea9..67d68c57cb862da9447484e40214438a13e64f7d 100644 (file)
--- a/src/detect-parse.c
+++ b/src/detect-parse.c
@@ -1202,7 +1202,7 @@ static int SigParseAction(Signature *s, const char *action)
     if (strcasecmp(action, "alert") == 0) {
         s->action = ACTION_ALERT;
     } else if (strcasecmp(action, "drop") == 0) {
-        s->action = ACTION_DROP;
+        s->action = ACTION_DROP | ACTION_ALERT;
     } else if (strcasecmp(action, "pass") == 0) {
         s->action = ACTION_PASS;
     } else if (strcasecmp(action, "reject") == 0 ||
@@ -1210,18 +1210,17 @@ static int SigParseAction(Signature *s, const char *action)
     {
         if (!(SigParseActionRejectValidate(action)))
             return -1;
-        s->action = ACTION_REJECT|ACTION_DROP;
+        s->action = ACTION_REJECT | ACTION_DROP | ACTION_ALERT;
     } else if (strcasecmp(action, "rejectdst") == 0) {
         if (!(SigParseActionRejectValidate(action)))
             return -1;
-        s->action = ACTION_REJECT_DST|ACTION_DROP;
+        s->action = ACTION_REJECT_DST | ACTION_DROP | ACTION_ALERT;
     } else if (strcasecmp(action, "rejectboth") == 0) {
         if (!(SigParseActionRejectValidate(action)))
             return -1;
-        s->action = ACTION_REJECT_BOTH|ACTION_DROP;
+        s->action = ACTION_REJECT_BOTH | ACTION_DROP | ACTION_ALERT;
     } else if (strcasecmp(action, "config") == 0) {
         s->action = ACTION_CONFIG;
-        s->flags |= SIG_FLAG_NOALERT;
     } else {
         SCLogError("An invalid action \"%s\" was given", action);
         return -1;

diff --git a/src/detect-xbits.c b/src/detect-xbits.c
index a3f67dbfc9b77ee06bea5a89cf32553b40ffc95a..31ba97bafc899be9ec6fd6d0caa865d51d14391c 100644 (file)
--- a/src/detect-xbits.c
+++ b/src/detect-xbits.c
@@ -25,6 +25,7 @@
 
 #include "suricata-common.h"
 #include "decode.h"
+#include "action-globals.h"
 #include "detect.h"
 #include "threads.h"
 #include "flow.h"
@@ -342,7 +343,7 @@ int DetectXbitSetup (DetectEngineCtx *de_ctx, Signature *s, const char *rawstr)
         return -1;
     } else if (cd == NULL) {
         /* noalert doesn't use a cd/sm struct. It flags the sig. We're done. */
-        s->flags |= SIG_FLAG_NOALERT;
+        s->action &= ~ACTION_ALERT;
         return 0;
     }
 

diff --git a/src/detect.h b/src/detect.h
index 2e30e19ec52d7a9f91bc106fd9b7b199be37415e..5c1583074f05d86d9d3a70e4e9d5ff0aa7d7d6d8 100644 (file)
--- a/src/detect.h
+++ b/src/detect.h
@@ -240,7 +240,8 @@ typedef struct DetectPort_ {
 #define SIG_FLAG_SP_ANY                 BIT_U32(2)  /**< source port is any */
 #define SIG_FLAG_DP_ANY                 BIT_U32(3)  /**< destination port is any */
 
-#define SIG_FLAG_NOALERT                BIT_U32(4)  /**< no alert flag is set */
+// vacancy
+
 #define SIG_FLAG_DSIZE                  BIT_U32(5)  /**< signature has a dsize setting */
 #define SIG_FLAG_APPLAYER               BIT_U32(6) /**< signature applies to app layer instead of packets */
 

diff --git a/src/packet.c b/src/packet.c
index 07f28b424f9c4ccbeec9a3f8e320e111bde41dae..bd63de7568351e19460b562ac1df1c8bed9efdba 100644 (file)
--- a/src/packet.c
+++ b/src/packet.c
@@ -27,11 +27,11 @@
  *
  *  Set drop (+reject) flags in both current and root packet.
  *
- *  \param action action bit flags. Must be limited to ACTION_DROP_REJECT
+ *  \param action action bit flags. Must be limited to ACTION_DROP_REJECT|ACTION_ALERT
  */
 void PacketDrop(Packet *p, const uint8_t action, enum PacketDropReason r)
 {
-    DEBUG_VALIDATE_BUG_ON((action & ~ACTION_DROP_REJECT) != 0);
+    DEBUG_VALIDATE_BUG_ON((action & ~(ACTION_DROP_REJECT | ACTION_ALERT)) != 0);
 
     if (p->drop_reason == PKT_DROP_REASON_NOT_SET)
         p->drop_reason = (uint8_t)r;

diff --git a/src/util-action.c b/src/util-action.c
index 1b24bc5b0792f203a0ef8440bca5f4e1b1d8768e..d3ea1b631a17a845ae642a290813be8ae28110cd 100644 (file)
--- a/src/util-action.c
+++ b/src/util-action.c
@@ -53,15 +53,22 @@ uint8_t action_order_sigs[4] = {ACTION_PASS, ACTION_DROP, ACTION_REJECT, ACTION_
 uint8_t ActionOrderVal(uint8_t action)
 {
     /* reject_both and reject_dst have the same prio as reject */
-    if( (action & ACTION_REJECT) ||
-        (action & ACTION_REJECT_BOTH) ||
-        (action & ACTION_REJECT_DST)) {
+    if (action & ACTION_REJECT_ANY) {
         action = ACTION_REJECT;
+    } else if (action & ACTION_DROP) {
+        action = ACTION_DROP;
+    } else if (action & ACTION_PASS) {
+        action = ACTION_PASS;
+    } else if (action & ACTION_ALERT) {
+        action = ACTION_ALERT;
+    } else if (action == 0) {
+        action = ACTION_ALERT;
     }
-    uint8_t i = 0;
-    for (; i < 4; i++) {
-        if (action_order_sigs[i] == action)
+
+    for (uint8_t i = 0; i < 4; i++) {
+        if (action_order_sigs[i] == action) {
             return i;
+        }
     }
     /* Unknown action, set just a low prio (high val) */
     return 10;

diff --git a/src/util-threshold-config.c b/src/util-threshold-config.c
index a85faad14febfbfaeba0dfe33de605d5ec784401..d12c89e07f3c321ca0c8e7d3ead3964a7dcba087 100644 (file)
--- a/src/util-threshold-config.c
+++ b/src/util-threshold-config.c
@@ -30,6 +30,7 @@
 
 #include "suricata-common.h"
 
+#include "action-globals.h"
 #include "host.h"
 #include "ippair.h"
 
@@ -254,7 +255,7 @@ static int SetupSuppressRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid,
         for (s = de_ctx->sig_list; s != NULL; s = s->next) {
             /* tag the rule as noalert */
             if (parsed_track == TRACK_RULE) {
-                s->flags |= SIG_FLAG_NOALERT;
+                s->action &= ~ACTION_ALERT;
                 continue;
             }
 
@@ -278,7 +279,7 @@ static int SetupSuppressRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid,
 
             /* tag the rule as noalert */
             if (parsed_track == TRACK_RULE) {
-                s->flags |= SIG_FLAG_NOALERT;
+                s->action &= ~ACTION_ALERT;
                 continue;
             }
 
@@ -304,7 +305,7 @@ static int SetupSuppressRule(DetectEngineCtx *de_ctx, uint32_t id, uint32_t gid,
                     id, gid);
         } else {
             if (parsed_track == TRACK_RULE) {
-                s->flags |= SIG_FLAG_NOALERT;
+                s->action &= ~ACTION_ALERT;
                 goto end;
             }