From: Mike Stepanek (mstepane) Date: Wed, 23 Oct 2019 12:40:46 +0000 (-0400) Subject: Merge pull request #1811 in SNORT/snort3 from ~THOPETER/snort3:nhttp128 to master X-Git-Tag: 3.0.0-263~10 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a03fd35e417af0a8fc85113ad065a78f930f12f7;p=thirdparty%2Fsnort3.git Merge pull request #1811 in SNORT/snort3 from ~THOPETER/snort3:nhttp128 to master Squashed commit of the following: commit 2020c443a5f649cfca30e0957378edb5bfa62ad7 Author: Tom Peters Date: Fri Oct 18 11:32:15 2019 -0400 http_inspect: test tool single-direction abort fix --- diff --git a/src/service_inspectors/http_inspect/http_msg_request.cc b/src/service_inspectors/http_inspect/http_msg_request.cc index 260f50416..a43d35614 100644 --- a/src/service_inspectors/http_inspect/http_msg_request.cc +++ b/src/service_inspectors/http_inspect/http_msg_request.cc @@ -128,6 +128,8 @@ bool HttpMsgRequest::http_name_nocase_ok(const uint8_t* start) bool HttpMsgRequest::handle_zero_nine() { + // FIXIT-M The following test seems too permissive about what constitutes HTTP/0.9. Consider + // not accepting "URIs" with internal whitespace or nonprinting characters. // 0.9 request line is supposed to be "GET \r\n" if ((start_line.length() >= 3) && !memcmp(start_line.start(), "GET", 3) && 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 9901b70d0..5d286f725 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc @@ -99,11 +99,8 @@ StreamSplitter::Status HttpStreamSplitter::status_value(StreamSplitter::Status r } if (HttpTestManager::use_test_input(type)) { - if (ret_val == StreamSplitter::ABORT) - return StreamSplitter::ABORT; return StreamSplitter::FLUSH; } - else #endif return ret_val; } @@ -144,9 +141,6 @@ StreamSplitter::Status HttpStreamSplitter::scan(Packet* pkt, const uint8_t* data SectionType type = session_data->type_expected[source_id]; - if (type == SEC_ABORT) - return status_value(StreamSplitter::ABORT); - #ifdef REG_TEST if (HttpTestManager::use_test_input(HttpTestManager::IN_HTTP)) { @@ -160,7 +154,14 @@ StreamSplitter::Status HttpStreamSplitter::scan(Packet* pkt, const uint8_t* data return StreamSplitter::FLUSH; data = test_data; } - else if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP)) +#endif + + if (type == SEC_ABORT) + return status_value(StreamSplitter::ABORT); + +#ifdef REG_TEST + if (HttpTestManager::use_test_output(HttpTestManager::IN_HTTP) && + !HttpTestManager::use_test_input(HttpTestManager::IN_HTTP)) { printf("Scan from flow data %" PRIu64 " direction %d length %u client port %hu server port %hu\n", session_data->seq_num, diff --git a/src/service_inspectors/http_inspect/http_test_input.cc b/src/service_inspectors/http_inspect/http_test_input.cc index be2ec946e..0f74cd312 100644 --- a/src/service_inspectors/http_inspect/http_test_input.cc +++ b/src/service_inspectors/http_inspect/http_test_input.cc @@ -86,14 +86,10 @@ void HttpTestInput::reset() // function is to read dev_notes.txt. void HttpTestInput::scan(uint8_t*& data, uint32_t& length, SourceId source_id, uint64_t seq_num) { - bool skip_to_break = false; if (seq_num != curr_seq_num) { assert(source_id == SRC_CLIENT); curr_seq_num = seq_num; - // If we have not yet found the break command we need to skim past everything and not - // return any data until we find it. - skip_to_break = !need_break; reset(); } @@ -189,28 +185,21 @@ void HttpTestInput::scan(uint8_t*& data, uint32_t& length, SourceId source_id, u strlen("request"))) { last_source_id = SRC_CLIENT; - if (!skip_to_break) - { - length = 0; - return; - } + length = 0; + return; } else if ((command_length == strlen("response")) && !memcmp(command_value, "response", strlen("response"))) { last_source_id = SRC_SERVER; - if (!skip_to_break) - { - length = 0; - return; - } + length = 0; + return; } else if ((command_length == strlen("break")) && !memcmp(command_value, "break", strlen("break"))) { reset(); - if (!skip_to_break) - need_break = true; + need_break = true; length = 0; return; } @@ -218,11 +207,8 @@ void HttpTestInput::scan(uint8_t*& data, uint32_t& length, SourceId source_id, u "tcpclose", strlen("tcpclose"))) { tcp_closed = true; - if (!skip_to_break) - { - length = 0; - return; - } + length = 0; + return; } else if ((command_length > strlen("fill")) && !memcmp(command_value, "fill", strlen("fill"))) @@ -235,13 +221,8 @@ void HttpTestInput::scan(uint8_t*& data, uint32_t& length, SourceId source_id, u // auto-fill ABCDEFGHIJABCD ... msg_buf[last_source_id][end_offset[last_source_id]++] = 'A' + k%10; } - if (skip_to_break) - end_offset[last_source_id] = 0; - else - { - length = end_offset[last_source_id] - previous_offset[last_source_id]; - return; - } + length = end_offset[last_source_id] - previous_offset[last_source_id]; + return; } else if ((command_length == strlen("partial")) && !memcmp(command_value, "partial", strlen("partial"))) @@ -282,13 +263,8 @@ void HttpTestInput::scan(uint8_t*& data, uint32_t& length, SourceId source_id, u assert(new_octet != EOF); msg_buf[last_source_id][end_offset[last_source_id]++] = new_octet; } - if (skip_to_break) - end_offset[last_source_id] = 0; - else - { - length = end_offset[last_source_id] - previous_offset[last_source_id]; - return; - } + length = end_offset[last_source_id] - previous_offset[last_source_id]; + return; } else if ((command_length > strlen("fileskip")) && !memcmp(command_value, "fileskip", strlen("fileskip"))) @@ -360,12 +336,6 @@ void HttpTestInput::scan(uint8_t*& data, uint32_t& length, SourceId source_id, u ending = true; } // Found the second consecutive blank line that ends the paragraph. - else if (skip_to_break) - { - end_offset[last_source_id] = 0; - ending = false; - state = WAITING; - } else { length = end_offset[last_source_id] - previous_offset[last_source_id]; @@ -437,8 +407,6 @@ void HttpTestInput::scan(uint8_t*& data, uint32_t& length, SourceId source_id, u assert(end_offset[last_source_id] < sizeof(msg_buf[last_source_id])); } // End-of-file. Return everything we have so far. - if (skip_to_break) - end_offset[last_source_id] = 0; length = end_offset[last_source_id] - previous_offset[last_source_id]; }