detect: factorize code for DetectSetupDirection

src/detect-file-data.c

@@ -79,7 +79,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;
@@ -141,11 +141,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 "
@@ -167,8 +162,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;

src/detect-filemagic.c

@@ -114,7 +114,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;
@@ -250,10 +251,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 (SCDetectBufferSetActiveList(de_ctx, s, g_file_magic_buffer_id) < 0)
         return -1;
 

src/detect-filename.c

@@ -100,7 +100,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");
 
@@ -208,10 +209,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 (SCDetectBufferSetActiveList(de_ctx, s, g_file_name_buffer_id) < 0)
         return -1;
     s->file_flags |= (FILE_SIG_NEED_FILE | FILE_SIG_NEED_FILENAME);

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

src/detect-http-headers-stub.h

@@ -162,17 +162,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 (SCDetectBufferSetActiveList(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;
 
@@ -189,7 +181,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

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,87 @@ 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)
+{
+    char *orig = *str;
+    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 (only_dir) {
+                SCLogError("unknown option: only accepts to_server or to_client");
+                return -1;
+            }
+            if (**str != ',') {
+                // leave to_client_something for next parser if not only_dir
+                *str = orig;
+                return 0;
+            } 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 (only_dir) {
+                SCLogError("unknown option: only accepts to_server or to_client");
+                return -1;
+            }
+            if (**str != ',') {
+                // leave to_client_something for next parser if not only_dir
+                *str = orig;
+                return 0;
+            } 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 +1132,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 +3623,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
  */
@@ -5303,6 +5361,63 @@ static int SigSetMultiAppProto(void)
     PASS;
 }
 
+static int DetectSetupDirection01(void)
+{
+    Signature *s = SigAlloc();
+    FAIL_IF_NULL(s);
+    // Basic case : ok
+    char *str = "to_client";
+    FAIL_IF(DetectSetupDirection(s, &str, true) < 0);
+    SigFree(NULL, s);
+    PASS;
+}
+
+static int DetectSetupDirection02(void)
+{
+    Signature *s = SigAlloc();
+    FAIL_IF_NULL(s);
+    char *str = "to_server";
+    FAIL_IF(DetectSetupDirection(s, &str, true) < 0);
+    // ok so far
+    str = "to_client";
+    FAIL_IF(DetectSetupDirection(s, &str, true) >= 0);
+    // fails because we cannot have both to_client and to_server for same signature
+    SigFree(NULL, s);
+    PASS;
+}
+
+static int DetectSetupDirection03(void)
+{
+    Signature *s = SigAlloc();
+    FAIL_IF_NULL(s);
+    char *str = "to_client , something";
+    FAIL_IF(DetectSetupDirection(s, &str, false) < 0);
+    FAIL_IF(strcmp(str, "something") != 0);
+    str = "to_client,something";
+    FAIL_IF(DetectSetupDirection(s, &str, false) < 0);
+    FAIL_IF(strcmp(str, "something") != 0);
+    SigFree(NULL, s);
+    PASS;
+}
+
+static int DetectSetupDirection04(void)
+{
+    Signature *s = SigAlloc();
+    FAIL_IF_NULL(s);
+    // invalid case
+    char *str = "to_client_toto";
+    FAIL_IF(DetectSetupDirection(s, &str, true) >= 0);
+    // test we do not change the string pointer if only_dir is false
+    str = "to_client_toto";
+    FAIL_IF(DetectSetupDirection(s, &str, false) < 0);
+    FAIL_IF(strcmp(str, "to_client_toto") != 0);
+    str = "to_client,something";
+    // fails because we call with only_dir=true
+    FAIL_IF(DetectSetupDirection(s, &str, true) >= 0);
+    SigFree(NULL, s);
+    PASS;
+}
+
 #endif /* UNITTESTS */
 
 #ifdef UNITTESTS
@@ -5381,5 +5496,10 @@ void SigParseRegisterTests(void)
 
     UtRegisterTest("SigSetMultiAppProto", SigSetMultiAppProto);
 
+    UtRegisterTest("DetectSetupDirection01", DetectSetupDirection01);
+    UtRegisterTest("DetectSetupDirection02", DetectSetupDirection02);
+    UtRegisterTest("DetectSetupDirection03", DetectSetupDirection03);
+    UtRegisterTest("DetectSetupDirection04", DetectSetupDirection04);
+
 #endif /* UNITTESTS */
 }

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 */

src/detect.h

@@ -1624,6 +1624,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
 {