Merge pull request #2603 in SNORT/snort3 from ~THOPETER/snort3:h2i16 to master

Squashed commit of the following:

commit 682542cf2fdb9d56f109e64a7df782f5100ad778
Author: Tom Peters <thopeter@cisco.com>
Date:   Thu Nov 5 15:52:09 2020 -0500

    http2_inspect: refactor data cutter

diff --git a/src/service_inspectors/http2_inspect/http2_data_cutter.cc b/src/service_inspectors/http2_inspect/http2_data_cutter.cc
index 642244e216b82bb65c1a1f37594e40fb31a8891d..70cb4f7702bf27587515638e5c649a0d6c514caa 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_data_cutter.cc
+++ b/src/service_inspectors/http2_inspect/http2_data_cutter.cc
@@ -37,17 +37,17 @@ Http2DataCutter::Http2DataCutter(Http2FlowData* _session_data, HttpCommon::Sourc
     session_data(_session_data), source_id(src_id)
 { }
 
-void Http2DataCutter::http2_scan(uint32_t length, uint32_t* flush_offset, uint32_t frame_len,
-    bool padded, uint32_t& data_offset)
+StreamSplitter::Status Http2DataCutter::scan(const uint8_t* data, uint32_t length,
+    uint32_t* flush_offset, uint32_t& data_offset, uint32_t frame_len, uint8_t frame_flags)
 {
-    cur_data_offset = data_offset;
+    const uint32_t cur_data_offset = data_offset;
     if (frame_bytes_seen == 0)
     {
         data_len = frame_len;
         data_bytes_read = 0;
         frame_bytes_seen = FRAME_HEADER_LENGTH;
 
-        if (padded)
+        if (frame_flags & PADDED)
         {
             data_len -= 1;
             frame_bytes_seen += 1;
@@ -56,21 +56,17 @@ void Http2DataCutter::http2_scan(uint32_t length, uint32_t* flush_offset, uint32
 
     uint32_t cur_pos = data_offset;
     const uint32_t missing = data_len - data_bytes_read;
-    cur_data = (missing <= (length - cur_pos)) ? missing : (length - cur_pos);
+    const uint32_t cur_data = (missing <= (length - cur_pos)) ? missing : (length - cur_pos);
     data_bytes_read += cur_data;
     cur_pos += cur_data;
 
     frame_bytes_seen += cur_pos - data_offset;
-    // FIXIT-L In theory data_offset should be reduced for any bytes not used by HI scan() later.
-    // But currently the value is not used in the case where we flush.
     data_offset = cur_pos;
     *flush_offset = cur_pos;
     session_data->scan_remaining_frame_octets[source_id] = frame_len - frame_bytes_seen;
-}
 
-StreamSplitter::Status Http2DataCutter::http_scan(const uint8_t* data, uint32_t* flush_offset,
-    bool end_stream)
-{
+    session_data->stream_in_hi = session_data->current_stream[source_id];
+
     StreamSplitter::Status scan_result = StreamSplitter::SEARCH;
 
     if (cur_data > 0)
@@ -87,6 +83,7 @@ StreamSplitter::Status Http2DataCutter::http_scan(const uint8_t* data, uint32_t*
             bytes_sent_http += http_flush_offset;
             const uint32_t unused_input = cur_data - http_flush_offset;
             data_bytes_read -= unused_input;
+            data_offset -= unused_input;
             *flush_offset -= unused_input;
         }
         else if (scan_result == StreamSplitter::SEARCH)
@@ -94,11 +91,14 @@ StreamSplitter::Status Http2DataCutter::http_scan(const uint8_t* data, uint32_t*
             bytes_sent_http += cur_data;
         }
         else if (scan_result == StreamSplitter::ABORT)
+        {
             // FIXIT-E eventually need to implement continued processing. We cannot abort just
             // because one stream went sideways. A better approach would be to put this one stream
             // into a pass through mode while continuing to process other streams. As long as we
             // can parse the framing and process most streams it is reasonable to continue.
+            session_data->stream_in_hi = NO_STREAM_ID;
             return StreamSplitter::ABORT;
+        }
     }
 
     if (data_bytes_read == data_len)
@@ -109,13 +109,14 @@ StreamSplitter::Status Http2DataCutter::http_scan(const uint8_t* data, uint32_t*
         session_data->scan_state[source_id] = SCAN_FRAME_HEADER;
         frame_bytes_seen = 0;
 
-        if (end_stream)
+        if (frame_flags & END_STREAM)
         {
             Http2Stream* const stream = session_data->find_stream(
                 session_data->current_stream[source_id]);
-            stream->finish_msg_body(source_id, false, !bytes_sent_http);
+            stream->finish_msg_body(source_id, false, bytes_sent_http == 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;
             return StreamSplitter::FLUSH;
         }
         else if (scan_result != StreamSplitter::FLUSH)
@@ -135,26 +136,16 @@ StreamSplitter::Status Http2DataCutter::http_scan(const uint8_t* data, uint32_t*
 
     if (scan_result != StreamSplitter::FLUSH)
         *flush_offset = 0;
-    return scan_result;
-}
-
-StreamSplitter::Status Http2DataCutter::scan(const uint8_t* data, uint32_t length,
-    uint32_t* flush_offset, uint32_t& data_offset, uint32_t frame_len, uint8_t frame_flags)
-{
-    http2_scan(length, flush_offset, frame_len, frame_flags & PADDED, data_offset);
 
-    session_data->stream_in_hi = session_data->current_stream[source_id];
-    StreamSplitter::Status status = http_scan(data, flush_offset, frame_flags & END_STREAM);
     session_data->stream_in_hi = NO_STREAM_ID;
 
-    return status;
+    return scan_result;
 }
 
 void Http2DataCutter::reassemble(const uint8_t* data, unsigned len)
 {
-    session_data->stream_in_hi = session_data->current_stream[source_id];
-
-    cur_data = cur_data_offset = 0;
+    uint32_t cur_data = 0;
+    uint32_t cur_data_offset = 0;
 
     unsigned cur_pos = 0;
     while ((cur_pos < len) || (reassemble_state == SEND_EMPTY_DATA))
@@ -171,19 +162,20 @@ void Http2DataCutter::reassemble(const uint8_t* data, unsigned len)
             if (reassemble_hdr_bytes_read == FRAME_HEADER_LENGTH)
             {
                 assert(!session_data->frame_lengths[source_id].empty());
+                reassemble_hdr_bytes_read = 0;
                 reassemble_data_len = session_data->frame_lengths[source_id].front();
                 session_data->frame_lengths[source_id].pop();
-                reassemble_frame_flags =
+                const uint8_t frame_flags =
                     get_frame_flags(session_data->lead_frame_header[source_id]);
                 cur_data_offset = cur_pos;
-                if ((reassemble_frame_flags & PADDED) != 0)
+                if (frame_flags & PADDED)
                     reassemble_state = GET_PADDING_LEN;
                 else if (reassemble_data_len > 0)
                     reassemble_state = SEND_DATA;
-                else if (reassemble_frame_flags & END_STREAM)
+                else if (frame_flags & END_STREAM)
                     reassemble_state = SEND_EMPTY_DATA;
                 else
-                    reassemble_state = DONE;
+                    reassemble_state = GET_FRAME_HDR;
             }
             break;
           }
@@ -195,10 +187,10 @@ void Http2DataCutter::reassemble(const uint8_t* data, unsigned len)
             cur_data_offset++;
             if (reassemble_data_len > 0)
                 reassemble_state = SEND_DATA;
-            else if (reassemble_frame_flags & END_STREAM)
+            else if (get_frame_flags(session_data->lead_frame_header[source_id]) & END_STREAM)
                 reassemble_state = SEND_EMPTY_DATA;
             else
-                reassemble_state = DONE;
+                reassemble_state = GET_FRAME_HDR;
             break;
           }
         case SEND_EMPTY_DATA:
@@ -213,9 +205,11 @@ void Http2DataCutter::reassemble(const uint8_t* data, unsigned len)
             unsigned copied;
             const uint32_t flags = (bytes_sent_http == (cur_data + reassemble_bytes_sent)) ?
                 PKT_PDU_TAIL : 0;
+            session_data->stream_in_hi = session_data->current_stream[source_id];
             StreamBuffer frame_buf = session_data->hi_ss[source_id]->reassemble(session_data->flow,
                 bytes_sent_http, 0, data + cur_data_offset, cur_data,
                 flags, copied);
+            session_data->stream_in_hi = NO_STREAM_ID;
             assert(copied == (unsigned)cur_data);
 
             if (frame_buf.data != nullptr)
@@ -229,27 +223,15 @@ void Http2DataCutter::reassemble(const uint8_t* data, unsigned len)
                 reassemble_bytes_sent += copied;
 
             if (reassemble_data_bytes_read == reassemble_data_len)
-                reassemble_state = DONE;
+            {
+                reassemble_data_bytes_read = 0;
+                reassemble_state = GET_FRAME_HDR;
+            }
 
             break;
           }
-        case DONE:
-            assert(false);
-            break;
         }
     }
-
-    session_data->stream_in_hi = NO_STREAM_ID;
     return;
 }
 
-void Http2DataCutter::reset()
-{
-    frame_bytes_seen = 0;
-    bytes_sent_http = 0;
-    reassemble_bytes_sent = 0;
-    reassemble_hdr_bytes_read = 0;
-    reassemble_data_bytes_read = 0;
-    reassemble_state = GET_FRAME_HDR;
-}
-

diff --git a/src/service_inspectors/http2_inspect/http2_data_cutter.h b/src/service_inspectors/http2_inspect/http2_data_cutter.h
index 28d3a8f561dd96455944595cf09c01da89882ad1..e1f1d512303e66b163b2cfe2e8d598dda37c3e6d 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_data_cutter.h
+++ b/src/service_inspectors/http2_inspect/http2_data_cutter.h
@@ -35,7 +35,6 @@ public:
         uint32_t* flush_offset, uint32_t& data_offset, uint32_t frame_len,
         uint8_t frame_flags);
     void reassemble(const uint8_t* data, unsigned len);
-    void reset();
 
 private:
     Http2FlowData* const session_data;
@@ -49,23 +48,14 @@ private:
     uint32_t data_bytes_read;
     // total per frame - reassemble
     uint32_t reassemble_data_len;
-    uint8_t reassemble_frame_flags;
     // accumulating - reassemble
     uint32_t reassemble_bytes_sent = 0;
     uint32_t reassemble_hdr_bytes_read = 0;
     uint32_t reassemble_data_bytes_read = 0;
-    // per call
-    uint32_t cur_data;
-    uint32_t cur_data_offset;
 
     // reassemble
-    enum ReassembleState { GET_FRAME_HDR, GET_PADDING_LEN, SEND_EMPTY_DATA, SEND_DATA, DONE };
+    enum ReassembleState { GET_FRAME_HDR, GET_PADDING_LEN, SEND_EMPTY_DATA, SEND_DATA };
     enum ReassembleState reassemble_state = GET_FRAME_HDR;
-
-    void http2_scan(uint32_t length, uint32_t* flush_offset, uint32_t frame_len, bool padded,
-        uint32_t& data_offset);
-    snort::StreamSplitter::Status http_scan(const uint8_t* data, uint32_t* flush_offset,
-        bool end_stream);
 };
 
 #endif

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 4c4ab301b054585e8d472944563c48ec8c04d78b..4e0b592a8998da2be4f6c3e63234e80d7fc20d6d 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
@@ -299,7 +299,6 @@ StreamSplitter::Status Http2StreamSplitter::implement_scan(Http2FlowData* sessio
                 {
                     status = data_frame_header_checks(session_data, flush_offset, source_id,
                         frame_length, data_offset);
-                    session_data->data_cutter[source_id].reset();
                 }
                 else
                     status = non_data_frame_header_checks(session_data, source_id, frame_length,