From: Tom Peters Date: Fri, 10 Oct 2014 14:30:17 +0000 (-0400) Subject: zero-chunk workaround X-Git-Tag: 3.0.0-233~1390 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=37ea0579aa8aad74ffb341311b69c9c0ea312e5b;p=thirdparty%2Fsnort3.git zero-chunk workaround --- diff --git a/src/service_inspectors/nhttp_inspect/nhttp_splitter.cc b/src/service_inspectors/nhttp_inspect/nhttp_splitter.cc index b4320e081..5947e2d6b 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_splitter.cc +++ b/src/service_inspectors/nhttp_inspect/nhttp_splitter.cc @@ -83,6 +83,60 @@ ScanResult NHttpStartSplitter::split(const uint8_t* buffer, uint32_t length) { return SCAN_NOTFOUND; } +ScanResult NHttpHeaderSplitter::split(const uint8_t* buffer, uint32_t length) { + conditional_reset(); + if (peek_status == SCAN_FOUND) { + return SCAN_FOUND; + } + buffer += peek_octets; + length -= peek_octets; + for (uint32_t k = 0; k < length; k++) { + if (buffer[k] == '\n') { + num_crlf++; + if ((first_lf == 0) && (num_crlf < octets_seen + k + 1)) { + first_lf = num_crlf; + } + else { + num_flush = k + 1 + peek_octets; + complete = true; + return SCAN_FOUND; + } + } + else if (buffer[k] == '\r') { + if (num_crlf == first_lf) { + num_crlf++; + } + else { + num_crlf = 1; + first_lf = 0; + } + } + else { + num_crlf = 0; + first_lf = 0; + } + } + peek_octets = 0; + octets_seen += length; + return SCAN_NOTFOUND; +} + +ScanResult NHttpHeaderSplitter::peek(const uint8_t* buffer, uint32_t length) { + assert(octets_seen == 0); + peek_status = split(buffer, length); + peek_octets = length; + return peek_status; +} + +void NHttpHeaderSplitter::conditional_reset() { + if (complete) { + peek_octets = 0; + peek_status = SCAN_NOTFOUND; + first_lf = 0; + } + NHttpSplitter::conditional_reset(); +} + ScanResult NHttpChunkSplitter::split(const uint8_t* buffer, uint32_t length) { conditional_reset(); if (header_complete) { @@ -101,12 +155,23 @@ ScanResult NHttpChunkSplitter::split(const uint8_t* buffer, uint32_t length) { } if (!length_started) { // chunk header specifies no length + // FIXIT-M need to find a way to flush partial chunk buffer complete = true; return SCAN_ABORT; } + if (expected_length == 0) { + // Workaround because stream cannot handle zero-length flush. Instead of flushing the zero-length chunk + // to flush the partial chunk buffer in reassembly, we save the terminal \n from the chunk header for + // use as an end-of-chunks signal. FIXIT-M + expected_length = 1; + zero_chunk = true; + num_flush = k; + } + else { + num_flush = k+1; + } // flush completed chunk header header_complete = true; - num_flush = k+1; return SCAN_DISCARD; } if (num_crlf == 1) { @@ -153,60 +218,7 @@ void NHttpChunkSplitter::conditional_reset() { digits_seen = 0; semicolon = false; header_complete = false; - } - NHttpSplitter::conditional_reset(); -} - -ScanResult NHttpHeaderSplitter::split(const uint8_t* buffer, uint32_t length) { - conditional_reset(); - if (peek_status == SCAN_FOUND) { - return SCAN_FOUND; - } - buffer += peek_octets; - length -= peek_octets; - for (uint32_t k = 0; k < length; k++) { - if (buffer[k] == '\n') { - num_crlf++; - if ((first_lf == 0) && (num_crlf < octets_seen + k + 1)) { - first_lf = num_crlf; - } - else { - num_flush = k + 1 + peek_octets; - complete = true; - return SCAN_FOUND; - } - } - else if (buffer[k] == '\r') { - if (num_crlf == first_lf) { - num_crlf++; - } - else { - num_crlf = 1; - first_lf = 0; - } - } - else { - num_crlf = 0; - first_lf = 0; - } - } - peek_octets = 0; - octets_seen += length; - return SCAN_NOTFOUND; -} - -ScanResult NHttpHeaderSplitter::peek(const uint8_t* buffer, uint32_t length) { - assert(octets_seen == 0); - peek_status = split(buffer, length); - peek_octets = length; - return peek_status; -} - -void NHttpHeaderSplitter::conditional_reset() { - if (complete) { - peek_octets = 0; - peek_status = SCAN_NOTFOUND; - first_lf = 0; + zero_chunk = false; } NHttpSplitter::conditional_reset(); } diff --git a/src/service_inspectors/nhttp_inspect/nhttp_splitter.h b/src/service_inspectors/nhttp_inspect/nhttp_splitter.h index 6421d3a37..af29874cf 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_splitter.h +++ b/src/service_inspectors/nhttp_inspect/nhttp_splitter.h @@ -44,6 +44,7 @@ public: uint32_t get_num_flush() { return num_flush; }; virtual uint32_t get_octets_seen() { return octets_seen; }; virtual uint32_t get_num_excess() { return 0; }; + virtual bool partial_ok() { return true; }; protected: uint32_t octets_seen = 0; @@ -78,14 +79,16 @@ private: class NHttpChunkSplitter : public NHttpSplitter { public: NHttpEnums::ScanResult split(const uint8_t* buffer, uint32_t length); - uint32_t get_num_excess() { return octets_seen; }; + uint32_t get_num_excess() { return zero_chunk ? 1 : 0; }; void conditional_reset(); + bool partial_ok() { return false; }; private: uint32_t expected_length = 0; bool length_started = false; uint32_t digits_seen = 0; bool semicolon = false; bool header_complete = false; + bool zero_chunk = false; }; class NHttpTrailerSplitter : public NHttpSplitter { diff --git a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc index d5dbbdb00..8bdcf4db3 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc +++ b/src/service_inspectors/nhttp_inspect/nhttp_stream_splitter.cc @@ -54,7 +54,8 @@ void NHttpStreamSplitter::prepare_flush(NHttpFlowData* session_data, uint32_t* f break; case SEC_CHUNK: paf_max = 16384 - session_data->chunk_buffer_length[source_id]; - if (num_octets == 0) { + if (num_excess == 1) { + // zero-length chunk session_data->type_expected[source_id] = SEC_TRAILER; } break; @@ -142,9 +143,15 @@ StreamSplitter::Status NHttpStreamSplitter::scan (Flow* flow, const uint8_t* dat return StreamSplitter::ABORT; } if (tcp_close) { - prepare_flush(session_data, flush_offset, source_id, type, true, INF_TRUNCATED, length, length, - splitter->get_num_excess()); - return StreamSplitter::FLUSH; + if (splitter->partial_ok()) { + prepare_flush(session_data, flush_offset, source_id, type, true, INF_TRUNCATED, length, length, + splitter->get_num_excess()); + return StreamSplitter::FLUSH; + } + else { + prepare_flush(session_data, flush_offset, source_id, SEC_DISCARD, true, 0, length, length, 0); + return StreamSplitter::FLUSH; + } } // Incomplete headers wait patiently for more data return NHttpTestManager::use_test_input() ? StreamSplitter::FLUSH : StreamSplitter::SEARCH; @@ -192,6 +199,8 @@ StreamSplitter::Status NHttpStreamSplitter::scan (Flow* flow, const uint8_t* dat const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned /* total */, unsigned offset, const uint8_t* data, unsigned len, uint32_t flags, unsigned& copied) { + // FIXIT-H there should be a tcp_close parameter from stream. Pending that we should create our own, integrate it + // with the test tool, and use it for chunk buffer flushing and to update the session data. static THREAD_LOCAL StreamBuffer nhttp_buf; NHttpFlowData* session_data = (NHttpFlowData*)flow->get_application_data(NHttpFlowData::nhttp_flow_id); @@ -213,8 +222,8 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned /* tota offset = 0; } - // FIXIT-P stream should be enhanced to do discarding for us - // For now flush-then-discard here is how scan() handles things we don't need to examine. + // FIXIT-P stream should be enhanced to do discarding for us. For now flush-then-discard here is how scan() handles + // things we don't need to examine. if ((session_data->section_type[source_id] == SEC_DISCARD) || (session_data->section_type[source_id] == SEC_ABORT)) { if (NHttpTestManager::use_test_output()) { @@ -248,9 +257,10 @@ const StreamBuffer* NHttpStreamSplitter::reassemble(Flow* flow, unsigned /* tota } else { // small chunks are aggregated before processing and are kept here until the buffer is full (paf_max) - // all the chunks in the buffer go to the inspector together. Zero-length chunk flushes accumulated chunks. - int32_t total_chunk_len = chunk_buffer_length + offset + len; - if ((total_chunk_len < 16384) && (len != 0)) { + // all the chunks in the buffer go to the inspector together. Zero-length chunk (len == 1, num_excess == 1) + // flushes accumulated chunks. + int32_t total_chunk_len = chunk_buffer_length + offset + len - num_excess; + if ((total_chunk_len < 16384) && (num_excess == 0)) { chunk_buffer_length = total_chunk_len; return nullptr; } diff --git a/src/service_inspectors/nhttp_inspect/nhttp_uri_norm.cc b/src/service_inspectors/nhttp_inspect/nhttp_uri_norm.cc index c82f07ec7..ddf9b1ac5 100644 --- a/src/service_inspectors/nhttp_inspect/nhttp_uri_norm.cc +++ b/src/service_inspectors/nhttp_inspect/nhttp_uri_norm.cc @@ -85,6 +85,7 @@ bool UriNormalizer::no_path_check(const uint8_t* in_buf, int32_t in_length, uint bool UriNormalizer::path_check(const uint8_t* in_buf, int32_t in_length, uint64_t& infractions) { for (int32_t k = 0; k < in_length; k++) { + // FIXIT-P Periods are common and most don't need to be normalized. Need a better test. if (uri_char[in_buf[k]] == CHAR_NORMAL) continue; if ((in_buf[k] == '/') && ((k == 0) || (in_buf[k-1] != '/'))) continue; infractions |= INF_URINEEDNORM;