From: Shivani Bhardwaj <shivani@oisf.net>
Date: Wed, 21 Feb 2024 06:42:30 +0000 (+0530)
Subject: detect/port: merge port ranges for same signatures
X-Git-Tag: suricata-8.0.0-beta1~1676
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=643ae85b5faa91958fe9ee79987fa20f06849059;p=thirdparty%2Fsuricata.git

detect/port: merge port ranges for same signatures

To avoid getting multiple entries in the final port list and to also
make the next step more efficient by reducing the size of the items to
traverse over.

Ticket 6792
Bug 6414
---

diff --git a/src/detect-engine-build.c b/src/detect-engine-build.c
index bc399f2e50..e812f86122 100644
--- a/src/detect-engine-build.c
+++ b/src/detect-engine-build.c
@@ -1163,9 +1163,7 @@ static inline uint32_t SetUniquePortPoints(
     }
 
     /* Treat right boundary as single point to avoid creating unneeded
-     * ranges later on
-     * TODO merge the extra entry created in the rulegroup
-     * for such points in *CreateGroupedPortList* fn */
+     * ranges later on */
     if (unique_list[p->port2] == 0) {
         size_list++;
     }
diff --git a/src/detect-engine-siggroup.c b/src/detect-engine-siggroup.c
index 52073cf0bf..004659b1b1 100644
--- a/src/detect-engine-siggroup.c
+++ b/src/detect-engine-siggroup.c
@@ -435,6 +435,31 @@ void SigGroupHeadSetSigCnt(SigGroupHead *sgh, uint32_t max_idx)
     return;
 }
 
+/**
+ * \brief Finds if two Signature Group Heads are the same.
+ *
+ * \param sgha First SGH to be compared
+ * \param sghb Secornd SGH to be compared
+ *
+ * \return true if they're a match, false otherwise
+ */
+bool SigGroupHeadEqual(const SigGroupHead *sgha, const SigGroupHead *sghb)
+{
+    if (sgha == NULL || sghb == NULL)
+        return false;
+
+    if (sgha->init->sig_size != sghb->init->sig_size)
+        return false;
+
+    if (sgha->init->max_sig_id != sghb->init->max_sig_id)
+        return false;
+
+    if (SCMemcmp(sgha->init->sig_array, sghb->init->sig_array, sgha->init->sig_size) != 0)
+        return false;
+
+    return true;
+}
+
 void SigGroupHeadSetProtoAndDirection(SigGroupHead *sgh,
                                       uint8_t ipproto, int dir)
 {
diff --git a/src/detect-engine-siggroup.h b/src/detect-engine-siggroup.h
index 45887edbfb..f0638293a6 100644
--- a/src/detect-engine-siggroup.h
+++ b/src/detect-engine-siggroup.h
@@ -42,6 +42,7 @@ int SigGroupHeadHashRemove(DetectEngineCtx *, SigGroupHead *);
 
 void SigGroupHeadInitDataFree(SigGroupHeadInitData *sghid);
 void SigGroupHeadSetSigCnt(SigGroupHead *sgh, uint32_t max_idx);
+bool SigGroupHeadEqual(const SigGroupHead *, const SigGroupHead *);
 void SigGroupHeadSetProtoAndDirection(SigGroupHead *sgh,
                                       uint8_t ipproto, int dir);
 int SigGroupHeadBuildMatchArray(DetectEngineCtx *de_ctx, SigGroupHead *sgh, uint32_t max_idx);
diff --git a/src/util-port-interval-tree.c b/src/util-port-interval-tree.c
index 53f8ef5508..d4c770585a 100644
--- a/src/util-port-interval-tree.c
+++ b/src/util-port-interval-tree.c
@@ -127,6 +127,35 @@ int SCPortIntervalInsert(DetectEngineCtx *de_ctx, SCPortIntervalTree *it, const
     return SC_OK;
 }
 
+/**
+ * \brief Function to remove multiple sig entries corresponding to the same
+ *        signature group and merge them into one.
+ *
+ * \param de_ctx Detection Engine Context
+ * \param list Pointer to the list to be modified
+ */
+static void SCPortIntervalSanitizeList(DetectEngineCtx *de_ctx, DetectPort **list)
+{
+    DetectPort *cur = (*list)->last;
+    if (cur == NULL)
+        return;
+
+    DetectPort *prev = (*list)->last->prev;
+    if (prev == NULL)
+        return;
+
+    /* rulegroup IDs are assigned much later so, compare SGHs */
+    if (SigGroupHeadEqual(prev->sh, cur->sh)) {
+        if (prev->port2 == (cur->port - 1)) {
+            /* Merge the port objects */
+            prev->port2 = cur->port2;
+            (*list)->last = prev;
+            (*list)->last->next = NULL;
+            DetectPortFree(de_ctx, cur);
+        }
+    }
+}
+
 /**
  * \brief Function to check if a port range overlaps with a given set of ports
  *
@@ -226,6 +255,7 @@ static void SCPortIntervalFindOverlaps(DetectEngineCtx *de_ctx, const uint16_t p
                            ((*list)->last->port2 != new_port->port)) {
                     DEBUG_VALIDATE_BUG_ON(new_port->port < (*list)->last->port);
                     (*list)->last->next = new_port;
+                    new_port->prev = (*list)->last;
                     (*list)->last = new_port;
                 } else {
                     SCLogDebug("Port already exists in the list");
@@ -260,6 +290,8 @@ static void SCPortIntervalFindOverlaps(DetectEngineCtx *de_ctx, const uint16_t p
         BUG_ON(popped == NULL);
         current = IRB_RIGHT(popped, irb);
     }
+    if (new_port != NULL)
+        SCPortIntervalSanitizeList(de_ctx, list);
     if (stack != NULL)
         SCFree(stack);
     return;