Skip to content

detect: factorize code for DetectSetupDirection (transactional rules) #13046

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

detect: factorize code for DetectSetupDirection (transactional rules) #13046

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 1 addition & 8 deletions src/detect-file-data.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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 "
Expand All @@ -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;
Expand Down
7 changes: 2 additions & 5 deletions src/detect-filemagic.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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;

Expand Down
7 changes: 2 additions & 5 deletions src/detect-filename.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");

Expand Down Expand Up @@ -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);
Expand Down
1 change: 1 addition & 0 deletions src/detect-filesize.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
11 changes: 2 additions & 9 deletions src/detect-http-headers-stub.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand All @@ -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
Expand Down
121 changes: 84 additions & 37 deletions src/detect-parse.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
Expand Down Expand Up @@ -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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not a single pointer char *str? We're not passing something back, are we?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We're not passing something back, are we?

Yes, we are moving forward the string pointer, so that the keyword filesize only "sees" >100 when the signature had filesize: to_client, >100;

{
if (strncmp(*str, "to_client", strlen("to_client")) == 0) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will also match to_client_toto I think. Will that be detected correctly?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes

After matching the prefix, we check further characters : optional spaces then comma or end of string

to_client_toto gets to line 887 SCLogError("expecting to_client [, other]");

*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)
{
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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
*/
Expand Down
1 change: 0 additions & 1 deletion src/detect-parse.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 */
2 changes: 2 additions & 0 deletions src/detect.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
Loading