diff --git a/doc/userguide/rules/intro.rst b/doc/userguide/rules/intro.rst index 41f7fe0b8327..4dcf97f2ad4a 100644 --- a/doc/userguide/rules/intro.rst +++ b/doc/userguide/rules/intro.rst @@ -227,10 +227,15 @@ 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. +It is also possible to have a double arrow (``=>``) which means that the +directionality for adresses and ports is used, +but such a rule can match a bidirectional transaction, using keywords +matching in each direction. +However, it is also possible to have a rule match both directions (``<>``):: source -> destination + source => destination source <> destination (both directions) The following example illustrates direction. In this example there is a client @@ -251,6 +256,32 @@ as the direction specifies that we do not want to evaluate the response packet. There is no 'reverse' style direction, i.e. there is no ``<-``. +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-abmibuous 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. +- They cann have ``fast_pattern`` or ``prefilter`` on a keyword which is on +the direction to client only if they do not have any have streaming buffers +for detection on the other side. +- They will refuse to load if a single directional rule is enough. + 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 a325f34c22a6..53d5067eb9c1 100644 --- a/src/app-layer-htp.c +++ b/src/app-layer-htp.c @@ -1377,6 +1377,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 b9e029788360..fae04cd88e28 100644 --- a/src/app-layer-smtp.c +++ b/src/app-layer-smtp.c @@ -475,6 +475,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 6ceeaa63f233..a4db5bcbdaf6 100644 --- a/src/detect-engine-mpm.c +++ b/src/detect-engine-mpm.c @@ -1049,6 +1049,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) @@ -1151,6 +1169,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 197416fdc941..587c1e324656 100644 --- a/src/detect-engine-mpm.h +++ b/src/detect-engine-mpm.h @@ -132,4 +132,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 58daeca6e736..fe4de99cefd4 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 5608ae218f51..e644db7e3c45 100644 --- a/src/detect-engine-register.c +++ b/src/detect-engine-register.c @@ -579,6 +579,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 cd2edf5979b8..e1dc42cc95a5 100644 --- a/src/detect-engine-register.h +++ b/src/detect-engine-register.h @@ -328,6 +328,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 74f87ff938f1..3adee226462d 100644 --- a/src/detect-engine-state.c +++ b/src/detect-engine-state.c @@ -92,9 +92,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; @@ -123,11 +123,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) { @@ -175,7 +179,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; @@ -246,17 +250,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[0].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 75a198e8fabd..f317ba71551d 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -741,6 +741,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); } @@ -1332,6 +1341,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); @@ -1370,7 +1403,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; @@ -1394,6 +1432,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 6748186727c9..7550c973f2f8 100644 --- a/src/detect-fast-pattern.c +++ b/src/detect-fast-pattern.c @@ -238,6 +238,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 533fc8441d93..949bda11aa7a 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 b7a737e6c7b7..5e399bd2ad18 100644 --- a/src/detect-filemagic.c +++ b/src/detect-filemagic.c @@ -212,6 +212,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 10646f019f1a..c7d193730b65 100644 --- a/src/detect-filename.c +++ b/src/detect-filename.c @@ -129,6 +129,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); @@ -176,6 +179,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; @@ -211,6 +217,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 c29957d2870b..9fbfac04f26d 100644 --- a/src/detect-filesize.c +++ b/src/detect-filesize.c @@ -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: diff --git a/src/detect-flow.c b/src/detect-flow.c index 696e5013a03e..46535ae695cb 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/flow-keywords.html#TODO"; + 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/flow-keywords.html#TODO"; + 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 de898f556966..5fe9d583c45d 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -1394,6 +1394,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.", @@ -1949,6 +1951,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); @@ -1986,8 +1991,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); + } } } @@ -2000,8 +2013,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) @@ -2013,8 +2024,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 9d595e66dc4a..37b1c88d6b0a 100644 --- a/src/detect.c +++ b/src/detect.c @@ -1107,9 +1107,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. */ @@ -1141,6 +1142,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)) { @@ -1149,8 +1154,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; @@ -1184,7 +1189,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", @@ -1243,7 +1260,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 @@ -1283,16 +1300,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; } @@ -1309,6 +1328,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( @@ -1425,6 +1490,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) { @@ -1463,47 +1529,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 52b456318969..a8f37e7fbc69 100644 --- a/src/detect.h +++ b/src/detect.h @@ -243,6 +243,7 @@ typedef struct DetectPort_ { #define SIG_FLAG_NOALERT BIT_U32(4) /**< no alert flag is set */ #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 @@ -294,6 +295,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 */ @@ -526,6 +534,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;