From: Victor Julien <victor@inliniac.net>
Date: Tue, 1 Oct 2019 08:55:37 +0000 (+0200)
Subject: detect/priority: change duplicate priority behavior
X-Git-Tag: suricata-5.0.0~78
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c471d81f04f1b4b423db7c88a967e2c91512773a;p=thirdparty%2Fsuricata.git

detect/priority: change duplicate priority behavior

Introduce Signature init_flag to indicate priority has been set.
This will be needed in a follow-up classtype update.

Detect duplicate priority instances in a keyword, and use the
highest priority in the rule. Do issue a warning in this case.
---

diff --git a/src/detect-priority.c b/src/detect-priority.c
index ca0ceaec78..748ae884e3 100644
--- a/src/detect-priority.c
+++ b/src/detect-priority.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
@@ -85,9 +85,15 @@ static int DetectPrioritySetup (DetectEngineCtx *de_ctx, Signature *s, const cha
                    "to priority keyword");
         return -1;
     }
-    /* if we have reached here, we have had a valid priority.  Assign it */
-    s->prio = prio;
 
+    if (s->init_data->init_flags & SIG_FLAG_INIT_PRIO_EXPLICT) {
+        SCLogWarning(SC_ERR_CONFLICTING_RULE_KEYWORDS, "duplicate priority "
+                "keyword. Using highest priority in the rule");
+        s->prio = MIN(s->prio, prio);
+    } else {
+        s->prio = prio;
+        s->init_data->init_flags |= SIG_FLAG_INIT_PRIO_EXPLICT;
+    }
     return 0;
 }
 
@@ -97,93 +103,68 @@ static int DetectPrioritySetup (DetectEngineCtx *de_ctx, Signature *s, const cha
 
 static int DetectPriorityTest01(void)
 {
-    int result = 0;
-
     DetectEngineCtx *de_ctx = DetectEngineCtxInit();
-    if (de_ctx == NULL) {
-        goto end;
-    }
+    FAIL_IF_NULL(de_ctx);
 
     de_ctx->sig_list = SigInit(de_ctx, "alert tcp any any -> any any "
                                "(msg:\"Priority test\"; priority:2; sid:1;)");
-    if (de_ctx->sig_list != NULL)
-        result = 1;
+    FAIL_IF_NULL(de_ctx->sig_list);
 
-    DetectEngineCtxFree(de_ctx);
+    FAIL_IF_NOT(de_ctx->sig_list->prio == 2);
 
-end:
-    return result;
+    DetectEngineCtxFree(de_ctx);
+    PASS;
 }
 
 static int DetectPriorityTest02(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);
 
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
+    Signature *sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
                   "(msg:\"Priority test\"; priority:1; sid:1;)");
-    de_ctx->sig_list = last = sig;
-    if (sig == NULL) {
-        result = 0;
-    } else {
-        result = 1;
-        result &= (sig->prio == 1);
-    }
+    FAIL_IF_NULL(sig);
+    FAIL_IF_NOT(sig->prio == 1);
 
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
-                  "(msg:\"Priority test\"; priority:boo; sid:1;)");
-    if (last != NULL)
-        last->next = sig;
-    result &= (sig == NULL);
-
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
-                  "(msg:\"Priority test\"; priority:10boo; sid:1;)");
-    if (last != NULL)
-        last->next = sig;
-    result &= (sig == NULL);
-
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
-                  "(msg:\"Priority test\"; priority:b10oo; sid:1;)");
-    if (last != NULL)
-        last->next = sig;
-    result &= (sig == NULL);
-
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
-                  "(msg:\"Priority test\"; priority:boo10; sid:1;)");
-    if (last != NULL)
-        last->next = sig;
-    result &= (sig == NULL);
-
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
-                  "(msg:\"Priority test\"; priority:-1; sid:1;)");
-    if (last != NULL)
-        last->next = sig;
-    result &= (sig == NULL);
-
-    sig = SigInit(de_ctx, "alert tcp any any -> any any "
-                  "(msg:\"Priority test\"; sid:1;)");
-    if (last != NULL)
-        last->next = sig;
-    if (sig == NULL) {
-        result &= 0;
-    } else {
-        result &= (sig->prio == 3);
-    }
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(msg:\"Priority test\"; priority:boo; sid:2;)");
+    FAIL_IF_NOT_NULL(sig);
 
-    SigCleanSignatures(de_ctx);
-    DetectEngineCtxFree(de_ctx);
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(msg:\"Priority test\"; priority:10boo; sid:3;)");
+    FAIL_IF_NOT_NULL(sig);
 
-end:
-    return result;
-}
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(msg:\"Priority test\"; priority:b10oo; sid:4;)");
+    FAIL_IF_NOT_NULL(sig);
+
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(msg:\"Priority test\"; priority:boo10; sid:5;)");
+    FAIL_IF_NOT_NULL(sig);
+
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(msg:\"Priority test\"; priority:-1; sid:6;)");
+    FAIL_IF_NOT_NULL(sig);
+
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(msg:\"Priority test\"; sid:7;)");
+    FAIL_IF_NULL(sig);
+    FAIL_IF_NOT(sig->prio == 3);
+
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(msg:\"Priority test\"; priority:5; priority:4; sid:8;)");
+    FAIL_IF_NULL(sig);
+    FAIL_IF_NOT(sig->prio == 4);
 
+    sig = DetectEngineAppendSig(de_ctx, "alert tcp any any -> any any "
+                  "(msg:\"Priority test\"; priority:5; priority:4; "
+                  "priority:1; sid:9;)");
+    FAIL_IF_NULL(sig);
+    FAIL_IF_NOT(sig->prio == 1);
 
+    DetectEngineCtxFree(de_ctx);
+    PASS;
+}
 #endif /* UNITTESTS */
 
 /**
@@ -191,12 +172,8 @@ end:
  */
 void SCPriorityRegisterTests(void)
 {
-
 #ifdef UNITTESTS
-
     UtRegisterTest("DetectPriorityTest01", DetectPriorityTest01);
     UtRegisterTest("DetectPriorityTest02", DetectPriorityTest02);
-
 #endif /* UNITTESTS */
-
 }
diff --git a/src/detect.h b/src/detect.h
index 8fcf1e4f56..d556dd1fd9 100644
--- a/src/detect.h
+++ b/src/detect.h
@@ -257,6 +257,7 @@ typedef struct DetectPort_ {
 #define SIG_FLAG_INIT_HAS_TRANSFORM         BIT_U32(5)
 #define SIG_FLAG_INIT_STATE_MATCH           BIT_U32(6)  /**< signature has matches that require stateful inspection */
 #define SIG_FLAG_INIT_NEED_FLUSH            BIT_U32(7)
+#define SIG_FLAG_INIT_PRIO_EXPLICT          BIT_U32(8)  /**< priority is explicitly set by the priority keyword */
 
 /* signature mask flags */
 /** \note: additions should be added to the rule analyzer as well */