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.

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");
     }
 }
 

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);
 }

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;

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

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 */