detect: save invalid rules

This keeps the invalid rules in string format into a list,
added in DetectEngineCtx.

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 \

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);
 

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 (*ValidateCallback)(const Signature *, const char **sigerror));
-_Bool DetectBufferRunValidateCallback(const int id, const Signature *s);
+_Bool DetectBufferRunValidateCallback(const int id, const Signature *s, const char **sigerror);
 
 /* prototypes */
 DetectEngineCtx *DetectEngineCtxInitWithPrefix(const char *prefix);

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;
                 }
             }

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;
             }
         }

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;

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)

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)

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);
             }
         }

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;

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 *);

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;
     }

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;

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 <glongo@stamus-networks.com>
+ *
+ * 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;
+}

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 <glongo@stamus-networks.com>
+ *
+ * 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);