From e0308a3a438878964b9711bf844215be2db1521c Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Thu, 18 Jan 2024 14:15:14 +0100 Subject: [PATCH] detect: allow rule which need both directions to match Ticket: 5665 This is done with `alert ip any any => any any` The => operator means that we will need both directions --- doc/userguide/rules/intro.rst | 53 ++++++++++++- src/app-layer-htp.c | 2 + src/app-layer-smtp.c | 2 + src/detect-engine-mpm.c | 24 ++++++ src/detect-engine-mpm.h | 2 + src/detect-engine-prefilter.h | 2 + src/detect-engine-register.c | 1 + src/detect-engine-register.h | 3 + src/detect-engine-state.c | 34 ++++----- src/detect-engine-state.h | 11 ++- src/detect-engine.c | 45 +++++++++++ src/detect-fast-pattern.c | 13 ++++ src/detect-file-data.c | 3 + src/detect-file-hash-common.c | 3 + src/detect-filemagic.c | 3 + src/detect-filename.c | 9 +++ src/detect-filesize.c | 3 + src/detect-flow.c | 52 +++++++++++++ src/detect-flow.h | 2 + src/detect-http-client-body.c | 2 + src/detect-parse.c | 36 +++++++-- src/detect-prefilter.c | 14 ++++ src/detect.c | 139 ++++++++++++++++++++-------------- src/detect.h | 10 +++ 24 files changed, 386 insertions(+), 82 deletions(-) diff --git a/doc/userguide/rules/intro.rst b/doc/userguide/rules/intro.rst index 56df9ab49437..943600c54d06 100644 --- a/doc/userguide/rules/intro.rst +++ b/doc/userguide/rules/intro.rst @@ -228,11 +228,14 @@ Direction The directional arrow indicates which way the signature will be evaluated. In most signatures an arrow to the right (``->``) is used. This means that only -packets with the same direction can match. However, it is also possible to -have a rule match both directions (``<>``):: +packets with the same direction can match. +There is also the double arrow (``=>``), which respects the directionality as ``->``, +but allows matching on bidirectional transactions, used with keywords matching each direction. +Finally, it is also possible to have a rule match either directions (``<>``):: source -> destination - source <> destination (both directions) + source => destination + source <> destination (either directions) The following example illustrates direction. In this example there is a client with IP address 1.2.3.4 using port 1024. A server with IP address 5.6.7.8, @@ -248,10 +251,54 @@ 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:: + + alert tcp 1.2.3.4 any <> 5.6.7.8 80 + +Suricata will duplicate it and use the same rule with headers in both directions : + + alert tcp 1.2.3.4 any -> 5.6.7.8 80 + alert tcp 5.6.7.8 80 -> 1.2.3.4 any + .. warning:: There is no 'reverse' style direction, i.e. there is no ``<-``. +Bidirectional rules +~~~~~~~~~~~~~~~~~~~ + +Here is an example of a bidirectional rule: + +.. container:: example-rule + + alert http any any :example-rule-emphasis:`=>` 5.6.7.8 80 (msg:"matching both uri and status"; sid: 1; http.uri; content: "/download"; http.stat_code; content: "200";) + +It will match on flows to 5.6.7.8 and port 80. +And it will match on a full transaction, using both the uri from the request, +and the stat_code from the response. +As such, it will match only when Suricata got both request and response. + +Bidirectional rules can use direction-ambiguous keywords, by first using +``bidir.toclient`` or ``bidir.toserver`` keywords. + +.. container:: example-rule + + alert http any any => 5.6.7.8 80 (msg:"matching json to server and xml to client"; sid: 1; :example-rule-emphasis:`bidir.toserver;` http.content_type; content: "json"; :example-rule-emphasis:`bidir.toclient;` http.content_type; content: "xml";) + +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 (not tested). +* They cannot have ``fast_pattern`` or ``prefilter`` the direction to client + if they also have a streaming buffer on the direction to server, see example below. +* They will refuse to load if a single directional rule is enough. + +This rule cannot have the ``fast_pattern`` to client, as ``file.data`` is a streaming buffer. + +.. container:: example-rule + + alert http any any => any any (bidir.toserver; file.data; content: "123"; http.stat_code; content: "500"; fast_patten;) + Rule options ------------ The rest of the rule consists of options. These are enclosed by parenthesis diff --git a/src/app-layer-htp.c b/src/app-layer-htp.c index 3f3037ca30a1..e8ab0e255730 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -1172,6 +1172,8 @@ static void FlagDetectStateNewFile(HtpTxUserData *tx, int dir) SCLogDebug("DETECT_ENGINE_STATE_FLAG_FILE_NEW set"); tx->tx_data.de_state->dir_state[1].flags |= DETECT_ENGINE_STATE_FLAG_FILE_NEW; } + tx->tx_data.de_state->dir_state[DETECT_ENGINE_STATE_DIRECTION_BOTHDIR].flags |= + DETECT_ENGINE_STATE_FLAG_FILE_NEW; } } diff --git a/src/app-layer-smtp.c b/src/app-layer-smtp.c index 03260bfd3ae8..76ecd29864a6 100644 --- a/src/app-layer-smtp.c +++ b/src/app-layer-smtp.c @@ -495,6 +495,8 @@ static void FlagDetectStateNewFile(SMTPTransaction *tx) if (tx && tx->tx_data.de_state) { SCLogDebug("DETECT_ENGINE_STATE_FLAG_FILE_NEW set"); tx->tx_data.de_state->dir_state[0].flags |= DETECT_ENGINE_STATE_FLAG_FILE_NEW; + tx->tx_data.de_state->dir_state[DETECT_ENGINE_STATE_DIRECTION_BOTHDIR].flags |= + DETECT_ENGINE_STATE_FLAG_FILE_NEW; } else if (tx == NULL) { SCLogDebug("DETECT_ENGINE_STATE_FLAG_FILE_NEW NOT set, no TX"); } else if (tx->tx_data.de_state == NULL) { diff --git a/src/detect-engine-mpm.c b/src/detect-engine-mpm.c index 5e8687e34686..97d43fb8d79c 100644 --- a/src/detect-engine-mpm.c +++ b/src/detect-engine-mpm.c @@ -1071,6 +1071,24 @@ static SigMatch *GetMpmForList(const Signature *s, SigMatch *list, SigMatch *mpm int g_skip_prefilter = 0; +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->dir == 1) { + // do not return yet in case we have app engines on both sides + r = true; + } else { + // ambiguous keywords have a app-engine to server + return false; + } + } + } + return r; +} + void RetrieveFPForSig(const DetectEngineCtx *de_ctx, Signature *s) { if (g_skip_prefilter) @@ -1173,6 +1191,12 @@ void RetrieveFPForSig(const DetectEngineCtx *de_ctx, Signature *s) tmp != NULL && priority == tmp->priority; 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; + } + } SCLogDebug("tmp->list_id %d tmp->priority %d", tmp->list_id, tmp->priority); if (tmp->list_id >= nlists) continue; diff --git a/src/detect-engine-mpm.h b/src/detect-engine-mpm.h index f11b7e231eab..ab4841539d73 100644 --- a/src/detect-engine-mpm.h +++ b/src/detect-engine-mpm.h @@ -136,4 +136,6 @@ struct MpmListIdDataArgs { void EngineAnalysisAddAllRulePatterns(DetectEngineCtx *de_ctx, const Signature *s); +bool DetectBufferToClient(const DetectEngineCtx *de_ctx, int buf_id, AppProto alproto); + #endif /* SURICATA_DETECT_ENGINE_MPM_H */ diff --git a/src/detect-engine-prefilter.h b/src/detect-engine-prefilter.h index ec58594b9b00..8a81aa170be1 100644 --- a/src/detect-engine-prefilter.h +++ b/src/detect-engine-prefilter.h @@ -33,6 +33,8 @@ typedef struct DetectTransaction_ { const uint64_t tx_id; struct AppLayerTxData *tx_data_ptr; DetectEngineStateDirection *de_state; + // state for bidirectional signatures + DetectEngineStateDirection *de_state_bidir; const uint64_t detect_flags; /* detect flags get/set from/to applayer */ uint64_t prefilter_flags; /* prefilter flags for direction, to be updated by prefilter code */ const uint64_t diff --git a/src/detect-engine-register.c b/src/detect-engine-register.c index 794f680dd46f..f6b3b8478a9b 100644 --- a/src/detect-engine-register.c +++ b/src/detect-engine-register.c @@ -568,6 +568,7 @@ void SigTableSetup(void) DetectOffsetRegister(); DetectReplaceRegister(); DetectFlowRegister(); + DetectBidirRegister(); DetectFlowAgeRegister(); DetectFlowPktsToClientRegister(); DetectFlowPktsToServerRegister(); diff --git a/src/detect-engine-register.h b/src/detect-engine-register.h index 7c3b5b4514b0..8e8d215de7fc 100644 --- a/src/detect-engine-register.h +++ b/src/detect-engine-register.h @@ -302,6 +302,9 @@ enum DetectKeywordId { DETECT_PREFILTER, + DETECT_BIDIR_TOCLIENT, + DETECT_BIDIR_TOSERVER, + DETECT_TRANSFORM_COMPRESS_WHITESPACE, DETECT_TRANSFORM_STRIP_WHITESPACE, DETECT_TRANSFORM_STRIP_PSEUDO_HEADERS, diff --git a/src/detect-engine-state.c b/src/detect-engine-state.c index e7a3f9fcb3c9..9fa5f72308f8 100644 --- a/src/detect-engine-state.c +++ b/src/detect-engine-state.c @@ -89,9 +89,9 @@ static DeStateStore *DeStateStoreAlloc(void) } #ifdef DEBUG_VALIDATION -static int DeStateSearchState(DetectEngineState *state, uint8_t direction, SigIntId num) +static int DeStateSearchState(DetectEngineState *state, uint8_t dsi, SigIntId num) { - DetectEngineStateDirection *dir_state = &state->dir_state[direction & STREAM_TOSERVER ? 0 : 1]; + DetectEngineStateDirection *dir_state = &state->dir_state[dsi]; DeStateStore *tx_store = dir_state->head; SigIntId store_cnt; SigIntId state_cnt = 0; @@ -120,11 +120,15 @@ static void DeStateSignatureAppend(DetectEngineState *state, { SCEnter(); - DetectEngineStateDirection *dir_state = - &state->dir_state[(direction & STREAM_TOSERVER) ? 0 : 1]; + uint8_t dsi = (direction & STREAM_TOSERVER) ? 0 : 1; + if (s->flags & SIG_FLAG_BOTHDIR) { + dsi = DETECT_ENGINE_STATE_DIRECTION_BOTHDIR; + } + + DetectEngineStateDirection *dir_state = &state->dir_state[dsi]; #ifdef DEBUG_VALIDATION - BUG_ON(DeStateSearchState(state, direction, s->num)); + BUG_ON(DeStateSearchState(state, dsi, s->num)); #endif DeStateStore *store = dir_state->tail; if (store == NULL) { @@ -172,7 +176,7 @@ void DetectEngineStateFree(DetectEngineState *state) DeStateStore *store_next; int i = 0; - for (i = 0; i < 2; i++) { + for (i = 0; i < DETECT_ENGINE_STATE_DIRECTIONS; i++) { store = state->dir_state[i].head; while (store != NULL) { store_next = store->next; @@ -239,17 +243,13 @@ void DetectRunStoreStateTx( static inline void ResetTxState(DetectEngineState *s) { if (s) { - s->dir_state[0].cnt = 0; - s->dir_state[0].filestore_cnt = 0; - s->dir_state[0].flags = 0; - /* reset 'cur' back to the list head */ - s->dir_state[0].cur = s->dir_state[0].head; - - s->dir_state[1].cnt = 0; - s->dir_state[1].filestore_cnt = 0; - s->dir_state[1].flags = 0; - /* reset 'cur' back to the list head */ - s->dir_state[1].cur = s->dir_state[1].head; + for (int i = 0; i < DETECT_ENGINE_STATE_DIRECTIONS; i++) { + s->dir_state[i].cnt = 0; + s->dir_state[i].filestore_cnt = 0; + s->dir_state[i].flags = 0; + /* reset 'cur' back to the list head */ + s->dir_state[i].cur = s->dir_state[i].head; + } } } diff --git a/src/detect-engine-state.h b/src/detect-engine-state.h index 326b3bad4ecd..34f78e89754c 100644 --- a/src/detect-engine-state.h +++ b/src/detect-engine-state.h @@ -89,8 +89,17 @@ typedef struct DetectEngineStateDirection_ { /* coccinelle: DetectEngineStateDirection:flags:DETECT_ENGINE_STATE_FLAG_ */ } DetectEngineStateDirection; +#define DETECT_ENGINE_STATE_DIRECTIONS 3 + +enum { + DETECT_ENGINE_STATE_DIRECTION_TOSERVER = 0, + DETECT_ENGINE_STATE_DIRECTION_TOCLIENT = 1, + DETECT_ENGINE_STATE_DIRECTION_BOTHDIR = 2, +}; + typedef struct DetectEngineState_ { - DetectEngineStateDirection dir_state[2]; + // to server, to client, and bidirectional + DetectEngineStateDirection dir_state[DETECT_ENGINE_STATE_DIRECTIONS]; } DetectEngineState; /** diff --git a/src/detect-engine.c b/src/detect-engine.c index 58b5c9967c8b..6721512dade8 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -761,6 +761,15 @@ int DetectEngineAppInspectionEngine2Signature(DetectEngineCtx *de_ctx, Signature for (const DetectEngineAppInspectionEngine *t = de_ctx->app_inspect_engines; t != NULL; t = t->next) { if (t->sm_list == s->init_data->buffers[x].id) { + if (s->flags & SIG_FLAG_BOTHDIR) { + // ambiguous keywords have app engines in both directions + // so we skip the wrong direction for this buffer + if (s->init_data->buffers[x].only_tc && t->dir == 0) { + continue; + } else if (s->init_data->buffers[x].only_ts && t->dir == 1) { + continue; + } + } AppendAppInspectEngine( de_ctx, t, s, smd, mpm_list, files_id, &last_id, &head_is_mpm); } @@ -1351,6 +1360,30 @@ bool DetectBufferIsPresent(const Signature *s, const uint32_t buf_id) return false; } +static bool DetectEngineBufferAmbiguousDir( + DetectEngineCtx *de_ctx, const int list, AppProto alproto) +{ + bool has_ts = false; + bool has_tc = false; + const DetectEngineAppInspectionEngine *app = de_ctx->app_inspect_engines; + for (; app != NULL; app = app->next) { + if (app->sm_list == list && (AppProtoEquals(alproto, app->alproto) || alproto == 0)) { + if (app->dir == 0) { + if (has_tc) { + return true; + } + has_ts = true; + } else if (app->dir == 1) { + if (has_ts) { + return true; + } + has_tc = true; + } + } + } + return false; +} + int DetectBufferSetActiveList(DetectEngineCtx *de_ctx, Signature *s, const int list) { BUG_ON(s->init_data == NULL); @@ -1389,7 +1422,12 @@ int DetectBufferSetActiveList(DetectEngineCtx *de_ctx, Signature *s, const int l } else if (DetectEngineBufferTypeSupportsMultiInstanceGetById(de_ctx, list)) { // fall through + } else if (b->only_tc && (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOSERVER)) { + // fall through + } else if (b->only_ts && (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT)) { + // fall through } else { + // we create a new buffer for the same id but forced different direction SCLogWarning("duplicate instance for %s in '%s'", DetectEngineBufferTypeGetNameById(de_ctx, list), s->sig_str); s->init_data->curbuf = b; @@ -1413,6 +1451,13 @@ int DetectBufferSetActiveList(DetectEngineCtx *de_ctx, Signature *s, const int l s->init_data->curbuf->tail = NULL; s->init_data->curbuf->multi_capable = DetectEngineBufferTypeSupportsMultiInstanceGetById(de_ctx, list); + if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOCLIENT) { + s->init_data->curbuf->only_tc = DetectEngineBufferAmbiguousDir(de_ctx, list, s->alproto); + } + if (s->init_data->init_flags & SIG_FLAG_INIT_BIDIR_TOSERVER) { + s->init_data->curbuf->only_ts = DetectEngineBufferAmbiguousDir(de_ctx, list, s->alproto); + } + SCLogDebug("new: idx %u list %d set up curbuf %p", s->init_data->buffer_index - 1, list, s->init_data->curbuf); diff --git a/src/detect-fast-pattern.c b/src/detect-fast-pattern.c index 1af1daeea3df..c73ca682e379 100644 --- a/src/detect-fast-pattern.c +++ b/src/detect-fast-pattern.c @@ -237,6 +237,19 @@ static int DetectFastPatternSetup(DetectEngineCtx *de_ctx, Signature *s, const c pm = pm2; } + if (s->flags & SIG_FLAG_BOTHDIR && s->init_data->curbuf != NULL) { + if (DetectBufferToClient(de_ctx, s->init_data->curbuf->id, s->alproto)) { + 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; + } + } + } + cd = (DetectContentData *)pm->ctx; if ((cd->flags & DETECT_CONTENT_NEGATED) && ((cd->flags & DETECT_CONTENT_DISTANCE) || diff --git a/src/detect-file-data.c b/src/detect-file-data.c index a721c08c7cf9..fc7e0601fd78 100644 --- a/src/detect-file-data.c +++ b/src/detect-file-data.c @@ -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; } diff --git a/src/detect-file-hash-common.c b/src/detect-file-hash-common.c index f81ce4be29ea..e7bd692b9c7b 100644 --- a/src/detect-file-hash-common.c +++ b/src/detect-file-hash-common.c @@ -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) { diff --git a/src/detect-filemagic.c b/src/detect-filemagic.c index f23434d8666e..2bd6b7ea12a3 100644 --- a/src/detect-filemagic.c +++ b/src/detect-filemagic.c @@ -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; diff --git a/src/detect-filename.c b/src/detect-filename.c index f75fdbd680fe..66eb5777561e 100644 --- a/src/detect-filename.c +++ b/src/detect-filename.c @@ -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); @@ -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; @@ -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; } diff --git a/src/detect-filesize.c b/src/detect-filesize.c index f021dd6e5eda..ec9279c838f6 100644 --- a/src/detect-filesize.c +++ b/src/detect-filesize.c @@ -133,6 +133,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); } diff --git a/src/detect-flow.c b/src/detect-flow.c index c268dedb155f..8e07ea16ba14 100644 --- a/src/detect-flow.c +++ b/src/detect-flow.c @@ -79,6 +79,48 @@ void DetectFlowRegister (void) DetectSetupParseRegexes(PARSE_REGEX, &parse_regex); } +static int DetectBidirToClientSetup(DetectEngineCtx *de_ctx, Signature *s, const char *flowstr) +{ + if (!(s->flags & SIG_FLAG_BOTHDIR)) { + SCLogError("Cannot have bidir keyword in a non bidirectional signature"); + return -1; + } + s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_TOCLIENT; + s->init_data->init_flags &= ~SIG_FLAG_INIT_BIDIR_TOSERVER; + return 0; +} + +static int DetectBidirToServerSetup(DetectEngineCtx *de_ctx, Signature *s, const char *flowstr) +{ + if (!(s->flags & SIG_FLAG_BOTHDIR)) { + SCLogError("Cannot have bidir keyword in a non bidirectional signature"); + return -1; + } + s->init_data->init_flags |= SIG_FLAG_INIT_BIDIR_TOSERVER; + s->init_data->init_flags &= ~SIG_FLAG_INIT_BIDIR_TOCLIENT; + return 0; +} + +/** + * \brief Registration function for flow: keyword + */ +void DetectBidirRegister(void) +{ + sigmatch_table[DETECT_BIDIR_TOCLIENT].name = "bidir.toclient"; + sigmatch_table[DETECT_BIDIR_TOCLIENT].desc = + "match next keywords only toclient side for bidirectional rules"; + sigmatch_table[DETECT_BIDIR_TOCLIENT].url = "/rules/intro.html#bidirectional-rules"; + sigmatch_table[DETECT_BIDIR_TOCLIENT].Setup = DetectBidirToClientSetup; + sigmatch_table[DETECT_BIDIR_TOCLIENT].flags |= SIGMATCH_NOOPT; + + sigmatch_table[DETECT_BIDIR_TOSERVER].name = "bidir.toserver"; + sigmatch_table[DETECT_BIDIR_TOSERVER].desc = + "match next keywords only toclient side for bidirectional rules"; + sigmatch_table[DETECT_BIDIR_TOSERVER].url = "/rules/intro.html#bidirectional-rules"; + sigmatch_table[DETECT_BIDIR_TOSERVER].Setup = DetectBidirToServerSetup; + sigmatch_table[DETECT_BIDIR_TOSERVER].flags |= SIGMATCH_NOOPT; +} + /** * \param pflags packet flags (p->flags) * \param pflowflags packet flow flags (p->flowflags) @@ -391,8 +433,18 @@ int DetectFlowSetup (DetectEngineCtx *de_ctx, Signature *s, const char *flowstr) bool appendsm = true; /* set the signature direction flags */ if (fd->flags & DETECT_FLOW_FLAG_TOSERVER) { + if (s->flags & SIG_FLAG_BOTHDIR) { + SCLogError( + "rule %u means to use both directions, cannot specify a flow direction", s->id); + goto error; + } s->flags |= SIG_FLAG_TOSERVER; } else if (fd->flags & DETECT_FLOW_FLAG_TOCLIENT) { + if (s->flags & SIG_FLAG_BOTHDIR) { + SCLogError( + "rule %u means to use both directions, cannot specify a flow direction", s->id); + goto error; + } s->flags |= SIG_FLAG_TOCLIENT; } else { s->flags |= SIG_FLAG_TOSERVER; diff --git a/src/detect-flow.h b/src/detect-flow.h index 990d07b83a04..43445290efa0 100644 --- a/src/detect-flow.h +++ b/src/detect-flow.h @@ -44,4 +44,6 @@ int DetectFlowSetupImplicit(Signature *s, uint32_t flags); /* prototypes */ void DetectFlowRegister (void); +void DetectBidirRegister(void); + #endif /* SURICATA_DETECT_FLOW_H */ diff --git a/src/detect-http-client-body.c b/src/detect-http-client-body.c index 5e5604ea594d..047b7df2cba5 100644 --- a/src/detect-http-client-body.c +++ b/src/detect-http-client-body.c @@ -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; } diff --git a/src/detect-parse.c b/src/detect-parse.c index 984501c1dd8a..677a4c200d3c 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -1378,6 +1378,8 @@ static int SigParseBasics(DetectEngineCtx *de_ctx, Signature *s, const char *sig if (strcmp(parser->direction, "<>") == 0) { s->init_data->init_flags |= SIG_FLAG_INIT_BIDIREC; + } else if (strcmp(parser->direction, "=>") == 0) { + s->flags |= SIG_FLAG_BOTHDIR; } else if (strcmp(parser->direction, "->") != 0) { SCLogError("\"%s\" is not a valid direction modifier, " "\"->\" and \"<>\" are supported.", @@ -1922,6 +1924,9 @@ static int SigValidate(DetectEngineCtx *de_ctx, Signature *s) } bufdir[nlists + 1]; memset(&bufdir, 0, (nlists + 1) * sizeof(struct BufferVsDir)); + int ts_excl = 0; + int tc_excl = 0; + for (uint32_t x = 0; x < s->init_data->buffer_index; x++) { SignatureInitDataBuffer *b = &s->init_data->buffers[x]; const DetectBufferType *bt = DetectEngineBufferTypeGetById(de_ctx, b->id); @@ -1959,8 +1964,16 @@ static int SigValidate(DetectEngineCtx *de_ctx, Signature *s) DetectEngineBufferTypeGetNameById(de_ctx, app->sm_list), app->dir, app->alproto); SCLogDebug("b->id %d nlists %d", b->id, nlists); - bufdir[b->id].ts += (app->dir == 0); - bufdir[b->id].tc += (app->dir == 1); + if (b->only_tc) { + if (app->dir == 1) + tc_excl++; + } else if (b->only_ts) { + if (app->dir == 0) + ts_excl++; + } else { + bufdir[b->id].ts += (app->dir == 0); + bufdir[b->id].tc += (app->dir == 1); + } } } @@ -1973,8 +1986,6 @@ static int SigValidate(DetectEngineCtx *de_ctx, Signature *s) } } - int ts_excl = 0; - int tc_excl = 0; int dir_amb = 0; for (int x = 0; x < nlists; x++) { if (bufdir[x].ts == 0 && bufdir[x].tc == 0) @@ -1986,8 +1997,21 @@ static int SigValidate(DetectEngineCtx *de_ctx, Signature *s) SCLogDebug("%s/%d: %d/%d", DetectEngineBufferTypeGetNameById(de_ctx, x), x, bufdir[x].ts, bufdir[x].tc); } - if (ts_excl && tc_excl) { - SCLogError("rule %u mixes keywords with conflicting directions", s->id); + if (s->flags & SIG_FLAG_BOTHDIR) { + if (!ts_excl || !tc_excl) { + SCLogError("rule %u should use both directions, but does not", s->id); + SCReturnInt(0); + } + if (dir_amb) { + SCLogError("rule %u means to use both directions, cannot have keywords ambiguous about " + "directions", + s->id); + SCReturnInt(0); + } + } else if (ts_excl && tc_excl) { + SCLogError("rule %u mixes keywords with conflicting directions, a bidirection rule with => " + "should be used", + s->id); SCReturnInt(0); } else if (ts_excl) { SCLogDebug("%u: implied rule direction is toserver", s->id); diff --git a/src/detect-prefilter.c b/src/detect-prefilter.c index f38b56bf8b9c..825fe87cc79d 100644 --- a/src/detect-prefilter.c +++ b/src/detect-prefilter.c @@ -29,6 +29,7 @@ #include "detect.h" #include "detect-parse.h" #include "detect-content.h" +#include "detect-engine-mpm.h" #include "detect-prefilter.h" #include "util-debug.h" @@ -75,6 +76,19 @@ static int DetectPrefilterSetup (DetectEngineCtx *de_ctx, Signature *s, const ch /* if the sig match is content, prefilter should act like * 'fast_pattern' w/o options. */ if (sm->type == DETECT_CONTENT) { + if (s->flags & SIG_FLAG_BOTHDIR && s->init_data->curbuf != NULL) { + 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; + } + } + } + DetectContentData *cd = (DetectContentData *)sm->ctx; if ((cd->flags & DETECT_CONTENT_NEGATED) && ((cd->flags & DETECT_CONTENT_DISTANCE) || diff --git a/src/detect.c b/src/detect.c index 03fa8437068d..00edd71e14c9 100644 --- a/src/detect.c +++ b/src/detect.c @@ -1138,9 +1138,11 @@ 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))) { + void *tx_ptr = DetectGetInnerTx(tx->tx_ptr, f->alproto, engine->alproto, flow_flags); if (tx_ptr == NULL) { if (engine->alproto != ALPROTO_UNKNOWN) { @@ -1176,6 +1178,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)) { @@ -1185,7 +1191,7 @@ static bool DetectRunTxInspectRule(ThreadVars *tv, 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_ptr, tx->tx_id); + de_ctx, det_ctx, engine, s, f, engine_flags, alstate, tx_ptr, tx->tx_id); TRACE_SID_TXS(s->id, tx, "engine %p match %d", engine, match); if (engine->stream) { can->stream_stored = true; @@ -1219,7 +1225,19 @@ static bool DetectRunTxInspectRule(ThreadVars *tv, inspect_flags |= BIT_U32(engine->id); } break; + } else if (!(inspect_flags & BIT_U32(engine->id)) && s->flags & SIG_FLAG_BOTHDIR && + direction != engine->dir) { + // for bidirectional rules, the engines on the opposite direction + // are ordered by progress on the different side + // so we have a two mixed-up lists, and we skip the elements + if (direction == 0 && engine->next == NULL) { + // do not match yet on request only + break; + } + engine = engine->next; + continue; } + engine = engine->next; } while (engine != NULL); TRACE_SID_TXS(s->id, tx, "inspect_flags %x, total_matches %u, engine %p", @@ -1278,7 +1296,7 @@ static bool DetectRunTxInspectRule(ThreadVars *tv, #define NO_TX \ { \ - NULL, 0, NULL, NULL, 0, 0, 0, 0, 0, \ + NULL, 0, NULL, NULL, NULL, 0, 0, 0, 0, 0, \ } /** \internal @@ -1318,16 +1336,18 @@ static DetectTransaction GetDetectTx(const uint8_t ipproto, const AppProto alpro DEBUG_VALIDATE_BUG_ON(prefilter_flags & APP_LAYER_TX_RESERVED_FLAGS); DetectTransaction tx = { - .tx_ptr = tx_ptr, - .tx_id = tx_id, - .tx_data_ptr = (struct AppLayerTxData *)txd, - .de_state = tx_dir_state, - .detect_flags = detect_flags, - .prefilter_flags = prefilter_flags, - .prefilter_flags_orig = prefilter_flags, - .tx_progress = tx_progress, - .tx_end_state = tx_end_state, - }; + .tx_ptr = tx_ptr, + .tx_id = tx_id, + .tx_data_ptr = (struct AppLayerTxData *)txd, + .de_state = tx_dir_state, + .de_state_bidir = + tx_de_state ? &tx_de_state->dir_state[DETECT_ENGINE_STATE_DIRECTION_BOTHDIR] : NULL, + .detect_flags = detect_flags, + .prefilter_flags = prefilter_flags, + .prefilter_flags_orig = prefilter_flags, + .tx_progress = tx_progress, + .tx_end_state = tx_end_state, + }; return tx; } @@ -1344,6 +1364,52 @@ static inline void StoreDetectFlags(DetectTransaction *tx, const uint8_t flow_fl } } +static bool RuleMatchCandidateMergeStoredState(DetectEngineCtx *de_ctx, + DetectEngineThreadCtx *det_ctx, DetectTransaction tx, DetectEngineStateDirection *de_state, + uint32_t *array_idx) +{ + if (de_state == NULL) { + return false; + } + const uint32_t old = *array_idx; + + /* if de_state->flags has 'new file' set and sig below has + * 'file inspected' flag, reset the file part of the state */ + const bool have_new_file = (de_state->flags & DETECT_ENGINE_STATE_FLAG_FILE_NEW); + if (have_new_file) { + SCLogDebug("%p/%" PRIu64 " destate: need to consider new file", tx.tx_ptr, tx.tx_id); + de_state->flags &= ~DETECT_ENGINE_STATE_FLAG_FILE_NEW; + } + + SigIntId state_cnt = 0; + DeStateStore *tx_store = de_state->head; + for (; tx_store != NULL; tx_store = tx_store->next) { + SCLogDebug("tx_store %p", tx_store); + + SigIntId store_cnt = 0; + for (store_cnt = 0; store_cnt < DE_STATE_CHUNK_SIZE && state_cnt < de_state->cnt; + store_cnt++, state_cnt++) { + DeStateStoreItem *item = &tx_store->store[store_cnt]; + SCLogDebug("rule id %u, inspect_flags %u", item->sid, item->flags); + if (have_new_file && (item->flags & DE_STATE_FLAG_FILE_INSPECT)) { + /* remove part of the state. File inspect engine will now + * be able to run again */ + item->flags &= ~(DE_STATE_FLAG_SIG_CANT_MATCH | DE_STATE_FLAG_FULL_INSPECT | + DE_STATE_FLAG_FILE_INSPECT); + SCLogDebug("rule id %u, post file reset inspect_flags %u", item->sid, item->flags); + } + det_ctx->tx_candidates[*array_idx].s = de_ctx->sig_array[item->sid]; + det_ctx->tx_candidates[*array_idx].id = item->sid; + det_ctx->tx_candidates[*array_idx].flags = &item->flags; + det_ctx->tx_candidates[*array_idx].stream_reset = 0; + (*array_idx)++; + } + } + SCLogDebug("%p/%" PRIu64 " rules added from 'continue' list: %u", tx.tx_ptr, tx.tx_id, + *array_idx - old); + return (old && old != *array_idx); // sort if continue list adds sids +} + // Merge 'state' rules from the regular prefilter // updates array_idx on the way static inline void RuleMatchCandidateMergeStateRules( @@ -1459,6 +1525,7 @@ static void DetectRunTx(ThreadVars *tv, uint32_t array_idx = 0; uint32_t total_rules = det_ctx->match_array_cnt; total_rules += (tx.de_state ? tx.de_state->cnt : 0); + total_rules += (tx.de_state_bidir ? tx.de_state_bidir->cnt : 0); /* run prefilter engines and merge results into a candidates array */ if (sgh->tx_engines) { @@ -1497,47 +1564,9 @@ static void DetectRunTx(ThreadVars *tv, RuleMatchCandidateMergeStateRules(det_ctx, &array_idx); /* merge stored state into results */ - if (tx.de_state != NULL) { - const uint32_t old = array_idx; - - /* if tx.de_state->flags has 'new file' set and sig below has - * 'file inspected' flag, reset the file part of the state */ - const bool have_new_file = (tx.de_state->flags & DETECT_ENGINE_STATE_FLAG_FILE_NEW); - if (have_new_file) { - SCLogDebug("%p/%"PRIu64" destate: need to consider new file", - tx.tx_ptr, tx.tx_id); - tx.de_state->flags &= ~DETECT_ENGINE_STATE_FLAG_FILE_NEW; - } - - SigIntId state_cnt = 0; - DeStateStore *tx_store = tx.de_state->head; - for (; tx_store != NULL; tx_store = tx_store->next) { - SCLogDebug("tx_store %p", tx_store); - - SigIntId store_cnt = 0; - for (store_cnt = 0; - store_cnt < DE_STATE_CHUNK_SIZE && state_cnt < tx.de_state->cnt; - store_cnt++, state_cnt++) - { - DeStateStoreItem *item = &tx_store->store[store_cnt]; - SCLogDebug("rule id %u, inspect_flags %u", item->sid, item->flags); - if (have_new_file && (item->flags & DE_STATE_FLAG_FILE_INSPECT)) { - /* remove part of the state. File inspect engine will now - * be able to run again */ - item->flags &= ~(DE_STATE_FLAG_SIG_CANT_MATCH|DE_STATE_FLAG_FULL_INSPECT|DE_STATE_FLAG_FILE_INSPECT); - SCLogDebug("rule id %u, post file reset inspect_flags %u", item->sid, item->flags); - } - det_ctx->tx_candidates[array_idx].s = de_ctx->sig_array[item->sid]; - det_ctx->tx_candidates[array_idx].id = item->sid; - det_ctx->tx_candidates[array_idx].flags = &item->flags; - det_ctx->tx_candidates[array_idx].stream_reset = 0; - array_idx++; - } - } - do_sort |= (old && old != array_idx); // sort if continue list adds sids - SCLogDebug("%p/%" PRIu64 " rules added from 'continue' list: %u", tx.tx_ptr, tx.tx_id, - array_idx - old); - } + do_sort |= RuleMatchCandidateMergeStoredState(de_ctx, det_ctx, tx, tx.de_state, &array_idx); + do_sort |= RuleMatchCandidateMergeStoredState( + de_ctx, det_ctx, tx, tx.de_state_bidir, &array_idx); if (do_sort) { qsort(det_ctx->tx_candidates, array_idx, sizeof(RuleMatchCandidateTx), DetectRunTxSortHelper); diff --git a/src/detect.h b/src/detect.h index cf8f4335b197..be8af44b6618 100644 --- a/src/detect.h +++ b/src/detect.h @@ -244,6 +244,7 @@ typedef struct DetectPort_ { #define SIG_FLAG_DSIZE BIT_U32(5) /**< signature has a dsize setting */ #define SIG_FLAG_APPLAYER BIT_U32(6) /**< signature applies to app layer instead of packets */ +#define SIG_FLAG_BOTHDIR BIT_U32(7) /**< signature needs both directions to match */ // vacancy @@ -295,6 +296,13 @@ typedef struct DetectPort_ { BIT_U32(8) /**< priority is explicitly set by the priority keyword */ #define SIG_FLAG_INIT_FILEDATA BIT_U32(9) /**< signature has filedata keyword */ #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 */ @@ -531,6 +539,8 @@ typedef struct SignatureInitDataBuffer_ { set up. */ bool multi_capable; /**< true if we can have multiple instances of this buffer, so e.g. for http.uri. */ + bool only_tc; /**< true if we can only used toclient. */ + bool only_ts; /**< true if we can only used toserver. */ /* sig match list */ SigMatch *head; SigMatch *tail;