From: Michael Altizer (mialtize) Date: Thu, 10 Nov 2016 20:06:51 +0000 (-0500) Subject: Merge pull request #699 in SNORT/snort3 from nhttp57 to master X-Git-Tag: 3.0.0-233~194 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=4568af01bf41791b6a43f00fbb529787d866348d;p=thirdparty%2Fsnort3.git Merge pull request #699 in SNORT/snort3 from nhttp57 to master Squashed commit of the following: commit 654554489f6827965f1052c224bf498d7c36b8ce Author: Tom Peters Date: Mon Oct 31 11:22:18 2016 -0400 NHI MIME file processing integration --- diff --git a/src/service_inspectors/http_inspect/http_flow_data.cc b/src/service_inspectors/http_inspect/http_flow_data.cc index 4e82854bf..a093b1d75 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.cc +++ b/src/service_inspectors/http_inspect/http_flow_data.cc @@ -68,11 +68,10 @@ HttpFlowData::~HttpFlowData() inflateEnd(compress_stream[k]); delete compress_stream[k]; } - } - - if (mime_state != nullptr) - { - delete mime_state; + if (mime_state[k] != nullptr) + { + delete mime_state[k]; + } } if (utf_state != nullptr ) @@ -101,6 +100,11 @@ void HttpFlowData::half_reset(SourceId source_id) delete compress_stream[source_id]; compress_stream[source_id] = nullptr; } + if (mime_state[source_id] != nullptr) + { + delete mime_state[source_id]; + mime_state[source_id] = nullptr; + } infractions[source_id].reset(); events[source_id].reset(); section_offset[source_id] = 0; @@ -112,11 +116,6 @@ void HttpFlowData::half_reset(SourceId source_id) type_expected[SRC_CLIENT] = SEC_REQUEST; expected_trans_num[SRC_CLIENT]++; method_id = METH__NOT_PRESENT; - if (mime_state != nullptr) - { - delete mime_state; - mime_state = nullptr; - } } else { @@ -202,6 +201,11 @@ void HttpFlowData::show(FILE* out_file) const fprintf(out_file, "Body octets: %" PRIi64 "/%" PRIi64 "\n", body_octets[0], body_octets[1]); fprintf(out_file, "Pipelining: front %d back %d overflow %d underflow %d\n", pipeline_front, pipeline_back, pipeline_overflow, pipeline_underflow); + fprintf(out_file, "Cutter: %s/%s\n", (cutter[0] != nullptr) ? "Present" : "nullptr", + (cutter[1] != nullptr) ? "Present" : "nullptr"); + fprintf(out_file, "utf_state: %s\n", (utf_state != nullptr) ? "Present" : "nullptr"); + fprintf(out_file, "mime_state: %s/%s\n", (mime_state[0] != nullptr) ? "Present" : "nullptr", + (mime_state[1] != nullptr) ? "Present" : "nullptr"); } #endif diff --git a/src/service_inspectors/http_inspect/http_flow_data.h b/src/service_inspectors/http_inspect/http_flow_data.h index 86546db08..dd9ed9552 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.h +++ b/src/service_inspectors/http_inspect/http_flow_data.h @@ -107,7 +107,7 @@ private: HttpEnums::STAT_NOT_PRESENT }; int64_t detect_depth_remaining[2] = { HttpEnums::STAT_NOT_PRESENT, HttpEnums::STAT_NOT_PRESENT }; - MimeSession* mime_state = nullptr; // SRC_CLIENT only + MimeSession* mime_state[2] = { nullptr, nullptr }; UtfDecodeSession* utf_state = nullptr; //SRC_SERVER only uint64_t expected_trans_num[2] = { 1, 1 }; diff --git a/src/service_inspectors/http_inspect/http_head_norm.cc b/src/service_inspectors/http_inspect/http_head_norm.cc index daac58537..d1e4ea072 100644 --- a/src/service_inspectors/http_inspect/http_head_norm.cc +++ b/src/service_inspectors/http_inspect/http_head_norm.cc @@ -77,9 +77,7 @@ void HeaderNormalizer::normalize(const HeaderId head_id, const int count, int num_matches = 0; int32_t buffer_length = 0; - // FIXIT-P initialization that serves no functional purpose to prevent compiler warning - int curr_match = -1; - + int curr_match; for (int k=0; k < num_headers; k++) { if (header_name_id[k] == head_id) diff --git a/src/service_inspectors/http_inspect/http_msg_body.cc b/src/service_inspectors/http_inspect/http_msg_body.cc index 55f69bc67..ed85bb778 100644 --- a/src/service_inspectors/http_inspect/http_msg_body.cc +++ b/src/service_inspectors/http_inspect/http_msg_body.cc @@ -57,25 +57,24 @@ void HttpMsgBody::analyze() do_utf_decoding(msg_text, decoded_body, decoded_body_alloc); if ( decoded_body_alloc ) { - detect_data.length = (decoded_body.length <= session_data->detect_depth_remaining[source_id]) ? - decoded_body.length : session_data->detect_depth_remaining[source_id]; - detect_data.start = decoded_body.start; + detect_data.set((decoded_body.length <= session_data->detect_depth_remaining[source_id]) ? + decoded_body.length : session_data->detect_depth_remaining[source_id], + decoded_body.start); } else { - detect_data.length = (msg_text.length <= session_data->detect_depth_remaining[source_id]) ? - msg_text.length : session_data->detect_depth_remaining[source_id]; - detect_data.start = msg_text.start; + detect_data.set((msg_text.length <= session_data->detect_depth_remaining[source_id]) ? + msg_text.length : session_data->detect_depth_remaining[source_id], + msg_text.start); } session_data->detect_depth_remaining[source_id] -= detect_data.length; // Always set file data. File processing will later set a new value in some cases. - file_data.length = detect_data.length; + file_data.set(detect_data); if (file_data.length > 0) { - file_data.start = detect_data.start; set_file_data(const_cast(file_data.start), (unsigned)file_data.length); } @@ -137,7 +136,7 @@ void HttpMsgBody::do_file_processing() const int32_t fp_length = (file_data.length <= session_data->file_depth_remaining[source_id]) ? file_data.length : session_data->file_depth_remaining[source_id]; - if (!session_data->mime_state) + if (!session_data->mime_state[source_id]) { FileFlows* file_flows = FileFlows::get_file_flows(flow); const bool download = (source_id == SRC_SERVER); @@ -169,14 +168,14 @@ void HttpMsgBody::do_file_processing() } else { - session_data->mime_state->process_mime_data(flow, file_data.start, + session_data->mime_state[source_id]->process_mime_data(flow, file_data.start, fp_length, true, SNORT_FILE_POSITION_UNKNOWN); session_data->file_depth_remaining[source_id] -= fp_length; if (session_data->file_depth_remaining[source_id] == 0) { - delete session_data->mime_state; - session_data->mime_state = nullptr; + delete session_data->mime_state[source_id]; + session_data->mime_state[source_id] = nullptr; } } } diff --git a/src/service_inspectors/http_inspect/http_msg_body_chunk.cc b/src/service_inspectors/http_inspect/http_msg_body_chunk.cc index 08f2f537e..7e2a4a5cb 100644 --- a/src/service_inspectors/http_inspect/http_msg_body_chunk.cc +++ b/src/service_inspectors/http_inspect/http_msg_body_chunk.cc @@ -30,15 +30,15 @@ using namespace HttpEnums; void HttpMsgBodyChunk::update_flow() { - // Cutter deleted when zero-length chunk received + // Cutter was deleted by splitter when zero-length chunk received if (session_data->cutter[source_id] == nullptr) { session_data->body_octets[source_id] = body_octets; session_data->trailer_prep(source_id); - if ((source_id == SRC_CLIENT) && (session_data->mime_state != nullptr)) + if (session_data->mime_state[source_id] != nullptr) { - delete session_data->mime_state; - session_data->mime_state = nullptr; + delete session_data->mime_state[source_id]; + session_data->mime_state[source_id] = nullptr; } if ((source_id == SRC_SERVER) && (session_data->utf_state != nullptr)) diff --git a/src/service_inspectors/http_inspect/http_msg_head_shared.cc b/src/service_inspectors/http_inspect/http_msg_head_shared.cc index 382523f2f..14a866e58 100644 --- a/src/service_inspectors/http_inspect/http_msg_head_shared.cc +++ b/src/service_inspectors/http_inspect/http_msg_head_shared.cc @@ -79,9 +79,8 @@ void HttpMsgHeadShared::create_norm_head_list() { headers_present[header_name_id[j]] = true; NormalizedHeader* tmp_ptr = norm_heads; - norm_heads = new NormalizedHeader; + norm_heads = new NormalizedHeader(header_name_id[j]); norm_heads->next = tmp_ptr; - norm_heads->id = header_name_id[j]; norm_heads->count = 1; } } @@ -299,6 +298,22 @@ const Field& HttpMsgHeadShared::get_classic_norm_cookie() classic_norm_cookie_alloc, params->uri_param); } +const Field& HttpMsgHeadShared::get_header_value_raw(HeaderId header_id) const +{ + // If the same header field appears more than once the first one will be returned. + if (!headers_present[header_id]) + return Field::FIELD_NULL; + for (int k=0; k < num_headers; k++) + { + if (header_name_id[k] == header_id) + { + return header_value[k]; + } + } + assert(false); + return Field::FIELD_NULL; +} + const Field& HttpMsgHeadShared::get_header_value_norm(HeaderId header_id) { NormalizedHeader* node = get_header_node(header_id); diff --git a/src/service_inspectors/http_inspect/http_msg_head_shared.h b/src/service_inspectors/http_inspect/http_msg_head_shared.h index 2aebba19a..7300aadff 100644 --- a/src/service_inspectors/http_inspect/http_msg_head_shared.h +++ b/src/service_inspectors/http_inspect/http_msg_head_shared.h @@ -36,15 +36,11 @@ class HttpMsgHeadShared : public HttpMsgSection public: void analyze() override; - int32_t get_num_headers() const { return num_headers; } const Field& get_classic_raw_header(); const Field& get_classic_raw_cookie(); const Field& get_classic_norm_header(); const Field& get_classic_norm_cookie(); - const Field& get_header_line(int k) const { return header_line[k]; } - const Field& get_header_name(int k) const { return header_name[k]; } - const Field& get_header_value(int k) const { return header_value[k]; } - HttpEnums::HeaderId get_header_name_id(int k) const { return header_name_id[k]; } + const Field& get_header_value_raw(HttpEnums::HeaderId header_id) const; const Field& get_header_value_norm(HttpEnums::HeaderId header_id); int get_header_count(HttpEnums::HeaderId header_id) const; @@ -64,6 +60,8 @@ protected: ~HttpMsgHeadShared(); // Get the next item in a comma-separated header value and convert it to an enum value static int32_t get_next_code(const Field& field, int32_t& offset, const StrCode table[]); + // Do a case insensitve search for "boundary=" in a Field + static bool boundary_present(const Field& field); #ifdef REG_TEST void print_headers(FILE* output); @@ -110,7 +108,8 @@ private: struct NormalizedHeader { - HttpEnums::HeaderId id; + NormalizedHeader(HttpEnums::HeaderId id_) : id(id_) {} + const HttpEnums::HeaderId id; int count; Field norm; NormalizedHeader* next; diff --git a/src/service_inspectors/http_inspect/http_msg_head_shared_util.cc b/src/service_inspectors/http_inspect/http_msg_head_shared_util.cc index ac021e2b6..164771a0f 100644 --- a/src/service_inspectors/http_inspect/http_msg_head_shared_util.cc +++ b/src/service_inspectors/http_inspect/http_msg_head_shared_util.cc @@ -18,6 +18,7 @@ // http_msg_head_shared_util.cc author Tom Peters #include "http_msg_head_shared.h" +#include int32_t HttpMsgHeadShared::get_next_code(const Field& field, int32_t& offset, const StrCode table[]) @@ -30,3 +31,19 @@ int32_t HttpMsgHeadShared::get_next_code(const Field& field, int32_t& offset, return str_to_code(start, length, table); } +// Case insensitive search for the substring "boundary=" +bool HttpMsgHeadShared::boundary_present(const Field& field) +{ + assert(field.length > 0); + + const char* const BOUNDARY = "boundary="; + const int BOUNDARY_LEN = 9; + + for (int k = 0; k + BOUNDARY_LEN <= field.length; k++) + { + if (strncasecmp(BOUNDARY, (const char*)(field.start + k), BOUNDARY_LEN) == 0) + return true; + } + return false; +} + diff --git a/src/service_inspectors/http_inspect/http_msg_header.cc b/src/service_inspectors/http_inspect/http_msg_header.cc index c1cd1cddd..8c0bd7525 100644 --- a/src/service_inspectors/http_inspect/http_msg_header.cc +++ b/src/service_inspectors/http_inspect/http_msg_header.cc @@ -50,7 +50,7 @@ void HttpMsgHeader::publish() { get_data_bus().publish(HTTP_REQUEST_HEADER_EVENT_KEY, http_event, flow); } - else if(source_id == SRC_SERVER) + else { get_data_bus().publish(HTTP_RESPONSE_HEADER_EVENT_KEY, http_event, flow); } @@ -213,27 +213,41 @@ void HttpMsgHeader::setup_file_processing() { // FIXIT-M Bidirectional file processing is problematic so we don't do it. When the library // fully supports it remove the outer if statement that prevents it from being done. - // In addition, mime_state needs to become bidirectional. if (session_data->file_depth_remaining[1-source_id] <= 0) { - if ((session_data->file_depth_remaining[source_id] = FileService::get_max_file_depth()) < 0) + if ((session_data->file_depth_remaining[source_id] = FileService::get_max_file_depth()) + < 0) { session_data->file_depth_remaining[source_id] = 0; return; } - // FIXIT-M check boundary to make sure this is not MIME for sure - if ((source_id == SRC_CLIENT) and (get_header_value_norm(HEAD_CONTENT_TYPE).length > 0 )) + + // Do we meet all the conditions for MIME file processing? + if (source_id == SRC_CLIENT) { - session_data->mime_state = new MimeSession(&decode_conf, &mime_conf); - // FIX-H use specific header instead of classic raw header - const Field& headers = get_classic_raw_header(); - if (headers.length > 0) + const Field& content_type = get_header_value_raw(HEAD_CONTENT_TYPE); + if (content_type.length > 0) { - session_data->mime_state->process_mime_data(flow, headers.start, - headers.length, true, SNORT_FILE_POSITION_UNKNOWN); + if (boundary_present(content_type)) + { + session_data->mime_state[source_id] = + new MimeSession(&decode_conf, &mime_conf); + // Show file processing the Content-Type header as if it were regular data. + // This will enable it to find the boundary string. + // FIXIT-L develop a proper interface for passing the boundary string. + // This interface is a leftover from when OHI pushed whole messages through + // this interface. + session_data->mime_state[source_id]->process_mime_data(flow, + content_type.start, content_type.length, true, + SNORT_FILE_POSITION_UNKNOWN); + session_data->mime_state[source_id]->process_mime_data(flow, + (const uint8_t*)"\r\n", 2, true, SNORT_FILE_POSITION_UNKNOWN); + } } } - else + + // Otherwise do regular file processing + if (session_data->mime_state[source_id] == nullptr) { FileFlows* file_flows = FileFlows::get_file_flows(flow); if (!file_flows) diff --git a/src/service_inspectors/http_inspect/http_msg_request.cc b/src/service_inspectors/http_inspect/http_msg_request.cc index eb7185e00..cd895e6c3 100644 --- a/src/service_inspectors/http_inspect/http_msg_request.cc +++ b/src/service_inspectors/http_inspect/http_msg_request.cc @@ -17,7 +17,6 @@ //-------------------------------------------------------------------------- // http_msg_request.cc author Tom Peters -#include #include #include #include diff --git a/src/service_inspectors/http_inspect/http_msg_section.cc b/src/service_inspectors/http_inspect/http_msg_section.cc index c3c641a86..4610ba750 100644 --- a/src/service_inspectors/http_inspect/http_msg_section.cc +++ b/src/service_inspectors/http_inspect/http_msg_section.cc @@ -51,7 +51,9 @@ HttpMsgSection::HttpMsgSection(const uint8_t* buffer, const uint16_t buf_size, method_id((source_id == SRC_CLIENT) ? session_data->method_id : METH__NOT_PRESENT), status_code_num((source_id == SRC_SERVER) ? session_data->status_code_num : STAT_NOT_PRESENT), delete_msg_on_destruct(buf_owner) -{ } +{ + assert((source_id == SRC_CLIENT) || (source_id == SRC_SERVER)); +} void HttpMsgSection::update_depth() const { diff --git a/src/service_inspectors/http_inspect/http_msg_status.cc b/src/service_inspectors/http_inspect/http_msg_status.cc index 1e056cc35..f5a36407e 100644 --- a/src/service_inspectors/http_inspect/http_msg_status.cc +++ b/src/service_inspectors/http_inspect/http_msg_status.cc @@ -17,7 +17,6 @@ //-------------------------------------------------------------------------- // http_msg_status.cc author Tom Peters -#include #include #include #include diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc index 1eea161a2..3d055702e 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc @@ -253,7 +253,7 @@ bool HttpStreamSplitter::finish(Flow* flow) (session_data->cutter[source_id] != nullptr) && (session_data->cutter[source_id]->get_octets_seen() == 0)) { - if (!session_data->mime_state) + if (!session_data->mime_state[source_id]) { FileFlows* file_flows = FileFlows::get_file_flows(flow); const bool download = (source_id == SRC_SERVER); @@ -261,10 +261,10 @@ bool HttpStreamSplitter::finish(Flow* flow) } else { - session_data->mime_state->process_mime_data(flow, nullptr, 0, true, + session_data->mime_state[source_id]->process_mime_data(flow, nullptr, 0, true, SNORT_FILE_POSITION_UNKNOWN); - delete session_data->mime_state; - session_data->mime_state = nullptr; + delete session_data->mime_state[source_id]; + session_data->mime_state[source_id] = nullptr; } return false; } diff --git a/src/service_inspectors/http_inspect/test/http_msg_head_shared_util_test.cc b/src/service_inspectors/http_inspect/test/http_msg_head_shared_util_test.cc index 4cbf61168..f4f64052f 100644 --- a/src/service_inspectors/http_inspect/test/http_msg_head_shared_util_test.cc +++ b/src/service_inspectors/http_inspect/test/http_msg_head_shared_util_test.cc @@ -32,7 +32,8 @@ long HttpTestManager::print_amount {}; bool HttpTestManager::print_hex {}; -TEST_GROUP(http_msg_head_shared_util) +// Tests for get_next_code() +TEST_GROUP(get_next_code) { enum Color { COLOR_OTHER=1, COLOR_GREEN, COLOR_BLUE, COLOR_RED, COLOR_YELLOW, COLOR_PURPLE }; int32_t offset = 0; @@ -58,7 +59,7 @@ TEST_GROUP(http_msg_head_shared_util) }; }; -TEST(http_msg_head_shared_util, basic) +TEST(get_next_code, basic) { Field input(10, (const uint8_t*) "green,blue"); Color color = (Color) HttpMsgHeadTest::get_next_code_test(input, offset, color_table); @@ -69,7 +70,7 @@ TEST(http_msg_head_shared_util, basic) CHECK(color == COLOR_BLUE); } -TEST(http_msg_head_shared_util, single_token) +TEST(get_next_code, single_token) { Field input(6, (const uint8_t*) "purple"); Color color = (Color) HttpMsgHeadTest::get_next_code_test(input, offset, color_table); @@ -77,7 +78,7 @@ TEST(http_msg_head_shared_util, single_token) CHECK(color == COLOR_PURPLE); } -TEST(http_msg_head_shared_util, unknown_token) +TEST(get_next_code, unknown_token) { Field input(14, (const uint8_t*) "madeup,red,red"); Color color = (Color) HttpMsgHeadTest::get_next_code_test(input, offset, color_table); @@ -85,7 +86,7 @@ TEST(http_msg_head_shared_util, unknown_token) CHECK(color == COLOR_OTHER); } -TEST(http_msg_head_shared_util, null_token) +TEST(get_next_code, null_token) { Field input(11, (const uint8_t*) "green,,blue"); Color color = (Color) HttpMsgHeadTest::get_next_code_test(input, offset, color_table); @@ -99,6 +100,110 @@ TEST(http_msg_head_shared_util, null_token) CHECK(color == COLOR_BLUE); } +// Tests for boundary_present() +TEST_GROUP(boundary_present) +{ + // This allows access to test a protected static member function + class HttpMsgHeadTest : public HttpMsgHeadShared + { + public: + static bool boundary_present_test(const Field& field) + { + return HttpMsgHeadShared::boundary_present(field); + } + }; +}; + +TEST(boundary_present, present) +{ + Field input(20, (const uint8_t*) "xxxxxboundary=cccccc"); + CHECK(HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, not_present) +{ + Field input(20, (const uint8_t*) "xxxxx123456789cccccc"); + CHECK(!HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, equal_only) +{ + Field input(20, (const uint8_t*) "xxxxx12345=789cccccc"); + CHECK(!HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, several_missing) +{ + Field input(16, (const uint8_t*) "123456b789ccry=c"); + CHECK(!HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, b_missing) +{ + Field input(50, (const uint8_t*) "12345678901234567890oundary=9012345678901234567890"); + CHECK(!HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, equal_missing) +{ + Field input(50, (const uint8_t*) "12345678901234567890boundary9012345678901234567890"); + CHECK(!HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, d_missing) +{ + Field input(50, (const uint8_t*) "12345678901234567890bounxary=012345678901234567890"); + CHECK(!HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, front) +{ + Field input(50, (const uint8_t*) "boundary=01234567890123456789012345678901234567890"); + CHECK(HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, end) +{ + Field input(50, (const uint8_t*) "01234567890123456789012345678901234567890boundary="); + CHECK(HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, front_edge_case) +{ + Field input(49, (const uint8_t*) "oundary=01234567890123456789012345678901234567890"); + CHECK(!HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, whole_buffer) +{ + Field input(9, (const uint8_t*) "boundary="); + CHECK(HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, all_upper) +{ + Field input(50, (const uint8_t*) "12345678901234567890BOUNDARY=012345678901234567890"); + CHECK(HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, mixed_case) +{ + Field input(50, (const uint8_t*) "12345678901234567890BoUnDaRy=012345678901234567890"); + CHECK(HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, false_starts) +{ + Field input(42, (const uint8_t*) "ARy=56789foundary=\0==ry=1234boundary=kkkkk"); + CHECK(HttpMsgHeadTest::boundary_present_test(input)); +} + +TEST(boundary_present, just_one_char) +{ + Field input(1, (const uint8_t*) "="); + CHECK(!HttpMsgHeadTest::boundary_present_test(input)); +} + int main(int argc, char** argv) { return CommandLineTestRunner::RunAllTests(argc, argv);