Detect validate callbacks 5634 v3

doc/userguide/upgrade.rst

@@ -90,6 +90,11 @@ Deprecations
   Suricata 9.0. Note that this is the standalone ``syslog`` output and
   does affect the ``eve`` outputs ability to send to syslog.
 
+Keyword changes
+~~~~~~~~~~~~~~~
+- ``ja3.hash`` and ``ja3s.hash`` no longer accept contents with non hexadecimal
+  characters, as they will never match.
+
 Logging changes
 ~~~~~~~~~~~~~~~
 - RFB security result is now consistently logged as ``security_result`` when it was

src/detect-engine.c

@@ -1300,8 +1300,9 @@ void DetectEngineBufferRunSetupCallback(const DetectEngineCtx *de_ctx, const int
     }
 }
 
-void DetectBufferTypeRegisterValidateCallback(const char *name,
-        bool (*ValidateCallback)(const Signature *, const char **sigerror))
+void DetectBufferTypeRegisterValidateCallback(
+        const char *name, bool (*ValidateCallback)(const Signature *, const DetectContentData *,
+                                  const char **sigerror))
 {
     BUG_ON(g_buffer_type_reg_closed);
     DetectBufferTypeRegister(name);
@@ -1315,7 +1316,20 @@ bool DetectEngineBufferRunValidateCallback(
 {
     const DetectBufferType *map = DetectEngineBufferTypeGetById(de_ctx, id);
     if (map && map->ValidateCallback) {
-        return map->ValidateCallback(s, sigerror);
+        for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
+            if (s->init_data->buffers[x].id != (uint32_t)map->id)
+                continue;
+            const SigMatch *sm = s->init_data->buffers[x].head;
+            for (; sm != NULL; sm = sm->next) {
+                if (sm->type != DETECT_CONTENT)
+                    continue;
+
+                const DetectContentData *cd = (DetectContentData *)sm->ctx;
+                if (!map->ValidateCallback(s, cd, sigerror)) {
+                    return false;
+                }
+            }
+        }
     }
     return true;
 }
@@ -4949,6 +4963,35 @@ void DetectEngineSetEvent(DetectEngineThreadCtx *det_ctx, uint8_t e)
     det_ctx->events++;
 }
 
+bool DetectMd5ValidateCallback(
+        const Signature *s, const DetectContentData *cd, const char **sigerror)
+{
+    if (cd->flags & DETECT_CONTENT_NOCASE) {
+        *sigerror = "md5-like keyword should not be used together with "
+                    "nocase, since the rule is automatically "
+                    "lowercased anyway which makes nocase redundant.";
+        SCLogWarning("rule %u: %s", s->id, *sigerror);
+    }
+
+    if (cd->content_len != SC_MD5_HEX_LEN) {
+        *sigerror = "Invalid length for md5-like keyword (should "
+                    "be 32 characters long). This rule will therefore "
+                    "never match.";
+        SCLogError("rule %u: %s", s->id, *sigerror);
+        return false;
+    }
+
+    for (size_t i = 0; i < cd->content_len; ++i) {
+        if (!isxdigit(cd->content[i])) {
+            *sigerror = "Invalid md5-like string (should be string of hexadecimal characters)."
+                        "This rule will therefore never match.";
+            SCLogWarning("rule %u: %s", s->id, *sigerror);
+            return false;
+        }
+    }
+    return true;
+}
+
 /*************************************Unittest*********************************/
 
 #ifdef UNITTESTS

src/detect-engine.h

@@ -27,6 +27,24 @@
 #include "detect.h"
 #include "suricata.h"
 
+#include "detect-content.h"
+
+typedef struct DetectBufferType_ {
+    char name[32];
+    char description[128];
+    int id;
+    int parent_id;
+    bool mpm;
+    bool packet; /**< compat to packet matches */
+    bool frame;  /**< is about Frame inspection */
+    bool supports_transforms;
+    bool multi_instance; /**< buffer supports multiple buffer instances per tx */
+    void (*SetupCallback)(const struct DetectEngineCtx_ *, struct Signature_ *);
+    bool (*ValidateCallback)(
+            const struct Signature_ *s, const DetectContentData *cd, const char **sigerror);
+    DetectEngineTransforms transforms;
+} DetectBufferType;
+
 void InspectionBufferInit(InspectionBuffer *buffer, uint32_t initial_size);
 void InspectionBufferSetup(DetectEngineThreadCtx *det_ctx, const int list_id,
         InspectionBuffer *buffer, const uint8_t *data, const uint32_t data_len);
@@ -58,8 +76,9 @@ void DetectBufferTypeSetDescriptionByName(const char *name, const char *desc);
 const char *DetectBufferTypeGetDescriptionByName(const char *name);
 void DetectBufferTypeRegisterSetupCallback(const char *name,
         void (*Callback)(const DetectEngineCtx *, Signature *));
-void DetectBufferTypeRegisterValidateCallback(const char *name,
-        bool (*ValidateCallback)(const Signature *, const char **sigerror));
+void DetectBufferTypeRegisterValidateCallback(
+        const char *name, bool (*ValidateCallback)(const Signature *s, const DetectContentData *cd,
+                                  const char **sigerror));
 
 /* detect engine related buffer funcs */
 
@@ -207,6 +226,9 @@ void DetectRunStoreStateTx(const SigGroupHead *sgh, Flow *f, void *tx, uint64_t
 
 void DetectEngineStateResetTxs(Flow *f);
 
+bool DetectMd5ValidateCallback(
+        const Signature *s, const DetectContentData *cd, const char **sigerror);
+
 void DeStateRegisterTests(void);
 
 #endif /* SURICATA_DETECT_ENGINE_H */

src/detect-http-host.c

@@ -62,7 +62,8 @@ static int DetectHttpHHSetup(DetectEngineCtx *, Signature *, const char *);
 #ifdef UNITTESTS
 static void DetectHttpHHRegisterTests(void);
 #endif
-static bool DetectHttpHostValidateCallback(const Signature *s, const char **sigerror);
+static bool DetectHttpHostValidateCallback(
+        const Signature *s, const DetectContentData *cd, const char **sigerror);
 static int DetectHttpHostSetup(DetectEngineCtx *, Signature *, const char *);
 static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx,
         const DetectEngineTransforms *transforms,
@@ -177,42 +178,32 @@ static int DetectHttpHHSetup(DetectEngineCtx *de_ctx, Signature *s, const char *
             de_ctx, s, arg, DETECT_AL_HTTP_HOST, g_http_host_buffer_id, ALPROTO_HTTP1);
 }
 
-static bool DetectHttpHostValidateCallback(const Signature *s, const char **sigerror)
+static bool DetectHttpHostValidateCallback(
+        const Signature *s, const DetectContentData *cd, const char **sigerror)
 {
-    for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
-        if (s->init_data->buffers[x].id != (uint32_t)g_http_host_buffer_id)
-            continue;
-        const SigMatch *sm = s->init_data->buffers[x].head;
-        for (; sm != NULL; sm = sm->next) {
-            if (sm->type == DETECT_CONTENT) {
-                DetectContentData *cd = (DetectContentData *)sm->ctx;
-                if (cd->flags & DETECT_CONTENT_NOCASE) {
-                    *sigerror = "http.host keyword "
-                                "specified along with \"nocase\". "
-                                "The hostname buffer is normalized "
-                                "to lowercase, specifying "
-                                "nocase is redundant.";
-                    SCLogWarning("rule %u: %s", s->id, *sigerror);
-                    return false;
-                } else {
-                    uint32_t u;
-                    for (u = 0; u < cd->content_len; u++) {
-                        if (isupper(cd->content[u]))
-                            break;
-                    }
-                    if (u != cd->content_len) {
-                        *sigerror = "A pattern with "
-                                    "uppercase characters detected for http.host. "
-                                    "The hostname buffer is normalized to lowercase, "
-                                    "please specify a lowercase pattern.";
-                        SCLogWarning("rule %u: %s", s->id, *sigerror);
-                        return false;
-                    }
-                }
-            }
+    if (cd->flags & DETECT_CONTENT_NOCASE) {
+        *sigerror = "http.host keyword "
+                    "specified along with \"nocase\". "
+                    "The hostname buffer is normalized "
+                    "to lowercase, specifying "
+                    "nocase is redundant.";
+        SCLogWarning("rule %u: %s", s->id, *sigerror);
+        return false;
+    } else {
+        uint32_t u;
+        for (u = 0; u < cd->content_len; u++) {
+            if (isupper(cd->content[u]))
+                break;
+        }
+        if (u != cd->content_len) {
+            *sigerror = "A pattern with "
+                        "uppercase characters detected for http.host. "
+                        "The hostname buffer is normalized to lowercase, "
+                        "please specify a lowercase pattern.";
+            SCLogWarning("rule %u: %s", s->id, *sigerror);
+            return false;
         }
     }
-
     return true;
 }
 

src/detect-http-method.c

@@ -65,7 +65,8 @@ static int DetectHttpMethodSetupSticky(DetectEngineCtx *de_ctx, Signature *s, co
 void DetectHttpMethodRegisterTests(void);
 #endif
 void DetectHttpMethodFree(void *);
-static bool DetectHttpMethodValidateCallback(const Signature *s, const char **sigerror);
+static bool DetectHttpMethodValidateCallback(
+        const Signature *s, const DetectContentData *cd, const char **sigerror);
 static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx,
         const DetectEngineTransforms *transforms, Flow *_f,
         const uint8_t _flow_flags, void *txv, const int list_id);
@@ -161,35 +162,26 @@ static int DetectHttpMethodSetupSticky(DetectEngineCtx *de_ctx, Signature *s, co
  *  \retval 1 valid
  *  \retval 0 invalid
  */
-static bool DetectHttpMethodValidateCallback(const Signature *s, const char **sigerror)
+static bool DetectHttpMethodValidateCallback(
+        const Signature *s, const DetectContentData *cd, const char **sigerror)
 {
-    for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
-        if (s->init_data->buffers[x].id != (uint32_t)g_http_method_buffer_id)
-            continue;
-        const SigMatch *sm = s->init_data->buffers[x].head;
-        for (; sm != NULL; sm = sm->next) {
-            if (sm->type != DETECT_CONTENT)
-                continue;
-            const DetectContentData *cd = (const DetectContentData *)sm->ctx;
-            if (cd->content && cd->content_len) {
-                if (cd->content[cd->content_len - 1] == 0x20) {
-                    *sigerror = "http_method pattern with trailing space";
-                    SCLogError("%s", *sigerror);
-                    return false;
-                } else if (cd->content[0] == 0x20) {
-                    *sigerror = "http_method pattern with leading space";
-                    SCLogError("%s", *sigerror);
-                    return false;
-                } else if (cd->content[cd->content_len - 1] == 0x09) {
-                    *sigerror = "http_method pattern with trailing tab";
-                    SCLogError("%s", *sigerror);
-                    return false;
-                } else if (cd->content[0] == 0x09) {
-                    *sigerror = "http_method pattern with leading tab";
-                    SCLogError("%s", *sigerror);
-                    return false;
-                }
-            }
+    if (cd->content && cd->content_len) {
+        if (cd->content[cd->content_len - 1] == 0x20) {
+            *sigerror = "http_method pattern with trailing space";
+            SCLogError("%s", *sigerror);
+            return false;
+        } else if (cd->content[0] == 0x20) {
+            *sigerror = "http_method pattern with leading space";
+            SCLogError("%s", *sigerror);
+            return false;
+        } else if (cd->content[cd->content_len - 1] == 0x09) {
+            *sigerror = "http_method pattern with trailing tab";
+            SCLogError("%s", *sigerror);
+            return false;
+        } else if (cd->content[0] == 0x09) {
+            *sigerror = "http_method pattern with leading tab";
+            SCLogError("%s", *sigerror);
+            return false;
         }
     }
     return true;

src/detect-http-protocol.c

@@ -128,24 +128,15 @@ static InspectionBuffer *GetData2(DetectEngineThreadCtx *det_ctx,
     return buffer;
 }
 
-static bool DetectHttpProtocolValidateCallback(const Signature *s, const char **sigerror)
+static bool DetectHttpProtocolValidateCallback(
+        const Signature *s, const DetectContentData *cd, const char **sigerror)
 {
 #ifdef HAVE_HTP_CONFIG_SET_ALLOW_SPACE_URI
-    for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
-        if (s->init_data->buffers[x].id != (uint32_t)g_buffer_id)
-            continue;
-        const SigMatch *sm = s->init_data->buffers[x].head;
-        for (; sm != NULL; sm = sm->next) {
-            if (sm->type != DETECT_CONTENT)
-                continue;
-            const DetectContentData *cd = (DetectContentData *)sm->ctx;
-            for (size_t i = 0; i < cd->content_len; ++i) {
-                if (cd->content[i] == ' ') {
-                    *sigerror = "Invalid http.protocol string containing a space";
-                    SCLogWarning("rule %u: %s", s->id, *sigerror);
-                    return false;
-                }
-            }
+    for (size_t i = 0; i < cd->content_len; ++i) {
+        if (cd->content[i] == ' ') {
+            *sigerror = "Invalid http.protocol string containing a space";
+            SCLogWarning("rule %u: %s", s->id, *sigerror);
+            return false;
         }
     }
 #endif

src/detect-http-raw-header.c

@@ -58,7 +58,8 @@ static int DetectHttpRawHeaderSetupSticky(DetectEngineCtx *de_ctx, Signature *s,
 #ifdef UNITTESTS
 static void DetectHttpRawHeaderRegisterTests(void);
 #endif
-static bool DetectHttpRawHeaderValidateCallback(const Signature *s, const char **sigerror);
+static bool DetectHttpRawHeaderValidateCallback(
+        const Signature *s, const DetectContentData *cd, const char **sigerror);
 static int g_http_raw_header_buffer_id = 0;
 static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx,
         const DetectEngineTransforms *transforms, Flow *_f,
@@ -163,7 +164,8 @@ static int DetectHttpRawHeaderSetupSticky(DetectEngineCtx *de_ctx, Signature *s,
     return 0;
 }
 
-static bool DetectHttpRawHeaderValidateCallback(const Signature *s, const char **sigerror)
+static bool DetectHttpRawHeaderValidateCallback(
+        const Signature *s, const DetectContentData *cd, const char **sigerror)
 {
     if ((s->flags & (SIG_FLAG_TOCLIENT|SIG_FLAG_TOSERVER)) == (SIG_FLAG_TOCLIENT|SIG_FLAG_TOSERVER)) {
         *sigerror = "http_raw_header signature "

src/detect-http-uri.c

@@ -59,9 +59,7 @@
 #ifdef UNITTESTS
 static void DetectHttpUriRegisterTests(void);
 #endif
-static void DetectHttpUriSetupCallback(const DetectEngineCtx *de_ctx,
-                                       Signature *s);
-static bool DetectHttpUriValidateCallback(const Signature *s, const char **sigerror);
+static void DetectHttpUriSetupCallback(const DetectEngineCtx *de_ctx, Signature *s);
 static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx,
         const DetectEngineTransforms *transforms,
         Flow *_f, const uint8_t _flow_flags,
@@ -71,9 +69,7 @@ static InspectionBuffer *GetData2(DetectEngineThreadCtx *det_ctx,
         const int list_id);
 static int DetectHttpUriSetupSticky(DetectEngineCtx *de_ctx, Signature *s, const char *str);
 static int DetectHttpRawUriSetup(DetectEngineCtx *, Signature *, const char *);
-static void DetectHttpRawUriSetupCallback(const DetectEngineCtx *de_ctx,
-                                          Signature *s);
-static bool DetectHttpRawUriValidateCallback(const Signature *s, const char **);
+static void DetectHttpRawUriSetupCallback(const DetectEngineCtx *de_ctx, Signature *s);
 static InspectionBuffer *GetRawData(DetectEngineThreadCtx *det_ctx,
         const DetectEngineTransforms *transforms,
         Flow *_f, const uint8_t _flow_flags,
@@ -125,8 +121,7 @@ void DetectHttpUriRegister (void)
     DetectBufferTypeRegisterSetupCallback("http_uri",
             DetectHttpUriSetupCallback);
 
-    DetectBufferTypeRegisterValidateCallback("http_uri",
-            DetectHttpUriValidateCallback);
+    DetectBufferTypeRegisterValidateCallback("http_uri", DetectUrilenValidateContent);
 
     g_http_uri_buffer_id = DetectBufferTypeGetByName("http_uri");
 
@@ -164,8 +159,7 @@ void DetectHttpUriRegister (void)
     DetectBufferTypeRegisterSetupCallback("http_raw_uri",
             DetectHttpRawUriSetupCallback);
 
-    DetectBufferTypeRegisterValidateCallback("http_raw_uri",
-            DetectHttpRawUriValidateCallback);
+    DetectBufferTypeRegisterValidateCallback("http_raw_uri", DetectUrilenValidateContent);
 
     g_http_raw_uri_buffer_id = DetectBufferTypeGetByName("http_raw_uri");
 }
@@ -187,11 +181,6 @@ int DetectHttpUriSetup(DetectEngineCtx *de_ctx, Signature *s, const char *str)
             de_ctx, s, str, DETECT_AL_HTTP_URI, g_http_uri_buffer_id, ALPROTO_HTTP1);
 }
 
-static bool DetectHttpUriValidateCallback(const Signature *s, const char **sigerror)
-{
-    return DetectUrilenValidateContent(s, g_http_uri_buffer_id, sigerror);
-}
-
 static void DetectHttpUriSetupCallback(const DetectEngineCtx *de_ctx,
                                        Signature *s)
 {
@@ -282,11 +271,6 @@ static int DetectHttpRawUriSetup(DetectEngineCtx *de_ctx, Signature *s, const ch
             de_ctx, s, arg, DETECT_AL_HTTP_RAW_URI, g_http_raw_uri_buffer_id, ALPROTO_HTTP1);
 }
 
-static bool DetectHttpRawUriValidateCallback(const Signature *s, const char **sigerror)
-{
-    return DetectUrilenValidateContent(s, g_http_raw_uri_buffer_id, sigerror);
-}
-
 static void DetectHttpRawUriSetupCallback(const DetectEngineCtx *de_ctx,
                                           Signature *s)
 {