From: Tom Peters (thopeter) Date: Mon, 25 Sep 2017 17:54:32 +0000 (-0400) Subject: Merge pull request #1025 in SNORT/snort3 from nhttp90 to master X-Git-Tag: 3.0.0-240~32 X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=f519810702bd2e69faefb03cd7b4832b074a7dff;p=thirdparty%2Fsnort3.git Merge pull request #1025 in SNORT/snort3 from nhttp90 to master Squashed commit of the following: commit 43ec1af54b6fc6cacf77da97e687fc6f1877f83a Author: Tom Peters Date: Thu Sep 14 13:17:29 2017 -0400 http_inspect, stream: HTTP headers no longer avoid detection when message unexpectedly ends after status line or headers --- 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 d565a019b..f53fdc141 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_finish.cc @@ -40,11 +40,19 @@ bool HttpStreamSplitter::finish(Flow* flow) return false; #ifdef REG_TEST - if (HttpTestManager::use_test_output() && !HttpTestManager::use_test_input()) + if (HttpTestManager::use_test_output()) { - printf("Finish from flow data %" PRIu64 " direction %d\n", session_data->seq_num, - source_id); - fflush(stdout); + if (HttpTestManager::use_test_input()) + { + if (!HttpTestManager::get_test_input_source()->finish()) + return false; + } + else + { + printf("Finish from flow data %" PRIu64 " direction %d\n", session_data->seq_num, + source_id); + fflush(stdout); + } } #endif @@ -82,6 +90,24 @@ bool HttpStreamSplitter::finish(Flow* flow) return true; } + // If the message has been truncated immediately following the start line or immediately + // following the headers (a body was expected) then we need to process an empty section to + // provide an inspection section. Otherwise the start line and headers won't go through + // detection. + if (((session_data->type_expected[source_id] == SEC_HEADER) || + (session_data->type_expected[source_id] == SEC_BODY_CL) || + (session_data->type_expected[source_id] == SEC_BODY_CHUNK) || + (session_data->type_expected[source_id] == SEC_BODY_OLD)) && + (session_data->cutter[source_id] == nullptr) && + (session_data->section_type[source_id] == SEC__NOT_COMPUTE)) + { + // Set up to process empty message section + uint32_t not_used; + prepare_flush(session_data, ¬_used, session_data->type_expected[source_id], 0, 0, 0, + false, 0, 0, true); + return true; + } + // If there is no more data to process we need to wrap up file processing right now if ((session_data->section_type[source_id] == SEC__NOT_COMPUTE) && (session_data->file_depth_remaining[source_id] > 0) && diff --git a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc index 8119640bf..ccec9be84 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_reassemble.cc @@ -263,6 +263,13 @@ const StreamBuffer HttpStreamSplitter::reassemble(Flow* flow, unsigned total, un } #endif + // Sometimes it is necessary to reassemble zero bytes when a connection is closing to trigger + // proper clean up. But even a zero-length buffer cannot be processed with a nullptr lest we + // get in trouble with memcpy() (undefined behavior) or some library. + assert((data != nullptr) || (len == 0)); + if (data == nullptr) + data = (const uint8_t*)""; + // FIXIT-H Workaround for TP Bug 149662 if (session_data->section_type[source_id] == SEC__NOT_COMPUTE) { 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 632854c5a..27042973d 100644 --- a/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc +++ b/src/service_inspectors/http_inspect/http_stream_splitter_scan.cc @@ -40,15 +40,17 @@ void HttpStreamSplitter::prepare_flush(HttpFlowData* session_data, uint32_t* flu session_data->octets_expected[source_id] = octets_seen + num_flushed; session_data->strict_length[source_id] = strict_length; -#ifdef REG_TEST - if (HttpTestManager::use_test_input()) + if (flush_offset != nullptr) { - HttpTestManager::get_test_input_source()->flush(num_flushed); - } - else +#ifdef REG_TEST + if (HttpTestManager::use_test_input()) + { + HttpTestManager::get_test_input_source()->flush(num_flushed); + } + else #endif - - *flush_offset = num_flushed; + *flush_offset = num_flushed; + } } HttpCutter* HttpStreamSplitter::get_cutter(SectionType type, @@ -134,10 +136,9 @@ StreamSplitter::Status HttpStreamSplitter::scan(Flow* flow, const uint8_t* data, // not support no headers. Processing this imaginary status line and empty headers allows // us to overcome this limitation and reuse the entire HTTP infrastructure. type = SEC_BODY_OLD; - uint32_t not_used; - prepare_flush(session_data, ¬_used, SEC_STATUS, 14, 0, 0, false, 0, 14, true); + prepare_flush(session_data, nullptr, SEC_STATUS, 14, 0, 0, false, 0, 14, true); my_inspector->process((const uint8_t*)"HTTP/0.9 200 .", 14, flow, SRC_SERVER, false); - prepare_flush(session_data, ¬_used, SEC_HEADER, 0, 0, 0, false, 0, 0, true); + prepare_flush(session_data, nullptr, SEC_HEADER, 0, 0, 0, false, 0, 0, true); my_inspector->process((const uint8_t*)"", 0, flow, SRC_SERVER, false); } diff --git a/src/service_inspectors/http_inspect/http_test_input.cc b/src/service_inspectors/http_inspect/http_test_input.cc index ee6873b22..5be543d4f 100644 --- a/src/service_inspectors/http_inspect/http_test_input.cc +++ b/src/service_inspectors/http_inspect/http_test_input.cc @@ -60,6 +60,7 @@ void HttpTestInput::reset() end_offset = 0; close_pending = false; close_notified = false; + finish_expected = false; need_break = false; if (include_file != nullptr) { @@ -113,7 +114,7 @@ void HttpTestInput::scan(uint8_t*& data, uint32_t& length, SourceId source_id, u // we run out of buffer space. memmove(msg_buf, msg_buf+flush_octets, end_offset); flush_octets = 0; - length = end_offset - previous_offset; + length = end_offset; return; } // If we reach here then StreamSplitter has already flushed all data read so far @@ -429,9 +430,12 @@ void HttpTestInput::reassemble(uint8_t** buffer, unsigned& length, SourceId sour // There is no more data. Clean up and notify caller about close. just_flushed = true; flushed = false; + end_offset = 0; + previous_offset = 0; close_pending = false; tcp_closed = false; tcp_close = true; + finish_expected = true; } else if (!flushed) { @@ -440,6 +444,7 @@ void HttpTestInput::reassemble(uint8_t** buffer, unsigned& length, SourceId sour previous_offset = end_offset; tcp_close = true; close_notified = true; + finish_expected = true; } else if (flush_octets == end_offset) { @@ -466,6 +471,7 @@ void HttpTestInput::reassemble(uint8_t** buffer, unsigned& length, SourceId sour flush_octets = end_offset; tcp_close = true; close_notified = true; + finish_expected = true; } return; } @@ -494,5 +500,15 @@ void HttpTestInput::reassemble(uint8_t** buffer, unsigned& length, SourceId sour flushed = false; } +bool HttpTestInput::finish() +{ + if (finish_expected) + { + finish_expected = false; + return true; + } + return false; +} + #endif diff --git a/src/service_inspectors/http_inspect/http_test_input.h b/src/service_inspectors/http_inspect/http_test_input.h index bae82de9c..6b1b2fce9 100644 --- a/src/service_inspectors/http_inspect/http_test_input.h +++ b/src/service_inspectors/http_inspect/http_test_input.h @@ -35,6 +35,7 @@ public: void flush(uint32_t num_octets); void reassemble(uint8_t** buffer, unsigned& length, HttpEnums::SourceId source_id, bool& tcp_close); + bool finish(); private: FILE* test_data_file; @@ -63,7 +64,7 @@ private: // number of octets that have been flushed and must be sent by reassemble uint32_t flush_octets = 0; - // number of characters in the buffer previously shown to PAF but not flushed yet + // number of characters in the buffer previously shown to splitter but not flushed yet uint32_t previous_offset = 0; // number of characters in the buffer @@ -75,6 +76,9 @@ private: // Close notification already provided bool close_notified = false; + // tcp_close notification given and we are waiting for a HttpStreamSplitter::finish() call. + bool finish_expected = false; + void reset(); }; diff --git a/src/stream/stream_splitter.cc b/src/stream/stream_splitter.cc index c405d07e6..cd62eb28d 100644 --- a/src/stream/stream_splitter.cc +++ b/src/stream/stream_splitter.cc @@ -40,12 +40,15 @@ const StreamBuffer StreamSplitter::reassemble( Flow*, unsigned, unsigned offset, const uint8_t* p, unsigned n, uint32_t flags, unsigned& copied) { + copied = n; + if (n == 0) + return { nullptr, 0 }; + unsigned max; uint8_t* pdu_buf = DetectionEngine::get_next_buffer(max); assert(offset + n < max); memcpy(pdu_buf+offset, p, n); - copied = n; if ( flags & PKT_PDU_TAIL ) return { pdu_buf, offset + n }; diff --git a/src/stream/tcp/tcp_reassembler.cc b/src/stream/tcp/tcp_reassembler.cc index f99356a68..f994b2621 100644 --- a/src/stream/tcp/tcp_reassembler.cc +++ b/src/stream/tcp/tcp_reassembler.cc @@ -564,6 +564,22 @@ void TcpReassembler::prep_pdu(Flow* flow, Packet* p, uint32_t pkt_flags, Packet* } } +Packet* TcpReassembler::initialize_pdu(Packet* p, uint32_t pkt_flags, struct timeval tv) +{ + DetectionEngine::onload(session->flow); + Packet* pdu = DetectionEngine::set_next_packet(); + + EncodeFlags enc_flags = 0; + DAQ_PktHdr_t pkth; + session->GetPacketHeaderFoo(&pkth, pkt_flags); + PacketManager::format_tcp(enc_flags, p, pdu, PSEUDO_PKT_TCP, &pkth, pkth.opaque); + prep_pdu(session->flow, p, pkt_flags, pdu); + ((DAQ_PktHdr_t*)pdu->pkth)->ts = tv; + pdu->dsize = 0; + pdu->data = nullptr; + return pdu; +} + int TcpReassembler::_flush_to_seq(uint32_t bytes, Packet* p, uint32_t pkt_flags) { Profile profile(s5TcpFlushPerfStats); @@ -596,27 +612,11 @@ int TcpReassembler::_flush_to_seq(uint32_t bytes, Packet* p, uint32_t pkt_flags) /* this is as much as we can pack into a stream buffer */ footprint = pdu->max_dsize; - DetectionEngine::onload(session->flow); - pdu = DetectionEngine::set_next_packet(); - - DAQ_PktHdr_t pkth; - session->GetPacketHeaderFoo(&pkth, pkt_flags); - - EncodeFlags enc_flags = 0; - PacketManager::format_tcp(enc_flags, p, pdu, PSEUDO_PKT_TCP, &pkth, pkth.opaque); - prep_pdu(session->flow, p, pkt_flags, pdu); - - ((DAQ_PktHdr_t*)pdu->pkth)->ts = seglist.next->tv; - - /* setup the pseudopacket payload */ - pdu->dsize = 0; - pdu->data = nullptr; - if ( tracker->splitter->is_paf() and ( tracker->get_tf_flags() & TF_MISSING_PREV_PKT ) ) fallback(); + Packet* pdu = initialize_pdu(p, pkt_flags, seglist.next->tv); int32_t flushed_bytes = flush_data_segments(p, footprint, pdu); - if ( flushed_bytes == 0 ) break; /* No more data... bail */ @@ -630,8 +630,7 @@ int TcpReassembler::_flush_to_seq(uint32_t bytes, Packet* p, uint32_t pkt_flags) else pdu->packet_flags |= ( PKT_REBUILT_STREAM | PKT_STREAM_EST ); - // FIXIT-H this came with merge should it be here? YES - //pdu->application_protocol_ordinal = p->application_protocol_ordinal; + pdu->set_application_protocol(p->get_application_protocol()); show_rebuilt_packet(pdu); tcpStats.rebuilt_packets++; tcpStats.rebuilt_bytes += flushed_bytes; @@ -708,6 +707,34 @@ int TcpReassembler::flush_to_seq(uint32_t bytes, Packet* p, uint32_t pkt_flags) return _flush_to_seq(bytes, p, pkt_flags); } +// flush a seglist up to the given point, generate a pseudopacket, and fire it thru the system. +int TcpReassembler::do_zero_byte_flush(Packet* p, uint32_t pkt_flags) +{ + unsigned bytes_copied = 0; + + const StreamBuffer sb = tracker->splitter->reassemble(session->flow, 0, 0, nullptr, 0, + (PKT_PDU_HEAD | PKT_PDU_TAIL), bytes_copied); + + if ( sb.data ) + { + Packet* pdu = initialize_pdu(p, pkt_flags, ((DAQ_PktHdr_t*)p->pkth)->ts); + /* setup the pseudopacket payload */ + pdu->data = sb.data; + pdu->dsize = sb.length; + pdu->packet_flags |= ( PKT_REBUILT_STREAM | PKT_STREAM_EST | PKT_PDU_HEAD | PKT_PDU_TAIL ); + pdu->set_application_protocol(p->get_application_protocol()); + flush_count++; + + show_rebuilt_packet(pdu); + ProfileExclude profile_exclude(s5TcpFlushPerfStats); + Snort::inspect(pdu); + if ( tracker->splitter ) + tracker->splitter->update(); + } + + return bytes_copied; +} + // get the footprint for the current seglist, the difference // between our base sequence and the last ack'd sequence we received @@ -766,7 +793,7 @@ uint32_t TcpReassembler::get_q_sequenced() // FIXIT-L flush_stream() calls should be replaced with calls to // CheckFlushPolicyOn*() with the exception that for the *OnAck() case, // any available ackd data must be flushed in both directions. -int TcpReassembler::flush_stream(Packet* p, uint32_t dir) +int TcpReassembler::flush_stream(Packet* p, uint32_t dir, bool final_flush) { // this is not always redundant; stream_reassemble rule option causes trouble if ( !tracker->flush_policy or !tracker->splitter ) @@ -779,14 +806,19 @@ int TcpReassembler::flush_stream(Packet* p, uint32_t dir) else bytes = get_q_footprint( ); - return flush_to_seq(bytes, p, dir); + if ( bytes ) + return flush_to_seq(bytes, p, dir); + else if( final_flush ) + return do_zero_byte_flush(p, dir); + + return 0; } void TcpReassembler::final_flush(Packet* p, uint32_t dir) { tracker->set_tf_flags(TF_FORCE_FLUSH); - if ( flush_stream(p, dir) ) + if ( flush_stream(p, dir, true) ) purge_flushed_ackd( ); tracker->clear_tf_flags(TF_FORCE_FLUSH); @@ -823,8 +855,6 @@ static Packet* set_packet(Flow* flow, uint32_t flags, bool c2s) void TcpReassembler::flush_queued_segments(Flow* flow, bool clear, Packet* p) { - bool data = p or seglist.head; - if ( !p ) { // this packet is required if we call finish and/or final_flush @@ -839,7 +869,7 @@ void TcpReassembler::flush_queued_segments(Flow* flow, bool clear, Packet* p) bool pending = clear and paf_initialized(&tracker->paf_state) and (!tracker->splitter or tracker->splitter->finish(flow) ); - if ( pending and data and !(flow->ssn_state.ignore_direction & ignore_dir) ) + if ( pending and !(flow->ssn_state.ignore_direction & ignore_dir) ) { final_flush(p, packet_dir); } diff --git a/src/stream/tcp/tcp_reassembler.h b/src/stream/tcp/tcp_reassembler.h index 5bbf6da55..c728c6c25 100644 --- a/src/stream/tcp/tcp_reassembler.h +++ b/src/stream/tcp/tcp_reassembler.h @@ -35,7 +35,7 @@ public: virtual int queue_packet_for_reassembly(TcpSegmentDescriptor&); virtual void purge_segment_list(); - virtual int flush_stream(Packet* p, uint32_t dir); + virtual int flush_stream(Packet* p, uint32_t dir, bool final_flush = false); virtual int purge_flushed_ackd(); virtual void flush_queued_segments(Flow* flow, bool clear, Packet* p = nullptr); virtual bool is_segment_pending_flush(); @@ -143,8 +143,10 @@ protected: uint32_t get_flush_data_len(TcpSegmentNode*, uint32_t to_seq, unsigned max); int flush_data_segments(Packet*, uint32_t total, Packet* pdu); void prep_pdu(Flow*, Packet*, uint32_t pkt_flags, Packet* pdu); + Packet* initialize_pdu(Packet* p, uint32_t pkt_flags, struct timeval tv); int _flush_to_seq(uint32_t bytes, Packet*, uint32_t pkt_flags); int flush_to_seq(uint32_t bytes, Packet*, uint32_t pkt_flags); + int do_zero_byte_flush(Packet* p, uint32_t pkt_flags); uint32_t get_q_footprint(); uint32_t get_q_sequenced(); void final_flush(Packet*, uint32_t dir); diff --git a/src/stream/tcp/tcp_session.cc b/src/stream/tcp/tcp_session.cc index 84aa0a5a0..8cd632437 100644 --- a/src/stream/tcp/tcp_session.cc +++ b/src/stream/tcp/tcp_session.cc @@ -1135,6 +1135,9 @@ void TcpSession::flush() if ( ( server->reassembler->is_segment_pending_flush() ) || (client->reassembler->is_segment_pending_flush() ) ) { + // FIXIT-L flush_queued_segments is basically a noop when the 'clear' parameter + // is passed in as false... review what happens in 2.9.x probably related + // to ssl encryption support server->reassembler->flush_queued_segments(flow, false); client->reassembler->flush_queued_segments(flow, false); }