From: Ruslan Usmanov Date: Tue, 12 Dec 2017 18:10:07 +0000 (-0500) Subject: rate_filter: by_rule fixed triggering algorithm X-Git-Tag: suricata-4.1.0-beta1~305 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fb87d21ec747871afa50114b5e78944d982bf935;p=thirdparty%2Fsuricata.git rate_filter: by_rule fixed triggering algorithm Fixes issue #2258 Program was triggering rate_filter by_rule earlier than needed and generally behaved like a threshold. --- diff --git a/src/detect-engine-threshold.c b/src/detect-engine-threshold.c index efd9513897..81613f3d33 100644 --- a/src/detect-engine-threshold.c +++ b/src/detect-engine-threshold.c @@ -227,13 +227,13 @@ DetectThresholdEntryAlloc(const DetectThresholdData *td, Packet *p, if (unlikely(ste == NULL)) { SCReturnPtr(NULL, "DetectThresholdEntry"); } + memset(ste, 0, sizeof(*ste)); ste->sid = sid; ste->gid = gid; ste->track = td->track; ste->seconds = td->seconds; - ste->tv_timeout = 0; SCReturnPtr(ste, "DetectThresholdEntry"); } @@ -614,50 +614,32 @@ static int ThresholdHandlePacketRule(DetectEngineCtx *de_ctx, Packet *p, { int ret = 0; - if (td->type != TYPE_RATE) - return 1; - DetectThresholdEntry* lookup_tsh = (DetectThresholdEntry *)de_ctx->ths_ctx.th_entry[s->num]; - if (lookup_tsh != NULL) { - /* Check if we have a timeout enabled, if so, - * 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 */ - lookup_tsh->tv_timeout = 0; - } else { - /* Already matching */ - /* Take the action to perform */ - RateFilterSetAction(p, pa, td->new_action); - ret = 1; - } + SCLogDebug("by_rule lookup_tsh %p num %u", lookup_tsh, s->num); - /* Update the matching state with the timeout interval */ - if ( (p->ts.tv_sec - lookup_tsh->tv_sec1) < td->seconds) { - lookup_tsh->current_count++; - if (lookup_tsh->current_count >= td->count) { - /* Then we must enable the new action by setting a - * timeout */ - lookup_tsh->tv_timeout = p->ts.tv_sec; - /* Take the action to perform */ + switch (td->type) { + case TYPE_RATE: + { + ret = 1; + if (lookup_tsh && IsThresholdReached(lookup_tsh, td, p->ts.tv_sec)) { RateFilterSetAction(p, pa, td->new_action); - ret = 1; } - } else { - lookup_tsh->tv_sec1 = p->ts.tv_sec; - lookup_tsh->current_count = 1; - } - } else { - if (td->count == 1) { - ret = 1; - } - - DetectThresholdEntry *e = DetectThresholdEntryAlloc(td, p, s->id, s->gid); - if (e != NULL) { - e->current_count = 1; - e->tv_sec1 = p->ts.tv_sec; - e->tv_timeout = 0; + else if (!lookup_tsh) { + DetectThresholdEntry *e = DetectThresholdEntryAlloc(td, p, s->id, s->gid); + if (e != NULL) { + e->current_count = 1; + e->tv_sec1 = p->ts.tv_sec; + e->tv_timeout = 0; - de_ctx->ths_ctx.th_entry[s->num] = e; + de_ctx->ths_ctx.th_entry[s->num] = e; + } + } + break; + } + default: + { + SCLogError(SC_ERR_INVALID_VALUE, "type %d is not supported", td->type); + break; } } diff --git a/src/util-threshold-config.c b/src/util-threshold-config.c index 370a9e476d..d8ad7c70c7 100644 --- a/src/util-threshold-config.c +++ b/src/util-threshold-config.c @@ -1276,8 +1276,7 @@ static FILE *SCThresholdConfGenerateValidDummyFD07(void) } /** - * \brief Creates a dummy threshold file, with all valid options, but - * with splitted rules (multiline), for testing purposes. + * \brief Creates a dummy threshold file, for testing rate_filter, track by_rule * * \retval fd Pointer to file descriptor. */ @@ -1285,8 +1284,7 @@ static FILE *SCThresholdConfGenerateValidDummyFD08(void) { FILE *fd = NULL; const char *buffer = - "rate_filter gen_id 1, sig_id 10, track by_rule, count 3, seconds 3, new_action drop, timeout 10\n" - "rate_filter gen_id 1, sig_id 11, track by_src, count 3, seconds 1, new_action drop, timeout 5\n"; + "rate_filter gen_id 1, sig_id 10, track by_rule, count 3, seconds 3, new_action drop, timeout 10\n"; fd = SCFmemopen((void *)buffer, strlen(buffer), "r"); if (fd == NULL) @@ -1741,7 +1739,11 @@ static int SCThresholdConfTest10(void) memset (&ts, 0, sizeof(struct timeval)); TimeGet(&ts); - Packet *p1 = UTHBuildPacket((uint8_t*)"lalala", 6, IPPROTO_TCP); + /* Create two different packets falling to the same rule, and + * because count:3, we should drop on match #4. + */ + Packet *p1 = UTHBuildPacketSrcDst((uint8_t*)"lalala", 6, IPPROTO_TCP, + "172.26.0.2", "172.26.0.11"); FAIL_IF_NULL(p1); Packet *p2 = UTHBuildPacketSrcDst((uint8_t*)"lalala", 6, IPPROTO_TCP, "172.26.0.1", "172.26.0.10"); @@ -1767,26 +1769,44 @@ static int SCThresholdConfTest10(void) SigGroupBuild(de_ctx); DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx); TimeGet(&p1->ts); + p2->ts = p1->ts; + /* All should be alerted, none dropped */ SigMatchSignatures(&th_v, de_ctx, det_ctx, p1); - FAIL_IF(PacketAlertCheck(p1, 10)); + FAIL_IF(PACKET_TEST_ACTION(p1, ACTION_DROP)); + FAIL_IF(PacketAlertCheck(p1, 10) != 1); + p1->action = 0; SigMatchSignatures(&th_v, de_ctx, det_ctx, p2); - FAIL_IF(PacketAlertCheck(p2, 10)); - + FAIL_IF(PACKET_TEST_ACTION(p2, ACTION_DROP)); + FAIL_IF(PacketAlertCheck(p2, 10) != 1); + p2->action = 0; SigMatchSignatures(&th_v, de_ctx, det_ctx, p1); - FAIL_IF_NOT(PacketAlertCheck(p1, 10) == 1); + FAIL_IF(PACKET_TEST_ACTION(p1, ACTION_DROP)); + FAIL_IF(PacketAlertCheck(p1, 10) != 1); + p1->action = 0; + + /* Match #4 should be dropped*/ + SigMatchSignatures(&th_v, de_ctx, det_ctx, p2); + FAIL_IF_NOT(PACKET_TEST_ACTION(p2, ACTION_DROP)); + FAIL_IF(PacketAlertCheck(p2, 10) != 1); + p2->action = 0; TimeSetIncrementTime(2); TimeGet(&p1->ts); - SigMatchSignatures(&th_v, de_ctx, det_ctx, p2); - FAIL_IF_NOT(PacketAlertCheck(p2, 10) == 1); + /* Still dropped because timeout not expired */ + SigMatchSignatures(&th_v, de_ctx, det_ctx, p1); + FAIL_IF_NOT(PACKET_TEST_ACTION(p1, ACTION_DROP)); + FAIL_IF(PacketAlertCheck(p1, 10) != 1); + p1->action = 0; TimeSetIncrementTime(10); TimeGet(&p1->ts); + /* Not dropped because timeout expired */ SigMatchSignatures(&th_v, de_ctx, det_ctx, p1); - FAIL_IF_NOT(PacketAlertCheck(p1, 10) == 0); + FAIL_IF(PACKET_TEST_ACTION(p1, ACTION_DROP)); + FAIL_IF(PacketAlertCheck(p1, 10) != 1); /* Ensure that a Threshold entry was installed at the sig */ FAIL_IF_NULL(de_ctx->ths_ctx.th_entry[s->num]);