Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Detect bidir 5665 v13 #11323

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions doc/userguide/rules/intro.rst
Original file line number Diff line number Diff line change
Expand Up @@ -278,8 +278,9 @@ Bidirectional rules can use direction-abmibuous keywords, by first using
Bidirectional rules have some limitations :
- They are only meant to work on transactions with first a request to the server,
and then a response to the client, and not the other way around.
- They cannot have ``fast_pattern`` or ``prefilter`` on a keyword which is on
the direction to client.
- They cann have ``fast_pattern`` or ``prefilter`` on a keyword which is on
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: s/cann/can

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cannot ;-)

the direction to client only if they do not have any have streaming buffers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
the direction to client only if they do not have any have streaming buffers
the direction to client only if they do not have any streaming buffers

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if they do have ;-)

for detection on the other side.
- They will refuse to load if a single directional rule is enough.

Rule options
Expand Down
1 change: 1 addition & 0 deletions src/detect-engine-mpm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1187,6 +1187,7 @@ void RetrieveFPForSig(const DetectEngineCtx *de_ctx, Signature *s)
tmp = tmp->next)
{
if (s->flags & SIG_FLAG_BOTHDIR) {
// prefer to choose a fast_pattern to server by default
if (DetectBufferToClient(de_ctx, tmp->list_id, s->alproto)) {
continue;
}
Expand Down
12 changes: 8 additions & 4 deletions src/detect-fast-pattern.c
Original file line number Diff line number Diff line change
Expand Up @@ -239,10 +239,14 @@ static int DetectFastPatternSetup(DetectEngineCtx *de_ctx, Signature *s, const c

if (s->flags & SIG_FLAG_BOTHDIR && s->init_data->curbuf != NULL) {
if (DetectBufferToClient(de_ctx, s->init_data->curbuf->id, s->alproto)) {
SCLogError("fast_pattern cannot be used on to_client keyword for "
"bidirectional rule %u",
s->id);
goto error;
if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER) {
SCLogError("fast_pattern cannot be used on to_client keyword for "
"bidirectional rule with a streaming buffer to server %u",
s->id);
goto error;
} else {
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT;
}
}
}

Expand Down
3 changes: 3 additions & 0 deletions src/detect-file-data.c
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,9 @@ static int DetectFiledataSetup (DetectEngineCtx *de_ctx, Signature *s, const cha
return -1;

s->init_data->init_flags |= SIG_FLAG_INIT_FILEDATA;
if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
}
SetupDetectEngineConfig(de_ctx);
return 0;
}
Expand Down
3 changes: 3 additions & 0 deletions src/detect-file-hash-common.c
Original file line number Diff line number Diff line change
Expand Up @@ -332,6 +332,9 @@ int DetectFileHashSetup(
}

s->file_flags |= FILE_SIG_NEED_FILE;
if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
}

// Setup the file flags depending on the hashing algorithm
if (type == DETECT_FILEMD5) {
Expand Down
3 changes: 3 additions & 0 deletions src/detect-filemagic.c
Original file line number Diff line number Diff line change
Expand Up @@ -211,6 +211,9 @@ static int DetectFilemagicSetup (DetectEngineCtx *de_ctx, Signature *s, const ch
}
s->init_data->list = DETECT_SM_LIST_NOTSET;
s->file_flags |= (FILE_SIG_NEED_FILE | FILE_SIG_NEED_MAGIC);
if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
}

if (DetectContentSetup(de_ctx, s, str) < 0) {
return -1;
Expand Down
9 changes: 9 additions & 0 deletions src/detect-filename.c
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,9 @@ static int DetectFileextSetup(DetectEngineCtx *de_ctx, Signature *s, const char
}
s->init_data->list = DETECT_SM_LIST_NOTSET;
s->file_flags |= (FILE_SIG_NEED_FILE | FILE_SIG_NEED_FILENAME);
if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
}

size_t dotstr_len = strlen(str) + 2;
char *dotstr = SCCalloc(1, dotstr_len);
Expand Down Expand Up @@ -175,6 +178,9 @@ static int DetectFilenameSetup (DetectEngineCtx *de_ctx, Signature *s, const cha
}
s->init_data->list = DETECT_SM_LIST_NOTSET;
s->file_flags |= (FILE_SIG_NEED_FILE | FILE_SIG_NEED_FILENAME);
if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
}

if (DetectContentSetup(de_ctx, s, str) < 0) {
return -1;
Expand Down Expand Up @@ -210,6 +216,9 @@ static int DetectFilenameSetupSticky(DetectEngineCtx *de_ctx, Signature *s, cons
if (DetectBufferSetActiveList(de_ctx, s, g_file_name_buffer_id) < 0)
return -1;
s->file_flags |= (FILE_SIG_NEED_FILE | FILE_SIG_NEED_FILENAME);
if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
}
return 0;
}

Expand Down
3 changes: 3 additions & 0 deletions src/detect-filesize.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,9 @@ static int DetectFilesizeSetup (DetectEngineCtx *de_ctx, Signature *s, const cha
}

s->file_flags |= (FILE_SIG_NEED_FILE|FILE_SIG_NEED_SIZE);
if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
}
SCReturnInt(0);

error:
Expand Down
2 changes: 2 additions & 0 deletions src/detect-http-client-body.c
Original file line number Diff line number Diff line change
Expand Up @@ -167,6 +167,8 @@ static int DetectHttpClientBodySetupSticky(DetectEngineCtx *de_ctx, Signature *s
return -1;
if (DetectSignatureSetAppProto(s, ALPROTO_HTTP) < 0)
return -1;
// we cannot use a bidirectional rule with a fast pattern to client and this
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
return 0;
}

Expand Down
14 changes: 9 additions & 5 deletions src/detect-prefilter.c
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,15 @@ static int DetectPrefilterSetup (DetectEngineCtx *de_ctx, Signature *s, const ch
* 'fast_pattern' w/o options. */
if (sm->type == DETECT_CONTENT) {
if (s->flags & SIG_FLAG_BOTHDIR && s->init_data->curbuf != NULL) {
if (DetectBufferToClient(de_ctx, s->init_data->curbuf->id, s->alproto)) {
SCLogError("prefilter cannot be used on to_client keyword for "
"bidirectional rule %u",
s->id);
SCReturnInt(-1);
if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER) {
if (DetectBufferToClient(de_ctx, s->init_data->curbuf->id, s->alproto)) {
SCLogError("prefilter cannot be used on to_client keyword for "
"bidirectional rule %u",
s->id);
SCReturnInt(-1);
} else {
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT;
}
}
}

Expand Down
13 changes: 9 additions & 4 deletions src/detect.c
Original file line number Diff line number Diff line change
Expand Up @@ -1109,9 +1109,10 @@ static bool DetectRunTxInspectRule(ThreadVars *tv,
const DetectEngineAppInspectionEngine *engine = s->app_inspect;
do {
TRACE_SID_TXS(s->id, tx, "engine %p inspect_flags %x", engine, inspect_flags);
// also if it is not the same direction, but
// this is a bidrectional signature, and we are toclient
if (!(inspect_flags & BIT_U32(engine->id)) &&
direction == engine->dir)
{
(direction == engine->dir || ((s->flags & SIG_FLAG_BOTHDIR) && direction == 1))) {
const bool skip_engine = (engine->alproto != 0 && engine->alproto != f->alproto);
/* special case: file_data on 'alert tcp' will have engines
* in the list that are not for us. */
Expand Down Expand Up @@ -1143,6 +1144,10 @@ static bool DetectRunTxInspectRule(ThreadVars *tv,
}
}

uint8_t engine_flags = flow_flags;
if (direction != engine->dir) {
engine_flags = flow_flags ^ (STREAM_TOCLIENT | STREAM_TOSERVER);
}
/* run callback: but bypass stream callback if we can */
uint8_t match;
if (unlikely(engine->stream && can->stream_stored)) {
Expand All @@ -1151,8 +1156,8 @@ static bool DetectRunTxInspectRule(ThreadVars *tv,
} else {
KEYWORD_PROFILING_SET_LIST(det_ctx, engine->sm_list);
DEBUG_VALIDATE_BUG_ON(engine->v2.Callback == NULL);
match = engine->v2.Callback(
de_ctx, det_ctx, engine, s, f, flow_flags, alstate, tx->tx_ptr, tx->tx_id);
match = engine->v2.Callback(de_ctx, det_ctx, engine, s, f, engine_flags, alstate,
tx->tx_ptr, tx->tx_id);
TRACE_SID_TXS(s->id, tx, "engine %p match %d", engine, match);
if (engine->stream) {
can->stream_stored = true;
Expand Down
5 changes: 5 additions & 0 deletions src/detect.h
Original file line number Diff line number Diff line change
Expand Up @@ -298,6 +298,11 @@ typedef struct DetectPort_ {
#define SIG_FLAG_INIT_JA BIT_U32(10) /**< signature has ja3/ja4 keyword */
#define SIG_FLAG_INIT_BIDIR_TOCLIENT BIT_U32(11) /**< signature now takes keywords toclient */
#define SIG_FLAG_INIT_BIDIR_TOSERVER BIT_U32(12) /**< signature now takes keywords toserver */
// Two following flags are meant to be mutually exclusive
#define SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER \
BIT_U32(13) /**< bidirectional signature uses a streaming buffer to server */
#define SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT \
BIT_U32(14) /**< bidirectional signature uses a fast pattern to client */

/* signature mask flags */
/** \note: additions should be added to the rule analyzer as well */
Expand Down