]> 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>
Wed, 14 Feb 2024 20:20:51 +0000 (21:20 +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")
src/detect-filestore.c
src/flow.h
src/util-file.c

index c510544469aab2ff1d86a32a74249fc0f020daa0..fa8492161e616c61699349a814e6fe55b05009f0 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 c7b5867ea896357f8fb1adf6a2350dddfe69617f..418becfc296383289166591a608e02be5a3e8887 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 3221d116870d7e2e0f51ab295282149d5cc1bfc1..a1c30d89ecded97b38067206603e98e556c4f415 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);