From 63caa0b40a66ecf1a34bbb6d942d4a044b7728a5 Mon Sep 17 00:00:00 2001
From: Philippe Antoine <pantoine@oisf.net>
Date: Thu, 25 Jan 2024 14:26:09 +0100
Subject: [PATCH] detect: respect directionality for filestore

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 | 25 +++++++++++++++++++------
 src/flow.h             |  5 +++--
 src/util-file.c        | 13 ++++++++-----
 3 files changed, 30 insertions(+), 13 deletions(-)

diff --git a/src/detect-filestore.c b/src/detect-filestore.c
index c510544469..fa8492161e 100644
--- a/src/detect-filestore.c
+++ b/src/detect-filestore.c
@@ -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);
diff --git a/src/flow.h b/src/flow.h
index c7b5867ea8..418becfc29 100644
--- a/src/flow.h
+++ b/src/flow.h
@@ -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 | \
diff --git a/src/util-file.c b/src/util-file.c
index 3221d11687..a1c30d89ec 100644
--- a/src/util-file.c
+++ b/src/util-file.c
@@ -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);
-- 
2.47.2