From: Mike Stepanek (mstepane) Date: Tue, 15 Dec 2020 18:28:10 +0000 (+0000) Subject: Merge pull request #2671 in SNORT/snort3 from ~THOPETER/snort3:h2i_extra_zero_fix... X-Git-Tag: 3.0.3-6~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f9846baa60df0a89b40f825b440d3e5331023a7c;p=thirdparty%2Fsnort3.git Merge pull request #2671 in SNORT/snort3 from ~THOPETER/snort3:h2i_extra_zero_fix to master Squashed commit of the following: commit 1478154ce4eb86a0c526ed6a16a7319e596c64d3 Author: mdagon Date: Wed Nov 25 11:45:43 2020 -0500 http2_inspect: remove 0 length scan for most cases --- diff --git a/src/service_inspectors/http2_inspect/http2_data_cutter.cc b/src/service_inspectors/http2_inspect/http2_data_cutter.cc index f8ee151dc..e34993a88 100644 --- a/src/service_inspectors/http2_inspect/http2_data_cutter.cc +++ b/src/service_inspectors/http2_inspect/http2_data_cutter.cc @@ -75,6 +75,13 @@ StreamSplitter::Status Http2DataCutter::scan(const uint8_t* data, uint32_t lengt Http2DummyPacket dummy_pkt; dummy_pkt.flow = session_data->flow; uint32_t unused = 0; + if ((data_bytes_read == data_len) && (frame_flags & END_STREAM)) + { + Http2Stream* const stream = + session_data->find_stream(session_data->current_stream[source_id]); + HttpFlowData* const hi_flow = stream->get_hi_flow_data(); + hi_flow->set_h2_body_state(source_id, HttpEnums::H2_BODY_LAST_SEG); + } scan_result = session_data->hi_ss[source_id]->scan(&dummy_pkt, data + cur_data_offset, cur_data, unused, &http_flush_offset); @@ -107,7 +114,9 @@ StreamSplitter::Status Http2DataCutter::scan(const uint8_t* data, uint32_t lengt { Http2Stream* const stream = session_data->find_stream( session_data->current_stream[source_id]); - stream->finish_msg_body(source_id, false, bytes_sent_http == 0); + assert(scan_result == StreamSplitter::FLUSH || data_len == 0); + stream->finish_msg_body(source_id, false, data_len == 0); + // FIXIT-E this flag seems to mean both END_STREAM and the end of this frame stream->set_end_stream_on_data_flush(source_id); session_data->stream_in_hi = NO_STREAM_ID; diff --git a/src/service_inspectors/http2_inspect/http2_stream.cc b/src/service_inspectors/http2_inspect/http2_stream.cc index 26b197a52..0a1575fb2 100644 --- a/src/service_inspectors/http2_inspect/http2_stream.cc +++ b/src/service_inspectors/http2_inspect/http2_stream.cc @@ -128,12 +128,15 @@ void Http2Stream::finish_msg_body(HttpCommon::SourceId source_id, bool expect_tr uint32_t http_flush_offset = 0; Http2DummyPacket dummy_pkt; dummy_pkt.flow = session_data->flow; - uint32_t unused = 0; const H2BodyState body_state = expect_trailers ? H2_BODY_COMPLETE_EXPECT_TRAILERS : H2_BODY_COMPLETE; get_hi_flow_data()->finish_h2_body(source_id, body_state, clear_partial_buffer); - const snort::StreamSplitter::Status scan_result = session_data->hi_ss[source_id]->scan( - &dummy_pkt, nullptr, 0, unused, &http_flush_offset); - assert(scan_result == snort::StreamSplitter::FLUSH); - UNUSED(scan_result); + if (clear_partial_buffer) + { + uint32_t unused = 0; + const snort::StreamSplitter::Status scan_result = session_data->hi_ss[source_id]->scan( + &dummy_pkt, nullptr, 0, unused, &http_flush_offset); + assert(scan_result == snort::StreamSplitter::FLUSH); + UNUSED(scan_result); + } } diff --git a/src/service_inspectors/http_inspect/http_cutter.cc b/src/service_inspectors/http_inspect/http_cutter.cc index a173b21f2..139d04662 100644 --- a/src/service_inspectors/http_inspect/http_cutter.cc +++ b/src/service_inspectors/http_inspect/http_cutter.cc @@ -765,9 +765,23 @@ ScanResult HttpBodyH2Cutter::cut(const uint8_t* /*buffer*/, uint32_t length, return SCAN_FOUND_PIECE; } } + else if (state == H2_BODY_LAST_SEG) + { + if (octets_seen + length <= flow_target) + num_flush = length; + else + num_flush = flow_target - octets_seen; + + total_octets_scanned += num_flush; + if (num_flush == length) + return SCAN_FOUND; + else + return SCAN_FOUND_PIECE; + } else { - // For now if end_stream is set for scan, a zero-length buffer is always sent to flush + // To end message body when trailers are received or a 0 length data frame with + // end of stream set is received, a zero-length buffer is sent to flush assert(length == 0); num_flush = 0; return SCAN_FOUND; diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h index c1202247b..a5cb91cee 100755 --- a/src/service_inspectors/http_inspect/http_enum.h +++ b/src/service_inspectors/http_inspect/http_enum.h @@ -385,8 +385,8 @@ extern const bool is_sp_tab_quote_dquote[256]; extern const bool is_print_char[256]; // printable includes SP, tab, CR, LF extern const bool is_sp_comma[256]; -enum H2BodyState { H2_BODY_NOT_COMPLETE, H2_BODY_COMPLETE, H2_BODY_COMPLETE_EXPECT_TRAILERS, - H2_BODY_NO_BODY }; +enum H2BodyState { H2_BODY_NOT_COMPLETE, H2_BODY_LAST_SEG, H2_BODY_COMPLETE, + H2_BODY_COMPLETE_EXPECT_TRAILERS, H2_BODY_NO_BODY }; } // end namespace HttpEnums diff --git a/src/service_inspectors/http_inspect/http_flow_data.cc b/src/service_inspectors/http_inspect/http_flow_data.cc index 12ea0ff2f..48d1ec522 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.cc +++ b/src/service_inspectors/http_inspect/http_flow_data.cc @@ -257,7 +257,8 @@ HttpInfractions* HttpFlowData::get_infractions(SourceId source_id) void HttpFlowData::finish_h2_body(HttpCommon::SourceId source_id, HttpEnums::H2BodyState state, bool clear_partial_buffer) { - assert(h2_body_state[source_id] == H2_BODY_NOT_COMPLETE); + assert((h2_body_state[source_id] == H2_BODY_NOT_COMPLETE) || + (h2_body_state[source_id] == H2_BODY_LAST_SEG)); h2_body_state[source_id] = state; partial_flush[source_id] = false; if (clear_partial_buffer) diff --git a/src/service_inspectors/http_inspect/http_flow_data.h b/src/service_inspectors/http_inspect/http_flow_data.h index 8f8ef970e..929528305 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.h +++ b/src/service_inspectors/http_inspect/http_flow_data.h @@ -74,6 +74,9 @@ public: void finish_h2_body(HttpCommon::SourceId source_id, HttpEnums::H2BodyState state, bool clear_partial_buffer); + void set_h2_body_state(HttpCommon::SourceId source_id, HttpEnums::H2BodyState state) + { h2_body_state[source_id] = state; } + void reset_partial_flush(HttpCommon::SourceId source_id) { partial_flush[source_id] = false; } diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc index a0c46cc78..d15c6c610 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc @@ -299,6 +299,8 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total, if ((session_data->section_offset[source_id] == 0) && (session_data->octets_expected[source_id] != partial_raw_bytes + total)) { + assert(!session_data->for_http2); + if (session_data->octets_expected[source_id] == 0) { // FIXIT-E This is a known problem. No data was scanned and yet somehow stream can