From e072e70ea6b2be7ae335a08c835b6cb61f6ae2ef Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Tue, 18 Oct 2016 18:09:39 +0200 Subject: [PATCH] alert: fix rate_filter issues Fix rate_filter issues: if action was modified it wouldn't be logged in EVE. To address this pass the PacketAlert structure to the threshold code so it can flag the PacketAlert as modified. Use this in logging. Update API to use const where possible. Fix a timout issue that this uncovered. --- src/decode.h | 2 + src/detect-engine-address.c | 2 +- src/detect-engine-address.h | 2 +- src/detect-engine-alert.c | 17 +++-- src/detect-engine-threshold.c | 126 +++++++++++++--------------------- src/detect-engine-threshold.h | 6 +- src/output-json-alert.c | 16 +++-- 7 files changed, 74 insertions(+), 97 deletions(-) diff --git a/src/decode.h b/src/decode.h index 9c6420389b..c4b686322c 100644 --- a/src/decode.h +++ b/src/decode.h @@ -274,6 +274,8 @@ typedef struct PacketAlert_ { #define PACKET_ALERT_FLAG_STREAM_MATCH 0x04 /** alert is in a tx, tx_id set */ #define PACKET_ALERT_FLAG_TX 0x08 +/** action was changed by rate_filter */ +#define PACKET_ALERT_RATE_FILTER_MODIFIED 0x10 #define PACKET_ALERT_MAX 15 diff --git a/src/detect-engine-address.c b/src/detect-engine-address.c index 60964cd9c8..1a277c6e17 100644 --- a/src/detect-engine-address.c +++ b/src/detect-engine-address.c @@ -1897,7 +1897,7 @@ void DetectAddressPrint(DetectAddress *gr) * \retval g On success pointer to an DetectAddress if we find a match * for the Address "a", in the DetectAddressHead "gh". */ -DetectAddress *DetectAddressLookupInHead(DetectAddressHead *gh, Address *a) +DetectAddress *DetectAddressLookupInHead(const DetectAddressHead *gh, Address *a) { SCEnter(); diff --git a/src/detect-engine-address.h b/src/detect-engine-address.h index dcbde2e427..8c5d1256f0 100644 --- a/src/detect-engine-address.h +++ b/src/detect-engine-address.h @@ -44,7 +44,7 @@ void DetectAddressPrintList(DetectAddress *); int DetectAddressInsert(DetectEngineCtx *, DetectAddressHead *, DetectAddress *); int DetectAddressJoin(DetectEngineCtx *, DetectAddress *, DetectAddress *); -DetectAddress *DetectAddressLookupInHead(DetectAddressHead *, Address *); +DetectAddress *DetectAddressLookupInHead(const DetectAddressHead *, Address *); DetectAddress *DetectAddressLookupInList(DetectAddress *, DetectAddress *); int DetectAddressMatch(DetectAddress *, Address *); diff --git a/src/detect-engine-alert.c b/src/detect-engine-alert.c index f48da78023..2abff2212b 100644 --- a/src/detect-engine-alert.c +++ b/src/detect-engine-alert.c @@ -67,12 +67,12 @@ PacketAlert *PacketAlertGetTag(void) * \retval 0 alert is suppressed */ static int PacketAlertHandle(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx, - Signature *s, Packet *p, uint16_t pos) + const Signature *s, Packet *p, PacketAlert *pa) { SCEnter(); int ret = 1; - DetectThresholdData *td = NULL; - SigMatch *sm; + const DetectThresholdData *td = NULL; + const SigMatch *sm; if (!(PKT_IS_IPV4(p) || PKT_IS_IPV6(p))) { SCReturnInt(1); @@ -90,7 +90,7 @@ static int PacketAlertHandle(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det /* PacketAlertThreshold returns 2 if the alert is suppressed but * we do need to apply rule actions to the packet. */ KEYWORD_PROFILING_START; - ret = PacketAlertThreshold(de_ctx, det_ctx, td, p, s); + ret = PacketAlertThreshold(de_ctx, det_ctx, td, p, s, pa); if (ret == 0 || ret == 2) { KEYWORD_PROFILING_END(det_ctx, DETECT_THRESHOLD, 0); /* It doesn't match threshold, remove it */ @@ -113,7 +113,7 @@ static int PacketAlertHandle(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det /* PacketAlertThreshold returns 2 if the alert is suppressed but * we do need to apply rule actions to the packet. */ KEYWORD_PROFILING_START; - ret = PacketAlertThreshold(de_ctx, det_ctx, td, p, s); + ret = PacketAlertThreshold(de_ctx, det_ctx, td, p, s, pa); if (ret == 0 || ret == 2) { KEYWORD_PROFILING_END(det_ctx, DETECT_THRESHOLD ,0); /* It doesn't match threshold, remove it */ @@ -239,14 +239,13 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx { SCEnter(); int i = 0; - Signature *s = NULL; SigMatch *sm = NULL; while (i < p->alerts.cnt) { SCLogDebug("Sig->num: %"PRIu16, p->alerts.alerts[i].num); - s = de_ctx->sig_array[p->alerts.alerts[i].num]; + const Signature *s = de_ctx->sig_array[p->alerts.alerts[i].num]; - int res = PacketAlertHandle(de_ctx, det_ctx, s, p, i); + int res = PacketAlertHandle(de_ctx, det_ctx, s, p, &p->alerts.alerts[i]); if (res > 0) { /* Now, if we have an alert, we have to check if we want * to tag this session or src/dst host */ @@ -255,7 +254,7 @@ void PacketAlertFinalize(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx while (sm) { /* tags are set only for alerts */ KEYWORD_PROFILING_START; - sigmatch_table[sm->type].Match(NULL, det_ctx, p, s, sm->ctx); + sigmatch_table[sm->type].Match(NULL, det_ctx, p, (Signature *)s, sm->ctx); KEYWORD_PROFILING_END(det_ctx, sm->type, 1); sm = sm->next; } diff --git a/src/detect-engine-threshold.c b/src/detect-engine-threshold.c index 2d37e55e7c..6b0d21141f 100644 --- a/src/detect-engine-threshold.c +++ b/src/detect-engine-threshold.c @@ -98,10 +98,11 @@ int ThresholdHostHasThreshold(Host *host) * * */ -DetectThresholdData *SigGetThresholdTypeIter(Signature *sig, Packet *p, SigMatch **psm, int list) +const DetectThresholdData *SigGetThresholdTypeIter(const Signature *sig, + Packet *p, const SigMatch **psm, int list) { - SigMatch *sm = NULL; - DetectThresholdData *tsh = NULL; + const SigMatch *sm = NULL; + const DetectThresholdData *tsh = NULL; if (sig == NULL) return NULL; @@ -178,7 +179,9 @@ int ThresholdTimeoutCheck(Host *host, struct timeval *tv) return retval; } -static inline DetectThresholdEntry *DetectThresholdEntryAlloc(DetectThresholdData *td, Packet *p, uint32_t sid, uint32_t gid) +static inline DetectThresholdEntry * +DetectThresholdEntryAlloc(const DetectThresholdData *td, Packet *p, + uint32_t sid, uint32_t gid) { SCEnter(); @@ -209,7 +212,7 @@ static DetectThresholdEntry *ThresholdHostLookupEntry(Host *h, uint32_t sid, uin return e; } -int ThresholdHandlePacketSuppress(Packet *p, DetectThresholdData *td, uint32_t sid, uint32_t gid) +int ThresholdHandlePacketSuppress(Packet *p, const DetectThresholdData *td, uint32_t sid, uint32_t gid) { int ret = 0; DetectAddress *m = NULL; @@ -244,12 +247,38 @@ int ThresholdHandlePacketSuppress(Packet *p, DetectThresholdData *td, uint32_t s return ret; } +static inline void RateFilterSetAction(Packet *p, PacketAlert *pa, uint8_t new_action) +{ + switch (new_action) { + case TH_ACTION_ALERT: + PACKET_ALERT(p); + pa->flags |= PACKET_ALERT_RATE_FILTER_MODIFIED; + break; + case TH_ACTION_DROP: + PACKET_DROP(p); + pa->flags |= PACKET_ALERT_RATE_FILTER_MODIFIED; + break; + case TH_ACTION_REJECT: + PACKET_REJECT(p); + pa->flags |= PACKET_ALERT_RATE_FILTER_MODIFIED; + break; + case TH_ACTION_PASS: + PACKET_PASS(p); + pa->flags |= PACKET_ALERT_RATE_FILTER_MODIFIED; + break; + default: + /* Weird, leave the default action */ + break; + } +} + /** * \retval 2 silent match (no alert but apply actions) * \retval 1 normal match * \retval 0 no match */ -int ThresholdHandlePacketHost(Host *h, Packet *p, DetectThresholdData *td, uint32_t sid, uint32_t gid) +int ThresholdHandlePacketHost(Host *h, Packet *p, const DetectThresholdData *td, + uint32_t sid, uint32_t gid, PacketAlert *pa) { int ret = 0; @@ -426,23 +455,7 @@ int ThresholdHandlePacketHost(Host *h, Packet *p, DetectThresholdData *td, uint3 } else { /* Already matching */ /* Take the action to perform */ - switch (td->new_action) { - case TH_ACTION_ALERT: - PACKET_ALERT(p); - break; - case TH_ACTION_DROP: - PACKET_DROP(p); - break; - case TH_ACTION_REJECT: - PACKET_REJECT(p); - break; - case TH_ACTION_PASS: - PACKET_PASS(p); - break; - default: - /* Weird, leave the default action */ - break; - } + RateFilterSetAction(p, pa, td->new_action); ret = 1; } /* else - if ((p->ts.tv_sec - lookup_tsh->tv_timeout) > td->timeout) */ @@ -455,23 +468,7 @@ int ThresholdHandlePacketHost(Host *h, Packet *p, DetectThresholdData *td, uint3 * timeout */ lookup_tsh->tv_timeout = p->ts.tv_sec; /* Take the action to perform */ - switch (td->new_action) { - case TH_ACTION_ALERT: - PACKET_ALERT(p); - break; - case TH_ACTION_DROP: - PACKET_DROP(p); - break; - case TH_ACTION_REJECT: - PACKET_REJECT(p); - break; - case TH_ACTION_PASS: - PACKET_PASS(p); - break; - default: - /* Weird, leave the default action */ - break; - } + RateFilterSetAction(p, pa, td->new_action); ret = 1; } } else { @@ -506,7 +503,8 @@ int ThresholdHandlePacketHost(Host *h, Packet *p, DetectThresholdData *td, uint3 return ret; } -static int ThresholdHandlePacketRule(DetectEngineCtx *de_ctx, Packet *p, DetectThresholdData *td, Signature *s) +static int ThresholdHandlePacketRule(DetectEngineCtx *de_ctx, Packet *p, + const DetectThresholdData *td, const Signature *s, PacketAlert *pa) { int ret = 0; @@ -519,27 +517,11 @@ static int ThresholdHandlePacketRule(DetectEngineCtx *de_ctx, Packet *p, DetectT * we still matching (and enabling the new_action) */ if ( (p->ts.tv_sec - lookup_tsh->tv_timeout) > td->timeout) { /* Ok, we are done, timeout reached */ - td->timeout = 0; + lookup_tsh->tv_timeout = 0; } else { /* Already matching */ /* Take the action to perform */ - switch (td->new_action) { - case TH_ACTION_ALERT: - PACKET_ALERT(p); - break; - case TH_ACTION_DROP: - PACKET_DROP(p); - break; - case TH_ACTION_REJECT: - PACKET_REJECT(p); - break; - case TH_ACTION_PASS: - PACKET_PASS(p); - break; - default: - /* Weird, leave the default action */ - break; - } + RateFilterSetAction(p, pa, td->new_action); ret = 1; } @@ -551,23 +533,7 @@ static int ThresholdHandlePacketRule(DetectEngineCtx *de_ctx, Packet *p, DetectT * timeout */ lookup_tsh->tv_timeout = p->ts.tv_sec; /* Take the action to perform */ - switch (td->new_action) { - case TH_ACTION_ALERT: - PACKET_ALERT(p); - break; - case TH_ACTION_DROP: - PACKET_DROP(p); - break; - case TH_ACTION_REJECT: - PACKET_REJECT(p); - break; - case TH_ACTION_PASS: - PACKET_PASS(p); - break; - default: - /* Weird, leave the default action */ - break; - } + RateFilterSetAction(p, pa, td->new_action); ret = 1; } } else { @@ -605,7 +571,7 @@ static int ThresholdHandlePacketRule(DetectEngineCtx *de_ctx, Packet *p, DetectT * \retval 0 do not alert on this event */ int PacketAlertThreshold(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx, - DetectThresholdData *td, Packet *p, Signature *s) + const DetectThresholdData *td, Packet *p, const Signature *s, PacketAlert *pa) { SCEnter(); @@ -619,18 +585,18 @@ int PacketAlertThreshold(DetectEngineCtx *de_ctx, DetectEngineThreadCtx *det_ctx } else if (td->track == TRACK_SRC) { Host *src = HostGetHostFromHash(&p->src); if (src) { - ret = ThresholdHandlePacketHost(src,p,td,s->id,s->gid); + ret = ThresholdHandlePacketHost(src,p,td,s->id,s->gid,pa); HostRelease(src); } } else if (td->track == TRACK_DST) { Host *dst = HostGetHostFromHash(&p->dst); if (dst) { - ret = ThresholdHandlePacketHost(dst,p,td,s->id,s->gid); + ret = ThresholdHandlePacketHost(dst,p,td,s->id,s->gid,pa); HostRelease(dst); } } else if (td->track == TRACK_RULE) { SCMutexLock(&de_ctx->ths_ctx.threshold_table_lock); - ret = ThresholdHandlePacketRule(de_ctx,p,td,s); + ret = ThresholdHandlePacketRule(de_ctx,p,td,s,pa); SCMutexUnlock(&de_ctx->ths_ctx.threshold_table_lock); } diff --git a/src/detect-engine-threshold.h b/src/detect-engine-threshold.h index b874ecb2fa..cef5f2058d 100644 --- a/src/detect-engine-threshold.h +++ b/src/detect-engine-threshold.h @@ -33,9 +33,11 @@ void ThresholdInit(void); int ThresholdHostStorageId(void); int ThresholdHostHasThreshold(Host *); -DetectThresholdData *SigGetThresholdTypeIter(Signature *, Packet *, SigMatch **, int list); +const DetectThresholdData *SigGetThresholdTypeIter(const Signature *, + Packet *, const SigMatch **, int list); int PacketAlertThreshold(DetectEngineCtx *, DetectEngineThreadCtx *, - DetectThresholdData *, Packet *, Signature *); + const DetectThresholdData *, Packet *, + const Signature *, PacketAlert *); void ThresholdHashInit(DetectEngineCtx *); void ThresholdContextDestroy(DetectEngineCtx *); diff --git a/src/output-json-alert.c b/src/output-json-alert.c index 1db766d61f..e4836f0a27 100644 --- a/src/output-json-alert.c +++ b/src/output-json-alert.c @@ -142,10 +142,18 @@ static void AlertJsonSsh(const Flow *f, json_t *js) void AlertJsonHeader(const Packet *p, const PacketAlert *pa, json_t *js) { char *action = "allowed"; - if (pa->action & (ACTION_REJECT|ACTION_REJECT_DST|ACTION_REJECT_BOTH)) { - action = "blocked"; - } else if ((pa->action & ACTION_DROP) && EngineModeIsIPS()) { - action = "blocked"; + /* use packet action if rate_filter modified the action */ + if (unlikely(pa->flags & PACKET_ALERT_RATE_FILTER_MODIFIED)) { + if (PACKET_TEST_ACTION(p, (ACTION_DROP|ACTION_REJECT| + ACTION_REJECT_DST|ACTION_REJECT_BOTH))) { + action = "blocked"; + } + } else { + if (pa->action & (ACTION_REJECT|ACTION_REJECT_DST|ACTION_REJECT_BOTH)) { + action = "blocked"; + } else if ((pa->action & ACTION_DROP) && EngineModeIsIPS()) { + action = "blocked"; + } } /* Add tx_id to root element for correlation with other events. */