Merge pull request #1442 in SNORT/snort3 from ~CWAXMAN/snort3:offload_kill_stream...

Squashed commit of the following:

commit 30faa7bb1f3f83b020ce7e5dd8d8c97b5d43f0e2
Author: Carter Waxman <cwaxman@cisco.com>
Date:   Tue Nov 27 14:13:12 2018 -0500

    regex worker: removed assert that didn't handle locks cleanly

commit 2a72bde15e444742d268a04253ae017c40a6eae6
Author: Carter Waxman <cwaxman@cisco.com>
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 <cwaxman@cisco.com>
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 <cwaxman@cisco.com>
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 756e2a3eb372361e839a309ea70923ada5fcdd60..6c4b8bee391a4f10770b80268f8c7a228f517912 100644 (file)
--- 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 95434cef9394ceaede9a09773236af2c9c2fa5c8..bb2c265a405a468b6fd54f16f252eb8b79287063 100644 (file)
--- 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 b76e069ae30ea3d7e5043ec4ba3efac16a35977a..9257569dd99375c9f374fca1a8b79b9e375787ae 100644 (file)
--- 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 814e777ac332c9defcb0e59a34528b1bd5e66033..39889d91f43908d8c5b0a79f5c62d7d70720e5bf 100644 (file)
--- 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 87240592f558cb2058166ced6b4294bd0f3cfd14..55de5bb38529a0e84b122494ee1feaa6d3478bda 100644 (file)
--- 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 97a4bcbf80543536d53fe09de44ff0cb41a964a1..49f4006f43e20fe470d7481b2b107726f09033ff 100644 (file)
--- 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 cf81b111ff336299449a0f5bf41a771c37ae4611..571b1c7adc7574262c4cb3ebf7633f940739712e 100644 (file)
--- 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 31445bb4dd6f50b0570d2e5782f461e8c7806d28..0c954983c30a2fde53aacc735711552f54f1c3b3 100644 (file)
--- 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 81969f5a4eb653fe923e995037204783bb748d8a..d5aad8b408f2dfadc2fef9b7e30af3a0eaea8a72 100644 (file)
--- 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 41f9fc98bd979b6887029e185ed6883bd419a6b0..e2cdfd4ec66438e7299ddf77d3456e7333cc68e1 100644 (file)
--- 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 2149fe006cd968a81a10156ceaa278786ceacae4..4ba25cc725b4afca299587eb9930be72f07a0de2 100644 (file)
--- 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);