detect/frame: improve frame detection

Add a per frame progress tracker.

diff --git a/src/app-layer-frames.c b/src/app-layer-frames.c
index f68f2c36ee2f7a6b2604576768af24d236069812..bebb4ad50002ec7261b3cf659222729fbd7c5b63 100644 (file)
--- a/src/app-layer-frames.c
+++ b/src/app-layer-frames.c
@@ -40,10 +40,10 @@ static void FrameDebug(const char *prefix, const Frames *frames, const Frame *fr
         type_name = AppLayerParserGetFrameNameById(frames->ipproto, frames->alproto, frame->type);
     }
     SCLogDebug("[%s] %p: frame:%p type:%u/%s id:%" PRIi64 " flags:%02x offset:%" PRIu64
-               ", len:%" PRIi64 ", events:%u %u/%u/%u/%u",
+               ", len:%" PRIi64 ", inspect_progress:%" PRIu64 ", events:%u %u/%u/%u/%u",
             prefix, frames, frame, frame->type, type_name, frame->id, frame->flags, frame->offset,
-            frame->len, frame->event_cnt, frame->events[0], frame->events[1], frame->events[2],
-            frame->events[3]);
+            frame->len, frame->inspect_progress, frame->event_cnt, frame->events[0],
+            frame->events[1], frame->events[2], frame->events[3]);
 #endif
 }
 

diff --git a/src/app-layer-frames.h b/src/app-layer-frames.h
index c0d1fb27c38b3b6ed86c6baf3df968aef505c8e8..98ddefede7eed7e296737308448ea2c10c5d2319 100644 (file)
--- a/src/app-layer-frames.h
+++ b/src/app-layer-frames.h
@@ -51,8 +51,8 @@ typedef struct Frame {
     int64_t len;
     int64_t id;
     uint64_t tx_id; /**< tx_id to match this frame. UINT64T_MAX if not used. */
+    uint64_t inspect_progress; /**< inspection tracker relative to the start of the frame */
 } Frame;
-// size 40
 
 #define FRAMES_STATIC_CNT 3
 
@@ -68,13 +68,11 @@ typedef struct Frames {
     AppProto alproto;
 #endif
 } Frames;
-// size 136
 
 typedef struct FramesContainer {
     Frames toserver;
     Frames toclient;
 } FramesContainer;
-// size 272
 
 void FramesFree(Frames *frames);
 void FramesPrune(Flow *f, Packet *p);

diff --git a/src/detect-engine-frame.c b/src/detect-engine-frame.c
index 739ef6cc810a45842c239c6fef06dc51f941d962..1046aede45f7fc288003522afac39886da76e3c1 100644 (file)
--- a/src/detect-engine-frame.c
+++ b/src/detect-engine-frame.c
@@ -219,6 +219,7 @@ static int FrameStreamDataFunc(
                " }, input: %p, input_len:%u, offset:%" PRIu64,
             fsd, fsd->det_ctx, fsd->transforms, fsd->frame, fsd->list_id, fsd->idx,
             fsd->data_offset, fsd->frame_offset, input, input_len, offset);
+    // PrintRawDataFp(stdout, input, input_len);
 
     InspectionBuffer *buffer =
             InspectionBufferMultipleForListGet(fsd->det_ctx, fsd->list_id, fsd->idx);
@@ -230,52 +231,72 @@ static int FrameStreamDataFunc(
     const uint8_t *data = input;
     uint8_t ci_flags = 0;
     uint32_t data_len;
-    uint64_t inspect_offset = 0;
-    if (fsd->frame_offset == offset) {
+
+    /* fo: frame offset; offset relative to start of the frame */
+    uint64_t fo_inspect_offset = 0;
+
+    if (frame->offset == 0 && offset == 0) {
         ci_flags |= DETECT_CI_FLAGS_START;
         SCLogDebug("have frame data start");
 
         if (frame->len >= 0) {
             data_len = MIN(input_len, frame->len);
+            if (data_len == frame->len) {
+                ci_flags |= DETECT_CI_FLAGS_END;
+                SCLogDebug("have frame data end");
+            }
         } else {
             data_len = input_len;
         }
-
-        if (data_len == frame->len) {
-            ci_flags |= DETECT_CI_FLAGS_END;
-            SCLogDebug("have frame data end");
-        }
-        // inspect_offset 0
     } else {
-        BUG_ON(offset < fsd->data_offset);
-        inspect_offset = offset - fsd->frame_offset;
+        const uint64_t so_frame_inspect_offset = frame->inspect_progress + frame->offset;
+        const uint64_t so_inspect_offset = MAX(offset, so_frame_inspect_offset);
+        fo_inspect_offset = so_inspect_offset - frame->offset;
 
+        if (frame->offset >= offset) {
+            ci_flags |= DETECT_CI_FLAGS_START;
+            SCLogDebug("have frame data start");
+        }
         if (frame->len >= 0) {
-            if (inspect_offset >= (uint64_t)frame->len) {
-                SCLogDebug("data entirely past frame");
+            if (fo_inspect_offset >= (uint64_t)frame->len) {
+                SCLogDebug("data entirely past frame (%" PRIu64 " > %" PRIi64 ")",
+                        fo_inspect_offset, frame->len);
                 return 1;
             }
-            uint32_t adjusted_frame_len = (uint32_t)((uint64_t)frame->len - inspect_offset);
-            SCLogDebug("frame len after applying offset %" PRIu64 ": %u", inspect_offset,
-                    adjusted_frame_len);
 
-            data_len = MIN(adjusted_frame_len, input_len);
-            SCLogDebug("usable data len for frame: %u", data_len);
+            /* so: relative to start of stream */
+            const uint64_t so_frame_offset = frame->offset;
+            const uint64_t so_frame_re = so_frame_offset + (uint64_t)frame->len;
+            const uint64_t so_input_re = offset + input_len;
+
+            /* in: relative to start of input data */
+            BUG_ON(so_inspect_offset < offset);
+            const uint32_t in_data_offset = so_inspect_offset - offset;
+            data += in_data_offset;
 
-            if ((uint64_t)data_len + inspect_offset == (uint64_t)frame->len) {
+            uint32_t in_data_excess = 0;
+            if (so_input_re >= so_frame_re) {
                 ci_flags |= DETECT_CI_FLAGS_END;
                 SCLogDebug("have frame data end");
+                in_data_excess = so_input_re - so_frame_re;
             }
+            data_len = input_len - in_data_offset - in_data_excess;
         } else {
-            data_len = input_len;
+            /* in: relative to start of input data */
+            BUG_ON(so_inspect_offset < offset);
+            const uint32_t in_data_offset = so_inspect_offset - offset;
+            data += in_data_offset;
+            data_len = input_len - in_data_offset;
         }
     }
     // PrintRawDataFp(stdout, data, data_len);
     InspectionBufferSetupMulti(buffer, fsd->transforms, data, data_len);
-    SCLogDebug("inspect_offset %" PRIu64, inspect_offset);
-    buffer->inspect_offset = inspect_offset;
+    SCLogDebug("inspect_offset %" PRIu64, fo_inspect_offset);
+    buffer->inspect_offset = fo_inspect_offset;
     buffer->flags = ci_flags;
     fsd->buffer = buffer;
+    fsd->det_ctx->frame_inspect_progress =
+            MAX(fo_inspect_offset + data_len, fsd->det_ctx->frame_inspect_progress);
     return 1; // for now only the first chunk
 }
 
@@ -314,12 +335,42 @@ InspectionBuffer *DetectFrame2InspectBuffer(DetectEngineThreadCtx *det_ctx,
     const uint64_t usable = StreamTcpGetUsable(stream, eof);
     if (usable <= frame_offset)
         return NULL;
-    const uint64_t offset = MAX(STREAM_BASE_OFFSET(stream), frame_offset);
+
+    uint64_t want = frame->inspect_progress;
+    if (frame->len == -1) {
+        if (eof) {
+            want = usable;
+        } else {
+            want += 2500;
+        }
+    } else {
+        /* don't have the full frame yet */
+        if (frame->offset + frame->len > usable) {
+            want += 2500;
+        } else {
+            want = frame->offset + frame->len;
+        }
+    }
+
+    const uint64_t have = usable;
+    if (have < want) {
+        SCLogDebug("wanted %" PRIu64 " bytes, got %" PRIu64, want, have);
+        return NULL;
+    }
+
+    const uint64_t available_data = usable - STREAM_BASE_OFFSET(stream);
+    SCLogDebug("check inspection for having 2500 bytes: %" PRIu64, available_data);
+    if (!eof && available_data < 2500 && (frame->len < 0 || frame->len > (int64_t)available_data)) {
+        SCLogDebug("skip inspection until we have 2500 bytes (have %" PRIu64 ")", available_data);
+        return NULL;
+    }
+
+    const uint64_t offset = MAX(STREAM_BASE_OFFSET(stream), frame->offset);
 
     struct FrameStreamData fsd = { det_ctx, transforms, frame, list_id, idx,
-        STREAM_BASE_OFFSET(stream), frame_offset, NULL };
+        STREAM_BASE_OFFSET(stream), MAX(frame->inspect_progress, frame->offset), NULL };
     StreamReassembleForFrame(ssn, stream, FrameStreamDataFunc, &fsd, offset, eof);
-    SCLogDebug("offset %" PRIu64, frame_offset);
+
     return fsd.buffer;
 }
 

diff --git a/src/detect.c b/src/detect.c
index 9fdd2e9c08723eb9795e9eaf8ea206acb687edb6..76b28ff394469d1d635d4490ef1149ae7576d849 100644 (file)
--- a/src/detect.c
+++ b/src/detect.c
@@ -1565,11 +1565,12 @@ static void DetectRunFrames(ThreadVars *tv, DetectEngineCtx *de_ctx, DetectEngin
 
     for (uint32_t idx = 0; idx < frames->cnt; idx++) {
         SCLogDebug("frame %u", idx);
-        const Frame *frame = FrameGetByIndex(frames, idx);
+        Frame *frame = FrameGetByIndex(frames, idx);
         if (frame == NULL) {
             continue;
         }
 
+        det_ctx->frame_inspect_progress = 0;
         uint32_t array_idx = 0;
         uint32_t total_rules = det_ctx->match_array_cnt;
 
@@ -1649,6 +1650,18 @@ static void DetectRunFrames(ThreadVars *tv, DetectEngineCtx *de_ctx, DetectEngin
             DetectVarProcessList(det_ctx, p->flow, p);
             RULE_PROFILING_END(det_ctx, s, r, p);
         }
+
+        /* update Frame::inspect_progress here instead of in the code above. The reason is that a
+         * frame might be used more than once in buffers with transforms. */
+        if (frame->inspect_progress < det_ctx->frame_inspect_progress) {
+            frame->inspect_progress = det_ctx->frame_inspect_progress;
+            SCLogDebug("frame->inspect_progress: %" PRIu64 " -> updated", frame->inspect_progress);
+        } else {
+            SCLogDebug(
+                    "frame->inspect_progress: %" PRIu64 " -> not updated", frame->inspect_progress);
+        }
+
+        SCLogDebug("%p/%" PRIi64 " rules inspected, running cleanup", frame, frame->id);
         InspectionBufferClean(det_ctx);
     }
 }

diff --git a/src/detect.h b/src/detect.h
index e70384710d6987c920a9e44cb479ca26d5f03393..234163500a4d1a4fd52f98dc6e32e6bc06cb1a13 100644 (file)
--- a/src/detect.h
+++ b/src/detect.h
@@ -1096,6 +1096,8 @@ typedef struct DetectEngineThreadCtx_ {
     /** ID of the transaction currently being inspected. */
     uint64_t tx_id;
     int64_t frame_id;
+    uint64_t frame_inspect_progress; /**< used to set Frame::inspect_progress after all inspection
+                                        on a frame is complete. */
     Packet *p;
 
     uint16_t alert_queue_size;