From 92581dbc0669464e2e3ed2b84c8e0695418879c3 Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 12 Jan 2024 11:14:27 +0100 Subject: [PATCH] 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. --- src/detect-engine-alert.c | 6 ++++-- src/detect-engine-analyzer.c | 3 ++- src/detect-flowbits.c | 5 +++-- src/detect-hostbits.c | 3 ++- src/detect-noalert.c | 3 ++- src/detect-parse.c | 9 ++++----- src/detect-xbits.c | 3 ++- src/detect.h | 3 ++- src/packet.c | 4 ++-- src/util-action.c | 19 +++++++++++++------ src/util-threshold-config.c | 7 ++++--- 11 files changed, 40 insertions(+), 25 deletions(-) diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c index bf6a760b88..750ed6a8be 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 } @@ -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 0ce0dbc4e8..8e90f7796a 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" @@ -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 dce56625ec..40f04d75f3 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" @@ -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 571510325a..86d93da4f3 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" @@ -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 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 74082cf521..67d68c57cb 100644 --- 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 a3f67dbfc9..31ba97bafc 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" @@ -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 2e30e19ec5..5c1583074f 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 07f28b424f..bd63de7568 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 a85faad14f..d12c89e07f 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" @@ -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; } -- 2.47.2