From 1851030010ddd298d4b35ea46c5395b4a105c27c Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Tue, 7 Jan 2025 14:34:19 +0100 Subject: [PATCH] detect: generic callback for md5-like keywords Ticket: 5634 --- doc/userguide/upgrade.rst | 5 ++++ src/detect-engine.c | 41 +++++++++++++++++++++++++++++++ src/detect-engine.h | 3 +++ src/detect-quic-cyu-hash.c | 43 +------------------------------- src/detect-ssh-hassh-server.c | 46 ++--------------------------------- src/detect-ssh-hassh.c | 43 +------------------------------- src/detect-tls-ja3-hash.c | 41 ++----------------------------- src/detect-tls-ja3s-hash.c | 41 ++----------------------------- 8 files changed, 57 insertions(+), 206 deletions(-) diff --git a/doc/userguide/upgrade.rst b/doc/userguide/upgrade.rst index d9b8495bf3..205bcd3d96 100644 --- 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 79c3478bfb..62b53e9a61 100644 --- 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 89649f0cc3..bba132e27f 100644 --- 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 ce2412fbae..47cb112ed6 100644 --- 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 be6d9a8a50..448aa1b2a6 100644 --- 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 02719be618..e3593b9073 100644 --- 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 5522a2a809..a326f1232e 100644 --- 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 484e02ebfa..ee2c7ef4f7 100644 --- 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) {