Merge pull request #2143 in SNORT/snort3 from ~MDAGON/snort3:0len to master

Squashed commit of the following:

commit 1692fd54db389cb3094ed99c499236550d3e2ef1
Author: mdagon <mdagon@cisco.com>
Date:   Thu Apr 2 15:07:35 2020 -0400

    http2_inspect: support 0 length data frames

diff --git a/src/service_inspectors/http2_inspect/http2_data_cutter.cc b/src/service_inspectors/http2_inspect/http2_data_cutter.cc
index 6cbba9668aa2d13f0a88fcf11379fe27c65343f4..a9b956c549c414c63d2883a138707613c686de65 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_data_cutter.cc
+++ b/src/service_inspectors/http2_inspect/http2_data_cutter.cc
@@ -50,9 +50,14 @@ bool Http2DataCutter::http2_scan(const uint8_t* data, uint32_t length,
         frame_length = data_len = frame_len;
         padding_len = data_bytes_read = padding_read = 0;
         frame_flags = flags;
-        frame_bytes_seen = cur_data_offset = FRAME_HEADER_LENGTH;
-        *flush_offset = FRAME_HEADER_LENGTH;
-        data_state = ((frame_flags & PADDED) !=0) ? PADDING_LENGTH : DATA;
+        *flush_offset = frame_bytes_seen = FRAME_HEADER_LENGTH;
+
+        if (frame_length == 0)
+            data_state = FULL_FRAME;
+        else if ((frame_flags & PADDED) !=0)
+            data_state = PADDING_LENGTH;
+        else
+            data_state = DATA;
     }
 
     uint32_t cur_pos = data_offset + leftover_bytes;
@@ -69,11 +74,14 @@ bool Http2DataCutter::http2_scan(const uint8_t* data, uint32_t length,
                 session_data->events[source_id]->create_event(EVENT_PADDING_LEN);
                 return false;
             }
-            // FIXIT temporary - till multiple data frames sent to http
-            if (data_len == (padding_len + 1))
-                return false;
             data_len -= (padding_len + 1);
-            data_state = DATA;
+
+            if (data_len)
+                data_state = DATA;
+            else if (padding_len)
+                data_state = PADDING;
+            else
+                data_state = FULL_FRAME;
             cur_pos++;
             cur_data_offset++;
             break;
@@ -154,6 +162,7 @@ StreamSplitter::Status Http2DataCutter::http_scan(const uint8_t* data, uint32_t*
             if (frame_flags & END_STREAM)
             {
                 finish_msg_body(session_data, source_id);
+                session_data->data_processing[source_id] = false;
                 return StreamSplitter::FLUSH;
             }
             else
@@ -205,9 +214,14 @@ const StreamBuffer Http2DataCutter::reassemble(const uint8_t* data, unsigned len
                 reassemble_frame_flags = get_frame_flags(session_data->frame_header[source_id]+
                     session_data->frame_header_offset[source_id]);
                 cur_data_offset = cur_pos;
-                reassemble_state = ((reassemble_frame_flags & PADDED) !=0) ? GET_PADDING_LEN :
-                    SEND_DATA;
                 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)
+                    reassemble_state = GET_PADDING_LEN;
+                else
+                    reassemble_state = SEND_DATA;
             }
 
             break;
@@ -217,7 +231,12 @@ const StreamBuffer Http2DataCutter::reassemble(const uint8_t* data, unsigned len
             reassemble_data_len -= (reassemble_padding_len + 1);
             cur_pos++;
             cur_data_offset++;
-            reassemble_state = SEND_DATA;
+            if (reassemble_data_len)
+                reassemble_state = SEND_DATA;
+            else if (reassemble_padding_len)
+                reassemble_state = SKIP_PADDING;
+            else
+                reassemble_state = CLEANUP;
             break;
         case SEND_DATA:
           {

diff --git a/src/service_inspectors/http2_inspect/http2_data_cutter.h b/src/service_inspectors/http2_inspect/http2_data_cutter.h
index 571b4c0fdbb5c0ff47a24dc91fbb1764ad0e427f..69519500b50b84daa894ce00237b0874e23feca6 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_data_cutter.h
+++ b/src/service_inspectors/http2_inspect/http2_data_cutter.h
@@ -35,6 +35,8 @@ public:
         uint8_t frame_flags =0);
     const snort::StreamBuffer reassemble(const uint8_t* data, unsigned len);
 
+    bool is_flush_required() { return bytes_sent_http != 0; }
+
 private:
 
     Http2FlowData* const session_data;

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 65bde60005e927002f0bd8117d639a964a98abce..1f8996eebbadea8d3da886d982b129a009a3c839 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
+++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc
@@ -72,19 +72,36 @@ StreamSplitter::Status Http2StreamSplitter::data_scan(Http2FlowData* session_dat
     if (stream && stream->abort_on_data_is_set(source_id))
         return StreamSplitter::ABORT;
 
-    HttpFlowData* http_flow = nullptr;
-    if (stream)
-        http_flow = (HttpFlowData*)stream->get_hi_flow_data();
+    if (!stream || stream->end_stream_is_set(source_id))
+    {
+        *session_data->infractions[source_id] += INF_FRAME_SEQUENCE;
+        session_data->events[source_id]->create_event(EVENT_FRAME_SEQUENCE);
+        return StreamSplitter::ABORT;
+    }
 
-    if (!stream || !http_flow || stream->end_stream_is_set(source_id) ||
-        (frame_length > 0 and (http_flow->get_type_expected(source_id) != HttpEnums::SEC_BODY_H2)))
+    HttpFlowData* const http_flow = (HttpFlowData*)stream->get_hi_flow_data();
+    if (http_flow == nullptr)
     {
         *session_data->infractions[source_id] += INF_FRAME_SEQUENCE;
         session_data->events[source_id]->create_event(EVENT_FRAME_SEQUENCE);
         return StreamSplitter::ABORT;
     }
 
-    if (frame_length == 0 or frame_length > MAX_OCTETS)
+    if (http_flow->get_type_expected(source_id) != HttpEnums::SEC_BODY_H2)
+    {
+        // If 0 length frame and http isn't expecting body, flush without involving http
+        if (frame_length == 0)
+        {
+            *flush_offset = data_offset;
+            return StreamSplitter::FLUSH;
+        }
+
+        *session_data->infractions[source_id] += INF_FRAME_SEQUENCE;
+        session_data->events[source_id]->create_event(EVENT_FRAME_SEQUENCE);
+        return StreamSplitter::ABORT;
+    }
+
+    if (frame_length > MAX_OCTETS)
         return StreamSplitter::ABORT;
 
     Http2DataCutter* data_cutter = stream->get_data_cutter(source_id);
@@ -182,14 +199,17 @@ void Http2StreamSplitter::flush_data(Http2FlowData* session_data, HttpCommon::So
 {
     session_data->current_stream[source_id] = old_stream;
     session_data->frame_type[source_id] = FT_DATA;
-    finish_msg_body(session_data, source_id);
+    Http2Stream* const stream = session_data->find_stream(
+        session_data->current_stream[source_id]);
+    Http2DataCutter* const data_cutter = stream->get_data_cutter(source_id);
+    if (data_cutter->is_flush_required())
+        finish_msg_body(session_data, source_id);
+    session_data->data_processing[source_id] = false;
     *flush_offset = FRAME_HEADER_LENGTH;
     session_data->flushing_data[source_id] = true;
     memcpy(session_data->leftover_hdr[source_id],
         session_data->scan_frame_header[source_id], FRAME_HEADER_LENGTH);
     session_data->num_frame_headers[source_id] -= 1;
-    Http2Stream* const stream = session_data->find_stream(
-        session_data->current_stream[source_id]);
     stream->set_abort_on_data(source_id);
 }
 

diff --git a/src/service_inspectors/http2_inspect/http2_utils.cc b/src/service_inspectors/http2_inspect/http2_utils.cc
index eb603bb472b7438c95f5dc5c6a2a88c34770f707..79a2f0330ce0bd75a8f28faa9af46bac162f2126 100644 (file)
--- a/src/service_inspectors/http2_inspect/http2_utils.cc
+++ b/src/service_inspectors/http2_inspect/http2_utils.cc
@@ -75,6 +75,5 @@ void finish_msg_body(Http2FlowData* session_data, HttpCommon::SourceId source_id
         &dummy_pkt, nullptr, 0, unused, &http_flush_offset);
     assert(scan_result == snort::StreamSplitter::FLUSH);
     UNUSED(scan_result);
-    session_data->data_processing[source_id] = false;
 }