Pull request #3167: Fixes for abort issues

Merge in SNORT/snort3 from ~KATHARVE/snort3:abort_issues to master

Squashed commit of the following:

commit 3a43d1e4887d820be2886edaa3185a5c8975fa5d
Author: Katura Harvey <katharve@cisco.com>
Date:   Mon Nov 15 11:32:41 2021 -0500

    http_inspect: update comments for asserts in eval and clear

commit 3ccf3b7e0f9c4b453f56015b52aeb16c1ed747c0
Author: Katura Harvey <katharve@cisco.com>
Date:   Mon Nov 15 11:27:37 2021 -0500

    stream_tcp: only fallback if stream splitter aborted and don't keep processing fragments after MagicSplitter returned STOP

commit 6731a11f9bf7b5de9c5e348d0f1311dd6a376ba9
Author: Katura Harvey <katharve@cisco.com>
Date:   Wed Oct 27 20:05:38 2021 -0400

    framework: don't call a gadget's eval() or clear()  after its stream splitter aborted

commit 3c60508fca0b13f14f55632b35d1ca84ea134e57
Author: Katura Harvey <katharve@cisco.com>
Date:   Mon Nov 15 11:22:41 2021 -0500

    http_inspect: fix total_bytes peg count

diff --git a/src/flow/flow.cc b/src/flow/flow.cc
index 307f6b258071c5b34d64ce7ee58e6742ce21c42b..58e4e0c1945faba465a74164a1679f63ddeb5dc7 100644 (file)
--- a/src/flow/flow.cc
+++ b/src/flow/flow.cc
@@ -546,6 +546,16 @@ bool Flow::is_pdu_inorder(uint8_t dir)
             && !(ssn_state.session_flags & SSNFLAG_MIDSTREAM));
 }
 
+bool Flow::is_direction_aborted(bool from_client) const
+{
+    const uint32_t session_flags = get_session_flags();
+
+    if (from_client)
+        return (session_flags & SSNFLAG_ABORT_SERVER);
+
+    return (session_flags & SSNFLAG_ABORT_CLIENT);
+}
+
 void Flow::set_service(Packet* pkt, const char* new_service)
 {
     service = new_service;

diff --git a/src/flow/flow.h b/src/flow/flow.h
index 2d248fa4d90cb613cea3d3f366eda23d807e764d..93b5639774ffcfaa2492034f28045b88f0ad630c 100644 (file)
--- a/src/flow/flow.h
+++ b/src/flow/flow.h
@@ -229,7 +229,7 @@ public:
     uint32_t set_session_flags(uint32_t ssn_flags)
     { return ssn_state.session_flags |= ssn_flags; }
 
-    uint32_t get_session_flags()
+    uint32_t get_session_flags() const
     { return ssn_state.session_flags; }
 
     uint32_t clear_session_flags(uint32_t ssn_flags)
@@ -255,6 +255,8 @@ public:
 
     bool is_pdu_inorder(uint8_t dir);
 
+    bool is_direction_aborted(bool from_client) const;
+
     void set_proxied()
     { ssn_state.session_flags |= SSNFLAG_PROXIED; }
 

diff --git a/src/framework/inspector.cc b/src/framework/inspector.cc
index 3e232707e766466e0d7e188c7ffb014f63d12e20..4c9f0b1143d8ba52f8c1e41177c8c30059ec4b44 100644 (file)
--- a/src/framework/inspector.cc
+++ b/src/framework/inspector.cc
@@ -104,6 +104,9 @@ bool Inspector::likes(Packet* p)
     if ( !(BIT((uint16_t)p->type()) & api->proto_bits) )
         return false;
 
+    if ( p->flow and p->flow->is_direction_aborted(p->is_from_client()) )
+        return false;
+
     if ( p->is_tcp() && api->type == IT_SERVICE )
         return p->has_paf_payload();
 

diff --git a/src/protocols/packet.cc b/src/protocols/packet.cc
index 21885a4e31a9a4182e6b52ae3ab12eea8152f755..e5106834257646f41110e47b86480652d0f975f2 100644 (file)
--- a/src/protocols/packet.cc
+++ b/src/protocols/packet.cc
@@ -224,7 +224,7 @@ const char* Packet::get_pseudo_type() const
 }
 
 // Things that are set prior to PDU creation and used after PDU creation
-static inline uint32_t get_session_flags(Packet& p)
+static inline uint32_t get_session_flags(const Packet& p)
 {
     if ( p.ptrs.get_pkt_type() == PktType::PDU )
         return p.context->get_session_flags();

diff --git a/src/service_inspectors/http_inspect/http_inspect.cc b/src/service_inspectors/http_inspect/http_inspect.cc
index 9d06c35d652b15e31f6035392f52f5ea65c5475b..57d805821be6f1b7685eea390fad9cc61e553161 100755 (executable)
--- a/src/service_inspectors/http_inspect/http_inspect.cc
+++ b/src/service_inspectors/http_inspect/http_inspect.cc
@@ -470,20 +470,22 @@ void HttpInspect::eval(Packet* p)
         return;
     }
 
-    if (!session_data->for_http2)
-        HttpModule::increment_peg_counts(PEG_TOTAL_BYTES, p->dsize);
-
-    // FIXIT-M Workaround for unexpected eval() calls. Convert to asserts when possible.
+    // FIXIT-M Workaround for unexpected eval() calls. Currently asserting when stream_user is in
+    // use due to calls to HttpInspect::eval on the raw stream_user packet
     if ((session_data->section_type[source_id] == SEC__NOT_COMPUTE) ||
         (session_data->type_expected[source_id] == SEC_ABORT)       ||
         (session_data->octets_reassembled[source_id] != p->dsize))
     {
-        // assert(session_data->type_expected[source_id] != SEC_ABORT);
-        // assert(session_data->section_type[source_id] != SEC__NOT_COMPUTE);
-        // assert(session_data->octets_reassembled[source_id] == p->dsize);
+        //assert(session_data->type_expected[source_id] != SEC_ABORT);
+        //assert(session_data->section_type[source_id] != SEC__NOT_COMPUTE);
+        //assert(session_data->octets_reassembled[source_id] == p->dsize);
         session_data->type_expected[source_id] = SEC_ABORT;
         return;
     }
+
+    if (!session_data->for_http2)
+        HttpModule::increment_peg_counts(PEG_TOTAL_BYTES, p->dsize);
+
     session_data->octets_reassembled[source_id] = STAT_NOT_PRESENT;
 
     // Don't make pkt_data for headers available to detection
@@ -640,7 +642,7 @@ void HttpInspect::clear(Packet* p)
 
     if ( current_section == nullptr )
     {
-        // assert(false); // FIXIT-M this happens a lot
+        //assert(false); //FIXIT-M This happens with stream_user
         return;
     }
 

diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc
index db9685c0b0a55161ed9ee5add1785cb8c71967ea..95899fced2f731309dfdd987a6d764a0086c2e95 100644 (file)
--- a/src/stream/tcp/tcp_reassembler.cc
+++ b/src/stream/tcp/tcp_reassembler.cc
@@ -863,7 +863,7 @@ int32_t TcpReassembler::scan_data_pre_ack(TcpReassemblerState& trs, uint32_t* fl
             return flush_pt;
         }
 
-        if ( !next_no_gap(*tsn) )
+        if (!next_no_gap(*tsn) || (trs.paf_state.paf == StreamSplitter::STOP))
             break;
 
         tsn = tsn->next;
@@ -979,7 +979,8 @@ int32_t TcpReassembler::scan_data_post_ack(TcpReassemblerState& trs, uint32_t* f
             return flush_pt;
         }
 
-        if ( flush_len < tsn->c_len || ( splitter->is_paf() and !next_no_gap(*tsn) ) )
+        if (flush_len < tsn->c_len || (splitter->is_paf() and !next_no_gap(*tsn)) ||
+            (trs.paf_state.paf == StreamSplitter::STOP))
             break;
 
         tsn = tsn->next;
@@ -1016,7 +1017,7 @@ int TcpReassembler::flush_on_data_policy(TcpReassemblerState& trs, Packet* p)
             }
             while ( trs.sos.seglist.head and !p->flow->is_inspection_disabled() );
 
-            if ( !flags && trs.tracker->is_splitter_paf() )
+            if ( (trs.paf_state.paf == StreamSplitter::ABORT) && trs.tracker->is_splitter_paf() )
             {
                 fallback(*trs.tracker, trs.server_side);
                 return flush_on_data_policy(trs, p);