From: Michael Altizer (mialtize) Date: Wed, 28 Nov 2018 21:24:27 +0000 (-0500) Subject: Merge pull request #1442 in SNORT/snort3 from ~CWAXMAN/snort3:offload_kill_stream... X-Git-Tag: 3.0.0-250~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c1f0b7fd50ac9b94c27d4c58b37f7a19366bc93a;p=thirdparty%2Fsnort3.git Merge pull request #1442 in SNORT/snort3 from ~CWAXMAN/snort3:offload_kill_stream to master Squashed commit of the following: commit 30faa7bb1f3f83b020ce7e5dd8d8c97b5d43f0e2 Author: Carter Waxman Date: Tue Nov 27 14:13:12 2018 -0500 regex worker: removed assert that didn't handle locks cleanly commit 2a72bde15e444742d268a04253ae017c40a6eae6 Author: Carter Waxman Date: Mon Nov 26 17:25:46 2018 -0500 detection, stream: fixed assuming packets were offloaded when previous packets on flow have been offloaded commit 5a03c7594249291950283c9a05e2a6e85a23dc95 Author: Carter Waxman Date: Wed Nov 21 14:11:02 2018 -0500 stream tcp: fixed sequence overlap handling when working with empty seglist commit 30c258f980faef8382623efac34aa44c6e1ff235 Author: Carter Waxman Date: Wed Nov 21 07:35:53 2018 -0500 stream tcp: fixed applying post-inspection operations to wrong rebuilt packet --- diff --git a/src/detection/context_switcher.cc b/src/detection/context_switcher.cc index 756e2a3eb..6c4b8bee3 100644 --- a/src/detection/context_switcher.cc +++ b/src/detection/context_switcher.cc @@ -70,6 +70,7 @@ IpsContext* ContextSwitcher::pop() return nullptr; IpsContext* c = idle.back(); + assert(!c->has_callbacks()); idle.pop_back(); return c; } @@ -80,6 +81,7 @@ void ContextSwitcher::start() assert(!idle.empty()); trace_logf(detection, TRACE_DETECTION_ENGINE, "(wire) %" PRIu64 " cs::start %u (i=%zu, b=%zu)\n", get_packet_number(), idle.back()->get_slot(), idle.size(), busy.size()); + assert(!idle.back()->has_callbacks()); busy.emplace_back(idle.back()); idle.pop_back(); @@ -94,6 +96,7 @@ void ContextSwitcher::stop() get_packet_number(), busy.back()->get_slot(), idle.size(), busy.size()); IpsContext* c = busy.back(); + assert(!c->has_callbacks()); c->clear_context_data(); idle.emplace_back(c); busy.back()->packet->active = nullptr; @@ -113,6 +116,8 @@ void ContextSwitcher::abort() idle.emplace_back(hold[i]); hold[i] = nullptr; + idle.back()->clear_callbacks(); + idle.back()->clear_context_data(); } } while ( !busy.empty() ) @@ -122,6 +127,8 @@ void ContextSwitcher::abort() idle.emplace_back(busy.back()); busy.pop_back(); + idle.back()->clear_callbacks(); + idle.back()->clear_context_data(); } } @@ -131,6 +138,7 @@ IpsContext* ContextSwitcher::interrupt() trace_logf(detection, TRACE_DETECTION_ENGINE, "%" PRIu64 " cs::interrupt %u (i=%zu, b=%zu)\n", idle.back()->packet_number, idle.back()->get_slot(), idle.size(), busy.size()); + assert(!idle.back()->has_callbacks()); busy.emplace_back(idle.back()); idle.pop_back(); return busy.back(); @@ -144,6 +152,7 @@ IpsContext* ContextSwitcher::complete() trace_logf(detection, TRACE_DETECTION_ENGINE, "%" PRIu64 " cs::complete %u (i=%zu, b=%zu)\n", c->packet_number, busy.back()->get_slot(), idle.size(), busy.size()); + assert(!c->has_callbacks()); c->clear_context_data(); idle.emplace_back(c); diff --git a/src/detection/detect.cc b/src/detection/detect.cc index 95434cef9..bb2c265a4 100644 --- a/src/detection/detect.cc +++ b/src/detection/detect.cc @@ -54,12 +54,14 @@ THREAD_LOCAL ProfileStats detectPerfStats; THREAD_LOCAL ProfileStats eventqPerfStats; THREAD_LOCAL ProfileStats rebuiltPacketPerfStats; -void snort_ignore(Packet*) { } +bool snort_ignore(Packet*) { return true; } -void snort_log(Packet* p) +bool snort_log(Packet* p) { pc.log_pkts++; EventManager::call_loggers(nullptr, p, nullptr, nullptr); + + return true; } void CallLogFuncs(Packet* p, ListHead* head, Event* event, const char* msg) diff --git a/src/detection/detect.h b/src/detection/detect.h index b76e069ae..9257569dd 100644 --- a/src/detection/detect.h +++ b/src/detection/detect.h @@ -39,8 +39,8 @@ extern THREAD_LOCAL snort::ProfileStats detectPerfStats; extern THREAD_LOCAL snort::ProfileStats rebuiltPacketPerfStats; // main loop hooks -void snort_ignore(snort::Packet*); -void snort_log(snort::Packet*); +bool snort_ignore(snort::Packet*); +bool snort_log(snort::Packet*); // alerts void CallLogFuncs(snort::Packet*, ListHead*, Event*, const char*); diff --git a/src/detection/detection_engine.cc b/src/detection/detection_engine.cc index 814e777ac..39889d91f 100644 --- a/src/detection/detection_engine.cc +++ b/src/detection/detection_engine.cc @@ -436,7 +436,7 @@ bool DetectionEngine::detect(Packet* p, bool offload_ok) return false; } -void DetectionEngine::inspect(Packet* p) +bool DetectionEngine::inspect(Packet* p) { bool inspected = false; { @@ -461,12 +461,13 @@ void DetectionEngine::inspect(Packet* p) if ( !all_disabled(p) ) { if ( detect(p, true) ) - return; // don't finish out offloaded packets + return false; // don't finish out offloaded packets } } finish_inspect_with_latency(p); } finish_inspect(p, inspected); + return true; } //-------------------------------------------------------------------------- diff --git a/src/detection/detection_engine.h b/src/detection/detection_engine.h index 87240592f..55de5bb38 100644 --- a/src/detection/detection_engine.h +++ b/src/detection/detection_engine.h @@ -82,7 +82,7 @@ public: static void clear_replacement(); static bool detect(Packet*, bool offload_ok = false); - static void inspect(Packet*); + static bool inspect(Packet*); static int queue_event(const struct OptTreeNode*); static int queue_event(unsigned gid, unsigned sid, Actions::Type = Actions::NONE); diff --git a/src/detection/ips_context.h b/src/detection/ips_context.h index 97a4bcbf8..49f4006f4 100644 --- a/src/detection/ips_context.h +++ b/src/detection/ips_context.h @@ -91,6 +91,9 @@ public: void clear_callbacks() { post_callbacks.clear(); } + bool has_callbacks() const + { return !post_callbacks.empty(); } + void post_detection(); public: diff --git a/src/detection/regex_offload.cc b/src/detection/regex_offload.cc index cf81b111f..571b1c7ad 100644 --- a/src/detection/regex_offload.cc +++ b/src/detection/regex_offload.cc @@ -104,10 +104,10 @@ void RegexOffload::worker(RegexRequest* req) if ( !req->offload ) continue; + } assert(req->packet); - assert(req->packet->flow->is_offloaded()); snort::SnortConfig::set_conf(req->packet->context->conf); // FIXIT-H reload issue fp_offload(req->packet); diff --git a/src/main/snort.cc b/src/main/snort.cc index 31445bb4d..0c954983c 100644 --- a/src/main/snort.cc +++ b/src/main/snort.cc @@ -194,7 +194,7 @@ static void register_profiles() // helpers //------------------------------------------------------------------------- -static void pass_pkts(Packet*) { } +static bool pass_pkts(Packet*) { return true; } static MainHook_f main_hook = pass_pkts; static void set_policy(Packet* p) @@ -877,14 +877,14 @@ void Snort::thread_term() delete[] s_data; } -void Snort::inspect(Packet* p) +bool Snort::inspect(Packet* p) { // Need to include this b/c call is outside the detect tree Profile detect_profile(detectPerfStats); Profile rebuilt_profile(rebuiltPacketPerfStats); DetectionEngine de; - main_hook(p); + return main_hook(p); } DAQ_Verdict Snort::process_packet( diff --git a/src/main/snort.h b/src/main/snort.h index 81969f5a4..d5aad8b40 100644 --- a/src/main/snort.h +++ b/src/main/snort.h @@ -34,7 +34,7 @@ class Flow; struct Packet; struct SnortConfig; -typedef void (* MainHook_f)(Packet*); +typedef bool (* MainHook_f)(Packet*); class TestPause { @@ -78,7 +78,7 @@ public: static DAQ_Verdict packet_callback(void*, const DAQ_PktHdr_t*, const uint8_t*); - static void inspect(Packet*); + static bool inspect(Packet*); static void set_main_hook(MainHook_f); static ContextSwitcher* get_switcher(); diff --git a/src/stream/tcp/segment_overlap_editor.h b/src/stream/tcp/segment_overlap_editor.h index 41f9fc98b..e2cdfd4ec 100644 --- a/src/stream/tcp/segment_overlap_editor.h +++ b/src/stream/tcp/segment_overlap_editor.h @@ -76,7 +76,6 @@ struct TcpReassemblerState { SegmentOverlapState sos; TcpStreamTracker* tracker; - snort::Packet* last_pdu = nullptr; uint32_t flush_count; // number of flushed queued segments uint32_t xtradata_mask; // extra data available to log bool server_side; diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc index 2149fe006..4ba25cc72 100644 --- a/src/stream/tcp/tcp_reassembler.cc +++ b/src/stream/tcp/tcp_reassembler.cc @@ -256,7 +256,6 @@ void TcpReassembler::purge_alerts(TcpReassemblerState& trs) Stream::log_extra_data(flow, trs.xtradata_mask, ai->event_id, ai->event_second); } trs.tracker->alert_count = 0; - last_pdu = nullptr; } int TcpReassembler::purge_to_seq(TcpReassemblerState& trs, uint32_t flush_seq) @@ -591,14 +590,16 @@ int TcpReassembler::_flush_to_seq( tcpStats.rebuilt_bytes += flushed_bytes; NoProfile exclude(s5TcpFlushPerfStats); - Snort::inspect(pdu); - if ( pdu->flow->is_offloaded() ) + if ( !Snort::inspect(pdu) ) last_pdu = pdu; + else + last_pdu = nullptr; } else { tcpStats.rebuilt_buffers++; // FIXIT-L this is not accurate + last_pdu = nullptr; } // FIXIT-L must check because above may clear trs.sos.session @@ -1014,6 +1015,7 @@ int32_t TcpReassembler::flush_pdu_ackd(TcpReassemblerState& trs, uint32_t* flags int TcpReassembler::flush_on_data_policy(TcpReassemblerState& trs, Packet* p) { uint32_t flushed = 0; + last_pdu = nullptr; switch ( trs.tracker->flush_policy ) { @@ -1056,7 +1058,8 @@ int TcpReassembler::flush_on_data_policy(TcpReassemblerState& trs, Packet* p) int TcpReassembler::flush_on_ack_policy(TcpReassemblerState& trs, Packet* p) { - uint32_t flushed = 0; + uint32_t flushed = 0; + last_pdu = nullptr; switch (trs.tracker->flush_policy) { @@ -1132,6 +1135,14 @@ void TcpReassembler::insert_segment_in_empty_seglist( return; } + if ( SEQ_GT(trs.sos.seglist_base_seq, seq + overlap) ) + { + overlap = trs.sos.seglist_base_seq- seq - overlap; + + if ( overlap >= tsd.get_seg_len() ) + return; + } + // BLOCK add new block to trs.sos.seglist containing data add_reassembly_segment( trs, tsd, tsd.get_seg_len(), overlap, 0, seq + overlap, nullptr);