Merge pull request #278 in SNORT/snort3 from ~JOCORNET/snort3:bitop_cleanup to master

Squashed commit of the following:

commit 7663a94772e7c2526fcce66a571a9e6a274534d3
Author: Joel Cornett <joel.cornett@gmail.com>
Date:   Tue Feb 23 16:10:20 2016 -0500

    added unit tests and minor fixes

commit c5c904e5e2f3ec1e816973c52843f97d981cf320
Author: Joel Cornett <joel.cornett@gmail.com>
Date:   Mon Feb 22 20:10:39 2016 -0500

    removed unique_ptr

commit c6328af429eebe799a98bb82e9f634ea522f355c
Author: Joel Cornett <joel.cornett@gmail.com>
Date:   Fri Feb 19 13:36:00 2016 -0500

    fixes for gcc

commit be04c3063a1998594fffe2d801983f3dc98e9478
Author: Joel Cornett <joel.cornett@gmail.com>
Date:   Fri Feb 19 13:24:37 2016 -0500

    cleaned up bitop

commit 3ebc109f59ad19f6553af3efd7c4409c14b04b70
Author: Joel Cornett <joel.cornett@gmail.com>
Date:   Fri Feb 19 13:24:26 2016 -0500

    fixed includes

diff --git a/src/detection/treenodes.cc b/src/detection/treenodes.cc
index e6e62372b0302afe4a69550a72c7cb73466dd1d1..7e76c0cdb9cf6ed73185a428998913c0fa3de501 100644 (file)
--- a/src/detection/treenodes.cc
+++ b/src/detection/treenodes.cc
@@ -20,21 +20,12 @@
 
 #include "treenodes.h"
 
-#ifdef HAVE_CONFIG_H
-#include "config.h"
-#endif
-
-#include <sys/types.h>
-#include <unistd.h>
-#include <stdlib.h>
-#include <string.h>
-#include <time.h>
-#include <errno.h>
-
-#include "detect.h"
+#include "framework/ips_option.h"
 #include "main/snort_types.h"
 #include "main/snort_debug.h"
-#include "framework/ips_option.h"
+#include "utils/util.h"
+
+#include "detect.h"
 
 /****************************************************************************
  *

diff --git a/src/ips_options/ips_flowbits.cc b/src/ips_options/ips_flowbits.cc
index 91af558384788112a755ad058425e225cd4788b3..16d9e23e231b29a961451115d7c43af88b0e79c0 100644 (file)
--- a/src/ips_options/ips_flowbits.cc
+++ b/src/ips_options/ips_flowbits.cc
@@ -271,55 +271,52 @@ int FlowBitsOption::eval(Cursor&, Packet* p)
 
 static inline int clear_group_bit(BitOp* bitop, char* group)
 {
-    FLOWBITS_GRP* flowbits_grp;
-    BitOp* GrpBitOp;
-    unsigned int i, max_bytes;
-
-    if ( group == NULL )
+    if ( !group )
         return 0;
 
     // FIXIT-M why is the hash lookup done at runtime for flowbits groups?
     // a pointer to flowbis_grp should be in flowbits config data
     // this *should* be safe but iff splay mode is disabled
-    flowbits_grp = (FLOWBITS_GRP*)sfghash_find(flowbits_grp_hash, group);
-    if ( flowbits_grp == NULL )
+    auto flowbits_grp = (FLOWBITS_GRP*)sfghash_find(flowbits_grp_hash, group);
+
+    if ( !flowbits_grp )
         return 0;
-    if ((bitop == NULL) || (bitop->get_max_bits() <= flowbits_grp->max_id) || flowbits_grp->count == 0)
+
+    if ( !bitop || (bitop->size() <= flowbits_grp->max_id) || !flowbits_grp->count )
         return 0;
-    GrpBitOp = flowbits_grp->GrpBitOp;
+
+    auto GrpBitOp = flowbits_grp->GrpBitOp;
 
     /* note, max_id is an index, not a count.
      * Calculate max_bytes by adding 8 to max_id, then dividing by 8.  */
-    max_bytes = (flowbits_grp->max_id + 8) >> 3;
-    for ( i = 0; i < max_bytes; i++ )
-    {
-        (*bitop)[i] &= ~((*GrpBitOp)[i]);
-    }
+    unsigned int max_bytes = (flowbits_grp->max_id + 8) >> 3;
+    for ( unsigned int i = 0; i < max_bytes; i++ )
+        bitop->get_buf_element(i) &= ~GrpBitOp->get_buf_element(i);
+
     return 1;
 }
 
 static inline int toggle_group_bit(BitOp* bitop, char* group)
 {
-    FLOWBITS_GRP* flowbits_grp;
-    BitOp* GrpBitOp;
-    unsigned int i, max_bytes;
-
-    if ( group == NULL )
+    if ( !group  )
         return 0;
-    flowbits_grp = (FLOWBITS_GRP*)sfghash_find(flowbits_grp_hash, group);
-    if ( flowbits_grp == NULL )
+
+    auto flowbits_grp = (FLOWBITS_GRP*)sfghash_find(flowbits_grp_hash, group);
+
+    if ( !flowbits_grp )
         return 0;
-    if ((bitop == NULL) || (bitop->get_max_bits() <= flowbits_grp->max_id) || flowbits_grp->count == 0)
+
+    if ( !bitop || (bitop->size() <= flowbits_grp->max_id) || !flowbits_grp->count )
         return 0;
-    GrpBitOp = flowbits_grp->GrpBitOp;
+
+    auto GrpBitOp = flowbits_grp->GrpBitOp;
 
     /* note, max_id is an index, not a count.
      * Calculate max_bytes by adding 8 to max_id, then dividing by 8.  */
-    max_bytes = (flowbits_grp->max_id + 8) >> 3;
-    for ( i = 0; i < max_bytes; i++ )
-    {
-        (*bitop)[i] ^= (*GrpBitOp)[i];
-    }
+    unsigned int max_bytes = (flowbits_grp->max_id + 8) >> 3;
+    for ( unsigned int i = 0; i < max_bytes; i++ )
+        bitop->get_buf_element(i) ^= GrpBitOp->get_buf_element(i);
+
     return 1;
 }
 
@@ -366,9 +363,9 @@ static inline int is_set_flowbits(
             return 0;
         for ( i = 0; i <= (unsigned int)(flowbits_grp->max_id >>3); i++ )
         {
-            uint8_t val = (*bitop)[i] &
-                (*(flowbits_grp->GrpBitOp))[i];
-            if (val != (*(flowbits_grp->GrpBitOp))[i])
+            uint8_t val = bitop->get_buf_element(i) & flowbits_grp->GrpBitOp->get_buf_element(i);
+
+            if ( val != flowbits_grp->GrpBitOp->get_buf_element(i) )
                 return 0;
         }
         return 1;
@@ -379,9 +376,8 @@ static inline int is_set_flowbits(
             return 0;
         for ( i = 0; i <= (unsigned int)(flowbits_grp->max_id >>3); i++ )
         {
-            uint8_t val = (*bitop)[i] &
-                (*(flowbits_grp->GrpBitOp))[i];
-            if (val)
+            uint8_t val = bitop->get_buf_element(i) & flowbits_grp->GrpBitOp->get_buf_element(i);
+            if ( val )
                 return 1;
         }
         return 0;

diff --git a/src/mime/file_mime_config.cc b/src/mime/file_mime_config.cc
index 4ab41f62674b16709863060511a2b104df5b91fa..9faecce8bbcb01ac9de2f52a413b433fd723019a 100644 (file)
--- a/src/mime/file_mime_config.cc
+++ b/src/mime/file_mime_config.cc
@@ -25,8 +25,8 @@
 
 #include "file_mime_config.h"
 
+#include "log/messages.h"
 #include "main/snort_types.h"
-
 #include "file_api/file_service.h"
 #include "file_mime_process.h"
 

diff --git a/src/ppm/ppm.cc b/src/ppm/ppm.cc
index d576551d6a08d64fd6ccd0417c219cfab5a5d829..a375a34aa4c3d38e85a307b468c7fa6fad90ac95 100644 (file)
--- a/src/ppm/ppm.cc
+++ b/src/ppm/ppm.cc
@@ -45,13 +45,15 @@
 #include "config.h"
 #endif
 
-#include "ppm_module.h"
 #include "detection/fp_create.h"
-#include "parser/parser.h"
+#include "detection/treenodes.h"
 #include "events/event_queue.h"
-#include "utils/stats.h"
+#include "log/messages.h"
 #include "sfip/sf_ip.h"
-#include "time/cpuclock.h"
+#include "utils/stats.h"
+#include "utils/util.h"
+
+#include "ppm_module.h"
 
 #define PPM_BASE_SUSPEND_RULE_GID 1000
 #define PPM_BASE_CLEAR_RULE_GID   2000

diff --git a/src/service_inspectors/dce_rpc/ips_dce_iface.cc b/src/service_inspectors/dce_rpc/ips_dce_iface.cc
index e3e68f55cba025644452a8dfe818fba0ed3b13db..f45e99302634c2aad3e7d36bd2e53717da8d2d2a 100644 (file)
--- a/src/service_inspectors/dce_rpc/ips_dce_iface.cc
+++ b/src/service_inspectors/dce_rpc/ips_dce_iface.cc
@@ -21,6 +21,8 @@
 
 #include "dce_utils.h"
 
+#include <cerrno>
+
 #include "framework/ips_option.h"
 #include "framework/module.h"
 #include "framework/parameter.h"

diff --git a/src/service_inspectors/dns/dns.cc b/src/service_inspectors/dns/dns.cc
index 4d3f6d1eef2f99668b5716760242809cbc1f33d5..ab38638a9ba61a1bfb89095e3e40486eb3cdfe84 100644 (file)
--- a/src/service_inspectors/dns/dns.cc
+++ b/src/service_inspectors/dns/dns.cc
@@ -33,6 +33,7 @@
 #include <sys/types.h>
 
 #include "events/event_queue.h"
+#include "log/messages.h"
 #include "main/snort_types.h"
 #include "main/snort_debug.h"
 #include "profiler/profiler.h"

diff --git a/src/service_inspectors/ftp_telnet/ftp_module.cc b/src/service_inspectors/ftp_telnet/ftp_module.cc
index fd5b98009d260775cbf8a2234ad990642632874a..75fe77219c20da0c528458344bdd4c58cb022ca9 100644 (file)
--- a/src/service_inspectors/ftp_telnet/ftp_module.cc
+++ b/src/service_inspectors/ftp_telnet/ftp_module.cc
@@ -19,8 +19,10 @@
 // ftp_module.cc author Russ Combs <rucombs@cisco.com>
 
 #include "ftp_module.h"
+
 #include <sstream>
 
+#include "log/messages.h"
 #include "parser/parser.h"
 
 using namespace std;

diff --git a/src/service_inspectors/ftp_telnet/telnet.cc b/src/service_inspectors/ftp_telnet/telnet.cc
index 69ca1aabf2af6b4cb2d47fc01e94face1b9ac861..0bd65c844904c05eb6a008222d61128ae57f9ee4 100644 (file)
--- a/src/service_inspectors/ftp_telnet/telnet.cc
+++ b/src/service_inspectors/ftp_telnet/telnet.cc
@@ -36,6 +36,7 @@
 #include "ftp_print.h"
 #include "telnet_module.h"
 
+#include "log/messages.h"
 #include "main/snort_types.h"
 #include "main/snort_debug.h"
 #include "profiler/profiler.h"

diff --git a/src/service_inspectors/gtp/gtp_parser.cc b/src/service_inspectors/gtp/gtp_parser.cc
index 5a7d40781b89dbd8081ea633a2aff9d6d0f86fa3..0c68f6a33af4abad7e445bdeeeb70fd1d4447147 100644 (file)
--- a/src/service_inspectors/gtp/gtp_parser.cc
+++ b/src/service_inspectors/gtp/gtp_parser.cc
@@ -28,6 +28,7 @@
 
 #include <ctype.h>
 
+#include "log/messages.h"
 #include "main/snort_types.h"
 #include "main/snort_debug.h"
 #include "events/event_queue.h"

diff --git a/src/service_inspectors/imap/imap.cc b/src/service_inspectors/imap/imap.cc
index 238903b8dc7767eb20e49048fdd6fa169c90adac..53d3d6e63480d0b0d5dba9cc12ef70723fdc4e3b 100644 (file)
--- a/src/service_inspectors/imap/imap.cc
+++ b/src/service_inspectors/imap/imap.cc
@@ -30,6 +30,7 @@
 #include <sys/types.h>
 
 #include "events/event_queue.h"
+#include "log/messages.h"
 #include "main/snort_types.h"
 #include "main/snort_debug.h"
 #include "profiler/profiler.h"
@@ -40,6 +41,7 @@
 #include "target_based/snort_protocols.h"
 #include "search_engines/search_tool.h"
 #include "utils/sfsnprintfappend.h"
+#include "utils/util.h"
 #include "protocols/ssl.h"
 #include "mime/file_mime_process.h"
 

diff --git a/src/service_inspectors/pop/pop.cc b/src/service_inspectors/pop/pop.cc
index 884cc1cffaed71a4935fee6dc6f88b78c97eb639..3633fe54a7dd6661fd6608cbdbad6aa232624834 100644 (file)
--- a/src/service_inspectors/pop/pop.cc
+++ b/src/service_inspectors/pop/pop.cc
@@ -28,6 +28,7 @@
 #include <sys/types.h>
 
 #include "events/event_queue.h"
+#include "log/messages.h"
 #include "main/snort_types.h"
 #include "main/snort_debug.h"
 #include "profiler/profiler.h"
@@ -37,6 +38,7 @@
 #include "target_based/snort_protocols.h"
 #include "search_engines/search_tool.h"
 #include "utils/sfsnprintfappend.h"
+#include "utils/util.h"
 #include "protocols/ssl.h"
 #include "file_api/file_api.h"
 #include "mime/file_mime_process.h"

diff --git a/src/service_inspectors/sip/ips_sip_method.cc b/src/service_inspectors/sip/ips_sip_method.cc
index 8488e7cd406c4d66e5c20d7eac571061dabe820e..b6428fe3deb092ddcb1c35b92fa54af635e761f0 100644 (file)
--- a/src/service_inspectors/sip/ips_sip_method.cc
+++ b/src/service_inspectors/sip/ips_sip_method.cc
@@ -27,13 +27,13 @@
 #endif
 
 #include "sip.h"
-
 #include "framework/ips_option.h"
 #include "framework/module.h"
 #include "framework/parameter.h"
 #include "detection/detect.h"
 #include "detection/detection_defines.h"
 #include "hash/sfhashfcn.h"
+#include "log/messages.h"
 #include "profiler/profiler.h"
 
 //-------------------------------------------------------------------------

diff --git a/src/service_inspectors/sip/ips_sip_stat_code.cc b/src/service_inspectors/sip/ips_sip_stat_code.cc
index 1d260769a926c44be629c745c4f108f1682fd0a5..7245b1e52c72e83966115b9d3dcc9a83864b132a 100644 (file)
--- a/src/service_inspectors/sip/ips_sip_stat_code.cc
+++ b/src/service_inspectors/sip/ips_sip_stat_code.cc
@@ -28,12 +28,13 @@
 
 #include "sip.h"
 
+#include "detection/detect.h"
+#include "detection/detection_defines.h"
 #include "framework/ips_option.h"
 #include "framework/module.h"
 #include "framework/parameter.h"
-#include "detection/detect.h"
-#include "detection/detection_defines.h"
 #include "hash/sfhashfcn.h"
+#include "log/messages.h"
 #include "profiler/profiler.h"
 
 //-------------------------------------------------------------------------

diff --git a/src/service_inspectors/ssh/ssh.cc b/src/service_inspectors/ssh/ssh.cc
index 47c663ed8e410ea84b8ff2bcd64776b37df80ee1..a7f573736db25fab12153f0a16d53d1ae3cecbae 100644 (file)
--- a/src/service_inspectors/ssh/ssh.cc
+++ b/src/service_inspectors/ssh/ssh.cc
@@ -32,6 +32,7 @@
 #include <sys/types.h>
 
 #include "events/event_queue.h"
+#include "log/messages.h"
 #include "main/snort_types.h"
 #include "main/snort_debug.h"
 #include "profiler/profiler.h"

diff --git a/src/service_inspectors/ssl/ssl_inspector.cc b/src/service_inspectors/ssl/ssl_inspector.cc
index 7c2b2b0ddc876f14785245e9ff6baf2ddd3716db..485ad84b10946bf6e677d1858c06c3ebcc938733 100644 (file)
--- a/src/service_inspectors/ssl/ssl_inspector.cc
+++ b/src/service_inspectors/ssl/ssl_inspector.cc
@@ -34,6 +34,7 @@
 #include <sys/types.h>
 
 #include "events/event_queue.h"
+#include "log/messages.h"
 #include "main/snort_types.h"
 #include "main/snort_debug.h"
 #include "profiler/profiler.h"

diff --git a/src/stream/libtcp/tcp_segment_descriptor.cc b/src/stream/libtcp/tcp_segment_descriptor.cc
index b3620f0cb3f8d33a69e86ef1cc5f699e382574b5..d39799dbac85846e4f284502de4c9562be1eeecb 100644 (file)
--- a/src/stream/libtcp/tcp_segment_descriptor.cc
+++ b/src/stream/libtcp/tcp_segment_descriptor.cc
@@ -19,10 +19,10 @@
 // tcp_segment_descriptor.cc author davis mcpherson <davmcphe@cisco.com>
 // Created on: Jul 30, 2015
 
+#include "log/messages.h"
 #include "main/snort_debug.h"
-
-#include "stream/tcp/tcp_defs.h"
 #include "protocols/tcp_options.h"
+#include "stream/tcp/tcp_defs.h"
 #include "stream/tcp/tcp_event_logger.h"
 #include "tcp_segment_descriptor.h"
 

diff --git a/src/utils/bitop.h b/src/utils/bitop.h
index 3d01fc9e64f3ba247681bd415bc089c7e14f5d04..1118d9cc6e2e43ebd3f5bd42a34cf70bfba46942 100644 (file)
--- a/src/utils/bitop.h
+++ b/src/utils/bitop.h
@@ -25,90 +25,85 @@
 
 // A simple, dynamically sized bit vector implementation
 
-#include <assert.h>
-#include <stdlib.h>
-#include <stdint.h>
-#include <string.h>
-
-#include "utils/util.h"
+#include <cassert>
+#include <cstring>
 
 class BitOp
 {
 public:
-    BitOp(unsigned int len)
-    {
-        assert(len);
-
-        bit_buf = (uint8_t*)SnortAlloc(len);
-
-        buf_size = (unsigned int)len;
-        max_bits = (unsigned int)(len << 3);
-    }
-
-    ~BitOp()
-    {
-        free(bit_buf);
-    }
+    BitOp(size_t);
+    ~BitOp();
 
     void reset();
     void set(unsigned int bit);
-    bool is_set(unsigned int bit);
+    bool is_set(unsigned int bit) const;
     void clear(unsigned int bit);
 
-    unsigned int get_max_bits()
-    { return max_bits; }
+    size_t size() const;
 
-    //FIXIT-L This should be eliminated and better encapsulated.
-    uint8_t& operator[](unsigned int pos)
-    { if ( pos > buf_size) pos = 0; return bit_buf[pos]; }
+    // FIXIT-L J add operator overloads for [], &=, |=, etc
+    size_t get_buf_size() const;
+    uint8_t& get_buf_element(size_t);
+    const uint8_t& get_buf_element(size_t) const;
 
 private:
+    uint8_t mask(size_t bit) const;
+
     uint8_t* bit_buf;
-    unsigned int buf_size;
-    unsigned int max_bits;
+    const size_t buf_size;
 };
 
+// -----------------------------------------------------------------------------
+// implementation
+// -----------------------------------------------------------------------------
+
+inline BitOp::BitOp(size_t len) :
+    bit_buf(new uint8_t[len]()), buf_size(len)
+{ }
+
+inline BitOp::~BitOp()
+{ delete[] bit_buf; }
+
+// FIXIT-L J ops that don't need to be inlined can probably be but into a .cc file
 // Reset the bit buffer so that it can be reused
 inline void BitOp::reset()
-{
-    memset(bit_buf, 0, buf_size);
-}
+{ memset(bit_buf, 0, buf_size); }
 
 // Set the bit in the specified position within the bit buffer.
 inline void BitOp::set(unsigned int bit)
 {
-    if ( max_bits <= bit )
-    {
-        assert(false);
-        return;
-    }
-    uint8_t mask = (uint8_t)(0x80 >> (bit & 7));
-    bit_buf[bit >> 3] |= mask;
+    assert(size() > bit);
+    bit_buf[bit >> 3] |= mask(bit);
 }
 
 // Checks if the bit at the specified position is set
-inline bool BitOp::is_set(unsigned int bit)
+inline bool BitOp::is_set(unsigned int bit) const
 {
-    if ( max_bits <= bit )
-    {
-        assert(false);
-        return false;
-    }
-    uint8_t mask = (uint8_t)(0x80 >> (bit & 7));
-    return (mask & bit_buf[bit >> 3]);
+    assert(size() > bit);
+    return mask(bit) & bit_buf[bit >> 3];
 }
 
 // Clear the bit in the specified position within the bit buffer.
 inline void BitOp::clear(unsigned int bit)
 {
-    if ( max_bits <= bit )
-    {
-        assert(false);
-        return;
-    }
-    uint8_t mask = (uint8_t)(0x80 >> (bit & 7));
-    bit_buf[bit >> 3] &= ~mask;
+    assert(size() > bit);
+    bit_buf[bit >> 3] &= ~mask(bit);
 }
 
+inline size_t BitOp::size() const
+{ return buf_size << 3; }
+
+inline uint8_t BitOp::mask(size_t bit) const
+{ return (uint8_t)(0x80 >> (bit & 7)); }
+
+inline size_t BitOp::get_buf_size() const
+{ return buf_size; }
+
+inline uint8_t& BitOp::get_buf_element(size_t i)
+{ return bit_buf[i]; }
+
+inline const uint8_t& BitOp::get_buf_element(size_t i) const
+{ return bit_buf[i]; }
+
 #endif
 

diff --git a/src/utils/bitop_test.cc b/src/utils/bitop_test.cc
new file mode 100644 (file)
index 0000000..63af427
--- /dev/null
+++ b/src/utils/bitop_test.cc
@@ -0,0 +1,49 @@
+#include "bitop.h"
+#include "catch/catch.hpp"
+
+static bool t_bitop_buffer_zero(BitOp& bitop)
+{
+    for ( size_t i = 0; i < bitop.get_buf_size(); ++i )
+        if ( bitop.get_buf_element(i) )
+            return false;
+
+    return true;
+}
+
+TEST_CASE( "bitop", "[bitop]" )
+{
+    BitOp bitop(3);
+
+    SECTION( "zero-initialized" )
+    {
+        CHECK( t_bitop_buffer_zero(bitop) );
+    }
+
+    SECTION( "reset" )
+    {
+        bitop.get_buf_element(0) = 0xff;
+        bitop.reset();
+
+        CHECK( t_bitop_buffer_zero(bitop) );
+    }
+
+    SECTION( "set/is_set/clear" )
+    {
+        bitop.set(6);
+
+        CHECK( bitop.get_buf_element(0) == 0x02 );
+
+        CHECK( bitop.is_set(6) );
+        CHECK_FALSE( bitop.is_set(7) );
+
+        bitop.set(7);
+        bitop.clear(6);
+
+        CHECK( bitop.get_buf_element(0) == 0x01 );
+    }
+
+    SECTION( "size" )
+    {
+        CHECK( bitop.size() == 24 );
+    }
+}

diff --git a/src/utils/util.cc b/src/utils/util.cc
index f1c7d5eecbb895b281c1578f2f7d578df6c38ff7..0f2abe164a210cd48b59e34de9e04f8c44d03394 100644 (file)
--- a/src/utils/util.cc
+++ b/src/utils/util.cc
@@ -990,3 +990,6 @@ const char* get_error(int errnum)
 #endif
 }
 
+#ifdef UNIT_TEST
+#include "bitop_test.cc"
+#endif