detect/classtype: change duplicate classtype behavior

Detect duplicate instances and use the one with the highest
priority.

Use new priority flag to make the logic around explicit priority
sets easier to follow.

Minor code cleanups. Also clean up unittests.

diff --git a/src/detect-classtype.c b/src/detect-classtype.c
index 6577f9946de8bdead6d8f0875a2e81811e13f791..6122e523f9aba7988ade37445b3ab215d57943b2 100644 (file)
--- a/src/detect-classtype.c
+++ b/src/detect-classtype.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 2007-2010 Open Information Security Foundation
+/* Copyright (C) 2007-2019 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
@@ -19,6 +19,7 @@
  * \file
  *
  * \author Anoop Saldanha <anoopsaldanha@gmail.com>
+ * \author Victor Julien <victor@inliniac.net>
  *
  * Implements classtype keyword.
  */
@@ -29,9 +30,7 @@
 #include "detect.h"
 #include "detect-parse.h"
 #include "detect-engine.h"
-#include "detect-engine-mpm.h"
 #include "detect-classtype.h"
-#include "flow-var.h"
 #include "util-classification-config.h"
 #include "util-error.h"
 #include "util-debug.h"
@@ -53,9 +52,7 @@ void DetectClasstypeRegister(void)
     sigmatch_table[DETECT_CLASSTYPE].name = "classtype";
     sigmatch_table[DETECT_CLASSTYPE].desc = "information about the classification of rules and alerts";
     sigmatch_table[DETECT_CLASSTYPE].url = DOC_URL DOC_VERSION "/rules/meta.html#classtype";
-    sigmatch_table[DETECT_CLASSTYPE].Match = NULL;
     sigmatch_table[DETECT_CLASSTYPE].Setup = DetectClasstypeSetup;
-    sigmatch_table[DETECT_CLASSTYPE].Free  = NULL;
     sigmatch_table[DETECT_CLASSTYPE].RegisterTests = DetectClasstypeRegisterTests;
 
     DetectSetupParseRegexes(PARSE_REGEX, &regex, &regex_study);
@@ -71,25 +68,22 @@ void DetectClasstypeRegister(void)
 static int DetectClasstypeParseRawString(const char *rawstr, char *out, size_t outsize)
 {
 #define MAX_SUBSTRINGS 30
-    int ret = 0;
     int ov[MAX_SUBSTRINGS];
     size_t len = strlen(rawstr);
 
-    ret = pcre_exec(regex, regex_study, rawstr, len, 0, 0, ov, 30);
+    int ret = pcre_exec(regex, regex_study, rawstr, len, 0, 0, ov, 30);
     if (ret < 0) {
         SCLogError(SC_ERR_PCRE_MATCH, "Invalid Classtype in Signature");
-        goto end;
+        return -1;
     }
 
     ret = pcre_copy_substring((char *)rawstr, ov, 30, 1, out, outsize);
     if (ret < 0) {
         SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_copy_substring failed");
-        goto end;
+        return -1;
     }
 
     return 0;
- end:
-    return -1;
 }
 
 /**
@@ -109,8 +103,7 @@ static int DetectClasstypeSetup(DetectEngineCtx *de_ctx, Signature *s, const cha
 
     if ((s->class > 0) || (s->class_msg != NULL)) {
         SCLogWarning(SC_ERR_CONFLICTING_RULE_KEYWORDS, "duplicated 'classtype' "
-                "keyword detected. Using first occurence in the rule");
-        return 0;
+                "keyword detected. Using instance with highest priority");
     }
 
     if (DetectClasstypeParseRawString(rawstr, parsed_ct_name, sizeof(parsed_ct_name)) < 0) {
@@ -121,27 +114,30 @@ static int DetectClasstypeSetup(DetectEngineCtx *de_ctx, Signature *s, const cha
 
     SCClassConfClasstype *ct = SCClassConfGetClasstype(parsed_ct_name, de_ctx);
     if (ct == NULL) {
-        SCLogError(SC_ERR_UNKNOWN_VALUE, "Unknown Classtype: \"%s\".  Invalidating the Signature",
+        SCLogError(SC_ERR_UNKNOWN_VALUE, "unknown classtype: \"%s\"",
                    parsed_ct_name);
         return -1;
     }
 
-    /* if we have retrieved the classtype, assign the message to be displayed
-     * for this Signature by fast.log, if a Packet matches this Signature */
-    s->class = ct->classtype_id;
-    s->class_msg = ct->classtype_desc;
-
-    /* if a priority keyword has appeared before the classtype, s->prio would
-     * hold a value which is != -1, in which case we don't overwrite the value.
-     * Otherwise, overwrite the value */
-    if (s->prio == -1)
+    if ((s->init_data->init_flags & SIG_FLAG_INIT_PRIO_EXPLICT) != 0) {
+        /* don't touch Signature::prio */
+        s->class = ct->classtype_id;
+        s->class_msg = ct->classtype_desc;
+    } else if (s->prio == -1) {
         s->prio = ct->priority;
+        s->class = ct->classtype_id;
+        s->class_msg = ct->classtype_desc;
+    } else {
+        if (ct->priority < s->prio) {
+            s->prio = ct->priority;
+            s->class = ct->classtype_id;
+            s->class_msg = ct->classtype_desc;
+        }
+    }
 
     return 0;
 }
 
-/*------------------------------Unittests-------------------------------------*/
-
 #ifdef UNITTESTS
 
 /**
@@ -150,26 +146,19 @@ static int DetectClasstypeSetup(DetectEngineCtx *de_ctx, Signature *s, const cha
  */
 static int DetectClasstypeTest01(void)
 {
-    int result = 0;
-
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
-    if (de_ctx == NULL) {
-        goto end;
-    }
+    FAIL_IF_NULL(de_ctx);
 
     FILE *fd = SCClassConfGenerateValidDummyClassConfigFD01();
+    FAIL_IF_NULL(fd);
     SCClassConfLoadClassficationConfigFile(de_ctx, fd);
-
-    de_ctx->sig_list = SigInit(de_ctx, "alert tcp any any -> any any "
+    de_ctx->sig_list = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
                                "(msg:\"Classtype test\"; "
                                "Classtype:not_available; sid:1;)");
-    if (de_ctx->sig_list == NULL)
-        result = 1;
+    FAIL_IF_NOT_NULL(de_ctx->sig_list);
 
     DetectEngineCtxFree(de_ctx);
-
-end:
-    return result;
+    PASS;
 }
 
 /**
@@ -179,64 +168,41 @@ end:
  */
 static int DetectClasstypeTest02(void)
 {
-    int result = 0;
-    Signature *last = NULL;
-    Signature *sig = NULL;
-
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
-    if (de_ctx == NULL) {
-        goto end;
-    }
+    FAIL_IF_NULL(de_ctx);
 
     FILE *fd = SCClassConfGenerateValidDummyClassConfigFD01();
+    FAIL_IF_NULL(fd);
     SCClassConfLoadClassficationConfigFile(de_ctx, fd);
 
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
-                  "(msg:\"Classtype test\"; Classtype:bad-unknown; sid:1;)");
-    if (sig == NULL) {
-        printf("first sig failed to parse: ");
-        result = 0;
-        goto end;
-    }
-    de_ctx->sig_list = last = sig;
-    result = (sig != NULL);
-
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
-                  "(msg:\"Classtype test\"; Classtype:not-there; sid:1;)");
-    last->next = sig;
-    result &= (sig == NULL);
-
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
-                  "(msg:\"Classtype test\"; Classtype:Bad-UnkNown; sid:1;)");
-    if (sig == NULL) {
-        printf("second sig failed to parse: ");
-        result = 0;
-        goto end;
-    }
-    last->next = sig;
-    last = sig;
-    result &= (sig != NULL);
-
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
-                  "(msg:\"Classtype test\"; Classtype:nothing-wrong; sid:1;)");
-    if (sig == NULL) {
-        result = 0;
-        goto end;
-    }
-    last->next = sig;
-    last = sig;
-    result &= (sig != NULL);
+    Signature *sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(Classtype:bad-unknown; sid:1;)");
+    FAIL_IF_NULL(sig);
 
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
-                  "(msg:\"Classtype test\"; Classtype:attempted_dos; sid:1;)");
-    last->next = sig;
-    result &= (sig == NULL);
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(Classtype:not-there; sid:2;)");
+    FAIL_IF_NOT_NULL(sig);
 
-    SigCleanSignatures(de_ctx);
-    DetectEngineCtxFree(de_ctx);
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(Classtype:Bad-UnkNown; sid:3;)");
+    FAIL_IF_NULL(sig);
 
-end:
-    return result;
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(Classtype:nothing-wrong; sid:4;)");
+    FAIL_IF_NULL(sig);
+
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(Classtype:attempted_dos; sid:5;)");
+    FAIL_IF_NOT_NULL(sig);
+
+    /* duplicate test */
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(Classtype:nothing-wrong; Classtype:Bad-UnkNown; sid:6;)");
+    FAIL_IF_NULL(sig);
+    FAIL_IF_NOT(sig->prio == 2);
+
+    DetectEngineCtxFree(de_ctx);
+    PASS;
 }
 
 /**
@@ -245,57 +211,30 @@ end:
  */
 static int DetectClasstypeTest03(void)
 {
-    int result = 0;
-    Signature *last = NULL;
-    Signature *sig = NULL;
-
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
-    if (de_ctx == NULL) {
-        goto end;
-    }
+    FAIL_IF_NULL(de_ctx);
 
     FILE *fd = SCClassConfGenerateValidDummyClassConfigFD01();
+    FAIL_IF_NULL(fd);
     SCClassConfLoadClassficationConfigFile(de_ctx, fd);
 
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
+    Signature *sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
                   "(msg:\"Classtype test\"; Classtype:bad-unknown; priority:1; sid:1;)");
-    if (sig == NULL) {
-        result = 0;
-        goto end;
-    }
-    de_ctx->sig_list = last = sig;
-    result = (sig != NULL);
-    result &= (sig->prio == 1);
+    FAIL_IF_NULL(sig);
+    FAIL_IF_NOT(sig->prio == 1);
 
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
                   "(msg:\"Classtype test\"; Classtype:unKnoWn; "
-                  "priority:3; sid:1;)");
-    if (sig == NULL) {
-        result = 0;
-        goto end;
-    }
-    last->next = sig;
-    last = sig;
-    result &= (sig != NULL);
-    result &= (sig->prio == 3);
-
-    sig = SigInit(de_ctx, "alert tcp any any -> any any (msg:\"Classtype test\"; "
-                  "Classtype:nothing-wrong; priority:1; sid:1;)");
-    if (sig == NULL) {
-        result = 0;
-        goto end;
-    }
-    last->next = sig;
-    last = sig;
-    result &= (sig != NULL);
-    result &= (sig->prio == 1);
+                  "priority:3; sid:2;)");
+    FAIL_IF_NULL(sig);
+    FAIL_IF_NOT(sig->prio == 3);
 
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any (msg:\"Classtype test\"; "
+                  "Classtype:nothing-wrong; priority:1; sid:3;)");
+    FAIL_IF_NOT(sig->prio == 1);
 
-    SigCleanSignatures(de_ctx);
     DetectEngineCtxFree(de_ctx);
-
-end:
-    return result;
+    PASS;
 }
 
 #endif /* UNITTESTS */
@@ -305,13 +244,9 @@ end:
  */
 static void DetectClasstypeRegisterTests(void)
 {
-
 #ifdef UNITTESTS
-
     UtRegisterTest("DetectClasstypeTest01", DetectClasstypeTest01);
     UtRegisterTest("DetectClasstypeTest02", DetectClasstypeTest02);
     UtRegisterTest("DetectClasstypeTest03", DetectClasstypeTest03);
-
 #endif /* UNITTESTS */
-
 }