From: Mike Stepanek (mstepane) Date: Fri, 4 Oct 2019 22:07:38 +0000 (-0400) Subject: Merge pull request #1776 in SNORT/snort3 from ~KATHARVE/snort3:h2i_cut_frame_headers... X-Git-Tag: 3.0.0-262~7 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0d102aec822d95560eaaa6ff4fa0e436a2b3c594;p=thirdparty%2Fsnort3.git Merge pull request #1776 in SNORT/snort3 from ~KATHARVE/snort3:h2i_cut_frame_headers to master Squashed commit of the following: commit ae747d91590506059c321c77bbc3eaf803c82b96 Author: Katura Harvey Date: Wed Oct 2 14:15:06 2019 -0400 http2_inspect: cut headers from frame_data buffer --- diff --git a/src/service_inspectors/http2_inspect/http2_enum.h b/src/service_inspectors/http2_inspect/http2_enum.h index cf02cbe17..bb1133374 100644 --- a/src/service_inspectors/http2_inspect/http2_enum.h +++ b/src/service_inspectors/http2_inspect/http2_enum.h @@ -32,7 +32,7 @@ static const uint32_t HTTP2_GID = 121; // Frame type codes (fourth octet of frame header) enum FrameType { FT_DATA=0, FT_HEADERS=1, FT_PRIORITY=2, FT_RST_STREAM=3, FT_SETTINGS=4, - FT_PUSH_PROMISE=5, FT_PING=6, FT_GOAWAY=7, FT_WINDOW_UPDATE=8, FT_CONTINUATION=9 }; + FT_PUSH_PROMISE=5, FT_PING=6, FT_GOAWAY=7, FT_WINDOW_UPDATE=8, FT_CONTINUATION=9, FT__NONE=255 }; // Message buffers available to clients // This enum must remain synchronized with Http2Api::classic_buffer_names[] @@ -81,6 +81,7 @@ enum HeaderFrameFlags END_HEADERS = 0x4, PADDED = 0x8, PRIORITY = 0x20, + NO_HEADER = 0x80, //No valid flags use this bit }; diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.cc b/src/service_inspectors/http2_inspect/http2_flow_data.cc index c074b58d2..741d6ed9d 100644 --- a/src/service_inspectors/http2_inspect/http2_flow_data.cc +++ b/src/service_inspectors/http2_inspect/http2_flow_data.cc @@ -71,28 +71,10 @@ Http2FlowData::~Http2FlowData() for (int k=0; k <= 1; k++) { delete[] frame_header[k]; - delete[] frame[k]; + delete[] currently_processing_frame_header[k]; + delete[] frame_data[k]; delete[] http2_decoded_header[k]; - delete[] header_frame_header[k]; delete infractions[k]; delete events[k]; } } - -int Http2FlowData::get_frame_type(HttpCommon::SourceId source_id) -{ - const int frame_type_index = 3; - if (frame_header[source_id]) - return frame_header[source_id][frame_type_index]; - else - return STAT_NO_SOURCE; -} - -int Http2FlowData::get_frame_flags(HttpCommon::SourceId source_id) -{ - const int frame_flags_index = 4; - if (frame_header[source_id]) - return frame_header[source_id][frame_flags_index]; - else - return STAT_NO_SOURCE; -} diff --git a/src/service_inspectors/http2_inspect/http2_flow_data.h b/src/service_inspectors/http2_inspect/http2_flow_data.h index fd181b8d9..b52bbdd95 100644 --- a/src/service_inspectors/http2_inspect/http2_flow_data.h +++ b/src/service_inspectors/http2_inspect/http2_flow_data.h @@ -60,29 +60,34 @@ public: protected: // 0 element refers to client frame, 1 element refers to server frame bool preface[2] = { true, false }; - bool header_coming[2] = { false, false }; uint8_t* frame_header[2] = { nullptr, nullptr }; - uint8_t* frame[2] = { nullptr, nullptr }; - uint32_t frame_size[2] = { 0, 0 }; + uint32_t frame_header_size[2] = { 0, 0 }; uint8_t* frame_data[2] = { nullptr, nullptr }; uint32_t frame_data_size[2] = { 0, 0 }; uint8_t* http2_decoded_header[2] = { nullptr, nullptr }; uint32_t http2_decoded_header_size[2] = { 0, 0 }; + bool frame_in_detection = false; + + // Internal to scan + bool continuation_expected[2] = { false, false }; + uint8_t* currently_processing_frame_header[2] = { nullptr, nullptr }; + uint32_t inspection_section_length[2] = { 0, 0 }; uint32_t leftover_data[2] = { 0, 0 }; + + // Used internally by scan and reassemble uint32_t octets_seen[2] = { 0, 0 }; - uint32_t header_octets_seen[2] = { 0, 0 }; - uint32_t inspection_section_length[2] = { 0, 0 }; - bool frame_in_detection = false; + uint8_t header_octets_seen[2] = { 0, 0 }; + + // Scan signals to reassemble + bool header_coming[2] = { false, false }; + uint32_t frames_aggregated[2] = { 0, 0 }; - int32_t get_frame_type(HttpCommon::SourceId source_id); - int32_t get_frame_flags(HttpCommon::SourceId source_id); - - bool continuation_expected = false; - //FIXIT-M Most of this will need to change when we handle multiple streams, so this vector is - //not intended to be the best long-term solution - std::vector continuation_frame_lengths; - uint8_t* header_frame_header[2] = { nullptr, nullptr }; - + // Internal to reassemble + uint32_t remaining_octets_to_next_header[2] = { 0, 0 }; + uint32_t remaining_frame_data_octets[2] = { 0, 0 }; + uint32_t remaining_frame_data_offset[2] = { 0, 0 }; + uint32_t frame_header_offset[2] = { 0, 0 }; + // These will eventually be moved over to the frame/stream object, as they are moved to the // transaction in NHI. Also as in NHI accessor methods will need to be added. Http2Infractions* infractions[2] = { new Http2Infractions, new Http2Infractions }; diff --git a/src/service_inspectors/http2_inspect/http2_inspect.cc b/src/service_inspectors/http2_inspect/http2_inspect.cc index e893ff56e..a43b109c9 100644 --- a/src/service_inspectors/http2_inspect/http2_inspect.cc +++ b/src/service_inspectors/http2_inspect/http2_inspect.cc @@ -105,8 +105,8 @@ void Http2Inspect::eval(Packet* p) #ifdef REG_TEST if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP2)) { - Field((session_data->frame_header[source_id] != nullptr) ? FRAME_HEADER_LENGTH : - HttpCommon::STAT_NOT_PRESENT, + Field((session_data->frame_header[source_id] != nullptr) ? + (int) session_data->frame_header_size[source_id] : HttpCommon::STAT_NOT_PRESENT, session_data->frame_header[source_id]).print(HttpTestManager::get_output_file(), "Frame Header"); Field((session_data->frame_data[source_id] != nullptr) ? @@ -134,18 +134,14 @@ void Http2Inspect::clear(Packet* p) delete[] session_data->frame_header[source_id]; session_data->frame_header[source_id] = nullptr; - delete[] session_data->frame[source_id]; - session_data->frame[source_id] = nullptr; + delete[] session_data->frame_data[source_id]; session_data->frame_data[source_id] = nullptr; session_data->frame_in_detection = false; delete[] session_data->http2_decoded_header[source_id]; session_data->http2_decoded_header[source_id] = nullptr; - session_data->continuation_expected = false; - if (session_data->header_frame_header[source_id]) - { - delete[] session_data->header_frame_header[source_id]; - session_data->header_frame_header[source_id] = nullptr; - } - session_data->continuation_frame_lengths.clear(); + session_data->continuation_expected[source_id] = false; + delete[] session_data->currently_processing_frame_header[source_id]; + session_data->currently_processing_frame_header[source_id] = nullptr; + session_data->frames_aggregated[source_id] = 0; } diff --git a/src/service_inspectors/http2_inspect/http2_inspect_impl.cc b/src/service_inspectors/http2_inspect/http2_inspect_impl.cc index e8f5135c0..e70564085 100644 --- a/src/service_inspectors/http2_inspect/http2_inspect_impl.cc +++ b/src/service_inspectors/http2_inspect/http2_inspect_impl.cc @@ -39,7 +39,7 @@ bool implement_get_buf(unsigned id, Http2FlowData* session_data, SourceId source if (session_data->frame_header[source_id] == nullptr) return false; b.data = session_data->frame_header[source_id]; - b.len = FRAME_HEADER_LENGTH; + b.len = session_data->frame_header_size[source_id]; break; case HTTP2_BUFFER_FRAME_DATA: if (session_data->frame_data[source_id] == nullptr) 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 9c31c68b7..7566f58ea 100644 --- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc +++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc @@ -38,6 +38,24 @@ static uint32_t get_frame_length(const uint8_t *frame_buffer) return (frame_buffer[0] << 16) + (frame_buffer[1] << 8) + frame_buffer[2]; } +static uint8_t get_frame_type(const uint8_t *frame_buffer) +{ + const uint8_t frame_type_index = 3; + if (frame_buffer) + return frame_buffer[frame_type_index]; + else + return FT__NONE; +} + +static uint8_t get_frame_flags(const uint8_t *frame_buffer) +{ + const uint8_t frame_flags_index = 4; + if (frame_buffer) + return frame_buffer[frame_flags_index]; + else + return NO_HEADER; +} + StreamSplitter::Status implement_scan(Http2FlowData* session_data, const uint8_t* data, uint32_t length, uint32_t* flush_offset, HttpCommon::SourceId source_id) { @@ -101,12 +119,13 @@ StreamSplitter::Status implement_scan(Http2FlowData* session_data, const uint8_t *flush_offset = 0; do { - if (session_data->frame_header[source_id] == nullptr) + if (session_data->currently_processing_frame_header[source_id] == nullptr) { session_data->header_coming[source_id] = true; - session_data->frame_header[source_id] = new uint8_t[FRAME_HEADER_LENGTH]; + session_data->currently_processing_frame_header[source_id] = new uint8_t[FRAME_HEADER_LENGTH]; session_data->octets_seen[source_id] = 0; session_data->header_octets_seen[source_id] = 0; + session_data->frames_aggregated[source_id] = 1; } // The first nine bytes are the frame header. But all nine might not all be present in the @@ -114,7 +133,8 @@ StreamSplitter::Status implement_scan(Http2FlowData* session_data, const uint8_t for (uint32_t k = 0; (k < length) && (session_data->header_octets_seen[source_id] < FRAME_HEADER_LENGTH); k++, session_data->header_octets_seen[source_id]++) { - session_data->frame_header[source_id][session_data->header_octets_seen[source_id]] = data[k]; + session_data->currently_processing_frame_header[source_id] + [session_data->header_octets_seen[source_id]] = data[k]; } if (session_data->header_octets_seen[source_id] < FRAME_HEADER_LENGTH) { @@ -122,10 +142,11 @@ StreamSplitter::Status implement_scan(Http2FlowData* session_data, const uint8_t status = StreamSplitter::SEARCH; break; } - int type = session_data->get_frame_type(source_id); + uint8_t type = get_frame_type(session_data->currently_processing_frame_header[source_id]); // Frame length does not include the frame header - uint32_t const frame_length = get_frame_length(session_data->frame_header[source_id]); + uint32_t const frame_length = get_frame_length(session_data-> + currently_processing_frame_header[source_id]); // For non-data frames, send a full frame to detection session_data->inspection_section_length[source_id] = frame_length + FRAME_HEADER_LENGTH; @@ -161,29 +182,23 @@ StreamSplitter::Status implement_scan(Http2FlowData* session_data, const uint8_t // Process all header frames as one unit - if the END_HEADERS flag is not set and scan // is out of data, tell stream to keep searching - if (type == FT_HEADERS && !(session_data->get_frame_flags(source_id) & END_HEADERS)) + uint8_t frame_flags = get_frame_flags(session_data-> + currently_processing_frame_header[source_id]); + if (type == FT_HEADERS && !(frame_flags & END_HEADERS)) { - session_data->continuation_expected = true; - // We need to save the header frame header for reassembly - session_data->header_frame_header[source_id] = new uint8_t[FRAME_HEADER_LENGTH]; - memcpy(session_data->header_frame_header[source_id], - session_data->frame_header[source_id], FRAME_HEADER_LENGTH); + session_data->continuation_expected[source_id] = true; - delete[] session_data->frame_header[source_id]; - session_data->frame_header[source_id] = nullptr; + session_data->header_octets_seen[source_id] = 0; + session_data->frames_aggregated[source_id] += 1; status = StreamSplitter::SEARCH; data += frame_length + FRAME_HEADER_LENGTH; } - else if ( type == FT_CONTINUATION && session_data->continuation_expected) + else if ( type == FT_CONTINUATION && session_data->continuation_expected[source_id]) { - if (!(session_data->get_frame_flags(source_id) & END_HEADERS)) + if (!(frame_flags & END_HEADERS)) { - // For continuation frames we only need the frame length - // FIXIT-M Need to verify that continuation frame has correct stream id - session_data->continuation_frame_lengths.push_back(frame_length); - - delete[] session_data->frame_header[source_id]; - session_data->frame_header[source_id] = nullptr; + session_data->header_octets_seen[source_id] = 0; + session_data->frames_aggregated[source_id] += 1; status = StreamSplitter::SEARCH; data += frame_length + FRAME_HEADER_LENGTH; } @@ -191,7 +206,7 @@ StreamSplitter::Status implement_scan(Http2FlowData* session_data, const uint8_t { // continuation frame ending headers status = StreamSplitter::FLUSH; - session_data->continuation_expected = false; + session_data->continuation_expected[source_id] = false; } } //FIXIT-M CONTINUATION frames can also follow PUSH_PROMISE frames, which is not @@ -202,7 +217,7 @@ StreamSplitter::Status implement_scan(Http2FlowData* session_data, const uint8_t session_data->events[source_id]->create_event(EVENT_UNEXPECTED_CONTINUATION); status = StreamSplitter::ABORT; } - else if (session_data->continuation_expected) + else if (session_data->continuation_expected[source_id]) { *session_data->infractions[source_id] += INF_MISSING_CONTINUATION; session_data->events[source_id]->create_event(EVENT_MISSING_CONTINUATION); @@ -225,109 +240,159 @@ const StreamBuffer implement_reassemble(Http2FlowData* session_data, unsigned to if (offset == 0) { - session_data->frame[source_id] = new uint8_t[total]; - session_data->frame_size[source_id] = total; - } - assert(session_data->frame_size[source_id] == total); - - memcpy(session_data->frame[source_id]+offset, data, len); - - if (flags & PKT_PDU_TAIL) - { - assert(offset+len == total); if (!session_data->header_coming[source_id]) + session_data->frame_data[source_id] = new uint8_t[total]; + else { - session_data->frame_data[source_id] = session_data->frame[source_id]; - session_data->frame_data_size[source_id] = session_data->frame_size[source_id]; - } - else if (session_data->frame_size[source_id] == FRAME_HEADER_LENGTH) - { - session_data->frame_data[source_id] = nullptr; - session_data->frame_data_size[source_id] = 0; + const uint32_t header_length = FRAME_HEADER_LENGTH * + session_data->frames_aggregated[source_id]; + session_data->frame_header[source_id] = new uint8_t[header_length]; + session_data->frame_header_size[source_id] = header_length; + if (total > FRAME_HEADER_LENGTH) + session_data->frame_data[source_id] = new uint8_t[total - header_length]; } - else + session_data->header_octets_seen[source_id] = 0; + session_data->frame_data_size[source_id] = 0; + session_data->frame_header_offset[source_id] = 0; + } + + if (!session_data->header_coming[source_id]) + { + memcpy(session_data->frame_data[source_id] + offset, data, len); + session_data->frame_data_size[source_id] += len; + } + else + { + uint32_t data_pos = 0; + do { - // Adjust for frame header - session_data->frame_data[source_id] = - session_data->frame[source_id] + FRAME_HEADER_LENGTH; - session_data->frame_data_size[source_id] = - session_data->frame_size[source_id] - FRAME_HEADER_LENGTH; - - const int type = session_data->get_frame_type(source_id); - - if (type == FT_HEADERS || type == FT_CONTINUATION) - { - assert(session_data->http2_decoded_header[source_id] == nullptr); - session_data->http2_decoded_header[source_id] = new uint8_t[MAX_OCTETS]; - uint8_t header_payload_offset = 0; - uint8_t pad_len = 0; + uint32_t remaining_len = len - data_pos; - uint8_t frame_flags; - uint32_t header_frame_length; - if (type == FT_HEADERS) - { - frame_flags = session_data->get_frame_flags(source_id); - header_frame_length = get_frame_length(session_data->frame_header[source_id]); - } - else + if (session_data->header_octets_seen[source_id] < FRAME_HEADER_LENGTH) + { + uint8_t remaining_header = FRAME_HEADER_LENGTH - + session_data->header_octets_seen[source_id]; + if (remaining_header > remaining_len) { - assert(session_data->header_frame_header[source_id] != nullptr); - frame_flags = session_data->header_frame_header[source_id][4]; - header_frame_length = get_frame_length(session_data->header_frame_header[source_id]); + memcpy(session_data->frame_header[source_id] + + session_data->frame_header_offset[source_id] + + session_data->header_octets_seen[source_id], data + data_pos, + remaining_len); + session_data->header_octets_seen[source_id] += remaining_len; + break; } + memcpy(session_data->frame_header[source_id] + + session_data->frame_header_offset[source_id] + + session_data->header_octets_seen[source_id], data + data_pos, + remaining_header); + session_data->header_octets_seen[source_id] += remaining_header; + data_pos += remaining_header; + remaining_len -= remaining_header; + } + + // done once per frame after we have the entire header + if (session_data->remaining_frame_data_octets[source_id] == 0) + { + uint32_t frame_length = 0; + uint32_t frame_data_offset = 0; + uint8_t pad_len = 0; + uint8_t frame_flags = 0; + + frame_length = get_frame_length(session_data->frame_header[source_id] + + session_data->frame_header_offset[source_id]); + frame_flags = get_frame_flags(session_data->frame_header[source_id] + + session_data->frame_header_offset[source_id]); if (frame_flags & PADDED) { - header_payload_offset += 1; + frame_data_offset += 1; pad_len = session_data->frame_data[source_id][0]; } //FIXIT-M handle stream dependency and weight. For now just skip over if (frame_flags & PRIORITY) { - header_payload_offset += 5; + frame_data_offset += 5; } + session_data->remaining_octets_to_next_header[source_id] = frame_length; + session_data->remaining_frame_data_octets[source_id] = frame_length - pad_len - frame_data_offset; + session_data->remaining_frame_data_offset[source_id] = frame_data_offset; + } - //FIXIT-H This will eventually be the decoded header buffer. For now just copy directly - uint32_t header_payload_len = header_frame_length - header_payload_offset - pad_len; - memcpy(session_data->http2_decoded_header[source_id], session_data->frame_data[source_id] + - header_payload_offset, header_payload_len); - session_data->http2_decoded_header_size[source_id] = header_payload_len; + if (remaining_len >= session_data->remaining_octets_to_next_header[source_id]) + { + // have the remainder of the full frame + memcpy(session_data->frame_data[source_id] + session_data->frame_data_size[source_id], + data + data_pos + session_data->remaining_frame_data_offset[source_id], + session_data->remaining_frame_data_octets[source_id]); + session_data->frame_data_size[source_id] += + session_data->remaining_frame_data_octets[source_id]; + data_pos += session_data->remaining_octets_to_next_header[source_id]; + session_data->remaining_octets_to_next_header[source_id] = 0; + session_data->remaining_frame_data_octets[source_id] = 0; + session_data->remaining_frame_data_offset[source_id] = 0; + session_data->header_octets_seen[source_id] = 0; + session_data->frame_header_offset[source_id] += FRAME_HEADER_LENGTH; + } + else if (remaining_len < session_data->remaining_frame_data_offset[source_id]) + { + // don't have the full stream dependency/weight, which precedes frame data + session_data->remaining_frame_data_offset[source_id] -= remaining_len; + session_data->remaining_octets_to_next_header[source_id] -= remaining_len; + return frame_buf; + } + else if (remaining_len < session_data->remaining_frame_data_octets[source_id]) + { + // don't have the full frame data + uint32_t data_len = remaining_len - session_data->remaining_frame_data_offset[source_id]; + memcpy(session_data->frame_data[source_id] + session_data->frame_data_size[source_id], + data + data_pos + session_data->remaining_frame_data_offset[source_id], + data_len); + session_data->frame_data_size[source_id] += data_len; + session_data->remaining_octets_to_next_header[source_id] -= remaining_len; + session_data->remaining_frame_data_octets[source_id] -= data_len; + session_data->remaining_frame_data_offset[source_id] = 0; + return frame_buf; + } + else + { + // have all the data but not all the padding following the data + memcpy(session_data->frame_data[source_id] + session_data->frame_data_size[source_id], + data + data_pos + session_data->remaining_frame_data_offset[source_id], + session_data->remaining_frame_data_octets[source_id]); + session_data->frame_data_size[source_id] += + session_data->remaining_frame_data_octets[source_id]; + session_data->remaining_octets_to_next_header[source_id] -= remaining_len; + session_data->remaining_frame_data_octets[source_id] = 0; + session_data->remaining_frame_data_offset[source_id] = 0; + } + } while (data_pos < len); + } - // check for continuation frames, skipping over frame headers - if (type == FT_CONTINUATION) - { - header_payload_offset += header_payload_len + pad_len + FRAME_HEADER_LENGTH; - for (uint32_t continuation_length : session_data->continuation_frame_lengths) - { - assert(header_payload_offset + continuation_length < total); - assert(session_data->http2_decoded_header_size[source_id] + continuation_length < - MAX_OCTETS); - memcpy(session_data->http2_decoded_header[source_id] + - session_data->http2_decoded_header_size[source_id], - session_data->frame_data[source_id] + header_payload_offset, - continuation_length); - session_data->http2_decoded_header_size[source_id] += continuation_length; - header_payload_offset += continuation_length + FRAME_HEADER_LENGTH; - } - - // The last continuation frame header is stored in the frame_header buffer - uint32_t final_continuation_length = - get_frame_length(session_data->frame_header[source_id]); - assert(header_payload_offset + final_continuation_length < total); - assert(session_data->http2_decoded_header_size[source_id] + - final_continuation_length < MAX_OCTETS); - memcpy(session_data->http2_decoded_header[source_id] + - session_data->http2_decoded_header_size[source_id], - session_data->frame_data[source_id] + header_payload_offset, - final_continuation_length); - session_data->http2_decoded_header_size[source_id] += final_continuation_length; - } + if (flags & PKT_PDU_TAIL) + { + if (session_data->header_coming[source_id]) + { + const uint8_t type = get_frame_type(session_data->frame_header[source_id]); + + if (type == FT_HEADERS || type == FT_CONTINUATION) + { + assert(session_data->http2_decoded_header[source_id] == nullptr); + session_data->http2_decoded_header[source_id] = new uint8_t[MAX_OCTETS]; + + //FIXIT-H This will eventually be the decoded header buffer. For now just copy + //directly + memcpy(session_data->http2_decoded_header[source_id], + session_data->frame_data[source_id], session_data->frame_data_size[source_id]); + session_data->http2_decoded_header_size[source_id] = session_data->frame_data_size[source_id]; } } // Return 0-length non-null buffer to stream which signals detection required, but don't // create pkt_data buffer frame_buf.length = 0; - frame_buf.data = session_data->frame[source_id]; + if (session_data->frame_data[source_id]) + frame_buf.data = session_data->frame_data[source_id]; + else + frame_buf.data = session_data->frame_header[source_id]; } return frame_buf; } diff --git a/src/service_inspectors/http2_inspect/test/http2_flow_data_test.h b/src/service_inspectors/http2_inspect/test/http2_flow_data_test.h index 443e8ff65..818d25081 100644 --- a/src/service_inspectors/http2_inspect/test/http2_flow_data_test.h +++ b/src/service_inspectors/http2_inspect/test/http2_flow_data_test.h @@ -42,13 +42,12 @@ public: void set_header_coming(bool value, HttpCommon::SourceId source_id) { header_coming[source_id] = value; } uint8_t* get_frame_header(HttpCommon::SourceId source_id) { return frame_header[source_id]; } - void set_frame_header(uint8_t* value, HttpCommon::SourceId source_id) - { frame_header[source_id] = value; } - uint8_t* get_frame(HttpCommon::SourceId source_id) { return frame[source_id]; } - void set_frame(uint8_t* value, HttpCommon::SourceId source_id) { frame[source_id] = value; } - uint32_t get_frame_size(HttpCommon::SourceId source_id) { return frame_size[source_id]; } - void set_frame_size(uint32_t value, HttpCommon::SourceId source_id) - { frame_size[source_id] = value; } + void set_frame_header(uint8_t* value, uint32_t length, HttpCommon::SourceId source_id) + { frame_header[source_id] = value; frame_header_size[source_id] = length; } + uint8_t* get_currently_processing_frame_header(HttpCommon::SourceId source_id) + { return currently_processing_frame_header[source_id]; } + void set_aggregated_frames(uint32_t value, HttpCommon::SourceId source_id) + { frames_aggregated[source_id] = value; } uint8_t* get_frame_data(HttpCommon::SourceId source_id) { return frame_data[source_id]; } void set_frame_data(uint8_t* value, HttpCommon::SourceId source_id) { frame_data[source_id] = value; } diff --git a/src/service_inspectors/http2_inspect/test/http2_inspect_impl_test.cc b/src/service_inspectors/http2_inspect/test/http2_inspect_impl_test.cc index 62efd5ec7..c9527e276 100644 --- a/src/service_inspectors/http2_inspect/test/http2_inspect_impl_test.cc +++ b/src/service_inspectors/http2_inspect/test/http2_inspect_impl_test.cc @@ -66,7 +66,8 @@ TEST(http2_get_buf_test, frame_header) { uint8_t* head_buf = new uint8_t[9]; memcpy(head_buf, "\x01\x02\x03\x04\x05\x06\x07\x08\x09", 9); - session_data->set_frame_header(head_buf, SRC_CLIENT); + session_data->set_frame_header(head_buf, 9, SRC_CLIENT); + session_data->set_aggregated_frames (1, SRC_CLIENT); const bool result = implement_get_buf(HTTP2_BUFFER_FRAME_HEADER, session_data, SRC_CLIENT, b); CHECK(result == true); CHECK(b.len == 9); @@ -91,7 +92,6 @@ TEST(http2_get_buf_test, frame_data) CHECK(result == true); CHECK(b.len == 26); CHECK(memcmp(b.data, "zyxwvutsrqponmlkjihgfedcba", 26) == 0); - delete[] data_buf; } TEST(http2_get_buf_test, frame_data_absent) diff --git a/src/service_inspectors/http2_inspect/test/http2_stream_splitter_impl_test.cc b/src/service_inspectors/http2_inspect/test/http2_stream_splitter_impl_test.cc index 2a0440fc3..85f7d8460 100644 --- a/src/service_inspectors/http2_inspect/test/http2_stream_splitter_impl_test.cc +++ b/src/service_inspectors/http2_inspect/test/http2_stream_splitter_impl_test.cc @@ -130,7 +130,7 @@ TEST(http2_scan_test, short_input) CHECK(result == StreamSplitter::FLUSH); CHECK(flush_offset == 19); CHECK(session_data->get_header_coming(SRC_SERVER)); - CHECK(memcmp(session_data->get_frame_header(SRC_SERVER), + CHECK(memcmp(session_data->get_currently_processing_frame_header(SRC_SERVER), "\x00\x00\x10\x04\x05\x06\x07\x08\x09", 9) == 0); } @@ -206,6 +206,7 @@ TEST_GROUP(http2_reassemble_test) TEST(http2_reassemble_test, basic_with_header) { session_data->set_header_coming(true, SRC_CLIENT); + session_data->set_aggregated_frames (1, SRC_CLIENT); implement_reassemble(session_data, 19, 0, (const uint8_t*)"\x00\x00\x0A\x02\x00\x00\x00\x00\x00" "0123456789", 19, PKT_PDU_TAIL, SRC_CLIENT); @@ -216,6 +217,7 @@ TEST(http2_reassemble_test, basic_with_header) TEST(http2_reassemble_test, basic_with_header_s2c) { session_data->set_header_coming(true, SRC_SERVER); + session_data->set_aggregated_frames (1, SRC_SERVER); implement_reassemble(session_data, 19, 0, (const uint8_t*)"\x00\x00\x0A\x02\x00\x00\x00\x00\x00" "0123456789", 19, PKT_PDU_TAIL, SRC_SERVER); @@ -236,16 +238,15 @@ TEST(http2_reassemble_test, basic_without_header) TEST(http2_reassemble_test, basic_three_pieces) { session_data->set_header_coming(true, SRC_CLIENT); + session_data->set_aggregated_frames (1, SRC_CLIENT); StreamBuffer buffer = implement_reassemble(session_data, 19, 0, (const uint8_t*)"\x00\x00\x0A\x02\x00\x00", 6, 0, SRC_CLIENT); CHECK(buffer.length == 0); CHECK(session_data->get_frame_data_size(SRC_CLIENT) == 0); - CHECK(session_data->get_frame_data(SRC_CLIENT) == nullptr); implement_reassemble(session_data, 19, 6, (const uint8_t*)"\x00\x00\x00" "01234", 8, 0, SRC_CLIENT); - CHECK(session_data->get_frame_data(SRC_CLIENT) == nullptr); buffer = implement_reassemble(session_data, 19, 14, (const uint8_t*)"56789", 5, PKT_PDU_TAIL, SRC_CLIENT); @@ -260,8 +261,8 @@ TEST(http2_reassemble_test, basic_without_header_two_pieces) implement_reassemble(session_data, 24, 0, (const uint8_t*)"P", 1, 0, SRC_CLIENT); - CHECK(session_data->get_frame_data_size(SRC_CLIENT) == 0); - CHECK(session_data->get_frame_data(SRC_CLIENT) == nullptr); + CHECK(session_data->get_frame_data_size(SRC_CLIENT) == 1); + CHECK(memcmp(session_data->get_frame_data(SRC_CLIENT), "P", 1) == 0); implement_reassemble(session_data, 24, 1, (const uint8_t*)"RI * HTTP/2.0\r\n\r\nSM\r\n\r\n", 23, PKT_PDU_TAIL, SRC_CLIENT);