]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
detect: respect directionality for filestore
authorPhilippe Antoine <pantoine@oisf.net>
Thu, 25 Jan 2024 13:26:09 +0000 (14:26 +0100)
committerVictor Julien <victor@inliniac.net>
Thu, 15 Feb 2024 14:34:04 +0000 (15:34 +0100)
Ticket: 6617

So that rules with keyword like `filestore:to_server,flow`
only store the files to server and not the ones to client...

Directionality only worked with the default scope, ie the
current file, and not the scope tx or scope flow.
For non-default scope, tx or flow, both directions were stored
whatever the directionality specified.

For these non-default scopes, this commit keeps a default
of both directions, but use only one direction if specified.

Need to split flag FLOWFILE_STORE per direction, so that Suricata
can retain this (optional) directional info from the filestore
keyword.

Fixes: 79499e476979 ("app-layer: move files into transactions")
(cherry picked from commit 63caa0b40a66ecf1a34bbb6d942d4a044b7728a5)

src/detect-filestore.c
src/flow.h
src/util-file.c

index 03bdbba98c7f46700f01e938182968cf8b747e64..c905f9b88d48a16a4fcc1d90078c0c7c978ebaa0 100644 (file)
@@ -118,7 +118,8 @@ static int FilestorePostMatchWithOptions(Packet *p, Flow *f, const DetectFilesto
     switch (filestore->direction) {
         case FILESTORE_DIR_DEFAULT:
             rule_dir = 1;
-            break;
+            // will use both sides if scope is not default
+            // fallthrough
         case FILESTORE_DIR_BOTH:
             toserver_dir = 1;
             toclient_dir = 1;
@@ -160,16 +161,28 @@ static int FilestorePostMatchWithOptions(Packet *p, Flow *f, const DetectFilesto
             AppLayerTxData *txd = AppLayerParserGetTxData(f->proto, f->alproto, txv);
             DEBUG_VALIDATE_BUG_ON(txd == NULL);
             if (txd != NULL) {
-                txd->file_flags |= FLOWFILE_STORE;
+                if (toclient_dir) {
+                    txd->file_flags |= FLOWFILE_STORE_TC;
+                }
+                if (toserver_dir) {
+                    txd->file_flags |= FLOWFILE_STORE_TS;
+                }
             }
         }
     } else if (this_flow) {
         /* set in flow and AppLayerStateData */
-        f->file_flags |= FLOWFILE_STORE;
-
         AppLayerStateData *sd = AppLayerParserGetStateData(f->proto, f->alproto, f->alstate);
-        if (sd != NULL) {
-            sd->file_flags |= FLOWFILE_STORE;
+        if (toclient_dir) {
+            f->file_flags |= FLOWFILE_STORE_TC;
+            if (sd != NULL) {
+                sd->file_flags |= FLOWFILE_STORE_TC;
+            }
+        }
+        if (toserver_dir) {
+            f->file_flags |= FLOWFILE_STORE_TS;
+            if (sd != NULL) {
+                sd->file_flags |= FLOWFILE_STORE_TS;
+            }
         }
     } else {
         FileStoreFileById(fc, file_id);
index 0a730e0ea3b8d59254d543a36dadb695203a4626..9866b568560b766450991a694787e46ed06ffcac 100644 (file)
@@ -142,8 +142,9 @@ typedef struct AppLayerParserState_ AppLayerParserState;
 #define FLOWFILE_NO_SIZE_TS             BIT_U16(10)
 #define FLOWFILE_NO_SIZE_TC             BIT_U16(11)
 
-/** store all files in the flow */
-#define FLOWFILE_STORE BIT_U16(12)
+/** store files in the flow */
+#define FLOWFILE_STORE_TS BIT_U16(12)
+#define FLOWFILE_STORE_TC BIT_U16(13)
 
 #define FLOWFILE_NONE_TS (FLOWFILE_NO_MAGIC_TS | \
                           FLOWFILE_NO_STORE_TS | \
index 0449a2edae6c09409f89c4c21f02350c662fbb1c..89ef50c1d4ebb633412678e0c1d857942cf0f3a3 100644 (file)
@@ -235,8 +235,11 @@ uint16_t FileFlowFlagsToFlags(const uint16_t flow_file_flags, uint8_t direction)
     uint16_t flags = 0;
 
     if (direction == STREAM_TOSERVER) {
-        if ((flow_file_flags & (FLOWFILE_NO_STORE_TS | FLOWFILE_STORE)) == FLOWFILE_NO_STORE_TS) {
+        if ((flow_file_flags & (FLOWFILE_NO_STORE_TS | FLOWFILE_STORE_TS)) ==
+                FLOWFILE_NO_STORE_TS) {
             flags |= FILE_NOSTORE;
+        } else if (flow_file_flags & FLOWFILE_STORE_TS) {
+            flags |= FILE_STORE;
         }
 
         if (flow_file_flags & FLOWFILE_NO_MAGIC_TS) {
@@ -255,8 +258,11 @@ uint16_t FileFlowFlagsToFlags(const uint16_t flow_file_flags, uint8_t direction)
             flags |= FILE_NOSHA256;
         }
     } else {
-        if ((flow_file_flags & (FLOWFILE_NO_STORE_TC | FLOWFILE_STORE)) == FLOWFILE_NO_STORE_TC) {
+        if ((flow_file_flags & (FLOWFILE_NO_STORE_TC | FLOWFILE_STORE_TC)) ==
+                FLOWFILE_NO_STORE_TC) {
             flags |= FILE_NOSTORE;
+        } else if (flow_file_flags & FLOWFILE_STORE_TC) {
+            flags |= FILE_STORE;
         }
 
         if (flow_file_flags & FLOWFILE_NO_MAGIC_TC) {
@@ -275,9 +281,6 @@ uint16_t FileFlowFlagsToFlags(const uint16_t flow_file_flags, uint8_t direction)
             flags |= FILE_NOSHA256;
         }
     }
-    if (flow_file_flags & FLOWFILE_STORE) {
-        flags |= FILE_STORE;
-    }
     DEBUG_VALIDATE_BUG_ON((flags & (FILE_STORE | FILE_NOSTORE)) == (FILE_STORE | FILE_NOSTORE));
 
     SCLogDebug("direction %02x flags %02x", direction, flags);