Merge pull request #2203 in SNORT/snort3 from ~MDAGON/snort3:h2i_fix to master

Squashed commit of the following:

commit d042400ce1e3cba30ec905ce603580d27fe60392
Author: mdagon <mdagon@cisco.com>
Date:   Tue May 5 12:14:49 2020 -0400

    http2_inspect: change partial flush handling

diff --git a/src/service_inspectors/http2_inspect/http2_data_cutter.cc b/src/service_inspectors/http2_inspect/http2_data_cutter.cc
index 9ddf91f78800d3cc3f5364f439b496093c6e9f98..bd2ecbcb9be91b299a7eec113d34fe14c585ea63 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_data_cutter.cc
+++ b/src/service_inspectors/http2_inspect/http2_data_cutter.cc
@@ -44,14 +44,12 @@ bool Http2DataCutter::http2_scan(const uint8_t* data, uint32_t length,
 {
     cur_data_offset = data_offset;
     cur_data = cur_padding = 0;
-
     if (frame_bytes_seen == 0)
     {
         frame_length = data_len = frame_len;
         padding_len = data_bytes_read = padding_read = 0;
         frame_flags = flags;
         *flush_offset = frame_bytes_seen = FRAME_HEADER_LENGTH;
-
         if (frame_length == 0)
             data_state = FULL_FRAME;
         else if ((frame_flags & PADDED) !=0)
@@ -194,7 +192,6 @@ const StreamBuffer Http2DataCutter::reassemble(const uint8_t* data, unsigned len
     cur_data = cur_padding = cur_data_offset = 0;
 
     unsigned cur_pos = 0;
-
     while (cur_pos < len)
     {
         switch (reassemble_state)
@@ -207,6 +204,7 @@ const StreamBuffer Http2DataCutter::reassemble(const uint8_t* data, unsigned len
                     session_data->leftover_hdr[source_id], FRAME_HEADER_LENGTH);
                 reassemble_hdr_bytes_read = FRAME_HEADER_LENGTH;
                 session_data->use_leftover_hdr[source_id] = false;
+                cur_pos++;
             }
             else
             {
@@ -229,7 +227,6 @@ const StreamBuffer Http2DataCutter::reassemble(const uint8_t* data, unsigned len
                     session_data->frame_header_offset[source_id]);
                 cur_data_offset = cur_pos;
                 session_data->frame_header_offset[source_id] += FRAME_HEADER_LENGTH;
-
                 if (reassemble_data_len == 0)
                     reassemble_state = CLEANUP;
                 else if ((reassemble_frame_flags & PADDED) !=0)

diff --git a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
index 685f192b68fae9bc3cc19368457b654fd22c8954..be133c42ca84c414aa7e532c7da65b1fe79273b1 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
@@ -198,7 +198,8 @@ void Http2StreamSplitter::partial_flush_data(Http2FlowData* session_data,
     if (data_cutter->is_flush_required())
         session_data->hi_ss[source_id]->init_partial_flush(session_data->flow);
     session_data->data_processing[source_id] = false;
-    *flush_offset = data_offset;
+    assert(data_offset != 0);
+    *flush_offset = data_offset - 1;
     session_data->flushing_data[source_id] = true;
     session_data->num_frame_headers[source_id] -= 1;
 }
@@ -232,17 +233,19 @@ bool Http2StreamSplitter::read_frame_hdr(Http2FlowData* session_data, const uint
     }
     else
     {
-        // Just finished flushing data. Use saved header.
+        // Just finished flushing data. Use saved header, skip first byte.
         session_data->num_frame_headers[source_id] = 1;
         session_data->flushing_data[source_id] = false;
         session_data->use_leftover_hdr[source_id] = true;
+        session_data->scan_octets_seen[source_id] = FRAME_HEADER_LENGTH;
+        data_offset++;
     }
 
     return true;
 }
 
-StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* session_data, const uint8_t* data,
-    uint32_t length, uint32_t* flush_offset, HttpCommon::SourceId source_id)
+StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* session_data,
+    const uint8_t* data, uint32_t length, uint32_t* flush_offset, HttpCommon::SourceId source_id)
 {
     StreamSplitter::Status status = StreamSplitter::FLUSH;
     if (session_data->preface[source_id])
@@ -326,8 +329,8 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio
 
 // FIXIT-M If there are any errors in header decoding, this currently tells stream not to send
 // headers to detection. This behavior may need to be changed.
-const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* session_data, unsigned total,
-    unsigned offset, const uint8_t* data, unsigned len, uint32_t flags,
+const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* session_data,
+    unsigned total, unsigned offset, const uint8_t* data, unsigned len, uint32_t flags,
     HttpCommon::SourceId source_id)
 {
     assert(offset+len <= total);
@@ -349,30 +352,10 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess
         session_data->frame_header_offset[source_id] = 0;
     }
 
-    if (total == 0 && session_data->use_leftover_hdr[source_id])
-    {
-        memcpy(session_data->frame_header[source_id],
-            session_data->leftover_hdr[source_id], FRAME_HEADER_LENGTH);
-        session_data->use_leftover_hdr[source_id] = false;
-        if (session_data->frame_type[source_id] == FT_DATA)
-        {
-            // Check if we reached end of stream and have a partial buffer pending
-            const uint8_t frame_flags = get_frame_flags(session_data->frame_header[source_id]);
-            if ((frame_flags & END_STREAM) && stream->is_partial_buf_pending(source_id))
-            {
-                unsigned copied;
-                StreamBuffer http_frame_buf = session_data->hi_ss[source_id]->reassemble(
-                    session_data->flow,
-                    0, 0, nullptr, 0, PKT_PDU_TAIL, copied);
-                session_data->frame_data[source_id] = const_cast<uint8_t*>(http_frame_buf.data);
-                session_data->frame_data_size[source_id] = http_frame_buf.length;
-            }
-        }
-    }
-    else if (session_data->frame_type[source_id] == FT_DATA)
+    if (session_data->frame_type[source_id] == FT_DATA)
     {
         if (session_data->flushing_data[source_id] && (flags & PKT_PDU_TAIL))
-            len -= FRAME_HEADER_LENGTH;
+            len -= (FRAME_HEADER_LENGTH - 1);
 
         if (len != 0)
         {
@@ -395,10 +378,18 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess
         if (offset == 0)
         {
             // This is the first reassemble() for this frame - allocate data buffer
-            session_data->frame_data_size[source_id] = total;
-            if (!session_data->use_leftover_hdr[source_id])
-                session_data->frame_data_size[source_id] -=
-                    session_data->frame_header_size[source_id];
+            if (session_data->use_leftover_hdr[source_id])
+            {
+                // total has 1 byte of header, missing 8 bytes of first header
+                session_data->frame_data_size[source_id] =
+                    total - 1 - (session_data->frame_header_size[source_id] - FRAME_HEADER_LENGTH);
+            }
+            else
+            {
+                session_data->frame_data_size[source_id] =
+                    total - session_data->frame_header_size[source_id];
+            }
+
             if (session_data->frame_data_size[source_id] > 0)
                 session_data->frame_data[source_id] = new uint8_t[
                     session_data->frame_data_size[source_id]];
@@ -461,6 +452,7 @@ const StreamBuffer Http2StreamSplitter::implement_reassemble(Http2FlowData* sess
                     session_data->leftover_hdr[source_id], FRAME_HEADER_LENGTH);
                 session_data->frame_header_offset[source_id] += FRAME_HEADER_LENGTH;
                 session_data->use_leftover_hdr[source_id] = false;
+                data_offset++;
             }
             else
             {