From: Victor Julien Date: Thu, 8 Sep 2022 11:56:53 +0000 (+0200) Subject: detect/frame: improve frame detection X-Git-Tag: suricata-7.0.0-rc1~109 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=aa376a3b216bce5766ebb249f5ef63a0efce42f6;p=thirdparty%2Fsuricata.git 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 f68f2c36ee..bebb4ad500 100644 --- 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 c0d1fb27c3..98ddefede7 100644 --- 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 739ef6cc81..1046aede45 100644 --- 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 9fdd2e9c08..76b28ff394 100644 --- 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 e70384710d..234163500a 100644 --- 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;