From: Russ Combs <rucombs@cisco.com>
Date: Thu, 3 Nov 2016 00:38:40 +0000 (-0400)
Subject: properly instantiate prune / purge packet
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=c6e53b27566d45d868b41700c08f67d2bcb942da;p=thirdparty%2Fsnort3.git

properly instantiate prune / purge packet
---

diff --git a/src/codecs/ip/cd_ipv4.cc b/src/codecs/ip/cd_ipv4.cc
index 941d88c66..d38ab0d00 100644
--- a/src/codecs/ip/cd_ipv4.cc
+++ b/src/codecs/ip/cd_ipv4.cc
@@ -185,6 +185,8 @@ bool Ipv4Codec::decode(const RawData& raw, CodecData& codec, DecodeData& snort)
             ip_len - raw.len, ip_len, raw.len);
 
         codec_event(codec, DECODE_IPV4_DGRAM_GT_CAPLEN);
+        // FIXIT-L we should decode this layer if possible instead of stopping now
+        // ip6 etc may have similar issues
         return false;
     }
 #if 0
diff --git a/src/detection/detection_engine.cc b/src/detection/detection_engine.cc
index a7961aafc..63c0ca86d 100644
--- a/src/detection/detection_engine.cc
+++ b/src/detection/detection_engine.cc
@@ -47,7 +47,6 @@
 
 Trace TRACE_NAME(detection);
 
-static THREAD_LOCAL unsigned s_events = 0;
 static THREAD_LOCAL Ring<unsigned>* offload_ids = nullptr;
 
 void DetectionEngine::thread_init()
@@ -383,7 +382,6 @@ int DetectionEngine::queue_event(const OptTreeNode* otn)
     if ( sfeventq_add(pq, en) )
         return -1;
 
-    s_events++;
     return 0;
 }
 
@@ -407,7 +405,6 @@ int DetectionEngine::queue_event(unsigned gid, unsigned sid, RuleType type)
     if ( sfeventq_add(pq, en) )
         return -1;
 
-    s_events++;
     return 0;
 }
 
@@ -426,9 +423,6 @@ static int log_events(void* event, void* user)
             return 0;  // not enabled
     }
 
-    if ( s_events > 0 )
-        s_events--;
-
     fpLogEvent(en->rtn, en->otn, (Packet*)user);
     sfthreshold_reset();
 
@@ -449,16 +443,9 @@ int DetectionEngine::log_events(Packet* p)
     return 0;
 }
 
-void DetectionEngine::reset_counts()
-{
-    pc.log_limit += s_events;
-    s_events = 0;
-}
-
 void DetectionEngine::reset(Packet* p)
 {
     SF_EVENTQ* pq = p->context->equeue;
-    sfeventq_reset(pq);
-    reset_counts();
+    pc.log_limit += sfeventq_reset(pq);
 }
 
diff --git a/src/detection/detection_engine.h b/src/detection/detection_engine.h
index 03c8fad77..f6111a311 100644
--- a/src/detection/detection_engine.h
+++ b/src/detection/detection_engine.h
@@ -82,9 +82,7 @@ public:
     static int queue_event(unsigned gid, unsigned sid, RuleType = RULE_TYPE__NONE);
 
     static int log_events(Packet*);
-
     static void reset(Packet*);
-    static void reset_counts();
 
     static void disable_all(Packet*);
     static bool all_disabled(Packet*);
diff --git a/src/detection/fp_detect.cc b/src/detection/fp_detect.cc
index c543f5981..0f584638f 100644
--- a/src/detection/fp_detect.cc
+++ b/src/detection/fp_detect.cc
@@ -176,8 +176,7 @@ int fpLogEvent(const RuleTreeNode* rtn, const OptTreeNode* otn, Packet* p)
     if ( p->ptrs.ip_api.is_valid() )
     {
         filterEvent = sfthreshold_test(
-            otn->sigInfo.generator,
-            otn->sigInfo.id,
+            otn->sigInfo.generator, otn->sigInfo.id,
             p->ptrs.ip_api.get_src(), p->ptrs.ip_api.get_dst(),
             p->pkth->ts.tv_sec);
     }
@@ -187,10 +186,8 @@ int fpLogEvent(const RuleTreeNode* rtn, const OptTreeNode* otn, Packet* p)
         cleared.clear();
 
         filterEvent = sfthreshold_test(
-            otn->sigInfo.generator,
-            otn->sigInfo.id,
-            &cleared, &cleared,
-            p->pkth->ts.tv_sec);
+            otn->sigInfo.generator, otn->sigInfo.id,
+            &cleared, &cleared, p->pkth->ts.tv_sec);
     }
 
     if ( (filterEvent < 0) || (filterEvent > 0 && !override) )
diff --git a/src/events/sfeventq.cc b/src/events/sfeventq.cc
index 664cad448..155ec6fe6 100644
--- a/src/events/sfeventq.cc
+++ b/src/events/sfeventq.cc
@@ -53,30 +53,20 @@
 
 #include "sfeventq.h"
 
+#include <assert.h>
 #include "utils/util.h"
 
 /*
-**  NAME
-**    sfeventq_new::
-*/
-/**
 **  Initialize the event queue.  Provide the max number of nodes that this
 **  queue will support, the number of top nodes to log in the queue, and the
 **  size of the event structure that the user will fill in.
-**
-**  @return integer
-**
-**  @retval -1 failure
-**  @retval  0 success
 */
 SF_EVENTQ* sfeventq_new(int max_nodes, int log_nodes, int event_size)
 {
-    SF_EVENTQ* eq;
-
     if ((max_nodes <= 0) || (log_nodes <= 0) || (event_size <= 0))
         return NULL;
 
-    eq = (SF_EVENTQ*)snort_calloc(sizeof(SF_EVENTQ));
+    SF_EVENTQ* eq = (SF_EVENTQ*)snort_calloc(sizeof(SF_EVENTQ));
 
     /* Initialize the memory for the nodes that we are going to use. */
     eq->node_mem = (SF_EVENTQ_NODE*)snort_calloc(max_nodes, sizeof(SF_EVENTQ_NODE));
@@ -87,16 +77,14 @@ SF_EVENTQ* sfeventq_new(int max_nodes, int log_nodes, int event_size)
     eq->event_size = event_size;
     eq->cur_nodes = 0;
     eq->cur_events = 0;
+    eq->fails = 0;
+
     eq->reserve_event = (char*)(&eq->event_mem[max_nodes * eq->event_size]);
 
     return eq;
 }
 
 /*
-**  NAME
-**    sfeventq_event_alloc::
-*/
-/**
 **  Allocate the memory for an event to add to the event queue.  This
 **  function is meant to be called first, the event structure filled in,
 **  and then added to the queue.  While you can allocate several times before
@@ -131,33 +119,20 @@ void* sfeventq_event_alloc(SF_EVENTQ* eq)
 }
 
 /*
-**  NAME
-**    sfeventq_reset::
-*/
-/**
 **  Resets the event queue.  We also set the reserve event back
 **  to the last event in the queue.
-**
-**  @return void
 */
-void sfeventq_reset(SF_EVENTQ* eq)
+unsigned sfeventq_reset(SF_EVENTQ* eq)
 {
+    unsigned fails = eq->fails;
+    eq->fails = 0;
     eq->head = NULL;
     eq->cur_nodes = 0;
     eq->cur_events = 0;
     eq->reserve_event = (char*)(&eq->event_mem[eq->max_nodes * eq->event_size]);
+    return fails;
 }
 
-/*
-**  NAME
-**    sfeventq_free::
-*/
-/**
-**  Cleanup the event queue.
-**
-**  @return none
-**
-*/
 void sfeventq_free(SF_EVENTQ* eq)
 {
     if (eq == NULL)
@@ -180,10 +155,6 @@ void sfeventq_free(SF_EVENTQ* eq)
 }
 
 /*
-**  NAME
-**    get_eventq_node::
-*/
-/**
 **  This function returns a ptr to the node to use.  We allocate the last
 **  event node if we have exhausted the event queue.  Before we allocate
 **  the last node, we determine if the incoming event has a higher
@@ -204,17 +175,11 @@ static SF_EVENTQ_NODE* get_eventq_node(SF_EVENTQ* eq, void*)
     if (eq->cur_nodes >= eq->max_nodes)
         return NULL;
 
-    /*
-    **  We grab the next node from the node memory.
-    */
+    //  We grab the next node from the node memory.
     return &eq->node_mem[eq->cur_nodes++];
 }
 
 /*
-**  NAME
-**    sfeventq_add:
-*/
-/**
 **  Add this event to the queue using the supplied ordering
 **  function.  If the queue is exhausted, then we compare the
 **  event to be added with the last event, and decide whether
@@ -227,10 +192,7 @@ static SF_EVENTQ_NODE* get_eventq_node(SF_EVENTQ* eq, void*)
 */
 int sfeventq_add(SF_EVENTQ* eq, void* event)
 {
-    SF_EVENTQ_NODE* node;
-
-    if (!event)
-        return -1;
+    assert(event);
 
     /*
     **  If get_eventq_node() returns NULL, this means that
@@ -238,26 +200,26 @@ int sfeventq_add(SF_EVENTQ* eq, void* event)
     **  is lower in priority then the last ranked event.
     **  So we just drop it.
     */
-    node = get_eventq_node(eq, event);
-    if (!node)
+    SF_EVENTQ_NODE* node = get_eventq_node(eq, event);
+
+    if ( !node )
+    {
+        ++eq->fails;
         return -1;
+    }
 
     node->event = event;
     node->next  = NULL;
     node->prev  = NULL;
 
-    /*
-    **  This is the first node
-    */
     if (eq->cur_nodes == 1)
     {
+        //  This is the first node
         eq->head = eq->last = node;
         return 0;
     }
 
-    /*
-    **  This means we are the last node.
-    */
+    //  This means we are the last node.
     node->prev = eq->last;
 
     eq->last->next = node;
@@ -267,10 +229,6 @@ int sfeventq_add(SF_EVENTQ* eq, void* event)
 }
 
 /*
-**  NAME
-**    sfeventq_action::
-*/
-/**
 **  Call the supplied user action function on the highest priority
 **  events.
 **
diff --git a/src/events/sfeventq.h b/src/events/sfeventq.h
index 4efd40e41..46f7388f2 100644
--- a/src/events/sfeventq.h
+++ b/src/events/sfeventq.h
@@ -61,11 +61,12 @@ struct SF_EVENTQ
     */
     int cur_nodes;
     int cur_events;
+    unsigned fails;
 };
 
 SF_EVENTQ* sfeventq_new(int max_nodes, int log_nodes, int event_size);
 void* sfeventq_event_alloc(SF_EVENTQ*);
-void sfeventq_reset(SF_EVENTQ*);
+unsigned sfeventq_reset(SF_EVENTQ*);  // returns fail count since last reset
 int sfeventq_add(SF_EVENTQ*, void* event);
 int sfeventq_action(SF_EVENTQ*, int (* action_func)(void* event, void* user), void* user);
 void sfeventq_free(SF_EVENTQ*);
diff --git a/src/flow/flow.cc b/src/flow/flow.cc
index c1a3e6ee6..7dc1b3e46 100644
--- a/src/flow/flow.cc
+++ b/src/flow/flow.cc
@@ -23,6 +23,7 @@
 
 #include "flow.h"
 
+#include "detection/detection_engine.h"
 #include "flow/ha.h"
 #include "flow/session.h"
 #include "ips_options/ips_flowbits.h"
@@ -125,6 +126,10 @@ inline void Flow::clean()
 
 void Flow::reset(bool do_cleanup)
 {
+    DetectionEngine::onload(this);
+    DetectionEngine::set_packet();
+    DetectionEngine de;
+
     if ( session )
     {
         if ( do_cleanup )
diff --git a/src/flow/flow_cache.cc b/src/flow/flow_cache.cc
index 1d2782916..139325007 100644
--- a/src/flow/flow_cache.cc
+++ b/src/flow/flow_cache.cc
@@ -24,7 +24,6 @@
 
 #include "flow/flow_cache.h"
 
-#include "detection/detection_engine.h"
 #include "flow/ha.h"
 #include "hash/zhash.h"
 #include "helpers/flag_context.h"
@@ -149,7 +148,6 @@ Flow* FlowCache::get(const FlowKey* key)
 
 int FlowCache::release(Flow* flow, PruneReason reason, bool do_cleanup)
 {
-    DetectionEngine::onload(flow);
     flow->reset(do_cleanup);
     prune_stats.update(reason);
     return remove(flow);
diff --git a/src/flow/session.h b/src/flow/session.h
index 0a935292e..c0bacb0b5 100644
--- a/src/flow/session.h
+++ b/src/flow/session.h
@@ -41,7 +41,7 @@ public:
     virtual void restart(Packet*) { }
     virtual void precheck(Packet*) { }
     virtual void clear() = 0;
-    virtual void cleanup() { clear(); }
+    virtual void cleanup(Packet* = nullptr) { clear(); }
 
     virtual bool add_alert(Packet*, uint32_t /*gid*/, uint32_t /*sid*/) { return false; }
     virtual bool check_alerted(Packet*, uint32_t /*gid*/, uint32_t /*sid*/) { return false; }
diff --git a/src/service_inspectors/http_inspect/http_event_gen.h b/src/service_inspectors/http_inspect/http_event_gen.h
index 0c93aeded..bf886d61e 100644
--- a/src/service_inspectors/http_inspect/http_event_gen.h
+++ b/src/service_inspectors/http_inspect/http_event_gen.h
@@ -38,6 +38,7 @@ class HttpEventGen
 public:
     virtual ~HttpEventGen() = default;
     void reset() { events_generated = 0; }
+
     virtual void create_event(HttpEnums::EventSid sid)
     {
         assert(((int)sid > 0) && ((int)sid <= MAX));
diff --git a/src/stream/base/stream_base.cc b/src/stream/base/stream_base.cc
index 41b38a53c..f0da55aa9 100644
--- a/src/stream/base/stream_base.cc
+++ b/src/stream/base/stream_base.cc
@@ -22,9 +22,9 @@
 
 #include "flow/flow_control.h"
 #include "flow/prune_stats.h"
-#include "protocols/packet.h"
 #include "managers/inspector_manager.h"
 #include "profiler/profiler_defs.h"
+#include "protocols/packet.h"
 
 #include "stream_ha.h"
 #include "stream_module.h"
diff --git a/src/stream/libtcp/tcp_stream_session.cc b/src/stream/libtcp/tcp_stream_session.cc
index 683dd9d7b..e71d9d81d 100644
--- a/src/stream/libtcp/tcp_stream_session.cc
+++ b/src/stream/libtcp/tcp_stream_session.cc
@@ -430,9 +430,9 @@ bool TcpStreamSession::setup(Packet*)
     return true;
 }
 
-void TcpStreamSession::cleanup()
+void TcpStreamSession::cleanup(Packet* p)
 {
-    clear_session( true, true, false );
+    clear_session(true, true, false, p);
 }
 
 void TcpStreamSession::clear()
diff --git a/src/stream/libtcp/tcp_stream_session.h b/src/stream/libtcp/tcp_stream_session.h
index a008d518c..e0f713a31 100644
--- a/src/stream/libtcp/tcp_stream_session.h
+++ b/src/stream/libtcp/tcp_stream_session.h
@@ -46,7 +46,7 @@ public:
 
     bool setup(Packet*) override;
     void clear() override;
-    void cleanup() override;
+    void cleanup(Packet* = nullptr) override;
     void set_splitter(bool, StreamSplitter*) override;
     StreamSplitter* get_splitter(bool) override;
     bool is_sequenced(uint8_t /*dir*/) override;
diff --git a/src/stream/stream.cc b/src/stream/stream.cc
index 164c4d64e..5a0974d02 100644
--- a/src/stream/stream.cc
+++ b/src/stream/stream.cc
@@ -178,6 +178,7 @@ void Stream::check_flow_closed(Packet* p)
     if (flow->session_state & STREAM_STATE_CLOSED)
     {
         assert(flow_con);
+        flow->session->cleanup(p);
         flow_con->delete_flow(flow, PruneReason::NONE);
         p->flow = nullptr;
     }
@@ -350,15 +351,6 @@ void Stream::purge_flows()
     if ( !flow_con )
         return;
 
-    // FIXIT-H stream tcp needs to do this and prep pkt to handle
-    // shutdown alerts while rebuilding (during flush before a 
-    // rebuilt packet is available)
-    DetectionEngine::set_packet();
-    DetectionEngine de;
-    // this is a hack to work around the above issue
-    DAQ_PktHdr_t* ph = (DAQ_PktHdr_t*)de.get_packet()->pkth;
-    memset(ph, 0, sizeof(*ph));
-
     flow_con->purge_flows(PktType::IP);
     flow_con->purge_flows(PktType::ICMP);
     flow_con->purge_flows(PktType::TCP);
@@ -369,15 +361,19 @@ void Stream::purge_flows()
 
 void Stream::timeout_flows(time_t cur_time)
 {
-    if ( flow_con )
-        // FIXIT-M batch here or loop vs looping over idle?
-        flow_con->timeout_flows(cur_time);
+    if ( !flow_con )
+        return;
+
+    // FIXIT-M batch here or loop vs looping over idle?
+    flow_con->timeout_flows(cur_time);
 }
 
 void Stream::prune_flows()
 {
-    if ( flow_con )
-        flow_con->prune_one(PruneReason::MEMCAP, false);
+    if ( !flow_con )
+        return;
+
+    flow_con->prune_one(PruneReason::MEMCAP, false);
 }
 
 bool Stream::expected_flow(Flow* f, Packet* p)
diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc
index d76a57033..697fc753d 100644
--- a/src/stream/tcp/tcp_reassembler.cc
+++ b/src/stream/tcp/tcp_reassembler.cc
@@ -31,6 +31,7 @@
 #include "profiler/profiler.h"
 #include "detection/detection_engine.h"
 #include "protocols/packet_manager.h"
+#include "time/packet_time.h"
 
 #include "tcp_module.h"
 #include "tcp_normalizer.h"
@@ -838,11 +839,8 @@ int TcpReassembler::flush_stream(Packet* p, uint32_t dir)
     return flush_to_seq(bytes, p, dir);
 }
 
-void TcpReassembler::final_flush(Packet* p, PegCount& peg, uint32_t dir)
+void TcpReassembler::final_flush(Packet* p, uint32_t dir)
 {
-    if ( !p )
-        peg++;
-
     tracker->set_tf_flags(TF_FORCE_FLUSH);
 
     if ( flush_stream(p, dir) )
@@ -851,17 +849,56 @@ void TcpReassembler::final_flush(Packet* p, PegCount& peg, uint32_t dir)
     tracker->clear_tf_flags(TF_FORCE_FLUSH);
 }
 
+static Packet* set_packet(Flow* flow, uint32_t flags, bool c2s)
+{
+    Packet* p = DetectionEngine::get_current_packet();
+    p->reset();
+
+    DAQ_PktHdr_t* ph = (DAQ_PktHdr_t*)p->pkth;
+    memset(ph, 0, sizeof(*ph));
+    packet_gettimeofday(&ph->ts);
+
+    p->ptrs.set_pkt_type(PktType::PDU);
+    p->proto_bits |= PROTO_BIT__TCP;
+    p->flow = flow;
+    p->packet_flags = flags;
+
+    if ( c2s )
+    {
+        p->ptrs.ip_api.set(flow->client_ip, flow->server_ip);
+        p->ptrs.sp = flow->client_port;
+        p->ptrs.dp = flow->server_port;
+    }
+    else
+    {
+        p->ptrs.ip_api.set(flow->server_ip, flow->client_ip);
+        p->ptrs.sp = flow->server_port;
+        p->ptrs.dp = flow->client_port;
+    }
+    return p;
+}
+
 void TcpReassembler::flush_queued_segments(Flow* flow, bool clear, Packet* p)
 {
+    bool data = p or seglist.head;
+
+    if ( !p )
+    {
+        // this packet is required if we call finish and/or final_flush
+        p = set_packet(flow, packet_dir, server_side);
+
+        if ( server_side )
+            tcpStats.s5tcp2++;
+        else
+            tcpStats.s5tcp1++;
+    }
+
     bool pending = clear and paf_initialized(&tracker->paf_state)
         and (!tracker->splitter or tracker->splitter->finish(flow) );
 
-    if ((pending and (p or seglist.head) and !(flow->ssn_state.ignore_direction & ignore_dir)))
+    if ( pending and data and !(flow->ssn_state.ignore_direction & ignore_dir) )
     {
-        if (server_side)
-            final_flush(p, tcpStats.s5tcp2, packet_dir);
-        else
-            final_flush(p, tcpStats.s5tcp1, packet_dir);
+        final_flush(p, packet_dir);
     }
 }
 
diff --git a/src/stream/tcp/tcp_reassembler.h b/src/stream/tcp/tcp_reassembler.h
index 837fd1f42..0bd9d3d76 100644
--- a/src/stream/tcp/tcp_reassembler.h
+++ b/src/stream/tcp/tcp_reassembler.h
@@ -147,7 +147,7 @@ protected:
     int flush_to_seq(uint32_t bytes, Packet*, uint32_t pkt_flags);
     uint32_t get_q_footprint();
     uint32_t get_q_sequenced();
-    void final_flush(Packet*, PegCount&, uint32_t dir);
+    void final_flush(Packet*, uint32_t dir);
     uint32_t get_reverse_packet_dir(const Packet*);
     uint32_t get_forward_packet_dir(const Packet*);
     int32_t flush_pdu_ips(uint32_t*);
diff --git a/src/stream/tcp/test/tcp_normalizer_test.cc b/src/stream/tcp/test/tcp_normalizer_test.cc
index d3ed70b98..c4ec32dc8 100644
--- a/src/stream/tcp/test/tcp_normalizer_test.cc
+++ b/src/stream/tcp/test/tcp_normalizer_test.cc
@@ -54,7 +54,7 @@ int TcpSession::process(Packet*){ return 0; }
 void TcpSession::restart(Packet*){ }
 void TcpSession::precheck(Packet*){ }
 void TcpSession::clear(){ }
-void TcpSession::cleanup(){ }
+void TcpSession::cleanup(Packet* = nullptr){ }
 bool TcpSession::add_alert(Packet*, unsigned int, unsigned int){ return true; }
 bool TcpSession::check_alerted(Packet*, unsigned int, unsigned int){ return true; }
 int TcpSession::update_alert(Packet*, unsigned int, unsigned int, unsigned int, unsigned int){ return 0; }
diff --git a/src/utils/stats.cc b/src/utils/stats.cc
index 1ed38c2d8..74c7e5c20 100644
--- a/src/utils/stats.cc
+++ b/src/utils/stats.cc
@@ -284,9 +284,6 @@ void DropStats()
 
     PacketManager::dump_stats();
 
-    // ensure proper counting of log_limit
-    DetectionEngine::reset_counts();
-
     LogLabel("Module Statistics");
     const char* exclude = "daq snort";
     ModuleManager::dump_stats(snort_conf, exclude);