Skip to content

Commit

Permalink
detect: respect directionality for filestore
Browse files Browse the repository at this point in the history
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: 79499e4 ("app-layer: move files into transactions")
  • Loading branch information
catenacyber committed Jan 25, 2024
1 parent c3b3c11 commit 307882f
Show file tree
Hide file tree
Showing 3 changed files with 30 additions and 13 deletions.
25 changes: 19 additions & 6 deletions src/detect-filestore.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions src/flow.h
Original file line number Diff line number Diff line change
Expand Up @@ -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 | \
Expand Down
13 changes: 8 additions & 5 deletions src/util-file.c
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand All @@ -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);
Expand Down

0 comments on commit 307882f

Please sign in to comment.