From 1328ecb8f6e981f6ce47704b17d7884f21d1d4cc Mon Sep 17 00:00:00 2001 From: Giuseppe Longo Date: Mon, 19 Oct 2015 15:02:13 +0200 Subject: [PATCH] detect: save invalid rules This keeps the invalid rules in string format into a list, added in DetectEngineCtx. --- src/Makefile.am | 1 + src/detect-engine.c | 27 ++++++-- src/detect-engine.h | 4 +- src/detect-http-hh.c | 15 +++-- src/detect-http-method.c | 16 +++-- src/detect-http-raw-header.c | 10 +-- src/detect-http-raw-uri.c | 6 +- src/detect-http-uri.c | 6 +- src/detect-parse.c | 2 +- src/detect-urilen.c | 3 +- src/detect-urilen.h | 2 +- src/detect.c | 8 +++ src/detect.h | 10 +++ src/util-detect.c | 120 +++++++++++++++++++++++++++++++++++ src/util-detect.h | 28 ++++++++ 15 files changed, 227 insertions(+), 31 deletions(-) create mode 100644 src/util-detect.c create mode 100644 src/util-detect.h diff --git a/src/Makefile.am b/src/Makefile.am index 171daf3c68..a97e4f05c8 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -389,6 +389,7 @@ util-decode-asn1.c util-decode-asn1.h \ util-decode-der.c util-decode-der.h \ util-decode-der-get.c util-decode-der-get.h \ util-decode-mime.c util-decode-mime.h \ +util-detect.c util-detect.h \ util-device.c util-device.h \ util-enum.c util-enum.h \ util-error.c util-error.h \ diff --git a/src/detect-engine.c b/src/detect-engine.c index 87f944e77d..8ff453f919 100644 --- a/src/detect-engine.c +++ b/src/detect-engine.c @@ -369,7 +369,7 @@ typedef struct DetectBufferType_ { _Bool mpm; _Bool packet; /**< compat to packet matches */ void (*SetupCallback)(Signature *); - _Bool (*ValidateCallback)(const Signature *); + _Bool (*ValidateCallback)(const Signature *, const char **sigerror); } DetectBufferType; static DetectBufferType **g_buffer_type_map = NULL; @@ -579,7 +579,7 @@ void DetectBufferRunSetupCallback(const int id, Signature *s) } void DetectBufferTypeRegisterValidateCallback(const char *name, - _Bool (*ValidateCallback)(const Signature *)) + _Bool (*ValidateCallback)(const Signature *, const char **sigerror)) { BUG_ON(g_buffer_type_reg_closed); DetectBufferTypeRegister(name); @@ -588,11 +588,11 @@ void DetectBufferTypeRegisterValidateCallback(const char *name, exists->ValidateCallback = ValidateCallback; } -_Bool DetectBufferRunValidateCallback(const int id, const Signature *s) +_Bool DetectBufferRunValidateCallback(const int id, const Signature *s, const char **sigerror) { const DetectBufferType *map = DetectBufferTypeGetById(id); if (map && map->ValidateCallback) { - return map->ValidateCallback(s); + return map->ValidateCallback(s, sigerror); } return TRUE; } @@ -1000,6 +1000,8 @@ static DetectEngineCtx *DetectEngineCtxInitReal(int minimal, const char *prefix) memset(de_ctx,0,sizeof(DetectEngineCtx)); memset(&de_ctx->sig_stat, 0, sizeof(SigFileLoaderStat)); + TAILQ_INIT(&de_ctx->sig_stat.failed_sigs); + de_ctx->sigerror = NULL; if (minimal) { de_ctx->minimal = 1; @@ -1098,6 +1100,22 @@ static void DetectEngineCtxFreeThreadKeywordData(DetectEngineCtx *de_ctx) de_ctx->keyword_list = NULL; } +static void DetectEngineCtxFreeFailedSigs(DetectEngineCtx *de_ctx) +{ + SigString *item = NULL; + SigString *sitem; + + TAILQ_FOREACH_SAFE(item, &de_ctx->sig_stat.failed_sigs, next, sitem) { + SCFree(item->filename); + SCFree(item->sig_str); + if (item->sig_error) { + SCFree(item->sig_error); + } + TAILQ_REMOVE(&de_ctx->sig_stat.failed_sigs, item, next); + SCFree(item); + } +} + /** * \brief Free a DetectEngineCtx:: * @@ -1148,6 +1166,7 @@ void DetectEngineCtxFree(DetectEngineCtx *de_ctx) DetectEngineCtxFreeThreadKeywordData(de_ctx); SRepDestroy(de_ctx); + DetectEngineCtxFreeFailedSigs(de_ctx); DetectAddressMapFree(de_ctx); diff --git a/src/detect-engine.h b/src/detect-engine.h index bd738550d5..833d3fcfe0 100644 --- a/src/detect-engine.h +++ b/src/detect-engine.h @@ -44,8 +44,8 @@ void DetectBufferTypeRegisterSetupCallback(const char *name, void (*Callback)(Signature *)); void DetectBufferRunSetupCallback(const int id, Signature *s); void DetectBufferTypeRegisterValidateCallback(const char *name, - _Bool (*ValidateCallback)(const Signature *)); -_Bool DetectBufferRunValidateCallback(const int id, const Signature *s); + _Bool (*ValidateCallback)(const Signature *, const char **sigerror)); +_Bool DetectBufferRunValidateCallback(const int id, const Signature *s, const char **sigerror); /* prototypes */ DetectEngineCtx *DetectEngineCtxInitWithPrefix(const char *prefix); diff --git a/src/detect-http-hh.c b/src/detect-http-hh.c index 2cb3e8221e..ad92c06427 100644 --- a/src/detect-http-hh.c +++ b/src/detect-http-hh.c @@ -63,7 +63,7 @@ static int DetectHttpHHSetup(DetectEngineCtx *, Signature *, const char *); static void DetectHttpHHRegisterTests(void); static void DetectHttpHHFree(void *); static void DetectHttpHostSetupCallback(Signature *s); -static _Bool DetectHttpHostValidateCallback(const Signature *s); +static _Bool DetectHttpHostValidateCallback(const Signature *s, const char **sigerror); static int g_http_host_buffer_id = 0; /** @@ -126,18 +126,20 @@ static void DetectHttpHostSetupCallback(Signature *s) s->mask |= SIG_MASK_REQUIRE_HTTP_STATE; } -static _Bool DetectHttpHostValidateCallback(const Signature *s) +static _Bool DetectHttpHostValidateCallback(const Signature *s, const char **sigerror) { const SigMatch *sm = s->init_data->smlists[g_http_host_buffer_id]; for ( ; sm != NULL; sm = sm->next) { if (sm->type == DETECT_CONTENT) { DetectContentData *cd = (DetectContentData *)sm->ctx; if (cd->flags & DETECT_CONTENT_NOCASE) { - SCLogWarning(SC_WARN_POOR_RULE, "rule %u: http_host keyword " + *sigerror = "http_host keyword " "specified along with \"nocase\". " "Since the hostname buffer we match against " "is actually lowercase. So having a " - "nocase is redundant.", s->id); + "nocase is redundant."; + SCLogWarning(SC_WARN_POOR_RULE, "rule %u: %s", s->id, *sigerror); + return FALSE; } else { uint32_t u; for (u = 0; u < cd->content_len; u++) { @@ -145,11 +147,12 @@ static _Bool DetectHttpHostValidateCallback(const Signature *s) break; } if (u != cd->content_len) { - SCLogWarning(SC_WARN_POOR_RULE, "rule %u: A pattern with " + *sigerror = "A pattern with " "uppercase chars detected for http_host. " "Since the hostname buffer we match against " "is lowercase only, please specify a " - "lowercase pattern.", s->id); + "lowercase pattern."; + SCLogWarning(SC_WARN_POOR_RULE, "rule %u: %s", s->id, *sigerror); return FALSE; } } diff --git a/src/detect-http-method.c b/src/detect-http-method.c index 0e7424ae86..72165c0ec1 100644 --- a/src/detect-http-method.c +++ b/src/detect-http-method.c @@ -65,7 +65,7 @@ static int DetectHttpMethodSetup(DetectEngineCtx *, Signature *, const char *); void DetectHttpMethodRegisterTests(void); void DetectHttpMethodFree(void *); static void DetectHttpMethodSetupCallback(Signature *s); -static _Bool DetectHttpMethodValidateCallback(const Signature *s); +static _Bool DetectHttpMethodValidateCallback(const Signature *s, const char **sigerror); /** * \brief Registration function for keyword: http_method @@ -144,7 +144,7 @@ static void DetectHttpMethodSetupCallback(Signature *s) * \retval 1 valid * \retval 0 invalid */ -static _Bool DetectHttpMethodValidateCallback(const Signature *s) +static _Bool DetectHttpMethodValidateCallback(const Signature *s, const char **sigerror) { const SigMatch *sm = s->init_data->smlists[g_http_method_buffer_id]; for ( ; sm != NULL; sm = sm->next) { @@ -153,16 +153,20 @@ static _Bool DetectHttpMethodValidateCallback(const Signature *s) const DetectContentData *cd = (const DetectContentData *)sm->ctx; if (cd->content && cd->content_len) { if (cd->content[cd->content_len-1] == 0x20) { - SCLogError(SC_ERR_INVALID_SIGNATURE, "http_method pattern with trailing space"); + *sigerror = "http_method pattern with trailing space"; + SCLogError(SC_ERR_INVALID_SIGNATURE, "%s", *sigerror); return FALSE; } else if (cd->content[0] == 0x20) { - SCLogError(SC_ERR_INVALID_SIGNATURE, "http_method pattern with leading space"); + *sigerror = "http_method pattern with leading space"; + SCLogError(SC_ERR_INVALID_SIGNATURE, "%s", *sigerror); return FALSE; } else if (cd->content[cd->content_len-1] == 0x09) { - SCLogError(SC_ERR_INVALID_SIGNATURE, "http_method pattern with trailing tab"); + *sigerror = "http_method pattern with trailing tab"; + SCLogError(SC_ERR_INVALID_SIGNATURE, "%s", *sigerror); return FALSE; } else if (cd->content[0] == 0x09) { - SCLogError(SC_ERR_INVALID_SIGNATURE, "http_method pattern with leading tab"); + *sigerror = "http_method pattern with leading tab"; + SCLogError(SC_ERR_INVALID_SIGNATURE, "%s", *sigerror); return FALSE; } } diff --git a/src/detect-http-raw-header.c b/src/detect-http-raw-header.c index 98b5f7687e..8c99d0f944 100644 --- a/src/detect-http-raw-header.c +++ b/src/detect-http-raw-header.c @@ -63,7 +63,7 @@ static int DetectHttpRawHeaderSetup(DetectEngineCtx *, Signature *, const char *); static void DetectHttpRawHeaderRegisterTests(void); static void DetectHttpRawHeaderFree(void *); -static _Bool DetectHttpRawHeaderValidateCallback(const Signature *s); +static _Bool DetectHttpRawHeaderValidateCallback(const Signature *s, const char **sigerror); static void DetectHttpRawHeaderSetupCallback(Signature *s); static int g_http_raw_header_buffer_id = 0; @@ -143,13 +143,15 @@ int DetectHttpRawHeaderSetup(DetectEngineCtx *de_ctx, Signature *s, const char * ALPROTO_HTTP); } -static _Bool DetectHttpRawHeaderValidateCallback(const Signature *s) +static _Bool DetectHttpRawHeaderValidateCallback(const Signature *s, const char **sigerror) { if ((s->flags & (SIG_FLAG_TOCLIENT|SIG_FLAG_TOSERVER)) == (SIG_FLAG_TOCLIENT|SIG_FLAG_TOSERVER)) { - SCLogError(SC_ERR_INVALID_SIGNATURE,"http_raw_header signature " + *sigerror = "http_raw_header signature " "without a flow direction. Use flow:to_server for " "inspecting request headers or flow:to_client for " - "inspecting response headers."); + "inspecting response headers."; + + SCLogError(SC_ERR_INVALID_SIGNATURE, "%s", *sigerror); SCReturnInt(FALSE); } return TRUE; diff --git a/src/detect-http-raw-uri.c b/src/detect-http-raw-uri.c index 9553df23f9..369fadd7cf 100644 --- a/src/detect-http-raw-uri.c +++ b/src/detect-http-raw-uri.c @@ -59,7 +59,7 @@ static int DetectHttpRawUriSetup(DetectEngineCtx *, Signature *, const char *); static void DetectHttpRawUriRegisterTests(void); static void DetectHttpRawUriSetupCallback(Signature *s); -static bool DetectHttpRawUriValidateCallback(const Signature *s); +static bool DetectHttpRawUriValidateCallback(const Signature *s, const char **); static int g_http_raw_uri_buffer_id = 0; /** @@ -113,9 +113,9 @@ static int DetectHttpRawUriSetup(DetectEngineCtx *de_ctx, Signature *s, const ch ALPROTO_HTTP); } -static bool DetectHttpRawUriValidateCallback(const Signature *s) +static bool DetectHttpRawUriValidateCallback(const Signature *s, const char **sigerror) { - return DetectUrilenValidateContent(s, g_http_raw_uri_buffer_id); + return DetectUrilenValidateContent(s, g_http_raw_uri_buffer_id, sigerror); } static void DetectHttpRawUriSetupCallback(Signature *s) diff --git a/src/detect-http-uri.c b/src/detect-http-uri.c index 6033887638..6eaebbb358 100644 --- a/src/detect-http-uri.c +++ b/src/detect-http-uri.c @@ -59,7 +59,7 @@ static void DetectHttpUriRegisterTests(void); static void DetectHttpUriSetupCallback(Signature *s); -static bool DetectHttpUriValidateCallback(const Signature *s); +static bool DetectHttpUriValidateCallback(const Signature *s, const char **sigerror); static int g_http_uri_buffer_id = 0; @@ -117,9 +117,9 @@ int DetectHttpUriSetup(DetectEngineCtx *de_ctx, Signature *s, const char *str) ALPROTO_HTTP); } -static bool DetectHttpUriValidateCallback(const Signature *s) +static bool DetectHttpUriValidateCallback(const Signature *s, const char **sigerror) { - return DetectUrilenValidateContent(s, g_http_uri_buffer_id); + return DetectUrilenValidateContent(s, g_http_uri_buffer_id, sigerror); } static void DetectHttpUriSetupCallback(Signature *s) diff --git a/src/detect-parse.c b/src/detect-parse.c index 9f6f37c994..71eea60f3e 100644 --- a/src/detect-parse.c +++ b/src/detect-parse.c @@ -1487,7 +1487,7 @@ static int SigValidate(DetectEngineCtx *de_ctx, Signature *s) int x; for (x = 0; x < nlists; x++) { if (s->init_data->smlists[x]) { - if (DetectBufferRunValidateCallback(x, s) == FALSE) { + if (DetectBufferRunValidateCallback(x, s, &de_ctx->sigerror) == FALSE) { SCReturnInt(0); } } diff --git a/src/detect-urilen.c b/src/detect-urilen.c index cd24edbae6..ab4d6b6242 100644 --- a/src/detect-urilen.c +++ b/src/detect-urilen.c @@ -342,7 +342,7 @@ void DetectUrilenApplyToContent(Signature *s, int list) } } -bool DetectUrilenValidateContent(const Signature *s, int list) +bool DetectUrilenValidateContent(const Signature *s, int list, const char **sigerror) { const SigMatch *sm = s->init_data->smlists[list]; for ( ; sm != NULL; sm = sm->next) { @@ -355,6 +355,7 @@ bool DetectUrilenValidateContent(const Signature *s, int list) } if (cd->depth && cd->depth < cd->content_len) { + *sigerror = "depth or urilen smaller than content len"; SCLogError(SC_ERR_INVALID_SIGNATURE, "depth or urilen %u smaller " "than content len %u", cd->depth, cd->content_len); return false; diff --git a/src/detect-urilen.h b/src/detect-urilen.h index b5fe79408f..30f53c6aa7 100644 --- a/src/detect-urilen.h +++ b/src/detect-urilen.h @@ -36,7 +36,7 @@ typedef struct DetectUrilenData_ { uint8_t raw_buffer; }DetectUrilenData; -bool DetectUrilenValidateContent(const Signature *s, int list); +bool DetectUrilenValidateContent(const Signature *s, int list, const char **); void DetectUrilenApplyToContent(Signature *s, int list); int DetectUrilenMatch (ThreadVars *, DetectEngineThreadCtx *, Flow *, uint8_t, void *, Signature *, SigMatch *); diff --git a/src/detect.c b/src/detect.c index c2171483ed..eb34f15329 100644 --- a/src/detect.c +++ b/src/detect.c @@ -228,6 +228,7 @@ #include "util-optimize.h" #include "util-path.h" #include "util-mpm-ac.h" +#include "util-detect.h" #include "runmodes.h" #ifdef HAVE_GLOB_H @@ -372,6 +373,13 @@ static int DetectLoadSigFile(DetectEngineCtx *de_ctx, char *sig_file, EngineAnalysisRulesFailure(line, sig_file, lineno - multiline); } bad++; + if (!SigStringAppend(&de_ctx->sig_stat, sig_file, line, de_ctx->sigerror, (lineno - multiline))) { + SCLogError(SC_ERR_MEM_ALLOC, "Error adding sig \"%s\" from " + "file %s at line %"PRId32"", line, sig_file, lineno - multiline); + } + if (de_ctx->sigerror) { + de_ctx->sigerror = NULL; + } } multiline = 0; } diff --git a/src/detect.h b/src/detect.h index 49a7ed1e76..b123fc1722 100644 --- a/src/detect.h +++ b/src/detect.h @@ -574,8 +574,17 @@ typedef struct ThresholdCtx_ { uint32_t th_size; } ThresholdCtx; +typedef struct SigString_ { + char *filename; + char *sig_str; + char *sig_error; + int line; + TAILQ_ENTRY(SigString_) next; +} SigString; + /** \brief Signature loader statistics */ typedef struct SigFileLoaderStat_ { + TAILQ_HEAD(, SigString_) failed_sigs; int bad_files; int total_files; int good_sigs_total; @@ -701,6 +710,7 @@ typedef struct DetectEngineCtx_ { /** Store rule file and line so that parsers can use them in errors. */ char *rule_file; int rule_line; + const char *sigerror; /** list of keywords that need thread local ctxs */ DetectEngineThreadKeywordCtxItem *keyword_list; diff --git a/src/util-detect.c b/src/util-detect.c new file mode 100644 index 0000000000..37ed2e3c40 --- /dev/null +++ b/src/util-detect.c @@ -0,0 +1,120 @@ +/* Copyright (C) 2017 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +/** + * \file + * + * \author Giuseppe Longo + * + * Detection engine helper functions + */ + +#include "suricata-common.h" +#include "suricata.h" +#include "detect.h" +#include "util-detect.h" + +/** + * \brief Allocate SigString list member + * + * \retval Pointer to SigString + */ +SigString *SigStringAlloc(void) +{ + SigString *sigstr = SCCalloc(1, sizeof(SigString)); + if (unlikely(sigstr == NULL)) + return NULL; + + sigstr->line = 0; + + return sigstr; +} + +/** + * \brief Assigns the filename, signature, lineno to SigString list member + * + * \param sig pointer to SigString + * \param sig_file filename that contains the signature + * \param sig_str signature in string format + * \param sig_error signature parsing error + * \param line line line number + * + * \retval 1 on success 0 on failure + */ +static int SigStringAddSig(SigString *sig, const char *sig_file, + const char *sig_str, const char *sig_error, + int line) +{ + if (sig_file == NULL || sig_str == NULL) { + return 0; + } + + sig->filename = SCStrdup(sig_file); + if (sig->filename == NULL) { + SCLogError(SC_ERR_MEM_ALLOC, "Error allocating memory"); + return 0; + } + + sig->sig_str = SCStrdup(sig_str); + if (sig->sig_str == NULL) { + SCLogError(SC_ERR_MEM_ALLOC, "Error allocating memory"); + SCFree(sig->filename); + return 0; + } + + if (sig_error) { + sig->sig_error = SCStrdup(sig_error); + if (sig->sig_error == NULL) { + SCLogError(SC_ERR_MEM_ALLOC, "Error allocating memory"); + SCFree(sig->filename); + SCFree(sig->sig_str); + return 0; + } + } + + sig->line = line; + + return 1; +} + +/** + * \brief Append a new list member to SigString list + * + * \param list pointer to the start of the SigString list + * \param sig_file filename that contains the signature + * \param sig_str signature in string format + * \param line line line number + * + * \retval 1 on success 0 on failure + */ +int SigStringAppend(SigFileLoaderStat *sig_stats, const char *sig_file, + const char *sig_str, const char *sig_error, int line) +{ + SigString *item = SigStringAlloc(); + if (item == NULL) { + return 0; + } + + if (!SigStringAddSig(item, sig_file, sig_str, sig_error, line)) { + SCFree(item); + return 0; + } + + TAILQ_INSERT_TAIL(&sig_stats->failed_sigs, item, next); + + return 1; +} diff --git a/src/util-detect.h b/src/util-detect.h new file mode 100644 index 0000000000..b483d507fb --- /dev/null +++ b/src/util-detect.h @@ -0,0 +1,28 @@ +/* Copyright (C) 2017 Open Information Security Foundation + * + * You can copy, redistribute or modify this Program under the terms of + * the GNU General Public License version 2 as published by the Free + * Software Foundation. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * version 2 along with this program; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA + * 02110-1301, USA. + */ + +/** + * \file + * + * \author Giuseppe Longo + * + * Detection engine helper functions + */ + +SigString *SigStringAlloc(void); +int SigStringAppend(SigFileLoaderStat *sigs_stats, const char *sig_file, const char *sig_str, + const char *sig_error, int line);