]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
detect: inspect all packets in multi-layer tunneling 9637/head
authorVictor Julien <vjulien@oisf.net>
Fri, 13 Oct 2023 11:47:05 +0000 (13:47 +0200)
committerVictor Julien <victor@inliniac.net>
Mon, 16 Oct 2023 19:16:37 +0000 (21:16 +0200)
When the decoders encounter multiple layers of tunneling, multiple tunnel
packets are created. These are then stored in ThreadVars::decode_pq, where
they are processed after the current thread "slot" is done. However, due
to a logic error, the tunnel packets after the first, where not called
for the correct position in the packet pipeline. This would lead to these
packets not going through the FlowWorker module, so skipping everything
from flow tracking, detection and logging.

This would only happen for single and workers, due to how the pipelines
are constructed.

The "slot" holding the decoder, would contain 2 packets in
ThreadVars::decode_pq. Then it would call the pipeline on the first
packet with the next slot of the pipeline through a indirect call to
TmThreadsSlotVarRun(), so it would be called for the FlowWorker.
However when that first (the most inner) packet was done, the call
to TmThreadsSlotVarRun() would again service the ThreadVars::decode_pq
and process it, again moving the slot pointer forward, so past the
FlowWorker.

This patch addresses the issue by making sure only a "decode" thread
slot will service the ThreadVars::decode_pq, thus never moving the
slot past the FlowWorker.

Bug: #6402.

src/tm-threads.c
src/tm-threads.h

index 63bf3be19db4eed8127f41fc98d138d6514647e0..b173cb84f442cdb15f07a3b22cb142920824e7cb 100644 (file)
@@ -142,9 +142,11 @@ TmEcode TmThreadsSlotVarRun(ThreadVars *tv, Packet *p, TmSlot *slot)
             TmThreadsSlotProcessPktFail(tv, s, NULL);
             return TM_ECODE_FAILED;
         }
-
-        if (TmThreadsProcessDecodePseudoPackets(tv, &tv->decode_pq, s->slot_next) != TM_ECODE_OK) {
-            return TM_ECODE_FAILED;
+        if (s->tm_flags & TM_FLAG_DECODE_TM) {
+            if (TmThreadsProcessDecodePseudoPackets(tv, &tv->decode_pq, s->slot_next) !=
+                    TM_ECODE_OK) {
+                return TM_ECODE_FAILED;
+            }
         }
     }
 
@@ -661,6 +663,7 @@ void TmSlotSetFuncAppend(ThreadVars *tv, TmModule *tm, const void *data)
     /* we don't have to check for the return value "-1".  We wouldn't have
      * received a TM as arg, if it didn't exist */
     slot->tm_id = TmModuleGetIDForTM(tm);
+    slot->tm_flags |= tm->flags;
 
     tv->tmm_flags |= tm->flags;
     tv->cap_flags |= tm->cap_flags;
index ec791bea47be8bc9ba478c271a8c218b3496ebbd..4ca55f9bc72cdec4ed4623fc6ac1bea2e3d3d38e 100644 (file)
@@ -63,14 +63,18 @@ typedef struct TmSlot_ {
 
     SC_ATOMIC_DECLARE(void *, slot_data);
 
+    /** copy of the TmModule::flags */
+    uint8_t tm_flags;
+
+    /* store the thread module id */
+    int tm_id;
+
     TmEcode (*SlotThreadInit)(ThreadVars *, const void *, void **);
     void (*SlotThreadExitPrintStats)(ThreadVars *, void *);
     TmEcode (*SlotThreadDeinit)(ThreadVars *, void *);
 
     /* data storage */
     const void *slot_initdata;
-    /* store the thread module id */
-    int tm_id;
 
 } TmSlot;