detect: generic callback for md5-like keywords

Ticket: 5634

diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst
index d9b8495bf38b67473f2f654f3f2c0e706ffc570d..205bcd3d967fa95e9ac78f0c801a956124ebebd0 100644 (file)
--- a/doc/userguide/upgrade.rst
+++ b/doc/userguide/upgrade.rst
@@ -154,6 +154,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

diff --git a/src/detect-engine.c b/src/detect-engine.c
index 79c3478bfbc16e95cd5590f24ef0baa3c1e8f8aa..62b53e9a618f8e2c843b6c297e8f3a81dc580e8c 100644 (file)
--- a/src/detect-engine.c
+++ b/src/detect-engine.c
@@ -5124,6 +5124,47 @@ void DetectEngineSetEvent(DetectEngineThreadCtx *det_ctx, uint8_t e)
     det_ctx->events++;
 }
 
+bool DetectMd5ValidateCallback(
+        const Signature *s, const char **sigerror, const DetectBufferType *map)
+{
+    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 (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: buffer %s: %s", s->id, map->name, *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: buffer %s: %s", s->id, map->name, *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: buffer %s: %s", s->id, map->name, *sigerror);
+                    return false;
+                }
+            }
+        }
+    }
+    return true;
+}
+
 /*************************************Unittest*********************************/
 
 #ifdef UNITTESTS

diff --git a/src/detect-engine.h b/src/detect-engine.h
index 89649f0cc3bb06a5a0590f77d73a4811fb668697..bba132e27fffb0c1e68a3d9516fd3e0d118a07c9 100644 (file)
--- a/src/detect-engine.h
+++ b/src/detect-engine.h
@@ -212,6 +212,9 @@ void DetectRunStoreStateTx(const SigGroupHead *sgh, Flow *f, void *tx, uint64_t
 
 void DetectEngineStateResetTxs(Flow *f);
 
+bool DetectMd5ValidateCallback(
+        const Signature *s, const char **sigerror, const DetectBufferType *map);
+
 void DeStateRegisterTests(void);
 
 /* packet injection */

diff --git a/src/detect-quic-cyu-hash.c b/src/detect-quic-cyu-hash.c
index ce2412fbaed65fd7751dde16b42193a6e6557e5f..47cb112ed6eb7bde2daeaf538d1cea429fb2d227 100644 (file)
--- a/src/detect-quic-cyu-hash.c
+++ b/src/detect-quic-cyu-hash.c
@@ -82,47 +82,6 @@ static InspectionBuffer *QuicHashGetData(DetectEngineThreadCtx *det_ctx,
     SCReturnPtr(buffer, "InspectionBuffer");
 }
 
-static bool DetectQuicHashValidateCallback(
-        const Signature *s, const char **sigerror, const DetectBufferType *dbt)
-{
-    for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
-        if (s->init_data->buffers[x].id != (uint32_t)dbt->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 (cd->flags & DETECT_CONTENT_NOCASE) {
-                *sigerror = BUFFER_NAME " 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 != 32) {
-                *sigerror = "Invalid length of the specified" BUFFER_NAME " (should "
-                            "be 32 characters long). This rule will therefore "
-                            "never match.";
-                SCLogWarning("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 " BUFFER_NAME
-                                " string (should be string of hexadecimal characters)."
-                                "This rule will therefore never match.";
-                    SCLogWarning("rule %u: %s", s->id, *sigerror);
-                    return false;
-                }
-            }
-        }
-    }
-    return true;
-}
-
 void DetectQuicCyuHashRegister(void)
 {
     /* quic.cyu.hash sticky buffer */
@@ -142,7 +101,7 @@ void DetectQuicCyuHashRegister(void)
 
     g_buffer_id = DetectBufferTypeGetByName(BUFFER_NAME);
 
-    DetectBufferTypeRegisterValidateCallback(BUFFER_NAME, DetectQuicHashValidateCallback);
+    DetectBufferTypeRegisterValidateCallback(BUFFER_NAME, DetectMd5ValidateCallback);
 
     DetectBufferTypeSupportsMultiInstance(BUFFER_NAME);
 }

diff --git a/src/detect-ssh-hassh-server.c b/src/detect-ssh-hassh-server.c
index be6d9a8a50b636434f5d366dbbe0221498c54cfc..448aa1b2a60bbd4a2ddfb4348b69798b12a83a98 100644 (file)
--- a/src/detect-ssh-hassh-server.c
+++ b/src/detect-ssh-hassh-server.c
@@ -118,49 +118,7 @@ static int DetectSshHasshServerSetup(DetectEngineCtx *de_ctx, Signature *s, cons
 
 }
 
-static bool DetectSshHasshServerHashValidateCallback(
-        const Signature *s, const char **sigerror, const DetectBufferType *dbt)
-{
-    for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
-        if (s->init_data->buffers[x].id != (uint32_t)dbt->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 (cd->flags & DETECT_CONTENT_NOCASE) {
-                *sigerror = "ssh.hassh.server 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 != 32) {
-                *sigerror = "Invalid length of the specified ssh.hassh.server (should "
-                            "be 32 characters long). This rule will therefore "
-                            "never match.";
-                SCLogWarning("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 ssh.hassh.server string (should be string of hexadecimal "
-                                "characters)."
-                                "This rule will therefore never match.";
-                    SCLogWarning("rule %u: %s", s->id, *sigerror);
-                    return false;
-                }
-            }
-        }
-    }
-    return true;
-}
-
-static void DetectSshHasshServerHashSetupCallback(const DetectEngineCtx *de_ctx,
-                                          Signature *s)
+static void DetectSshHasshServerHashSetupCallback(const DetectEngineCtx *de_ctx, Signature *s)
 {
     for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
         if (s->init_data->buffers[x].id != (uint32_t)g_ssh_hassh_buffer_id)
@@ -207,5 +165,5 @@ void DetectSshHasshServerRegister(void)
     g_ssh_hassh_buffer_id = DetectBufferTypeGetByName(BUFFER_NAME);
 
     DetectBufferTypeRegisterSetupCallback(BUFFER_NAME, DetectSshHasshServerHashSetupCallback);
-    DetectBufferTypeRegisterValidateCallback(BUFFER_NAME, DetectSshHasshServerHashValidateCallback);
+    DetectBufferTypeRegisterValidateCallback(BUFFER_NAME, DetectMd5ValidateCallback);
 }

diff --git a/src/detect-ssh-hassh.c b/src/detect-ssh-hassh.c
index 02719be6189a1ace3b1b81b2eac189725c875564..e3593b9073659cdafb2184312d10f4f9608149f9 100644 (file)
--- a/src/detect-ssh-hassh.c
+++ b/src/detect-ssh-hassh.c
@@ -117,47 +117,6 @@ static int DetectSshHasshSetup(DetectEngineCtx *de_ctx, Signature *s, const char
 
 }
 
-static bool DetectSshHasshHashValidateCallback(
-        const Signature *s, const char **sigerror, const DetectBufferType *dbt)
-{
-    for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
-        if (s->init_data->buffers[x].id != (uint32_t)dbt->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 (cd->flags & DETECT_CONTENT_NOCASE) {
-                *sigerror = "ssh.hassh 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 != 32) {
-                *sigerror = "Invalid length of the specified ssh.hassh (should "
-                            "be 32 characters long). This rule will therefore "
-                            "never match.";
-                SCLogWarning("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 ssh.hassh string (should be string of hexadecimal characters)."
-                            "This rule will therefore never match.";
-                    SCLogWarning("rule %u: %s", s->id, *sigerror);
-                    return false;
-                }
-            }
-        }
-    }
-    return true;
-}
-
 static void DetectSshHasshHashSetupCallback(const DetectEngineCtx *de_ctx,
                                           Signature *s)
 {
@@ -206,6 +165,6 @@ void DetectSshHasshRegister(void)
     g_ssh_hassh_buffer_id = DetectBufferTypeGetByName(BUFFER_NAME);
 
     DetectBufferTypeRegisterSetupCallback(BUFFER_NAME, DetectSshHasshHashSetupCallback);
-    DetectBufferTypeRegisterValidateCallback(BUFFER_NAME, DetectSshHasshHashValidateCallback);
+    DetectBufferTypeRegisterValidateCallback(BUFFER_NAME, DetectMd5ValidateCallback);
 }
 

diff --git a/src/detect-tls-ja3-hash.c b/src/detect-tls-ja3-hash.c
index 5522a2a809ab0accca7e5b011604487ffce5ad44..a326f1232e7ea641129f177753a6b1dd7ecb3e77 100644 (file)
--- a/src/detect-tls-ja3-hash.c
+++ b/src/detect-tls-ja3-hash.c
@@ -70,10 +70,7 @@ static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx,
        const DetectEngineTransforms *transforms,
        Flow *f, const uint8_t flow_flags,
        void *txv, const int list_id);
-static void DetectTlsJa3HashSetupCallback(const DetectEngineCtx *de_ctx,
-       Signature *s);
-static bool DetectTlsJa3HashValidateCallback(
-        const Signature *s, const char **sigerror, const DetectBufferType *dbt);
+static void DetectTlsJa3HashSetupCallback(const DetectEngineCtx *de_ctx, Signature *s);
 static int g_tls_ja3_hash_buffer_id = 0;
 #endif
 
@@ -112,8 +109,7 @@ void DetectTlsJa3HashRegister(void)
     DetectBufferTypeRegisterSetupCallback("ja3.hash",
             DetectTlsJa3HashSetupCallback);
 
-    DetectBufferTypeRegisterValidateCallback("ja3.hash",
-            DetectTlsJa3HashValidateCallback);
+    DetectBufferTypeRegisterValidateCallback("ja3.hash", DetectMd5ValidateCallback);
 
     g_tls_ja3_hash_buffer_id = DetectBufferTypeGetByName("ja3.hash");
 #endif /* HAVE_JA3 */
@@ -178,39 +174,6 @@ static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx,
     return buffer;
 }
 
-static bool DetectTlsJa3HashValidateCallback(
-        const Signature *s, const char **sigerror, const DetectBufferType *dbt)
-{
-    for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
-        if (s->init_data->buffers[x].id != (uint32_t)dbt->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 (cd->flags & DETECT_CONTENT_NOCASE) {
-                *sigerror = "ja3.hash 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)
-                return true;
-
-            *sigerror = "Invalid length of the specified JA3 hash (should "
-                        "be 32 characters long). This rule will therefore "
-                        "never match.";
-            SCLogWarning("rule %u: %s", s->id, *sigerror);
-            return false;
-        }
-    }
-    return true;
-}
-
 static void DetectTlsJa3HashSetupCallback(const DetectEngineCtx *de_ctx,
                                           Signature *s)
 {

diff --git a/src/detect-tls-ja3s-hash.c b/src/detect-tls-ja3s-hash.c
index 484e02ebfa0b0e0efd2d295ac1c0bf1703f28a59..ee2c7ef4f73e35535a89a3defe86424819bae988 100644 (file)
--- a/src/detect-tls-ja3s-hash.c
+++ b/src/detect-tls-ja3s-hash.c
@@ -70,10 +70,7 @@ static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx,
        const DetectEngineTransforms *transforms,
        Flow *f, const uint8_t flow_flags,
        void *txv, const int list_id);
-static void DetectTlsJa3SHashSetupCallback(const DetectEngineCtx *de_ctx,
-       Signature *s);
-static bool DetectTlsJa3SHashValidateCallback(
-        const Signature *s, const char **sigerror, const DetectBufferType *dbt);
+static void DetectTlsJa3SHashSetupCallback(const DetectEngineCtx *de_ctx, Signature *s);
 static int g_tls_ja3s_hash_buffer_id = 0;
 #endif
 
@@ -111,8 +108,7 @@ void DetectTlsJa3SHashRegister(void)
     DetectBufferTypeRegisterSetupCallback("ja3s.hash",
             DetectTlsJa3SHashSetupCallback);
 
-    DetectBufferTypeRegisterValidateCallback("ja3s.hash",
-            DetectTlsJa3SHashValidateCallback);
+    DetectBufferTypeRegisterValidateCallback("ja3s.hash", DetectMd5ValidateCallback);
 
     g_tls_ja3s_hash_buffer_id = DetectBufferTypeGetByName("ja3s.hash");
 #endif /* HAVE_JA3 */
@@ -176,39 +172,6 @@ static InspectionBuffer *GetData(DetectEngineThreadCtx *det_ctx,
     return buffer;
 }
 
-static bool DetectTlsJa3SHashValidateCallback(
-        const Signature *s, const char **sigerror, const DetectBufferType *dbt)
-{
-    for (uint32_t x = 0; x < s->init_data->buffer_index; x++) {
-        if (s->init_data->buffers[x].id != (uint32_t)dbt->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 (cd->flags & DETECT_CONTENT_NOCASE) {
-                *sigerror = "ja3s.hash 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)
-                return true;
-
-            *sigerror = "Invalid length of the specified JA3S hash (should "
-                        "be 32 characters long). This rule will therefore "
-                        "never match.";
-            SCLogError("rule %u: %s", s->id, *sigerror);
-            return false;
-        }
-    }
-    return true;
-}
-
 static void DetectTlsJa3SHashSetupCallback(const DetectEngineCtx *de_ctx,
                                            Signature *s)
 {