Flow drops/v4

src/detect-engine-alert.c

@@ -334,7 +334,7 @@ static inline void FlowApplySignatureActions(
      * - sig is IP or PD only
      * - match is in applayer
      * - match is in stream */
-    if (s->action & (ACTION_DROP | ACTION_PASS)) {
+    if (pa->action & (ACTION_DROP | ACTION_PASS)) {
         DEBUG_VALIDATE_BUG_ON(s->type == SIG_TYPE_NOT_SET);
         DEBUG_VALIDATE_BUG_ON(s->type == SIG_TYPE_MAX);
 

src/detect.c

@@ -1709,9 +1709,11 @@ static void DetectFlow(ThreadVars *tv,
         return;
     }
 
-    /* if flow is set to drop, we enforce that here */
+    /* we check the flow drop here, and not the packet drop. This is
+     * to allow stream engine "invalid" drop packets to still be
+     * evaluated by the stream event rules. */
     if (f->flags & FLOW_ACTION_DROP) {
-        PacketDrop(p, ACTION_DROP, PKT_DROP_REASON_FLOW_DROP);
+        DEBUG_VALIDATE_BUG_ON(!(PKT_IS_PSEUDOPKT(p)) && !PacketCheckAction(p, ACTION_DROP));
         SCReturn;
     }
 

src/flow-manager.c

@@ -281,7 +281,8 @@ static uint32_t ProcessAsideQueue(FlowManagerTimeoutThread *td, FlowTimeoutCount
     while ((f = FlowQueuePrivateGetFromTop(&td->aside_queue)) != NULL) {
         /* flow is still locked */
 
-        if (f->proto == IPPROTO_TCP && !(f->flags & FLOW_TIMEOUT_REASSEMBLY_DONE) &&
+        if (f->proto == IPPROTO_TCP &&
+                !(f->flags & (FLOW_TIMEOUT_REASSEMBLY_DONE | FLOW_ACTION_DROP)) &&
                 !FlowIsBypassed(f) && FlowForceReassemblyNeedReassembly(f) == 1) {
             /* Send the flow to its thread */
             FlowForceReassemblyForFlow(f);

src/flow-worker.c

@@ -34,6 +34,8 @@
 #include "suricata-common.h"
 #include "suricata.h"
 
+#include "action-globals.h"
+#include "packet.h"
 #include "decode.h"
 #include "detect.h"
 #include "stream-tcp.h"
@@ -183,8 +185,9 @@ static void CheckWorkQueue(ThreadVars *tv, FlowWorkerThreadData *fw, FlowTimeout
         f->flow_end_flags |= FLOW_END_FLAG_TIMEOUT; //TODO emerg
 
         if (f->proto == IPPROTO_TCP) {
-            if (!(f->flags & FLOW_TIMEOUT_REASSEMBLY_DONE) && !FlowIsBypassed(f) &&
-                    FlowForceReassemblyNeedReassembly(f) == 1 && f->ffr != 0) {
+            if (!(f->flags & (FLOW_TIMEOUT_REASSEMBLY_DONE | FLOW_ACTION_DROP)) &&
+                    !FlowIsBypassed(f) && FlowForceReassemblyNeedReassembly(f) == 1 &&
+                    f->ffr != 0) {
                 /* read detect thread in case we're doing a reload */
                 void *detect_thread = SC_ATOMIC_GET(fw->detect_thread);
                 int cnt = FlowFinish(tv, f, fw, detect_thread);
@@ -566,9 +569,11 @@ static TmEcode FlowWorker(ThreadVars *tv, Packet *p, void *data)
 
         /* handle the app layer part of the UDP packet payload */
     } else if (p->flow && p->proto == IPPROTO_UDP) {
-        FLOWWORKER_PROFILING_START(p, PROFILE_FLOWWORKER_APPLAYERUDP);
-        AppLayerHandleUdp(tv, fw->stream_thread->ra_ctx->app_tctx, p, p->flow);
-        FLOWWORKER_PROFILING_END(p, PROFILE_FLOWWORKER_APPLAYERUDP);
+        if (!PacketCheckAction(p, ACTION_DROP)) {
+            FLOWWORKER_PROFILING_START(p, PROFILE_FLOWWORKER_APPLAYERUDP);
+            AppLayerHandleUdp(tv, fw->stream_thread->ra_ctx->app_tctx, p, p->flow);
+            FLOWWORKER_PROFILING_END(p, PROFILE_FLOWWORKER_APPLAYERUDP);
+        }
     }
 
     PacketUpdateEngineEventCounters(tv, fw->dtv, p);

src/flow.c

@@ -25,6 +25,9 @@
 
 #include "suricata-common.h"
 #include "suricata.h"
+
+#include "action-globals.h"
+#include "packet.h"
 #include "decode.h"
 #include "conf.h"
 #include "threadvars.h"
@@ -505,6 +508,9 @@ void FlowHandlePacketUpdate(Flow *f, Packet *p, ThreadVars *tv, DecodeThreadVars
         FlowUpdateState(f, FLOW_STATE_ESTABLISHED);
     }
 
+    if (f->flags & FLOW_ACTION_DROP) {
+        PacketDrop(p, ACTION_DROP, PKT_DROP_REASON_FLOW_DROP);
+    }
     /*set the detection bypass flags*/
     if (f->flags & FLOW_NOPACKET_INSPECTION) {
         SCLogDebug("setting FLOW_NOPACKET_INSPECTION flag on flow %p", f);

src/stream-tcp.c

@@ -5356,11 +5356,9 @@ int StreamTcpPacket (ThreadVars *tv, Packet *p, StreamTcpThread *stt,
      * applayer detection, then drop the rest of the packets of the
      * same stream and avoid inspecting it any further */
     if (StreamTcpCheckFlowDrops(p) == 1) {
-        SCLogDebug("This flow/stream triggered a drop rule");
-        FlowSetNoPacketInspectionFlag(p->flow);
-        DecodeSetNoPacketInspectionFlag(p);
+        DEBUG_VALIDATE_BUG_ON(!(PKT_IS_PSEUDOPKT(p)) && !PacketCheckAction(p, ACTION_DROP));
+        SCLogDebug("flow triggered a drop rule");
         StreamTcpDisableAppLayer(p->flow);
-        PacketDrop(p, ACTION_DROP, PKT_DROP_REASON_FLOW_DROP);
         /* return the segments to the pool */
         StreamTcpSessionPktFree(p);
         SCReturnInt(0);

src/tests/detect.c

@@ -4740,117 +4740,6 @@ static int SigTestDropFlow03(void)
     return result;
 }
 
-/** \test test if the engine set flag to drop pkts of a flow that
- *        triggered a drop action on IDS mode, but continue the inspection
- *        as usual (instead of on IPS mode) */
-static int SigTestDropFlow04(void)
-{
-    Flow f;
-    HtpState *http_state = NULL;
-    uint8_t http_buf1[] = "POST /one HTTP/1.0\r\n"
-        "User-Agent: Mozilla/1.0\r\n"
-        "Cookie: hellocatch\r\n\r\n";
-    uint32_t http_buf1_len = sizeof(http_buf1) - 1;
-
-    uint8_t http_buf2[] = "POST /two HTTP/1.0\r\n"
-        "User-Agent: Mozilla/1.0\r\n"
-        "Cookie: hellocatch\r\n\r\n";
-    uint32_t http_buf2_len = sizeof(http_buf1) - 1;
-
-    TcpSession ssn;
-    Packet *p1 = NULL;
-    Packet *p2 = NULL;
-    Signature *s = NULL;
-    ThreadVars tv;
-    DetectEngineThreadCtx *det_ctx = NULL;
-    AppLayerParserThreadCtx *alp_tctx = AppLayerParserThreadCtxAlloc();
-
-    memset(&tv, 0, sizeof(ThreadVars));
-    memset(&f, 0, sizeof(Flow));
-    memset(&ssn, 0, sizeof(TcpSession));
-
-    p1 = UTHBuildPacket(NULL, 0, IPPROTO_TCP);
-    p2 = UTHBuildPacket(NULL, 0, IPPROTO_TCP);
-
-    FLOW_INITIALIZE(&f);
-    f.protoctx = (void *)&ssn;
-    f.proto = IPPROTO_TCP;
-    f.flags |= FLOW_IPV4;
-
-    p1->flow = &f;
-    p1->flowflags |= FLOW_PKT_TOSERVER;
-    p1->flowflags |= FLOW_PKT_ESTABLISHED;
-    p1->flags |= PKT_HAS_FLOW|PKT_STREAM_EST;
-
-    p2->flow = &f;
-    p2->flowflags |= FLOW_PKT_TOSERVER;
-    p2->flowflags |= FLOW_PKT_ESTABLISHED;
-    p2->flags |= PKT_HAS_FLOW|PKT_STREAM_EST;
-    f.alproto = ALPROTO_HTTP1;
-
-    StreamTcpInitConfig(true);
-
-    DetectEngineCtx *de_ctx = DetectEngineCtxInit();
-    FAIL_IF_NULL(de_ctx);
-    de_ctx->flags |= DE_QUIET;
-
-    s = DetectEngineAppendSig(de_ctx, "drop tcp any any -> any 80 "
-                                      "(msg:\"Test proto match\"; uricontent:\"one\";"
-                                      "sid:1;)");
-    FAIL_IF_NULL(s);
-
-    /* the no inspection flag should be set after the first sig gets triggered,
-     * so the second packet should not match the next sig (because of no inspection) */
-    s = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any 80 "
-                                      "(msg:\"Test proto match\"; uricontent:\"two\";"
-                                      "sid:2;)");
-    FAIL_IF_NULL(s);
-
-    SigGroupBuild(de_ctx);
-    DetectEngineThreadCtxInit(&tv, (void *)de_ctx, (void *)&det_ctx);
-
-    int r = AppLayerParserParse(
-            NULL, alp_tctx, &f, ALPROTO_HTTP1, STREAM_TOSERVER, http_buf1, http_buf1_len);
-    FAIL_IF_NOT(r == 0);
-
-    http_state = f.alstate;
-    FAIL_IF_NULL(http_state);
-
-    /* do detect */
-    SigMatchSignatures(&tv, de_ctx, det_ctx, p1);
-
-    FAIL_IF_NOT(PacketAlertCheck(p1, 1));
-    FAIL_IF(PacketAlertCheck(p1, 2));
-
-    FAIL_IF_NOT(p1->flow->flags & FLOW_ACTION_DROP);
-    FAIL_IF_NOT(PacketTestAction(p1, ACTION_DROP));
-
-    FAIL_IF(p2->flags & PKT_NOPACKET_INSPECTION);
-
-    r = AppLayerParserParse(
-            NULL, alp_tctx, &f, ALPROTO_HTTP1, STREAM_TOSERVER, http_buf2, http_buf2_len);
-    FAIL_IF_NOT(r == 0);
-
-    /* do detect */
-    SigMatchSignatures(&tv, de_ctx, det_ctx, p2);
-
-    FAIL_IF(PacketAlertCheck(p2, 1));
-    FAIL_IF(PacketAlertCheck(p2, 2));
-    FAIL_IF_NOT(PacketTestAction(p2, ACTION_DROP));
-
-    AppLayerParserThreadCtxFree(alp_tctx);
-    DetectEngineThreadCtxDeinit(&tv, det_ctx);
-    DetectEngineCtxFree(de_ctx);
-
-    StreamTcpFreeConfig(true);
-    FLOW_DESTROY(&f);
-
-    UTHFreePackets(&p1, 1);
-    UTHFreePackets(&p2, 1);
-
-    PASS;
-}
-
 /** \test ICMP packet shouldn't be matching port based sig
  *        Bug #611 */
 static int SigTestPorts01(void)
@@ -5247,7 +5136,6 @@ void SigRegisterTests(void)
     UtRegisterTest("SigTestDropFlow01", SigTestDropFlow01);
     UtRegisterTest("SigTestDropFlow02", SigTestDropFlow02);
     UtRegisterTest("SigTestDropFlow03", SigTestDropFlow03);
-    UtRegisterTest("SigTestDropFlow04", SigTestDropFlow04);
 
     UtRegisterTest("DetectAddressYamlParsing01", DetectAddressYamlParsing01);
     UtRegisterTest("DetectAddressYamlParsing02", DetectAddressYamlParsing02);