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 0444610a8ae8cc19cd2d4a04263ffad679a9824b..bbfc1a5e29bc84e625744b467351fe0c4d36f73d 100644 (file)
--- 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 8c7082c53f82ac960ce07e100c029fd693bea043..e50b4ce964dd1a2f3162bcfd649ce810d3b85454 100644 (file)
--- 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 45449ee1b337a7fd1fb0ee7a896f49f60e8440a4..436fdf125966b483e23f6f92f178bb7b270a393b 100644 (file)
--- 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 c8944cffd17cca5161d6503767bb2f8903d58e6c..c5264816ee5ba74b9a57bee24cc951ba2944a4b0 100644 (file)
--- 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 2b45f3360769214f534c490f13dc089db1026b89..c9aab3e1266bc79e9ee7f50cba205f002dbe4942 100644 (file)
--- 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 */