From: Victor Julien Date: Thu, 21 Apr 2016 11:17:33 +0000 (+0200) Subject: detect file: improve multi file handling X-Git-Tag: suricata-3.1RC1~229 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c4a9580fce91bafc5e2cfcf366dd50a3ec16eaaa;p=thirdparty%2Fsuricata.git detect file: improve multi file handling When multiple files were in a tx, the first one(s) closed/complete and a new open one as well, a match in the former could lead to not inspecting the latter. This patch adds a workaround for this case, by allowing the file inspection code to return a special code for 'match, but more files available in tx'. The stateful detection engine will then not make this match final for the tx. It relies on the file pruning to kick in to make sure the already complete files are removed from the tx before the next time the detection engine is called on the tx. --- diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c index 0444610a8a..bbfc1a5e29 100644 --- a/src/app-layer-smtp.c +++ b/src/app-layer-smtp.c @@ -360,6 +360,7 @@ static void SMTPPruneFiles(FileContainer *files) File *file = files->head; while (file) { + SCLogDebug("file %p, file->chunks_head %p", file, file->chunks_head); if (file->chunks_head) { uint32_t window = smtp_config.content_inspect_window; if (file->chunks_head->stream_offset == 0) @@ -387,6 +388,10 @@ static void FlagDetectStateNewFile(SMTPTransaction *tx) if (tx && tx->de_state) { SCLogDebug("DETECT_ENGINE_STATE_FLAG_FILE_TS_NEW set"); tx->de_state->dir_state[0].flags |= DETECT_ENGINE_STATE_FLAG_FILE_TS_NEW; + } else if (tx == NULL) { + SCLogDebug("DETECT_ENGINE_STATE_FLAG_FILE_TS_NEW NOT set, no TX"); + } else if (tx->de_state == NULL) { + SCLogDebug("DETECT_ENGINE_STATE_FLAG_FILE_TS_NEW NOT set, no TX DESTATE"); } } diff --git a/src/detect-engine-file.c b/src/detect-engine-file.c index 8c7082c53f..e50b4ce964 100644 --- a/src/detect-engine-file.c +++ b/src/detect-engine-file.c @@ -88,7 +88,6 @@ static int DetectFileInspect(ThreadVars *tv, DetectEngineThreadCtx *det_ctx, File *file = ffc->head; for (; file != NULL; file = file->next) { SCLogDebug("file"); - if (file->state == FILE_STATE_NONE) { SCLogDebug("file state FILE_STATE_NONE"); continue; @@ -188,6 +187,11 @@ static int DetectFileInspect(ThreadVars *tv, DetectEngineThreadCtx *det_ctx, } } + if (r == DETECT_ENGINE_INSPECT_SIG_NO_MATCH && store_r == DETECT_ENGINE_INSPECT_SIG_MATCH) { + SCLogDebug("stored MATCH, current file NOMATCH"); + SCReturnInt(DETECT_ENGINE_INSPECT_SIG_MATCH_MORE_FILES); + } + if (store_r == DETECT_ENGINE_INSPECT_SIG_MATCH) r = DETECT_ENGINE_INSPECT_SIG_MATCH; SCReturnInt(r); @@ -285,9 +289,11 @@ int DetectFileInspectSmtp(ThreadVars *tv, } else if (match == DETECT_ENGINE_INSPECT_SIG_CANT_MATCH_FILESTORE) { SCLogDebug("sid %u can't match on this transaction (filestore sig)", s->id); r = DETECT_ENGINE_INSPECT_SIG_CANT_MATCH_FILESTORE; + } else if (match == DETECT_ENGINE_INSPECT_SIG_MATCH_MORE_FILES) { + SCLogDebug("match with more files ahead"); + r = match; } - end: SCReturnInt(r); } diff --git a/src/detect-engine-state.c b/src/detect-engine-state.c index 45449ee1b3..436fdf1259 100644 --- a/src/detect-engine-state.c +++ b/src/detect-engine-state.c @@ -544,6 +544,13 @@ int DeStateDetectStartDetection(ThreadVars *tv, DetectEngineCtx *de_ctx, engine = engine->next; total_matches++; continue; + } else if (match == DETECT_ENGINE_INSPECT_SIG_MATCH_MORE_FILES) { + /* if the file engine matched, but indicated more + * files are still in progress, we don't set inspect + * flags as these would end inspection for this tx */ + engine = engine->next; + total_matches++; + continue; } else if (match == DETECT_ENGINE_INSPECT_SIG_CANT_MATCH) { inspect_flags |= DE_STATE_FLAG_SIG_CANT_MATCH; inspect_flags |= engine->inspect_flags; @@ -852,6 +859,13 @@ static int DoInspectItem(ThreadVars *tv, engine = engine->next; total_matches++; continue; + } else if (match == DETECT_ENGINE_INSPECT_SIG_MATCH_MORE_FILES) { + /* if the file engine matched, but indicated more + * files are still in progress, we don't set inspect + * flags as these would end inspection for this tx */ + engine = engine->next; + total_matches++; + continue; } else if (match == DETECT_ENGINE_INSPECT_SIG_CANT_MATCH) { inspect_flags |= DE_STATE_FLAG_SIG_CANT_MATCH; inspect_flags |= engine->inspect_flags; diff --git a/src/detect-engine-state.h b/src/detect-engine-state.h index c8944cffd1..c5264816ee 100644 --- a/src/detect-engine-state.h +++ b/src/detect-engine-state.h @@ -52,6 +52,12 @@ #define DETECT_ENGINE_INSPECT_SIG_MATCH 1 #define DETECT_ENGINE_INSPECT_SIG_CANT_MATCH 2 #define DETECT_ENGINE_INSPECT_SIG_CANT_MATCH_FILESTORE 3 +/** hack to work around a file inspection limitation. Since there can be + * multiple files in a TX and the detection engine really don't know + * about that, we have to give the file inspection engine a way to + * indicate that one of the files matched, but that there are still + * more files that have ongoing inspection. */ +#define DETECT_ENGINE_INSPECT_SIG_MATCH_MORE_FILES 4 /** number of DeStateStoreItem's in one DeStateStore object */ #define DE_STATE_CHUNK_SIZE 15 diff --git a/src/util-file.c b/src/util-file.c index 2b45f33607..c9aab3e126 100644 --- a/src/util-file.c +++ b/src/util-file.c @@ -199,6 +199,8 @@ static int FilePruneFile(File *file) } } + SCLogDebug("file->state %d. Is >= FILE_STATE_CLOSED: %s", file->state, (file->state >= FILE_STATE_CLOSED) ? "yes" : "no"); + /* file is done when state is closed+, logging/storing is done (if any) */ if (file->state >= FILE_STATE_CLOSED && (!RunModeOutputFileEnabled() || (file->flags & FILE_LOGGED)) && @@ -220,6 +222,8 @@ void FilePrune(FileContainer *ffc) BUG_ON(file != ffc->head); + SCLogDebug("removing file %p", file); + File *file_next = file->next; /* update head and tail */