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.
(cherry picked from commit 92581dbc0669464e2e3ed2b84c8e0695418879c3)

diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c
index b6d690aff85e7627727458335abdad46f353a24b..29c6ece7781c673e001f870708945b050b340d31 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
         }
 
@@ -409,7 +409,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 9729046b41a8d63a3b689281710774530fd18e1a..212d04d46cd08331151fc2119b7926e5ec56bf15 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"
@@ -964,7 +965,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 4a216cc7916198a58e70a0431400b4f9c8852abb..358bd7e0b1d1ef55a8b372b0f0b94c54b07502cf 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"
@@ -288,7 +289,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;
@@ -930,7 +931,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 764bf62805c1c6d85b5fe320ac1988ecd563e8fa..97ebd22a68e8dba5ebed287bc54af368403ddf75 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"
@@ -378,7 +379,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 a2e7158bda142648a1e52768da5f44563c7cf547..5a2f44c97cec507ef72a130595cc3acdca1dc43a 100644 (file)
--- a/src/detect-parse.c
+++ b/src/detect-parse.c
@@ -1215,7 +1215,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 ||
@@ -1223,18 +1223,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 4fae4414819f021fef7d516762483198f3f05636..f6a25663dfa3aad33d12eaed43c167900a2097d5 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"
@@ -341,9 +342,9 @@ int DetectXbitSetup (DetectEngineCtx *de_ctx, Signature *s, const char *rawstr)
     int result = DetectXbitParse(de_ctx, rawstr, &cd);
     if (result < 0) {
         return -1;
-    /* noalert doesn't use a cd/sm struct. It flags the sig. We're done. */
-    } else if (result == 0 && cd == NULL) {
-        s->flags |= SIG_FLAG_NOALERT;
+    } else if (cd == NULL) {
+        /* noalert doesn't use a cd/sm struct. It flags the sig. We're done. */
+        s->action &= ~ACTION_ALERT;
         return 0;
     }
 

diff --git a/src/detect.h b/src/detect.h
index 3ec38d3006b52931b17eeba00c8d03ec24595473..f147d0fda8e71744ad8b6d3356738badb6141725 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 0a1abda2c5486eab88bad2e34aa0d0f340af3b6f..3295764edf547c13e2988b05a220f0d57277587a 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 5d762a8f709182d7efb4990c12f432f8e73bd8c3..614031be13cd61d14cea16cfcb0eb3dfc32654d2 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"
 
@@ -258,7 +259,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;
             }
 
@@ -287,7 +288,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;
             }
 
@@ -319,7 +320,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;
             }