From: Tom Peters (thopeter) Date: Fri, 7 Sep 2018 15:19:04 +0000 (-0400) Subject: Merge pull request #1349 in SNORT/snort3 from nhttp113 to master X-Git-Tag: 3.0.0-248~12 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=977bb94599bc1a4b4959b301497185159954154e;p=thirdparty%2Fsnort3.git Merge pull request #1349 in SNORT/snort3 from nhttp113 to master Squashed commit of the following: commit ff828f6ea9547c4377e8b1162c920839d4b78acb Author: Tom Peters Date: Fri Jul 27 11:25:05 2018 -0400 http_inspect: split and inspect immediately upon reaching depth --- diff --git a/src/service_inspectors/http_inspect/http_cutter.cc b/src/service_inspectors/http_inspect/http_cutter.cc index d6ff25b86..454bc3026 100644 --- a/src/service_inspectors/http_inspect/http_cutter.cc +++ b/src/service_inspectors/http_inspect/http_cutter.cc @@ -26,7 +26,7 @@ using namespace HttpEnums; ScanResult HttpStartCutter::cut(const uint8_t* buffer, uint32_t length, - HttpInfractions* infractions, HttpEventGen* events, uint32_t, uint32_t) + HttpInfractions* infractions, HttpEventGen* events, uint32_t, bool) { for (uint32_t k = 0; k < length; k++) { @@ -154,7 +154,7 @@ HttpStartCutter::ValidationResult HttpStatusCutter::validate(uint8_t octet, } ScanResult HttpHeaderCutter::cut(const uint8_t* buffer, uint32_t length, - HttpInfractions* infractions, HttpEventGen* events, uint32_t, uint32_t) + HttpInfractions* infractions, HttpEventGen* events, uint32_t, bool) { // Header separators: leading \r\n, leading \n, nonleading \r\n\r\n, nonleading \n\r\n, // nonleading \r\n\n, and nonleading \n\n. The separator itself becomes num_excess which is @@ -251,9 +251,9 @@ ScanResult HttpHeaderCutter::cut(const uint8_t* buffer, uint32_t length, } ScanResult HttpBodyClCutter::cut(const uint8_t*, uint32_t length, HttpInfractions*, - HttpEventGen*, uint32_t flow_target, uint32_t flow_max) + HttpEventGen*, uint32_t flow_target, bool stretch) { - assert(remaining > 0); + assert(remaining > octets_seen); // Are we skipping to the next message? if (flow_target == 0) @@ -272,24 +272,62 @@ ScanResult HttpBodyClCutter::cut(const uint8_t*, uint32_t length, HttpInfraction } } - // The normal body section size is flow_target. But if there are only flow_max or less - // remaining we take the whole thing rather than leave a small final section. - if (remaining <= flow_max) + // A target that is bigger than the entire rest of the message body makes no sense + if (remaining <= flow_target) { - num_flush = remaining; - remaining = 0; - return SCAN_FOUND; + flow_target = remaining; + stretch = false; } - else + + if (!stretch) { num_flush = flow_target; - remaining -= num_flush; + if (num_flush < remaining) + { + remaining -= num_flush; + return SCAN_FOUND_PIECE; + } + else + { + remaining = 0; + return SCAN_FOUND; + } + } + + if (octets_seen + length < flow_target) + { + octets_seen += length; + return SCAN_NOT_FOUND; + } + + if (octets_seen + length < remaining) + { + // The message body continues beyond this segment + // Stretch the section to include this entire segment provided it is not too big + if (octets_seen + length <= flow_target + MAX_SECTION_STRETCH) + num_flush = length; + else + num_flush = flow_target - octets_seen; + remaining -= octets_seen + num_flush; return SCAN_FOUND_PIECE; } + + if (remaining - flow_target <= MAX_SECTION_STRETCH) + { + // Stretch the section to finish the message body + num_flush = remaining - octets_seen; + remaining = 0; + return SCAN_FOUND; + } + + // Cannot stretch to the end of the message body. Cut at the original target. + num_flush = flow_target - octets_seen; + remaining -= flow_target; + return SCAN_FOUND_PIECE; } ScanResult HttpBodyOldCutter::cut(const uint8_t*, uint32_t length, HttpInfractions*, HttpEventGen*, - uint32_t flow_target, uint32_t) + uint32_t flow_target, bool stretch) { if (flow_target == 0) { @@ -302,16 +340,35 @@ ScanResult HttpBodyOldCutter::cut(const uint8_t*, uint32_t length, HttpInfractio return SCAN_DISCARD_PIECE; } - num_flush = flow_target; - return SCAN_FOUND_PIECE; + if (octets_seen + length < flow_target) + { + // Not enough data yet to create a message section + octets_seen += length; + return SCAN_NOT_FOUND; + } + else if (stretch && (octets_seen + length <= flow_target + MAX_SECTION_STRETCH)) + { + // Cut the section at the end of this TCP segment to avoid splitting a packet + num_flush = length; + return SCAN_FOUND_PIECE; + } + else + { + // Cut the section at the target length. Either stretching is not allowed or the end of + // the segment is too far away. + num_flush = flow_target - octets_seen; + return SCAN_FOUND_PIECE; + } } ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length, - HttpInfractions* infractions, HttpEventGen* events, uint32_t flow_target, uint32_t) + HttpInfractions* infractions, HttpEventGen* events, uint32_t flow_target, bool stretch) { // Are we skipping through the rest of this chunked body to the trailers and the next message? const bool discard_mode = (flow_target == 0); + const uint32_t adjusted_target = stretch ? MAX_SECTION_STRETCH + flow_target : flow_target; + for (int32_t k=0; k < static_cast(length); k++) { switch (curr_state) @@ -485,16 +542,16 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length, // Moving through the chunk data { uint32_t skip_amount = (length-k <= expected) ? length-k : expected; - if (!discard_mode && (skip_amount > flow_target-data_seen)) - { // Do not exceed requested section size - skip_amount = flow_target-data_seen; + if (!discard_mode && (skip_amount > adjusted_target-data_seen)) + { // Do not exceed requested section size (including stretching) + skip_amount = adjusted_target-data_seen; } k += skip_amount - 1; if ((expected -= skip_amount) == 0) { curr_state = CHUNK_DCRLF1; } - if ((data_seen += skip_amount) == flow_target) + if ((data_seen += skip_amount) == adjusted_target) { data_seen = 0; num_flush = k+1; @@ -556,13 +613,13 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length, // that there were chunk header bytes between the last good chunk and the point where // the failure occurred. These will not have been counted in data_seen because we // planned to delete them during reassembly. Because they are not part of a valid chunk - // they will be reassembled after all. This will overrun the flow_target making the + // they will be reassembled after all. This will overrun the adjusted_target making the // message section a little bigger than planned. It's not important. uint32_t skip_amount = length-k; - skip_amount = (skip_amount <= flow_target-data_seen) ? skip_amount : - flow_target-data_seen; + skip_amount = (skip_amount <= adjusted_target-data_seen) ? skip_amount : + adjusted_target-data_seen; k += skip_amount - 1; - if ((data_seen += skip_amount) == flow_target) + if ((data_seen += skip_amount) == adjusted_target) { data_seen = 0; num_flush = k+1; @@ -577,6 +634,14 @@ ScanResult HttpBodyChunkCutter::cut(const uint8_t* buffer, uint32_t length, return SCAN_DISCARD_PIECE; } + if (data_seen >= flow_target) + { + // We passed the flow_target and stretched to the end of the segment + data_seen = 0; + num_flush = length; + return SCAN_FOUND_PIECE; + } + octets_seen += length; return SCAN_NOT_FOUND; } diff --git a/src/service_inspectors/http_inspect/http_cutter.h b/src/service_inspectors/http_inspect/http_cutter.h index 593a86a27..114630d16 100644 --- a/src/service_inspectors/http_inspect/http_cutter.h +++ b/src/service_inspectors/http_inspect/http_cutter.h @@ -35,8 +35,8 @@ class HttpCutter public: virtual ~HttpCutter() = default; virtual HttpEnums::ScanResult cut(const uint8_t* buffer, uint32_t length, - HttpInfractions* infractions, HttpEventGen* events, uint32_t flow_target, - uint32_t flow_max) = 0; + HttpInfractions* infractions, HttpEventGen* events, uint32_t flow_target, bool stretch) + = 0; uint32_t get_num_flush() const { return num_flush; } uint32_t get_octets_seen() const { return octets_seen; } uint32_t get_num_excess() const { return num_crlf; } @@ -56,7 +56,7 @@ class HttpStartCutter : public HttpCutter { public: HttpEnums::ScanResult cut(const uint8_t* buffer, uint32_t length, - HttpInfractions* infractions, HttpEventGen* events, uint32_t, uint32_t) override; + HttpInfractions* infractions, HttpEventGen* events, uint32_t, bool) override; protected: enum ValidationResult { V_GOOD, V_BAD, V_TBD }; @@ -85,7 +85,7 @@ class HttpHeaderCutter : public HttpCutter { public: HttpEnums::ScanResult cut(const uint8_t* buffer, uint32_t length, - HttpInfractions* infractions, HttpEventGen* events, uint32_t, uint32_t) override; + HttpInfractions* infractions, HttpEventGen* events, uint32_t, bool) override; uint32_t get_num_head_lines() const override { return num_head_lines; } private: @@ -100,7 +100,8 @@ public: explicit HttpBodyClCutter(int64_t expected_length) : remaining(expected_length) { assert(remaining > 0); } HttpEnums::ScanResult cut(const uint8_t*, uint32_t length, HttpInfractions*, HttpEventGen*, - uint32_t flow_target, uint32_t flow_max) override; + uint32_t flow_target, bool stretch) override; + void soft_reset() override { octets_seen = 0; } private: int64_t remaining; @@ -110,14 +111,15 @@ class HttpBodyOldCutter : public HttpCutter { public: HttpEnums::ScanResult cut(const uint8_t*, uint32_t, HttpInfractions*, HttpEventGen*, - uint32_t flow_target, uint32_t) override; + uint32_t flow_target, bool stretch) override; + void soft_reset() override { octets_seen = 0; } }; class HttpBodyChunkCutter : public HttpCutter { public: HttpEnums::ScanResult cut(const uint8_t* buffer, uint32_t length, - HttpInfractions* infractions, HttpEventGen* events, uint32_t flow_target, uint32_t) + HttpInfractions* infractions, HttpEventGen* events, uint32_t flow_target, bool stretch) override; bool get_is_broken_chunk() const override { return curr_state == HttpEnums::CHUNK_BAD; } uint32_t get_num_good_chunks() const override { return num_good_chunks; } diff --git a/src/service_inspectors/http_inspect/http_enum.h b/src/service_inspectors/http_inspect/http_enum.h index a2a96bc36..e721a3f70 100644 --- a/src/service_inspectors/http_inspect/http_enum.h +++ b/src/service_inspectors/http_inspect/http_enum.h @@ -26,8 +26,8 @@ namespace HttpEnums { static const int MAX_OCTETS = 63780; static const int GZIP_BLOCK_SIZE = 2048; -static const int FINAL_GZIP_BLOCK_SIZE = 2304; // compromise value, too big causes gzip overruns - // too small leaves too many little end sections +static const int MAX_SECTION_STRETCH = 1460; + static const uint32_t HTTP_GID = 119; static const int GZIP_WINDOW_BITS = 31; static const int DEFLATE_WINDOW_BITS = 15; @@ -284,7 +284,7 @@ enum EventSid EVENT_UNESCAPED_SPACE_URI, EVENT_PIPELINE_MAX, - EVENT_ANOM_SERVER = 101, + EVENT_OBSOLETE_ANOM_SERVER = 101, // Previously used, do not reuse this number EVENT_INVALID_STATCODE, EVENT_UNUSED_1, EVENT_UTF_NORM_FAIL, diff --git a/src/service_inspectors/http_inspect/http_flow_data.cc b/src/service_inspectors/http_inspect/http_flow_data.cc index 00e216040..610ea5e18 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.cc +++ b/src/service_inspectors/http_inspect/http_flow_data.cc @@ -101,7 +101,7 @@ void HttpFlowData::half_reset(SourceId source_id) data_length[source_id] = STAT_NOT_PRESENT; body_octets[source_id] = STAT_NOT_PRESENT; section_size_target[source_id] = 0; - section_size_max[source_id] = 0; + stretch_section_to_packet[source_id] = false; file_depth_remaining[source_id] = STAT_NOT_PRESENT; detect_depth_remaining[source_id] = STAT_NOT_PRESENT; detection_status[source_id] = DET_REACTIVATING; diff --git a/src/service_inspectors/http_inspect/http_flow_data.h b/src/service_inspectors/http_inspect/http_flow_data.h index 6e0124231..11011f229 100644 --- a/src/service_inspectors/http_inspect/http_flow_data.h +++ b/src/service_inspectors/http_inspect/http_flow_data.h @@ -114,9 +114,9 @@ private: uint64_t zero_nine_expected = 0; int64_t data_length[2] = { HttpEnums::STAT_NOT_PRESENT, HttpEnums::STAT_NOT_PRESENT }; uint32_t section_size_target[2] = { 0, 0 }; - uint32_t section_size_max[2] = { 0, 0 }; HttpEnums::CompressId compression[2] = { HttpEnums::CMP_NONE, HttpEnums::CMP_NONE }; HttpEnums::DetectionStatus detection_status[2] = { HttpEnums::DET_ON, HttpEnums::DET_ON }; + bool stretch_section_to_packet[2] = { false, false }; // *** Inspector's internal data about the current message struct FdCallbackContext 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 80669069d..7be55ff77 100644 --- a/src/service_inspectors/http_inspect/http_msg_body_chunk.cc +++ b/src/service_inspectors/http_inspect/http_msg_body_chunk.cc @@ -27,10 +27,11 @@ using namespace HttpEnums; void HttpMsgBodyChunk::update_flow() { - // Cutter was deleted by splitter when zero-length chunk received + session_data->body_octets[source_id] = body_octets; + + // Cutter was deleted by splitter when zero-length chunk received or at TCP close if (session_data->cutter[source_id] == nullptr) { - session_data->body_octets[source_id] = body_octets; session_data->trailer_prep(source_id); if (session_data->mime_state[source_id] != nullptr) { @@ -46,7 +47,6 @@ void HttpMsgBodyChunk::update_flow() } else { - session_data->body_octets[source_id] = body_octets; update_depth(); } session_data->section_type[source_id] = SEC__NOT_COMPUTE; diff --git a/src/service_inspectors/http_inspect/http_msg_section.cc b/src/service_inspectors/http_inspect/http_msg_section.cc index 2dcc4a145..e6ee69606 100644 --- a/src/service_inspectors/http_inspect/http_msg_section.cc +++ b/src/service_inspectors/http_inspect/http_msg_section.cc @@ -70,40 +70,48 @@ void HttpMsgSection::create_event(int sid) void HttpMsgSection::update_depth() const { - if ((session_data->detect_depth_remaining[source_id] <= 0) && + int64_t& file_depth_remaining = session_data->file_depth_remaining[source_id]; + int64_t& detect_depth_remaining = session_data->detect_depth_remaining[source_id]; + + if ((detect_depth_remaining <= 0) && (session_data->detection_status[source_id] == DET_ON)) { session_data->detection_status[source_id] = DET_DEACTIVATING; } - if ((session_data->file_depth_remaining[source_id] <= 0) && - (session_data->detect_depth_remaining[source_id] <= 0)) + if (detect_depth_remaining <= 0) { - // Don't need any more of the body - session_data->section_size_target[source_id] = 0; - session_data->section_size_max[source_id] = 0; + if (file_depth_remaining <= 0) + { + // Don't need any more of the body + session_data->section_size_target[source_id] = 0; + } + else + { + // Just for file processing + session_data->section_size_target[source_id] = snort::SnortConfig::get_conf()->max_pdu; + session_data->stretch_section_to_packet[source_id] = true; + } return; } - const int random_increment = FlushBucket::get_size() - 192; - assert((random_increment >= -64) && (random_increment <= 63)); + const unsigned target_size = (session_data->compression[source_id] == CMP_NONE) ? + snort::SnortConfig::get_conf()->max_pdu : GZIP_BLOCK_SIZE; - switch (session_data->compression[source_id]) + if (detect_depth_remaining <= target_size) { - case CMP_NONE: - { - unsigned max_pdu = snort::SnortConfig::get_conf()->max_pdu; - session_data->section_size_target[source_id] = max_pdu + random_increment; - session_data->section_size_max[source_id] = max_pdu + (max_pdu >> 1); - break; - } - case CMP_GZIP: - case CMP_DEFLATE: - session_data->section_size_target[source_id] = GZIP_BLOCK_SIZE + random_increment; - session_data->section_size_max[source_id] = FINAL_GZIP_BLOCK_SIZE; - break; - default: - assert(false); + // Go to detection as soon as detect depth is reached + session_data->section_size_target[source_id] = detect_depth_remaining; + session_data->stretch_section_to_packet[source_id] = true; + } + else + { + // Randomize the split point a little bit to make it harder to evade detection. + // FlushBucket provides pseudo random numbers in the range 128 to 255. + const int random_increment = FlushBucket::get_size() - 192; + assert((random_increment >= -64) && (random_increment <= 63)); + session_data->section_size_target[source_id] = target_size + random_increment; + session_data->stretch_section_to_packet[source_id] = false; } } diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc b/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc index 72b872538..4c8f82abf 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc @@ -90,6 +90,9 @@ bool HttpStreamSplitter::finish(snort::Flow* flow) session_data->cutter[source_id]->get_num_good_chunks(), session_data->cutter[source_id]->get_octets_seen(), true); + delete session_data->cutter[source_id]; + session_data->cutter[source_id] = nullptr; + return true; } 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 3f1af00f0..97fc984e9 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc @@ -167,7 +167,8 @@ StreamSplitter::Status HttpStreamSplitter::scan(Flow* flow, const uint8_t* data, const uint32_t max_length = MAX_OCTETS - cutter->get_octets_seen(); const ScanResult cut_result = cutter->cut(data, (length <= max_length) ? length : max_length, session_data->get_infractions(source_id), session_data->get_events(source_id), - session_data->section_size_target[source_id], session_data->section_size_max[source_id]); + session_data->section_size_target[source_id], + session_data->stretch_section_to_packet[source_id]); switch (cut_result) { case SCAN_NOT_FOUND: diff --git a/src/service_inspectors/http_inspect/http_tables.cc b/src/service_inspectors/http_inspect/http_tables.cc index 3fcf95fb1..cf2cd3505 100644 --- a/src/service_inspectors/http_inspect/http_tables.cc +++ b/src/service_inspectors/http_inspect/http_tables.cc @@ -322,7 +322,7 @@ const snort::RuleMap HttpModule::http_events[] = { EVENT_SIMPLE_REQUEST, "simple request" }, { EVENT_UNESCAPED_SPACE_URI, "unescaped space in HTTP URI" }, { EVENT_PIPELINE_MAX, "too many pipelined requests" }, - { EVENT_ANOM_SERVER, "anomalous http server on undefined HTTP port" }, + { EVENT_OBSOLETE_ANOM_SERVER, "obsolete event--deleted" }, { EVENT_INVALID_STATCODE, "invalid status code in HTTP response" }, { EVENT_UNUSED_1, "unused event number--should not appear" }, { EVENT_UTF_NORM_FAIL, "HTTP response has UTF charset that failed to normalize" },