From: Davis McPherson (davmcphe) <davmcphe@cisco.com>
Date: Fri, 4 Dec 2020 15:10:31 +0000 (+0000)
Subject: Merge pull request #2609 in SNORT/snort3 from ~DAVMCPHE/snort3:stream_ha_deactive... 
X-Git-Tag: 3.0.3-6~22
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=e608b695671d1f62db745c78780b72a5e075976b;p=thirdparty%2Fsnort3.git

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

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 23a64d425..6e9bcd160 100644
--- 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 edc32c4a9..1b048514c 100644
--- 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 590eb0478..ff5f3a8ab 100644
--- 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 7d622cf98..0eaa8e07f 100644
--- 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 )