From 8ee9d1216a982611d5990043353347b2d36d817d 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/detect-engine-mpm.c | 31 ++++++++++++++++++++ src/detect-engine-mpm.h | 2 ++ src/detect-engine-register.c | 1 + src/detect-engine-register.h | 3 ++ src/detect-engine-state.c | 5 ++++ src/detect-engine.c | 47 +++++++++++++++++++++++++++++++ src/detect-fast-pattern.c | 12 ++++++++ src/detect-file-data.c | 10 +++++++ src/detect-file-hash-common.c | 10 +++++++ src/detect-filemagic.c | 10 +++++++ src/detect-filename.c | 16 +++++++++++ src/detect-filesize.c | 11 ++++++++ src/detect-flow.c | 52 ++++++++++++++++++++++++++++++++++ src/detect-flow.h | 2 ++ src/detect-http-client-body.c | 8 ++++++ src/detect-parse.c | 36 ++++++++++++++++++++---- src/detect-prefilter.c | 14 +++++++++ src/detect.c | 24 ++++++++++++++-- src/detect.h | 10 +++++++ 20 files changed, 345 insertions(+), 12 deletions(-) diff --git a/doc/userguide/rules/intro.rst b/doc/userguide/rules/intro.rst index 56df9ab49437..5e56cee41101 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/detect-engine-mpm.c b/src/detect-engine-mpm.c index 2f5a10f9d9e8..89d86560afed 100644 --- a/src/detect-engine-mpm.c +++ b/src/detect-engine-mpm.c @@ -1071,6 +1071,26 @@ 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 == ALPROTO_UNKNOWN)) { + 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) @@ -1176,6 +1196,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; @@ -1211,7 +1237,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)); diff --git a/src/detect-engine-mpm.h b/src/detect-engine-mpm.h index 10bdb86f5bcb..34d67ae7679f 100644 --- a/src/detect-engine-mpm.h +++ b/src/detect-engine-mpm.h @@ -131,4 +131,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-register.c b/src/detect-engine-register.c index b9faff49bae8..507ac5f72265 100644 --- a/src/detect-engine-register.c +++ b/src/detect-engine-register.c @@ -606,6 +606,7 @@ void SigTableSetup(void) DetectOffsetRegister(); DetectReplaceRegister(); DetectFlowRegister(); + DetectBidirRegister(); DetectFlowAgeRegister(); DetectFlowPktsRegister(); DetectFlowPktsToServerRegister(); diff --git a/src/detect-engine-register.h b/src/detect-engine-register.h index a9e674159e36..3fa40f585893 100644 --- a/src/detect-engine-register.h +++ b/src/detect-engine-register.h @@ -306,6 +306,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..851f4ded6d6b 100644 --- a/src/detect-engine-state.c +++ b/src/detect-engine-state.c @@ -231,6 +231,11 @@ void DetectRunStoreStateTx( SCLogDebug("destate created for %"PRIu64, tx_id); } DeStateSignatureAppend(tx_data->de_state, s, inspect_flags, flow_flags); + if (s->flags & SIG_FLAG_BOTHDIR) { + // add also in the other DetectEngineStateDirection + DeStateSignatureAppend(tx_data->de_state, s, inspect_flags, + flow_flags ^ (STREAM_TOSERVER | STREAM_TOCLIENT)); + } StoreStateTxHandleFiles(sgh, f, tx_data->de_state, flow_flags, tx, tx_id, file_no_match); SCLogDebug("Stored for TX %"PRIu64, tx_id); diff --git a/src/detect-engine.c b/src/detect-engine.c index d1429f19f3c0..c15bdee383e3 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -764,6 +764,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); } @@ -1354,6 +1363,32 @@ 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) +{ + 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); @@ -1392,7 +1427,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; @@ -1416,6 +1456,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..17c1fddb30f7 100644 --- a/src/detect-fast-pattern.c +++ b/src/detect-fast-pattern.c @@ -237,6 +237,18 @@ 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; + } + 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 e15db5b20731..0c1a493577c8 100644 --- a/src/detect-file-data.c +++ b/src/detect-file-data.c @@ -151,6 +151,16 @@ 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) { + // 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); return 0; } diff --git a/src/detect-file-hash-common.c b/src/detect-file-hash-common.c index f81ce4be29ea..1e7b142e798d 100644 --- a/src/detect-file-hash-common.c +++ b/src/detect-file-hash-common.c @@ -332,6 +332,16 @@ 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; + } // 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 0f8d94a7b5b1..b4427149ae9e 100644 --- a/src/detect-filemagic.c +++ b/src/detect-filemagic.c @@ -211,6 +211,16 @@ 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; + } if (DetectContentSetup(de_ctx, s, str) < 0) { return -1; diff --git a/src/detect-filename.c b/src/detect-filename.c index ef144cf44086..53a0b7392a42 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,16 @@ 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; + } if (DetectContentSetup(de_ctx, s, str) < 0) { return -1; @@ -210,6 +223,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..7814513a2f9d 100644 --- a/src/detect-filesize.c +++ b/src/detect-filesize.c @@ -133,6 +133,17 @@ 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); } 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 7747b61b858b..fec295333d7b 100644 --- a/src/detect-http-client-body.c +++ b/src/detect-http-client-body.c @@ -167,6 +167,14 @@ 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 + 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; } diff --git a/src/detect-parse.c b/src/detect-parse.c index 10e9903e80af..6e198817b8c4 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -1404,6 +1404,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.", @@ -1947,6 +1949,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); @@ -1984,8 +1989,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); + } } } @@ -2001,8 +2014,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) @@ -2014,8 +2025,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 b96d96d93a46..ad58eb224480 100644 --- a/src/detect.c +++ b/src/detect.c @@ -1176,9 +1176,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 bidirectional 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) { @@ -1214,6 +1216,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)) { @@ -1223,7 +1229,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; @@ -1257,7 +1263,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", diff --git a/src/detect.h b/src/detect.h index ea81092487b5..3d3eb167c4f5 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 */ @@ -533,6 +541,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;