Skip to content

Commit

Permalink
fixup! detect: allow rule which need both directions to match
Browse files Browse the repository at this point in the history
  • Loading branch information
catenacyber committed Oct 8, 2024
1 parent e0308a3 commit 182efd9
Show file tree
Hide file tree
Showing 12 changed files with 61 additions and 11 deletions.
2 changes: 1 addition & 1 deletion doc/userguide/rules/intro.rst
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ Now, let's say we have a rule with the following header::
Only the traffic from the client to the server will be matched by this rule,
as the direction specifies that we do not want to evaluate the response packet.

Now,if we have a rule with the following header::
Now, if we have a rule with the following header::

alert tcp 1.2.3.4 any <> 5.6.7.8 80

Expand Down
9 changes: 8 additions & 1 deletion src/detect-engine-mpm.c
Original file line number Diff line number Diff line change
Expand Up @@ -1071,12 +1071,14 @@ static SigMatch *GetMpmForList(const Signature *s, SigMatch *list, SigMatch *mpm

int g_skip_prefilter = 0;

// tells if a buffer id is only used to client
bool DetectBufferToClient(const DetectEngineCtx *de_ctx, int buf_id, AppProto alproto)
{
bool r = false;
const DetectEngineAppInspectionEngine *app = de_ctx->app_inspect_engines;
for (; app != NULL; app = app->next) {
if (app->sm_list == buf_id && (AppProtoEquals(alproto, app->alproto) || alproto == 0)) {
if (app->sm_list == buf_id &&
(AppProtoEquals(alproto, app->alproto) || alproto == ALPROTO_UNKNOWN)) {
if (app->dir == 1) {
// do not return yet in case we have app engines on both sides
r = true;
Expand Down Expand Up @@ -1232,7 +1234,12 @@ void RetrieveFPForSig(const DetectEngineCtx *de_ctx, Signature *s)
}
} else {
for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
if (s->init_data->buffers[x].only_tc) {
// prefer to choose a fast_pattern to server by default
continue;
}
const int list_id = s->init_data->buffers[x].id;

if (final_sm_list[i] == list_id) {
SCLogDebug("%u: list_id %d: %s", s->id, list_id,
DetectEngineBufferTypeGetNameById(de_ctx, list_id));
Expand Down
12 changes: 6 additions & 6 deletions src/detect-engine-state.c
Original file line number Diff line number Diff line change
Expand Up @@ -89,9 +89,9 @@ static DeStateStore *DeStateStoreAlloc(void)
}

#ifdef DEBUG_VALIDATION
static int DeStateSearchState(DetectEngineState *state, uint8_t dsi, SigIntId num)
static int DeStateSearchState(DetectEngineState *state, uint8_t dir_state_index, SigIntId num)
{
DetectEngineStateDirection *dir_state = &state->dir_state[dsi];
DetectEngineStateDirection *dir_state = &state->dir_state[dir_state_index];
DeStateStore *tx_store = dir_state->head;
SigIntId store_cnt;
SigIntId state_cnt = 0;
Expand Down Expand Up @@ -120,15 +120,15 @@ static void DeStateSignatureAppend(DetectEngineState *state,
{
SCEnter();

uint8_t dsi = (direction & STREAM_TOSERVER) ? 0 : 1;
uint8_t dir_state_index = (direction & STREAM_TOSERVER) ? 0 : 1;
if (s->flags & SIG_FLAG_BOTHDIR) {
dsi = DETECT_ENGINE_STATE_DIRECTION_BOTHDIR;
dir_state_index = DETECT_ENGINE_STATE_DIRECTION_BOTHDIR;
}

DetectEngineStateDirection *dir_state = &state->dir_state[dsi];
DetectEngineStateDirection *dir_state = &state->dir_state[dir_state_index];

#ifdef DEBUG_VALIDATION
BUG_ON(DeStateSearchState(state, dsi, s->num));
BUG_ON(DeStateSearchState(state, dir_state_index, s->num));
#endif
DeStateStore *store = dir_state->tail;
if (store == NULL) {
Expand Down
2 changes: 2 additions & 0 deletions src/detect-engine.c
Original file line number Diff line number Diff line change
Expand Up @@ -1360,6 +1360,8 @@ bool DetectBufferIsPresent(const Signature *s, const uint32_t buf_id)
return false;
}

// Tells if a buffer (from its list id) is ambiguous about directions
// meaning it can match on both to client and to server, like http.connection for example
static bool DetectEngineBufferAmbiguousDir(
DetectEngineCtx *de_ctx, const int list, AppProto alproto)
{
Expand Down
3 changes: 1 addition & 2 deletions src/detect-fast-pattern.c
Original file line number Diff line number Diff line change
Expand Up @@ -244,9 +244,8 @@ static int DetectFastPatternSetup(DetectEngineCtx *de_ctx, Signature *s, const c
"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;
}
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT;
}
}

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

s->init_data->init_flags |= SIG_FLAG_INIT_FILEDATA;
if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
// we cannot use a bidirectional rule with a fast pattern to client and this
if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT) {
SCLogError("fast_pattern cannot be used on to_client keyword for "
"bidirectional rule with a streaming buffer to server %u",
s->id);
return -1;
}
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
}
SetupDetectEngineConfig(de_ctx);
Expand Down
7 changes: 7 additions & 0 deletions src/detect-file-hash-common.c
Original file line number Diff line number Diff line change
Expand Up @@ -333,6 +333,13 @@ int DetectFileHashSetup(

s->file_flags |= FILE_SIG_NEED_FILE;
if ((s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) == 0) {
// we cannot use a bidirectional rule with a fast pattern to client and this
if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT) {
SCLogError("fast_pattern cannot be used on to_client keyword for "
"bidirectional rule with a streaming buffer to server %u",
s->id);
goto error;
}
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
}

Expand Down
7 changes: 7 additions & 0 deletions src/detect-filemagic.c
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,13 @@ 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) {
// we cannot use a bidirectional rule with a fast pattern to client and this
if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT) {
SCLogError("fast_pattern cannot be used on to_client keyword for "
"bidirectional rule with a streaming buffer to server %u",
s->id);
SCReturnInt(-1);
}
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
}

Expand Down
7 changes: 7 additions & 0 deletions src/detect-filename.c
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,13 @@ 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) {
// we cannot use a bidirectional rule with a fast pattern to client and this
if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT) {
SCLogError("fast_pattern cannot be used on to_client keyword for "
"bidirectional rule with a streaming buffer to server %u",
s->id);
SCReturnInt(-1);
}
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
}

Expand Down
8 changes: 8 additions & 0 deletions src/detect-filesize.c
Original file line number Diff line number Diff line change
Expand Up @@ -134,6 +134,14 @@ 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) {
// we cannot use a bidirectional rule with a fast pattern to client and this
if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT) {
SCLogError("fast_pattern cannot be used on to_client keyword for "
"bidirectional rule with a streaming buffer to server %u",
s->id);
DetectFilesizeFree(de_ctx, fsd);
SCReturnInt(-1);
}
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
}
SCReturnInt(0);
Expand Down
6 changes: 6 additions & 0 deletions src/detect-http-client-body.c
Original file line number Diff line number Diff line change
Expand Up @@ -168,6 +168,12 @@ static int DetectHttpClientBodySetupSticky(DetectEngineCtx *de_ctx, Signature *s
if (DetectSignatureSetAppProto(s, ALPROTO_HTTP) < 0)
return -1;
// we cannot use a bidirectional rule with a fast pattern to client and this
if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_FAST_TOCLIENT) {
SCLogError("fast_pattern cannot be used on to_client keyword for "
"bidirectional rule with a streaming buffer to server %u",
s->id);
return -1;
}
s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_STREAMING_TOSERVER;
return 0;
}
Expand Down
2 changes: 1 addition & 1 deletion src/detect.c
Original file line number Diff line number Diff line change
Expand Up @@ -1139,7 +1139,7 @@ static bool DetectRunTxInspectRule(ThreadVars *tv,
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
// this is a bidirectional signature, and we are toclient
if (!(inspect_flags & BIT_U32(engine->id)) &&
(direction == engine->dir || ((s->flags & SIG_FLAG_BOTHDIR) && direction == 1))) {

Expand Down

0 comments on commit 182efd9

Please sign in to comment.