From: Russ Combs (rucombs) Date: Mon, 23 May 2016 22:28:21 +0000 (-0400) Subject: Merge pull request #481 in SNORT/snort3 from packet-struct to master X-Git-Tag: 3.0.0-233~396 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=b2009ddb04b31e1f7eeb00d8b738ea53bcfd31b5;p=thirdparty%2Fsnort3.git Merge pull request #481 in SNORT/snort3 from packet-struct to master Squashed commit of the following: commit 59def90162924e1037a0f52e6ed12d6133a53219 Author: Victor Roemer Date: Thu May 19 15:10:42 2016 -0400 Add ctor/dtor to packet and cleanup packet_manager. --- diff --git a/src/log/log_text.cc b/src/log/log_text.cc index 283c4de72..5412de3dc 100644 --- a/src/log/log_text.cc +++ b/src/log/log_text.cc @@ -792,8 +792,7 @@ static void LogICMPEmbeddedIP(TextLog* log, Packet* p) return; // FIXIT-L -- Allocating a new Packet here is ridiculously excessive. - Packet* orig_p = PacketManager::encode_new(); - orig_p->reset(); + Packet* orig_p = new Packet; Packet& op = *orig_p; if (!layer::set_api_ip_embed_icmp(p, op.ptrs.ip_api)) @@ -876,7 +875,7 @@ static void LogICMPEmbeddedIP(TextLog* log, Packet* p) TextLog_Puts(log, "** END OF DUMP"); } - PacketManager::encode_delete(orig_p); + delete orig_p; } /*-------------------------------------------------------------------- diff --git a/src/log/test/obfuscator_test.cc b/src/log/test/obfuscator_test.cc index 6a61b6774..4f74648a4 100644 --- a/src/log/test/obfuscator_test.cc +++ b/src/log/test/obfuscator_test.cc @@ -22,6 +22,7 @@ #include #include +#include TEST_GROUP(ObfuscatorTests) { diff --git a/src/main/snort.cc b/src/main/snort.cc index 291d829a3..6d41f2159 100644 --- a/src/main/snort.cc +++ b/src/main/snort.cc @@ -609,7 +609,7 @@ void Snort::thread_init(const char* intf) if (daq_instance->configure(snort_conf)) daq_instance->start(); - s_packet = PacketManager::encode_new(false); + s_packet = new Packet(false); CodecManager::thread_init(snort_conf); // this depends on instantiated daq capabilities @@ -650,7 +650,7 @@ void Snort::thread_term() if ( s_packet ) { - PacketManager::encode_delete(s_packet); + delete s_packet; s_packet = nullptr; } diff --git a/src/managers/codec_manager.cc b/src/managers/codec_manager.cc index 59789d941..0bc4215b7 100644 --- a/src/managers/codec_manager.cc +++ b/src/managers/codec_manager.cc @@ -297,6 +297,5 @@ CodecWrapper* CodecManager::instantiate(const char* name, Module* m, SnortConfig return new CodecWrapper(api, p); } - #endif diff --git a/src/managers/codec_manager.h b/src/managers/codec_manager.h index d1e195685..c5d764b90 100644 --- a/src/managers/codec_manager.h +++ b/src/managers/codec_manager.h @@ -95,6 +95,9 @@ public: static CodecWrapper* instantiate(const char*, Module*, SnortConfig*); #endif + static uint8_t get_max_layers() + { return max_layers; } + private: struct CodecApiWrapper; diff --git a/src/network_inspectors/packet_capture/packet_capture.cc b/src/network_inspectors/packet_capture/packet_capture.cc index 11f8dc9fd..e2ebc8335 100644 --- a/src/network_inspectors/packet_capture/packet_capture.cc +++ b/src/network_inspectors/packet_capture/packet_capture.cc @@ -223,7 +223,7 @@ const BaseApi* nin_packet_capture = &pc_api.base; #ifdef UNIT_TEST Packet* init_null_packet() { - static Packet p; + static Packet p(false); static DAQ_PktHdr_t h; p.pkt = nullptr; @@ -319,7 +319,7 @@ TEST_CASE("blank filter", "[PacketCapture]") const uint8_t cooked[] = "AbCdEfGhIjKlMnOpQrStUvWxYz"; struct pcap_pkthdr hdr; - Packet p; + Packet p(false); DAQ_PktHdr_t daq_hdr; p.pkt = cooked; p.pkth = &daq_hdr; @@ -397,9 +397,7 @@ TEST_CASE("bpf filter", "[PacketCapture]") "\x45\x00\x00\x14\x96\x22\x40\x00\x39\x06\xb1\xeb\x0b\x52\xf0\x52" "\x0a\x96"; - struct pcap_pkthdr hdr; - - Packet p_match, p_non_match; + Packet p_match(false), p_non_match(false); DAQ_PktHdr_t daq_hdr; p_match.pkth = &daq_hdr; diff --git a/src/network_inspectors/perf_monitor/flow_tracker.cc b/src/network_inspectors/perf_monitor/flow_tracker.cc index e3ff189fa..c688e6082 100644 --- a/src/network_inspectors/perf_monitor/flow_tracker.cc +++ b/src/network_inspectors/perf_monitor/flow_tracker.cc @@ -158,8 +158,7 @@ public: TEST_CASE("no protocol", "[FlowTracker]") { Packet p; - DAQ_PktHdr_t pkth; - uint32_t* len_ptr = (uint32_t*) &pkth.caplen; + uint32_t* len_ptr = (uint32_t*) &p.pkth->caplen; PerfConfig config; config.format = PERF_MOCK; @@ -174,7 +173,6 @@ TEST_CASE("no protocol", "[FlowTracker]") p.ptrs.tcph = nullptr; p.ptrs.udph = nullptr; p.ptrs.icmph = nullptr; - p.pkth = &pkth; *len_ptr = 127; tracker.update(&p); @@ -204,9 +202,8 @@ TEST_CASE("no protocol", "[FlowTracker]") TEST_CASE("icmp", "[FlowTracker]") { Packet p; - DAQ_PktHdr_t pkth; icmp::ICMPHdr icmp; - uint32_t* len_ptr = (uint32_t*) &pkth.caplen; + uint32_t* len_ptr = (uint32_t*) &p.pkth->caplen; uint8_t* type_ptr = (uint8_t*) &icmp.type; PerfConfig config; @@ -222,7 +219,6 @@ TEST_CASE("icmp", "[FlowTracker]") p.ptrs.tcph = nullptr; p.ptrs.udph = nullptr; p.ptrs.icmph = &icmp; - p.pkth = &pkth; *len_ptr = 127; *type_ptr = 3; @@ -251,9 +247,8 @@ TEST_CASE("icmp", "[FlowTracker]") TEST_CASE("tcp", "[FlowTracker]") { Packet p; - DAQ_PktHdr_t pkth; tcp::TCPHdr tcp; - uint32_t* len_ptr = (uint32_t*) &pkth.caplen; + uint32_t* len_ptr = (uint32_t*) &p.pkth->caplen; PerfConfig config; config.format = PERF_MOCK; @@ -266,7 +261,6 @@ TEST_CASE("tcp", "[FlowTracker]") p.ptrs.tcph = &tcp; p.ptrs.udph = nullptr; p.ptrs.icmph = nullptr; - p.pkth = &pkth; tracker.reset(); @@ -307,9 +301,8 @@ TEST_CASE("tcp", "[FlowTracker]") TEST_CASE("udp", "[FlowTracker]") { Packet p; - DAQ_PktHdr_t pkth; udp::UDPHdr udp; - uint32_t* len_ptr = (uint32_t*) &pkth.caplen; + uint32_t* len_ptr = (uint32_t*) &p.pkth->caplen; PerfConfig config; config.format = PERF_MOCK; @@ -322,7 +315,6 @@ TEST_CASE("udp", "[FlowTracker]") p.ptrs.tcph = nullptr; p.ptrs.udph = &udp; p.ptrs.icmph = nullptr; - p.pkth = &pkth; tracker.reset(); diff --git a/src/network_inspectors/port_scan/port_scan.cc b/src/network_inspectors/port_scan/port_scan.cc index e7d6b2f18..532102274 100644 --- a/src/network_inspectors/port_scan/port_scan.cc +++ b/src/network_inspectors/port_scan/port_scan.cc @@ -875,7 +875,7 @@ bool PortScan::configure(SnortConfig* sc) void PortScan::tinit() { - g_tmp_pkt = PacketManager::encode_new(); + g_tmp_pkt = new Packet; ps_init_hash(config->common->memcap); if ( !config->logfile ) @@ -900,8 +900,8 @@ void PortScan::tterm() g_logfile = nullptr; } ps_cleanup(); - PacketManager::encode_delete(g_tmp_pkt); - g_tmp_pkt = NULL; + delete g_tmp_pkt; + g_tmp_pkt = nullptr; } void PortScan::show(SnortConfig*) diff --git a/src/protocols/packet.cc b/src/protocols/packet.cc index 80e9b9e9e..936d12ed8 100644 --- a/src/protocols/packet.cc +++ b/src/protocols/packet.cc @@ -23,45 +23,60 @@ #include "protocols/packet_manager.h" #include "protocols/protocol_ids.h" #include "log/obfuscator.h" +#include "log/messages.h" +#include "managers/codec_manager.h" +#include "utils/util.h" -#if 0 -uint8_t Packet::ip_proto_next() const +Packet::Packet(bool packet_data) { - if (is_ip4()) + layers = new Layer[CodecManager::get_max_layers()]; + allocated = packet_data; + + if (!packet_data) { - return ptrs.ip_api.get_ip4h()->proto(); + pkt = nullptr; + pkth = nullptr; } - else if (is_ip6()) + else { - const ip::IP6Hdr* const ip6h = ptrs.ip_api.get_ip6h(); - int lyr = num_layers-1; + uint8_t* b = new uint8_t[sizeof(*pkth) + Codec::PKT_MAX + SPARC_TWIDDLE]; + pkth = (DAQ_PktHdr_t*)b; + b += sizeof(*pkth); + b += SPARC_TWIDDLE; + pkt = b; + } - for (; lyr >= 0; lyr--) - if (layers[lyr].start == (const uint8_t*)(ip6h)) - break; + obfuscator = nullptr; -#if 0 - Since this packet 'is_ip6()', we ar gauranteed to find the layer - if (lyr < 0) - return IPPROTO_ID_RESERVED; -#endif + reset(); +} - while (lyr < num_layers) - { - const uint16_t prot = layers[lyr].prot_id; +Packet::~Packet() +{ + if (allocated) + delete[] (uint8_t*)pkth; + delete[] layers; +} - if (!is_ip_protocol(prot)) - return (uint8_t)prot; +void Packet::reset() +{ + if (obfuscator) + delete obfuscator; - ++lyr; - } - } + flow = nullptr; + endianness = nullptr; + obfuscator = nullptr; + packet_flags = 0; + xtradata_mask = 0; + proto_bits = 0; + alt_dsize = 0; + num_layers = 0; + ip_proto_next = IpProtocol::PROTO_NOT_SET; + disable_inspect = false; - return IPPROTO_ID_RESERVED; + ptrs.reset(); } -#endif - bool Packet::get_ip_proto_next(uint8_t& lyr, IpProtocol& proto) const { if (lyr > num_layers) @@ -184,11 +199,3 @@ const char* Packet::get_pseudo_type() const return "other"; } -void Packet::reset() -{ - if (obfuscator) - delete obfuscator; - - memset(this, 0, offsetof(Packet, pkth)); - ptrs.reset(); -} diff --git a/src/protocols/packet.h b/src/protocols/packet.h index 4f6cb076d..7b3d79a9f 100644 --- a/src/protocols/packet.h +++ b/src/protocols/packet.h @@ -125,7 +125,12 @@ constexpr uint8_t DEFAULT_LAYERMAX = 40; // payload data only, no headers. struct SO_PUBLIC Packet { + Packet(bool packet_data = true); + ~Packet(); + class Flow* flow; /* for session tracking */ + class Endianness* endianness; + class Obfuscator* obfuscator; uint32_t packet_flags; /* special flags for the packet */ uint32_t xtradata_mask; @@ -134,11 +139,9 @@ struct SO_PUBLIC Packet uint16_t alt_dsize; /* the dsize of a packet before munging (used for log)*/ uint8_t num_layers; /* index into layers for next encap */ + // FIXIT-M: Consider moving ip_proto_next below `pkth`. IpProtocol ip_proto_next; /* the protocol ID after IP and all IP6 extension */ bool disable_inspect; - class Endianness* endianness; - class Obfuscator* obfuscator; - // nothing after this point is zeroed ... // Everything beyond this point is set by PacketManager::decode() @@ -272,6 +275,9 @@ struct SO_PUBLIC Packet void set_application_protocol(int16_t ap) { if ( flow ) flow->ssn_state.application_protocol = ap; } + +private: + bool allocated; }; /* Macros to deal with sequence numbers - p810 TCP Illustrated vol 2 */ diff --git a/src/protocols/packet_manager.cc b/src/protocols/packet_manager.cc index cd6f32563..4ca968d40 100644 --- a/src/protocols/packet_manager.cc +++ b/src/protocols/packet_manager.cc @@ -119,52 +119,6 @@ void PacketManager::pop_teredo(Packet* p, RawData& raw) // Initialization and setup //------------------------------------------------------------------------- -Packet* PacketManager::encode_new(bool packet_data) -{ - Packet* p = (Packet*)SnortAlloc(sizeof(*p)); - Layer* lyr = new Layer[CodecManager::max_layers]; - - if ( !p || !lyr) - FatalError("encode_new() => Failed to allocate packet\n"); - - if (!packet_data) - { - p->pkt = nullptr; - p->pkth = nullptr; - } - else - { - uint8_t* b = (uint8_t*)SnortAlloc(sizeof(*p->pkth) + Codec::PKT_MAX + SPARC_TWIDDLE); - - if (!b) - FatalError("encode_new() => Failed to allocate packet\n"); - - p->pkth = (DAQ_PktHdr_t*)b; - b += sizeof(*p->pkth); - b += SPARC_TWIDDLE; - p->pkt = b; - } - - p->layers = lyr; - return p; -} - -void PacketManager::encode_delete(Packet* p) -{ - if (p) - { - if (p->pkth) - free((void*)p->pkth); - - if (p->layers) - delete[] p->layers; - - p->pkth = nullptr; - p->layers = nullptr; - free(p); - } -} - // Assertions required for this code to work // Look below inside main decode() loop for these static_asserts @@ -202,7 +156,6 @@ void PacketManager::decode( p->reset(); p->pkth = pkthdr; p->pkt = pkt; - p->ptrs.reset(); layer::set_packet_pointer(p); s_stats[total_processed]++; diff --git a/src/protocols/packet_manager.h b/src/protocols/packet_manager.h index 24d80b4d9..2d2ca3a7f 100644 --- a/src/protocols/packet_manager.h +++ b/src/protocols/packet_manager.h @@ -57,12 +57,6 @@ public: // decode this packet and set all relevent packet fields. static void decode(Packet*, const struct _daq_pkthdr*, const uint8_t*, bool cooked = false); - // allocate a Packet for later formatting (cloning) - static Packet* encode_new(bool allocate_packet_data = true); - - // release the allocated Packet - static void encode_delete(Packet*); - // when encoding, rather than copy the destination MAC address from the // inbound packet, manually set the MAC address. static void encode_set_dst_mac(uint8_t*); diff --git a/src/stream/ip/ip_defrag.cc b/src/stream/ip/ip_defrag.cc index 6b39689a8..0bfc74778 100644 --- a/src/stream/ip/ip_defrag.cc +++ b/src/stream/ip/ip_defrag.cc @@ -749,7 +749,7 @@ static void FragRebuild(FragTracker* ft, Packet* p) // XXX NOT YET IMPLEMENTED - debugging if (!defrag_pkts[encap_frag_cnt]) - defrag_pkts[encap_frag_cnt] = PacketManager::encode_new(); + defrag_pkts[encap_frag_cnt] = new Packet(); dpkt = defrag_pkts[encap_frag_cnt]; @@ -1076,7 +1076,7 @@ void Defrag::tinit() for (int i = 1; i < layers; i++) defrag_pkts[i] = nullptr; - defrag_pkts[0] = PacketManager::encode_new(); + defrag_pkts[0] = new Packet(); pkt_snaplen = SFDAQ::get_snap_len(); } @@ -1086,7 +1086,7 @@ void Defrag::tterm() { if (defrag_pkts[i] != nullptr) { - PacketManager::encode_delete(defrag_pkts[i]); + delete defrag_pkts[i]; defrag_pkts[i] = nullptr; } } diff --git a/src/stream/libtcp/tcp_stream_session.cc b/src/stream/libtcp/tcp_stream_session.cc index 78c5d58c0..47ff8d3f8 100644 --- a/src/stream/libtcp/tcp_stream_session.cc +++ b/src/stream/libtcp/tcp_stream_session.cc @@ -470,7 +470,7 @@ void TcpStreamSession::set_memcap(Memcap& mc) void TcpStreamSession::sinit(void) { - s5_pkt = PacketManager::encode_new(); + s5_pkt = new Packet(); //AtomSplitter::init(); // FIXIT-L PAF implement } @@ -478,7 +478,7 @@ void TcpStreamSession::sterm(void) { if (s5_pkt) { - PacketManager::encode_delete(s5_pkt); + delete s5_pkt; s5_pkt = nullptr; } } diff --git a/src/stream/user/user_session.cc b/src/stream/user/user_session.cc index cea4fbc1d..3cb4fc926 100644 --- a/src/stream/user/user_session.cc +++ b/src/stream/user/user_session.cc @@ -140,8 +140,7 @@ void UserTracker::term() void UserTracker::detect(const Packet* p, const StreamBuffer* sb, uint32_t flags) { - Packet up; - up.reset(); + Packet up(false); up.pkth = p->pkth; up.ptrs = p->ptrs;