From: Russ Combs Date: Thu, 19 Mar 2015 18:29:16 +0000 (-0400) Subject: eliminate dedicated nhttp chunk buffer; minor nhttp cleanup in StreamSplitter X-Git-Tag: 3.0.0-233~1018 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=635a9929547766a3ded09da1a830f3fea3fee191;p=thirdparty%2Fsnort3.git eliminate dedicated nhttp chunk buffer; minor nhttp cleanup in StreamSplitter --- diff --git a/ChangeLog b/ChangeLog index 0f993af58..4b8677086 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,8 @@ +Pending - build 143 + +-- eliminate dedicated nhttp chunk buffer +-- minor nhttp cleanup in StreamSplitter + 15/03/18 - build 142 -- fixed host lookup issue diff --git a/src/service_inspectors/nhttp_inspect/nhttp_flow_data.cc b/src/service_inspectors/nhttp_inspect/nhttp_flow_data.cc index f64c5bcf8..1b11e17b3 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_flow_data.cc +++ b/src/service_inspectors/nhttp_inspect/nhttp_flow_data.cc @@ -50,7 +50,6 @@ NHttpFlowData::~NHttpFlowData() { delete[] section_buffer[k]; } - delete[] chunk_buffer[k]; delete transaction[k]; delete splitter[k]; } @@ -90,8 +89,6 @@ void NHttpFlowData::show(FILE* out_file) const header_octets_visible[1]); fprintf(out_file, "Section buffer length: %d/%d\n", section_buffer_length[0], section_buffer_length[1]); - fprintf(out_file, "Chunk buffer length: %d/%d\n", chunk_buffer_length[0], - chunk_buffer_length[1]); fprintf(out_file, "Pipelining: front %d back %d overflow %d underflow %d\n", pipeline_front, pipeline_back, pipeline_overflow, pipeline_underflow); diff --git a/src/service_inspectors/nhttp_inspect/nhttp_flow_data.h b/src/service_inspectors/nhttp_inspect/nhttp_flow_data.h index 2da65fd37..c76a7feb2 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_flow_data.h +++ b/src/service_inspectors/nhttp_inspect/nhttp_flow_data.h @@ -60,12 +60,6 @@ private: uint8_t* section_buffer[2] = { nullptr, nullptr }; int32_t section_buffer_length[2] = { 0, 0 }; bool section_buffer_owned[2] = { true, true }; - uint8_t* chunk_buffer[2] = { nullptr, nullptr }; - int32_t chunk_buffer_length[2] = { 0, 0 }; - - // never actually false and not checked, just here to simplify code - bool chunk_buffer_owned[2] = { true, true }; - bool zero_chunk[2] = { false, false }; NHttpInfractions chunk_infractions[2]; diff --git a/src/service_inspectors/nhttp_inspect/nhttp_msg_body.h b/src/service_inspectors/nhttp_inspect/nhttp_msg_body.h index d689aabda..a302063eb 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_msg_body.h +++ b/src/service_inspectors/nhttp_inspect/nhttp_msg_body.h @@ -39,7 +39,7 @@ public: void legacy_clients() override; protected: - int64_t data_length; + int64_t data_length; // FIXIT-M this has no meaning in chunk subclass. Potential source of errors. int64_t body_octets; Field data; diff --git a/src/service_inspectors/nhttp_inspect/nhttp_normalizers.cc b/src/service_inspectors/nhttp_inspect/nhttp_normalizers.cc index 753bdd502..3bfe3ece6 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_normalizers.cc +++ b/src/service_inspectors/nhttp_inspect/nhttp_normalizers.cc @@ -27,8 +27,8 @@ using namespace NHttpEnums; // Collection of stock normalization functions. This will probably grow throughout the life of the -// software. New functions must follow the standard signature. -// The void* at the end is for any special configuration data the function requires. +// software. New functions must follow the standard signature. The void* at the end is for any +// special configuration data the function requires. int32_t norm_decimal_integer(const uint8_t* in_buf, int32_t in_length, uint8_t* out_buf, NHttpInfractions& infractions, const void*) diff --git a/src/service_inspectors/nhttp_inspect/nhttp_scratch_pad.h b/src/service_inspectors/nhttp_inspect/nhttp_scratch_pad.h index d552c192d..bce63b5a1 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_scratch_pad.h +++ b/src/service_inspectors/nhttp_inspect/nhttp_scratch_pad.h @@ -22,7 +22,7 @@ //------------------------------------------------------------------------- // ScratchPad class -// Memory management for NHttpMsgHeader class +// Storage management class //------------------------------------------------------------------------- // Working space and storage for all the derived fields diff --git a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc index bcab38325..46fd47ee1 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc +++ b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc @@ -48,7 +48,7 @@ void NHttpStreamSplitter::prepare_flush(NHttpFlowData* session_data, uint32_t* f paf_max = DATABLOCKSIZE; break; case SEC_CHUNK: - paf_max = DATABLOCKSIZE - session_data->chunk_buffer_length[source_id]; + paf_max = DATABLOCKSIZE - session_data->section_buffer_length[source_id]; break; default: paf_max = MAXOCTETS; @@ -288,7 +288,8 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total, if (!NHttpTestManager::use_test_input() && NHttpTestManager::use_test_output()) { printf("reassemble() from flow %p direction %d total %u length %u offset %u\n", - (void*)flow, 1 - (int)to_server(), total, len, offset); fflush(nullptr); + (void*)flow, 1 - (int)to_server(), total, len, offset); + fflush(stdout); } NHttpFlowData* session_data = (NHttpFlowData*)flow->get_application_data( @@ -296,28 +297,31 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total, assert(session_data != nullptr); copied = len; - if (NHttpTestManager::use_test_input()) + if (NHttpTestManager::use_test_output()) { - if (!(flags & PKT_PDU_TAIL)) + if (NHttpTestManager::use_test_input()) { - return nullptr; + if (!(flags & PKT_PDU_TAIL)) + { + return nullptr; + } + uint8_t* test_buffer; + NHttpTestManager::get_test_input_source()->reassemble(&test_buffer, len, source_id, + session_data, tcp_close); + if (test_buffer == nullptr) + { + // Source ID does not match test data, no test data was flushed, or there is no + // more test data + return nullptr; + } + data = test_buffer; + offset = 0; } - uint8_t* test_buffer; - NHttpTestManager::get_test_input_source()->reassemble(&test_buffer, len, source_id, - session_data, tcp_close); - if (test_buffer == nullptr) + else { - // Source ID does not match test data, no test data was flushed, or there is no more - // test data - return nullptr; + printf("Reassemble from flow data %p direction %d\n", (void*)session_data, source_id); + fflush(stdout); } - data = test_buffer; - offset = 0; - } - else if (NHttpTestManager::use_test_output()) - { - printf("Reassemble from flow data %p direction %d\n", (void*)session_data, source_id); - fflush(stdout); } if (session_data->section_type[source_id] == SEC__NOTCOMPUTE) @@ -346,28 +350,20 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total, return nullptr; } - bool is_chunk = (session_data->section_type[source_id] == SEC_CHUNK); - - uint8_t*& chunk_buffer = session_data->chunk_buffer[source_id]; - int32_t& chunk_buffer_length = session_data->chunk_buffer_length[source_id]; - uint8_t*& buffer = !is_chunk ? session_data->section_buffer[source_id] : chunk_buffer; - int32_t& buffer_length = !is_chunk ? session_data->section_buffer_length[source_id] : - chunk_buffer_length; - bool& buffer_owned = !is_chunk ? session_data->section_buffer_owned[source_id] : - session_data->chunk_buffer_owned[source_id]; + uint8_t*& buffer = session_data->section_buffer[source_id]; + int32_t& buffer_length = session_data->section_buffer_length[source_id]; if (buffer == nullptr) { buffer = new uint8_t[MAXOCTETS]; - assert(buffer != nullptr); - buffer_owned = true; + session_data->section_buffer_owned[source_id] = true; } memcpy(buffer + buffer_length + offset, data, len); if (flags & PKT_PDU_TAIL) { ProcessResult send_to_detection; - if (!is_chunk) + if (session_data->section_type[source_id] != SEC_CHUNK) { // start line/headers/body individual section processing with aggregation prior to // being sent to detection @@ -391,20 +387,20 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total, // FIXIT-M this implementation of the zero-length chunk is temporary until stream can // support a zero- // octet flush. - const int32_t total_chunk_len = chunk_buffer_length + offset + len - - session_data->zero_chunk[source_id]; + const int32_t total_chunk_len = buffer_length + offset + len - + (int)session_data->zero_chunk[source_id]; if ((total_chunk_len < DATABLOCKSIZE) && (!session_data->zero_chunk[source_id]) && !tcp_close) { - chunk_buffer_length = total_chunk_len; + buffer_length = total_chunk_len; return nullptr; } if (total_chunk_len == 0) { // Zero-length chunk cannot be processed by itself. - delete[] chunk_buffer; - chunk_buffer = nullptr; - chunk_buffer_length = 0; + delete[] buffer; + buffer = nullptr; + buffer_length = 0; // zero-length chunk is not visible to inspector. Transition to trailer must be // handled here. session_data->section_type[source_id] = SEC__NOTCOMPUTE; @@ -414,8 +410,8 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total, paf_max = DATABLOCKSIZE; session_data->infractions[source_id] = session_data->chunk_infractions[source_id]; session_data->chunk_infractions[source_id] = NHttpInfractions(); - send_to_detection = my_inspector->process(chunk_buffer, total_chunk_len, flow, - source_id, true); + send_to_detection = my_inspector->process(buffer, total_chunk_len, flow, source_id, + true); if (session_data->zero_chunk[source_id]) { // zero-length chunk is not visible to inspector. Transition to trailer must be @@ -448,7 +444,7 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned total, return nullptr; case RES_AGGREGATE: buffer_length += offset + len; - buffer_owned = false; + session_data->section_buffer_owned[source_id] = false; return nullptr; } } diff --git a/src/service_inspectors/nhttp_inspect/nhttp_test_input.cc b/src/service_inspectors/nhttp_inspect/nhttp_test_input.cc index a141e1eca..fc29dac22 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_test_input.cc +++ b/src/service_inspectors/nhttp_inspect/nhttp_test_input.cc @@ -302,7 +302,10 @@ void NHttpTestInput::reassemble(uint8_t** buffer, unsigned& length, SourceId sou // multiple times as we generate all the maximum size body sections needed for a single // flush. tcp_close = false; - const unsigned paf_max = DATABLOCKSIZE - session_data->chunk_buffer_length[source_id]; + unsigned paf_max = DATABLOCKSIZE; + if (session_data->section_type[source_id] == SEC_CHUNK) { + paf_max -= session_data->section_buffer_length[source_id]; + } length = (flush_octets <= paf_max) ? flush_octets : paf_max; for (uint32_t k = end_offset; k < length; k++) {