From: Mike Stepanek (mstepane) Date: Thu, 10 Oct 2019 18:50:35 +0000 (-0400) Subject: Merge pull request #1791 in SNORT/snort3 from ~THOPETER/snort3:http2_cleanup to master X-Git-Tag: 3.0.0-263~28 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ece1c91241114df90b83aa7c38494dffdda686b3;p=thirdparty%2Fsnort3.git Merge pull request #1791 in SNORT/snort3 from ~THOPETER/snort3:http2_cleanup to master Squashed commit of the following: commit a36d5d0cb46a91592a7edbf061f9af4c9ee7beae Author: Tom Peters Date: Wed Oct 9 16:47:52 2019 -0400 http2_inspect: cleanup --- diff --git a/src/service_inspectors/ftp_telnet/ftp_parse.cc b/src/service_inspectors/ftp_telnet/ftp_parse.cc index 7121512c4..fe1f71538 100644 --- a/src/service_inspectors/ftp_telnet/ftp_parse.cc +++ b/src/service_inspectors/ftp_telnet/ftp_parse.cc @@ -592,7 +592,7 @@ int ProcessFTPCmdValidity( iRet = DoNextFormat(HeadFmt, 0, ErrorString, ErrStrLen); - /* Need to check to be sure we got a complete command */ + /* Need to check to be sure we got a complete command */ if (iRet) { return FTPP_FATAL_ERR; diff --git a/src/service_inspectors/ftp_telnet/ftpp_return_codes.h b/src/service_inspectors/ftp_telnet/ftpp_return_codes.h index 6e0460149..5db4bf6bc 100644 --- a/src/service_inspectors/ftp_telnet/ftpp_return_codes.h +++ b/src/service_inspectors/ftp_telnet/ftpp_return_codes.h @@ -35,7 +35,6 @@ #ifndef FTPP_RETURN_CODES_H #define FTPP_RETURN_CODES_H -#define FTPP_BOOL_FALSE 0 #define FTPP_SUCCESS 0 /* diff --git a/src/service_inspectors/http2_inspect/dev_notes.txt b/src/service_inspectors/http2_inspect/dev_notes.txt index c636cc013..204ab473f 100644 --- a/src/service_inspectors/http2_inspect/dev_notes.txt +++ b/src/service_inspectors/http2_inspect/dev_notes.txt @@ -1,14 +1,16 @@ The HTTP/2 inspector (H2I) will convert HTTP/2 frames into HTTP/1.1 message sections and feed them to the new HTTP inspector (NHI) for further processing. -The current implementation is the very first step. It splits an HTTP/2 stream into frame sections and -forwards them for inspection. It does not interface with NHI and does not address the multiplexed -nature of HTTP/2. +The current implementation is the very first step. It splits an HTTP/2 stream into frame sections +and forwards them for inspection. It does not interface with NHI and does not address the +multiplexed nature of HTTP/2. The Http2StreamSplitter strips the frame headers from the frame data and stores them in separate -buffers. As in NHI, long data frames are split into 16kb chunks for inspection. If a header frame is -followed by continuation frames, all the header frames are flushed together for inspection. The +buffers. As in NHI, long data frames are split into 16kb chunks for inspection. If a header frame +is followed by continuation frames, all the header frames are flushed together for inspection. The frame headers from each frame are stored contiguously in the frame_header buffer. After cutting out the frame headers, the frame data is stored as a single block, consisting of the HPACK encoded HTTP/2 headers. HPACK decoding is under ongoing development. Decoded headers will be stored in the http2_decoded_header buffer. + +H2I supports the NHI test tool. See ../http_inspect/dev_notes.txt for usage instructions. 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 18fcc4cee..eae9fcb60 100644 --- a/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc +++ b/src/service_inspectors/http2_inspect/http2_stream_splitter_impl.cc @@ -273,6 +273,7 @@ const StreamBuffer implement_reassemble(Http2FlowData* session_data, unsigned to if (offset == 0) { + // This is the first reassemble() for this frame and we need to allocate some buffers if (!session_data->header_coming[source_id]) session_data->frame_data[source_id] = new uint8_t[total]; else @@ -299,8 +300,12 @@ const StreamBuffer implement_reassemble(Http2FlowData* session_data, unsigned to uint32_t data_pos = 0; do { + // Each pass through the loop handles one frame + // Multiple frames occur when a Headers frame and Continuation frame(s) are flushed + // together uint32_t remaining_len = len - data_pos; + // Process the frame header if (session_data->header_octets_seen[source_id] < FRAME_HEADER_LENGTH) { uint8_t remaining_header = FRAME_HEADER_LENGTH - @@ -347,7 +352,8 @@ const StreamBuffer implement_reassemble(Http2FlowData* session_data, unsigned to 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_octets[source_id] = + frame_length - pad_len - frame_data_offset; session_data->remaining_frame_data_offset[source_id] = frame_data_offset; } @@ -405,26 +411,19 @@ const StreamBuffer implement_reassemble(Http2FlowData* session_data, unsigned to { 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) + if (get_frame_type(session_data->frame_header[source_id]) == FT_HEADERS) { assert(session_data->http2_decoded_header[source_id] == nullptr); - //FIXIT-H This will eventually be the decoded header buffer. For now just copy - //directly - if (!decode_headers(session_data, source_id, session_data->frame_data[source_id], - session_data->frame_data_size[source_id])) + // FIXIT-H This will eventually be the decoded header buffer. Under development. + if (!decode_headers(session_data, source_id, session_data->frame_data[source_id], + session_data->frame_data_size[source_id])) return frame_buf; } } // Return 0-length non-null buffer to stream which signals detection required, but don't // create pkt_data buffer - frame_buf.length = 0; - 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]; + frame_buf.data = (const uint8_t*)""; } return frame_buf; } @@ -434,9 +433,8 @@ ValidationResult validate_preface(const uint8_t* data, const uint32_t length, { const uint32_t preface_length = 24; - static const uint8_t connection_prefix[] = {'P', 'R', 'I', ' ', '*', ' ', - 'H', 'T', 'T', 'P', '/', '2', '.', '0', '\r', '\n', '\r', '\n', 'S', 'M', - '\r', '\n', '\r', '\n'}; + static const uint8_t connection_prefix[] = {'P', 'R', 'I', ' ', '*', ' ', 'H', 'T', 'T', 'P', + '/', '2', '.', '0', '\r', '\n', '\r', '\n', 'S', 'M', '\r', '\n', '\r', '\n'}; assert(octets_seen < preface_length); @@ -681,8 +679,8 @@ bool decode_header_line(Http2FlowData* session_data, HttpCommon::SourceId source encoded_header_length, Http2StreamSplitter::decode_int5, bytes_consumed, bytes_written); } -//FIXIT-H This will eventually be the decoded header buffer. For now only string literals are -//decoded +// FIXIT-H This will eventually be the decoded header buffer. For now only string literals are +// decoded bool decode_headers(Http2FlowData* session_data, HttpCommon::SourceId source_id, const uint8_t* encoded_header_buffer, const uint32_t header_length) { @@ -714,9 +712,8 @@ bool decode_headers(Http2FlowData* session_data, HttpCommon::SourceId source_id, #ifdef REG_TEST if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP2)) { - fprintf(HttpTestManager::get_output_file(), - "Error decoding headers. "); - if(session_data->http2_decoded_header_size[source_id] > 0) + fprintf(HttpTestManager::get_output_file(), "Error decoding headers. "); + if (session_data->http2_decoded_header_size[source_id] > 0) Field(session_data->http2_decoded_header_size[source_id], session_data->http2_decoded_header[source_id]).print( HttpTestManager::get_output_file(), "Partially Decoded Header"); @@ -725,7 +722,7 @@ bool decode_headers(Http2FlowData* session_data, HttpCommon::SourceId source_id, return false; } - // write the last crlf to end the header + // write the last CRLF to end the header if (!write_decoded_headers(session_data, source_id, (const uint8_t*)"\r\n", 2, session_data->http2_decoded_header[source_id] + session_data->http2_decoded_header_size[source_id], MAX_OCTETS - @@ -734,12 +731,12 @@ bool decode_headers(Http2FlowData* session_data, HttpCommon::SourceId source_id, session_data->http2_decoded_header_size[source_id] += line_bytes_written; #ifdef REG_TEST - if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP2)) - { - Field(session_data->http2_decoded_header_size[source_id], - session_data->http2_decoded_header[source_id]).print(HttpTestManager::get_output_file(), - "Decoded Header"); - } + if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP2)) + { + Field(session_data->http2_decoded_header_size[source_id], + session_data->http2_decoded_header[source_id]). + print(HttpTestManager::get_output_file(), "Decoded Header"); + } #endif return success;