Merge pull request #1213 in SNORT/snort3 from rep_zone2 to master

Squashed commit of the following:

commit 86acc8a1a6e79cac222ca191ea6cb8fc4b796248
Author: huica <huica@cisco.com>
Date:   Tue May 1 12:45:31 2018 -0400

    repuation: remove the limit for zone id

diff --git a/src/network_inspectors/reputation/reputation_config.h b/src/network_inspectors/reputation/reputation_config.h
index 228cda63e0ce31d31c3b82262c106e138d94771e..afae40e7f345a4e9ab0cbbc670086707200e5760 100644 (file)
--- a/src/network_inspectors/reputation/reputation_config.h
+++ b/src/network_inspectors/reputation/reputation_config.h
@@ -25,6 +25,7 @@
 #include "sfrt/sfrt_flat.h"
 
 #include <vector>
+#include <set>
 #include <string>
 
 #define NUM_INDEX_PER_ENTRY 4
@@ -54,8 +55,7 @@ enum IPdecision
     DECISION_MAX
 };
 
-#define MAX_NUM_ZONES             1052  // FIXIT-L hard limit due to hardware platform
-#define MAX_MANIFEST_LINE_LENGTH  (8*MAX_NUM_ZONES)
+#define MAX_NUM_ZONES             UINT32_MAX
 #define MAX_LIST_ID               UINT32_MAX
 
 struct ListFile
@@ -63,20 +63,14 @@ struct ListFile
     std::string file_name;
     int file_type;
     uint32_t list_id;
-    bool zones[MAX_NUM_ZONES];
-};
-
-typedef std::vector<ListFile*> ListFiles;
-
-struct ListInfo
-{
+    bool all_zones_enabled = false;
+    std::set<unsigned int> zones;
     uint8_t list_index;
     uint8_t list_type;
-    uint32_t list_id;
-    bool zones[MAX_NUM_ZONES];
-    char padding[2 + MAX_NUM_ZONES - MAX_NUM_ZONES/4*4];
 };
 
+typedef std::vector<ListFile*> ListFiles;
+
 struct ReputationConfig
 {
     uint32_t memcap = 500;

diff --git a/src/network_inspectors/reputation/reputation_inspect.cc b/src/network_inspectors/reputation/reputation_inspect.cc
index f84771c9ea72a1175fd9bca223fec6cf09fc4b31..3e0dac9ca61aca74f2580d1a9bb91f9af8b0f021 100644 (file)
--- a/src/network_inspectors/reputation/reputation_inspect.cc
+++ b/src/network_inspectors/reputation/reputation_inspect.cc
@@ -175,12 +175,10 @@ static inline IPdecision get_reputation(ReputationConfig* config, IPrepInfo* rep
     uint32_t* listid, uint32_t ingress_zone, uint32_t egress_zone)
 {
     IPdecision decision = DECISION_NULL;
-    uint8_t* base;
-    ListInfo* list_info;
 
     /*Walk through the IPrepInfo lists*/
-    base = (uint8_t*)config->ip_list;
-    list_info =  (ListInfo*)(&base[config->ip_list->list_info]);
+    uint8_t* base = (uint8_t*)config->ip_list;
+    ListFiles& list_info =  config->list_files;
 
     while (rep_info)
     {
@@ -191,19 +189,21 @@ static inline IPdecision get_reputation(ReputationConfig* config, IPrepInfo* rep
             if (!list_index)
                 break;
             list_index--;
-            if (list_info[list_index].zones[ingress_zone] || list_info[list_index].zones[egress_zone])
+            if (list_info[list_index]->all_zones_enabled ||
+                list_info[list_index]->zones.count(ingress_zone) ||
+                list_info[list_index]->zones.count(egress_zone))
             {
-                if (WHITELISTED_UNBLACK == (IPdecision)list_info[list_index].list_type)
+                if (WHITELISTED_UNBLACK == (IPdecision)list_info[list_index]->list_type)
                     return DECISION_NULL;
-                if (config->priority == (IPdecision)list_info[list_index].list_type )
+                if (config->priority == (IPdecision)list_info[list_index]->list_type )
                 {
-                    *listid = list_info[list_index].list_id;
-                    return  ((IPdecision)list_info[list_index].list_type);
+                    *listid = list_info[list_index]->list_id;
+                    return  ((IPdecision)list_info[list_index]->list_type);
                 }
-                else if ( decision < list_info[list_index].list_type)
+                else if ( decision < list_info[list_index]->list_type)
                 {
-                    decision = (IPdecision)list_info[list_index].list_type;
-                    *listid = list_info[list_index].list_id;
+                    decision = (IPdecision)list_info[list_index]->list_type;
+                    *listid = list_info[list_index]->list_id;
                 }
             }
         }
@@ -261,12 +261,6 @@ static IPdecision reputation_decision(ReputationConfig* config, Packet* p)
             egress_zone = ingress_zone;
         else
             egress_zone = p->pkth->egress_group;
-
-        /*Make sure zone ids are in the support range*/
-        if (ingress_zone >= MAX_NUM_ZONES)
-            ingress_zone = 0;
-        if (egress_zone >= MAX_NUM_ZONES)
-            egress_zone = 0;
     }
 
     ip::IpApi tmp_api = p->ptrs.ip_api;

diff --git a/src/network_inspectors/reputation/reputation_parse.cc b/src/network_inspectors/reputation/reputation_parse.cc
index c3c3293dab545f7330ec570554994a85e505daed..0043665b583e9057fb197e8f227c8e355fdb2ff9 100644 (file)
--- a/src/network_inspectors/reputation/reputation_parse.cc
+++ b/src/network_inspectors/reputation/reputation_parse.cc
@@ -72,6 +72,8 @@ unsigned long total_invalids;
 
 int totalNumEntries = 0;
 
+static void load_list_file(ListFile*, ReputationConfig* config);
+
 ReputationConfig::~ReputationConfig()
 {
     if (reputation_segment != nullptr)
@@ -123,7 +125,6 @@ void ip_list_init(uint32_t max_entries, ReputationConfig* config)
         config->reputation_segment = (uint8_t*)snort_alloc(mem_size);
 
         segment_meminit(config->reputation_segment, mem_size);
-        uint8_t* base = config->reputation_segment;
 
         /*DIR_16x7_4x4 for performance, but memory usage is high
          *Use  DIR_8x16 worst case IPV4 5K, IPV6 15K (bytes)
@@ -137,37 +138,23 @@ void ip_list_init(uint32_t max_entries, ReputationConfig* config)
             return;
         }
 
-        MEM_OFFSET list_ptr =
-            segment_snort_calloc((size_t)config->list_files.size(), sizeof(ListInfo));
-        if ( !list_ptr )
-        {
-            ErrorMessage("Failed to create IP list.\n");
-            return;
-        }
-
-        config->ip_list->list_info = list_ptr;
-        ListInfo* listInfo = (ListInfo*)&base[list_ptr];
-
         total_duplicates = 0;
         for (size_t i = 0; i < config->list_files.size(); i++)
         {
-            listInfo[i].list_index = (uint8_t)i + 1;
+            config->list_files[i]->list_index = (uint8_t)i + 1;
             if (config->list_files[i]->file_type == WHITE_LIST)
             {
                 if (config->white_action == UNBLACK)
-                    listInfo[i].list_type = WHITELISTED_UNBLACK;
+                    config->list_files[i]->list_type = WHITELISTED_UNBLACK;
                 else
-                    listInfo[i].list_type = WHITELISTED_TRUST;
+                    config->list_files[i]->list_type = WHITELISTED_TRUST;
             }
             else if (config->list_files[i]->file_type == BLACK_LIST)
-                listInfo[i].list_type = BLACKLISTED;
+                config->list_files[i]->list_type = BLACKLISTED;
             else if (config->list_files[i]->file_type == MONITOR_LIST)
-                listInfo[i].list_type = MONITORED;
+                config->list_files[i]->list_type = MONITORED;
 
-            listInfo[i].list_id = config->list_files[i]->list_id;
-            memcpy(listInfo[i].zones, config->list_files[i]->zones, MAX_NUM_ZONES);
-            load_list_file(config->list_files[i]->file_name.c_str(), list_ptr, config);
-            list_ptr += sizeof(ListInfo);
+            load_list_file(config->list_files[i], config);
         }
     }
 }
@@ -531,12 +518,6 @@ static int update_path_to_file(char* full_filename, unsigned int max_size, const
 {
     const char* snort_conf_dir = get_snort_conf_dir();
 
-    if (!snort_conf_dir || !(*snort_conf_dir) || !full_filename || !filename)
-    {
-        ErrorMessage("can't create path.\n");
-        return 0;
-    }
-
     /*file_name is too long*/
     if ( max_size < strlen(filename) )
     {
@@ -571,15 +552,11 @@ static int update_path_to_file(char* full_filename, unsigned int max_size, const
     return 1;
 }
 
-static char* get_list_type_name(INFO info)
+static char* get_list_type_name(ListFile* list_info)
 {
-    uint8_t* base;
-    ListInfo* info_value;
-    base = (uint8_t*)segment_basePtr();
-    info_value = (ListInfo*)(&base[info]);
-    if (!info_value)
+    if (!list_info)
         return nullptr;
-    switch (info_value->list_type)
+    switch (list_info->list_type)
     {
     case DECISION_NULL:
         return nullptr;
@@ -592,12 +569,11 @@ static char* get_list_type_name(INFO info)
     case WHITELISTED_TRUST:
         return white_info;
     default:
-        break;
+        return nullptr;
     }
-    return nullptr;
 }
 
-void load_list_file(const char* filename, INFO info, ReputationConfig* config)
+static void load_list_file(ListFile* list_info, ReputationConfig* config)
 {
     char linebuf[MAX_ADDR_LINE_LENGTH];
     char full_path_filename[PATH_MAX+1];
@@ -605,7 +581,6 @@ void load_list_file(const char* filename, INFO info, ReputationConfig* config)
     FILE* fp = nullptr;
     char* cmt = nullptr;
     char* list_type_name;
-    ListInfo* list_info;
     IPrepInfo* ip_info;
     MEM_OFFSET ip_info_ptr;
     uint8_t* base;
@@ -616,12 +591,12 @@ void load_list_file(const char* filename, INFO info, ReputationConfig* config)
     unsigned int fail_count = 0;   /*number of invalid entries in this file*/
     unsigned int num_loaded_before = 0;     /*number of valid entries loaded */
 
-    if ((nullptr == filename)||(0 == info)|| (nullptr == config)||config->memcap_reached)
+    if (config->memcap_reached)
         return;
 
-    update_path_to_file(full_path_filename, PATH_MAX, filename);
+    update_path_to_file(full_path_filename, PATH_MAX, list_info->file_name.c_str());
 
-    list_type_name = get_list_type_name(info);
+    list_type_name = get_list_type_name(list_info);
 
     if (!list_type_name)
         return;
@@ -634,7 +609,6 @@ void load_list_file(const char* filename, INFO info, ReputationConfig* config)
     }
     base = (uint8_t*)config->ip_list;
     ip_info = ((IPrepInfo*)&base[ip_info_ptr]);
-    list_info = ((ListInfo*)&base[info]);
     ip_info->list_indexes[0] = list_info->list_index;
 
     LogMessage("    Processing %s file %s\n", list_type_name, full_path_filename);
@@ -780,7 +754,7 @@ void add_black_white_List(ReputationConfig* config)
     if (config->blacklist_path)
     {
         ListFile* listItem = new ListFile;
-        memset(listItem->zones, true, MAX_NUM_ZONES);
+        listItem->all_zones_enabled = true;
         listItem->file_name = config->blacklist_path;
         listItem->file_type = BLACK_LIST;
         listItem->list_id = 0;
@@ -789,7 +763,7 @@ void add_black_white_List(ReputationConfig* config)
     if (config->whitelist_path)
     {
         ListFile* listItem = new ListFile;
-        memset(listItem->zones, true, MAX_NUM_ZONES);
+        listItem->all_zones_enabled = true;
         listItem->file_name = config->whitelist_path;
         listItem->file_type = WHITE_LIST;
         listItem->list_id = 0;
@@ -850,15 +824,15 @@ static int get_file_type(char* type_name)
 //    file_name, list_id, action (black, white, monitor), zone information
 //If no zone information provided, this means all zones are applied.
 
-static bool process_line_in_manifest(ListFile* list_item, const char* manifest, char* line,
+static bool process_line_in_manifest(ListFile* list_item, const char* manifest, const char* line,
     int line_number, ReputationConfig* config)
 {
     char* token;
     int token_index = 0;
-    char* next_ptr = line;
+    char* next_ptr = (char*)line;
     bool has_zone = false;
 
-    memset(list_item->zones, false, MAX_NUM_ZONES);
+    list_item->zones.clear();
 
     while ((token = strtok_r(next_ptr, MANIFEST_SEPARATORS, &next_ptr)) != NULL)
     {
@@ -878,7 +852,7 @@ static bool process_line_in_manifest(ListFile* list_item, const char* manifest,
             if ( *end_str )
             {
                 ErrorMessage("%s(%d) => Bad value (%s) specified for listID. "
-                    "Please specify an integer between %d and %d.\n",
+                    "Please specify an integer between %u and %u.\n",
                     manifest, line_number, token, 0, MAX_LIST_ID);
                 return false;
             }
@@ -886,7 +860,7 @@ static bool process_line_in_manifest(ListFile* list_item, const char* manifest,
             if ((list_id < 0)  || (list_id > MAX_LIST_ID) || (errno == ERANGE))
             {
                 ErrorMessage(" %s(%d) => Value specified (%s) is out of "
-                    "bounds.  Please specify an integer between %d and %d.\n",
+                    "bounds.  Please specify an integer between %u and %u.\n",
                     manifest, line_number, token, 0, MAX_LIST_ID);
                 return false;
             }
@@ -915,19 +889,19 @@ static bool process_line_in_manifest(ListFile* list_item, const char* manifest,
             if ( *end_str )
             {
                 ErrorMessage("%s(%d) => Bad value (%s) specified for zone. "
-                    "Please specify an integer between %d and %d.\n",
-                    manifest, line_number, token, 0, MAX_NUM_ZONES - 1);
+                    "Please specify an integer between %u and %u.\n",
+                    manifest, line_number, token, 0, MAX_NUM_ZONES);
                 return false;
             }
-            if ((zone_id < 0)  || (zone_id >= MAX_NUM_ZONES ) || (errno == ERANGE))
+            if ((zone_id < 0)  || (zone_id > MAX_NUM_ZONES ) || (errno == ERANGE))
             {
                 ErrorMessage(" %s(%d) => Value specified (%s) for zone is "
-                    "out of bounds. Please specify an integer between %d and %d.\n",
-                    manifest, line_number, token, 0, MAX_NUM_ZONES - 1);
+                    "out of bounds. Please specify an integer between %u and %u.\n",
+                    manifest, line_number, token, 0, MAX_NUM_ZONES );
                 return false;
             }
 
-            list_item->zones[zone_id] = true;
+            list_item->zones.insert(zone_id);
             has_zone = true;
         }
 
@@ -945,7 +919,7 @@ static bool process_line_in_manifest(ListFile* list_item, const char* manifest,
 
     if (!has_zone)
     {
-        memset(list_item->zones, true, MAX_NUM_ZONES);
+        list_item->all_zones_enabled = true;
     }
 
     config->list_files.push_back(list_item);
@@ -955,7 +929,7 @@ static bool process_line_in_manifest(ListFile* list_item, const char* manifest,
 int read_manifest(const char* manifest_file, ReputationConfig* config)
 {
     int line_number = 0;
-    char line[MAX_MANIFEST_LINE_LENGTH];
+    std::string line;
     char full_path_dir[PATH_MAX+1];
 
     update_path_to_file(full_path_dir, PATH_MAX, config->list_dir.c_str());
@@ -970,20 +944,18 @@ int read_manifest(const char* manifest_file, ReputationConfig* config)
         return -1;
     }
 
-    while (!fs.eof())
+    while (std::getline(fs, line))
     {
-        fs.getline(line, sizeof(line));
-
-        char* nextPtr = NULL;
         line_number++;
 
         /* remove comments */
-        if ( (nextPtr = strchr(line, '#')) )
-            *nextPtr = '\0';
+        size_t pos = line.find_first_of('#');
+        if (pos != line.npos)
+           line[pos] = '\0';
 
         //Processing the line
         ListFile* list_item = new ListFile;
-        if (!process_line_in_manifest(list_item, manifest_file, line, line_number, config))
+        if (!process_line_in_manifest(list_item, manifest_file, line.c_str(), line_number, config))
             delete list_item;
     }
 

diff --git a/src/network_inspectors/reputation/reputation_parse.h b/src/network_inspectors/reputation/reputation_parse.h
index 433a0670ddd6483b475555b2d56c0b3ebfc5d398..0dfe2b1b0f25ebe9de652f205dd1c825119744bb 100644 (file)
--- a/src/network_inspectors/reputation/reputation_parse.h
+++ b/src/network_inspectors/reputation/reputation_parse.h
@@ -26,7 +26,6 @@
 
 void ip_list_init(uint32_t,ReputationConfig *config);
 void estimate_num_entries(ReputationConfig* config);
-void load_list_file(const char* filename, INFO info, ReputationConfig* config);
 int read_manifest(const char* filename, ReputationConfig* config);
 void add_black_white_List(ReputationConfig* config);