Merge pull request #2671 in SNORT/snort3 from ~THOPETER/snort3:h2i_extra_zero_fix...

Squashed commit of the following:

commit 1478154ce4eb86a0c526ed6a16a7319e596c64d3
Author: mdagon <mdagon@cisco.com>
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 f8ee151dc622b4e19ecbbd850fcbb882087b6cb0..e34993a88afe3a1b3743fab1b3c50179606aad91 100644 (file)
--- 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 26b197a52e7f6a48d42a633293476da8bc80abfc..0a1575fb23910d5e00833a02099b2e9cc6e643cd 100644 (file)
--- 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 a173b21f237195aa7a00de6b9499b195aa1be3e4..139d046624d81a2303c3661f9125d8909910090b 100644 (file)
--- 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 c1202247b648a52c77b0413b21a23a137174bfcd..a5cb91cee6da689f748852b84e663025d17aa416 100755 (executable)
--- 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 12ea0ff2f3f580f080a007f05fc774fa742bef89..48d1ec522357201ad75a66ab5e91cbfeff640675 100644 (file)
--- 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 8f8ef970e6b8c996d2580f055e30b92c6be3fd3e..9295283054945ef255199fc69e22a8dda7079b75 100644 (file)
--- 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 a0c46cc785628f58f31410b0b6c0ab010da18350..d15c6c6106cde993d39c5d3a04f9262012df0253 100644 (file)
--- 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