From: Mike Stepanek (mstepane) Date: Thu, 9 Apr 2020 20:21:52 +0000 (+0000) Subject: Merge pull request #2143 in SNORT/snort3 from ~MDAGON/snort3:0len to master X-Git-Tag: 3.0.1-2~26 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=01d40414886b1edba4df0e3b380f98f6f81adbc1;p=thirdparty%2Fsnort3.git Merge pull request #2143 in SNORT/snort3 from ~MDAGON/snort3:0len to master Squashed commit of the following: commit 1692fd54db389cb3094ed99c499236550d3e2ef1 Author: mdagon 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 6cbba9668..a9b956c54 100644 --- 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 571b4c0fd..69519500b 100644 --- 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 65bde6000..1f8996eeb 100644 --- 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 eb603bb47..79a2f0330 100644 --- 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; }