From: Victor Julien Date: Fri, 12 Jan 2024 10:14:27 +0000 (+0100) Subject: detect: set ACTION_ALERT for rules that should alert X-Git-Tag: suricata-7.0.7~79 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=5bcbbf356961b493af1276554079dacfab67d4b9;p=thirdparty%2Fsuricata.git 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 b6d690aff8..29c6ece778 100644 --- 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 9729046b41..212d04d46c 100644 --- 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 4a216cc791..358bd7e0b1 100644 --- 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 764bf62805..97ebd22a68 100644 --- 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 fce16875e4..c0d90eca2f 100644 --- 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 a2e7158bda..5a2f44c97c 100644 --- 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 4fae441481..f6a25663df 100644 --- 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 3ec38d3006..f147d0fda8 100644 --- 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 0a1abda2c5..3295764edf 100644 --- 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 1b24bc5b07..d3ea1b631a 100644 --- 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 5d762a8f70..614031be13 100644 --- 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; }