From: Russ Combs (rucombs) Date: Tue, 23 Feb 2016 23:39:36 +0000 (-0500) Subject: Merge pull request #278 in SNORT/snort3 from ~JOCORNET/snort3:bitop_cleanup to master X-Git-Tag: 3.0.0-233~588 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=fd181636f8fa6427ad3f3e408b80cd3219dafa3c;p=thirdparty%2Fsnort3.git Merge pull request #278 in SNORT/snort3 from ~JOCORNET/snort3:bitop_cleanup to master Squashed commit of the following: commit 7663a94772e7c2526fcce66a571a9e6a274534d3 Author: Joel Cornett Date: Tue Feb 23 16:10:20 2016 -0500 added unit tests and minor fixes commit c5c904e5e2f3ec1e816973c52843f97d981cf320 Author: Joel Cornett Date: Mon Feb 22 20:10:39 2016 -0500 removed unique_ptr commit c6328af429eebe799a98bb82e9f634ea522f355c Author: Joel Cornett Date: Fri Feb 19 13:36:00 2016 -0500 fixes for gcc commit be04c3063a1998594fffe2d801983f3dc98e9478 Author: Joel Cornett Date: Fri Feb 19 13:24:37 2016 -0500 cleaned up bitop commit 3ebc109f59ad19f6553af3efd7c4409c14b04b70 Author: Joel Cornett Date: Fri Feb 19 13:24:26 2016 -0500 fixed includes --- diff --git a/src/detection/treenodes.cc b/src/detection/treenodes.cc index e6e62372b..7e76c0cdb 100644 --- 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 -#include -#include -#include -#include -#include - -#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 91af55838..16d9e23e2 100644 --- 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 4ab41f626..9faecce8b 100644 --- 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 d576551d6..a375a34aa 100644 --- 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 e3e68f55c..f45e99302 100644 --- 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 + #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 4d3f6d1ee..ab38638a9 100644 --- a/src/service_inspectors/dns/dns.cc +++ b/src/service_inspectors/dns/dns.cc @@ -33,6 +33,7 @@ #include #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 fd5b98009..75fe77219 100644 --- 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 #include "ftp_module.h" + #include +#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 69ca1aabf..0bd65c844 100644 --- 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 5a7d40781..0c68f6a33 100644 --- a/src/service_inspectors/gtp/gtp_parser.cc +++ b/src/service_inspectors/gtp/gtp_parser.cc @@ -28,6 +28,7 @@ #include +#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 238903b8d..53d3d6e63 100644 --- a/src/service_inspectors/imap/imap.cc +++ b/src/service_inspectors/imap/imap.cc @@ -30,6 +30,7 @@ #include #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 884cc1cff..3633fe54a 100644 --- a/src/service_inspectors/pop/pop.cc +++ b/src/service_inspectors/pop/pop.cc @@ -28,6 +28,7 @@ #include #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 8488e7cd4..b6428fe3d 100644 --- 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 1d260769a..7245b1e52 100644 --- 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 47c663ed8..a7f573736 100644 --- a/src/service_inspectors/ssh/ssh.cc +++ b/src/service_inspectors/ssh/ssh.cc @@ -32,6 +32,7 @@ #include #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 7c2b2b0dd..485ad84b1 100644 --- a/src/service_inspectors/ssl/ssl_inspector.cc +++ b/src/service_inspectors/ssl/ssl_inspector.cc @@ -34,6 +34,7 @@ #include #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 b3620f0cb..d39799dba 100644 --- 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 // 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 3d01fc9e6..1118d9cc6 100644 --- a/src/utils/bitop.h +++ b/src/utils/bitop.h @@ -25,90 +25,85 @@ // A simple, dynamically sized bit vector implementation -#include -#include -#include -#include - -#include "utils/util.h" +#include +#include 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 index 000000000..63af4275c --- /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 f1c7d5eec..0f2abe164 100644 --- 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