Merge pull request #2609 in SNORT/snort3 from ~DAVMCPHE/snort3:stream_ha_deactive...

Squashed commit of the following:

commit e5fe144e3e7b55dd493680d3730ed31664776083
Author: davis mcpherson <davmcphe@cisco.com>
Date:   Tue Nov 10 09:49:42 2020 -0500

    stream_ha: only flush on ha deactivate if not in STANDBY, set ha state to STANDBY when new Flow created

diff --git a/src/stream/base/stream_ha.cc b/src/stream/base/stream_ha.cc
index 23a64d425402f199f2548155b02e7c6118bf741e..6e9bcd160013c8a7b28a2e516cd6387762169664 100644 (file)
--- a/src/stream/base/stream_ha.cc
+++ b/src/stream/base/stream_ha.cc
@@ -107,6 +107,7 @@ bool StreamHAClient::consume(Flow*& flow, const FlowKey* key, HAMessage& msg, ui
         DataBus::publish(STREAM_HA_NEW_FLOW_EVENT, event, flow);
 
         flow->ha_state->clear(FlowHAState::NEW);
+        flow->ha_state->add(FlowHAState::STANDBY);
         if ( hac->flags & SessionHAContent::FLAG_LOW )
         {
             flow->server_ip.set(flow->key->ip_l);

diff --git a/src/stream/tcp/tcp_ha.cc b/src/stream/tcp/tcp_ha.cc
index edc32c4a9bc3f1a1e44b82c85c39c568f733bbc9..1b048514ca8397384d43ead07eadb49897458655 100644 (file)
--- a/src/stream/tcp/tcp_ha.cc
+++ b/src/stream/tcp/tcp_ha.cc
@@ -48,7 +48,10 @@ void TcpHA::deactivate_session(Flow* flow)
 {
     assert( flow );
     if ( flow->session )
-        ((TcpSession*)(flow->session))->clear_session(true, true, false);
+    {
+        flow->flush(true);
+        ((TcpSession*)(flow->session))->clear_session(true, false, false);
+    }
 
     flow->clear_session_state(STREAM_STATE_SYN | STREAM_STATE_SYN_ACK |
         STREAM_STATE_ACK | STREAM_STATE_ESTABLISHED);

diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc
index 590eb047859b3164a7fb28a3c51346666c4d6597..ff5f3a8abbd67f679c3fa4265dc52679cb15dfcc 100644 (file)
--- a/src/stream/tcp/tcp_reassembler.cc
+++ b/src/stream/tcp/tcp_reassembler.cc
@@ -797,14 +797,14 @@ void TcpReassembler::final_flush(TcpReassemblerState& trs, Packet* p, uint32_t d
 
 static Packet* set_packet(Flow* flow, uint32_t flags, bool c2s)
 {
-    // FIXIT-M this implicitly relies on a fresh packet/context being pushed by Flow::reset()
-    //   calling DetectionEngine::set_next_packet() while passing a null Packet through the
-    //   cleanup routines, which is super hinky, but also why we don't need to call p->reset().
-    // The end result is a skeleton of a TCP PDU packet with no data and the IPs/ports/flow set.
-    //   We should probably be clearing more Packet fields.
+    // if not in the context of a wire packet the flush initiator must have
+    // created a packet context by calling DetectionEngine::set_next_packet()
     Packet* p = DetectionEngine::get_current_packet();
-
     assert(p->pkth == p->context->pkth);
+
+    // FIXIT-M p points to a skeleton of a TCP PDU packet with no data and we now
+    // initialize the IPs/ports/flow and other fields accessed as we reassemble
+    // and flush the PDU. There are probably other Packet fields that should be set here...
     DAQ_PktHdr_t* ph = p->context->pkth;
     memset(ph, 0, sizeof(*ph));
     packet_gettimeofday(&ph->ts);

diff --git a/src/stream/tcp/tcp_session.cc b/src/stream/tcp/tcp_session.cc
index 7d622cf98bfefb216c3c4b86b76a57cdab6e76de..0eaa8e07f4909ffd0bfa2d6aa810a1bc7e0844aa 100644 (file)
--- a/src/stream/tcp/tcp_session.cc
+++ b/src/stream/tcp/tcp_session.cc
@@ -152,19 +152,9 @@ void TcpSession::restart(Packet* p)
     tcpStats.restarts++;
 }
 
-//-------------------------------------------------------------------------
-// when client ports are configured, that means c2s and is stored on the
-// client side; when the session starts, the server policy is obtained from
-// the client side because segments are stored on the receiving side.
-//
-// this could be improved further by storing the c2s policy on the server
-// side and then obtaining server policy from the server on session
-// startup.
-//
-// either way, this client / server distinction must be kept in mind to
-// make sense of the code in this file.
-//-------------------------------------------------------------------------
-
+// if the flush_segments parameter is true and clear_session is being called while not in
+// the context of a wire packet then the caller must create a packet context by calling
+// DetectionEngine::set_next_packet() before calling clear_session
 void TcpSession::clear_session(bool free_flow_data, bool flush_segments, bool restart, Packet* p)
 {
     assert(!p or p->flow == flow);
@@ -911,6 +901,8 @@ void TcpSession::flush_talker(Packet* p, bool final_flush)
         flush_tracker( client, p, PKT_FROM_SERVER, final_flush);
 }
 
+// if not in the context of a wire packet the caller must create a packet context
+// by calling DetectionEngine::set_next_packet() before calling TcpSession::flush()
 void TcpSession::flush()
 {
     if ( !tcp_init )