From 6992640468d91c805019301afa278b6c6be53559 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Fri, 18 Apr 2025 16:13:27 +0200 Subject: [PATCH] detect: factorize code for DetectSetupDirection Ticket: 7665 Instead of each keyword calling DetectSetupDirection, use a new flag SIGMATCH_SUPPORT_DIR so that DetectSetupDirection gets called, before parsing the rest of the keyword. Allows to support filesize keyword in transactional signatures --- src/detect-file-data.c | 9 +-- src/detect-filemagic.c | 7 +- src/detect-filename.c | 7 +- src/detect-filesize.c | 1 + src/detect-http-headers-stub.h | 11 +-- src/detect-parse.c | 121 +++++++++++++++++++++++---------- src/detect-parse.h | 1 - src/detect.h | 2 + 8 files changed, 94 insertions(+), 65 deletions(-) diff --git a/src/detect-file-data.c b/src/detect-file-data.c index f1e17c853548..76ce6f775b4a 100644 --- a/src/detect-file-data.c +++ b/src/detect-file-data.c @@ -78,7 +78,7 @@ void DetectFiledataRegister(void) #ifdef UNITTESTS sigmatch_table[DETECT_FILE_DATA].RegisterTests = DetectFiledataRegisterTests; #endif - sigmatch_table[DETECT_FILE_DATA].flags = SIGMATCH_OPTIONAL_OPT; + sigmatch_table[DETECT_FILE_DATA].flags = SIGMATCH_OPTIONAL_OPT | SIGMATCH_SUPPORT_DIR; filehandler_table[DETECT_FILE_DATA].name = "file_data"; filehandler_table[DETECT_FILE_DATA].priority = 2; @@ -140,11 +140,6 @@ static int DetectFiledataSetup (DetectEngineCtx *de_ctx, Signature *s, const cha return -1; } - if (DetectSetupDirection(s, str) < 0) { - SCLogError("file.data failed to setup direction"); - return -1; - } - if (s->alproto == ALPROTO_SMTP && (s->init_data->init_flags & SIG_FLAG_INIT_FLOW) && !(s->flags & SIG_FLAG_TOSERVER) && (s->flags & SIG_FLAG_TOCLIENT)) { SCLogError("The 'file-data' keyword cannot be used with SMTP flow:to_client or " @@ -166,8 +161,6 @@ static int DetectFiledataSetup (DetectEngineCtx *de_ctx, Signature *s, const cha } s->init_data->init_flags |= SIG_FLAG_INIT_TXDIR_STREAMING_TOSERVER; } - s->init_data->init_flags &= ~SIG_FLAG_INIT_FORCE_TOSERVER; - s->init_data->init_flags &= ~SIG_FLAG_INIT_FORCE_TOCLIENT; SetupDetectEngineConfig(de_ctx); return 0; diff --git a/src/detect-filemagic.c b/src/detect-filemagic.c index 77966ebd99e9..e039a9f77e44 100644 --- a/src/detect-filemagic.c +++ b/src/detect-filemagic.c @@ -113,7 +113,8 @@ void DetectFilemagicRegister(void) sigmatch_table[DETECT_FILE_MAGIC].desc = "sticky buffer to match on the file magic"; sigmatch_table[DETECT_FILE_MAGIC].url = "/rules/file-keywords.html#filemagic"; sigmatch_table[DETECT_FILE_MAGIC].Setup = DetectFilemagicSetupSticky; - sigmatch_table[DETECT_FILE_MAGIC].flags = SIGMATCH_OPTIONAL_OPT | SIGMATCH_INFO_STICKY_BUFFER; + sigmatch_table[DETECT_FILE_MAGIC].flags = + SIGMATCH_OPTIONAL_OPT | SIGMATCH_INFO_STICKY_BUFFER | SIGMATCH_SUPPORT_DIR; filehandler_table[DETECT_FILE_MAGIC].name = "file.magic", filehandler_table[DETECT_FILE_MAGIC].priority = 2; @@ -249,10 +250,6 @@ static int DetectFilemagicSetup (DetectEngineCtx *de_ctx, Signature *s, const ch */ static int DetectFilemagicSetupSticky(DetectEngineCtx *de_ctx, Signature *s, const char *str) { - if (DetectSetupDirection(s, str) < 0) { - SCLogError("file.magic failed to setup direction"); - return -1; - } if (DetectBufferSetActiveList(de_ctx, s, g_file_magic_buffer_id) < 0) return -1; diff --git a/src/detect-filename.c b/src/detect-filename.c index 65e0151e8bee..e1a02858a4f0 100644 --- a/src/detect-filename.c +++ b/src/detect-filename.c @@ -99,7 +99,8 @@ void DetectFilenameRegister(void) sigmatch_table[DETECT_FILE_NAME].desc = "sticky buffer to match on the file name"; sigmatch_table[DETECT_FILE_NAME].url = "/rules/file-keywords.html#filename"; sigmatch_table[DETECT_FILE_NAME].Setup = DetectFilenameSetupSticky; - sigmatch_table[DETECT_FILE_NAME].flags = SIGMATCH_OPTIONAL_OPT | SIGMATCH_INFO_STICKY_BUFFER; + sigmatch_table[DETECT_FILE_NAME].flags = + SIGMATCH_OPTIONAL_OPT | SIGMATCH_INFO_STICKY_BUFFER | SIGMATCH_SUPPORT_DIR; DetectBufferTypeSetDescriptionByName("file.name", "file name"); @@ -207,10 +208,6 @@ static int DetectFilenameSetup (DetectEngineCtx *de_ctx, Signature *s, const cha */ static int DetectFilenameSetupSticky(DetectEngineCtx *de_ctx, Signature *s, const char *str) { - if (DetectSetupDirection(s, str) < 0) { - SCLogError("file.name failed to setup direction"); - return -1; - } if (DetectBufferSetActiveList(de_ctx, s, g_file_name_buffer_id) < 0) return -1; s->file_flags |= (FILE_SIG_NEED_FILE | FILE_SIG_NEED_FILENAME); diff --git a/src/detect-filesize.c b/src/detect-filesize.c index e0b430bbfd87..8128b2f63478 100644 --- a/src/detect-filesize.c +++ b/src/detect-filesize.c @@ -66,6 +66,7 @@ void DetectFilesizeRegister(void) sigmatch_table[DETECT_FILESIZE].FileMatch = DetectFilesizeMatch; sigmatch_table[DETECT_FILESIZE].Setup = DetectFilesizeSetup; sigmatch_table[DETECT_FILESIZE].Free = DetectFilesizeFree; + sigmatch_table[DETECT_FILESIZE].flags = SIGMATCH_SUPPORT_DIR; #ifdef UNITTESTS sigmatch_table[DETECT_FILESIZE].RegisterTests = DetectFilesizeRegisterTests; #endif diff --git a/src/detect-http-headers-stub.h b/src/detect-http-headers-stub.h index 529dca5ad5ae..208d602be900 100644 --- a/src/detect-http-headers-stub.h +++ b/src/detect-http-headers-stub.h @@ -161,17 +161,9 @@ static InspectionBuffer *GetResponseData2(DetectEngineThreadCtx *det_ctx, */ static int DetectHttpHeadersSetupSticky(DetectEngineCtx *de_ctx, Signature *s, const char *str) { - if (DetectSetupDirection(s, str) < 0) { - SCLogError(KEYWORD_NAME " failed to setup direction"); - return -1; - } - if (DetectBufferSetActiveList(de_ctx, s, g_buffer_id) < 0) return -1; - s->init_data->init_flags &= ~SIG_FLAG_INIT_FORCE_TOSERVER; - s->init_data->init_flags &= ~SIG_FLAG_INIT_FORCE_TOCLIENT; - if (DetectSignatureSetAppProto(s, ALPROTO_HTTP) < 0) return -1; @@ -188,7 +180,8 @@ static void DetectHttpHeadersRegisterStub(void) sigmatch_table[KEYWORD_ID].url = "/rules/" KEYWORD_DOC; sigmatch_table[KEYWORD_ID].Setup = DetectHttpHeadersSetupSticky; #if defined(KEYWORD_TOSERVER) && defined(KEYWORD_TOSERVER) - sigmatch_table[KEYWORD_ID].flags |= SIGMATCH_OPTIONAL_OPT | SIGMATCH_INFO_STICKY_BUFFER; + sigmatch_table[KEYWORD_ID].flags |= + SIGMATCH_OPTIONAL_OPT | SIGMATCH_INFO_STICKY_BUFFER | SIGMATCH_SUPPORT_DIR; #else sigmatch_table[KEYWORD_ID].flags |= SIGMATCH_NOOPT | SIGMATCH_INFO_STICKY_BUFFER; #endif diff --git a/src/detect-parse.c b/src/detect-parse.c index ad7696c731c2..66a890e52305 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -530,6 +530,12 @@ SigMatch *SigMatchAppendSMToList( /* buffer set up by sigmatch is tracked in case we add a stickybuffer for the * same list. */ s->init_data->curbuf->sm_init = true; + if (s->init_data->init_flags & SIG_FLAG_INIT_FORCE_TOCLIENT) { + s->init_data->curbuf->only_tc = true; + } + if (s->init_data->init_flags & SIG_FLAG_INIT_FORCE_TOSERVER) { + s->init_data->curbuf->only_ts = true; + } SCLogDebug("s->init_data->buffer_index %u", s->init_data->buffer_index); } } @@ -858,6 +864,76 @@ int SigMatchListSMBelongsTo(const Signature *s, const SigMatch *key_sm) return -1; } +/** + * \brief Parse and setup a direction + * + * \param s signature + * \param str argument to the keyword + * \param only_dir argument wether the keyword only accepts a direction + * + * \retval 0 on success, -1 on failure + */ +static int DetectSetupDirection(Signature *s, char **str, bool only_dir) +{ + if (strncmp(*str, "to_client", strlen("to_client")) == 0) { + *str += strlen("to_client"); + // skip space + while (**str && isblank(**str)) { + (*str)++; + } + // check comma or nothing + if (**str) { + if (**str != ',') { + SCLogError("expecting to_client [, other]"); + return -1; + } else { + (*str)++; + } + while (**str && isblank(**str)) { + (*str)++; + } + } + s->init_data->init_flags |= SIG_FLAG_INIT_FORCE_TOCLIENT; + if ((s->flags & SIG_FLAG_TXBOTHDIR) == 0) { + if (s->flags & SIG_FLAG_TOSERVER) { + SCLogError("contradictory directions"); + return -1; + } + s->flags |= SIG_FLAG_TOCLIENT; + } + } else if (strncmp(*str, "to_server", strlen("to_server")) == 0) { + *str += strlen("to_server"); + // skip space + while (**str && isblank(**str)) { + (*str)++; + } + // check comma or nothing + if (**str) { + if (**str != ',') { + SCLogError("expecting to_server [, other]"); + return -1; + } else { + (*str)++; + } + while (**str && isblank(**str)) { + (*str)++; + } + } + s->init_data->init_flags |= SIG_FLAG_INIT_FORCE_TOSERVER; + if ((s->flags & SIG_FLAG_TXBOTHDIR) == 0) { + if (s->flags & SIG_FLAG_TOCLIENT) { + SCLogError("contradictory directions"); + return -1; + } + s->flags |= SIG_FLAG_TOSERVER; + } + } else if (only_dir) { + SCLogError("unknown option: only accepts to_server or to_client"); + return -1; + } + return 0; +} + static int SigParseOptions(DetectEngineCtx *de_ctx, Signature *s, char *optstr, char *output, size_t output_size, bool requires) { @@ -1045,7 +1121,15 @@ static int SigParseOptions(DetectEngineCtx *de_ctx, Signature *s, char *optstr, } } /* setup may or may not add a new SigMatch to the list */ + if (st->flags & SIGMATCH_SUPPORT_DIR) { + if (DetectSetupDirection(s, &ptr, st->flags & SIGMATCH_OPTIONAL_OPT) < 0) { + SCLogError("%s failed to setup direction", st->name); + goto error; + } + } setup_ret = st->Setup(de_ctx, s, ptr); + s->init_data->init_flags &= ~SIG_FLAG_INIT_FORCE_TOSERVER; + s->init_data->init_flags &= ~SIG_FLAG_INIT_FORCE_TOCLIENT; } else { /* setup may or may not add a new SigMatch to the list */ setup_ret = st->Setup(de_ctx, s, NULL); @@ -3528,43 +3612,6 @@ void DetectSetupParseRegexes(const char *parse_str, DetectParseRegex *detect_par } } -/** - * \brief Parse and setup a direction - * - * \param s siganture - * \param str argument to the keyword - * - * \retval 0 on success, -1 on failure - */ -int DetectSetupDirection(Signature *s, const char *str) -{ - if (str) { - if (strcmp(str, "to_client") == 0) { - s->init_data->init_flags |= SIG_FLAG_INIT_FORCE_TOCLIENT; - if ((s->flags & SIG_FLAG_TXBOTHDIR) == 0) { - if (s->flags & SIG_FLAG_TOSERVER) { - SCLogError("contradictory directions"); - return -1; - } - s->flags |= SIG_FLAG_TOCLIENT; - } - } else if (strcmp(str, "to_server") == 0) { - s->init_data->init_flags |= SIG_FLAG_INIT_FORCE_TOSERVER; - if ((s->flags & SIG_FLAG_TXBOTHDIR) == 0) { - if (s->flags & SIG_FLAG_TOCLIENT) { - SCLogError("contradictory directions"); - return -1; - } - s->flags |= SIG_FLAG_TOSERVER; - } - } else { - SCLogError("unknown option: only accepts to_server or to_client"); - return -1; - } - } - return 0; -} - /* * TESTS */ diff --git a/src/detect-parse.h b/src/detect-parse.h index 18c998f3e276..ddc0b11984d7 100644 --- a/src/detect-parse.h +++ b/src/detect-parse.h @@ -122,7 +122,6 @@ int SC_Pcre2SubstringCopy( int SC_Pcre2SubstringGet(pcre2_match_data *match_data, uint32_t number, PCRE2_UCHAR **bufferptr, PCRE2_SIZE *bufflen); -int DetectSetupDirection(Signature *s, const char *str); void DetectRegisterAppLayerHookLists(void); #endif /* SURICATA_DETECT_PARSE_H */ diff --git a/src/detect.h b/src/detect.h index a663bf07fa85..9267e1a17ee5 100644 --- a/src/detect.h +++ b/src/detect.h @@ -1620,6 +1620,8 @@ typedef struct SigGroupHead_ { #define SIGMATCH_STRICT_PARSING BIT_U16(11) /** keyword supported by firewall rules */ #define SIGMATCH_SUPPORT_FIREWALL BIT_U16(12) +/** keyword supporting setting an optional direction */ +#define SIGMATCH_SUPPORT_DIR BIT_U16(13) enum DetectEngineTenantSelectors {