]> git.ipfire.org Git - thirdparty/snort3.git/commitdiff
Merge pull request #1728 in SNORT/snort3 from ~MASHASAN/snort3:filter_rna_events...
authorMike Stepanek (mstepane) <mstepane@cisco.com>
Mon, 9 Sep 2019 16:50:58 +0000 (12:50 -0400)
committerMike Stepanek (mstepane) <mstepane@cisco.com>
Mon, 9 Sep 2019 16:50:58 +0000 (12:50 -0400)
Squashed commit of the following:

commit 15a663184d9fc02316049b28f071efa7ee986695
Author: Masud Hasan <mashasan@cisco.com>
Date:   Tue Aug 27 12:30:24 2019 -0400

    rna: Support for filtering rna events by host ip

src/helpers/CMakeLists.txt
src/helpers/discovery_filter.cc [new file with mode: 0644]
src/helpers/discovery_filter.h [new file with mode: 0644]
src/network_inspectors/rna/rna_inspector.cc
src/network_inspectors/rna/rna_pnd.cc
src/network_inspectors/rna/rna_pnd.h
src/sfip/sf_ipvar.cc

index f4741eaa4e112db2cc461bf918f2b16eee161e85..7ffdc32797a9eed39bff543cdcaf98944cb28397 100644 (file)
@@ -10,6 +10,8 @@ add_library (helpers OBJECT
     chunk.h
     directory.cc
     directory.h
+    discovery_filter.cc
+    discovery_filter.h
     flag_context.h
     markup.cc
     markup.h
diff --git a/src/helpers/discovery_filter.cc b/src/helpers/discovery_filter.cc
new file mode 100644 (file)
index 0000000..8e5f70a
--- /dev/null
@@ -0,0 +1,239 @@
+//--------------------------------------------------------------------------
+// Copyright (C) 2019-2019 Cisco and/or its affiliates. All rights reserved.
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License Version 2 as published
+// by the Free Software Foundation.  You may not use, modify or distribute
+// this program under any other version of the GNU General Public License.
+//
+// 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 along
+// with this program; if not, write to the Free Software Foundation, Inc.,
+// 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+//--------------------------------------------------------------------------
+
+// discovery_filter.cc author Masud Hasan <mashasan@cisco.com>
+
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+
+#include "discovery_filter.h"
+
+#include <fstream>
+#include <sstream>
+#include <string>
+
+#include "log/messages.h"
+
+#ifdef UNIT_TEST
+#include "catch/snort_catch.h"
+#endif
+
+using namespace snort;
+using namespace std;
+
+#define DF_APP        "app"
+#define DF_HOST       "host"
+#define DF_USER       "user"
+
+#define DF_APP_CHECKED    0x01
+#define DF_APP_MONITORED  0x02
+#define DF_HOST_CHECKED   0x04
+#define DF_HOST_MONITORED 0x08
+#define DF_USER_CHECKED   0x10
+#define DF_USER_MONITORED 0x20
+
+DiscoveryFilter::DiscoveryFilter(const string& conf_path)
+{
+    if ( conf_path.empty() )
+        return;
+
+    ifstream in_stream(conf_path);
+    if ( !in_stream )
+        return;
+
+    uint32_t line_num = 0;
+    string line, config_type, config_key, config_value;
+
+    while ( in_stream )
+    {
+        line = "";
+        getline(in_stream, line);
+        ++line_num;
+        if ( line.empty() or line.front() == '#' )
+            continue;
+
+        istringstream line_stream(line);
+        config_type = config_key = config_value = "";
+        line_stream >> config_type >> config_key >> config_value;
+
+        if ( config_type == "config" )
+        {
+            if ( config_key.empty() or config_value.empty() )
+            {
+                WarningMessage("Discovery Filter: Empty configuration items at line %u from %s\n",
+                    line_num, conf_path.c_str());
+                continue;
+            }
+
+            // host or user discovery will also enable application discovery
+            if ( config_key == "AnalyzeApplication" )
+            {
+                add_ip(DF_APP, config_value);
+            }
+            else if ( config_key == "AnalyzeHost" )
+            {
+                add_ip(DF_APP, config_value);
+                add_ip(DF_HOST, config_value);
+            }
+            else if ( config_key == "AnalyzeUser" )
+            {
+                add_ip(DF_APP, config_value);
+                add_ip(DF_USER, config_value);
+            }
+            else if ( config_key == "AnalyzeHostUser" or config_key == "Analyze" )
+            {
+                add_ip(DF_APP, config_value);
+                add_ip(DF_HOST, config_value);
+                add_ip(DF_USER, config_value);
+            }
+        }
+    }
+
+    in_stream.close();
+}
+
+DiscoveryFilter::~DiscoveryFilter()
+{
+    sfvt_free_table(vartable);
+}
+
+bool DiscoveryFilter::is_app_monitored(const Packet* p, uint8_t* flag)
+{
+    if ( flag == nullptr )
+        return is_monitored(p, DF_APP);
+    return is_monitored(p, DF_APP, *flag, DF_APP_CHECKED, DF_APP_MONITORED);
+}
+
+bool DiscoveryFilter::is_host_monitored(const Packet* p, uint8_t* flag)
+{
+    if ( flag == nullptr )
+        return is_monitored(p, DF_HOST);
+    return is_monitored(p, DF_HOST, *flag, DF_HOST_CHECKED, DF_HOST_MONITORED);
+}
+
+bool DiscoveryFilter::is_user_monitored(const Packet* p, uint8_t* flag)
+{
+    if ( flag == nullptr )
+        return is_monitored(p, DF_USER);
+    return is_monitored(p, DF_USER, *flag, DF_USER_CHECKED, DF_USER_MONITORED);
+}
+
+bool DiscoveryFilter::is_monitored(const Packet* p, const char* type, uint8_t& flag,
+    uint8_t checked, uint8_t monitored)
+{
+    if ( flag & checked )
+        return flag & monitored;
+
+    flag |= checked;
+
+    if ( is_monitored(p, type) )
+    {
+        flag |= monitored;
+        return true;
+    }
+
+    flag &= ~monitored;
+    return false;
+}
+
+bool DiscoveryFilter::is_monitored(const Packet* p, const char* type)
+{
+    if ( !vartable )
+        return true; // when not configured, 'any' ip/port/zone are monitored by default
+
+    // Do port-based filtering first, which is independent of application/host/user type.
+    // Keep an unordered map of <port, exclusion> where exclusion object holds pointers
+    // to ip-list from vartable for each direction (src/dst) and protocol (tcp/udp).
+
+    // To do zone-based filtering, keep an unordered map of <zone, ip-list pointer> where
+    // the pointer refers to a list from vartable. The list itself should be created
+    // during parsing by appending zone id to the name of ip-list. Absence of the list
+    // for a particular zone means lookup the 'any' (-1) zone list.
+
+    if ( !varip or strcmp(varip->name, type) )
+    {
+        varip = sfvt_lookup_var(vartable, type);
+        if ( !varip )
+            return false;
+    }
+
+    return sfvar_ip_in(varip, p->ptrs.ip_api.get_src()); // check source ip only
+}
+
+void DiscoveryFilter::add_ip(const char* name, string ip)
+{
+    if ( !vartable )
+        vartable = sfvt_alloc_table();
+    else if ( !varip or strcmp(varip->name, name) )
+        varip = sfvt_lookup_var(vartable, name);
+
+    if ( varip )
+        sfvt_add_to_var(vartable, varip, ip.c_str());
+    else
+    {
+        ip = " " + ip;
+        ip = name + ip;
+        sfvt_add_str(vartable, ip.c_str(), &varip);
+    }
+}
+
+#ifdef UNIT_TEST
+TEST_CASE("Discovery Filter", "[is_monitored]")
+{
+    string conf("test.txt");
+    ofstream out_stream(conf.c_str());
+    out_stream << "config Error\n"; // invalid
+    out_stream << "config AnalyzeUser ::/0 3\n"; // any ipv6, zone 3
+    out_stream << "config AnalyzeApplication 0.0.0.0/0 -1\n"; // any ipv4, any zone
+    out_stream.close();
+
+    Packet p;
+    SfIp ip;
+    ip.set("1.1.1.1");
+    p.ptrs.ip_api.set(ip, ip);
+    DiscoveryFilter df(conf);
+
+    // Without flag
+    CHECK(df.is_app_monitored(&p, nullptr) == true);
+    CHECK(df.is_host_monitored(&p, nullptr) == false);
+    CHECK(df.is_user_monitored(&p, nullptr) == false);
+
+    // With flag
+    uint8_t flag = 0;
+    CHECK((flag & DF_APP_CHECKED) != DF_APP_CHECKED);
+    CHECK((flag & DF_APP_MONITORED) != DF_APP_MONITORED);
+    CHECK(df.is_app_monitored(&p, &flag) == true); // first attempt
+    CHECK((flag & DF_APP_CHECKED) == DF_APP_CHECKED);
+    CHECK((flag & DF_APP_MONITORED) == DF_APP_MONITORED);
+    CHECK(df.is_app_monitored(&p, &flag) == true); // second attempt
+    CHECK((flag & DF_APP_CHECKED) == DF_APP_CHECKED);
+    CHECK((flag & DF_APP_MONITORED) == DF_APP_MONITORED);
+
+    CHECK((flag & DF_USER_CHECKED) != DF_USER_CHECKED);
+    CHECK((flag & DF_USER_MONITORED) != DF_USER_MONITORED);
+    CHECK(df.is_user_monitored(&p, &flag) == false); // first attempt
+    CHECK((flag & DF_USER_CHECKED) == DF_USER_CHECKED);
+    CHECK((flag & DF_USER_MONITORED) != DF_USER_MONITORED);
+    CHECK(df.is_user_monitored(&p, &flag) == false); // second attempt
+    CHECK((flag & DF_USER_CHECKED) == DF_USER_CHECKED);
+    CHECK((flag & DF_USER_MONITORED) != DF_USER_MONITORED);
+
+    remove("test.txt");
+}
+#endif
diff --git a/src/helpers/discovery_filter.h b/src/helpers/discovery_filter.h
new file mode 100644 (file)
index 0000000..ec32c2d
--- /dev/null
@@ -0,0 +1,50 @@
+//--------------------------------------------------------------------------
+// Copyright (C) 2019-2019 Cisco and/or its affiliates. All rights reserved.
+//
+// This program is free software; you can redistribute it and/or modify it
+// under the terms of the GNU General Public License Version 2 as published
+// by the Free Software Foundation.  You may not use, modify or distribute
+// this program under any other version of the GNU General Public License.
+//
+// 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 along
+// with this program; if not, write to the Free Software Foundation, Inc.,
+// 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
+//--------------------------------------------------------------------------
+
+// discovery_filter.h author Masud Hasan <mashasan@cisco.com>
+
+#ifndef DISCOVERY_FILTER_H
+#define DISCOVERY_FILTER_H
+
+#include "protocols/packet.h"
+#include "sfip/sf_ipvar.h"
+#include "sfip/sf_vartable.h"
+
+// Holds configurations to filter traffic discovery based network address, port, and zone
+class DiscoveryFilter
+{
+public:
+    DiscoveryFilter(const std::string& conf_path);
+    ~DiscoveryFilter();
+
+    // If flag is provided (preferable), results are stored in flag to avoid future lookups
+    bool is_app_monitored(const snort::Packet* p, uint8_t* flag = nullptr);
+    bool is_host_monitored(const snort::Packet* p, uint8_t* flag = nullptr);
+    bool is_user_monitored(const snort::Packet* p, uint8_t* flag = nullptr);
+
+private:
+    bool is_monitored(const snort::Packet* p, const char* type, uint8_t& flag,
+        uint8_t checked, uint8_t monitored);
+    bool is_monitored(const snort::Packet* p, const char* type);
+    void add_ip(const char* name, std::string ip);
+
+    vartable_t* vartable = nullptr;
+    sfip_var_t* varip = nullptr;
+};
+
+#endif
index 3dcdd84af6235b9dd6bf2b8fceb4399866b5b1bb..bbfc697805740b8ccf07aecd79eb5ac9b90b9014 100644 (file)
@@ -53,7 +53,10 @@ RnaInspector::RnaInspector(RnaModule* mod)
 {
     mod_conf = mod->get_config();
     load_rna_conf();
-    pnd = new RnaPnd(mod_conf? mod_conf->enable_logger : false);
+    if ( mod_conf )
+        pnd = new RnaPnd(mod_conf->enable_logger, mod_conf->rna_conf_path);
+    else
+        pnd = new RnaPnd(false, "");
 }
 
 RnaInspector::~RnaInspector()
index 834be46298db5c5ad92ac3ea3cbf6af84c943064..f4130bf8492c3ea4a9b36e74637647756a575358 100644 (file)
@@ -79,26 +79,26 @@ static inline bool is_eligible_udp(const Packet* p)
 
 void RnaPnd::analyze_flow_icmp(const Packet* p)
 {
-    if ( is_eligible_ip(p) )
+    if ( is_eligible_ip(p) and filter.is_host_monitored(p) )
         discover_network_icmp(p);
 }
 
 void RnaPnd::analyze_flow_ip(const Packet* p)
 {
-    if ( is_eligible_ip(p) )
+    if ( is_eligible_ip(p) and filter.is_host_monitored(p) )
         discover_network_ip(p);
 }
 
 void RnaPnd::analyze_flow_non_ip(const Packet* p)
 {
-    if ( is_eligible_packet(p) )
+    if ( is_eligible_packet(p) and filter.is_host_monitored(p) )
         discover_network_non_ip(p);
 }
 
 void RnaPnd::analyze_flow_tcp(const Packet* p, TcpPacketType type)
 {
     // If and when flow stores rna state, process the flow data here before global cache access
-    if ( is_eligible_tcp(p) )
+    if ( is_eligible_tcp(p) and filter.is_host_monitored(p) )
         discover_network_tcp(p);
 
     UNUSED(type);
@@ -106,7 +106,7 @@ void RnaPnd::analyze_flow_tcp(const Packet* p, TcpPacketType type)
 
 void RnaPnd::analyze_flow_udp(const Packet* p)
 {
-    if ( is_eligible_udp(p) )
+    if ( is_eligible_udp(p) and filter.is_host_monitored(p) )
         discover_network_udp(p);
 }
 
@@ -171,7 +171,7 @@ TEST_CASE("RNA pnd", "[non-ip]")
 
         ip::IP4Hdr h4;
         p.ptrs.ip_api.set(&h4);
-        RnaPnd pnd(false);
+        RnaPnd pnd(false, "");
         pnd.analyze_flow_non_ip(&p);
         CHECK(is_eligible_packet(&p) == true);
     }
index e7be2d71481d64d7f19ae7fd9d9567897c1562d1..e798554a3534c5c811e07c5de78dc67fb5485676 100644 (file)
@@ -20,6 +20,8 @@
 #ifndef RNA_PND_H
 #define RNA_PND_H
 
+#include "helpers/discovery_filter.h"
+
 #include "rna_logger.h"
 
 namespace snort
@@ -35,7 +37,8 @@ enum class TcpPacketType
 class RnaPnd
 {
 public:
-    RnaPnd(const bool en) : logger(RnaLogger(en)) { }
+    RnaPnd(const bool en, const std::string& conf)
+        : logger(RnaLogger(en)), filter(DiscoveryFilter(conf)) { }
 
     void analyze_flow_icmp(const snort::Packet* p);
     void analyze_flow_ip(const snort::Packet* p);
@@ -53,6 +56,7 @@ private:
     void discover_network(const snort::Packet* p, u_int8_t ttl);
 
     RnaLogger logger;
+    DiscoveryFilter filter;
 };
 
 #endif
index 9f332fe08cac098675c77b3069eceb6e4415ef90..eabc982cfe2e81bc5f1636d7e6988853975da0e9 100644 (file)
@@ -210,6 +210,64 @@ static inline sfip_node_t* _sfvar_deep_copy_list(const sfip_node_t* idx)
     return ret;
 }
 
+/* Returns true if the node is contained by a network from the sorted list; deletes/consumes
+ * the node. Returns false otherwise; does not delete the node but removes any network from the
+ * list that is contained by the node so that the caller can insert the node without overlap. */
+static inline bool list_contains_node(sfip_node_t*& head, sfip_node_t*& tail, uint32_t& count,
+    sfip_node_t*& node)
+{
+    sfip_node_t* cur = nullptr;
+    sfip_node_t* prev = nullptr;
+
+    if ( node->ip->get_family() == AF_INET )
+    {
+        for ( cur = head; cur; prev = cur, cur = cur->next )
+            if ( !cur->ip->is_set() or cur->ip->get_family() != AF_INET )
+                continue;
+            else if ( node->ip->fast_cont4(*cur->ip->get_addr()) )
+                break;
+            else if ( cur->ip->fast_cont4(*node->ip->get_addr()) )
+            {
+                sfip_node_free(node);
+                return true;
+            }
+    }
+    else if ( node->ip->get_family() == AF_INET6 )
+    {
+        for ( cur = head; cur; prev = cur, cur = cur->next )
+            if ( !cur->ip->is_set() or cur->ip->get_family() != AF_INET6 )
+                continue;
+            else if ( node->ip->fast_cont6(*cur->ip->get_addr()) )
+                break;
+            else if ( cur->ip->fast_cont6(*node->ip->get_addr()) )
+            {
+                sfip_node_free(node);
+                return true;
+            }
+    }
+
+    if ( cur ) // this network is contained by the node
+    {
+        if ( prev )
+        {
+            prev->next = cur->next;
+            if ( !prev->next )
+                tail = prev;
+        }
+        else if ( cur->next )
+            head = cur->next;
+        else // only one node
+        {
+            head = tail = node;
+            sfip_node_free(cur);
+            return true;
+        }
+        sfip_node_free(cur); // overlaps removed so that caller can insert the node
+        --count;
+    }
+    return false;
+}
+
 /* Deep copy. Returns identical, new, linked list of sfipnodes. */
 sfip_var_t* sfvar_deep_copy(const sfip_var_t* var)
 {
@@ -233,7 +291,7 @@ static sfip_node_t* merge_lists(sfip_node_t* list1, sfip_node_t* list2, uint16_t
     uint16_t list2_len, uint32_t& merge_len)
 {
     sfip_node_t* listHead = nullptr, * merge_list = nullptr, * tmp = nullptr, * node = nullptr;
-    uint16_t num_nodes = 0;
+    uint32_t num_nodes = 0;
 
     if (!list1 && !list2)
     {
@@ -269,6 +327,25 @@ static sfip_node_t* merge_lists(sfip_node_t* list1, sfip_node_t* list2, uint16_t
     /*Iterate till one of the list is NULL. Append each node to merge_list*/
     while (list1 && list2)
     {
+        if ( num_nodes )
+        {
+            tmp = list1->next;
+            if ( list_contains_node(listHead, merge_list, num_nodes, list1) )
+            {
+                list1 = tmp;
+                list1_len--;
+                continue;
+            }
+
+            tmp = list2->next;
+            if ( list_contains_node(listHead, merge_list, num_nodes, list2) )
+            {
+                list2 = tmp;
+                list2_len--;
+                continue;
+            }
+        }
+
         SfIpRet ret = list1->ip->compare(*(list2->ip));
         if (ret == SFIP_LESSER)
         {
@@ -294,6 +371,8 @@ static sfip_node_t* merge_lists(sfip_node_t* list1, sfip_node_t* list2, uint16_t
             list1_len--;
             list2_len--;
         }
+        else // SFIP_FAILURE
+            break;
 
         if (!merge_list)
         {
@@ -305,20 +384,34 @@ static sfip_node_t* merge_lists(sfip_node_t* list1, sfip_node_t* list2, uint16_t
             merge_list->next = node;
             merge_list = merge_list->next;
         }
+        node->next = nullptr;
         num_nodes++;
     }
 
     /*list2 is NULL. Append list1*/
-    if (list1)
+    while ( list1 )
     {
-        merge_list->next = list1;
-        num_nodes += list1_len;
+        tmp = list1->next;
+        if ( !list_contains_node(listHead, merge_list, num_nodes, list1) and merge_list )
+        {
+            merge_list->next = list1;
+            num_nodes += list1_len;
+            break;
+        }
+        list1 = tmp;
     }
+
     /*list1 is NULL. Append list2*/
-    if (list2)
+    while ( list2 )
     {
-        merge_list->next = list2;
-        num_nodes += list2_len;
+        tmp = list2->next;
+        if ( !list_contains_node(listHead, merge_list, num_nodes, list2) and merge_list )
+        {
+            merge_list->next = list2;
+            num_nodes += list2_len;
+            break;
+        }
+        list2 = tmp;
     }
 
     merge_len = num_nodes;
@@ -388,44 +481,36 @@ static SfIpRet sfvar_add_node(sfip_var_t* var, sfip_node_t* node, int negated)
         return SFIP_SUCCESS;
     }
 
-    /* "Anys" should always be inserted first
-       Otherwise, check if this IP is less than the head's IP */
+    /* "Anys" should always be inserted first */
     if (node->flags & SFIP_ANY)
     {
-        sfip_node_t* tmp;
         /*Free the list when adding any*/
         while (*head)
         {
-            tmp = (*head)->next;
+            swp = (*head)->next;
             sfip_node_free(*head);
-            *head = tmp;
+            *head = swp;
         }
         *head = node;
         *count = 1;
         return SFIP_SUCCESS;
     }
-    else
+
+    if ( list_contains_node(*head, swp, *count, node) )
+        return SFIP_SUCCESS;
+
+    /* To insert the new node, first check if this IP is less than the head's IP */
+    SfIpRet node_cmp_ret = node->ip->compare(*((*head)->ip));
+    if (node_cmp_ret == SFIP_EQUAL)
     {
-        SfIpRet node_cmp_ret = node->ip->compare(*((*head)->ip));
-        if (node_cmp_ret == SFIP_EQUAL)
-        {
-            sfip_node_free(node);
-            return SFIP_SUCCESS;
-        }
-        else if (node_cmp_ret == SFIP_LESSER)
-        {
-            node->next = *head;
-            *head = node;
-            ++*count;
-            return SFIP_SUCCESS;
-        }
+        sfip_node_free(node);
+        return SFIP_SUCCESS;
     }
-
-    if ((node->flags & SFIP_ANY) ||
-        node->ip->get_addr()->compare(*(*head)->ip->get_addr()) == SFIP_LESSER)
+    else if (node_cmp_ret == SFIP_LESSER)
     {
         node->next = *head;
         *head = node;
+        ++*count;
         return SFIP_SUCCESS;
     }
 
@@ -442,7 +527,7 @@ static SfIpRet sfvar_add_node(sfip_var_t* var, sfip_node_t* node, int negated)
     /* Insertion sort */
     for (p = *head; p->next; p=p->next)
     {
-        SfIpRet node_cmp_ret = node->ip->compare(*(p->next->ip));
+        node_cmp_ret = node->ip->compare(*(p->next->ip));
         if (node_cmp_ret == SFIP_EQUAL)
         {
             sfip_node_free(node);
@@ -1146,18 +1231,23 @@ TEST_CASE("SfIpVarListMerge", "[SfIpVar]")
         CHECK(!strcmp("192.168.2.1,192.168.6.1,255.255.241.0", sfipvar_test_buff));
 
         // add CIDR addresses
-        CHECK(sfvt_add_str(table, "my_cidr [ 192.168.0.0/16, f0:e0:d0:c0::8/64, 10.10.1.8/19] ",
-            &var2) == SFIP_SUCCESS);
+        CHECK(sfvt_add_str(table, "my_cidr [ 192.168.0.0/16, f0:e0:d0:c0::8/64, 10.10.1.8/19,"
+            " f0:e0:d1:c1::1/32]", &var2) == SFIP_SUCCESS);
         print_var_list(var2->head);
-        CHECK(!strcmp("10.10.0.0,192.168.0.0,00f0:00e0:00d0:00c0:0000:0000:0000:0000",
+        CHECK(!strcmp("10.10.0.0,192.168.0.0,00f0:00e0:0000:0000:0000:0000:0000:0000",
             sfipvar_test_buff));
 
         // merge the list
+        CHECK(SFIP_SUCCESS == sfvar_add(var2, var1));
+        print_var_list(var2->head);
+        CHECK(!strcmp("10.10.0.0,192.168.0.0,255.255.241.0,255.255.248.0,"
+            "00f0:00e0:0000:0000:0000:0000:0000:0000", sfipvar_test_buff));
+
+        // merge the list again; should yield the same result
         CHECK(SFIP_SUCCESS == sfvar_add(var1, var2));
         print_var_list(var1->head);
-        CHECK(!strcmp("10.10.0.0,192.168.0.0,192.168.0.1,192.168.0.2,192.168.2.1,192.168.5.0," \
-            "255.255.241.0,255.255.248.0,00f0:00e0:00d0:00c0:0000:0000:0000:0000",
-            sfipvar_test_buff));
+        CHECK(!strcmp("10.10.0.0,192.168.0.0,255.255.241.0,255.255.248.0,"
+            "00f0:00e0:0000:0000:0000:0000:0000:0000", sfipvar_test_buff));
 
         sfvt_free_table(table);
     }
@@ -1172,8 +1262,7 @@ TEST_CASE("SfIpVarListMerge", "[SfIpVar]")
             == SFIP_SUCCESS);
         CHECK(SFIP_SUCCESS == sfvar_add(var1, var2));
         print_var_list(var1->head, true);
-        CHECK(!strcmp("192.168.240.0/116,192.168.248.0/117,192.168.255.2/127,192.168.255.2/128",
-            sfipvar_test_buff));
+        CHECK(!strcmp("192.168.240.0/116", sfipvar_test_buff));
 
         //[ 1.2.3.8, 1.2.3.9, 1.2.3.10 ] with [ 1.2.3.255/25 ]
         CHECK(sfvt_add_str(table, "ip21 [ 1.2.3.8, 1.2.3.9, 1.2.3.10]", &var1) == SFIP_SUCCESS);
@@ -1190,6 +1279,67 @@ TEST_CASE("SfIpVarListMerge", "[SfIpVar]")
         CHECK(!strcmp("10.9.8.7/128", sfipvar_test_buff));
         print_var_list(var1->neg_head, true);
         CHECK(!strcmp("!10.9.8.7/128", sfipvar_test_buff));
+
+        // Duplicate ip in lists of a single ip
+        CHECK(sfvt_add_str(table, "once [ 1.1.1.1/16 ]", &var1)
+            == SFIP_SUCCESS);
+        CHECK(sfvt_add_str(table, "again [ 1.1.1.1/16 ]", &var2)
+            == SFIP_SUCCESS);
+        CHECK(SFIP_SUCCESS == sfvar_add(var1, var2));
+        print_var_list(var1->head);
+        CHECK(!strcmp("1.1.0.0", sfipvar_test_buff));
+
+        // Subset of ip in lists of a single ip
+        CHECK(sfvt_add_str(table, "single [ 1.1.1.1 ]", &var1)
+            == SFIP_SUCCESS);
+        CHECK(sfvt_add_str(table, "similar [ 1.1.1.1/16 ]", &var2)
+            == SFIP_SUCCESS);
+        CHECK(SFIP_SUCCESS == sfvar_add(var1, var2));
+        print_var_list(var1->head);
+        CHECK(!strcmp("1.1.0.0", sfipvar_test_buff));
+        sfvt_free_table(table);
+    }
+
+    SECTION("merge contained IPs and negated-IPs")
+    {
+        table = sfvt_alloc_table();
+
+        CHECK(sfvt_add_str(table, "foo 1.2.3.4, cafe:feed:beef::0/48", &var1) == SFIP_SUCCESS);
+        CHECK(sfvt_add_to_var(table, var1, "1.2.3.5/24, cafe:feed::0/16") == SFIP_SUCCESS);
+        CHECK(sfvt_add_to_var(table, var1, "!9.8.7.6, !dead:beef:abcd::5") == SFIP_SUCCESS);
+        CHECK(sfvt_add_to_var(table, var1, "!9.8.5.4/8, cafe:feed:abcd::0") == SFIP_SUCCESS);
+        CHECK(sfvt_add_to_var(table, var1, "!9.8.3.2, 1.2.6.7/16") == SFIP_SUCCESS);
+        CHECK(sfvt_add_to_var(table, var1, "1.2.3.4/32, cafe:feed::0") == SFIP_SUCCESS);
+        CHECK(sfvt_add_to_var(table, var1, "!dead:beef:abcd::0/32") == SFIP_SUCCESS);
+
+        /* Check merged IP lists */
+        print_var_list(var1->head);
+        CHECK(!strcmp("1.2.0.0,cafe:0000:0000:0000:0000:0000:0000:0000", sfipvar_test_buff));
+        print_var_list(var1->neg_head);
+        CHECK(!strcmp("!9.0.0.0,!dead:beef:0000:0000:0000:0000:0000:0000", sfipvar_test_buff));
+
+        /* Check lookups */
+        snort::SfIp* ip = (snort::SfIp *)snort_alloc(sizeof(snort::SfIp));
+        CHECK(ip->set("9.8.3.2") == SFIP_SUCCESS);
+        CHECK((sfvar_ip_in(var1, ip) == false));
+        snort_free(ip);
+
+        ip = (snort::SfIp *)snort_alloc(sizeof(snort::SfIp));
+        uint16_t srcBits;
+        CHECK(ip->set("1.2.3.4/24", &srcBits) == SFIP_SUCCESS);
+        CHECK((sfvar_ip_in(var1, ip) == true));
+        snort_free(ip);
+
+        ip = (snort::SfIp *)snort_alloc(sizeof(snort::SfIp));
+        CHECK(ip->set("dead:beef::0") == SFIP_SUCCESS);
+        CHECK((sfvar_ip_in(var1, ip) == false));
+        snort_free(ip);
+
+        ip = (snort::SfIp *)snort_alloc(sizeof(snort::SfIp));
+        CHECK(ip->set("cafe:abcd::9999") == SFIP_SUCCESS);
+        CHECK((sfvar_ip_in(var1, ip) == true));
+        snort_free(ip);
+
         sfvt_free_table(table);
     }
 }