thresholds: Improve validation of threshold.config

This commit improves the handling of threshold.config. When used with
"-T", a non-zero return code occurs when the file cannot be validated.

To maintain legacy behavior, when "-T" is not used and threshold.config
contains one or more invalid lines, Suricata continues execution.

diff --git a/src/detect-engine-loader.c b/src/detect-engine-loader.c
index 271c31e9500731752ea316f300c611b0c5e28035..4c2f4e58d43f7f51116efd9788ac45a64066f8b9 100644 (file)
--- a/src/detect-engine-loader.c
+++ b/src/detect-engine-loader.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2015 Open Information Security Foundation
+/* Copyright (C) 2021 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
@@ -364,7 +364,10 @@ int SigLoadSignatures(DetectEngineCtx *de_ctx, char *sig_file, int sig_file_excl
     SCSigOrderSignatures(de_ctx);
     SCSigSignatureOrderingModuleCleanup(de_ctx);
 
-    SCThresholdConfInitContext(de_ctx);
+    if (SCThresholdConfInitContext(de_ctx) < 0) {
+        ret = -1;
+        goto end;
+    }
 
     /* Setup the signature group lookup structure and pattern matchers */
     if (SigGroupBuild(de_ctx) < 0)

diff --git a/src/util-threshold-config.c b/src/util-threshold-config.c
index b08935db06f23c250408cf6fb4b9845a473b6c1a..60c3eb9a83718f7c930a25bb3e8f70d2583e31a7 100644 (file)
--- a/src/util-threshold-config.c
+++ b/src/util-threshold-config.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2007-2016 Open Information Security Foundation
+/* Copyright (C) 2007-2021 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
@@ -238,6 +238,7 @@ static const char *SCThresholdConfGetConfFilename(const DetectEngineCtx *de_ctx)
 int SCThresholdConfInitContext(DetectEngineCtx *de_ctx)
 {
     const char *filename = NULL;
+    int ret = 0;
 #ifndef UNITTESTS
     FILE *fd = NULL;
 #else
@@ -253,7 +254,15 @@ int SCThresholdConfInitContext(DetectEngineCtx *de_ctx)
     }
 #endif
 
-    SCThresholdConfParseFile(de_ctx, fd);
+    if (SCThresholdConfParseFile(de_ctx, fd) < 0) {
+        SCLogWarning(
+                SC_WARN_THRESH_CONFIG, "Error loading threshold configuration from %s", filename);
+        /* maintain legacy behavior so no errors unless config testing */
+        if (RunmodeGetCurrent() == RUNMODE_CONF_TEST) {
+            ret = -1;
+        }
+        goto error;
+    }
     SCThresholdConfDeInitContext(de_ctx, fd);
 
 #ifdef UNITTESTS
@@ -264,7 +273,8 @@ int SCThresholdConfInitContext(DetectEngineCtx *de_ctx)
 
 error:
     SCThresholdConfDeInitContext(de_ctx, fd);
-    return -1;
+
+return ret;
 }
 
 /**
@@ -1064,7 +1074,7 @@ static int SCThresholdConfLineIsMultiline(char *line)
  * \param de_ctx Pointer to the Detection Engine Context.
  * \param fd Pointer to file descriptor.
  */
-void SCThresholdConfParseFile(DetectEngineCtx *de_ctx, FILE *fp)
+int SCThresholdConfParseFile(DetectEngineCtx *de_ctx, FILE *fp)
 {
     char line[8192] = "";
     int rule_num = 0;
@@ -1073,7 +1083,7 @@ void SCThresholdConfParseFile(DetectEngineCtx *de_ctx, FILE *fp)
     int esc_pos = 0;
 
     if (fp == NULL)
-        return;
+        return -1;
 
     while (fgets(line + esc_pos, (int)sizeof(line) - esc_pos, fp) != NULL) {
         if (SCThresholdConfIsLineBlankOrComment(line)) {
@@ -1082,15 +1092,19 @@ void SCThresholdConfParseFile(DetectEngineCtx *de_ctx, FILE *fp)
 
         esc_pos = SCThresholdConfLineIsMultiline(line);
         if (esc_pos == 0) {
-            rule_num++;
-            SCLogDebug("Adding threshold.config rule num %"PRIu32"( %s )", rule_num, line);
-            SCThresholdConfAddThresholdtype(line, de_ctx);
+            if (SCThresholdConfAddThresholdtype(line, de_ctx) < 0) {
+                if (RunmodeGetCurrent() == RUNMODE_CONF_TEST)
+                    return -1;
+            } else {
+                SCLogDebug("Adding threshold.config rule num %" PRIu32 "( %s )", rule_num, line);
+                rule_num++;
+            }
         }
     }
 
     SCLogInfo("Threshold config parsed: %d rule(s) found", rule_num);
 
-    return;
+    return 0;
 }
 
 #ifdef UNITTESTS
@@ -1334,7 +1348,7 @@ static int SCThresholdConfTest01(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD01();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD,
             DETECT_THRESHOLD, -1);
@@ -1367,7 +1381,7 @@ static int SCThresholdConfTest02(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD01();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD,
             DETECT_THRESHOLD, -1);
@@ -1400,7 +1414,7 @@ static int SCThresholdConfTest03(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD01();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD,
             DETECT_THRESHOLD, -1);
@@ -1433,7 +1447,7 @@ static int SCThresholdConfTest04(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateInValidDummyFD02();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD,
             DETECT_THRESHOLD, -1);
@@ -1469,7 +1483,7 @@ static int SCThresholdConfTest05(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD03();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     Signature *s = de_ctx->sig_list;
     SigMatch *m = DetectGetLastSMByListId(s, DETECT_SM_LIST_THRESHOLD,
@@ -1517,7 +1531,7 @@ static int SCThresholdConfTest06(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD04();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD,
             DETECT_THRESHOLD, -1);
@@ -1550,7 +1564,7 @@ static int SCThresholdConfTest07(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD05();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD,
             DETECT_DETECTION_FILTER, -1);
@@ -1584,7 +1598,7 @@ static int SCThresholdConfTest08(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD06();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigMatch *m = DetectGetLastSMByListId(sig, DETECT_SM_LIST_THRESHOLD,
             DETECT_DETECTION_FILTER, -1);
@@ -1631,7 +1645,7 @@ static int SCThresholdConfTest09(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD07();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigGroupBuild(de_ctx);
     DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx);
@@ -1725,7 +1739,7 @@ static int SCThresholdConfTest10(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD08();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigGroupBuild(de_ctx);
     DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx);
@@ -1818,7 +1832,7 @@ static int SCThresholdConfTest11(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD09();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigGroupBuild(de_ctx);
     DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx);
@@ -1927,7 +1941,7 @@ static int SCThresholdConfTest12(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD10();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigGroupBuild(de_ctx);
     DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx);
@@ -2017,7 +2031,7 @@ static int SCThresholdConfTest13(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD11();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigMatch *m = DetectGetLastSMByListId(sig,
             DETECT_SM_LIST_SUPPRESS, DETECT_THRESHOLD, -1);
@@ -2073,7 +2087,7 @@ static int SCThresholdConfTest14(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD11();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigGroupBuild(de_ctx);
     DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx);
@@ -2129,7 +2143,7 @@ static int SCThresholdConfTest15(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD11();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigGroupBuild(de_ctx);
     DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx);
@@ -2181,7 +2195,7 @@ static int SCThresholdConfTest16(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD11();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigGroupBuild(de_ctx);
     DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx);
@@ -2232,7 +2246,7 @@ static int SCThresholdConfTest17(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD11();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigGroupBuild(de_ctx);
     DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx);
@@ -2289,7 +2303,7 @@ static int SCThresholdConfTest18(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateInvalidDummyFD12();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
     SigGroupBuild(de_ctx);
 
     FAIL_IF_NULL(s->sm_arrays[DETECT_SM_LIST_SUPPRESS]);
@@ -2340,7 +2354,7 @@ static int SCThresholdConfTest19(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateInvalidDummyFD13();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
     SigGroupBuild(de_ctx);
     FAIL_IF_NULL(s->sm_arrays[DETECT_SM_LIST_SUPPRESS]);
     SigMatchData *smd = s->sm_arrays[DETECT_SM_LIST_SUPPRESS];
@@ -2390,7 +2404,7 @@ static int SCThresholdConfTest20(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD20();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
     SigGroupBuild(de_ctx);
     FAIL_IF_NULL(s->sm_arrays[DETECT_SM_LIST_SUPPRESS]);
 
@@ -2435,7 +2449,7 @@ static int SCThresholdConfTest21(void)
     FAIL_IF_NULL(s);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD20();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
     SigGroupBuild(de_ctx);
     FAIL_IF_NULL(s->sm_arrays[DETECT_SM_LIST_SUPPRESS]);
 
@@ -2522,7 +2536,7 @@ static int SCThresholdConfTest22(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD22();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigGroupBuild(de_ctx);
     DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx);
@@ -2658,7 +2672,7 @@ static int SCThresholdConfTest23(void)
     FAIL_IF_NOT_NULL(g_ut_threshold_fp);
     g_ut_threshold_fp = SCThresholdConfGenerateValidDummyFD23();
     FAIL_IF_NULL(g_ut_threshold_fp);
-    SCThresholdConfInitContext(de_ctx);
+    FAIL_IF(-1 == SCThresholdConfInitContext(de_ctx));
 
     SigGroupBuild(de_ctx);
     DetectEngineThreadCtxInit(&th_v, (void *)de_ctx, (void *)&det_ctx);

diff --git a/src/util-threshold-config.h b/src/util-threshold-config.h
index aec87d2152a6d1ae61159347d82456f2279e290e..982bf3e1c9c3f0ea56cd844fd87e03811bfdbf9c 100644 (file)
--- a/src/util-threshold-config.h
+++ b/src/util-threshold-config.h
@@ -24,7 +24,7 @@
 #ifndef __UTIL_THRESHOLD_CONFIG_H__
 #define __UTIL_THRESHOLD_CONFIG_H__
 
-void SCThresholdConfParseFile(DetectEngineCtx *, FILE *);
+int SCThresholdConfParseFile(DetectEngineCtx *, FILE *);
 int SCThresholdConfInitContext(DetectEngineCtx *);
 
 void SCThresholdConfRegisterTests(void);