From: Steve Chew (stechew) Date: Fri, 22 Jan 2021 22:33:24 +0000 (+0000) Subject: Merge pull request #2689 in SNORT/snort3 from ~SBAIGAL/snort3:ftps_eof to master X-Git-Tag: 3.1.1.0~9 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=7a04d720dd7fbd7a1563399af87f9bec9e16b952;p=thirdparty%2Fsnort3.git Merge pull request #2689 in SNORT/snort3 from ~SBAIGAL/snort3:ftps_eof to master Squashed commit of the following: commit bf862aa1e46a75147da1332d0f343faed2b273d6 Author: Steven Baigal (sbaigal) Date: Tue Dec 15 13:09:53 2020 -0500 ftp: using hold_packet to handle ftp-data eof --- diff --git a/src/service_inspectors/ftp_telnet/ftp_data.cc b/src/service_inspectors/ftp_telnet/ftp_data.cc index 52bc27c30..a12502075 100644 --- a/src/service_inspectors/ftp_telnet/ftp_data.cc +++ b/src/service_inspectors/ftp_telnet/ftp_data.cc @@ -239,47 +239,16 @@ void FtpDataFlowData::handle_expected(Packet* p) } } -void FtpDataFlowData::handle_retransmit(Packet* p) -{ - FTP_DATA_SESSION* data_ssn = &session; - - if ((data_ssn->eof_seq > 0) and (data_ssn->eof_seq == p->ptrs.tcph->seq() + p->dsize)) - { - // only process the final data segment - initFilePosition(&data_ssn->position, get_file_processed_size(p->flow)); - finalFilePosition(&data_ssn->position); - - FileFlows* file_flows = FileFlows::get_file_flows(p->flow); - if (file_flows) - { - file_flows->file_process(DetectionEngine::get_current_packet(), - p->data, 0, SNORT_FILE_END, data_ssn->direction, data_ssn->path_hash); - - eof_handled = true; - } - } -} - void FtpDataFlowData::handle_eof(Packet* p) { FTP_DATA_SESSION* data_ssn = &session; - data_ssn->eof_seq = 0; if (!PROTO_IS_FTP_DATA(data_ssn) || !FTPDataDirection(p, data_ssn)) return; - if (p->dsize != 0) - { - initFilePosition(&data_ssn->position, get_file_processed_size(p->flow)); - finalFilePosition(&data_ssn->position); - eof_handled = true; - } - else - { - Active* act = p->active; - act->set_delayed_action(Active::ACT_RETRY, true); - data_ssn->eof_seq = p->ptrs.tcph->seq(); - } + initFilePosition(&data_ssn->position, get_file_processed_size(p->flow)); + finalFilePosition(&data_ssn->position); + eof_handled = true; if (data_ssn->mss_changed) ftstats.total_sessions_mss_changed++; diff --git a/src/service_inspectors/ftp_telnet/ftpdata_splitter.cc b/src/service_inspectors/ftp_telnet/ftpdata_splitter.cc index 8414d8fb4..00d6ff21a 100644 --- a/src/service_inspectors/ftp_telnet/ftpdata_splitter.cc +++ b/src/service_inspectors/ftp_telnet/ftpdata_splitter.cc @@ -39,29 +39,18 @@ void FtpDataSplitter::restart_scan() bytes = segs = 0; } -static void set_ftp_flush_flag(Flow* flow) -{ - FtpDataFlowData* fdfd = (FtpDataFlowData*)flow->get_flow_data(FtpDataFlowData::inspector_id); - if ( fdfd ) - fdfd->session.packet_flags |= FTPDATA_FLG_FLUSH; -} - StreamSplitter::Status FtpDataSplitter::scan(Packet* pkt, const uint8_t*, uint32_t len, uint32_t, uint32_t* fp) { Flow* flow = pkt->flow; assert(flow); - FtpDataFlowData* fdfd = (FtpDataFlowData*)flow->get_flow_data(FtpDataFlowData::inspector_id); - if (!fdfd) - return SEARCH; + FtpDataFlowData* fdfd = nullptr; + if ( len ) { if(expected_seg_size == 0) { - // FIXIT-M: Can we do better than this guess if no MSS is specified? - // Malware detection won't work if expected_seg_size doesn't match - // the payload lengths on packets before the last packet. expected_seg_size = 1448; if(flow->session and flow->pkt_type == PktType::TCP) @@ -72,43 +61,40 @@ StreamSplitter::Status FtpDataSplitter::scan(Packet* pkt, const uint8_t*, uint32 expected_seg_size -= tcp_options_len; } } - + segs++; + bytes += len; if ( len != expected_seg_size ) { + fdfd = (FtpDataFlowData*)flow->get_flow_data(FtpDataFlowData::inspector_id); + if (!fdfd) + return SEARCH; + ftstats.total_packets_mss_changed++; fdfd->session.mss_changed = true; - if (fdfd->session.bytes_seen == 0) - { - // Segmented pkt is smaller than expected_seg_size - set_ftp_flush_flag(flow); - } - else if (pkt->ptrs.tcph and !pkt->ptrs.tcph->is_fin()) + expected_seg_size = len; + + if (pkt->ptrs.tcph and !pkt->ptrs.tcph->is_fin()) { - Active* act = pkt->active; - // add packet to retry queue to consider this is not end of ftp flow - if (!(pkt->flow->flags.trigger_detained_packet_event)) - act->set_delayed_action(Active::ACT_RETRY, true); + // set flag for signature calculation in case this is the last packet + fdfd->session.packet_flags |= FTPDATA_FLG_FLUSH; + pkt->active->hold_packet(pkt); + return SEARCH; } - expected_seg_size = len; - restart_scan(); - *fp = len; - fdfd->session.bytes_seen += len; - return FLUSH; - } - else - { - segs++; - bytes += len; } - fdfd->session.bytes_seen += len; + } - if ((segs >= 2 and bytes >= min) or (pkt->ptrs.tcph and pkt->ptrs.tcph->is_fin())) - { - // Either FIN or smaller size do FLUSH to continue inspection - restart_scan(); - *fp = len; - return FLUSH; - } + if ((segs >= 2 and bytes >= min) or (pkt->ptrs.tcph and pkt->ptrs.tcph->is_fin())) + { + fdfd = (FtpDataFlowData*)flow->get_flow_data(FtpDataFlowData::inspector_id); + if (!fdfd) + return SEARCH; + + restart_scan(); + *fp = len; + // avoid unnecessary signature calc by clearing the flag set by detained packet + if (fdfd->session.packet_flags & FTPDATA_FLG_FLUSH) + fdfd->session.packet_flags &= ~FTPDATA_FLG_FLUSH; + return FLUSH; } return SEARCH; diff --git a/src/service_inspectors/ftp_telnet/ftpp_si.h b/src/service_inspectors/ftp_telnet/ftpp_si.h index ac1d77831..e792ba75f 100644 --- a/src/service_inspectors/ftp_telnet/ftpp_si.h +++ b/src/service_inspectors/ftp_telnet/ftpp_si.h @@ -212,12 +212,10 @@ struct FTP_DATA_SESSION FTP_TELNET_SESSION ft_ssn; snort::FlowKey ftp_key; char* filename; - uint32_t eof_seq; size_t path_hash; int data_chan; int file_xfer_info; FilePosition position; - uint32_t bytes_seen; unsigned char mode; unsigned char packet_flags; bool direction; @@ -235,7 +233,6 @@ public: void handle_expected(snort::Packet*) override; void handle_eof(snort::Packet*) override; - void handle_retransmit(snort::Packet*) override; size_t size_of() override { return sizeof(*this); }