From: Tom Peters (thopeter) Date: Fri, 19 Nov 2021 20:13:10 +0000 (+0000) Subject: Pull request #3167: Fixes for abort issues X-Git-Tag: 3.1.18.0~19 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=883266660f7357eb1360d26dab6726446a0232ec;p=thirdparty%2Fsnort3.git 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 Date: Mon Nov 15 11:32:41 2021 -0500 http_inspect: update comments for asserts in eval and clear commit 3ccf3b7e0f9c4b453f56015b52aeb16c1ed747c0 Author: Katura Harvey 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 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 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 307f6b258..58e4e0c19 100644 --- 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 2d248fa4d..93b563977 100644 --- 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 3e232707e..4c9f0b114 100644 --- 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 21885a4e3..e51068342 100644 --- 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 9d06c35d6..57d805821 100755 --- 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 db9685c0b..95899fced 100644 --- 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);